diff --git a/prdoc/pr_10444.prdoc b/prdoc/pr_10444.prdoc new file mode 100644 index 0000000000000..5382e903605c8 --- /dev/null +++ b/prdoc/pr_10444.prdoc @@ -0,0 +1,16 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Improve `charge_transaction_payment benchmark` ergonomics + +doc: + - audience: Runtime Dev + description: | + Adds a `setup_benchmark_environment()` hook to allow runtimes to configure + required state before running the benchmark (e.g., setting block author for + fee distribution). Also fixes `amount_to_endow` calculation to use actual + computed fee and ensure it meets the existential deposit. + +crates: + - name: pallet-transaction-payment + bump: patch diff --git a/substrate/frame/transaction-payment/src/benchmarking.rs b/substrate/frame/transaction-payment/src/benchmarking.rs index 70ecfbf149e57..9969083cc1783 100644 --- a/substrate/frame/transaction-payment/src/benchmarking.rs +++ b/substrate/frame/transaction-payment/src/benchmarking.rs @@ -20,13 +20,29 @@ extern crate alloc; use super::*; -use crate::Pallet; use frame_benchmarking::v2::*; use frame_support::dispatch::{DispatchInfo, PostDispatchInfo}; use frame_system::{EventRecord, RawOrigin}; use sp_runtime::traits::{AsTransactionAuthorizedOrigin, DispatchTransaction, Dispatchable}; -fn assert_last_event(generic_event: ::RuntimeEvent) { +/// Re-export the pallet for benchmarking with custom Config trait. +pub struct Pallet(crate::Pallet); + +/// Benchmark configuration trait. +/// +/// This extends the pallet's Config trait to allow runtimes to set up any +/// required state before running benchmarks. For example, runtimes that +/// distribute fees to block authors may need to set the author before +/// the benchmark runs. +pub trait Config: crate::Config { + /// Called at the start of each benchmark to set up any required state. + /// + /// The default implementation is a no-op. Runtimes can override this + /// to perform setup like setting the block author for fee distribution. + fn setup_benchmark_environment() {} +} + +fn assert_last_event(generic_event: ::RuntimeEvent) { let events = frame_system::Pallet::::events(); let system_event: ::RuntimeEvent = generic_event.into(); // compare to the last event record @@ -44,19 +60,17 @@ mod benchmarks { #[benchmark] fn charge_transaction_payment() { + T::setup_benchmark_environment(); + let caller: T::AccountId = account("caller", 0, 0); let existential_deposit = >::minimum_balance(); - let (amount_to_endow, tip) = if existential_deposit.is_zero() { - let min_tip: <::OnChargeTransaction as payment::OnChargeTransaction>::Balance = 1_000_000_000u32.into(); - (min_tip * 1000u32.into(), min_tip) - } else { - (existential_deposit * 1000u32.into(), existential_deposit) - }; - - >::endow_account(&caller, amount_to_endow); + // Use a reasonable minimum tip that works for most runtimes + let min_tip: BalanceOf = 1_000_000_000u32.into(); + let tip = if existential_deposit.is_zero() { min_tip } else { existential_deposit }; + // Build the call and dispatch info first so we can compute the actual fee let ext: ChargeTransactionPayment = ChargeTransactionPayment::from(tip); let inner = frame_system::Call::remark { remark: alloc::vec![] }; let call = T::RuntimeCall::from(inner); @@ -72,18 +86,32 @@ mod benchmarks { pays_fee: Pays::Yes, }; + // Calculate the actual fee that will be charged, then endow enough to cover it + // with a 10x buffer to account for any fee multiplier variations. + // Ensure we endow at least the existential deposit so the account can exist. + let len: u32 = 10; + let expected_fee = crate::Pallet::::compute_fee(len, &info, tip); + let amount_to_endow = expected_fee.max(existential_deposit).saturating_mul(10u32.into()); + + >::endow_account(&caller, amount_to_endow); + #[block] { assert!(ext - .test_run(RawOrigin::Signed(caller.clone()).into(), &call, &info, 10, 0, |_| Ok( - post_info - )) + .test_run( + RawOrigin::Signed(caller.clone()).into(), + &call, + &info, + len as usize, + 0, + |_| Ok(post_info) + ) .unwrap() .is_ok()); } post_info.actual_weight.as_mut().map(|w| w.saturating_accrue(extension_weight)); - let actual_fee = Pallet::::compute_actual_fee(10, &info, &post_info, tip); + let actual_fee = crate::Pallet::::compute_actual_fee(len, &info, &post_info, tip); assert_last_event::( Event::::TransactionFeePaid { who: caller, actual_fee, tip }.into(), ); diff --git a/substrate/frame/transaction-payment/src/lib.rs b/substrate/frame/transaction-payment/src/lib.rs index 4c20b0fdd298f..40d0cfd97609e 100644 --- a/substrate/frame/transaction-payment/src/lib.rs +++ b/substrate/frame/transaction-payment/src/lib.rs @@ -78,7 +78,7 @@ mod mock; mod tests; #[cfg(feature = "runtime-benchmarks")] -mod benchmarking; +pub mod benchmarking; mod payment; mod types;