Skip to content

Benchmark instances of pallet-collective#3207

Closed
snowmead wants to merge 6 commits intomoonbeam-foundation:masterfrom
snowmead:benchmark-pallet-collective
Closed

Benchmark instances of pallet-collective#3207
snowmead wants to merge 6 commits intomoonbeam-foundation:masterfrom
snowmead:benchmark-pallet-collective

Conversation

@snowmead
Copy link
Copy Markdown
Contributor

@snowmead snowmead commented Mar 5, 2025

What does it do?

Adds pallet-collective instances to all runtime benchmarks.

Added a temporary workaround to support benchmarking different pallet instances until we upgrade polkadot-sdk.

Is there something left for follow-up PRs?

We should remove the workaround after our polkadot-sdk fork has been upgraded to stable2503.

The fix was done in this PR paritytech/polkadot-sdk#6435

We also need to benchmark both instances of pallet-collective: pallet_collective and pallet_collective_open_tech_committee.

@snowmead snowmead marked this pull request as ready for review March 6, 2025 18:28
@snowmead snowmead requested review from a team as code owners March 6, 2025 18:28
Copy link
Copy Markdown
Contributor

@RomarQ RomarQ left a comment

Choose a reason for hiding this comment

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

Added a few remarks

[pallet_parameters, Parameters]
[pallet_xcm_weight_trader, XcmWeightTrader]
// pallet_collective instances
[pallet_collective, TreasuryCouncilCollective]
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.

Would be better to also rename this one:

Suggested change
[pallet_collective, TreasuryCouncilCollective]
[pallet_collective_treasury_council, TreasuryCouncilCollective]

Comment on lines +19 to +26
{
benchmark_type_aliases: {
$($bench_type_aliases:tt)*
}
custom_impls: {
$($custom:tt)*
}
} => {
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.

If we no longer need this, lets revert it. It will conflict with the changes I will add for the bridge mvp.

@snowmead
Copy link
Copy Markdown
Contributor Author

snowmead commented Mar 7, 2025

Opening PR from a non-fork

@snowmead snowmead closed this Mar 7, 2025
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.

2 participants