diff --git a/pallets/runtime/tests/src/pips_test.rs b/pallets/runtime/tests/src/pips_test.rs index 8562b2cec9..30faa1b802 100644 --- a/pallets/runtime/tests/src/pips_test.rs +++ b/pallets/runtime/tests/src/pips_test.rs @@ -178,7 +178,7 @@ fn assert_state(id: PipId, care_about_pruned: bool, state: ProposalState) { } } -fn assert_balance(acc: Public, free: u128, locked: u128) { +pub fn assert_balance(acc: Public, free: u128, locked: u128) { assert_eq!(Balances::free_balance(&acc), free); assert_eq!(Balances::usable_balance(&acc), free - locked); } diff --git a/pallets/runtime/tests/src/utility_test.rs b/pallets/runtime/tests/src/utility_test.rs index eef67c6bbf..5848eea5c4 100644 --- a/pallets/runtime/tests/src/utility_test.rs +++ b/pallets/runtime/tests/src/utility_test.rs @@ -1,80 +1,137 @@ use super::{ - storage::{register_keyring_account_with_balance, Call, TestStorage}, + pips_test::assert_balance, + storage::{register_keyring_account_with_balance, Call, EventTest, TestStorage}, ExtBuilder, }; use pallet_balances::{self as balances, Call as BalancesCall}; -use pallet_utility as utility; +use pallet_utility::{self as utility, Event}; use polymesh_common_utilities::traits::transaction_payment::CddAndFeeDetails; use codec::Encode; -use frame_support::{assert_err, assert_ok}; +use frame_support::{assert_err, assert_ok, dispatch::DispatchError}; use pallet_utility::UniqueCall; use polymesh_primitives::Signatory; -use sp_core::sr25519::Signature; +use sp_core::sr25519::{Public, Signature}; use test_client::AccountKeyring; type Balances = balances::Module; type Utility = utility::Module; type Error = utility::Error; type Origin = ::Origin; +type System = frame_system::Module; -#[test] -fn batch_with_signed_works() { - ExtBuilder::default() - .build() - .execute_with(batch_with_signed_works_we); +fn transfer(to: Public, amount: u128) -> Call { + Call::Balances(BalancesCall::transfer(to, amount)) } -fn batch_with_signed_works_we() { - let alice = AccountKeyring::Alice.public(); - TestStorage::set_payer_context(Some(alice)); - let _alice_did = register_keyring_account_with_balance(AccountKeyring::Alice, 1_000).unwrap(); +const ERROR: DispatchError = DispatchError::Module { + index: 0, + error: 2, + message: None, +}; - let bob = AccountKeyring::Bob.public(); - TestStorage::set_payer_context(Some(bob)); - let _bob_did = register_keyring_account_with_balance(AccountKeyring::Bob, 1_000).unwrap(); +fn assert_event(event: Event) { + assert_eq!( + System::events().pop().unwrap().event, + EventTest::pallet_utility(event) + ) +} + +fn batch_test(test: impl FnOnce(Public, Public)) { + ExtBuilder::default().build().execute_with(|| { + System::set_block_number(1); + + let alice = AccountKeyring::Alice.public(); + TestStorage::set_payer_context(Some(alice)); + let _ = register_keyring_account_with_balance(AccountKeyring::Alice, 1_000).unwrap(); - assert_eq!(Balances::free_balance(alice), 959); - assert_eq!(Balances::free_balance(bob), 959); + let bob = AccountKeyring::Bob.public(); + TestStorage::set_payer_context(Some(bob)); + let _ = register_keyring_account_with_balance(AccountKeyring::Bob, 1_000).unwrap(); - let batched_calls = vec![ - Call::Balances(BalancesCall::transfer(bob, 400)), - Call::Balances(BalancesCall::transfer(bob, 400)), - ]; + assert_balance(alice, 959, 0); + assert_balance(bob, 959, 0); - assert_ok!(Utility::batch(Origin::signed(alice), batched_calls)); - assert_eq!(Balances::free_balance(alice), 159); - assert_eq!(Balances::free_balance(bob), 959 + 400 + 400); + test(alice, bob) + }); } #[test] -fn batch_early_exit_works() { - ExtBuilder::default() - .build() - .execute_with(batch_early_exit_works_we); +fn batch_with_signed_works() { + batch_test(|alice, bob| { + let calls = vec![transfer(bob, 400), transfer(bob, 400)]; + assert_ok!(Utility::batch(Origin::signed(alice), calls)); + assert_balance(alice, 159, 0); + assert_balance(bob, 959 + 400 + 400, 0); + assert_event(Event::BatchCompleted); + }); } -fn batch_early_exit_works_we() { - let alice = AccountKeyring::Alice.public(); - TestStorage::set_payer_context(Some(alice)); - let _alice_did = register_keyring_account_with_balance(AccountKeyring::Alice, 1_000).unwrap(); +#[test] +fn batch_early_exit_works() { + batch_test(|alice, bob| { + let calls = vec![transfer(bob, 400), transfer(bob, 900), transfer(bob, 400)]; + assert_ok!(Utility::batch(Origin::signed(alice), calls)); + assert_balance(alice, 559, 0); + assert_balance(bob, 959 + 400, 0); + assert_event(Event::BatchInterrupted(1, ERROR)); + }) +} - let bob = AccountKeyring::Bob.public(); - TestStorage::set_payer_context(Some(bob)); - let _bob_did = register_keyring_account_with_balance(AccountKeyring::Bob, 1_000).unwrap(); +#[test] +fn batch_optimistic_works() { + batch_test(|alice, bob| { + let calls = vec![transfer(bob, 401), transfer(bob, 402)]; + assert_ok!(Utility::batch_optimistic(Origin::signed(alice), calls)); + assert_event(Event::BatchCompleted); + assert_balance(alice, 959 - 401 - 402, 0); + assert_balance(bob, 959 + 401 + 402, 0); + }); +} - assert_eq!(Balances::free_balance(alice), 959); - assert_eq!(Balances::free_balance(bob), 959); +#[test] +fn batch_optimistic_failures_listed() { + batch_test(|alice, bob| { + assert_ok!(Utility::batch_optimistic( + Origin::signed(alice), + vec![ + transfer(bob, 401), // YAY. + transfer(bob, 900), // NAY. + transfer(bob, 800), // NAY. + transfer(bob, 402), // YAY. + transfer(bob, 403), // NAY. + ] + )); + assert_event(Event::BatchOptimisticFailed(vec![ + (1, ERROR), + (2, ERROR), + (4, ERROR), + ])); + assert_balance(alice, 959 - 401 - 402, 0); + assert_balance(bob, 959 + 401 + 402, 0); + }); +} - let batched_calls = vec![ - Call::Balances(BalancesCall::transfer(bob, 400)), - Call::Balances(BalancesCall::transfer(bob, 900)), - Call::Balances(BalancesCall::transfer(bob, 400)), - ]; +#[test] +fn batch_atomic_works() { + batch_test(|alice, bob| { + let calls = vec![transfer(bob, 401), transfer(bob, 402)]; + assert_ok!(Utility::batch_atomic(Origin::signed(alice), calls)); + assert_event(Event::BatchCompleted); + assert_balance(alice, 959 - 401 - 402, 0); + assert_balance(bob, 959 + 401 + 402, 0); + }); +} - assert_ok!(Utility::batch(Origin::signed(alice), batched_calls)); - assert_eq!(Balances::free_balance(alice), 559); - assert_eq!(Balances::free_balance(bob), 959 + 400); +#[test] +fn batch_atomic_early_exit_works() { + batch_test(|alice, bob| { + let calls = vec![transfer(bob, 400), transfer(bob, 900), transfer(bob, 400)]; + assert_ok!(Utility::batch_atomic(Origin::signed(alice), calls)); + assert_balance(alice, 959, 0); + assert_balance(bob, 959, 0); + assert_event(Event::BatchInterrupted(1, ERROR)); + }) } #[test] @@ -86,17 +143,16 @@ fn relay_happy_case() { fn _relay_happy_case() { let alice = AccountKeyring::Alice.public(); - let _alice_did = register_keyring_account_with_balance(AccountKeyring::Alice, 1_000).unwrap(); + let _ = register_keyring_account_with_balance(AccountKeyring::Alice, 1_000).unwrap(); let bob = AccountKeyring::Bob.public(); - let _bob_did = register_keyring_account_with_balance(AccountKeyring::Bob, 1_000).unwrap(); + let _ = register_keyring_account_with_balance(AccountKeyring::Bob, 1_000).unwrap(); let charlie = AccountKeyring::Charlie.public(); - let _charlie_did = - register_keyring_account_with_balance(AccountKeyring::Charlie, 1_000).unwrap(); + let _ = register_keyring_account_with_balance(AccountKeyring::Charlie, 1_000).unwrap(); - assert_eq!(Balances::free_balance(bob), 1000); - assert_eq!(Balances::free_balance(charlie), 1000); + assert_balance(bob, 1000, 0); + assert_balance(charlie, 1000, 0); let origin = Origin::signed(alice); let transaction = UniqueCall::new( @@ -111,8 +167,8 @@ fn _relay_happy_case() { transaction )); - assert_eq!(Balances::free_balance(bob), 950); - assert_eq!(Balances::free_balance(charlie), 1_050); + assert_balance(bob, 950, 0); + assert_balance(charlie, 1_050, 0); } #[test] @@ -124,7 +180,7 @@ fn relay_unhappy_cases() { fn _relay_unhappy_cases() { let alice = AccountKeyring::Alice.public(); - let _alice_did = register_keyring_account_with_balance(AccountKeyring::Alice, 1_000).unwrap(); + let _ = register_keyring_account_with_balance(AccountKeyring::Alice, 1_000).unwrap(); let bob = AccountKeyring::Bob.public(); @@ -156,7 +212,7 @@ fn _relay_unhappy_cases() { Error::TargetCddMissing ); - let _bob_did = register_keyring_account_with_balance(AccountKeyring::Bob, 1_000).unwrap(); + let _ = register_keyring_account_with_balance(AccountKeyring::Bob, 1_000).unwrap(); let transaction = UniqueCall::new( Utility::nonce(bob) + 1, diff --git a/pallets/utility/src/lib.rs b/pallets/utility/src/lib.rs index 73444c08ca..e102f81896 100644 --- a/pallets/utility/src/lib.rs +++ b/pallets/utility/src/lib.rs @@ -61,6 +61,7 @@ use frame_system as system; use frame_system::{ensure_root, ensure_signed, RawOrigin}; use polymesh_common_utilities::{ balances::CheckCdd, identity::AuthorizationNonce, identity::Trait as IdentityTrait, + with_transaction, }; use sp_runtime::{traits::Dispatchable, traits::Verify, DispatchError, RuntimeDebug}; use sp_std::prelude::*; @@ -101,9 +102,12 @@ decl_event! { /// Events type. pub enum Event { - /// Batch of dispatches did not complete fully. Index of first failing dispatch given, as - /// well as the error. + /// Batch of dispatches did not complete fully. + /// Index of first failing dispatch given, as well as the error. BatchInterrupted(u32, DispatchError), + /// Batch of dispatches did not complete fully. + /// Includes any failed dispatches with their indices and their associated error. + BatchOptimisticFailed(Vec<(u32, DispatchError)>), /// Batch of dispatches completed fully with no error. BatchCompleted, } @@ -125,6 +129,37 @@ impl UniqueCall { } } +fn combine_classes(mut classes: impl Iterator) -> DispatchClass { + let all_operational = classes.all(|class| class == DispatchClass::Operational); + if all_operational { + DispatchClass::Operational + } else { + DispatchClass::Normal + } +} + +fn batch_weight(calls: &[impl GetDispatchInfo]) -> (Weight, DispatchClass) { + let weight = calls + .into_iter() + .map(|call| call.get_dispatch_info().weight) + .fold(550_000_000, |a: Weight, n| a.saturating_add(n)) + .saturating_add(10_000_000); + let class = combine_classes(calls.into_iter().map(|call| call.get_dispatch_info().class)); + (weight, class) +} + +fn dispatch_call( + origin: T::Origin, + is_root: bool, + call: ::Call, +) -> DispatchResultWithPostInfo { + if is_root { + call.dispatch_bypass_filter(origin) + } else { + call.dispatch(origin) + } +} + decl_module! { pub struct Module for enum Call where origin: T::Origin { type Error = Error; @@ -150,30 +185,11 @@ decl_module! { /// `BatchInterrupted` event is deposited, along with the number of successful calls made /// and the error of the failed call. If all were successful, then the `BatchCompleted` /// event is deposited. - #[weight = ( - calls.iter() - .map(|call| call.get_dispatch_info().weight) - .fold(550_000_000, |a: Weight, n| a.saturating_add(n).saturating_add(10_000_000)), - { - let all_operational = calls.iter() - .map(|call| call.get_dispatch_info().class) - .all(|class| class == DispatchClass::Operational); - if all_operational { - DispatchClass::Operational - } else { - DispatchClass::Normal - } - }, - )] + #[weight = batch_weight(&calls)] pub fn batch(origin, calls: Vec<::Call>) { let is_root = ensure_root(origin.clone()).is_ok(); for (index, call) in calls.into_iter().enumerate() { - let result = if is_root { - call.dispatch_bypass_filter(origin.clone()) - } else { - call.dispatch(origin.clone()) - }; - if let Err(e) = result { + if let Err(e) = dispatch_call::(origin.clone(), is_root, call) { Self::deposit_event(Event::BatchInterrupted(index as u32, e.error)); return Ok(()); } @@ -181,6 +197,77 @@ decl_module! { Self::deposit_event(Event::BatchCompleted); } + /// Dispatch multiple calls from the sender's origin. + /// + /// This will execute all calls, in order, stopping at the first failure, + /// in which case the state changes are rolled back. + /// On failure, an event `BatchInterrupted(failure_idx, error)` is deposited. + /// + /// May be called from any origin. + /// + ///# Parameters + /// - `calls`: The calls to be dispatched from the same origin. + /// + /// # Weight + /// - The sum of the weights of the `calls`. + /// - One event. + /// + /// This will return `Ok` in all circumstances. + /// To determine the success of the batch, an event is deposited. + /// If any call failed, then `BatchInterrupted` is deposited. + /// If all were successful, then the `BatchCompleted` event is deposited. + #[weight = batch_weight(&calls)] + pub fn batch_atomic(origin, calls: Vec<::Call>) { + let is_root = ensure_root(origin.clone()).is_ok(); + Self::deposit_event(match with_transaction(|| { + for (index, call) in calls.into_iter().enumerate() { + if let Err(e) = dispatch_call::(origin.clone(), is_root, call) { + return Err((index as u32, e.error)) + } + } + Ok(()) + }) { + Ok(()) => Event::BatchCompleted, + Err((i, e)) => Event::BatchInterrupted(i, e) + }); + } + + /// Dispatch multiple calls from the sender's origin. + /// + /// This will execute all calls, in order, irrespective of failures. + /// Any failures will be available in a `BatchOptimisticFailed` event. + /// + /// May be called from any origin. + /// + ///# Parameters + /// - `calls`: The calls to be dispatched from the same origin. + /// + /// # Weight + /// - The sum of the weights of the `calls`. + /// - One event. + /// + /// This will return `Ok` in all circumstances. + /// To determine the success of the batch, an event is deposited. + /// If any call failed, then `BatchOptimisticFailed` is deposited, + /// with a vector of `(index, error)`. + /// If all were successful, then the `BatchCompleted` event is deposited. + #[weight = batch_weight(&calls)] + pub fn batch_optimistic(origin, calls: Vec<::Call>) { + let is_root = ensure_root(origin.clone()).is_ok(); + // Optimistically (hey, it's in the function name, :wink:) assume no errors. + let mut errors = Vec::new(); + for (index, call) in calls.into_iter().enumerate() { + if let Err(e) = dispatch_call::(origin.clone(), is_root, call) { + errors.push((index as u32, e.error)); + } + } + Self::deposit_event(if errors.is_empty() { + Event::BatchCompleted + } else { + Event::BatchOptimisticFailed(errors) + }) + } + /// Relay a call for a target from an origin /// /// Relaying in this context refers to the ability of origin to make a call on behalf of