diff --git a/src/lib.rs b/src/lib.rs index 89c1b8a..51e1a51 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,7 +28,7 @@ pub mod pallet { use codec::{FullCodec, FullEncode}; use frame_support::{ - dispatch::{DispatchResultWithPostInfo, GetDispatchInfo, PostDispatchInfo}, + dispatch::DispatchResultWithPostInfo, pallet_prelude::*, traits::{Currency, Get, LockableCurrency, ReservableCurrency}, }; @@ -44,7 +44,6 @@ pub mod pallet { types::ScriptTransaction, }; use sp_core::crypto::AccountId32; - use sp_runtime::traits::Dispatchable; use sp_std::{vec, vec::Vec}; use super::*; @@ -69,7 +68,7 @@ pub mod pallet { /// Storage for multi-signature/signer requests. #[pallet::storage] pub type MultisigStorage = - StorageMap<_, Blake2_128Concat, CallHash, Multisig>; + StorageMap<_, Blake2_128Concat, CallHash, Multisig>; /// MoveVM pallet configuration trait #[pallet::config] @@ -87,12 +86,6 @@ pub mod pallet { #[pallet::constant] type MaxScriptSigners: Get; - /// The overarching call type. - type RuntimeCall: Parameter - + Dispatchable - + GetDispatchInfo - + From>; - /// Because this pallet emits events, it depends on the runtime's definition of an event. type RuntimeEvent: From> + IsType<::RuntimeEvent>; @@ -203,6 +196,7 @@ pub mod pallet { // Make sure the scripts are not maliciously trying to use forged signatures. 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 (mut signature_handler, call_hash) = if signer_count > 1 { // Generate the call hash to identify this multi-sign call request. @@ -211,16 +205,13 @@ pub mod pallet { let call_hash: CallHash = hasher.finalize().into(); let multisig = MultisigStorage::::get(call_hash).unwrap_or( - Multisig::::new(&args, signer_count) + Multisig::::new(accounts) .map_err(Into::>::into)?, ); (ScriptSignatureHandler::::from(multisig), call_hash) } else { - ( - ScriptSignatureHandler::::new(&args, signer_count)?, - [0u8; 32], - ) + (ScriptSignatureHandler::::new(accounts)?, [0u8; 32]) }; if signer_count > 0 { signature_handler.sign_script(&who, cheque_limit.into())?; @@ -458,6 +449,25 @@ pub mod pallet { vm.get_resource(&address, tag) .map_err(|e| format!("error in get_resource: {e:?}").into()) } + + fn extract_account_ids_from_args( + script_args: &[&[u8]], + signer_count: usize, + ) -> Result, Error> { + if signer_count > script_args.len() { + return Err(Error::::ScriptSignatureFailure); + } + + let mut accounts = Vec::::new(); + for signer in &script_args[..signer_count] { + let account_address = + bcs::from_bytes(signer).map_err(|_| Error::::UnableToDeserializeAccount)?; + let account = Self::to_native_account(&account_address)?; + accounts.push(account); + } + + Ok(accounts) + } } #[pallet::error] @@ -483,6 +493,8 @@ pub mod pallet { StdlibAddressNotAllowed, /// Error about signing multi-signature execution request twice. UserHasAlreadySigned, + /// Script contains more signers than allowed maximum number of signers. + MaxSignersExceeded, // Errors that can be received from MoveVM /// Unknown validation status diff --git a/src/mock.rs b/src/mock.rs index 58fbe69..67dd8bb 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -77,14 +77,13 @@ impl pallet_balances::Config for Test { parameter_types! { pub const MaxRequestLifetime: u32 = 5; - pub const MaxScriptSigners: u32 = 3; + pub const MaxScriptSigners: u32 = 8; } impl pallet_move::Config for Test { type Currency = Balances; type MaxRequestLifetime = MaxRequestLifetime; type MaxScriptSigners = MaxScriptSigners; - type RuntimeCall = RuntimeCall; type RuntimeEvent = RuntimeEvent; type WeightInfo = (); } @@ -149,23 +148,30 @@ pub const BOB_ADDR: &str = "5FHneW46xGXgs5mUiveU4sbTyGBzmstUspZC92UhjJM694ty"; pub const ALICE_ADDR: &str = "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY"; pub const DAVE_ADDR: &str = "5DAAnrj7VHTznn2AWBemMuyBwZWs6FNFjdyVXUeYum3PTXFy"; -pub fn addr32_from_ss58(ss58addr: &str) -> Result> { +pub(crate) fn addr32_from_ss58(ss58addr: &str) -> Result> { let (pk, _) = Public::from_ss58check_with_version(ss58addr) .map_err(|_| Error::::InvalidAccountSize)?; let account: AccountId32 = pk.into(); Ok(account) } -pub fn addr32_to_move(addr32: &AccountId32) -> Result> { +pub(crate) fn addr32_to_move(addr32: &AccountId32) -> Result> { MoveModule::to_move_address(addr32) } -pub fn addrs_from_ss58(ss58: &str) -> Result<(AccountId32, AccountAddress), Error> { +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)) } +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 mod assets { const MOVE_PROJECTS: &str = "src/tests/assets/move-projects"; diff --git a/src/signer.rs b/src/signer.rs index ab659d8..341ca10 100644 --- a/src/signer.rs +++ b/src/signer.rs @@ -1,14 +1,14 @@ use core::marker::PhantomData; use codec::{Decode, Encode, MaxEncodedLen}; -use frame_support::{pallet_prelude::RuntimeDebug, traits::Get, BoundedBTreeMap}; +use frame_support::{pallet_prelude::RuntimeDebug, traits::Get, BoundedBTreeMap, Parameter}; use frame_system::Config as SysConfig; -use move_core_types::account_address::AccountAddress; use scale_info::TypeInfo; +use sp_runtime::traits::MaybeSerializeDeserialize; use crate::{ balance::{BalanceAdapter, BalanceOf}, - Config, Error, Pallet, + Config, Error, }; /// This definition stores the hash value of a script transaction. @@ -26,6 +26,7 @@ pub enum Signature { #[derive(Clone, Eq, PartialEq)] pub enum MultisigError { + MaxSignersExceeded, ScriptSignatureFailure, UnableToDeserializeAccount, UserHasAlreadySigned, @@ -34,6 +35,7 @@ pub enum MultisigError { impl From for Error { fn from(err: MultisigError) -> Self { match err { + MultisigError::MaxSignersExceeded => Error::::MaxSignersExceeded, MultisigError::ScriptSignatureFailure => Error::::ScriptSignatureFailure, MultisigError::UnableToDeserializeAccount => Error::::UnableToDeserializeAccount, MultisigError::UserHasAlreadySigned => Error::::UserHasAlreadySigned, @@ -50,27 +52,27 @@ pub struct SignerData { } #[derive(Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)] -#[scale_info(skip_type_params(S))] -pub struct Multisig(BoundedBTreeMap) +#[scale_info(skip_type_params(Size))] +pub struct Multisig(BoundedBTreeMap) where - S: Get; + AccountId: Parameter + Ord + MaybeSerializeDeserialize, + Size: Get; -impl Multisig +impl Multisig where - S: Get, + AccountId: Parameter + Ord + MaybeSerializeDeserialize, + Size: Get, { /// Creates a new [`Multisig`] with all blank signatures for the provided script. - pub fn new(script_args: &[&[u8]], signer_count: usize) -> Result { - if signer_count > script_args.len() || signer_count > (S::get() as usize) { - return Err(MultisigError::ScriptSignatureFailure); + pub fn new(signers: Vec) -> Result { + if signers.len() > (Size::get() as usize) { + return Err(MultisigError::MaxSignersExceeded); } - let mut multisig_info = Multisig::::default(); - for signer in &script_args[..signer_count] { - let account_address = - bcs::from_bytes(signer).map_err(|_| MultisigError::UnableToDeserializeAccount)?; + let mut multisig_info = Multisig::::default(); + for account in signers.iter() { multisig_info - .try_insert(account_address, SignerData::default()) + .try_insert(account.clone(), SignerData::default()) .map_err(|_| MultisigError::UnableToDeserializeAccount)?; } @@ -78,24 +80,36 @@ where } } -impl> core::ops::Deref for Multisig { - type Target = BoundedBTreeMap; +impl core::ops::Deref for Multisig +where + AccountId: Parameter + Ord + MaybeSerializeDeserialize, + Size: Get, +{ + type Target = BoundedBTreeMap; fn deref(&self) -> &Self::Target { &self.0 } } -impl> core::ops::DerefMut for Multisig { +impl core::ops::DerefMut for Multisig +where + AccountId: Parameter + Ord + MaybeSerializeDeserialize, + Size: Get, +{ fn deref_mut(&mut self) -> &mut Self::Target { &mut self.0 } } // Because in substrate_move::AccountAddress Default impl is missing. -impl> Default for Multisig { +impl Default for Multisig +where + AccountId: Parameter + Ord + MaybeSerializeDeserialize, + Size: Get, +{ fn default() -> Self { - Multisig(BoundedBTreeMap::::new()) + Multisig(BoundedBTreeMap::::new()) } } @@ -104,23 +118,25 @@ impl> Default for Multisig { pub(crate) struct ScriptSignatureHandler where T: Config + SysConfig, + T::AccountId: Parameter + Ord + MaybeSerializeDeserialize, BalanceOf: From + Into, { _pd_config: PhantomData, /// All required multisig_info. - multisig_info: Multisig, + multisig_info: Multisig, } impl ScriptSignatureHandler where T: Config + SysConfig, + T::AccountId: Parameter + Ord + MaybeSerializeDeserialize, BalanceOf: From + Into, { /// Creates a new [`ScriptSignatureHandler`] with all blank signatures for the provided script. - pub(crate) fn new(script_args: &[&[u8]], signer_count: usize) -> Result> { + pub(crate) fn new(accounts: Vec) -> Result> { Ok(Self { _pd_config: PhantomData, - multisig_info: Multisig::::new(script_args, signer_count) + multisig_info: Multisig::::new(accounts) .map_err(Into::>::into)?, }) } @@ -137,18 +153,16 @@ where account: &T::AccountId, cheque_limit: u128, ) -> Result<(), Error> { - let account_address = Pallet::::to_move_address(account)?; - - if let Some(ms_data) = self.multisig_info.get_mut(&account_address) { + if let Some(ms_data) = self.multisig_info.get_mut(account) { if matches!(ms_data.signature, Signature::Approved) { - Err(Error::UserHasAlreadySigned) + Err(Error::::UserHasAlreadySigned) } else { ms_data.signature = Signature::Approved; ms_data.cheque_limit = cheque_limit; Ok(()) } } else { - Err(Error::ScriptSignatureFailure) + Err(Error::::ScriptSignatureFailure) } } @@ -167,38 +181,33 @@ where } let mut balances = BalanceAdapter::::new(); - for (address, ms_data) in self.multisig_info.iter() { - let account = Pallet::::to_native_account(address)?; + for (account, ms_data) in self.multisig_info.iter() { balances - .write_cheque(&account, &BalanceOf::::from(ms_data.cheque_limit)) + .write_cheque(account, &BalanceOf::::from(ms_data.cheque_limit)) .map_err(|_| Error::::InsufficientBalance)?; } Ok(balances) } - /// Consumes `ScriptSignatureHandler` and returns innner `Multisig`. - pub(crate) fn into_inner(self) -> Multisig { + /// Consumes [`ScriptSignatureHandler`] and returns innner `Multisig`. + pub(crate) fn into_inner(self) -> Multisig { self.multisig_info } - /// Consumes `ScriptSignatureHandler` and returns accounts of all signers as vector. + /// Consumes [`ScriptSignatureHandler`] and returns accounts of all signers as vector. pub(crate) fn into_signer_accounts(self) -> Result, Error> { - let mut accounts = Vec::::new(); - for key in self.multisig_info.keys() { - let acc = Pallet::::to_native_account(key)?; - accounts.push(acc); - } + let accounts: Vec = self.multisig_info.keys().cloned().collect(); Ok(accounts) } } -impl From> for ScriptSignatureHandler +impl From> for ScriptSignatureHandler where T: Config + SysConfig, BalanceOf: From + Into, { - fn from(multisig_info: Multisig) -> Self { + fn from(multisig_info: Multisig) -> Self { Self { _pd_config: PhantomData, multisig_info, diff --git a/src/tests/signer.rs b/src/tests/signer.rs index 14fa89f..eadf736 100644 --- a/src/tests/signer.rs +++ b/src/tests/signer.rs @@ -133,28 +133,107 @@ 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] -#[ignore = "to be updated"] fn general_script_eight_normal_signers_works() { - ExtBuilder::default().build().execute_with(|| { - let mut pg = ParamGenerator::new(); - let (bob_addr_32, bob_addr_mv) = addrs_from_ss58(BOB_ADDR).unwrap(); - - // 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 s1 = pg.address(&bob_addr_mv); - let extra = pg.rand::(); - let params: Vec<&[u8]> = vec![&s1, &s1, &s1, &s1, &s1, &s1, &s1, &s1, &extra]; - - assert_ok!(execute_script(bob_addr_32, script, params, type_args)); - }) + 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(); + + 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), + ]) + .build() + .execute_with(|| { + // 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 extra = pg.rand::(); + let params: Vec<&[u8]> = vec![&s1, &s2, &s3, &s4, &s5, &s6, &s7, &s8, &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, + script.clone(), + params.clone(), + type_args.clone() + )); + }) } /// Script with many signers parameters fails if all signers don't provide an actual signature. #[test] -#[ignore = "to be updated"] fn general_script_eight_normal_signers_where_eve_tries_to_forge_signers_fails() { ExtBuilder::default().build().execute_with(|| { let mut pg = ParamGenerator::new(); @@ -172,8 +251,14 @@ fn general_script_eight_normal_signers_where_eve_tries_to_forge_signers_fails() let extra = pg.rand::(); let params: Vec<&[u8]> = vec![&eve, &eve, &alice, &eve, &eve, &eve, &eve, &eve, &extra]; + assert_ok!(execute_script( + eve_addr_32.clone(), + script.clone(), + params.clone(), + type_args.clone() + )); let result = execute_script(eve_addr_32, script, params, type_args); - assert_err!(result, Error::::ScriptSignatureFailure); + assert_err!(result, Error::::UserHasAlreadySigned); }) }