From 1ab58ff1ca0b3dcfc4ea5303b55ce77e7d6b38cc Mon Sep 17 00:00:00 2001 From: Kai <7630809+Kailai-Wang@users.noreply.github.com> Date: Tue, 15 Oct 2024 09:31:39 +0200 Subject: [PATCH] add tests (#3127) --- parachain/pallets/omni-account/src/lib.rs | 84 +++---- parachain/pallets/omni-account/src/tests.rs | 254 ++++++++++++++++---- 2 files changed, 249 insertions(+), 89 deletions(-) diff --git a/parachain/pallets/omni-account/src/lib.rs b/parachain/pallets/omni-account/src/lib.rs index aa61609ee0..8b16863eca 100644 --- a/parachain/pallets/omni-account/src/lib.rs +++ b/parachain/pallets/omni-account/src/lib.rs @@ -125,6 +125,8 @@ pub mod pallet { #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { + /// An account store is created + AccountStoreCreated { who: T::AccountId, account_store_hash: H256 }, /// Some member account is added AccountAdded { who: T::AccountId, member_account_hash: H256 }, /// Some member accounts are removed @@ -193,38 +195,62 @@ pub mod pallet { #[pallet::call_index(2)] #[pallet::weight((195_000_000, DispatchClass::Normal))] + pub fn create_account_store(origin: OriginFor, identity: Identity) -> DispatchResult { + // initial creation request has to come from `TEECallOrigin` + let _ = T::TEECallOrigin::ensure_origin(origin)?; + let hash = identity.hash(); + let omni_account = T::OmniAccountConverter::convert(&identity); + + ensure!(!MemberAccountHash::::contains_key(hash), Error::::AccountAlreadyAdded); + + let mut member_accounts: MemberAccounts = BoundedVec::new(); + member_accounts + .try_push(identity.into()) + .map_err(|_| Error::::AccountStoreLenLimitReached)?; + + MemberAccountHash::::insert(hash, omni_account.clone()); + AccountStore::::insert(omni_account.clone(), member_accounts.clone()); + AccountStoreHash::::insert(omni_account.clone(), member_accounts.hash()); + + Self::deposit_event(Event::AccountStoreCreated { + who: omni_account, + account_store_hash: member_accounts.hash(), + }); + + Ok(()) + } + + #[pallet::call_index(3)] + #[pallet::weight((195_000_000, DispatchClass::Normal))] pub fn add_account( origin: OriginFor, - who: Identity, // `Identity` used to derive OmniAccount member_account: MemberAccount, // account to be added ) -> DispatchResult { - // We can't use `T::OmniAccountOrigin` here as the ownership of member account needs to - // be firstly validated by the TEE-worker before dispatching the extrinsic - let _ = T::TEECallOrigin::ensure_origin(origin)?; + // mutation of AccountStore requires `OmniAccountOrigin`, same as "remove" and "publicize" + let who = T::OmniAccountOrigin::ensure_origin(origin)?; ensure!( !MemberAccountHash::::contains_key(member_account.hash()), Error::::AccountAlreadyAdded ); + + let mut member_accounts = + AccountStore::::get(&who).ok_or(Error::::UnknownAccountStore)?; + let hash = member_account.hash(); - let mut store = Self::get_or_create_account_store(who.clone())?; - store + member_accounts .try_push(member_account) .map_err(|_| Error::::AccountStoreLenLimitReached)?; - let who_account = T::OmniAccountConverter::convert(&who); - MemberAccountHash::::insert(hash, who_account.clone()); - AccountStoreHash::::insert(who_account.clone(), store.hash()); - AccountStore::::insert(who_account.clone(), store); + MemberAccountHash::::insert(hash, who.clone()); + AccountStore::::insert(who.clone(), member_accounts.clone()); + AccountStoreHash::::insert(who.clone(), member_accounts.hash()); - Self::deposit_event(Event::AccountAdded { - who: who_account, - member_account_hash: hash, - }); + Self::deposit_event(Event::AccountAdded { who, member_account_hash: hash }); Ok(()) } - #[pallet::call_index(3)] + #[pallet::call_index(4)] #[pallet::weight((195_000_000, DispatchClass::Normal))] pub fn remove_accounts( origin: OriginFor, @@ -260,7 +286,7 @@ pub mod pallet { /// make a member account public in the AccountStore /// we force `Identity` type to avoid misuse and additional check - #[pallet::call_index(4)] + #[pallet::call_index(5)] #[pallet::weight((195_000_000, DispatchClass::Normal))] pub fn publicize_account(origin: OriginFor, member_account: Identity) -> DispatchResult { let who = T::OmniAccountOrigin::ensure_origin(origin)?; @@ -281,32 +307,6 @@ pub mod pallet { Ok(()) } } - - impl Pallet { - pub fn get_or_create_account_store(who: Identity) -> Result, Error> { - match AccountStore::::get(T::OmniAccountConverter::convert(&who)) { - Some(member_accounts) => Ok(member_accounts), - None => Self::create_account_store(who), - } - } - - fn create_account_store(who: Identity) -> Result, Error> { - let hash = who.hash(); - let who_account = T::OmniAccountConverter::convert(&who); - - ensure!(!MemberAccountHash::::contains_key(hash), Error::::AccountAlreadyAdded); - - let mut member_accounts: MemberAccounts = BoundedVec::new(); - member_accounts - .try_push(who.into()) - .map_err(|_| Error::::AccountStoreLenLimitReached)?; - - MemberAccountHash::::insert(hash, who_account.clone()); - AccountStore::::insert(who_account, member_accounts.clone()); - - Ok(member_accounts) - } - } } pub struct EnsureOmniAccount(PhantomData); diff --git a/parachain/pallets/omni-account/src/tests.rs b/parachain/pallets/omni-account/src/tests.rs index f2213e0218..1ae090c741 100644 --- a/parachain/pallets/omni-account/src/tests.rs +++ b/parachain/pallets/omni-account/src/tests.rs @@ -21,6 +21,11 @@ use sp_core::hashing::blake2_256; use sp_runtime::{traits::BadOrigin, ModuleError}; use sp_std::vec; +fn add_account_call(account: MemberAccount) -> Box { + let call = RuntimeCall::OmniAccount(crate::Call::add_account { member_account: account }); + Box::new(call) +} + fn remove_accounts_call(hashes: Vec) -> Box { let call = RuntimeCall::OmniAccount(crate::Call::remove_accounts { member_account_hashes: hashes }); @@ -37,6 +42,62 @@ fn make_balance_transfer_call(dest: AccountId, value: Balance) -> Box = + vec![MemberAccount::Public(who_identity.clone())].try_into().unwrap(); + + System::assert_last_event( + Event::AccountStoreCreated { + who: who_omni_account, + account_store_hash: member_accounts.hash(), + } + .into(), + ); + + // create it the second time will fail + assert_noop!( + OmniAccount::create_account_store(RuntimeOrigin::signed(tee_signer), who_identity,), + Error::::AccountAlreadyAdded + ); + }); +} + +#[test] +fn add_account_without_creating_store_fails() { + new_test_ext().execute_with(|| { + let tee_signer = get_tee_signer(); + + let who = alice(); + let who_identity = Identity::from(who.clone()); + + let bob_member_account = + MemberAccount::Private(bob().encode(), Identity::from(bob()).hash()); + + let call = add_account_call(bob_member_account.clone()); + assert_noop!( + OmniAccount::dispatch_as_omni_account( + RuntimeOrigin::signed(tee_signer.clone()), + who_identity.hash(), + call + ), + Error::::AccountNotFound + ); + }); +} + #[test] fn add_account_works() { new_test_ext().execute_with(|| { @@ -63,12 +124,19 @@ fn add_account_works() { .encode(), )); - assert_ok!(OmniAccount::add_account( + assert_ok!(OmniAccount::create_account_store( RuntimeOrigin::signed(tee_signer.clone()), who_identity.clone(), - bob_member_account.clone(), )); - System::assert_last_event( + + let call = add_account_call(bob_member_account.clone()); + assert_ok!(OmniAccount::dispatch_as_omni_account( + RuntimeOrigin::signed(tee_signer.clone()), + who_identity.hash(), + call + )); + + System::assert_has_event( Event::AccountAdded { who: who_omni_account.clone(), member_account_hash: bob_member_account.hash(), @@ -85,12 +153,14 @@ fn add_account_works() { expected_account_store_hash ); - assert_ok!(OmniAccount::add_account( - RuntimeOrigin::signed(tee_signer), - who_identity.clone(), - charlie_member_account.clone(), + let call = add_account_call(charlie_member_account.clone()); + assert_ok!(OmniAccount::dispatch_as_omni_account( + RuntimeOrigin::signed(tee_signer.clone()), + who_identity.hash(), + call )); - System::assert_last_event( + + System::assert_has_event( Event::AccountAdded { who: who_identity.to_omni_account(), member_account_hash: charlie_member_account.hash(), @@ -129,50 +199,89 @@ fn add_account_works() { #[test] fn add_account_origin_check_works() { new_test_ext().execute_with(|| { + let tee_signer = get_tee_signer(); let who = Identity::from(alice()); let member_account = MemberAccount::Private(vec![1, 2, 3], H256::from(blake2_256(&[1, 2, 3]))); assert_noop!( - OmniAccount::add_account(RuntimeOrigin::signed(bob()), who, member_account), + OmniAccount::add_account(RuntimeOrigin::signed(tee_signer), member_account.clone()), + BadOrigin + ); + + assert_noop!( + OmniAccount::add_account(RuntimeOrigin::signed(who.to_omni_account()), member_account), BadOrigin ); }); } #[test] -fn add_account_already_linked_works() { +fn add_account_with_already_linked_account_fails() { new_test_ext().execute_with(|| { let tee_signer = get_tee_signer(); let who = alice(); let who_identity = Identity::from(who.clone()); + let who_omni_account = who_identity.to_omni_account(); let member_account = MemberAccount::Public(Identity::from(bob())); - assert_ok!(OmniAccount::add_account( + assert_ok!(OmniAccount::create_account_store( RuntimeOrigin::signed(tee_signer.clone()), who_identity.clone(), - member_account.clone(), )); - assert_noop!( - OmniAccount::add_account( - RuntimeOrigin::signed(tee_signer.clone()), - who_identity.clone(), - member_account, - ), - Error::::AccountAlreadyAdded + + let call = add_account_call(member_account.clone()); + assert_ok!(OmniAccount::dispatch_as_omni_account( + RuntimeOrigin::signed(tee_signer.clone()), + who_identity.hash(), + call.clone() + )); + + assert_ok!(OmniAccount::dispatch_as_omni_account( + RuntimeOrigin::signed(tee_signer.clone()), + who_identity.hash(), + call + )); + + System::assert_has_event( + Event::DispatchedAsOmniAccount { + who: who_omni_account, + result: Err(DispatchError::Module(ModuleError { + index: 5, + error: [0, 0, 0, 0], + message: Some("AccountAlreadyAdded"), + })), + } + .into(), ); // intent to create a new AccountStore with an account that is already added - let who = Identity::from(bob()); + let who = Identity::from(charlie()); let alice_member_account = MemberAccount::Public(Identity::from(alice())); - assert_noop!( - OmniAccount::add_account( - RuntimeOrigin::signed(tee_signer), - who.clone(), - alice_member_account, - ), - Error::::AccountAlreadyAdded + + assert_ok!(OmniAccount::create_account_store( + RuntimeOrigin::signed(tee_signer.clone()), + who.clone(), + )); + + let call = add_account_call(alice_member_account.clone()); + assert_ok!(OmniAccount::dispatch_as_omni_account( + RuntimeOrigin::signed(tee_signer.clone()), + who.hash(), + call + )); + + System::assert_has_event( + Event::DispatchedAsOmniAccount { + who: who.to_omni_account(), + result: Err(DispatchError::Module(ModuleError { + index: 5, + error: [0, 0, 0, 0], + message: Some("AccountAlreadyAdded"), + })), + } + .into(), ); }); } @@ -186,6 +295,11 @@ fn add_account_store_len_limit_reached_works() { let who_identity = Identity::from(who.clone()); let who_omni_account = who_identity.to_omni_account(); + assert_ok!(OmniAccount::create_account_store( + RuntimeOrigin::signed(tee_signer.clone()), + who_identity.clone(), + )); + let member_account_2 = MemberAccount::Private(vec![1, 2, 3], H256::from(blake2_256(&[1, 2, 3]))); let member_account_3 = @@ -197,15 +311,28 @@ fn add_account_store_len_limit_reached_works() { member_account_3.clone(), ]); - AccountStore::::insert(who_omni_account, member_accounts); + AccountStore::::insert(who_omni_account.clone(), member_accounts); - assert_noop!( - OmniAccount::add_account( - RuntimeOrigin::signed(tee_signer), - who_identity, - MemberAccount::Private(vec![7, 8, 9], H256::from(blake2_256(&[7, 8, 9]))), - ), - Error::::AccountStoreLenLimitReached + let call = add_account_call(MemberAccount::Private( + vec![7, 8, 9], + H256::from(blake2_256(&[7, 8, 9])), + )); + assert_ok!(OmniAccount::dispatch_as_omni_account( + RuntimeOrigin::signed(tee_signer.clone()), + who_identity.hash(), + call + )); + + System::assert_has_event( + Event::DispatchedAsOmniAccount { + who: who_omni_account, + result: Err(DispatchError::Module(ModuleError { + index: 5, + error: [1, 0, 0, 0], + message: Some("AccountStoreLenLimitReached"), + })), + } + .into(), ); }); } @@ -224,10 +351,16 @@ fn remove_account_works() { let identity_hash = member_account.hash(); let hashes = vec![identity_hash]; - assert_ok!(OmniAccount::add_account( + assert_ok!(OmniAccount::create_account_store( RuntimeOrigin::signed(tee_signer.clone()), who_identity.clone(), - member_account.clone(), + )); + + let call = add_account_call(member_account.clone()); + assert_ok!(OmniAccount::dispatch_as_omni_account( + RuntimeOrigin::signed(tee_signer.clone()), + who_identity.hash(), + call )); // normal signed origin should give `BadOrigin`, no matter @@ -294,10 +427,19 @@ fn remove_account_empty_account_check_works() { let who_identity_hash = who_identity.hash(); let who_omni_account = who_identity.to_omni_account(); - assert_ok!(OmniAccount::add_account( + assert_ok!(OmniAccount::create_account_store( RuntimeOrigin::signed(tee_signer.clone()), - who_identity, - MemberAccount::Private(vec![1, 2, 3], H256::from(blake2_256(&[1, 2, 3]))), + who_identity.clone(), + )); + + let call = add_account_call(MemberAccount::Private( + vec![1, 2, 3], + H256::from(blake2_256(&[1, 2, 3])), + )); + assert_ok!(OmniAccount::dispatch_as_omni_account( + RuntimeOrigin::signed(tee_signer.clone()), + who_identity.hash(), + call )); let call = remove_accounts_call(vec![]); @@ -334,10 +476,16 @@ fn publicize_account_works() { let public_account = MemberAccount::Public(Identity::from(bob())); let public_account_hash = public_account.hash(); - assert_ok!(OmniAccount::add_account( + assert_ok!(OmniAccount::create_account_store( RuntimeOrigin::signed(tee_signer.clone()), who_identity.clone(), - private_account.clone() + )); + + let call = add_account_call(private_account.clone()); + assert_ok!(OmniAccount::dispatch_as_omni_account( + RuntimeOrigin::signed(tee_signer.clone()), + who_identity.hash(), + call )); let expected_member_accounts: MemberAccounts = @@ -407,10 +555,16 @@ fn publicize_account_identity_not_found_works() { Error::::AccountNotFound ); - assert_ok!(OmniAccount::add_account( + assert_ok!(OmniAccount::create_account_store( RuntimeOrigin::signed(tee_signer.clone()), - who_identity, - private_account.clone(), + who_identity.clone(), + )); + + let call = add_account_call(private_account.clone()); + assert_ok!(OmniAccount::dispatch_as_omni_account( + RuntimeOrigin::signed(tee_signer.clone()), + who_identity.hash(), + call )); let charlie_identity = Identity::from(charlie()); @@ -448,10 +602,16 @@ fn dispatch_as_signed_works() { let private_account = MemberAccount::Private(vec![1, 2, 3], Identity::from(bob()).hash()); - assert_ok!(OmniAccount::add_account( + assert_ok!(OmniAccount::create_account_store( RuntimeOrigin::signed(tee_signer.clone()), who_identity.clone(), - private_account, + )); + + let call = add_account_call(private_account); + assert_ok!(OmniAccount::dispatch_as_omni_account( + RuntimeOrigin::signed(tee_signer.clone()), + who_identity.hash(), + call )); let call = make_balance_transfer_call(bob(), 5);