From 4f393a140ed12c6f2acec212e325b5d331a78ec7 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Fri, 12 Sep 2025 14:27:55 +0100 Subject: [PATCH 1/4] simulate rank_to_vote --- .../frame/ranked-collective/src/benchmarking.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/substrate/frame/ranked-collective/src/benchmarking.rs b/substrate/frame/ranked-collective/src/benchmarking.rs index da402d48d0419..c59425c0188e6 100644 --- a/substrate/frame/ranked-collective/src/benchmarking.rs +++ b/substrate/frame/ranked-collective/src/benchmarking.rs @@ -191,6 +191,16 @@ mod benchmarks { // Create a caller based on the rank. let caller = make_member::(rank); + // Simulate the rank_to_votes conversion process + let vote_weight = if let Some(ref class) = class { + let min_rank = T::MinRankOfClass::convert(class.clone()); + let member_rank = Members::::get(&caller).map(|m| m.rank).unwrap_or(0); + let excess = member_rank.checked_sub(min_rank).unwrap_or(0); + T::VoteWeight::convert(excess) + } else { + 0 + }; + // Determine the poll to use: create an ongoing poll if class exists, or use an invalid // poll. let poll = if let Some(ref class) = class { @@ -227,8 +237,8 @@ mod benchmarks { // If the class exists, verify the vote event and tally. if let Some(_) = class { - let tally = Tally::from_parts(0, 0, 1); - let vote_event = Event::Voted { who: caller, poll, vote: VoteRecord::Nay(1), tally }; + let tally = Tally::from_parts(0, 0, vote_weight); + let vote_event = Event::Voted { who: caller, poll, vote: VoteRecord::Nay(vote_weight), tally }; assert_last_event::(vote_event.into()); } From 7f6d6f4679aefe2035ca032953ac009e2a55120c Mon Sep 17 00:00:00 2001 From: doordashcon Date: Fri, 12 Sep 2025 21:34:38 +0100 Subject: [PATCH 2/4] add prdoc --- prdoc/pr_9731.prdoc | 14 ++++++++++++++ .../ranked-collective/src/benchmarking.rs | 19 ++++++++++--------- 2 files changed, 24 insertions(+), 9 deletions(-) create mode 100644 prdoc/pr_9731.prdoc diff --git a/prdoc/pr_9731.prdoc b/prdoc/pr_9731.prdoc new file mode 100644 index 0000000000000..faacd7faf07c0 --- /dev/null +++ b/prdoc/pr_9731.prdoc @@ -0,0 +1,14 @@ +title: simulate `rank_to_votes` conversion in vote benchmark + +doc: +- audience: Runtime Dev + description: | + Fixes benchmarking failures for the vote extrinsic in pallet-ranked-collective by properly + simulating the rank_to_votes conversion process. Previously used hardcoded vote values + which caused assertion failures when using custom VoteWeight converters (e.g., in + [Ambassador Collective configuration](https://github.com/polkadot-fellows/runtimes/pull/736)). The fix calculates vote weight based on member's + actual rank and minimum required rank for the class. + +crates: +- name: pallet-ranked-collective + bump: patch \ No newline at end of file diff --git a/substrate/frame/ranked-collective/src/benchmarking.rs b/substrate/frame/ranked-collective/src/benchmarking.rs index c59425c0188e6..94b9768afc9de 100644 --- a/substrate/frame/ranked-collective/src/benchmarking.rs +++ b/substrate/frame/ranked-collective/src/benchmarking.rs @@ -192,14 +192,14 @@ mod benchmarks { let caller = make_member::(rank); // Simulate the rank_to_votes conversion process - let vote_weight = if let Some(ref class) = class { - let min_rank = T::MinRankOfClass::convert(class.clone()); - let member_rank = Members::::get(&caller).map(|m| m.rank).unwrap_or(0); - let excess = member_rank.checked_sub(min_rank).unwrap_or(0); - T::VoteWeight::convert(excess) - } else { - 0 - }; + let vote_weight = if let Some(ref class) = class { + let min_rank = T::MinRankOfClass::convert(class.clone()); + let member_rank = Members::::get(&caller).map(|m| m.rank).unwrap_or(0); + let excess = member_rank.checked_sub(min_rank).unwrap_or(0); + T::VoteWeight::convert(excess) + } else { + 0 + }; // Determine the poll to use: create an ongoing poll if class exists, or use an invalid // poll. @@ -238,7 +238,8 @@ mod benchmarks { // If the class exists, verify the vote event and tally. if let Some(_) = class { let tally = Tally::from_parts(0, 0, vote_weight); - let vote_event = Event::Voted { who: caller, poll, vote: VoteRecord::Nay(vote_weight), tally }; + let vote_event = + Event::Voted { who: caller, poll, vote: VoteRecord::Nay(vote_weight), tally }; assert_last_event::(vote_event.into()); } From 8813601eed3c0635c017cd38b6ec8e4e886904a2 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Fri, 12 Sep 2025 21:35:49 +0100 Subject: [PATCH 3/4] nit --- prdoc/pr_9731.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_9731.prdoc b/prdoc/pr_9731.prdoc index faacd7faf07c0..5ce5576d95fcc 100644 --- a/prdoc/pr_9731.prdoc +++ b/prdoc/pr_9731.prdoc @@ -4,7 +4,7 @@ doc: - audience: Runtime Dev description: | Fixes benchmarking failures for the vote extrinsic in pallet-ranked-collective by properly - simulating the rank_to_votes conversion process. Previously used hardcoded vote values + simulating the `rank_to_votes` conversion process. Previously used hardcoded vote values which caused assertion failures when using custom VoteWeight converters (e.g., in [Ambassador Collective configuration](https://github.com/polkadot-fellows/runtimes/pull/736)). The fix calculates vote weight based on member's actual rank and minimum required rank for the class. From 5b777ca370647a0eb6ba1331102d834be885b212 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 15 Sep 2025 09:39:00 +0200 Subject: [PATCH 4/4] Take the vote weight from the event --- .../ranked-collective/src/benchmarking.rs | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/substrate/frame/ranked-collective/src/benchmarking.rs b/substrate/frame/ranked-collective/src/benchmarking.rs index 94b9768afc9de..34f4205ee67ce 100644 --- a/substrate/frame/ranked-collective/src/benchmarking.rs +++ b/substrate/frame/ranked-collective/src/benchmarking.rs @@ -58,7 +58,9 @@ fn make_member, I: 'static>(rank: Rank) -> T::AccountId { } #[instance_benchmarks( -where <>::Polls as frame_support::traits::Polling>>>::Index: From +where + <>::Polls as frame_support::traits::Polling>>>::Index: From, + ::RuntimeEvent: TryInto>, )] mod benchmarks { use super::*; @@ -191,16 +193,6 @@ mod benchmarks { // Create a caller based on the rank. let caller = make_member::(rank); - // Simulate the rank_to_votes conversion process - let vote_weight = if let Some(ref class) = class { - let min_rank = T::MinRankOfClass::convert(class.clone()); - let member_rank = Members::::get(&caller).map(|m| m.rank).unwrap_or(0); - let excess = member_rank.checked_sub(min_rank).unwrap_or(0); - T::VoteWeight::convert(excess) - } else { - 0 - }; - // Determine the poll to use: create an ongoing poll if class exists, or use an invalid // poll. let poll = if let Some(ref class) = class { @@ -237,10 +229,22 @@ mod benchmarks { // If the class exists, verify the vote event and tally. if let Some(_) = class { - let tally = Tally::from_parts(0, 0, vote_weight); - let vote_event = - Event::Voted { who: caller, poll, vote: VoteRecord::Nay(vote_weight), tally }; - assert_last_event::(vote_event.into()); + // Get the actual vote weight from the latest event's VoteRecord::Nay + let mut events = frame_system::Pallet::::events(); + let last_event = events.pop().expect("At least one event should exist"); + let event: Event = last_event + .event + .try_into() + .unwrap_or_else(|_| panic!("Event conversion failed")); + + match event { + Event::Voted { vote: VoteRecord::Nay(vote_weight), who, poll: poll2, tally } => { + assert_eq!(tally, Tally::from_parts(0, 0, vote_weight)); + assert_eq!(caller, who); + assert_eq!(poll, poll2); + }, + _ => panic!("Invalid event"), + }; } Ok(())