From f9784337796ab903a6decbb72b0a8a12727612fa Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 22 Sep 2022 13:15:59 +0200 Subject: [PATCH 01/19] Improved election pallet testing --- .../election-provider-multi-phase/src/lib.rs | 155 ++++++++++++++++-- .../election-provider-multi-phase/src/mock.rs | 8 + 2 files changed, 146 insertions(+), 17 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 1188e3353bc96..b89c7c78c9da4 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -1827,9 +1827,9 @@ mod tests { use super::*; use crate::{ mock::{ - multi_phase_events, raw_solution, roll_to, AccountId, ExtBuilder, MockWeightInfo, - MockedWeightInfo, MultiPhase, Origin, Runtime, SignedMaxSubmissions, System, - TargetIndex, Targets, + multi_phase_events, raw_solution, roll_to, roll_to_elect, roll_to_unsigned, AccountId, + ExtBuilder, MockWeightInfo, MockedWeightInfo, MultiPhase, Origin, Runtime, + SignedMaxSubmissions, System, TargetIndex, Targets, }, Phase, }; @@ -1864,7 +1864,7 @@ mod tests { assert!(MultiPhase::snapshot().is_some()); assert_eq!(MultiPhase::round(), 1); - roll_to(25); + roll_to_unsigned(); assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); assert_eq!( multi_phase_events(), @@ -1879,7 +1879,7 @@ mod tests { assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); assert!(MultiPhase::snapshot().is_some()); - roll_to(30); + roll_to_elect(); assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); assert!(MultiPhase::snapshot().is_some()); @@ -1902,6 +1902,24 @@ mod tests { roll_to(55); assert!(MultiPhase::current_phase().is_unsigned_open_at(55)); + + assert_eq!( + multi_phase_events(), + vec![ + Event::SignedPhaseStarted { round: 1 }, + Event::UnsignedPhaseStarted { round: 1 }, + Event::ElectionFinalized { + compute: ElectionCompute::Fallback, + score: ElectionScore { + minimal_stake: 0, + sum_stake: 0, + sum_stake_squared: 0 + } + }, + Event::SignedPhaseStarted { round: 2 }, + Event::UnsignedPhaseStarted { round: 2 } + ] + ); }) } @@ -1918,13 +1936,28 @@ mod tests { assert!(MultiPhase::current_phase().is_unsigned_open_at(20)); assert!(MultiPhase::snapshot().is_some()); - roll_to(30); + roll_to_elect(); assert!(MultiPhase::current_phase().is_unsigned_open_at(20)); assert_ok!(MultiPhase::elect()); assert!(MultiPhase::current_phase().is_off()); assert!(MultiPhase::snapshot().is_none()); + + assert_eq!( + multi_phase_events(), + vec![ + Event::UnsignedPhaseStarted { round: 1 }, + Event::ElectionFinalized { + compute: ElectionCompute::Fallback, + score: ElectionScore { + minimal_stake: 0, + sum_stake: 0, + sum_stake_squared: 0 + } + } + ] + ); }); } @@ -1941,13 +1974,28 @@ mod tests { assert!(MultiPhase::current_phase().is_signed()); assert!(MultiPhase::snapshot().is_some()); - roll_to(30); + roll_to_elect(); assert!(MultiPhase::current_phase().is_signed()); assert_ok!(MultiPhase::elect()); assert!(MultiPhase::current_phase().is_off()); assert!(MultiPhase::snapshot().is_none()); + + assert_eq!( + multi_phase_events(), + vec![ + Event::SignedPhaseStarted { round: 1 }, + Event::ElectionFinalized { + compute: ElectionCompute::Fallback, + score: ElectionScore { + minimal_stake: 0, + sum_stake: 0, + sum_stake_squared: 0 + } + } + ] + ) }); } @@ -1963,13 +2011,22 @@ mod tests { roll_to(20); assert!(MultiPhase::current_phase().is_off()); - roll_to(30); + roll_to_elect(); assert!(MultiPhase::current_phase().is_off()); // This module is now only capable of doing on-chain backup. assert_ok!(MultiPhase::elect()); assert!(MultiPhase::current_phase().is_off()); + + // DOUBLE CHECK THIS + assert_eq!( + multi_phase_events(), + vec![Event::ElectionFinalized { + compute: ElectionCompute::Fallback, + score: ElectionScore { minimal_stake: 0, sum_stake: 0, sum_stake_squared: 0 } + }] + ); }); } @@ -2044,6 +2101,31 @@ mod tests { assert!(MultiPhase::desired_targets().is_none()); assert!(MultiPhase::queued_solution().is_none()); assert!(MultiPhase::signed_submissions().is_empty()); + + assert_eq!( + multi_phase_events(), + vec![ + Event::SignedPhaseStarted { round: 1 }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::Slashed { account: 99, value: 5 }, + Event::Slashed { account: 99, value: 5 }, + Event::Slashed { account: 99, value: 5 }, + Event::Slashed { account: 99, value: 5 }, + Event::Slashed { account: 99, value: 5 }, + Event::ElectionFinalized { + compute: ElectionCompute::Fallback, + score: ElectionScore { + minimal_stake: 0, + sum_stake: 0, + sum_stake_squared: 0 + } + } + ] + ); }) } @@ -2059,7 +2141,7 @@ mod tests { let solution = raw_solution(); assert_ok!(MultiPhase::submit(crate::mock::Origin::signed(99), Box::new(solution))); - roll_to(30); + roll_to_elect(); assert_ok!(MultiPhase::elect()); assert_eq!( @@ -2085,7 +2167,7 @@ mod tests { #[test] fn check_events_with_compute_unsigned() { ExtBuilder::default().build_and_execute(|| { - roll_to(25); + roll_to_unsigned(); assert!(MultiPhase::current_phase().is_unsigned()); // ensure we have snapshots in place. @@ -2104,7 +2186,7 @@ mod tests { )); assert!(MultiPhase::queued_solution().is_some()); - roll_to(30); + roll_to_elect(); assert_ok!(MultiPhase::elect()); assert_eq!( @@ -2132,7 +2214,7 @@ mod tests { #[test] fn fallback_strategy_works() { ExtBuilder::default().onchain_fallback(true).build_and_execute(|| { - roll_to(25); + roll_to_unsigned(); assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); // Zilch solutions thus far, but we get a result. @@ -2145,11 +2227,27 @@ mod tests { (30, Support { total: 40, voters: vec![(2, 5), (4, 5), (30, 30)] }), (40, Support { total: 60, voters: vec![(2, 5), (3, 10), (4, 5), (40, 40)] }) ] - ) + ); + + assert_eq!( + multi_phase_events(), + vec![ + Event::SignedPhaseStarted { round: 1 }, + Event::UnsignedPhaseStarted { round: 1 }, + Event::ElectionFinalized { + compute: ElectionCompute::Fallback, + score: ElectionScore { + minimal_stake: 0, + sum_stake: 0, + sum_stake_squared: 0 + } + } + ] + ); }); ExtBuilder::default().onchain_fallback(false).build_and_execute(|| { - roll_to(25); + roll_to_unsigned(); assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); // Zilch solutions thus far. @@ -2157,13 +2255,22 @@ mod tests { assert_eq!(MultiPhase::elect().unwrap_err(), ElectionError::Fallback("NoFallback.")); // phase is now emergency. assert_eq!(MultiPhase::current_phase(), Phase::Emergency); + + assert_eq!( + multi_phase_events(), + vec![ + Event::SignedPhaseStarted { round: 1 }, + Event::UnsignedPhaseStarted { round: 1 }, + Event::ElectionFailed + ] + ); }) } #[test] fn governance_fallback_works() { ExtBuilder::default().onchain_fallback(false).build_and_execute(|| { - roll_to(25); + roll_to_unsigned(); assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); // Zilch solutions thus far. @@ -2218,13 +2325,21 @@ mod tests { assert_eq!(MultiPhase::current_phase(), Phase::Off); // Unsigned phase failed to open. - roll_to(25); + roll_to_unsigned(); assert_eq!(MultiPhase::current_phase(), Phase::Off); // On-chain backup works though. roll_to(29); let supports = MultiPhase::elect().unwrap(); assert!(supports.len() > 0); + + assert_eq!( + multi_phase_events(), + vec![Event::ElectionFinalized { + compute: ElectionCompute::Fallback, + score: ElectionScore { minimal_stake: 0, sum_stake: 0, sum_stake_squared: 0 } + }] + ); }); } @@ -2241,13 +2356,15 @@ mod tests { assert_eq!(MultiPhase::current_phase(), Phase::Off); // Unsigned phase failed to open. - roll_to(25); + roll_to_unsigned(); assert_eq!(MultiPhase::current_phase(), Phase::Off); roll_to(29); let err = MultiPhase::elect().unwrap_err(); assert_eq!(err, ElectionError::Fallback("NoFallback.")); assert_eq!(MultiPhase::current_phase(), Phase::Emergency); + + assert_eq!(multi_phase_events(), vec![Event::ElectionFailed]); }); } @@ -2268,6 +2385,8 @@ mod tests { MultiPhase::snapshot_metadata().unwrap(), SolutionOrSnapshotSize { voters: 2, targets: 4 } ); + + assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } @@ -2298,6 +2417,8 @@ mod tests { MultiPhase::feasibility_check(solution, ElectionCompute::Signed), FeasibilityError::UntrustedScoreTooLow, ); + + assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index efe9d8ea587cb..559eed04ac17a 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -99,6 +99,14 @@ pub fn roll_to(n: BlockNumber) { } } +pub fn roll_to_unsigned() { + roll_to(25); +} + +pub fn roll_to_elect() { + roll_to(30); +} + pub fn roll_to_with_ocw(n: BlockNumber) { let now = System::block_number(); for i in now + 1..=n { From 2f2c8b545d2e778e72f1ae661644c573de5e1cc9 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 22 Sep 2022 13:21:46 +0200 Subject: [PATCH 02/19] fmt --- frame/election-provider-multi-phase/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 32a8a6615c3e5..a8d817761dd64 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -1827,9 +1827,9 @@ mod tests { use super::*; use crate::{ mock::{ - multi_phase_events, raw_solution, roll_to, roll_to_elect, roll_to_unsigned, AccountId, ExtBuilder, MockWeightInfo, - MockedWeightInfo, MultiPhase, Runtime, RuntimeOrigin, SignedMaxSubmissions, System, - TargetIndex, Targets, + multi_phase_events, raw_solution, roll_to, roll_to_elect, roll_to_unsigned, AccountId, + ExtBuilder, MockWeightInfo, MockedWeightInfo, MultiPhase, Runtime, RuntimeOrigin, + SignedMaxSubmissions, System, TargetIndex, Targets, }, Phase, }; From 0494a896b5fcdf7e45730ded1e61dec315f71190 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 22 Sep 2022 13:38:48 +0200 Subject: [PATCH 03/19] remove comment --- frame/election-provider-multi-phase/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index a8d817761dd64..b0f34770c199c 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -2019,7 +2019,6 @@ mod tests { assert!(MultiPhase::current_phase().is_off()); - // DOUBLE CHECK THIS assert_eq!( multi_phase_events(), vec![Event::ElectionFinalized { From d64de36490ae1988542e29398517dc5a7e6bcf9e Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 22 Sep 2022 13:59:08 +0200 Subject: [PATCH 04/19] more checks --- .../election-provider-multi-phase/src/lib.rs | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index b0f34770c199c..7d84f0b004949 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -1633,8 +1633,8 @@ mod feasibility_check { use super::*; use crate::mock::{ - raw_solution, roll_to, EpochLength, ExtBuilder, MultiPhase, Runtime, SignedPhase, - TargetIndex, UnsignedPhase, VoterIndex, + multi_phase_events, raw_solution, roll_to, EpochLength, ExtBuilder, MultiPhase, Runtime, + SignedPhase, TargetIndex, UnsignedPhase, VoterIndex, }; use frame_support::{assert_noop, assert_ok}; @@ -1654,6 +1654,8 @@ mod feasibility_check { MultiPhase::feasibility_check(solution, COMPUTE), FeasibilityError::SnapshotUnavailable ); + + assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } @@ -1669,6 +1671,8 @@ mod feasibility_check { MultiPhase::feasibility_check(solution, COMPUTE), FeasibilityError::InvalidRound ); + + assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } @@ -1686,6 +1690,8 @@ mod feasibility_check { // It should succeed assert_ok!(MultiPhase::feasibility_check(raw, COMPUTE)); + + assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } @@ -1709,6 +1715,8 @@ mod feasibility_check { MultiPhase::feasibility_check(raw, COMPUTE), FeasibilityError::WrongWinnerCount, ); + + assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } @@ -1742,6 +1750,8 @@ mod feasibility_check { MultiPhase::feasibility_check(raw, COMPUTE), FeasibilityError::NposElection(sp_npos_elections::Error::SolutionInvalidIndex) ); + + assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } @@ -1770,6 +1780,8 @@ mod feasibility_check { MultiPhase::feasibility_check(solution, COMPUTE), FeasibilityError::NposElection(sp_npos_elections::Error::SolutionInvalidIndex), ); + + assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } @@ -1799,6 +1811,8 @@ mod feasibility_check { MultiPhase::feasibility_check(solution, COMPUTE), FeasibilityError::InvalidVote, ); + + assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } @@ -1818,6 +1832,8 @@ mod feasibility_check { MultiPhase::feasibility_check(solution, COMPUTE), FeasibilityError::InvalidScore, ); + + assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } } From 0d3478f9d22e1747a0b268fbf8ad0d0e229e1826 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 22 Sep 2022 15:43:21 +0200 Subject: [PATCH 05/19] fixes in logic --- .../election-provider-multi-phase/src/lib.rs | 23 +++++++++---------- .../election-provider-multi-phase/src/mock.rs | 10 ++++---- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 7d84f0b004949..17617425d8de3 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -1843,8 +1843,8 @@ mod tests { use super::*; use crate::{ mock::{ - multi_phase_events, raw_solution, roll_to, roll_to_elect, roll_to_unsigned, AccountId, - ExtBuilder, MockWeightInfo, MockedWeightInfo, MultiPhase, Runtime, RuntimeOrigin, + multi_phase_events, raw_solution, roll_to, roll_to_unsigned, AccountId, ExtBuilder, + MockWeightInfo, MockedWeightInfo, MultiPhase, Runtime, RuntimeOrigin, SignedMaxSubmissions, System, TargetIndex, Targets, }, Phase, @@ -1895,7 +1895,7 @@ mod tests { assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); assert!(MultiPhase::snapshot().is_some()); - roll_to_elect(); + roll_to(15); assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); assert!(MultiPhase::snapshot().is_some()); @@ -1952,7 +1952,7 @@ mod tests { assert!(MultiPhase::current_phase().is_unsigned_open_at(20)); assert!(MultiPhase::snapshot().is_some()); - roll_to_elect(); + roll_to(15); assert!(MultiPhase::current_phase().is_unsigned_open_at(20)); assert_ok!(MultiPhase::elect()); @@ -1990,7 +1990,7 @@ mod tests { assert!(MultiPhase::current_phase().is_signed()); assert!(MultiPhase::snapshot().is_some()); - roll_to_elect(); + roll_to(15); assert!(MultiPhase::current_phase().is_signed()); assert_ok!(MultiPhase::elect()); @@ -2027,7 +2027,7 @@ mod tests { roll_to(20); assert!(MultiPhase::current_phase().is_off()); - roll_to_elect(); + roll_to(15); assert!(MultiPhase::current_phase().is_off()); // This module is now only capable of doing on-chain backup. @@ -2162,7 +2162,7 @@ mod tests { Box::new(solution) )); - roll_to_elect(); + roll_to(15); assert_ok!(MultiPhase::elect()); assert_eq!( @@ -2171,7 +2171,6 @@ mod tests { Event::SignedPhaseStarted { round: 1 }, Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, Event::Rewarded { account: 99, value: 7 }, - Event::UnsignedPhaseStarted { round: 1 }, Event::ElectionFinalized { compute: ElectionCompute::Signed, score: ElectionScore { @@ -2179,7 +2178,7 @@ mod tests { sum_stake: 100, sum_stake_squared: 5200 } - } + }, ], ); }) @@ -2207,7 +2206,7 @@ mod tests { )); assert!(MultiPhase::queued_solution().is_some()); - roll_to_elect(); + roll_to(15); assert_ok!(MultiPhase::elect()); assert_eq!( @@ -2346,7 +2345,7 @@ mod tests { assert_eq!(MultiPhase::current_phase(), Phase::Off); // Unsigned phase failed to open. - roll_to_unsigned(); + roll_to(25); assert_eq!(MultiPhase::current_phase(), Phase::Off); // On-chain backup works though. @@ -2377,7 +2376,7 @@ mod tests { assert_eq!(MultiPhase::current_phase(), Phase::Off); // Unsigned phase failed to open. - roll_to_unsigned(); + roll_to(25); assert_eq!(MultiPhase::current_phase(), Phase::Off); roll_to(29); diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 0fd4b4dc6bfda..69e3776305a05 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -99,12 +99,14 @@ pub fn roll_to(n: BlockNumber) { } } -pub fn roll_to_unsigned() { - roll_to(25); +fn roll_next() { + roll_to(System::block_number() + 1); } -pub fn roll_to_elect() { - roll_to(30); +pub fn roll_to_unsigned() { + while !matches!(MultiPhase::current_phase(), Phase::Unsigned(_)) { + roll_next(); + } } pub fn roll_to_with_ocw(n: BlockNumber) { From be05c37650607261a73c6e4b39faa4fac3897952 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 22 Sep 2022 15:48:22 +0200 Subject: [PATCH 06/19] roll_to_signed --- frame/election-provider-multi-phase/src/lib.rs | 10 +++++----- frame/election-provider-multi-phase/src/mock.rs | 5 +++++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 17617425d8de3..33a50756c086e 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -1843,8 +1843,8 @@ mod tests { use super::*; use crate::{ mock::{ - multi_phase_events, raw_solution, roll_to, roll_to_unsigned, AccountId, ExtBuilder, - MockWeightInfo, MockedWeightInfo, MultiPhase, Runtime, RuntimeOrigin, + multi_phase_events, raw_solution, roll_to, roll_to_signed, roll_to_unsigned, AccountId, + ExtBuilder, MockWeightInfo, MockedWeightInfo, MultiPhase, Runtime, RuntimeOrigin, SignedMaxSubmissions, System, TargetIndex, Targets, }, Phase, @@ -1913,7 +1913,7 @@ mod tests { roll_to(44); assert!(MultiPhase::current_phase().is_off()); - roll_to(45); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); roll_to(55); @@ -1986,7 +1986,7 @@ mod tests { roll_to(19); assert!(MultiPhase::current_phase().is_off()); - roll_to(20); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); assert!(MultiPhase::snapshot().is_some()); @@ -2153,7 +2153,7 @@ mod tests { roll_to(14); assert_eq!(MultiPhase::current_phase(), Phase::Off); - roll_to(15); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); let solution = raw_solution(); diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 69e3776305a05..4d5fb7ffcef0f 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -108,6 +108,11 @@ pub fn roll_to_unsigned() { roll_next(); } } +pub fn roll_to_signed() { + while !matches!(MultiPhase::current_phase(), Phase::Signed) { + roll_next(); + } +} pub fn roll_to_with_ocw(n: BlockNumber) { let now = System::block_number(); From a0e24ca73188d59d434d1a8f13dee6608c0278c4 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 22 Sep 2022 17:16:34 +0200 Subject: [PATCH 07/19] switch to roll_to_signed --- frame/election-provider-multi-phase/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 33a50756c086e..229e6e5604e31 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -1869,7 +1869,7 @@ mod tests { assert!(MultiPhase::snapshot().is_none()); assert_eq!(MultiPhase::round(), 1); - roll_to(15); + roll_to_signed(); assert_eq!(MultiPhase::current_phase(), Phase::Signed); assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); assert!(MultiPhase::snapshot().is_some()); @@ -2053,7 +2053,7 @@ mod tests { roll_to(14); assert_eq!(MultiPhase::current_phase(), Phase::Off); - roll_to(15); + roll_to_signed(); assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); assert_eq!(MultiPhase::current_phase(), Phase::Signed); assert_eq!(MultiPhase::round(), 1); @@ -2091,7 +2091,7 @@ mod tests { roll_to(14); assert_eq!(MultiPhase::current_phase(), Phase::Off); - roll_to(15); + roll_to_signed(); assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); assert_eq!(MultiPhase::current_phase(), Phase::Signed); assert_eq!(MultiPhase::round(), 1); @@ -2398,7 +2398,7 @@ mod tests { crate::mock::MaxElectingVoters::set(2); // Signed phase opens just fine. - roll_to(15); + roll_to_signed(); assert_eq!(MultiPhase::current_phase(), Phase::Signed); assert_eq!( @@ -2413,7 +2413,7 @@ mod tests { #[test] fn untrusted_score_verification_is_respected() { ExtBuilder::default().build_and_execute(|| { - roll_to(15); + roll_to_signed(); assert_eq!(MultiPhase::current_phase(), Phase::Signed); // set the solution balancing to get the desired score. From e861d57606caaebf35a6f4f738066264ae975130 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Sat, 24 Sep 2022 10:24:38 +0200 Subject: [PATCH 08/19] Update frame/election-provider-multi-phase/src/mock.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- frame/election-provider-multi-phase/src/mock.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 4d5fb7ffcef0f..ce772673feb1c 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -99,18 +99,14 @@ pub fn roll_to(n: BlockNumber) { } } -fn roll_next() { - roll_to(System::block_number() + 1); -} - pub fn roll_to_unsigned() { while !matches!(MultiPhase::current_phase(), Phase::Unsigned(_)) { - roll_next(); + roll_to(System::block_number() + 1); } } pub fn roll_to_signed() { while !matches!(MultiPhase::current_phase(), Phase::Signed) { - roll_next(); + roll_to(System::block_number() + 1); } } From 97b30c4c267055b87faae439e7d99bdef6984695 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sat, 24 Sep 2022 10:37:31 +0200 Subject: [PATCH 09/19] remove useless checks --- .../election-provider-multi-phase/src/lib.rs | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 229e6e5604e31..ce52a0905954d 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -1654,8 +1654,6 @@ mod feasibility_check { MultiPhase::feasibility_check(solution, COMPUTE), FeasibilityError::SnapshotUnavailable ); - - assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } @@ -1671,8 +1669,6 @@ mod feasibility_check { MultiPhase::feasibility_check(solution, COMPUTE), FeasibilityError::InvalidRound ); - - assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } @@ -1690,8 +1686,6 @@ mod feasibility_check { // It should succeed assert_ok!(MultiPhase::feasibility_check(raw, COMPUTE)); - - assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } @@ -1715,8 +1709,6 @@ mod feasibility_check { MultiPhase::feasibility_check(raw, COMPUTE), FeasibilityError::WrongWinnerCount, ); - - assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } @@ -1750,8 +1742,6 @@ mod feasibility_check { MultiPhase::feasibility_check(raw, COMPUTE), FeasibilityError::NposElection(sp_npos_elections::Error::SolutionInvalidIndex) ); - - assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } @@ -1780,8 +1770,6 @@ mod feasibility_check { MultiPhase::feasibility_check(solution, COMPUTE), FeasibilityError::NposElection(sp_npos_elections::Error::SolutionInvalidIndex), ); - - assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } @@ -1811,8 +1799,6 @@ mod feasibility_check { MultiPhase::feasibility_check(solution, COMPUTE), FeasibilityError::InvalidVote, ); - - assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } @@ -1832,8 +1818,6 @@ mod feasibility_check { MultiPhase::feasibility_check(solution, COMPUTE), FeasibilityError::InvalidScore, ); - - assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } } @@ -2405,8 +2389,6 @@ mod tests { MultiPhase::snapshot_metadata().unwrap(), SolutionOrSnapshotSize { voters: 2, targets: 4 } ); - - assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } @@ -2437,8 +2419,6 @@ mod tests { MultiPhase::feasibility_check(solution, ElectionCompute::Signed), FeasibilityError::UntrustedScoreTooLow, ); - - assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); }) } From b2d1859f93f720d95fa8a4b9f7d82b31bea5cf94 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sun, 25 Sep 2022 10:41:55 +0200 Subject: [PATCH 10/19] remove warning --- frame/election-provider-multi-phase/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index ce52a0905954d..8db7e7389584b 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -1633,8 +1633,8 @@ mod feasibility_check { use super::*; use crate::mock::{ - multi_phase_events, raw_solution, roll_to, EpochLength, ExtBuilder, MultiPhase, Runtime, - SignedPhase, TargetIndex, UnsignedPhase, VoterIndex, + raw_solution, roll_to, EpochLength, ExtBuilder, MultiPhase, Runtime, SignedPhase, + TargetIndex, UnsignedPhase, VoterIndex, }; use frame_support::{assert_noop, assert_ok}; From 818c4048ef480a849923ae33ee75db6505cbdea9 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 3 Oct 2022 12:28:51 +0200 Subject: [PATCH 11/19] add checks to signed.rs --- .../src/signed.rs | 88 ++++++++++++++++++- 1 file changed, 85 insertions(+), 3 deletions(-) diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs index 1cf071e6796f1..de575d04f1b86 100644 --- a/frame/election-provider-multi-phase/src/signed.rs +++ b/frame/election-provider-multi-phase/src/signed.rs @@ -528,10 +528,11 @@ mod tests { use super::*; use crate::{ mock::{ - balances, raw_solution, roll_to, Balances, ExtBuilder, MockedWeightInfo, MultiPhase, - Runtime, RuntimeOrigin, SignedMaxRefunds, SignedMaxSubmissions, SignedMaxWeight, + balances, multi_phase_events, raw_solution, roll_to, Balances, ExtBuilder, + MockedWeightInfo, MultiPhase, Runtime, RuntimeOrigin, SignedMaxRefunds, + SignedMaxSubmissions, SignedMaxWeight, }, - Error, Perbill, Phase, + Error, Event, Perbill, Phase, }; use frame_support::{assert_noop, assert_ok, assert_storage_noop}; @@ -565,6 +566,14 @@ mod tests { assert_eq!(balances(&99), (95, 5)); assert_eq!(MultiPhase::signed_submissions().iter().next().unwrap().deposit, 5); + + assert_eq!( + multi_phase_events(), + vec![ + Event::SignedPhaseStarted { round: 1 }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false } + ] + ); }) } @@ -582,6 +591,15 @@ mod tests { assert!(MultiPhase::finalize_signed_phase()); assert_eq!(balances(&99), (100 + 7 + 8, 0)); + + assert_eq!( + multi_phase_events(), + vec![ + Event::SignedPhaseStarted { round: 1 }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::Rewarded { account: 99, value: 7 } + ] + ); }) } @@ -604,6 +622,15 @@ mod tests { assert!(!MultiPhase::finalize_signed_phase()); // and the bond is gone. assert_eq!(balances(&99), (95, 0)); + + assert_eq!( + multi_phase_events(), + vec![ + Event::SignedPhaseStarted { round: 1 }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::Slashed { account: 99, value: 5 } + ] + ); }) } @@ -633,6 +660,15 @@ mod tests { assert_eq!(balances(&99), (100 + 7 + 8, 0)); // 999 gets everything back, including the call fee. assert_eq!(balances(&999), (100 + 8, 0)); + assert_eq!( + multi_phase_events(), + vec![ + Event::SignedPhaseStarted { round: 1 }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::Rewarded { account: 99, value: 7 } + ] + ); }) } @@ -699,6 +735,18 @@ mod tests { assert_eq!(balances(&account), (100, 0)); } } + assert_eq!( + multi_phase_events(), + vec![ + Event::SignedPhaseStarted { round: 1 }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::Rewarded { account: 99, value: 7 } + ] + ); }); } @@ -747,6 +795,20 @@ mod tests { }; assert_ok!(MultiPhase::submit(RuntimeOrigin::signed(99), Box::new(solution))); + assert_eq!( + multi_phase_events(), + vec![ + Event::SignedPhaseStarted { round: 1 }, + Event::SolutionStored { + compute: ElectionCompute::Signed, + prev_ejected: false + }, + Event::SolutionStored { + compute: ElectionCompute::Signed, + prev_ejected: true + } + ] + ); }) } @@ -951,6 +1013,17 @@ mod tests { assert_eq!(balances(&999), (95, 0)); // 9999 gets everything back, including the call fee. assert_eq!(balances(&9999), (100 + 8, 0)); + assert_eq!( + multi_phase_events(), + vec![ + Event::SignedPhaseStarted { round: 1 }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::Slashed { account: 999, value: 5 }, + Event::Rewarded { account: 99, value: 7 } + ] + ); }) } @@ -1070,6 +1143,15 @@ mod tests { // calling it again doesn't change anything assert_storage_noop!(MultiPhase::finalize_signed_phase()); + + assert_eq!( + multi_phase_events(), + vec![ + Event::SignedPhaseStarted { round: 1 }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::Rewarded { account: 99, value: 7 } + ] + ); }) } } From 956de552157060eedff7e0461b5cfe78a05cdb06 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 3 Oct 2022 12:44:51 +0200 Subject: [PATCH 12/19] add some checks to unsigned.rs --- .../src/unsigned.rs | 34 ++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index cf8df237bafb0..0f67da9e7ae5f 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -855,11 +855,11 @@ mod tests { use super::*; use crate::{ mock::{ - roll_to, roll_to_with_ocw, trim_helpers, witness, BlockNumber, ExtBuilder, Extrinsic, - MinerMaxWeight, MultiPhase, Runtime, RuntimeCall as OuterCall, RuntimeOrigin, System, - TestNposSolution, TrimHelpers, UnsignedPhase, + multi_phase_events, roll_to, roll_to_with_ocw, trim_helpers, witness, BlockNumber, + ExtBuilder, Extrinsic, MinerMaxWeight, MultiPhase, Runtime, RuntimeCall as OuterCall, + RuntimeOrigin, System, TestNposSolution, TrimHelpers, UnsignedPhase, }, - CurrentPhase, InvalidTransaction, Phase, QueuedSolution, TransactionSource, + CurrentPhase, Event, InvalidTransaction, Phase, QueuedSolution, TransactionSource, TransactionValidityError, }; use codec::Decode; @@ -1122,6 +1122,17 @@ mod tests { witness )); assert!(MultiPhase::queued_solution().is_some()); + assert_eq!( + multi_phase_events(), + vec![ + Event::SignedPhaseStarted { round: 1 }, + Event::UnsignedPhaseStarted { round: 1 }, + Event::SolutionStored { + compute: ElectionCompute::Unsigned, + prev_ejected: false + } + ] + ); }) } @@ -1463,6 +1474,21 @@ mod tests { // the submitted solution changes because the cache was cleared. assert_eq!(tx_cache_1, tx_cache_3); + assert_eq!( + multi_phase_events(), + vec![ + Event::SignedPhaseStarted { round: 1 }, + Event::UnsignedPhaseStarted { round: 1 }, + Event::ElectionFinalized { + compute: ElectionCompute::Fallback, + score: ElectionScore { + minimal_stake: 0, + sum_stake: 0, + sum_stake_squared: 0 + } + } + ] + ); }) } From b4c162c52aa9823326408a7c50b208f597d0c4ab Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 3 Oct 2022 12:47:27 +0200 Subject: [PATCH 13/19] fmt --- frame/election-provider-multi-phase/src/unsigned.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index 0555fb770b0bb..dc773f107fdf0 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -1050,9 +1050,9 @@ mod tests { use super::*; use crate::{ mock::{ - multi_phase_events, roll_to, roll_to_with_ocw, trim_helpers, witness, BlockNumber, ExtBuilder, Extrinsic, - MinerMaxWeight, MultiPhase, Runtime, RuntimeCall, RuntimeOrigin, System, - TestNposSolution, TrimHelpers, UnsignedPhase, + multi_phase_events, roll_to, roll_to_with_ocw, trim_helpers, witness, BlockNumber, + ExtBuilder, Extrinsic, MinerMaxWeight, MultiPhase, Runtime, RuntimeCall, RuntimeOrigin, + System, TestNposSolution, TrimHelpers, UnsignedPhase, }, CurrentPhase, Event, InvalidTransaction, Phase, QueuedSolution, TransactionSource, TransactionValidityError, From 779e30040c03eddf540843d656995ed48495a0c9 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 3 Oct 2022 12:55:08 +0200 Subject: [PATCH 14/19] use roll_to_signed and roll_to_unsigned --- .../src/signed.rs | 36 +++++++------- .../src/unsigned.rs | 47 ++++++++++--------- 2 files changed, 42 insertions(+), 41 deletions(-) diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs index 2cc251ffd052d..a61ac58fb492f 100644 --- a/frame/election-provider-multi-phase/src/signed.rs +++ b/frame/election-provider-multi-phase/src/signed.rs @@ -528,8 +528,8 @@ mod tests { use super::*; use crate::{ mock::{ - balances, multi_phase_events, raw_solution, roll_to, Balances, ExtBuilder, - MockedWeightInfo, MultiPhase, Runtime, RuntimeOrigin, SignedMaxRefunds, + balances, multi_phase_events, raw_solution, roll_to, roll_to_signed, Balances, + ExtBuilder, MockedWeightInfo, MultiPhase, Runtime, RuntimeOrigin, SignedMaxRefunds, SignedMaxSubmissions, SignedMaxWeight, }, Error, Event, Perbill, Phase, @@ -556,7 +556,7 @@ mod tests { #[test] fn should_pay_deposit() { ExtBuilder::default().build_and_execute(|| { - roll_to(15); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); let solution = raw_solution(); @@ -580,7 +580,7 @@ mod tests { #[test] fn good_solution_is_rewarded() { ExtBuilder::default().build_and_execute(|| { - roll_to(15); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); let solution = raw_solution(); @@ -606,7 +606,7 @@ mod tests { #[test] fn bad_solution_is_slashed() { ExtBuilder::default().build_and_execute(|| { - roll_to(15); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); let mut solution = raw_solution(); @@ -637,7 +637,7 @@ mod tests { #[test] fn suppressed_solution_gets_bond_back() { ExtBuilder::default().build_and_execute(|| { - roll_to(15); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); let mut solution = raw_solution(); @@ -675,7 +675,7 @@ mod tests { #[test] fn cannot_submit_worse_with_full_queue() { ExtBuilder::default().build_and_execute(|| { - roll_to(15); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); for s in 0..SignedMaxSubmissions::get() { @@ -703,7 +703,7 @@ mod tests { #[test] fn call_fee_refund_is_limited_by_signed_max_refunds() { ExtBuilder::default().build_and_execute(|| { - roll_to(15); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); assert_eq!(SignedMaxRefunds::get(), 1); assert!(SignedMaxSubmissions::get() > 2); @@ -756,7 +756,7 @@ mod tests { .signed_max_submission(1) .better_signed_threshold(Perbill::from_percent(20)) .build_and_execute(|| { - roll_to(15); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); let mut solution = RawSolution { @@ -815,7 +815,7 @@ mod tests { #[test] fn weakest_is_removed_if_better_provided() { ExtBuilder::default().build_and_execute(|| { - roll_to(15); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); for s in 0..SignedMaxSubmissions::get() { @@ -862,7 +862,7 @@ mod tests { #[test] fn replace_weakest_works() { ExtBuilder::default().build_and_execute(|| { - roll_to(15); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); for s in 1..SignedMaxSubmissions::get() { @@ -909,7 +909,7 @@ mod tests { #[test] fn early_ejected_solution_gets_bond_back() { ExtBuilder::default().signed_deposit(2, 0, 0).build_and_execute(|| { - roll_to(15); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); for s in 0..SignedMaxSubmissions::get() { @@ -940,7 +940,7 @@ mod tests { #[test] fn equally_good_solution_is_not_accepted() { ExtBuilder::default().signed_max_submission(3).build_and_execute(|| { - roll_to(15); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); for i in 0..SignedMaxSubmissions::get() { @@ -977,7 +977,7 @@ mod tests { // - bad_solution_is_slashed // - suppressed_solution_gets_bond_back ExtBuilder::default().build_and_execute(|| { - roll_to(15); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); assert_eq!(balances(&99), (100, 0)); @@ -1033,7 +1033,7 @@ mod tests { .signed_weight(Weight::from_ref_time(40).set_proof_size(u64::MAX)) .mock_weight_info(MockedWeightInfo::Basic) .build_and_execute(|| { - roll_to(15); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); let (raw, witness) = MultiPhase::mine_solution().unwrap(); @@ -1067,7 +1067,7 @@ mod tests { #[test] fn insufficient_deposit_does_not_store_submission() { ExtBuilder::default().build_and_execute(|| { - roll_to(15); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); let solution = raw_solution(); @@ -1087,7 +1087,7 @@ mod tests { #[test] fn insufficient_deposit_with_full_queue_works_properly() { ExtBuilder::default().build_and_execute(|| { - roll_to(15); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); for s in 0..SignedMaxSubmissions::get() { @@ -1133,7 +1133,7 @@ mod tests { #[test] fn finalize_signed_phase_is_idempotent_given_submissions() { ExtBuilder::default().build_and_execute(|| { - roll_to(15); + roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); let solution = raw_solution(); diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index dc773f107fdf0..7340605dfe621 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -1050,9 +1050,10 @@ mod tests { use super::*; use crate::{ mock::{ - multi_phase_events, roll_to, roll_to_with_ocw, trim_helpers, witness, BlockNumber, - ExtBuilder, Extrinsic, MinerMaxWeight, MultiPhase, Runtime, RuntimeCall, RuntimeOrigin, - System, TestNposSolution, TrimHelpers, UnsignedPhase, + multi_phase_events, roll_to, roll_to_signed, roll_to_unsigned, roll_to_with_ocw, + trim_helpers, witness, BlockNumber, ExtBuilder, Extrinsic, MinerMaxWeight, MultiPhase, + Runtime, RuntimeCall, RuntimeOrigin, System, TestNposSolution, TrimHelpers, + UnsignedPhase, }, CurrentPhase, Event, InvalidTransaction, Phase, QueuedSolution, TransactionSource, TransactionValidityError, @@ -1100,7 +1101,7 @@ mod tests { )); // signed - roll_to(15); + roll_to_signed(); assert_eq!(MultiPhase::current_phase(), Phase::Signed); assert!(matches!( ::validate_unsigned( @@ -1116,7 +1117,7 @@ mod tests { )); // unsigned - roll_to(25); + roll_to_unsigned(); assert!(MultiPhase::current_phase().is_unsigned()); assert!(::validate_unsigned( @@ -1147,7 +1148,7 @@ mod tests { #[test] fn validate_unsigned_retracts_low_score() { ExtBuilder::default().desired_targets(0).build_and_execute(|| { - roll_to(25); + roll_to_unsigned(); assert!(MultiPhase::current_phase().is_unsigned()); let solution = RawSolution:: { @@ -1193,7 +1194,7 @@ mod tests { #[test] fn validate_unsigned_retracts_incorrect_winner_count() { ExtBuilder::default().desired_targets(1).build_and_execute(|| { - roll_to(25); + roll_to_unsigned(); assert!(MultiPhase::current_phase().is_unsigned()); let raw = RawSolution:: { @@ -1222,7 +1223,7 @@ mod tests { .miner_tx_priority(20) .desired_targets(0) .build_and_execute(|| { - roll_to(25); + roll_to_unsigned(); assert!(MultiPhase::current_phase().is_unsigned()); let solution = RawSolution:: { @@ -1253,7 +1254,7 @@ mod tests { Some(\"PreDispatchWrongWinnerCount\") })")] fn unfeasible_solution_panics() { ExtBuilder::default().build_and_execute(|| { - roll_to(25); + roll_to_unsigned(); assert!(MultiPhase::current_phase().is_unsigned()); // This is in itself an invalid BS solution. @@ -1275,7 +1276,7 @@ mod tests { deprive validator from their authoring reward.")] fn wrong_witness_panics() { ExtBuilder::default().build_and_execute(|| { - roll_to(25); + roll_to_unsigned(); assert!(MultiPhase::current_phase().is_unsigned()); // This solution is unfeasible as well, but we won't even get there. @@ -1299,7 +1300,7 @@ mod tests { #[test] fn miner_works() { ExtBuilder::default().build_and_execute(|| { - roll_to(25); + roll_to_unsigned(); assert!(MultiPhase::current_phase().is_unsigned()); // ensure we have snapshots in place. @@ -1337,7 +1338,7 @@ mod tests { .miner_weight(Weight::from_ref_time(100).set_proof_size(u64::MAX)) .mock_weight_info(crate::mock::MockedWeightInfo::Basic) .build_and_execute(|| { - roll_to(25); + roll_to_unsigned(); assert!(MultiPhase::current_phase().is_unsigned()); let (raw, witness) = MultiPhase::mine_solution().unwrap(); @@ -1371,7 +1372,7 @@ mod tests { fn miner_will_not_submit_if_not_enough_winners() { let (mut ext, _) = ExtBuilder::default().desired_targets(8).build_offchainify(0); ext.execute_with(|| { - roll_to(25); + roll_to_unsigned(); assert!(MultiPhase::current_phase().is_unsigned()); // Force the number of winners to be bigger to fail @@ -1397,7 +1398,7 @@ mod tests { .add_voter(8, 5, bounded_vec![10]) .better_unsigned_threshold(Perbill::from_percent(50)) .build_and_execute(|| { - roll_to(25); + roll_to_unsigned(); assert!(MultiPhase::current_phase().is_unsigned()); assert_eq!(MultiPhase::desired_targets().unwrap(), 1); @@ -1499,7 +1500,7 @@ mod tests { ext.execute_with(|| { let offchain_repeat = ::OffchainRepeat::get(); - roll_to(25); + roll_to_unsigned(); assert!(MultiPhase::current_phase().is_unsigned()); // first execution -- okay. @@ -1540,7 +1541,7 @@ mod tests { let guard = StorageValueRef::persistent(&OFFCHAIN_LOCK); let last_block = StorageValueRef::persistent(OFFCHAIN_LAST_BLOCK); - roll_to(25); + roll_to_unsigned(); assert!(MultiPhase::current_phase().is_unsigned()); // initially, the lock is not set. @@ -1561,7 +1562,7 @@ mod tests { // ensure that if the guard is in hold, a new execution is not allowed. let (mut ext, pool) = ExtBuilder::default().build_offchainify(0); ext.execute_with(|| { - roll_to(25); + roll_to_unsigned(); assert!(MultiPhase::current_phase().is_unsigned()); // artificially set the value, as if another thread is mid-way. @@ -1589,7 +1590,7 @@ mod tests { fn ocw_only_runs_when_unsigned_open_now() { let (mut ext, pool) = ExtBuilder::default().build_offchainify(0); ext.execute_with(|| { - roll_to(25); + roll_to_unsigned(); assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); // we must clear the offchain storage to ensure the offchain execution check doesn't get @@ -1823,7 +1824,7 @@ mod tests { #[test] fn trim_assignments_length_does_not_modify_when_short_enough() { ExtBuilder::default().build_and_execute(|| { - roll_to(25); + roll_to_unsigned(); // given let TrimHelpers { mut assignments, encoded_size_of, .. } = trim_helpers(); @@ -1848,7 +1849,7 @@ mod tests { #[test] fn trim_assignments_length_modifies_when_too_long() { ExtBuilder::default().build().execute_with(|| { - roll_to(25); + roll_to_unsigned(); // given let TrimHelpers { mut assignments, encoded_size_of, .. } = trim_helpers(); @@ -1874,7 +1875,7 @@ mod tests { #[test] fn trim_assignments_length_trims_lowest_stake() { ExtBuilder::default().build().execute_with(|| { - roll_to(25); + roll_to_unsigned(); // given let TrimHelpers { voters, mut assignments, encoded_size_of, voter_index } = @@ -1937,7 +1938,7 @@ mod tests { // or when we trim it to zero. ExtBuilder::default().build_and_execute(|| { // we need snapshot for `trim_helpers` to work. - roll_to(25); + roll_to_unsigned(); let TrimHelpers { mut assignments, encoded_size_of, .. } = trim_helpers(); assert!(assignments.len() > 0); @@ -1959,7 +1960,7 @@ mod tests { #[test] fn mine_solution_solutions_always_within_acceptable_length() { ExtBuilder::default().build_and_execute(|| { - roll_to(25); + roll_to_unsigned(); // how long would the default solution be? let solution = MultiPhase::mine_solution().unwrap(); From b9a008d91232d32abde116b34176c90488aa9636 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Tue, 4 Oct 2022 09:59:43 +0200 Subject: [PATCH 15/19] remove nonsense --- frame/election-provider-multi-phase/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 9e58f1579712e..70d2b352eea21 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -1894,7 +1894,7 @@ mod tests { assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); assert!(MultiPhase::snapshot().is_some()); - roll_to(15); + roll_to(30); assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); assert!(MultiPhase::snapshot().is_some()); From ac843245493784bcb539316789024fe6f7332a9f Mon Sep 17 00:00:00 2001 From: Szegoo Date: Tue, 4 Oct 2022 10:02:55 +0200 Subject: [PATCH 16/19] remove even more nonsense --- frame/election-provider-multi-phase/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 70d2b352eea21..3744d2ac774a4 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -1951,7 +1951,7 @@ mod tests { assert!(MultiPhase::current_phase().is_unsigned_open_at(20)); assert!(MultiPhase::snapshot().is_some()); - roll_to(15); + roll_to(30); assert!(MultiPhase::current_phase().is_unsigned_open_at(20)); assert_ok!(MultiPhase::elect()); @@ -1989,7 +1989,7 @@ mod tests { assert!(MultiPhase::current_phase().is_signed()); assert!(MultiPhase::snapshot().is_some()); - roll_to(15); + roll_to(30); assert!(MultiPhase::current_phase().is_signed()); assert_ok!(MultiPhase::elect()); @@ -2026,7 +2026,7 @@ mod tests { roll_to(20); assert!(MultiPhase::current_phase().is_off()); - roll_to(15); + roll_to(30); assert!(MultiPhase::current_phase().is_off()); // This module is now only capable of doing on-chain backup. @@ -2161,7 +2161,7 @@ mod tests { Box::new(solution) )); - roll_to(15); + roll_to(30); assert_ok!(MultiPhase::elect()); assert_eq!( @@ -2205,7 +2205,7 @@ mod tests { )); assert!(MultiPhase::queued_solution().is_some()); - roll_to(15); + roll_to(30); assert_ok!(MultiPhase::elect()); assert_eq!( From bd128f3ffe2886b64b57968c505349ec35f7fb67 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Tue, 4 Oct 2022 10:12:31 +0200 Subject: [PATCH 17/19] fix --- frame/election-provider-multi-phase/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 3744d2ac774a4..d340b0778097c 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -2170,6 +2170,7 @@ mod tests { Event::SignedPhaseStarted { round: 1 }, Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, Event::Rewarded { account: 99, value: 7 }, + Event::UnsignedPhaseStarted { round: 1 }, Event::ElectionFinalized { compute: ElectionCompute::Signed, score: ElectionScore { @@ -2177,7 +2178,7 @@ mod tests { sum_stake: 100, sum_stake_squared: 5200 } - }, + } ], ); }) From 93cc43f2a51744cbd740d13072148dd23a3eb11a Mon Sep 17 00:00:00 2001 From: Szegoo Date: Tue, 4 Oct 2022 19:48:25 +0200 Subject: [PATCH 18/19] fix --- .../src/signed.rs | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs index a61ac58fb492f..f245cb0a3fc99 100644 --- a/frame/election-provider-multi-phase/src/signed.rs +++ b/frame/election-provider-multi-phase/src/signed.rs @@ -719,7 +719,7 @@ mod tests { assert_eq!(balances(&account), (95, 5)); } - assert!(MultiPhase::finalize_signed_phase()); + assert_ok!(MultiPhase::do_elect()); for s in 0..SignedMaxSubmissions::get() { let account = 99 + s as u64; @@ -739,12 +739,20 @@ mod tests { multi_phase_events(), vec![ Event::SignedPhaseStarted { round: 1 }, - Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, - Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, - Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, - Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, - Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, - Event::Rewarded { account: 99, value: 7 } + Event::SolutionStored { compute: Signed, prev_ejected: false }, + Event::SolutionStored { compute: Signed, prev_ejected: false }, + Event::SolutionStored { compute: Signed, prev_ejected: false }, + Event::SolutionStored { compute: Signed, prev_ejected: false }, + Event::SolutionStored { compute: Signed, prev_ejected: false }, + Event::Rewarded { account: 99, value: 7 }, + Event::ElectionFinalized { + compute: Signed, + score: ElectionScore { + minimal_stake: 40, + sum_stake: 100, + sum_stake_squared: 5200 + } + } ] ); }); From a4035042acb940216dd2010280403462e494b48d Mon Sep 17 00:00:00 2001 From: Szegoo Date: Tue, 4 Oct 2022 19:54:00 +0200 Subject: [PATCH 19/19] remove useless checks --- frame/election-provider-multi-phase/src/lib.rs | 11 ----------- frame/election-provider-multi-phase/src/signed.rs | 12 ++++++------ 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index d340b0778097c..3dc6161bb202a 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -2049,8 +2049,6 @@ mod tests { // An early termination in the signed phase, with no queued solution. ExtBuilder::default().build_and_execute(|| { // Signed phase started at block 15 and will end at 25. - roll_to(14); - assert_eq!(MultiPhase::current_phase(), Phase::Off); roll_to_signed(); assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); @@ -2058,7 +2056,6 @@ mod tests { assert_eq!(MultiPhase::round(), 1); // An unexpected call to elect. - roll_to(20); assert_ok!(MultiPhase::elect()); // We surely can't have any feasible solutions. This will cause an on-chain election. @@ -2087,8 +2084,6 @@ mod tests { // an early termination in the signed phase, with no queued solution. ExtBuilder::default().build_and_execute(|| { // signed phase started at block 15 and will end at 25. - roll_to(14); - assert_eq!(MultiPhase::current_phase(), Phase::Off); roll_to_signed(); assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted { round: 1 }]); @@ -2108,7 +2103,6 @@ mod tests { } // an unexpected call to elect. - roll_to(20); assert_ok!(MultiPhase::elect()); // all storage items must be cleared. @@ -2149,9 +2143,6 @@ mod tests { #[test] fn check_events_with_compute_signed() { ExtBuilder::default().build_and_execute(|| { - roll_to(14); - assert_eq!(MultiPhase::current_phase(), Phase::Off); - roll_to_signed(); assert!(MultiPhase::current_phase().is_signed()); @@ -2206,7 +2197,6 @@ mod tests { )); assert!(MultiPhase::queued_solution().is_some()); - roll_to(30); assert_ok!(MultiPhase::elect()); assert_eq!( @@ -2349,7 +2339,6 @@ mod tests { assert_eq!(MultiPhase::current_phase(), Phase::Off); // On-chain backup works though. - roll_to(29); let supports = MultiPhase::elect().unwrap(); assert!(supports.len() > 0); diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs index f245cb0a3fc99..175c92757f35e 100644 --- a/frame/election-provider-multi-phase/src/signed.rs +++ b/frame/election-provider-multi-phase/src/signed.rs @@ -739,14 +739,14 @@ mod tests { multi_phase_events(), vec![ Event::SignedPhaseStarted { round: 1 }, - Event::SolutionStored { compute: Signed, prev_ejected: false }, - Event::SolutionStored { compute: Signed, prev_ejected: false }, - Event::SolutionStored { compute: Signed, prev_ejected: false }, - Event::SolutionStored { compute: Signed, prev_ejected: false }, - Event::SolutionStored { compute: Signed, prev_ejected: false }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, + Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false }, Event::Rewarded { account: 99, value: 7 }, Event::ElectionFinalized { - compute: Signed, + compute: ElectionCompute::Signed, score: ElectionScore { minimal_stake: 40, sum_stake: 100,