Merge pull request #1433 from galacticcouncil/fix/remove-process-votes-from-on-before-vote

fix(staking): remove O(N) vote processing from vote extrinsic

Author: enthusiastmartin

Cargo.lock

@@ -2508,7 +2508,7 @@ fn increase_stake_should_work_when_referendum_ongoing_and_votes_processed() {
 }
 
 #[test]
-fn voting_on_next_referenda_should_process_votes() {
+fn removing_vote_should_process_votes() {
 	TestNet::reset();
 	Hydra::execute_with(|| {
 		init_omnipool();
@@ -2569,27 +2569,41 @@ fn voting_on_next_referenda_should_process_votes() {
 
 		end_referendum();
 
+		let alice_position_id = pallet_staking::Pallet::<hydradx_runtime::Runtime>::get_user_position_id(&ALICE.into())
+			.unwrap()
+			.unwrap();
+
+		// Before remove_vote, the finished vote has not been settled: still recorded in Votes,
+		// no points awarded, and not present in VotesRewarded.
+		let position_before =
+			pallet_staking::Pallet::<hydradx_runtime::Runtime>::get_position(alice_position_id).unwrap();
+		assert!(
+			pallet_staking::Pallet::<hydradx_runtime::Runtime>::get_position_votes(alice_position_id)
+				.votes
+				.iter()
+				.any(|(idx, _)| *idx == r)
+		);
 		assert!(
 			pallet_staking::Pallet::<hydradx_runtime::Runtime>::processed_votes::<AccountId, u32>(ALICE.into(), r)
 				.is_none()
 		);
 
-		let r = begin_referendum();
-		assert_ok!(ConvictionVoting::vote(
+		assert_ok!(ConvictionVoting::remove_vote(
 			hydradx_runtime::RuntimeOrigin::signed(ALICE.into()),
-			r,
-			AccountVote::Standard {
-				vote: Vote {
-					aye: true,
-					conviction: Conviction::Locked6x,
-				},
-				balance: 1_000_000 * UNITS,
-			}
+			Some(ROOT_TRACK),
+			r
 		));
+
+		// After remove_vote, settlement has happened: vote removed from Votes and points awarded.
 		assert!(
-			pallet_staking::Pallet::<hydradx_runtime::Runtime>::processed_votes::<AccountId, u32>(ALICE.into(), 0)
-				.is_some()
+			!pallet_staking::Pallet::<hydradx_runtime::Runtime>::get_position_votes(alice_position_id)
+				.votes
+				.iter()
+				.any(|(idx, _)| *idx == r)
 		);
+		let position_after =
+			pallet_staking::Pallet::<hydradx_runtime::Runtime>::get_position(alice_position_id).unwrap();
+		assert!(position_after.get_action_points() > position_before.get_action_points());
 	});
 }
 

integration-tests/src/staking.rs

@@ -35,8 +35,6 @@ where
 				}
 			};
 
-			Pallet::<T>::process_votes(who, position_id, position)?;
-
 			let amount = vote.balance();
 			let conviction = if let AccountVote::Standard { vote, .. } = vote {
 				match vote.conviction {

pallets/staking/src/integrations/conviction_voting.rs

@@ -269,7 +269,7 @@ fn process_votes_should_do_nothing_when_referendum_doesnt_exists() {
 }
 
 #[test]
-fn process_votes_should_work_when_on_vote_is_called() {
+fn on_before_vote_should_not_process_finished_votes() {
 	ExtBuilder::default()
 		.with_endowed_accounts(vec![
 			(ALICE, HDX, 150_000 * ONE),
@@ -351,14 +351,9 @@ fn process_votes_should_work_when_on_vote_is_called() {
 			));
 
 			//Assert
-			assert_eq!(
-				Position {
-					action_points: 64_u128,
-					..position_before
-				},
-				Staking::positions(position_id).unwrap()
-			);
-			assert_eq!(Votes::<Test>::get(position_id).votes.len(), 3);
+			// on_before_vote no longer processes finished votes - settlement happens at remove_vote time.
+			assert_eq!(position_before, Staking::positions(position_id).unwrap());
+			assert_eq!(Votes::<Test>::get(position_id).votes.len(), 7);
 		});
 }
 

pallets/staking/src/tests/tests.rs

@@ -162,6 +162,7 @@ sp-version = { workspace = true }
 sp-trie = { workspace = true }
 sp-io = { workspace = true }
 primitive-types = { workspace = true }
+static_assertions = { workspace = true }
 
 # Frontier
 fp-rpc = { workspace = true }

runtime/hydradx/Cargo.toml

@@ -1629,10 +1629,6 @@ impl GetByKey<FixedU128, Point> for StakingMinSlash {
 	}
 }
 
-parameter_types! {
-	pub const MaxVotes: u32 = 25;
-}
-
 impl pallet_staking::Config for Runtime {
 	type AuthorityOrigin = EitherOf<EnsureRoot<Self::AccountId>, GeneralAdmin>;
 	type AssetId = AssetId;
@@ -1653,7 +1649,7 @@ impl pallet_staking::Config for Runtime {
 	type NFTCollectionId = ConstU128<2222>;
 	type Collections = FreezableNFT<Runtime, Self::RuntimeOrigin>;
 	type NFTHandler = Uniques;
-	type MaxVotes = MaxVotes;
+	type MaxVotes = governance::MaxVotes;
 	type ReferendumInfo = pallet_staking::integrations::conviction_voting::DirectReferendumStatus<Runtime>;
 	type MaxPointsPerAction = PointsPerAction;
 	type Vesting = VestingInfo<Runtime>;
@@ -1664,6 +1660,12 @@ impl pallet_staking::Config for Runtime {
 	type MaxLocks = MaxLocks;
 }
 
+//Make sure staking and conviction voting are using same `MaxVotes` value
+static_assertions::const_assert_eq!(
+	<Runtime as pallet_staking::Config>::MaxVotes::get(),
+	<Runtime as pallet_conviction_voting::Config>::MaxVotes::get()
+);
+
 // LBP
 pub struct AssetPairAccountId<T: frame_system::Config>(PhantomData<T>);
 impl<T: frame_system::Config> AssetPairAccountIdFor<AssetId, T::AccountId> for AssetPairAccountId<T>

runtime/hydradx/src/assets.rs

@@ -180,14 +180,16 @@ impl pallet_treasury::Config for Runtime {
 
 parameter_types! {
 	pub const VoteLockingPeriod: BlockNumber = 7 * DAYS;
+	#[derive(Debug)]
+	pub const MaxVotes: u32 = 25;
 }
 
 impl pallet_conviction_voting::Config for Runtime {
 	type WeightInfo = weights::pallet_conviction_voting::HydraWeight<Runtime>;
 	type RuntimeEvent = RuntimeEvent;
 	type Currency = Balances;
 	type VoteLockingPeriod = VoteLockingPeriod;
-	type MaxVotes = ConstU32<25>;
+	type MaxVotes = MaxVotes;
 	type MaxTurnout = frame_support::traits::tokens::currency::ActiveIssuanceOf<Balances, Self::AccountId>;
 	type Polls = Referenda;
 	type VotingHooks = pallet_staking::integrations::conviction_voting::StakingConvictionVoting<Runtime>;

runtime/hydradx/src/governance/mod.rs

@@ -19,7 +19,7 @@
 //! Autogenerated weights for `pallet_conviction_voting`
 //!
 //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 49.1.0
-//! DATE: 2026-04-16, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
+//! DATE: 2026-05-08, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
 //! WORST CASE MAP SIZE: `1000000`
 //! HOSTNAME: `bench-bot`, CPU: `Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz`
 //! WASM-EXECUTION: `Compiled`, CHAIN: `None`, DB CACHE: `1024`
@@ -64,12 +64,12 @@ impl<T: frame_system::Config> pallet_conviction_voting::WeightInfo for HydraWeig
 	/// Proof: `Uniques::Account` (`max_values`: None, `max_size`: Some(112), added: 2587, mode: `MaxEncodedLen`)
 	/// Storage: `Staking::Positions` (r:1 w:1)
 	/// Proof: `Staking::Positions` (`max_values`: None, `max_size`: Some(132), added: 2607, mode: `MaxEncodedLen`)
+	/// Storage: `Balances::Locks` (r:1 w:1)
+	/// Proof: `Balances::Locks` (`max_values`: None, `max_size`: Some(1299), added: 3774, mode: `MaxEncodedLen`)
 	/// Storage: `Staking::Votes` (r:1 w:1)
 	/// Proof: `Staking::Votes` (`max_values`: None, `max_size`: Some(558), added: 3033, mode: `MaxEncodedLen`)
-	/// Storage: `Referenda::ReferendumInfoFor` (r:25 w:1)
+	/// Storage: `Referenda::ReferendumInfoFor` (r:1 w:1)
 	/// Proof: `Referenda::ReferendumInfoFor` (`max_values`: None, `max_size`: Some(936), added: 3411, mode: `MaxEncodedLen`)
-	/// Storage: `Balances::Locks` (r:1 w:1)
-	/// Proof: `Balances::Locks` (`max_values`: None, `max_size`: Some(1299), added: 3774, mode: `MaxEncodedLen`)
 	/// Storage: `ConvictionVoting::VotingFor` (r:1 w:1)
 	/// Proof: `ConvictionVoting::VotingFor` (`max_values`: None, `max_size`: Some(1429), added: 3904, mode: `MaxEncodedLen`)
 	/// Storage: `ConvictionVoting::ClassLocksFor` (r:1 w:1)
@@ -82,23 +82,23 @@ impl<T: frame_system::Config> pallet_conviction_voting::WeightInfo for HydraWeig
 	/// Proof: `Scheduler::Agenda` (`max_values`: None, `max_size`: Some(38963), added: 41438, mode: `MaxEncodedLen`)
 	fn vote_new() -> Weight {
 		// Proof Size summary in bytes:
-		//  Measured:  `12032`
-		//  Estimated: `86265`
-		// Minimum execution time: 256_121_000 picoseconds.
-		Weight::from_parts(258_036_000, 86265)
-			.saturating_add(T::DbWeight::get().reads(35_u64))
+		//  Measured:  `4692`
+		//  Estimated: `42428`
+		// Minimum execution time: 124_368_000 picoseconds.
+		Weight::from_parts(125_529_000, 42428)
+			.saturating_add(T::DbWeight::get().reads(11_u64))
 			.saturating_add(T::DbWeight::get().writes(7_u64))
 	}
 	/// Storage: `Uniques::Account` (r:2 w:0)
 	/// Proof: `Uniques::Account` (`max_values`: None, `max_size`: Some(112), added: 2587, mode: `MaxEncodedLen`)
 	/// Storage: `Staking::Positions` (r:1 w:1)
 	/// Proof: `Staking::Positions` (`max_values`: None, `max_size`: Some(132), added: 2607, mode: `MaxEncodedLen`)
+	/// Storage: `Balances::Locks` (r:1 w:1)
+	/// Proof: `Balances::Locks` (`max_values`: None, `max_size`: Some(1299), added: 3774, mode: `MaxEncodedLen`)
 	/// Storage: `Staking::Votes` (r:1 w:1)
 	/// Proof: `Staking::Votes` (`max_values`: None, `max_size`: Some(558), added: 3033, mode: `MaxEncodedLen`)
-	/// Storage: `Referenda::ReferendumInfoFor` (r:25 w:1)
+	/// Storage: `Referenda::ReferendumInfoFor` (r:1 w:1)
 	/// Proof: `Referenda::ReferendumInfoFor` (`max_values`: None, `max_size`: Some(936), added: 3411, mode: `MaxEncodedLen`)
-	/// Storage: `Balances::Locks` (r:1 w:1)
-	/// Proof: `Balances::Locks` (`max_values`: None, `max_size`: Some(1299), added: 3774, mode: `MaxEncodedLen`)
 	/// Storage: `ConvictionVoting::VotingFor` (r:1 w:1)
 	/// Proof: `ConvictionVoting::VotingFor` (`max_values`: None, `max_size`: Some(1429), added: 3904, mode: `MaxEncodedLen`)
 	/// Storage: `ConvictionVoting::ClassLocksFor` (r:1 w:1)
@@ -113,11 +113,11 @@ impl<T: frame_system::Config> pallet_conviction_voting::WeightInfo for HydraWeig
 	/// Proof: `Scheduler::Retries` (`max_values`: None, `max_size`: Some(30), added: 2505, mode: `MaxEncodedLen`)
 	fn vote_existing() -> Weight {
 		// Proof Size summary in bytes:
-		//  Measured:  `12424`
-		//  Estimated: `86265`
-		// Minimum execution time: 282_759_000 picoseconds.
-		Weight::from_parts(290_746_000, 86265)
-			.saturating_add(T::DbWeight::get().reads(36_u64))
+		//  Measured:  `5084`
+		//  Estimated: `83866`
+		// Minimum execution time: 149_529_000 picoseconds.
+		Weight::from_parts(152_350_000, 83866)
+			.saturating_add(T::DbWeight::get().reads(12_u64))
 			.saturating_add(T::DbWeight::get().writes(9_u64))
 	}
 	/// Storage: `ConvictionVoting::VotingFor` (r:1 w:1)
@@ -140,8 +140,8 @@ impl<T: frame_system::Config> pallet_conviction_voting::WeightInfo for HydraWeig
 		// Proof Size summary in bytes:
 		//  Measured:  `3672`
 		//  Estimated: `83866`
-		// Minimum execution time: 114_162_000 picoseconds.
-		Weight::from_parts(115_861_000, 83866)
+		// Minimum execution time: 115_365_000 picoseconds.
+		Weight::from_parts(117_060_000, 83866)
 			.saturating_add(T::DbWeight::get().reads(9_u64))
 			.saturating_add(T::DbWeight::get().writes(7_u64))
 	}
@@ -161,8 +161,8 @@ impl<T: frame_system::Config> pallet_conviction_voting::WeightInfo for HydraWeig
 		// Proof Size summary in bytes:
 		//  Measured:  `3147`
 		//  Estimated: `6164`
-		// Minimum execution time: 83_726_000 picoseconds.
-		Weight::from_parts(84_478_000, 6164)
+		// Minimum execution time: 84_125_000 picoseconds.
+		Weight::from_parts(84_753_000, 6164)
 			.saturating_add(T::DbWeight::get().reads(7_u64))
 			.saturating_add(T::DbWeight::get().writes(3_u64))
 	}
@@ -185,12 +185,12 @@ impl<T: frame_system::Config> pallet_conviction_voting::WeightInfo for HydraWeig
 	/// The range of component `r` is `[0, 3]`.
 	fn delegate(r: u32, ) -> Weight {
 		// Proof Size summary in bytes:
-		//  Measured:  `253 + r * (1028 ±0)`
+		//  Measured:  `298 + r * (1028 ±0)`
 		//  Estimated: `83866 + r * (3411 ±0)`
-		// Minimum execution time: 59_745_000 picoseconds.
-		Weight::from_parts(64_994_711, 83866)
-			// Standard Error: 293_828
-			.saturating_add(Weight::from_parts(36_220_681, 0).saturating_mul(r.into()))
+		// Minimum execution time: 61_872_000 picoseconds.
+		Weight::from_parts(67_798_487, 83866)
+			// Standard Error: 288_095
+			.saturating_add(Weight::from_parts(36_733_587, 0).saturating_mul(r.into()))
 			.saturating_add(T::DbWeight::get().reads(6_u64))
 			.saturating_add(T::DbWeight::get().reads((2_u64).saturating_mul(r.into())))
 			.saturating_add(T::DbWeight::get().writes(4_u64))
@@ -210,10 +210,10 @@ impl<T: frame_system::Config> pallet_conviction_voting::WeightInfo for HydraWeig
 		// Proof Size summary in bytes:
 		//  Measured:  `477 + r * (890 ±0)`
 		//  Estimated: `83866 + r * (3411 ±0)`
-		// Minimum execution time: 30_226_000 picoseconds.
-		Weight::from_parts(34_231_950, 83866)
-			// Standard Error: 217_216
-			.saturating_add(Weight::from_parts(33_445_261, 0).saturating_mul(r.into()))
+		// Minimum execution time: 31_153_000 picoseconds.
+		Weight::from_parts(34_865_705, 83866)
+			// Standard Error: 215_623
+			.saturating_add(Weight::from_parts(33_825_763, 0).saturating_mul(r.into()))
 			.saturating_add(T::DbWeight::get().reads(2_u64))
 			.saturating_add(T::DbWeight::get().reads((2_u64).saturating_mul(r.into())))
 			.saturating_add(T::DbWeight::get().writes(2_u64))
@@ -232,10 +232,10 @@ impl<T: frame_system::Config> pallet_conviction_voting::WeightInfo for HydraWeig
 	/// Proof: `Balances::Freezes` (`max_values`: None, `max_size`: Some(49), added: 2524, mode: `MaxEncodedLen`)
 	fn unlock() -> Weight {
 		// Proof Size summary in bytes:
-		//  Measured:  `1456`
+		//  Measured:  `1501`
 		//  Estimated: `4894`
-		// Minimum execution time: 63_264_000 picoseconds.
-		Weight::from_parts(64_155_000, 4894)
+		// Minimum execution time: 66_518_000 picoseconds.
+		Weight::from_parts(66_986_000, 4894)
 			.saturating_add(T::DbWeight::get().reads(5_u64))
 			.saturating_add(T::DbWeight::get().writes(3_u64))
 	}