-
Notifications
You must be signed in to change notification settings - Fork 380
Runtime: Success value and reachable location for Polkadot Collectives benchmarks #2784
Conversation
@@ -106,12 +106,15 @@ impl pallet_ranked_collective::Config<FellowshipCollectiveInstance> for Runtime | |||
type WeightInfo = weights::pallet_ranked_collective::WeightInfo<Runtime>; | |||
type RuntimeEvent = RuntimeEvent; | |||
// Promotions and the induction of new members are serviced by `FellowshipCore` pallet instance. | |||
type PromoteOrigin = EnsureRootWithSuccess<Self::AccountId, ConstU16<{ ranks::DAN_9 }>>; | |||
// The maximum value of `u16` set as a success value for the root to ensure the benchmarks will pass. | |||
type PromoteOrigin = EnsureRootWithSuccess<Self::AccountId, ConstU16<65535>>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we have this behind #[cfg(feature = "runtime-benchmarks")]
and have DAN_9 if the feature is not enabled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this was pulled out into a type def then that type could also be referenced in the DemoteOrigin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I liked this way for simplicity, both ways looks ok to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i added a separate promote origin for benches, please approve if looks good
parachains/runtimes/collectives/collectives-polkadot/src/fellowship/mod.rs
Outdated
Show resolved
Hide resolved
to my comment above,
for pallet_xcm_benchmarks is it probably ok, because pallet_xcm can really use Parent in calls, |
@bkontur thanks! makes total sense to me, i am checking how to make it work more accurate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@muharem
I cannot say if ranks::DAN_9
-> 65535
is ok or not, but besides that the rest lgtm
@bkchr needs a locks review... |
bot merge |
This Pull Request addresses two issues encountered during the benchmark run:
Ranked collectives benchmarks:
An error occurs when the benchmarks attempt to promote a member above rank 9.
To resolve this, we have set the success value for the Root as the maximum value of u16.
We have employed a similar approach in the Polkadot runtime.
Salary pallet benchmarks:
In the benchmark environment, sending a message to a Parachain fails due to the absence of a channel between the Parachains.
To address this, we wrap the Pay implementation to open a channel with ensure_successful.