From 8682a086375c7006eee703a376d30e896f192936 Mon Sep 17 00:00:00 2001 From: Michael Eberhardt Date: Thu, 7 Mar 2024 18:28:31 +0100 Subject: [PATCH 1/3] feat: implement currency locking and extend testing --- pallet/src/lib.rs | 22 ++++- pallet/src/mock.rs | 18 ++++ pallet/src/signer.rs | 19 +++- pallet/src/tests/signer.rs | 158 +++++++++++++++++++++++++++++- pallet/src/tests/update_stdlib.rs | 20 +--- 5 files changed, 211 insertions(+), 26 deletions(-) diff --git a/pallet/src/lib.rs b/pallet/src/lib.rs index 986f542..622a90a 100644 --- a/pallet/src/lib.rs +++ b/pallet/src/lib.rs @@ -34,14 +34,16 @@ pub mod pallet { extern crate alloc; #[cfg(not(feature = "std"))] use alloc::format; - use blake2::{Blake2s256, Digest}; + use blake2::{digest::consts::U8, Blake2b, Blake2s256, Digest}; use core::convert::AsRef; use codec::{FullCodec, FullEncode}; use frame_support::{ dispatch::DispatchResultWithPostInfo, pallet_prelude::*, - traits::{Currency, Get, LockableCurrency, ReservableCurrency}, + traits::{ + tokens::currency::LockIdentifier, Currency, Get, LockableCurrency, ReservableCurrency, + }, }; use frame_system::pallet_prelude::*; pub use move_core_types::language_storage::TypeTag; @@ -247,7 +249,6 @@ pub mod pallet { gas_limit: u64, cheque_limit: BalanceOf, ) -> DispatchResultWithPostInfo { - // TODO(neutrinoks): Add currency locking in multi-signature executions. // A signer for the extrinsic and a signer for the Move script. let who = ensure_signed(origin)?; @@ -286,7 +287,8 @@ pub mod pallet { ) }; if signer_count > 0 { - signature_handler.sign_script(&who, cheque_limit.into())?; + let lock_id = Self::multi_signer_lock_id(&who, &transaction_bc[..], &cheque_limit); + signature_handler.sign_script(&who, cheque_limit.into(), lock_id)?; } // If the script is signed correctly, we can execute it in MoveVM and update the @@ -561,6 +563,18 @@ pub mod pallet { hasher.update(transaction_bc); hasher.finalize().into() } + + pub fn multi_signer_lock_id( + who: &T::AccountId, + transaction_bc: &[u8], + cheque_limit: &BalanceOf, + ) -> LockIdentifier { + let mut hasher = Blake2b::::new(); + hasher.update(transaction_bc); + hasher.update(&who.encode()[..]); + hasher.update(&cheque_limit.encode()[..]); + hasher.finalize().into() + } } #[pallet::error] diff --git a/pallet/src/mock.rs b/pallet/src/mock.rs index 6d3966e..bcb51ce 100644 --- a/pallet/src/mock.rs +++ b/pallet/src/mock.rs @@ -1,4 +1,6 @@ use frame_support::{ + dispatch::DispatchErrorWithPostInfo, + pallet_prelude::{DispatchError, DispatchResultWithPostInfo}, parameter_types, traits::{ConstU128, ConstU16, ConstU32, ConstU64, OnFinalize, OnIdle, OnInitialize}, }; @@ -181,6 +183,22 @@ pub(crate) fn last_event() -> RuntimeEvent { System::events().pop().expect("Event expected").event } +pub(crate) fn verify_module_error_with_msg( + res: DispatchResultWithPostInfo, + e_msg: &str, +) -> Result { + if let Err(DispatchErrorWithPostInfo { + error: DispatchError::Module(moderr), + .. + }) = res + { + if let Some(msg) = moderr.message { + return Ok(msg == e_msg); + } + } + Err(format!("{res:?} does not match '{e_msg}'")) +} + pub mod assets { const MOVE_PROJECTS: &str = "src/tests/assets/move-projects"; diff --git a/pallet/src/signer.rs b/pallet/src/signer.rs index 456afb2..121964a 100644 --- a/pallet/src/signer.rs +++ b/pallet/src/signer.rs @@ -1,7 +1,17 @@ use core::marker::PhantomData; use codec::{Decode, Encode, MaxEncodedLen}; -use frame_support::{pallet_prelude::RuntimeDebug, traits::Get, BoundedBTreeMap, Parameter}; +use frame_support::{ + pallet_prelude::RuntimeDebug, + traits::{ + tokens::{ + currency::{LockIdentifier, LockableCurrency}, + WithdrawReasons, + }, + Get, + }, + BoundedBTreeMap, Parameter, +}; use frame_system::{pallet_prelude::BlockNumberFor, Config as SysConfig}; use scale_info::TypeInfo; use sp_runtime::traits::MaybeSerializeDeserialize; @@ -54,6 +64,8 @@ pub struct SignerData { pub signature: Signature, /// Individual cheque-limit. pub cheque_limit: u128, + /// Lock ID for locked currency. + pub lock_id: LockIdentifier, } /// Storage struct definition for a multi-signer request. @@ -183,6 +195,7 @@ where &mut self, account: &T::AccountId, cheque_limit: u128, + lock_id: LockIdentifier, ) -> Result<(), Error> { if let Some(ms_data) = self.multisig_info.get_mut(account) { if matches!(ms_data.signature, Signature::Approved) { @@ -190,6 +203,9 @@ where } else { ms_data.signature = Signature::Approved; ms_data.cheque_limit = cheque_limit; + ms_data.lock_id = lock_id; + let amount = BalanceOf::::from(cheque_limit); + T::Currency::set_lock(lock_id, account, amount, WithdrawReasons::all()); Ok(()) } } else { @@ -209,6 +225,7 @@ where pub(crate) fn write_cheques(&self) -> Result, Error> { let mut balances = BalanceAdapter::::new(); for (account, ms_data) in self.multisig_info.iter() { + T::Currency::remove_lock(ms_data.lock_id, account); balances .write_cheque(account, &BalanceOf::::from(ms_data.cheque_limit)) .map_err(|_| Error::::InsufficientBalance)?; diff --git a/pallet/src/tests/signer.rs b/pallet/src/tests/signer.rs index 02ce490..a89ce6f 100644 --- a/pallet/src/tests/signer.rs +++ b/pallet/src/tests/signer.rs @@ -1,6 +1,10 @@ use crate::{mock::*, no_type_args, script_transaction, Error, Event, MultisigStorage}; -use frame_support::{assert_err, assert_ok, pallet_prelude::DispatchResultWithPostInfo}; +use frame_support::{ + assert_err, assert_ok, + pallet_prelude::DispatchResultWithPostInfo, + traits::{tokens::WithdrawReasons, Currency}, +}; use move_core_types::{language_storage::TypeTag, u256::U256}; use move_vm_backend::types::MAX_GAS_AMOUNT; use rand::{distributions::Standard, prelude::Distribution, rngs::ThreadRng, Rng}; @@ -296,6 +300,7 @@ fn script_with_vector_containing_signer_fails() { #[test] fn multiple_signers_in_multisig_script_works() { const BALANCE: Balance = 80_000_000_000_000; + const CHANGE: Balance = 20_000_000_000_000; let (bob_addr_32, bob_addr_mv) = addrs_from_ss58(BOB_ADDR).unwrap(); let (alice_addr_32, alice_addr_mv) = addrs_from_ss58(ALICE_ADDR).unwrap(); let (dave_addr_32, dave_addr_mv) = addrs_from_ss58(DAVE_ADDR).unwrap(); @@ -353,7 +358,29 @@ fn multiple_signers_in_multisig_script_works() { &2u8 ); let call_hash = MoveModule::transaction_bc_call_hash(&transaction_bc[..]); + + // Verify that no lock has been set so far and that the Multisig request entry cannot + // be found in storage. assert!(MultisigStorage::::try_get(call_hash).is_err()); + assert_ok!(Balances::ensure_can_withdraw( + &alice_addr_32, + BALANCE, + WithdrawReasons::TRANSFER, + 0 + )); + assert_ok!(Balances::ensure_can_withdraw( + &dave_addr_32, + BALANCE, + WithdrawReasons::TRANSFER, + 0 + )); + assert_ok!(Balances::ensure_can_withdraw( + &eve_addr_32, + BALANCE, + WithdrawReasons::TRANSFER, + 0 + )); + assert_ok!(MoveModule::execute( RuntimeOrigin::signed(alice_addr_32.clone()), transaction_bc.clone(), @@ -367,6 +394,25 @@ fn multiple_signers_in_multisig_script_works() { }) ); assert!(MultisigStorage::::try_get(call_hash).is_ok()); + assert!(Balances::ensure_can_withdraw( + &alice_addr_32, + BALANCE, + WithdrawReasons::TRANSFER, + 0 + ) + .is_err()); + assert_ok!(Balances::ensure_can_withdraw( + &dave_addr_32, + BALANCE, + WithdrawReasons::TRANSFER, + 0 + )); + assert_ok!(Balances::ensure_can_withdraw( + &eve_addr_32, + BALANCE, + WithdrawReasons::TRANSFER, + 0 + )); assert_ok!(MoveModule::execute( RuntimeOrigin::signed(dave_addr_32.clone()), @@ -380,6 +426,27 @@ fn multiple_signers_in_multisig_script_works() { who: dave_addr_32.clone() }) ); + // Now this candidate should also not be able to transfer the locked tokens. + assert!(Balances::ensure_can_withdraw( + &alice_addr_32, + BALANCE, + WithdrawReasons::TRANSFER, + 0 + ) + .is_err()); + assert!(Balances::ensure_can_withdraw( + &dave_addr_32, + BALANCE, + WithdrawReasons::TRANSFER, + 0 + ) + .is_err()); + assert_ok!(Balances::ensure_can_withdraw( + &eve_addr_32, + BALANCE, + WithdrawReasons::TRANSFER, + 0 + )); assert_ok!(MoveModule::execute( RuntimeOrigin::signed(eve_addr_32.clone()), @@ -397,6 +464,24 @@ fn multiple_signers_in_multisig_script_works() { ] }) ); + assert_ok!(Balances::ensure_can_withdraw( + &alice_addr_32, + CHANGE, + WithdrawReasons::TRANSFER, + 0 + )); + assert_ok!(Balances::ensure_can_withdraw( + &dave_addr_32, + CHANGE, + WithdrawReasons::TRANSFER, + 0 + )); + assert_ok!(Balances::ensure_can_withdraw( + &eve_addr_32, + CHANGE, + WithdrawReasons::TRANSFER, + 0 + )); // Now after all signers have signed the multsig script, we can expect that the script // will be executed and the multisg storage will remove the pending mutlisig script // data, since the script has been executed. @@ -448,7 +533,8 @@ fn verify_old_multi_signer_requests_getting_removed() { no_type_args!(), &alice_addr_mv, &dave_addr_mv, - &eve_addr_mv + &eve_addr_mv, + &2u8 ); assert_ok!(MoveModule::execute( RuntimeOrigin::signed(alice_addr_32.clone()), @@ -497,3 +583,71 @@ fn verify_old_multi_signer_requests_getting_removed() { ); }) } + +#[test] +fn cheque_limit_in_multi_signer_execution_works() { + const BALANCE: Balance = 80_000_000_000_000; + let (bob_addr_32, bob_addr_mv) = addrs_from_ss58(BOB_ADDR).unwrap(); + let (alice_addr_32, alice_addr_mv) = addrs_from_ss58(ALICE_ADDR).unwrap(); + let (dave_addr_32, dave_addr_mv) = addrs_from_ss58(DAVE_ADDR).unwrap(); + let (eve_addr_32, eve_addr_mv) = addrs_from_ss58(EVE_ADDR).unwrap(); + + ExtBuilder::default() + .with_balances(vec![ + (bob_addr_32.clone(), BALANCE), + (alice_addr_32.clone(), BALANCE), + (dave_addr_32.clone(), BALANCE), + (eve_addr_32.clone(), BALANCE), + ]) + .build() + .execute_with(|| { + // Roll to first block in case of block based event checkings and processes. + roll_to(1); + + // Initialisation & Setup by developer Bob. + let module = assets::read_module_from_project("multiple-signers", "Dorm"); + assert_ok!(MoveModule::publish_module( + RuntimeOrigin::signed(bob_addr_32.clone()), + module, + MAX_GAS_AMOUNT + )); + let script = assets::read_script_from_project("multiple-signers", "init_module"); + let transaction_bc = script_transaction!(script, no_type_args!(), &bob_addr_mv); + assert_ok!(MoveModule::execute( + RuntimeOrigin::signed(bob_addr_32.clone()), + transaction_bc.clone(), + MAX_GAS_AMOUNT, + BALANCE, + )); + + // Now only 2 of 3 planned signers will sign the script execution. + let script = assets::read_script_from_project("multiple-signers", "rent_apartment"); + let transaction_bc = script_transaction!( + script, + no_type_args!(), + &alice_addr_mv, + &dave_addr_mv, + &eve_addr_mv, + &2u8 + ); + assert_ok!(MoveModule::execute( + RuntimeOrigin::signed(dave_addr_32.clone()), + transaction_bc.clone(), + MAX_GAS_AMOUNT, + BALANCE, + )); + assert_ok!(MoveModule::execute( + RuntimeOrigin::signed(eve_addr_32.clone()), + transaction_bc.clone(), + MAX_GAS_AMOUNT, + BALANCE, + )); + let res = MoveModule::execute( + RuntimeOrigin::signed(alice_addr_32.clone()), + transaction_bc.clone(), + MAX_GAS_AMOUNT, + BALANCE / 2, + ); + assert!(verify_module_error_with_msg(res, "Aborted").unwrap()); + }) +} diff --git a/pallet/src/tests/update_stdlib.rs b/pallet/src/tests/update_stdlib.rs index 2ad71d3..68af93d 100644 --- a/pallet/src/tests/update_stdlib.rs +++ b/pallet/src/tests/update_stdlib.rs @@ -3,9 +3,7 @@ use crate::mock::*; use crate::{no_type_args, script_transaction}; -use frame_support::{ - assert_err, assert_ok, dispatch::DispatchErrorWithPostInfo, pallet_prelude::*, -}; +use frame_support::{assert_err, assert_ok, pallet_prelude::*}; use move_stdlib::move_stdlib_bundle; use move_vm_backend::types::MAX_GAS_AMOUNT; @@ -17,22 +15,6 @@ fn mock_substrate_stdlib() -> Vec { assets::read_bundle_from_project("testing-substrate-stdlib", "testing-substrate-stdlib") } -fn verify_module_error_with_msg( - res: DispatchResultWithPostInfo, - e_msg: &str, -) -> Result { - if let Err(DispatchErrorWithPostInfo { - error: DispatchError::Module(moderr), - .. - }) = res - { - if let Some(msg) = moderr.message { - return Ok(msg == e_msg); - } - } - Err(format!("{res:?} does not match '{e_msg}'")) -} - #[test] fn regular_user_update_fail() { let bob_addr_native = addr32_from_ss58(BOB_ADDR).unwrap(); From 6874b5ba429b724fdda030417ed3b6af57329663 Mon Sep 17 00:00:00 2001 From: Michael Eberhardt Date: Fri, 8 Mar 2024 10:01:03 +0100 Subject: [PATCH 2/3] ci: update directory src to pallet after re-structuring --- .github/workflows/common.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/common.yml b/.github/workflows/common.yml index 2247813..36e9010 100644 --- a/.github/workflows/common.yml +++ b/.github/workflows/common.yml @@ -3,7 +3,7 @@ name: Common jobs (fmt, clippy, test) on: pull_request: paths: - - src/** + - pallet/** - Cargo.toml - rpc/** - .github/workflows/common.yml From 5ad5f2efab05279bb8e70153bd008e9991b33bdb Mon Sep 17 00:00:00 2001 From: Michael Eberhardt Date: Fri, 8 Mar 2024 11:41:18 +0100 Subject: [PATCH 3/3] feat: improve readability in testing --- pallet/src/mock.rs | 30 ++++--- pallet/src/tests/signer.rs | 168 +++++++++++++++---------------------- 2 files changed, 86 insertions(+), 112 deletions(-) diff --git a/pallet/src/mock.rs b/pallet/src/mock.rs index bcb51ce..478e516 100644 --- a/pallet/src/mock.rs +++ b/pallet/src/mock.rs @@ -18,7 +18,17 @@ pub use move_core_types::account_address::AccountAddress; pub use move_vm_backend_common::types::ScriptTransaction; pub use sp_runtime::AccountId32; -type Block = frame_system::mocking::MockBlock; +// Primitive type definitions for this mockup. +pub type Balance = u128; +pub type Block = frame_system::mocking::MockBlock; +// Key constants or frequently used ones. +pub const EXISTENTIAL_DEPOSIT: Balance = 100; +pub const EMPTY_CHEQUE: Balance = 0; +pub const CAFE_ADDR: &str = "5C4hrfjw9DjXZTzV3MwzrrAr9P1MJhSrvWGWqi1eSv4fmh4G"; // == 0xCAFE +pub const BOB_ADDR: &str = "5FHneW46xGXgs5mUiveU4sbTyGBzmstUspZC92UhjJM694ty"; +pub const ALICE_ADDR: &str = "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY"; +pub const DAVE_ADDR: &str = "5DAAnrj7VHTznn2AWBemMuyBwZWs6FNFjdyVXUeYum3PTXFy"; +pub const EVE_ADDR: &str = "5HGjWAeFDfFCWPsjFQdVV2Msvz2XtMktvgocEZcCj68kUMaw"; // Configure a mock runtime to test the pallet. frame_support::construct_runtime!( @@ -56,9 +66,6 @@ impl frame_system::Config for Test { type MaxConsumers = frame_support::traits::ConstU32<16>; } -pub type Balance = u128; -pub const EXISTENTIAL_DEPOSIT: u128 = 100; - impl pallet_balances::Config for Test { type MaxLocks = ConstU32<50>; type MaxReserves = (); @@ -144,14 +151,7 @@ impl ExtBuilder { } } -// Common constants accross the tests. -pub const EMPTY_CHEQUE: u128 = 0; // Not all scripts need the `cheque_amount` parameter. -pub const CAFE_ADDR: &str = "5C4hrfjw9DjXZTzV3MwzrrAr9P1MJhSrvWGWqi1eSv4fmh4G"; // == 0xCAFE -pub const BOB_ADDR: &str = "5FHneW46xGXgs5mUiveU4sbTyGBzmstUspZC92UhjJM694ty"; -pub const ALICE_ADDR: &str = "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY"; -pub const DAVE_ADDR: &str = "5DAAnrj7VHTznn2AWBemMuyBwZWs6FNFjdyVXUeYum3PTXFy"; -pub const EVE_ADDR: &str = "5HGjWAeFDfFCWPsjFQdVV2Msvz2XtMktvgocEZcCj68kUMaw"; - +/// Creates a native 32-byte address from a given ss58 string. pub(crate) fn addr32_from_ss58(ss58addr: &str) -> Result> { let (pk, _) = Public::from_ss58check_with_version(ss58addr) .map_err(|_| Error::::InvalidAccountSize)?; @@ -159,16 +159,19 @@ pub(crate) fn addr32_from_ss58(ss58addr: &str) -> Result Result> { MoveModule::to_move_address(addr32) } +/// Creates a native 32-byte address and it's Move memory address by given ss58 string. pub(crate) fn addrs_from_ss58(ss58: &str) -> Result<(AccountId32, AccountAddress), Error> { let addr_32 = addr32_from_ss58(ss58)?; let addr_mv = addr32_to_move(&addr_32)?; Ok((addr_32, addr_mv)) } +/// Rolls forward in history to the given block height. pub(crate) fn roll_to(n: BlockNumberFor) { let weight = ::WeightInfo::chore_multisig_storage() * 2; while System::block_number() < n { @@ -179,10 +182,13 @@ pub(crate) fn roll_to(n: BlockNumberFor) { } } +/// Returns the last emitted event by the blockchain. pub(crate) fn last_event() -> RuntimeEvent { System::events().pop().expect("Event expected").event } +/// In case of an error returned from MoveVM, this method compares the encapsuled error string at +/// the level of the returned `DispatchResultWithPostInfo`. pub(crate) fn verify_module_error_with_msg( res: DispatchResultWithPostInfo, e_msg: &str, diff --git a/pallet/src/tests/signer.rs b/pallet/src/tests/signer.rs index a89ce6f..40aadbd 100644 --- a/pallet/src/tests/signer.rs +++ b/pallet/src/tests/signer.rs @@ -2,7 +2,7 @@ use crate::{mock::*, no_type_args, script_transaction, Error, Event, MultisigSto use frame_support::{ assert_err, assert_ok, - pallet_prelude::DispatchResultWithPostInfo, + pallet_prelude::{DispatchResult, DispatchResultWithPostInfo}, traits::{tokens::WithdrawReasons, Currency}, }; use move_core_types::{language_storage::TypeTag, u256::U256}; @@ -11,7 +11,7 @@ use rand::{distributions::Standard, prelude::Distribution, rngs::ThreadRng, Rng} use serde::Serialize; fn execute_script( - who: AccountId32, + who: &AccountId32, script: Vec, params: Vec<&[u8]>, type_args: Vec, @@ -31,6 +31,15 @@ fn execute_script( ) } +fn ensure_can_withdraw(who: &AccountId32, amount: Balance) -> DispatchResult { + Balances::ensure_can_withdraw( + who, // Account to be checked + amount, // Amount that shall be able to withdrawn + WithdrawReasons::TRANSFER, // Any kind of reason, because we have locked all kinds + 0, // Expected account balance after that possible withdrawel + ) +} + // Quick BCS parameter deserializer. struct ParamGenerator { rng: ThreadRng, @@ -106,7 +115,7 @@ fn general_script_no_params_works() { let script = assets::read_script_from_project("signer-scripts", "no_param_at_all"); let type_args: Vec = vec![]; let params: Vec<&[u8]> = vec![]; - assert_ok!(execute_script(bob_addr_32, script, params, type_args)); + assert_ok!(execute_script(&bob_addr_32, script, params, type_args)); }) } @@ -130,7 +139,7 @@ fn general_script_no_signers_param_at_all_works() { let f = pg.rand::(); let params: Vec<&[u8]> = vec![&iter, &a, &b, &c, &d, &e, &f]; - assert_ok!(execute_script(bob_addr_32, script, params, type_args)); + assert_ok!(execute_script(&bob_addr_32, script, params, type_args)); }) } @@ -157,7 +166,7 @@ fn general_script_eight_normal_signers_works() { let params: Vec<&[u8]> = vec![&s1, &s1, &s1, &s1, &s1, &s1, &s1, &s1, &extra]; assert_ok!(execute_script( - bob_addr_32.clone(), + &bob_addr_32, script.clone(), params.clone(), type_args.clone() @@ -191,20 +200,20 @@ fn eve_cant_execute_multisig_script_without_other_signers_works() { let params: Vec<&[u8]> = vec![&eve, &eve, &alice, &eve, &eve, &eve, &eve, &eve, &extra]; assert_ok!(execute_script( - eve_addr_32.clone(), + &eve_addr_32, script.clone(), params.clone(), type_args.clone() )); let result = execute_script( - eve_addr_32, + &eve_addr_32, script.clone(), params.clone(), type_args.clone(), ); assert_err!(result, Error::::UserHasAlreadySigned); assert_ok!(execute_script( - alice_addr_32.clone(), + &alice_addr_32, script.clone(), params.clone(), type_args.clone() @@ -241,7 +250,7 @@ fn signer_before_all_possible_vectors_works() { &bob, &v_u8, &v_u16, &v_u32, &v_u64, &v_u128, &v_u256, &v_addr, &v_bool, ]; - assert_ok!(execute_script(bob_addr_32, script, params, type_args)); + assert_ok!(execute_script(&bob_addr_32, script, params, type_args)); }) } @@ -272,7 +281,7 @@ fn signer_after_all_possible_vectors_fails() { &v_u8, &v_u16, &v_u32, &v_u64, &v_u128, &v_u256, &v_addr, &v_bool, &bob, ]; - let result = execute_script(bob_addr_32, script, params, type_args); + let result = execute_script(&bob_addr_32, script, params, type_args); assert_err!(result, Error::::InvalidMainFunctionSignature); }) } @@ -292,7 +301,7 @@ fn script_with_vector_containing_signer_fails() { let v_addr = pg.address_vec(vec![&bob_addr_mv]); let params: Vec<&[u8]> = vec![&v_addr]; - let result = execute_script(bob_addr_32, script, params, type_args); + let result = execute_script(&bob_addr_32, script, params, type_args); assert_err!(result, Error::::InvalidMainFunctionSignature); }) } @@ -360,100 +369,69 @@ fn multiple_signers_in_multisig_script_works() { let call_hash = MoveModule::transaction_bc_call_hash(&transaction_bc[..]); // Verify that no lock has been set so far and that the Multisig request entry cannot - // be found in storage. + // be found in storage, because it hasn't been created so far. assert!(MultisigStorage::::try_get(call_hash).is_err()); - assert_ok!(Balances::ensure_can_withdraw( - &alice_addr_32, - BALANCE, - WithdrawReasons::TRANSFER, - 0 - )); - assert_ok!(Balances::ensure_can_withdraw( - &dave_addr_32, - BALANCE, - WithdrawReasons::TRANSFER, - 0 - )); - assert_ok!(Balances::ensure_can_withdraw( - &eve_addr_32, - BALANCE, - WithdrawReasons::TRANSFER, - 0 - )); + assert_ok!(ensure_can_withdraw(&alice_addr_32, BALANCE)); + assert_ok!(ensure_can_withdraw(&dave_addr_32, BALANCE)); + assert_ok!(ensure_can_withdraw(&eve_addr_32, BALANCE)); + // Alice as the first one will now start the multi-signing execution request. assert_ok!(MoveModule::execute( RuntimeOrigin::signed(alice_addr_32.clone()), transaction_bc.clone(), MAX_GAS_AMOUNT, BALANCE, )); + + // Expect event `SignedMultisigScript` to be emitted with Alice address. assert_eq!( last_event(), RuntimeEvent::MoveModule(Event::::SignedMultisigScript { who: alice_addr_32.clone() }) ); + + // Now we expect an entry in the `MultisigStorage`, because we have a pending request. assert!(MultisigStorage::::try_get(call_hash).is_ok()); - assert!(Balances::ensure_can_withdraw( - &alice_addr_32, - BALANCE, - WithdrawReasons::TRANSFER, - 0 - ) - .is_err()); - assert_ok!(Balances::ensure_can_withdraw( - &dave_addr_32, - BALANCE, - WithdrawReasons::TRANSFER, - 0 - )); - assert_ok!(Balances::ensure_can_withdraw( - &eve_addr_32, - BALANCE, - WithdrawReasons::TRANSFER, - 0 - )); + // By signing the pending multisig script, Alice locked her funds until the script is + // either executed (or discarded due to timeout). + assert!(ensure_can_withdraw(&alice_addr_32, BALANCE).is_err()); + // Eve and Dave still haven't signed the scripts, so their funds are not locked. + assert_ok!(ensure_can_withdraw(&dave_addr_32, BALANCE)); + assert_ok!(ensure_can_withdraw(&eve_addr_32, BALANCE)); + + // Now Dave is the next one. assert_ok!(MoveModule::execute( RuntimeOrigin::signed(dave_addr_32.clone()), transaction_bc.clone(), MAX_GAS_AMOUNT, BALANCE, )); + + // We expect the same kind of event, because the request is still pending. assert_eq!( last_event(), RuntimeEvent::MoveModule(Event::::SignedMultisigScript { who: dave_addr_32.clone() }) ); - // Now this candidate should also not be able to transfer the locked tokens. - assert!(Balances::ensure_can_withdraw( - &alice_addr_32, - BALANCE, - WithdrawReasons::TRANSFER, - 0 - ) - .is_err()); - assert!(Balances::ensure_can_withdraw( - &dave_addr_32, - BALANCE, - WithdrawReasons::TRANSFER, - 0 - ) - .is_err()); - assert_ok!(Balances::ensure_can_withdraw( - &eve_addr_32, - BALANCE, - WithdrawReasons::TRANSFER, - 0 - )); + // Dave should also not be able to transfer the locked tokens anymore. + assert!(ensure_can_withdraw(&alice_addr_32, BALANCE).is_err()); + assert!(ensure_can_withdraw(&dave_addr_32, BALANCE).is_err()); + assert_ok!(ensure_can_withdraw(&eve_addr_32, BALANCE)); + + // Last signer Eve will now finish the request, we expect it to be executed, cleared up + // from the storage and remaining funds to be unlocked again. assert_ok!(MoveModule::execute( RuntimeOrigin::signed(eve_addr_32.clone()), transaction_bc.clone(), MAX_GAS_AMOUNT, BALANCE, )); + + // No more `SignedMultisigScript` event expected - the `ExecuteCalled` instead. assert_eq!( last_event(), RuntimeEvent::MoveModule(Event::::ExecuteCalled { @@ -464,28 +442,14 @@ fn multiple_signers_in_multisig_script_works() { ] }) ); - assert_ok!(Balances::ensure_can_withdraw( - &alice_addr_32, - CHANGE, - WithdrawReasons::TRANSFER, - 0 - )); - assert_ok!(Balances::ensure_can_withdraw( - &dave_addr_32, - CHANGE, - WithdrawReasons::TRANSFER, - 0 - )); - assert_ok!(Balances::ensure_can_withdraw( - &eve_addr_32, - CHANGE, - WithdrawReasons::TRANSFER, - 0 - )); - // Now after all signers have signed the multsig script, we can expect that the script - // will be executed and the multisg storage will remove the pending mutlisig script - // data, since the script has been executed. + + // Multisig entry in storage should be cleaned up again after executing it. assert!(MultisigStorage::::try_get(call_hash).is_err()); + + // Remaining funds should be unlocked again. + assert_ok!(ensure_can_withdraw(&alice_addr_32, CHANGE)); + assert_ok!(ensure_can_withdraw(&dave_addr_32, CHANGE)); + assert_ok!(ensure_can_withdraw(&eve_addr_32, CHANGE)); }) } @@ -529,12 +493,12 @@ fn verify_old_multi_signer_requests_getting_removed() { // Now only 2 of 3 planned signers will sign the script execution. let script = assets::read_script_from_project("multiple-signers", "rent_apartment"); let transaction_bc = script_transaction!( - script, - no_type_args!(), - &alice_addr_mv, + script, // script bytecode + no_type_args!(), // no generic arguments + &alice_addr_mv, // the three potential tenants &dave_addr_mv, &eve_addr_mv, - &2u8 + &2u8 // number of months ); assert_ok!(MoveModule::execute( RuntimeOrigin::signed(alice_addr_32.clone()), @@ -585,7 +549,7 @@ fn verify_old_multi_signer_requests_getting_removed() { } #[test] -fn cheque_limit_in_multi_signer_execution_works() { +fn insufficient_cheque_limit_aborts_the_multisig_script_works() { const BALANCE: Balance = 80_000_000_000_000; let (bob_addr_32, bob_addr_mv) = addrs_from_ss58(BOB_ADDR).unwrap(); let (alice_addr_32, alice_addr_mv) = addrs_from_ss58(ALICE_ADDR).unwrap(); @@ -623,12 +587,12 @@ fn cheque_limit_in_multi_signer_execution_works() { // Now only 2 of 3 planned signers will sign the script execution. let script = assets::read_script_from_project("multiple-signers", "rent_apartment"); let transaction_bc = script_transaction!( - script, - no_type_args!(), - &alice_addr_mv, + script, // script bytecode + no_type_args!(), // no generic arguments + &alice_addr_mv, // the three potential tenants &dave_addr_mv, &eve_addr_mv, - &2u8 + &2u8 // number of months ); assert_ok!(MoveModule::execute( RuntimeOrigin::signed(dave_addr_32.clone()), @@ -642,12 +606,16 @@ fn cheque_limit_in_multi_signer_execution_works() { MAX_GAS_AMOUNT, BALANCE, )); + // One of the signers will set his cheque-limit too low to rent the apartment. The + // script "rent_apartment" expects every of the signers to pay the same equal amount. let res = MoveModule::execute( RuntimeOrigin::signed(alice_addr_32.clone()), transaction_bc.clone(), MAX_GAS_AMOUNT, BALANCE / 2, ); + // Verify that the execution will be aborted since on of the signers has a too low + // cheque-limit to pay his part of the bill. assert!(verify_module_error_with_msg(res, "Aborted").unwrap()); }) }