From f9e4e87efa9a1bfc61f4dbceb4e985b86a0de865 Mon Sep 17 00:00:00 2001 From: Jun Jiang Date: Thu, 5 May 2022 18:44:13 +0800 Subject: [PATCH] Add force_batch to utility pallet (#11148) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add batch_try to utility pallet * lint * rename utility.batch_try -> utility.force_batch * Remove un-needed index field for utility.ItemFailed event * Remove indexes of utility,BatchCompletedWithErrors * Apply suggestions from code review Co-authored-by: Louis Merlin Co-authored-by: Louis Merlin Co-authored-by: Bastian Köcher --- substrate/frame/utility/src/benchmarking.rs | 13 ++++ substrate/frame/utility/src/lib.rs | 74 +++++++++++++++++++++ substrate/frame/utility/src/tests.rs | 32 +++++++++ substrate/frame/utility/src/weights.rs | 11 +++ 4 files changed, 130 insertions(+) diff --git a/substrate/frame/utility/src/benchmarking.rs b/substrate/frame/utility/src/benchmarking.rs index 27f346f13f..018280f69b 100644 --- a/substrate/frame/utility/src/benchmarking.rs +++ b/substrate/frame/utility/src/benchmarking.rs @@ -73,5 +73,18 @@ benchmarks! { let pallets_origin = Into::::into(pallets_origin); }: _(RawOrigin::Root, Box::new(pallets_origin), call) + force_batch { + let c in 0 .. 1000; + let mut calls: Vec<::Call> = Vec::new(); + for i in 0 .. c { + let call = frame_system::Call::remark { remark: vec![] }.into(); + calls.push(call); + } + let caller = whitelisted_caller(); + }: _(RawOrigin::Signed(caller), calls) + verify { + assert_last_event::(Event::BatchCompleted.into()) + } + impl_benchmark_test_suite!(Pallet, crate::tests::new_test_ext(), crate::tests::Test); } diff --git a/substrate/frame/utility/src/lib.rs b/substrate/frame/utility/src/lib.rs index ec48087e2e..9a8384f836 100644 --- a/substrate/frame/utility/src/lib.rs +++ b/substrate/frame/utility/src/lib.rs @@ -113,8 +113,12 @@ pub mod pallet { BatchInterrupted { index: u32, error: DispatchError }, /// Batch of dispatches completed fully with no error. BatchCompleted, + /// Batch of dispatches completed but has errors. + BatchCompletedWithErrors, /// A single item within a Batch of dispatches has completed with no error. ItemCompleted, + /// A single item within a Batch of dispatches has completed with error. + ItemFailed { error: DispatchError }, /// A call was dispatched. DispatchedAs { result: DispatchResult }, } @@ -385,6 +389,76 @@ pub mod pallet { }); Ok(()) } + + /// Send a batch of dispatch calls. + /// Unlike `batch`, it allows errors and won't interrupt. + /// + /// May be called from any origin. + /// + /// - `calls`: The calls to be dispatched from the same origin. The number of call must not + /// exceed the constant: `batched_calls_limit` (available in constant metadata). + /// + /// If origin is root then call are dispatch without checking origin filter. (This includes + /// bypassing `frame_system::Config::BaseCallFilter`). + /// + /// # + /// - Complexity: O(C) where C is the number of calls to be batched. + /// # + #[pallet::weight({ + let dispatch_infos = calls.iter().map(|call| call.get_dispatch_info()).collect::>(); + let dispatch_weight = dispatch_infos.iter() + .map(|di| di.weight) + .fold(0, |total: Weight, weight: Weight| total.saturating_add(weight)) + .saturating_add(T::WeightInfo::force_batch(calls.len() as u32)); + let dispatch_class = { + let all_operational = dispatch_infos.iter() + .map(|di| di.class) + .all(|class| class == DispatchClass::Operational); + if all_operational { + DispatchClass::Operational + } else { + DispatchClass::Normal + } + }; + (dispatch_weight, dispatch_class) + })] + pub fn force_batch( + origin: OriginFor, + calls: Vec<::Call>, + ) -> DispatchResultWithPostInfo { + let is_root = ensure_root(origin.clone()).is_ok(); + let calls_len = calls.len(); + ensure!(calls_len <= Self::batched_calls_limit() as usize, Error::::TooManyCalls); + + // Track the actual weight of each of the batch calls. + let mut weight: Weight = 0; + // Track failed dispatch occur. + let mut has_error: bool = false; + for call in calls.into_iter() { + let info = call.get_dispatch_info(); + // If origin is root, don't apply any dispatch filters; root can call anything. + let result = if is_root { + call.dispatch_bypass_filter(origin.clone()) + } else { + call.dispatch(origin.clone()) + }; + // Add the weight of this call. + weight = weight.saturating_add(extract_actual_weight(&result, &info)); + if let Err(e) = result { + has_error = true; + Self::deposit_event(Event::ItemFailed { error: e.error }); + } else { + Self::deposit_event(Event::ItemCompleted); + } + } + if has_error { + Self::deposit_event(Event::BatchCompletedWithErrors); + } else { + Self::deposit_event(Event::BatchCompleted); + } + let base_weight = T::WeightInfo::batch(calls_len as u32); + Ok(Some(base_weight + weight).into()) + } } } diff --git a/substrate/frame/utility/src/tests.rs b/substrate/frame/utility/src/tests.rs index 44b07f70db..f53459a707 100644 --- a/substrate/frame/utility/src/tests.rs +++ b/substrate/frame/utility/src/tests.rs @@ -606,3 +606,35 @@ fn batch_limit() { assert_noop!(Utility::batch_all(Origin::signed(1), calls), Error::::TooManyCalls); }); } + +#[test] +fn force_batch_works() { + new_test_ext().execute_with(|| { + assert_eq!(Balances::free_balance(1), 10); + assert_eq!(Balances::free_balance(2), 10); + assert_ok!(Utility::force_batch( + Origin::signed(1), + vec![ + call_transfer(2, 5), + call_foobar(true, 75, None), + call_transfer(2, 10), + call_transfer(2, 5), + ] + ),); + System::assert_last_event(utility::Event::BatchCompletedWithErrors.into()); + System::assert_has_event( + utility::Event::ItemFailed { error: DispatchError::Other("") }.into(), + ); + assert_eq!(Balances::free_balance(1), 0); + assert_eq!(Balances::free_balance(2), 20); + + assert_ok!(Utility::force_batch( + Origin::signed(2), + vec![call_transfer(1, 5), call_transfer(1, 5),] + ),); + System::assert_last_event(utility::Event::BatchCompleted.into()); + + assert_ok!(Utility::force_batch(Origin::signed(1), vec![call_transfer(2, 50),]),); + System::assert_last_event(utility::Event::BatchCompletedWithErrors.into()); + }); +} diff --git a/substrate/frame/utility/src/weights.rs b/substrate/frame/utility/src/weights.rs index e5f3cb0f58..34dad8d735 100644 --- a/substrate/frame/utility/src/weights.rs +++ b/substrate/frame/utility/src/weights.rs @@ -50,6 +50,7 @@ pub trait WeightInfo { fn as_derivative() -> Weight; fn batch_all(c: u32, ) -> Weight; fn dispatch_as() -> Weight; + fn force_batch(c: u32, ) -> Weight; } /// Weights for pallet_utility using the Substrate node and recommended hardware. @@ -71,6 +72,11 @@ impl WeightInfo for SubstrateWeight { fn dispatch_as() -> Weight { (8_463_000 as Weight) } + fn force_batch(c: u32, ) -> Weight { + (13_988_000 as Weight) + // Standard Error: 1_000 + .saturating_add((2_481_000 as Weight).saturating_mul(c as Weight)) + } } // For backwards compatibility and tests @@ -91,4 +97,9 @@ impl WeightInfo for () { fn dispatch_as() -> Weight { (8_463_000 as Weight) } + fn force_batch(c: u32, ) -> Weight { + (13_988_000 as Weight) + // Standard Error: 1_000 + .saturating_add((2_481_000 as Weight).saturating_mul(c as Weight)) + } }