review comments

Author: enthusiastmartin

pallets/gigahdx-rewards/src/lib.rs

@@ -28,9 +28,9 @@
 //! - First `on_remove_vote` for a completed referendum lazily transfers
 //!   `track_pct × accumulator_balance` into the allocated pot and snapshots
 //!   the frozen denominator.
-//! - Per-user shares are pro-rata against that frozen denominator. The last
-//!   claimant scoops any remaining dust, draining the pool to exactly zero
-//!   and triggering storage cleanup.
+//! - Per-user shares are pro-rata against that frozen denominator. Once the
+//!   last counted voter is paid, any rounding dust left in the pool is
+//!   recycled to the accumulator pot and the pool entry is cleaned up.
 //! - `claim_rewards` atomically compounds the user's accumulated HDX back
 //!   into their gigahdx position.
 

pallets/gigahdx-rewards/src/types.rs

@@ -64,11 +64,13 @@ pub struct ReferendaReward<TrackId> {
 	/// Frozen denominator for pro-rata math.
 	pub total_weighted_votes: u128,
 	/// Countdown of voters who still hold a `UserVoteRecord` for this
-	/// referendum. When this reaches zero on a per-user payout, the last
-	/// claimant scoops `remaining_reward` and the pool entry is deleted.
+	/// referendum. When this reaches zero on a per-user payout, any
+	/// `remaining_reward` dust is recycled to the accumulator pot and the
+	/// pool entry is deleted.
 	pub voters_remaining: u32,
-	/// Decremented on each per-user payout. Equals `total_reward` at
-	/// allocation; drained to exactly zero by the final claimant.
+	/// Decremented on each per-user payout by the capped share. Equals
+	/// `total_reward` at allocation; any residual after the final payout is
+	/// recycled to the accumulator pot.
 	pub remaining_reward: Balance,
 }
 

pallets/gigahdx-rewards/src/voting_hooks.rs

@@ -156,11 +156,38 @@ impl<T: Config> VotingHooks<T::AccountId, ReferendumIndex, Balance> for VotingHo
 		None
 	}
 
+	// `on_before_vote` / `on_remove_vote` short-circuit at `Stakes[who].hdx == 0`,
+	// so the conviction-voting `vote` / `remove_vote` benchmarks must make `who`
+	// a gigahdx staker for their weight to cover this hook's per-vote storage
+	// writes (tally + `UserVoteRecords` + `freeze`/`unfreeze`). Seed the record
+	// directly — the hook only reads `.hdx` and mutates `.frozen`, so no
+	// money-market / lock setup is needed.
+	//
+	// The one-time-per-referendum allocation path (`Status::Completed` → pot
+	// transfer + `record_user_reward`) is not reachable here: the benchmark's
+	// poll stays Ongoing, so that bounded cost is paid by — and documented on —
+	// the first post-completion remover rather than charged per vote.
 	#[cfg(feature = "runtime-benchmarks")]
-	fn on_vote_worst_case(_who: &T::AccountId) {}
+	fn on_vote_worst_case(who: &T::AccountId) {
+		seed_staker_worst_case::<T>(who);
+	}
 
 	#[cfg(feature = "runtime-benchmarks")]
-	fn on_remove_vote_worst_case(_who: &T::AccountId) {}
+	fn on_remove_vote_worst_case(who: &T::AccountId) {
+		seed_staker_worst_case::<T>(who);
+	}
+}
+
+#[cfg(feature = "runtime-benchmarks")]
+fn seed_staker_worst_case<T: Config>(who: &T::AccountId) {
+	pallet_gigahdx::Stakes::<T>::insert(
+		who,
+		pallet_gigahdx::StakeRecord {
+			hdx: 1_000_000_000_000_000,
+			gigahdx: 1_000_000_000_000_000,
+			..Default::default()
+		},
+	);
 }
 
 /// Allocate the pool on first call for a completed referendum, then credit

pallets/gigahdx/src/lib.rs

@@ -119,6 +119,16 @@ pub mod pallet {
 		pub unstaking_count: u16,
 	}
 
+	impl StakeRecord {
+		/// True when the record carries no state and can be reaped. Shared by
+		/// `unlock` and `unfreeze` so their reap predicates stay in lockstep —
+		/// dropping a record while any field is non-zero (e.g. residual
+		/// `unstaking`) would orphan `PendingUnstakes` entries or its lock.
+		fn is_empty(&self) -> bool {
+			self.hdx == 0 && self.gigahdx == 0 && self.frozen == 0 && self.unstaking == 0 && self.unstaking_count == 0
+		}
+	}
+
 	/// One pending-unstake position. Keyed by the originating block; cooldown
 	/// expiry is `block + Config::CooldownPeriod`.
 	#[derive(Encode, Decode, DecodeWithMemTracking, TypeInfo, MaxEncodedLen, Clone, PartialEq, Eq, Debug)]
@@ -359,22 +369,7 @@ pub mod pallet {
 		#[pallet::weight(T::WeightInfo::giga_stake().saturating_add(T::MoneyMarket::supply_weight()))]
 		pub fn giga_stake(origin: OriginFor<T>, amount: Balance) -> DispatchResult {
 			let who = ensure_signed(origin)?;
-			ensure!(amount >= T::MinStake::get(), Error::<T>::BelowMinStake);
-
-			// Strict policy: refuse if the caller carries any lock the runtime
-			// does not whitelist for overlap. Lock-layering via `max()` would
-			// otherwise let the same HDX back both a gigahdx stake and another
-			// pallet's claim after a single transfer of the unlocked portion.
-			ensure!(T::ExternalClaims::on(&who) == 0, Error::<T>::BlockedByExternalLock);
-
-			// Own commitment (active + pending unstakes) still has to fit
-			// under free_balance — re-staking under the same `ghdxlock` must
-			// not exceed what the user actually owns.
-			let stake = Stakes::<T>::get(&who).unwrap_or_default();
-			let own_claim = stake.hdx.saturating_add(stake.unstaking);
-			let stakeable = T::NativeCurrency::free_balance(&who).saturating_sub(own_claim);
-			ensure!(stakeable >= amount, Error::<T>::InsufficientFreeBalance);
-
+			Self::ensure_stakeable(&who, amount)?;
 			Self::do_stake(&who, amount)?;
 			Ok(())
 		}
@@ -477,7 +472,7 @@ pub mod pallet {
 				if let Some(s) = maybe.as_mut() {
 					s.unstaking = s.unstaking.saturating_sub(entry.amount);
 					s.unstaking_count = s.unstaking_count.saturating_sub(1);
-					if s.hdx == 0 && s.gigahdx == 0 && s.frozen == 0 && s.unstaking_count == 0 {
+					if s.is_empty() {
 						*maybe = None;
 					}
 				}
@@ -562,13 +557,9 @@ pub mod pallet {
 
 			let hdx_unlocked = T::LegacyStaking::force_unstake(&who)?;
 
-			ensure!(hdx_unlocked >= T::MinStake::get(), Error::<T>::BelowMinStake);
-			ensure!(T::ExternalClaims::on(&who) == 0, Error::<T>::BlockedByExternalLock);
-
-			let stake = Stakes::<T>::get(&who).unwrap_or_default();
-			let own_claim = stake.hdx.saturating_add(stake.unstaking);
-			let stakeable = T::NativeCurrency::free_balance(&who).saturating_sub(own_claim);
-			ensure!(stakeable >= hdx_unlocked, Error::<T>::InsufficientFreeBalance);
+			// Admission runs *after* `force_unstake` so the legacy lock is no
+			// longer counted against `ExternalClaims`.
+			Self::ensure_stakeable(&who, hdx_unlocked)?;
 
 			let gigahdx_received = Self::do_stake(&who, hdx_unlocked)?;
 			Self::deposit_event(Event::MigratedFromLegacy {
@@ -773,18 +764,31 @@ pub mod pallet {
 			Stakes::<T>::mutate_exists(who, |maybe| {
 				if let Some(stake) = maybe.as_mut() {
 					stake.frozen = stake.frozen.saturating_sub(delta);
-					if stake.hdx == 0
-						&& stake.gigahdx == 0
-						&& stake.frozen == 0
-						&& stake.unstaking == 0
-						&& stake.unstaking_count == 0
-					{
+					if stake.is_empty() {
 						*maybe = None;
 					}
 				}
 			});
 		}
 
+		/// Admission gate shared by `giga_stake` and `migrate`.
+		///
+		/// Enforces the `MinStake` floor, the strict no-overlapping-lock policy
+		/// (lock-layering via `max()` would otherwise let the same HDX back both
+		/// a gigahdx stake and another pallet's claim after a single transfer of
+		/// the unlocked portion), and that the caller's own commitment (active +
+		/// pending unstakes) plus `amount` still fits under their free balance.
+		fn ensure_stakeable(who: &T::AccountId, amount: Balance) -> DispatchResult {
+			ensure!(amount >= T::MinStake::get(), Error::<T>::BelowMinStake);
+			ensure!(T::ExternalClaims::on(who) == 0, Error::<T>::BlockedByExternalLock);
+
+			let stake = Stakes::<T>::get(who).unwrap_or_default();
+			let own_claim = stake.hdx.saturating_add(stake.unstaking);
+			let stakeable = T::NativeCurrency::free_balance(who).saturating_sub(own_claim);
+			ensure!(stakeable >= amount, Error::<T>::InsufficientFreeBalance);
+			Ok(())
+		}
+
 		/// Computes the stHDX amount at the current rate, mints it into `who`,
 		/// supplies it to the money market, credits the resulting aToken amount
 		/// to `Stakes[who]`, locks `amount` HDX under `Config::LockId`, and emits