Improve `charge_transaction_payment benchmark` ergonomics #10444

manuelmauro · 2025-11-27T11:16:43Z

Description

Runtimes that distribute transaction fees to block authors (like Moonbeam) fail on the charge_transaction_payment benchmark because no author is set when the benchmark runs. The fee distribution logic panics when trying to credit a non-existent author.

This PR introduces a benchmarking::Config trait with a setup_benchmark_environment() hook that runtimes can implement to set up required state before the benchmark executes.

Additionally, amount_to_endow is now calculated using compute_fee() to determine the actual fee (with a 10x buffer), ensuring it is at least the existential deposit.

Integration

Runtimes that need to set up state before running the charge_transaction_payment benchmark should implement pallet_transaction_payment::benchmarking::Config:

+ impl pallet_transaction_payment::benchmarking::Config for Runtime {
+     fn setup_benchmark_environment() {
+         // Set up any required state, e.g., block author for fee distribution
+         let author: AccountId = frame_benchmarking::whitelisted_caller();
+         pallet_author_inherent::Author::<Runtime>::put(author);
+     }
+ }

And update the benchmark list to use the wrapper type:

- [pallet_transaction_payment, TransactionPayment]
+ [pallet_transaction_payment, TransactionPaymentBenchmark::<Runtime>]

Runtimes that don't need custom setup can use the default implementation (no-op).

Review Notes

A new benchmarking::Config trait extends crate::Config with a setup_benchmark_environment() method (default no-op)
A wrapper Pallet<T> struct is introduced in the benchmarking module to use this extended trait
The benchmark calls T::setup_benchmark_environment() at the start
amount_to_endow is calculated as compute_fee(...).saturating_mul(10).max(existential_deposit) to ensure the account can exist and has sufficient funds

Checklist

My PR includes a detailed description as outlined in the "Description" and its two subsections above.
My PR follows the labeling requirements of this project (at minimum one label for T required)
- External contributors: Use /cmd label <label-name> to add labels
- Maintainers can also add labels manually
I have made corresponding changes to the documentation (if applicable)
I have added tests that prove my fix is effective or that my feature works (if applicable)

manuelmauro · 2025-11-27T11:17:06Z

/cmd label T1-FRAME

manuelmauro · 2025-11-27T11:17:30Z

/cmd prdoc

github-actions · 2025-11-27T11:19:27Z

Command "label T1-FRAME" has failed ❌! See logs here

github-actions · 2025-11-27T11:20:36Z

Command "prdoc" has failed ❌! See logs here

bkchr · 2025-11-27T12:56:39Z

substrate/frame/transaction-payment/src/benchmarking.rs

+/// Re-export the pallet for benchmarking with custom Config trait.
+pub struct Pallet<T: Config>(crate::Pallet<T>);


Why do we need this?

You can see and example use case here: moonbeam-foundation/moonbeam#3558

bkchr · 2025-11-27T12:57:11Z

substrate/frame/transaction-payment/src/benchmarking.rs

+/// 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 {


Can we not just name this BenchmarkConfig? and then also put this below into the where_bound?

Follows a similar pattern as https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/xcm/pallet-xcm/src/benchmarking.rs#L27

Yeah could you try to just implement the benchmarking trait for the "normal Pallet"? Maybe it doesn't work, but right now I don't see the need for declaring this extra pallet struct.

The downside of this approach is that everyone would need to implement the new Config trait ext even if they do not need any special setup.

For instance we'd need to add the following to /substrate/frame/transaction-payment/src/mock.rs

#[cfg(feature = "runtime-benchmarks")] impl crate::benchmarking::Config for Runtime {}

bkchr · 2025-11-27T12:58:47Z

substrate/frame/transaction-payment/src/lib.rs


 #[cfg(feature = "runtime-benchmarks")]
-mod benchmarking;
+pub mod benchmarking;


Probably enough to just export the config, instead of making the entire module public

Similarly to https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/xcm/pallet-xcm/src/lib.rs#L22 but I am happy to change it

bkchr · 2025-11-27T13:00:58Z

substrate/frame/transaction-payment/src/benchmarking.rs

-		let (amount_to_endow, tip) = if existential_deposit.is_zero() {
-			let min_tip: <<T as pallet::Config>::OnChargeTransaction as payment::OnChargeTransaction<T>>::Balance = 1_000_000_000u32.into();
-			(min_tip * 1000u32.into(), min_tip)


Why did you change this?

The amount to endow is computed dynamically later on, while the tip logic stays the same.

bkchr · 2025-11-27T13:01:22Z

substrate/frame/transaction-payment/src/benchmarking.rs

+		let len: u32 = 10;
+		let expected_fee = crate::Pallet::<T>::compute_fee(len, &info, tip);
+		let amount_to_endow = expected_fee
+			.saturating_mul(10u32.into())


We should multiply by ED if not zero.

I am sorry, I am not sure I understand why we need to do this. Could you please expand on the rationale? Happy to make this change.

Fixed a problem with the computation here: 204b3e0

fix: ⚡ fix charge_transaction_payment benchmark

63e82f2

manuelmauro requested a review from a team as a code owner November 27, 2025 11:16

manuelmauro added a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Nov 27, 2025

Add prdoc for PR paritytech#10444

3be0033

Add prdoc for PR paritytech#10444

f825eab

manuelmauro force-pushed the fix-charge-transaction-payment-benchmark branch from 3be0033 to f825eab Compare November 27, 2025 11:24

bkchr added the T2-pallets This PR/Issue is related to a particular pallet. label Nov 27, 2025

bkchr reviewed Nov 27, 2025

View reviewed changes

manuelmauro added 2 commits November 27, 2025 15:44

fix: 🐛 fix amount_to_endow computation

204b3e0

refactor: 🎨 format code

07e44cf

		/// Re-export the pallet for benchmarking with custom Config trait.
		pub struct Pallet<T: Config>(crate::Pallet<T>);

Improve charge_transaction_payment benchmark ergonomics #10444

Are you sure you want to change the base?

Improve charge_transaction_payment benchmark ergonomics #10444

Conversation

manuelmauro commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Integration

Review Notes

Checklist

Uh oh!

manuelmauro commented Nov 27, 2025

Uh oh!

manuelmauro commented Nov 27, 2025

Uh oh!

github-actions bot commented Nov 27, 2025

Uh oh!

github-actions bot commented Nov 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Improve `charge_transaction_payment benchmark` ergonomics #10444

Improve `charge_transaction_payment benchmark` ergonomics #10444

manuelmauro commented Nov 27, 2025 •

edited

Loading