Skip to content

test: init benchmark infra#110

Merged
undercover-cactus merged 52 commits intomainfrom
test/benchmark-init
Aug 6, 2025
Merged

test: init benchmark infra#110
undercover-cactus merged 52 commits intomainfrom
test/benchmark-init

Conversation

@elfedy
Copy link
Copy Markdown
Contributor

@elfedy elfedy commented Jun 26, 2025

  • Add run_benchmarks.sh script to run runtime benchmarks
  • Sets up benchmark configs and directory structure to store weights (operator/runtime/<RUNTIME>/weights)
  • (naive) fixes to some benchmarks:
    • pallet_datahaven_native_transfer:
      • use a mock for NativeTokenId
      • look at the balance difference of the treasury instead of the total (this makes the benchmark agnostic to genesis setup)
    • snowbridge_pallet_system / snowbridge_pallet_system_v2 use native token xcm location vs relay chain one. Add missing benchmark methods and update fixture with valid data.
    • snowbridge_pallet_ethereum_client: update fixtures with valid data
    • snowbrige_pallet_inbound_queue_v2: set EthereumGatewayAddress when initializing storage on benchmark and use a mock message processor ( as fixture has CreateAsset payload which is not supported in the EigenLayerMessageProcessor)
    • snowbridge_pallet_outbound_queue_v2: add missing submit_delivery_receipt benchmark which required a dedicated fixture (all copied from the upstream pallet)
    • pallet_treasury: Use an ExistentialDeposit of 1 on benchmark, else payout fails.
    • pallet_transaction_payment: Use a custom WeightToFee that makes the Fee small, else account in benchmark cannot pay for fees (It is funded a multiplier of ExistentialDeposit and is expected for that to be enough, but it's not in our particular setup).
  • comment out pallet_identity and pallet_im_online due to incompatibilities (to be addressed later)
  • Basic benchmark run to set WeightInfo from weights in configs (real run should be done later using target hardware)

@TDemeco
Copy link
Copy Markdown
Contributor

TDemeco commented Jul 25, 2025

FYI, the issues regarding pallet-identity and pallet-im-online are fixed here and here respectively, the only things missing are:

  • Propagating the fix for pallet-identity forwards to stable2503 as well, so when you update to it the benchmarks don't fail again. For stable2506 this is not needed as another fix was implemented, so keep in mind when updating to that release that you'll have to update your signature method for benchmarks.
  • Backporting the fix for pallet-im-online to stable2412 and stable2503. For stable2506 it's probably not needed as my fix will be included in the next patch which is probably going to be released in around two to three weeks

Copy link
Copy Markdown
Contributor

@stiiifff stiiifff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks @elfedy !

echo ""

# Track success/failure
declare -A RESULTS
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-A fails on MacOS ... I put a lowercase -a and then all is fine.

// working with the relay chain native token. We will need to adapt to that
// benchmark when we move to using the upstream pallet
let native_token_location: Location = Location::here();
let asset = Box::new(VersionedLocation::from(native_token_location));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to open a ticket for that ?

@undercover-cactus
Copy link
Copy Markdown
Contributor

I tried to run the script and I have this error. How can I get it to work ?

error[E0658]: use of unstable library feature `unsigned_is_multiple_of`
   --> /home/lola/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/yamux-0.13.6/src/frame/header.rs:301:16
    |
301 |         self.0.is_multiple_of(2)
    |                ^^^^^^^^^^^^^^
    |
    = note: see issue #128101 <https://github.com/rust-lang/rust/issues/128101> for more information

@elfedy
Copy link
Copy Markdown
Contributor Author

elfedy commented Jul 29, 2025

I tried to run the script and I have this error. How can I get it to work ?

Did you try building with rust nightly?

@undercover-cactus
Copy link
Copy Markdown
Contributor

I tried to run the script and I have this error. How can I get it to work ?

Did you try building with rust nightly?

My issue was just that my version of rust was a bit old. I did a rustup update and it works.

@undercover-cactus
Copy link
Copy Markdown
Contributor

I have updated 2412 to the latest release. I uncommented IAmOnline and Identity to test benchmarking. The first one is working fine but Identity cannot complete the benchmark.

Here the error :

2025-08-04T10:47:32.710572Z  INFO frame::benchmark::pallet: [ 58 % ] Starting benchmark: pallet_identity::set_username_for    
2025-08-04T10:47:32.712993Z ERROR evm: Error recovering: Incorrect value of V    
2025-08-04T10:47:32.713002Z ERROR runtime: panicked at /home/lola/.cargo/git/checkouts/polkadot-sdk-dee0edd6eefa0594/247f2e4/substrate/frame/identity/src/benchmarking.rs:639:3:
assertion failed: signature.verify(&bounded_username[..], &who_account)    
2025-08-04T10:47:32.713432Z ERROR frame::benchmark::pallet: Benchmark pallet_identity::set_username_for failed: Invalid input: Could not call runtime API to dispatch a benchmark: Execution aborted due to trap: wasm trap: wasm `unreachable` instruction executed
WASM backtrace:
error while executing at wasm backtrace:
    0: 0x4c3632 - datahaven_testnet_runtime.wasm!rust_begin_unwind
    1: 0xf44b - datahaven_testnet_runtime.wasm!core::panicking::panic_fmt::h6062c143c5ccad95
    2: 0xf4f4 - datahaven_testnet_runtime.wasm!core::panicking::panic::h3d27c5c0abc1adf0
    3: 0xa8df4 - datahaven_testnet_runtime.wasm!<pallet_identity::benchmarking::benchmarks::SelectedBenchmark as frame_benchmarking::utils::BenchmarkingSetup<T>>::instance::h37b58c7b3d708f44
    4: 0x2f3b22 - datahaven_testnet_runtime.wasm!pallet_identity::benchmarking::benchmarks::<impl frame_benchmarking::utils::Benchmarking for pallet_identity::pallet::Pallet<T>>::run_benchmark::h19890cc4d23814f6
    5: 0x41287 - datahaven_testnet_runtime.wasm!<datahaven_testnet_runtime::Runtime as frame_benchmarking::utils::runtime_decl_for_benchmark::BenchmarkV1<sp_runtime::generic::block::Block<sp_runtime::generic::header::Header<u32,sp_runtime::traits::BlakeTwo256>,fp_self_contained::unchecked_extrinsic::UncheckedExtrinsic<fp_account::AccountId20,datahaven_testnet_runtime::RuntimeCall,fp_account::EthereumSignature,(frame_system::extensions::check_non_zero_sender::CheckNonZeroSender<datahaven_testnet_runtime::Runtime>,frame_system::extensions::check_spec_version::CheckSpecVersion<datahaven_testnet_runtime::Runtime>,frame_system::extensions::check_tx_version::CheckTxVersion<datahaven_testnet_runtime::Runtime>,frame_system::extensions::check_genesis::CheckGenesis<datahaven_testnet_runtime::Runtime>,frame_system::extensions::check_mortality::CheckMortality<datahaven_testnet_runtime::Runtime>,frame_system::extensions::check_nonce::CheckNonce<datahaven_testnet_runtime::Runtime>,frame_system::extensions::check_weight::CheckWeight<datahaven_testnet_runtime::Runtime>,pallet_transaction_payment::ChargeTransactionPayment<datahaven_testnet_runtime::Runtime>,frame_metadata_hash_extension::CheckMetadataHash<datahaven_testnet_runtime::Runtime>)>>>>::dispatch_benchmark::h6dd873c93a159c5e
    6: 0x1861ad - datahaven_testnet_runtime.wasm!Benchmark_dispatch_benchmark    

The verify function is failing. @TDemeco Any idea why it would fail ?

@elfedy
Copy link
Copy Markdown
Contributor Author

elfedy commented Aug 4, 2025

The verify function is failing.

@undercover-cactus did you implement the benchmark_helper for the runtimes? I believe you need to create an Ethereum signature else the default is sr25519 which is an invalid signature scheme for us.

@undercover-cactus
Copy link
Copy Markdown
Contributor

I have implemented it.

I don't have an error with the wrong V anymore but I still can't have it working. After some digging, I think we could have an issue with AccountId20.

In order to get the account the benchmark code use into_account().

		// Use benchmark_helper to generate the signature
		let (public_bytes, signature_bytes) = T::benchmark_helper(&bounded_username[..]);

		// Decode the public key and signature
		let public = T::SigningPublicKey::decode(&mut &public_bytes[..])
			.expect("benchmark_helper should return valid encoded public key");
		let who_account: T::AccountId = public.into_account();
		let who_lookup = T::Lookup::unlookup(who_account.clone());

However when we look at the implementation of into_account() for AccountId20 it doesn't do the keccak256 hashing
https://github.com/polkadot-evm/frontier/blob/master/primitives/account/src/lib.rs#L252-L257

We end up with the who_account being just the 20 first bytes of the public key. And because of that the verify call fail.

Copy link
Copy Markdown
Contributor

@TDemeco TDemeco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update this and don't forget to re-run all benchmarks for all runtimes to include pallet-im-online and pallet-identity weights

@undercover-cactus undercover-cactus merged commit 843502d into main Aug 6, 2025
100 of 106 checks passed
@undercover-cactus undercover-cactus deleted the test/benchmark-init branch August 6, 2025 11:44
stiiifff pushed a commit that referenced this pull request Aug 7, 2025
* Add `run_benchmarks.sh` script to run runtime benchmarks
* Sets up benchmark configs and directory structure to store weights
(`operator/runtime/<RUNTIME>/weights`)
* (naive) fixes to some benchmarks: 
    * `pallet_datahaven_native_transfer`: 
      * use a mock for `NativeTokenId`
* look at the balance difference of the treasury instead of the total
(this makes the benchmark agnostic to genesis setup)
* `snowbridge_pallet_system` / `snowbridge_pallet_system_v2` use native
token xcm location vs relay chain one. Add missing benchmark methods and
update fixture with valid data.
* `snowbridge_pallet_ethereum_client`: update fixtures with valid data
* `snowbrige_pallet_inbound_queue_v2`: set EthereumGatewayAddress when
initializing storage on benchmark and use a mock message processor ( as
fixture has `CreateAsset` payload which is not supported in the
`EigenLayerMessageProcessor`)
* `snowbridge_pallet_outbound_queue_v2`: add missing
`submit_delivery_receipt` benchmark which required a dedicated fixture
(all copied from the upstream pallet)
* `pallet_treasury`: Use an `ExistentialDeposit` of `1` on benchmark,
else payout fails.
* `pallet_transaction_payment`: Use a custom `WeightToFee` that makes
the Fee small, else account in benchmark cannot pay for fees (It is
funded a multiplier of `ExistentialDeposit` and is expected for that to
be enough, but it's not in our particular setup).
* comment out `pallet_identity` and `pallet_im_online` due to
incompatibilities (to be addressed later)
* Basic benchmark run to set `WeightInfo` from `weights` in configs
(real run should be done later using target hardware)

---------

Co-authored-by: Ahmad Kaouk <[email protected]>
Co-authored-by: Tobi Demeco <[email protected]>
Co-authored-by: undercover-cactus <[email protected]>
Co-authored-by: TDemeco <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants