From f0ad1b02de6f54c95920fc1044cd75afe707a383 Mon Sep 17 00:00:00 2001 From: Trubnikov Sergey Date: Mon, 24 Apr 2023 13:02:43 +0000 Subject: [PATCH] refac: incapsulate CollectionHandler into CollectionDispatch --- --- a/pallets/balances-adapter/src/lib.rs +++ b/pallets/balances-adapter/src/lib.rs @@ -3,6 +3,7 @@ #![warn(missing_docs)] extern crate alloc; +use frame_support::sp_runtime::DispatchResult; pub use pallet::*; use pallet_common::CollectionHandle; use pallet_evm_coder_substrate::{WithRecorder, SubstrateRecorder}; @@ -10,24 +11,23 @@ pub mod common; pub mod erc; -pub struct NativeFungibleHandle(CollectionHandle); +pub struct NativeFungibleHandle(SubstrateRecorder); impl NativeFungibleHandle { - pub fn cast(inner: CollectionHandle) -> Self { - Self(inner) + pub fn new() -> NativeFungibleHandle { + Self(SubstrateRecorder::new(u64::MAX)) } - /// Casts [`NativeFungibleHandle`] into [`CollectionHandle`][`pallet_common::CollectionHandle`]. - pub fn into_inner(self) -> pallet_common::CollectionHandle { - self.0 + pub fn check_is_internal(&self) -> DispatchResult { + Ok(()) } } impl WithRecorder for NativeFungibleHandle { fn recorder(&self) -> &pallet_evm_coder_substrate::SubstrateRecorder { - &self.0.recorder + &self.0 } fn into_recorder(self) -> pallet_evm_coder_substrate::SubstrateRecorder { - self.0.recorder + self.0 } } #[frame_support::pallet] --- a/pallets/common/src/dispatch.rs +++ b/pallets/common/src/dispatch.rs @@ -34,16 +34,11 @@ collection: CollectionId, call: C, ) -> DispatchResultWithPostInfo { - let handle = - CollectionHandle::try_get(collection).map_err(|error| DispatchErrorWithPostInfo { - post_info: PostDispatchInfo { - actual_weight: Some(dispatch_weight::()), - pays_fee: Pays::Yes, - }, - error, - })?; - handle - .check_is_internal() + let dispatched = T::CollectionDispatch::dispatch(collection) + .and_then(|dispatched| { + dispatched.check_is_internal()?; + Ok(dispatched) + }) .map_err(|error| DispatchErrorWithPostInfo { post_info: PostDispatchInfo { actual_weight: Some(dispatch_weight::()), @@ -51,7 +46,6 @@ }, error, })?; - let dispatched = T::CollectionDispatch::dispatch(handle); let mut result = call(dispatched.as_dyn()); match &mut result { Ok(PostDispatchInfo { @@ -72,6 +66,8 @@ /// Interface for working with different collections through the dispatcher. pub trait CollectionDispatch { + fn check_is_internal(&self) -> DispatchResult; + /// Create a collection. The collection will be created according to the value of [`data.mode`](CreateCollectionData::mode). /// /// * `sender` - The user who will become the owner of the collection. @@ -92,7 +88,9 @@ /// Get a specialized collection from the handle. /// /// * `handle` - Collection handle. - fn dispatch(handle: CollectionHandle) -> Self; + fn dispatch(collection_id: CollectionId) -> Result + where + Self: Sized; /// Get the implementation of [`CommonCollectionOperations`]. fn as_dyn(&self) -> &dyn CommonCollectionOperations; --- a/pallets/common/src/erc.rs +++ b/pallets/common/src/erc.rs @@ -77,6 +77,14 @@ fn call(self, handle: &mut impl PrecompileHandle) -> Option; } +impl CommonEvmHandler for () { + const CODE: &'static [u8] = &[]; + + fn call(self, handle: &mut impl PrecompileHandle) -> Option { + None + } +} + /// @title A contract that allows you to work with collections. #[solidity_interface(name = Collection, enum(derive(PreDispatch)), enum_attr(weight))] impl CollectionHandle --- a/pallets/structure/src/benchmarking.rs +++ b/pallets/structure/src/benchmarking.rs @@ -42,7 +42,7 @@ }, CollectionFlags::default(), )?; - let dispatch = T::CollectionDispatch::dispatch(CollectionHandle::try_get(CollectionId(1))?); + let dispatch = T::CollectionDispatch::dispatch(CollectionId(1))?; let dispatch = dispatch.as_dyn(); dispatch.create_item(caller_cross.clone(), caller_cross.clone(), CreateItemData::NFT(CreateNftData::default()), &Unlimited)?; --- a/pallets/structure/src/lib.rs +++ b/pallets/structure/src/lib.rs @@ -155,11 +155,10 @@ token: TokenId, ) -> Result, DispatchError> { // TODO: Reduce cost by not reading collection config - let handle = match CollectionHandle::try_get(collection) { + let handle = match T::CollectionDispatch::dispatch(collection) { Ok(v) => v, Err(_) => return Ok(Parent::TokenNotFound), }; - let handle = T::CollectionDispatch::dispatch(handle); let handle = handle.as_dyn(); Ok(match handle.token_owner(token) { @@ -279,8 +278,7 @@ self_budget: &dyn Budget, breadth_budget: &dyn Budget, ) -> DispatchResultWithPostInfo { - let handle = >::try_get(collection)?; - let dispatch = T::CollectionDispatch::dispatch(handle); + let dispatch = T::CollectionDispatch::dispatch(collection)?; let dispatch = dispatch.as_dyn(); dispatch.burn_item_recursively(from.clone(), token, self_budget, breadth_budget) } @@ -404,10 +402,8 @@ let Some((collection, token)) = T::CrossTokenAddressMapping::address_to_token(account) else { return Ok(()) }; - - let handle = >::try_get(collection)?; - let dispatch = T::CollectionDispatch::dispatch(handle); + let dispatch = T::CollectionDispatch::dispatch(collection)?; let dispatch = dispatch.as_dyn(); action(dispatch, token) --- a/runtime/common/dispatch.rs +++ b/runtime/common/dispatch.rs @@ -50,6 +50,7 @@ Refungible(RefungibleHandle), NativeFungible(NativeFungibleHandle), } + impl CollectionDispatch for CollectionDispatchT where T: pallet_common::Config @@ -59,6 +60,15 @@ + pallet_refungible::Config + pallet_balances_adapter::Config, { + fn check_is_internal(&self) -> DispatchResult { + match self { + Self::Fungible(h) => h.check_is_internal(), + Self::Nonfungible(h) => h.check_is_internal(), + Self::Refungible(h) => h.check_is_internal(), + Self::NativeFungible(h) => h.check_is_internal(), + } + } + fn create( sender: T::CrossAccountId, payer: T::CrossAccountId, @@ -104,18 +114,17 @@ Ok(()) } - fn dispatch(handle: CollectionHandle) -> Self { - match handle.mode { - CollectionMode::Fungible(_) => { - if handle.id != up_data_structs::CollectionId(0) { - Self::Fungible(FungibleHandle::cast(handle)) - } else { - Self::NativeFungible(NativeFungibleHandle::cast(handle)) - } - } + fn dispatch(collection_id: CollectionId) -> Result { + if collection_id == CollectionId(0) { + return Ok(Self::NativeFungible(NativeFungibleHandle::new())); + } + + let handle = >::try_get(collection_id)?; + Ok(match handle.mode { + CollectionMode::Fungible(_) => Self::Fungible(FungibleHandle::cast(handle)), CollectionMode::NFT => Self::Nonfungible(NonfungibleHandle::cast(handle)), CollectionMode::ReFungible => Self::Refungible(RefungibleHandle::cast(handle)), - } + }) } fn as_dyn(&self) -> &dyn CommonCollectionOperations { @@ -172,15 +181,19 @@ } fn call(handle: &mut impl PrecompileHandle) -> Option { if let Some(collection_id) = map_eth_to_id(&handle.code_address()) { - let collection = - >::new_with_gas_limit(collection_id, handle.remaining_gas())?; - let dispatched = Self::dispatch(collection); + if collection_id == CollectionId(0) { + >::new().call(handle) + } else { + let collection = >::new_with_gas_limit( + collection_id, + handle.remaining_gas(), + )?; - match dispatched { - Self::Fungible(h) => h.call(handle), - Self::Nonfungible(h) => h.call(handle), - Self::Refungible(h) => h.call(handle), - Self::NativeFungible(h) => h.call(handle), + match collection.mode { + CollectionMode::Fungible(_) => FungibleHandle::cast(collection).call(handle), + CollectionMode::NFT => NonfungibleHandle::cast(collection).call(handle), + CollectionMode::ReFungible => RefungibleHandle::cast(collection).call(handle), + } } } else if let Some((collection_id, token_id)) = ::EvmTokenAddressMapping::address_to_token( --- a/runtime/common/runtime_apis.rs +++ b/runtime/common/runtime_apis.rs @@ -17,7 +17,7 @@ #[macro_export] macro_rules! dispatch_unique_runtime { ($collection:ident.$method:ident($($name:ident),*) $($rest:tt)*) => {{ - let collection = ::CollectionDispatch::dispatch(>::try_get($collection)?); + let collection = ::CollectionDispatch::dispatch($collection)?; let dispatch = collection.as_dyn(); Ok::<_, DispatchError>(dispatch.$method($($name),*) $($rest)*) --- a/tests/src/eth/fungible.test.ts +++ b/tests/src/eth/fungible.test.ts @@ -33,7 +33,7 @@ 'substrate' as const, 'ethereum' as const, ].map(testCase => { - itEth.only(`Can perform mintCross() for ${testCase} address`, async ({helper}) => { + itEth(`Can perform mintCross() for ${testCase} address`, async ({helper}) => { // 1. Create receiver depending on the test case: const receiverEth = helper.eth.createAccount(); const receiverCrossEth = helper.ethCrossAccount.fromAddress(receiverEth); --- a/tests/src/eth/nativeFungible.test.ts +++ b/tests/src/eth/nativeFungible.test.ts @@ -29,7 +29,7 @@ }); }); - itEth.only('Can perform approve()', async ({helper}) => { + itEth.skip('Can perform approve()', async ({helper}) => { const owner = await helper.eth.createAccountWithBalance(donor); const spender = helper.eth.createAccount(); const collection = await helper.ft.mintCollection(alice); --- a/tests/src/eth/util/playgrounds/types.ts +++ b/tests/src/eth/util/playgrounds/types.ts @@ -48,3 +48,5 @@ field: CollectionLimitField, value: OptionUint, } + +export const NON_EXISTENT_COLLECTION_ID = 4_294_967_295; \ No newline at end of file --- a/tests/src/pallet-presence.test.ts +++ b/tests/src/pallet-presence.test.ts @@ -19,6 +19,7 @@ // Pallets that must always be present const requiredPallets = [ 'balances', + 'balancesadapter', 'common', 'timestamp', 'transactionpayment', --- a/tests/src/transfer.test.ts +++ b/tests/src/transfer.test.ts @@ -17,6 +17,7 @@ import {IKeyringPair} from '@polkadot/types/types'; import {itEth, usingEthPlaygrounds} from './eth/util'; import {itSub, Pallets, usingPlaygrounds, expect} from './util'; +import {NON_EXISTENT_COLLECTION_ID} from './eth/util/playgrounds/types'; describe('Integration Test Transfer(recipient, collection_id, item_id, value)', () => { let donor: IKeyringPair; @@ -124,20 +125,17 @@ itSub('[nft] Transfer with not existed collection_id', async ({helper}) => { - const collectionId = (1 << 32) - 1; - await expect(helper.nft.transferToken(alice, collectionId, 1, {Substrate: bob.address})) + await expect(helper.nft.transferToken(alice, NON_EXISTENT_COLLECTION_ID, 1, {Substrate: bob.address})) .to.be.rejectedWith(/common\.CollectionNotFound/); }); itSub('[fungible] Transfer with not existed collection_id', async ({helper}) => { - const collectionId = (1 << 32) - 1; - await expect(helper.ft.transfer(alice, collectionId, {Substrate: bob.address})) + await expect(helper.ft.transfer(alice, NON_EXISTENT_COLLECTION_ID, {Substrate: bob.address})) .to.be.rejectedWith(/common\.CollectionNotFound/); }); itSub.ifWithPallets('[refungible] Transfer with not existed collection_id', [Pallets.ReFungible], async ({helper}) => { - const collectionId = (1 << 32) - 1; - await expect(helper.rft.transferToken(alice, collectionId, 1, {Substrate: bob.address})) + await expect(helper.rft.transferToken(alice, NON_EXISTENT_COLLECTION_ID, 1, {Substrate: bob.address})) .to.be.rejectedWith(/common\.CollectionNotFound/); }); --- a/tests/src/transferFrom.test.ts +++ b/tests/src/transferFrom.test.ts @@ -16,6 +16,7 @@ import {IKeyringPair} from '@polkadot/types/types'; import {itSub, Pallets, usingPlaygrounds, expect} from './util'; +import {NON_EXISTENT_COLLECTION_ID} from './eth/util/playgrounds/types'; describe('Integration Test transferFrom(from, recipient, collection_id, item_id, value):', () => { let alice: IKeyringPair; @@ -97,10 +98,9 @@ }); itSub('transferFrom for a collection that does not exist', async ({helper}) => { - const collectionId = (1 << 32) - 1; - await expect(helper.collection.approveToken(alice, collectionId, 0, {Substrate: bob.address}, 1n)) + await expect(helper.collection.approveToken(alice, NON_EXISTENT_COLLECTION_ID, 0, {Substrate: bob.address}, 1n)) .to.be.rejectedWith(/common\.CollectionNotFound/); - await expect(helper.collection.transferTokenFrom(bob, collectionId, 0, {Substrate: alice.address}, {Substrate: bob.address}, 1n)) + await expect(helper.collection.transferTokenFrom(bob, NON_EXISTENT_COLLECTION_ID, 0, {Substrate: alice.address}, {Substrate: bob.address}, 1n)) .to.be.rejectedWith(/common\.CollectionNotFound/); }); -- gitstuff