NoOp Impl Polling Trait #5311
NoOp Impl Polling Trait #5311
Conversation
|
@Doordashcon do you have any reasoning for this? |
|
@bkchr just a follow up on this? |
The comment is saying that this no-op should be implemented on |
|
Implemented |
| @@ -126,7 +126,7 @@ benchmarks_instance_pallet! { | |||
| } | |||
|
|
|||
| vote { | |||
| let class = T::Polls::classes().into_iter().next().unwrap(); | |||
| let class = T::Polls::classes().into_iter().next().ok_or(BenchmarkError::Skip)?; | |||
There was a problem hiding this comment.
what weight Skip will result?
There was a problem hiding this comment.
The Secretary Collective requires skipping these extrinsics to generate weights for the ranked_collective pallet instance which uses this NoOp implementation.
There was a problem hiding this comment.
Yeah I have not checked the actual code, what we need to do here if classes() returns nothing, we need to pass some invalid poll index. The function is still trying to access the storage etc.
| @@ -126,7 +126,7 @@ benchmarks_instance_pallet! { | |||
| } | |||
|
|
|||
| vote { | |||
| let class = T::Polls::classes().into_iter().next().unwrap(); | |||
| let class = T::Polls::classes().into_iter().next().ok_or(BenchmarkError::Skip)?; | |||
There was a problem hiding this comment.
Yeah I have not checked the actual code, what we need to do here if classes() returns nothing, we need to pass some invalid poll index. The function is still trying to access the storage etc.
|
@Doordashcon can you please fix it as done in this pr #3049? |
updated other benchmarks as well, hopefully ready for review @muharem @bkchr 🙏 |
updates the
Pollingtrait implementation in tests.