diff --git a/Cargo.toml b/Cargo.toml index 9a7e3c1..8c5aeab 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,6 +20,7 @@ codec = { package = "parity-scale-codec", version = "3.6", default-features = fa hashbrown = { version = "0.14" } hex = { version = "0.4", default-features = false } jsonrpsee = { version = "0.16", features = ["server", "macros"] } +log = { version = "0.4", default-features = false } rand = { version = "0.8", default-features = false } scale-info = { version = "2.10", default-features = false, features = ["derive"] } serde = { version = "1.0", default-features = false, features = ["derive"] } diff --git a/pallet/Cargo.toml b/pallet/Cargo.toml index ffd0a17..9ea9b64 100644 --- a/pallet/Cargo.toml +++ b/pallet/Cargo.toml @@ -19,6 +19,7 @@ hashbrown = { workspace = true } frame-benchmarking = { workspace = true, optional = true } frame-support = { workspace = true } frame-system = { workspace = true } +log = { workspace = true } move-core-types = { workspace = true } move-vm-backend = { workspace = true } move-vm-backend-common = { workspace = true } diff --git a/pallet/src/lib.rs b/pallet/src/lib.rs index e8c8472..986f542 100644 --- a/pallet/src/lib.rs +++ b/pallet/src/lib.rs @@ -16,6 +16,16 @@ pub mod weights; pub use pallet::*; pub use weights::*; +#[macro_export] +macro_rules! log { + ($level:tt, $patter:expr $(, $values:expr)* $(,)?) => { + log::$level!( + target: "runtime::pallet-move", + concat!("[{:?}] 📄 ", $patter), >::block_number() $(, $values)* + ) + }; +} + // The pallet is defined below. #[frame_support::pallet] pub mod pallet { @@ -56,6 +66,9 @@ pub mod pallet { type MvmResult = Result>, BalanceAdapter>, Vec>; + /// Maximum number of multisig storage entries to be checked per block. + const MAX_MULTISIG_CHECKING_PER_BLOCK: u64 = 20; + #[pallet::pallet] #[pallet::without_storage_info] // Allows to define storage items without fixed size pub struct Pallet(_); @@ -68,8 +81,11 @@ pub mod pallet { /// Storage for multi-signature/signer requests. #[pallet::storage] - pub type MultisigStorage = - StorageMap<_, Blake2_128Concat, CallHash, Multisig>; + pub type MultisigStorage = StorageMap<_, Blake2_128Concat, CallHash, MultisigOf>; + + /// Storage for chore mechanism for old Multisigs in `MultisigStorage`. + #[pallet::storage] + pub type ChoreOnIdleStorage = StorageValue<_, u64>; /// MoveVM pallet configuration trait #[pallet::config] @@ -81,7 +97,7 @@ pub mod pallet { /// Maximum life time for requests. #[pallet::constant] - type MaxRequestLifetime: Get; + type MaxLifetimeRequests: Get>; /// Maximum number of signatories in multi-signer requests. #[pallet::constant] @@ -109,7 +125,7 @@ pub mod pallet { ModulePublished { who: T::AccountId }, /// Event about removed multi-signing request. /// [vec] - MultiSignRequestRemoved { who: Vec }, + MultiSignRequestRemoved { call: Vec }, /// Event about another signature for a multi-signer execution request. /// [account, multisignstate] SignedMultisigScript { who: T::AccountId }, @@ -152,9 +168,62 @@ pub mod pallet { } #[pallet::hooks] - impl Hooks> for Pallet { - fn on_idle(_n: BlockNumberFor, remaining_weight: Weight) -> Weight { - // TODO: Add maintainance for storage MultisigStorage (remove old requests). + impl Hooks> for Pallet + where + BalanceOf: From + Into, + { + fn on_idle(block_height: BlockNumberFor, mut remaining_weight: Weight) -> Weight { + let len = MultisigStorage::::iter_keys().count() as u64; + let lifetime = T::MaxLifetimeRequests::get(); + // Abort if storage is empty or the allowed lifetime is longer than the blockchain's + // existence (otherwise, an integer underflow can occur). + if len == 0 || block_height < lifetime { + return remaining_weight - T::DbWeight::get().reads(1); + } + + // We will read three times for sure and write one time for sure for the storage, + // no matter if we execute the internal chore method or not. + remaining_weight -= T::DbWeight::get().reads_writes(3, 1); + + let mut idx: u64 = ChoreOnIdleStorage::::get().unwrap_or(0); + if idx >= len { + idx = 0; + } + + let keys = MultisigStorage::::iter_keys().skip(idx as usize); + let block_tbr = block_height - lifetime; + let mut call = Vec::::new(); + let mut count: u64 = 0; + + for key in keys { + if let Some(call_hash) = Self::chore_multisig_storage(key, block_tbr) { + call.push(call_hash); + } + count += 1; + if let Some(weight) = + remaining_weight.checked_sub(&T::WeightInfo::chore_multisig_storage()) + { + remaining_weight = weight; + if count >= MAX_MULTISIG_CHECKING_PER_BLOCK { + break; + } + } else { + remaining_weight = Weight::zero(); + break; + } + } + + let n_removed = call.len() as u64; + idx += count - n_removed; + if idx >= len - n_removed { + idx = 0; + } + ChoreOnIdleStorage::::put(idx); + + if !call.is_empty() { + Self::deposit_event(Event::::MultiSignRequestRemoved { call }); + } + remaining_weight } } @@ -178,6 +247,7 @@ 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)?; @@ -198,21 +268,22 @@ pub mod pallet { let signer_count = verify_script_integrity_and_check_signers(&bytecode).map_err(Error::::from)?; let accounts = Self::extract_account_ids_from_args(&args, signer_count)?; + let block_height = >::block_number(); let (mut signature_handler, call_hash) = if signer_count > 1 { // Generate the call hash to identify this multi-sign call request. - let mut hasher = Blake2s256::new(); - hasher.update(&transaction_bc[..]); - let call_hash: CallHash = hasher.finalize().into(); + let call_hash = Self::transaction_bc_call_hash(&transaction_bc[..]); let multisig = MultisigStorage::::get(call_hash).unwrap_or( - Multisig::::new(accounts) - .map_err(Into::>::into)?, + MultisigOf::::new(accounts, block_height).map_err(Into::>::into)?, ); (ScriptSignatureHandler::::from(multisig), call_hash) } else { - (ScriptSignatureHandler::::new(accounts)?, [0u8; 32]) + ( + ScriptSignatureHandler::::new(accounts, block_height)?, + [0u8; 32], + ) }; if signer_count > 0 { signature_handler.sign_script(&who, cheque_limit.into())?; @@ -220,11 +291,16 @@ pub mod pallet { // If the script is signed correctly, we can execute it in MoveVM and update the // blockchain storage or the balance sheet. + // If we have only one signer, it will skip this; if not, we have to wait for more signers, so we store it as a multi-signer request. if !signature_handler.all_signers_approved() { MultisigStorage::::insert(call_hash, signature_handler.into_inner()); Self::deposit_event(Event::SignedMultisigScript { who }); return result::execute_only_signing(); } + // If we have multiple signers and they all have signed, we have to remove the multi-signer request from the MultisigStorage. + if signer_count > 1 { + MultisigStorage::::remove(call_hash); + } // We need to provide MoveVM read only access to balance sheet - MoveVM is allowed to // update the cheques that are used afterwards to update the balances afterwards. @@ -469,6 +545,22 @@ pub mod pallet { Ok(accounts) } + + fn chore_multisig_storage(key: CallHash, block_tbr: BlockNumberFor) -> Option { + let multisig = MultisigStorage::::get(key)?; + if *multisig.block_number() > block_tbr { + None + } else { + MultisigStorage::::remove(key); + Some(key) + } + } + + pub fn transaction_bc_call_hash(transaction_bc: &[u8]) -> CallHash { + let mut hasher = Blake2s256::new(); + hasher.update(transaction_bc); + hasher.finalize().into() + } } #[pallet::error] diff --git a/pallet/src/mock.rs b/pallet/src/mock.rs index 67dd8bb..6d3966e 100644 --- a/pallet/src/mock.rs +++ b/pallet/src/mock.rs @@ -1,7 +1,8 @@ use frame_support::{ parameter_types, - traits::{ConstU128, ConstU16, ConstU32, ConstU64}, + traits::{ConstU128, ConstU16, ConstU32, ConstU64, OnFinalize, OnIdle, OnInitialize}, }; +use frame_system::pallet_prelude::BlockNumberFor; use sp_core::{crypto::Ss58Codec, sr25519::Public, H256}; use sp_runtime::{ traits::{BlakeTwo256, IdentityLookup}, @@ -9,7 +10,7 @@ use sp_runtime::{ }; use crate as pallet_move; -use crate::Error; +use crate::{Error, WeightInfo}; pub use move_core_types::account_address::AccountAddress; pub use move_vm_backend_common::types::ScriptTransaction; @@ -76,13 +77,13 @@ impl pallet_balances::Config for Test { } parameter_types! { - pub const MaxRequestLifetime: u32 = 5; + pub const MaxLifetimeRequests: BlockNumberFor = 5; pub const MaxScriptSigners: u32 = 8; } impl pallet_move::Config for Test { type Currency = Balances; - type MaxRequestLifetime = MaxRequestLifetime; + type MaxLifetimeRequests = MaxLifetimeRequests; type MaxScriptSigners = MaxScriptSigners; type RuntimeEvent = RuntimeEvent; type WeightInfo = (); @@ -147,6 +148,7 @@ pub const CAFE_ADDR: &str = "5C4hrfjw9DjXZTzV3MwzrrAr9P1MJhSrvWGWqi1eSv4fmh4G"; pub const BOB_ADDR: &str = "5FHneW46xGXgs5mUiveU4sbTyGBzmstUspZC92UhjJM694ty"; pub const ALICE_ADDR: &str = "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY"; pub const DAVE_ADDR: &str = "5DAAnrj7VHTznn2AWBemMuyBwZWs6FNFjdyVXUeYum3PTXFy"; +pub const EVE_ADDR: &str = "5HGjWAeFDfFCWPsjFQdVV2Msvz2XtMktvgocEZcCj68kUMaw"; pub(crate) fn addr32_from_ss58(ss58addr: &str) -> Result> { let (pk, _) = Public::from_ss58check_with_version(ss58addr) @@ -165,11 +167,18 @@ pub(crate) fn addrs_from_ss58(ss58: &str) -> Result<(AccountId32, AccountAddress Ok((addr_32, addr_mv)) } -pub(crate) fn int_to_addr32(n: u128) -> AccountId32 { - let mut addr = [0u8; 32]; - let bytes: [u8; 16] = n.to_be_bytes(); - bytes.iter().enumerate().for_each(|(i, b)| addr[i] = *b); - AccountId32::from(addr) +pub(crate) fn roll_to(n: BlockNumberFor) { + let weight = ::WeightInfo::chore_multisig_storage() * 2; + while System::block_number() < n { + >::on_finalize(System::block_number()); + System::set_block_number(System::block_number() + 1); + >::on_idle(System::block_number(), weight); + >::on_initialize(System::block_number()); + } +} + +pub(crate) fn last_event() -> RuntimeEvent { + System::events().pop().expect("Event expected").event } pub mod assets { diff --git a/pallet/src/signer.rs b/pallet/src/signer.rs index 341ca10..456afb2 100644 --- a/pallet/src/signer.rs +++ b/pallet/src/signer.rs @@ -2,15 +2,20 @@ use core::marker::PhantomData; use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{pallet_prelude::RuntimeDebug, traits::Get, BoundedBTreeMap, Parameter}; -use frame_system::Config as SysConfig; +use frame_system::{pallet_prelude::BlockNumberFor, Config as SysConfig}; use scale_info::TypeInfo; use sp_runtime::traits::MaybeSerializeDeserialize; +use sp_std::vec::Vec; use crate::{ - balance::{BalanceAdapter, BalanceOf}, + balance::{AccountIdOf, BalanceAdapter, BalanceOf}, Config, Error, }; +// Some alias definition to make life easier. +pub type MaxSignersOf = ::MaxScriptSigners; +pub type MultisigOf = Multisig, BlockNumberFor, MaxSignersOf>; + /// This definition stores the hash value of a script transaction. pub type CallHash = [u8; 32]; @@ -51,25 +56,37 @@ pub struct SignerData { pub cheque_limit: u128, } +/// Storage struct definition for a multi-signer request. #[derive(Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)] #[scale_info(skip_type_params(Size))] -pub struct Multisig(BoundedBTreeMap) +pub struct Multisig where AccountId: Parameter + Ord + MaybeSerializeDeserialize, - Size: Get; + BlockNumber: Parameter + Ord + MaybeSerializeDeserialize + Default, + Size: Get, +{ + /// The signers of a script transaction. + signers: BoundedBTreeMap, + /// The block number when this `Multisig` was created and stored. + block_height: BlockNumber, +} -impl Multisig +impl Multisig where AccountId: Parameter + Ord + MaybeSerializeDeserialize, + BlockNumber: Parameter + Ord + MaybeSerializeDeserialize + Default, Size: Get, { /// Creates a new [`Multisig`] with all blank signatures for the provided script. - pub fn new(signers: Vec) -> Result { + pub fn new(signers: Vec, block_height: BlockNumber) -> Result { if signers.len() > (Size::get() as usize) { return Err(MultisigError::MaxSignersExceeded); } - let mut multisig_info = Multisig::::default(); + let mut multisig_info = Multisig:: { + block_height, + ..Default::default() + }; for account in signers.iter() { multisig_info .try_insert(account.clone(), SignerData::default()) @@ -78,65 +95,79 @@ where Ok(multisig_info) } + + pub fn block_number(&self) -> &BlockNumber { + &self.block_height + } } -impl core::ops::Deref for Multisig +impl core::ops::Deref for Multisig where AccountId: Parameter + Ord + MaybeSerializeDeserialize, + BlockNumber: Parameter + Ord + MaybeSerializeDeserialize + Default, Size: Get, { type Target = BoundedBTreeMap; fn deref(&self) -> &Self::Target { - &self.0 + &self.signers } } -impl core::ops::DerefMut for Multisig +impl core::ops::DerefMut for Multisig where AccountId: Parameter + Ord + MaybeSerializeDeserialize, + BlockNumber: Parameter + Ord + MaybeSerializeDeserialize + Default, Size: Get, { fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.0 + &mut self.signers } } // Because in substrate_move::AccountAddress Default impl is missing. -impl Default for Multisig +impl Default for Multisig where AccountId: Parameter + Ord + MaybeSerializeDeserialize, + BlockNumber: Parameter + Ord + MaybeSerializeDeserialize + Default, Size: Get, { fn default() -> Self { - Multisig(BoundedBTreeMap::::new()) + Multisig { + signers: BoundedBTreeMap::::new(), + block_height: BlockNumber::default(), + } } } /// Script signature handler tracks required signatures for the provided script. -#[derive(Clone, Eq, PartialEq)] pub(crate) struct ScriptSignatureHandler where T: Config + SysConfig, T::AccountId: Parameter + Ord + MaybeSerializeDeserialize, + BlockNumberFor: Parameter + Ord + MaybeSerializeDeserialize, BalanceOf: From + Into, { _pd_config: PhantomData, /// All required multisig_info. - multisig_info: Multisig, + multisig_info: MultisigOf, } impl ScriptSignatureHandler where T: Config + SysConfig, T::AccountId: Parameter + Ord + MaybeSerializeDeserialize, + BlockNumberFor: Parameter + Ord + MaybeSerializeDeserialize, BalanceOf: From + Into, { /// Creates a new [`ScriptSignatureHandler`] with all blank signatures for the provided script. - pub(crate) fn new(accounts: Vec) -> Result> { + pub(crate) fn new( + accounts: Vec, + block_height: BlockNumberFor, + ) -> Result> { Ok(Self { _pd_config: PhantomData, - multisig_info: Multisig::::new(accounts) + multisig_info: MultisigOf::::new(accounts, block_height) .map_err(Into::>::into)?, }) } @@ -176,10 +207,6 @@ where /// Creates a [`BalanceAdapter`] from the internal stored cheque-limits. /// Function returns an error if not all signers have signed. pub(crate) fn write_cheques(&self) -> Result, Error> { - if !self.all_signers_approved() { - return Err(Error::::ScriptSignatureFailure); - } - let mut balances = BalanceAdapter::::new(); for (account, ms_data) in self.multisig_info.iter() { balances @@ -191,7 +218,7 @@ where } /// Consumes [`ScriptSignatureHandler`] and returns innner `Multisig`. - pub(crate) fn into_inner(self) -> Multisig { + pub(crate) fn into_inner(self) -> MultisigOf { self.multisig_info } @@ -202,12 +229,14 @@ where } } -impl From> for ScriptSignatureHandler +impl From> for ScriptSignatureHandler where T: Config + SysConfig, + T::AccountId: Parameter + Ord + MaybeSerializeDeserialize, + BlockNumberFor: Parameter + Ord + MaybeSerializeDeserialize, BalanceOf: From + Into, { - fn from(multisig_info: Multisig) -> Self { + fn from(multisig_info: MultisigOf) -> Self { Self { _pd_config: PhantomData, multisig_info, diff --git a/pallet/src/tests/assets/move-projects/multiple-signers/build.sh b/pallet/src/tests/assets/move-projects/multiple-signers/build.sh index 2416aba..9deb228 100755 --- a/pallet/src/tests/assets/move-projects/multiple-signers/build.sh +++ b/pallet/src/tests/assets/move-projects/multiple-signers/build.sh @@ -3,12 +3,12 @@ cd $(dirname $0) # Participants SS58-addresses. BOB=5FHneW46xGXgs5mUiveU4sbTyGBzmstUspZC92UhjJM694ty ALICE=5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY -CHARLIE=5FLSigC9HGRKVhB9FiEo4Y3koPsNmBmLJbpXg2mp1hXcS59Y +DAVE=5DAAnrj7VHTznn2AWBemMuyBwZWs6FNFjdyVXUeYum3PTXFy EVE=5HGjWAeFDfFCWPsjFQdVV2Msvz2XtMktvgocEZcCj68kUMaw # Build the Move sources. smove build # Generate script-transactions. # 1. init_module(Bob) smove create-transaction --compiled-script-path build/multiple-signers/bytecode_scripts/init_module.mv --args signer:$BOB -# 2. rent_apartment(Alice, Alice-Stash, Bob-Stash) -smove create-transaction --compiled-script-path build/multiple-signers/bytecode_scripts/rent_apartment.mv --args signer:$ALICE signer:$CHARLIE signer:$EVE +# 2. rent_apartment(Alice, Dave, Eve, 2) +smove create-transaction --compiled-script-path build/multiple-signers/bytecode_scripts/rent_apartment.mv --args signer:$ALICE signer:$DAVE signer:$EVE u8:2 diff --git a/pallet/src/tests/assets/move-projects/smove-build-all.sh b/pallet/src/tests/assets/move-projects/smove-build-all.sh index 6c7bad5..b5da71e 100755 --- a/pallet/src/tests/assets/move-projects/smove-build-all.sh +++ b/pallet/src/tests/assets/move-projects/smove-build-all.sh @@ -8,6 +8,7 @@ build_dir=( "car-wash-example" "get-resource" "move-basics" + "multiple-signers" "signer-scripts" ) bundle_dir=( diff --git a/pallet/src/tests/assets/move-projects/smove-clean-all.sh b/pallet/src/tests/assets/move-projects/smove-clean-all.sh index c83ef9a..f126c30 100755 --- a/pallet/src/tests/assets/move-projects/smove-clean-all.sh +++ b/pallet/src/tests/assets/move-projects/smove-clean-all.sh @@ -9,6 +9,7 @@ build_dir=( "car-wash-example" "get-resource" "move-basics" + "multiple-signers" "signer-scripts" # Bundles "testing-move-stdlib" diff --git a/pallet/src/tests/signer.rs b/pallet/src/tests/signer.rs index eadf736..02ce490 100644 --- a/pallet/src/tests/signer.rs +++ b/pallet/src/tests/signer.rs @@ -1,5 +1,4 @@ -use crate::mock::*; -use crate::Error; +use crate::{mock::*, no_type_args, script_transaction, Error, Event, MultisigStorage}; use frame_support::{assert_err, assert_ok, pallet_prelude::DispatchResultWithPostInfo}; use move_core_types::{language_storage::TypeTag, u256::U256}; @@ -134,112 +133,48 @@ fn general_script_no_signers_param_at_all_works() { /// Script with many signers parameters executes correctly when all signers are signed by one account. #[test] fn general_script_eight_normal_signers_works() { - let user1_native = int_to_addr32(1); - let user1_move = addr32_to_move(&user1_native).unwrap(); - let user2_native = int_to_addr32(2); - let user2_move = addr32_to_move(&user2_native).unwrap(); - let user3_native = int_to_addr32(3); - let user3_move = addr32_to_move(&user3_native).unwrap(); - let user4_native = int_to_addr32(4); - let user4_move = addr32_to_move(&user4_native).unwrap(); - let user5_native = int_to_addr32(5); - let user5_move = addr32_to_move(&user5_native).unwrap(); - let user6_native = int_to_addr32(6); - let user6_move = addr32_to_move(&user6_native).unwrap(); - let user7_native = int_to_addr32(7); - let user7_move = addr32_to_move(&user7_native).unwrap(); - let user8_native = int_to_addr32(8); - let user8_move = addr32_to_move(&user8_native).unwrap(); + let (bob_addr_32, bob_addr_mv) = addrs_from_ss58(BOB_ADDR).unwrap(); ExtBuilder::default() - .with_balances(vec![ - (user1_native.clone(), EXISTENTIAL_DEPOSIT), - (user2_native.clone(), EXISTENTIAL_DEPOSIT), - (user3_native.clone(), EXISTENTIAL_DEPOSIT), - (user4_native.clone(), EXISTENTIAL_DEPOSIT), - (user5_native.clone(), EXISTENTIAL_DEPOSIT), - (user6_native.clone(), EXISTENTIAL_DEPOSIT), - (user7_native.clone(), EXISTENTIAL_DEPOSIT), - (user8_native.clone(), EXISTENTIAL_DEPOSIT), - ]) + .with_balances(vec![(bob_addr_32.clone(), EXISTENTIAL_DEPOSIT)]) .build() .execute_with(|| { + // Roll to first block in case of block based event checkings and processes. + roll_to(1); + // eight_normal_signers(_s1: signer, _s2: signer, _s3: &signer, _s4: signer, _s5: &signer, // _s6: signer, _s7: &signer, _s8: &signer, _extra: u32) let script = assets::read_script_from_project("signer-scripts", "eight_normal_signers"); let type_args: Vec = vec![]; let mut pg = ParamGenerator::new(); - let s1 = pg.address(&user1_move); - let s2 = pg.address(&user2_move); - let s3 = pg.address(&user3_move); - let s4 = pg.address(&user4_move); - let s5 = pg.address(&user5_move); - let s6 = pg.address(&user6_move); - let s7 = pg.address(&user7_move); - let s8 = pg.address(&user8_move); + let s1 = pg.address(&bob_addr_mv); let extra = pg.rand::(); - let params: Vec<&[u8]> = vec![&s1, &s2, &s3, &s4, &s5, &s6, &s7, &s8, &extra]; + let params: Vec<&[u8]> = vec![&s1, &s1, &s1, &s1, &s1, &s1, &s1, &s1, &extra]; assert_ok!(execute_script( - user1_native, - script.clone(), - params.clone(), - type_args.clone() - )); - assert_ok!(execute_script( - user2_native, - script.clone(), - params.clone(), - type_args.clone() - )); - assert_ok!(execute_script( - user3_native, - script.clone(), - params.clone(), - type_args.clone() - )); - assert_ok!(execute_script( - user4_native, - script.clone(), - params.clone(), - type_args.clone() - )); - assert_ok!(execute_script( - user5_native, - script.clone(), - params.clone(), - type_args.clone() - )); - assert_ok!(execute_script( - user6_native, - script.clone(), - params.clone(), - type_args.clone() - )); - assert_ok!(execute_script( - user7_native, - script.clone(), - params.clone(), - type_args.clone() - )); - assert_ok!(execute_script( - user8_native, + bob_addr_32.clone(), script.clone(), params.clone(), type_args.clone() )); + assert_eq!( + last_event(), + RuntimeEvent::MoveModule(Event::::ExecuteCalled { + who: vec![bob_addr_32] + }) + ); }) } /// Script with many signers parameters fails if all signers don't provide an actual signature. #[test] -fn general_script_eight_normal_signers_where_eve_tries_to_forge_signers_fails() { +fn eve_cant_execute_multisig_script_without_other_signers_works() { ExtBuilder::default().build().execute_with(|| { let mut pg = ParamGenerator::new(); // Eve is basically Bob here, but since Bob is pretending to be bad, we'll rename him. let (eve_addr_32, eve_addr_mv) = addrs_from_ss58(BOB_ADDR).unwrap(); - let (_, alice_addr_mv) = addrs_from_ss58(ALICE_ADDR).unwrap(); + let (alice_addr_32, alice_addr_mv) = addrs_from_ss58(ALICE_ADDR).unwrap(); // eight_normal_signers(_s1: signer, _s2: signer, _s3: &signer, _s4: signer, _s5: &signer, // _s6: signer, _s7: &signer, _s8: &signer, _extra: u32) @@ -257,8 +192,19 @@ fn general_script_eight_normal_signers_where_eve_tries_to_forge_signers_fails() params.clone(), type_args.clone() )); - let result = execute_script(eve_addr_32, script, params, type_args); + let result = execute_script( + eve_addr_32, + script.clone(), + params.clone(), + type_args.clone(), + ); assert_err!(result, Error::::UserHasAlreadySigned); + assert_ok!(execute_script( + alice_addr_32.clone(), + script.clone(), + params.clone(), + type_args.clone() + )); }) } @@ -346,3 +292,208 @@ fn script_with_vector_containing_signer_fails() { assert_err!(result, Error::::InvalidMainFunctionSignature); }) } + +#[test] +fn multiple_signers_in_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(); + 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 + )); + assert_eq!( + last_event(), + RuntimeEvent::MoveModule(Event::::ModulePublished { + who: bob_addr_32.clone() + }) + ); + + 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, + )); + assert_eq!( + last_event(), + RuntimeEvent::MoveModule(Event::::ExecuteCalled { + who: vec![bob_addr_32.clone()] + }) + ); + + // Now our three tenants want to rent the 3-room apartment. + 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 + ); + let call_hash = MoveModule::transaction_bc_call_hash(&transaction_bc[..]); + assert!(MultisigStorage::::try_get(call_hash).is_err()); + assert_ok!(MoveModule::execute( + RuntimeOrigin::signed(alice_addr_32.clone()), + transaction_bc.clone(), + MAX_GAS_AMOUNT, + BALANCE, + )); + assert_eq!( + last_event(), + RuntimeEvent::MoveModule(Event::::SignedMultisigScript { + who: alice_addr_32.clone() + }) + ); + assert!(MultisigStorage::::try_get(call_hash).is_ok()); + + assert_ok!(MoveModule::execute( + RuntimeOrigin::signed(dave_addr_32.clone()), + transaction_bc.clone(), + MAX_GAS_AMOUNT, + BALANCE, + )); + assert_eq!( + last_event(), + RuntimeEvent::MoveModule(Event::::SignedMultisigScript { + who: dave_addr_32.clone() + }) + ); + + assert_ok!(MoveModule::execute( + RuntimeOrigin::signed(eve_addr_32.clone()), + transaction_bc.clone(), + MAX_GAS_AMOUNT, + BALANCE, + )); + assert_eq!( + last_event(), + RuntimeEvent::MoveModule(Event::::ExecuteCalled { + who: vec![ + dave_addr_32.clone(), + alice_addr_32.clone(), + eve_addr_32.clone() + ] + }) + ); + // 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. + assert!(MultisigStorage::::try_get(call_hash).is_err()); + }) +} + +/// Multi-signer script execution request gets removed after defined period. +#[test] +fn verify_old_multi_signer_requests_getting_removed() { + 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 + ); + assert_ok!(MoveModule::execute( + RuntimeOrigin::signed(alice_addr_32.clone()), + transaction_bc.clone(), + MAX_GAS_AMOUNT, + BALANCE, + )); + assert_ok!(MoveModule::execute( + RuntimeOrigin::signed(dave_addr_32.clone()), + transaction_bc.clone(), + MAX_GAS_AMOUNT, + BALANCE, + )); + // Sloppy or distrustful Eve is missing... + + // Verify expected Multisig data in storage. + let call_hash = MoveModule::transaction_bc_call_hash(&transaction_bc[..]); + let _request = MultisigStorage::::try_get(call_hash).unwrap(); + + // Let's roll forward to block number 4 and check our request still exists. + roll_to(5); + assert!(MultisigStorage::::try_get(call_hash).is_ok()); + + // One more block forward and it shall be removed! + roll_to(6); + assert!(MultisigStorage::::try_get(call_hash).is_err()); + assert_eq!( + last_event(), + RuntimeEvent::MoveModule(Event::::MultiSignRequestRemoved { + call: vec![call_hash], + }) + ); + + // If Eve now tries to sign that multi-signer request, a new request will be created. + assert_ok!(MoveModule::execute( + RuntimeOrigin::signed(eve_addr_32.clone()), + transaction_bc.clone(), + MAX_GAS_AMOUNT, + BALANCE, + )); + assert_eq!( + last_event(), + RuntimeEvent::MoveModule(Event::::SignedMultisigScript { + who: eve_addr_32.clone() + }) + ); + }) +} diff --git a/pallet/src/weights.rs b/pallet/src/weights.rs index 4be976f..30f7e49 100644 --- a/pallet/src/weights.rs +++ b/pallet/src/weights.rs @@ -40,6 +40,7 @@ pub trait WeightInfo { fn publish_module_bundle() -> Weight; fn update_stdlib() -> Weight; fn execute_as_multi() -> Weight; + fn chore_multisig_storage() -> Weight; } /// Weights for pallet_move using the Substrate node and recommended hardware. @@ -72,6 +73,9 @@ impl WeightInfo for SubstrateWeight { fn execute_as_multi() -> Weight { Weight::from_parts(1_000_000, 0) } + fn chore_multisig_storage() -> Weight { + Weight::from_parts(1_000_000, 0) + } } // For backwards compatibility and tests. @@ -103,4 +107,7 @@ impl WeightInfo for () { fn execute_as_multi() -> Weight { Weight::from_parts(1_000_000, 0) } + fn chore_multisig_storage() -> Weight { + Weight::from_parts(1_000_000, 0) + } }