From 63e82f238b81645312dc800460fbd0790aff8585 Mon Sep 17 00:00:00 2001 From: Manuel Mauro Date: Thu, 27 Nov 2025 10:32:24 +0200 Subject: [PATCH 1/4] fix: :zap: fix charge_transaction_payment benchmark --- .../transaction-payment/src/benchmarking.rs | 49 ++++++++++++++----- .../frame/transaction-payment/src/lib.rs | 2 +- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/substrate/frame/transaction-payment/src/benchmarking.rs b/substrate/frame/transaction-payment/src/benchmarking.rs index 70ecfbf149e57..ca87f01aa6073 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,10 +86,21 @@ 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 + .saturating_mul(10u32.into()) + .max(existential_deposit); + + >::endow_account(&caller, amount_to_endow); + #[block] { assert!(ext - .test_run(RawOrigin::Signed(caller.clone()).into(), &call, &info, 10, 0, |_| Ok( + .test_run(RawOrigin::Signed(caller.clone()).into(), &call, &info, len as usize, 0, |_| Ok( post_info )) .unwrap() @@ -83,7 +108,7 @@ mod benchmarks { } 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; From f825eaba31bf9b00d323d1310d47e31fbb33a54f Mon Sep 17 00:00:00 2001 From: Manuel Mauro Date: Thu, 27 Nov 2025 13:22:49 +0200 Subject: [PATCH 2/4] Add prdoc for PR #10444 --- prdoc/pr_10444.prdoc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 prdoc/pr_10444.prdoc 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 From 204b3e064f3fa016eed24eaf77f8a3d65508d5a9 Mon Sep 17 00:00:00 2001 From: Manuel Mauro Date: Thu, 27 Nov 2025 15:44:08 +0200 Subject: [PATCH 3/4] fix: :bug: fix amount_to_endow computation --- substrate/frame/transaction-payment/src/benchmarking.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/transaction-payment/src/benchmarking.rs b/substrate/frame/transaction-payment/src/benchmarking.rs index ca87f01aa6073..8071124156d63 100644 --- a/substrate/frame/transaction-payment/src/benchmarking.rs +++ b/substrate/frame/transaction-payment/src/benchmarking.rs @@ -92,8 +92,8 @@ mod benchmarks { let len: u32 = 10; let expected_fee = crate::Pallet::::compute_fee(len, &info, tip); let amount_to_endow = expected_fee - .saturating_mul(10u32.into()) - .max(existential_deposit); + .max(existential_deposit) + .saturating_mul(10u32.into()); >::endow_account(&caller, amount_to_endow); From 07e44cf42f09e6e9f6bed3838a88bba73a7be8d9 Mon Sep 17 00:00:00 2001 From: Manuel Mauro Date: Thu, 27 Nov 2025 15:44:30 +0200 Subject: [PATCH 4/4] refactor: :art: format code --- .../frame/transaction-payment/src/benchmarking.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/substrate/frame/transaction-payment/src/benchmarking.rs b/substrate/frame/transaction-payment/src/benchmarking.rs index 8071124156d63..9969083cc1783 100644 --- a/substrate/frame/transaction-payment/src/benchmarking.rs +++ b/substrate/frame/transaction-payment/src/benchmarking.rs @@ -91,18 +91,21 @@ mod benchmarks { // 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()); + 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, len as usize, 0, |_| Ok( - post_info - )) + .test_run( + RawOrigin::Signed(caller.clone()).into(), + &call, + &info, + len as usize, + 0, + |_| Ok(post_info) + ) .unwrap() .is_ok()); }