From 6e587b5dcab3297185291469316a5e3652e93828 Mon Sep 17 00:00:00 2001 From: Gabe Rodriguez Date: Fri, 31 Oct 2025 15:29:34 -0400 Subject: [PATCH 1/5] Convert warmup/cooldown rate to integers --- Cargo.lock | 2 + interface/Cargo.toml | 1 + interface/src/lib.rs | 1 + interface/src/state.rs | 206 ++++++---- interface/src/warmup_cooldown_allowance.rs | 418 +++++++++++++++++++++ program/Cargo.toml | 4 +- program/src/helpers/merge.rs | 21 +- program/tests/interface.rs | 19 +- program/tests/stake_instruction.rs | 22 +- 9 files changed, 594 insertions(+), 100 deletions(-) create mode 100644 interface/src/warmup_cooldown_allowance.rs diff --git a/Cargo.lock b/Cargo.lock index 68da0e9b..581f4d32 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7150,6 +7150,7 @@ dependencies = [ "bincode", "borsh 1.5.7", "num-traits", + "proptest", "serde", "serde_derive", "serial_test", @@ -7226,6 +7227,7 @@ dependencies = [ "solana-sdk-ids 3.0.0", "solana-signature", "solana-signer", + "solana-stake-interface 2.0.1", "solana-stake-interface 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "solana-svm-log-collector", "solana-system-interface 2.0.0", diff --git a/interface/Cargo.toml b/interface/Cargo.toml index a987f507..a82c9f13 100644 --- a/interface/Cargo.toml +++ b/interface/Cargo.toml @@ -36,6 +36,7 @@ solana-sysvar-id = { version = "3.0.0", optional = true } anyhow = "1" assert_matches = "1.5.0" bincode = "1.3.3" +proptest = "1.9.0" serial_test = "3.2.0" solana-account = {version = "3.2.0", features = ["bincode"] } solana-borsh = "3.0.0" diff --git a/interface/src/lib.rs b/interface/src/lib.rs index d82a9532..9784ed83 100644 --- a/interface/src/lib.rs +++ b/interface/src/lib.rs @@ -13,6 +13,7 @@ pub mod state; #[cfg(feature = "sysvar")] pub mod sysvar; pub mod tools; +pub mod warmup_cooldown_allowance; pub mod program { solana_pubkey::declare_id!("Stake11111111111111111111111111111111111111"); diff --git a/interface/src/state.rs b/interface/src/state.rs index f473eac8..3b97f3db 100644 --- a/interface/src/state.rs +++ b/interface/src/state.rs @@ -12,6 +12,7 @@ use { instruction::LockupArgs, stake_flags::StakeFlags, stake_history::{StakeHistoryEntry, StakeHistoryGetEntry}, + warmup_cooldown_allowance::{calculate_activation_allowance, calculate_cooldown_allowance}, }, solana_clock::{Clock, Epoch, UnixTimestamp}, solana_instruction::error::InstructionError, @@ -23,10 +24,19 @@ pub type StakeActivationStatus = StakeHistoryEntry; // Means that no more than RATE of current effective stake may be added or subtracted per // epoch. +#[deprecated( + since = "2.0.1", + note = "Use PREV_WARMUP_COOLDOWN_RATE_FRACTION instead" +)] pub const DEFAULT_WARMUP_COOLDOWN_RATE: f64 = 0.25; +#[deprecated( + since = "2.0.1", + note = "Use CURR_WARMUP_COOLDOWN_RATE_FRACTION instead" +)] pub const NEW_WARMUP_COOLDOWN_RATE: f64 = 0.09; pub const DEFAULT_SLASH_PENALTY: u8 = ((5 * u8::MAX as usize) / 100) as u8; +#[deprecated(since = "2.0.1", note = "Use warmup_cooldown_rate_fraction() instead")] pub fn warmup_cooldown_rate(current_epoch: Epoch, new_rate_activation_epoch: Option) -> f64 { if current_epoch < new_rate_activation_epoch.unwrap_or(u64::MAX) { DEFAULT_WARMUP_COOLDOWN_RATE @@ -457,23 +467,19 @@ pub struct Delegation { pub activation_epoch: Epoch, /// epoch the stake was deactivated, `std::u64::MAX` if not deactivated pub deactivation_epoch: Epoch, - /// how much stake we can activate per-epoch as a fraction of currently effective stake - #[deprecated( - since = "1.16.7", - note = "Please use `solana_sdk::stake::state::warmup_cooldown_rate()` instead" - )] - pub warmup_cooldown_rate: f64, + /// Formerly the `warmup_cooldown_rate: f64`, but floats are not eBPF-compatible. + /// It is unused, but this field is now reserved to maintain layout compatibility. + pub _reserved: [u8; 8], } impl Default for Delegation { fn default() -> Self { - #[allow(deprecated)] Self { voter_pubkey: Pubkey::default(), stake: 0, activation_epoch: 0, deactivation_epoch: u64::MAX, - warmup_cooldown_rate: DEFAULT_WARMUP_COOLDOWN_RATE, + _reserved: [0; 8], } } } @@ -537,11 +543,13 @@ impl Delegation { }) { // target_epoch > self.deactivation_epoch + // + // We advance epoch-by-epoch from just after the deactivation epoch up to the target_epoch, + // removing (cooling down) the account's share of effective stake each epoch, + // potentially rate-limited by cluster history. - // loop from my deactivation epoch until the target epoch - // current effective stake is updated using its previous epoch's cluster stake let mut current_epoch; - let mut current_effective_stake = effective_stake; + let mut remaining_deactivating_stake = effective_stake; loop { current_epoch = prev_epoch + 1; // if there is no deactivating stake at prev epoch, we should have been @@ -550,38 +558,38 @@ impl Delegation { break; } - // I'm trying to get to zero, how much of the deactivation in stake - // this account is entitled to take - let weight = - current_effective_stake as f64 / prev_cluster_stake.deactivating as f64; - let warmup_cooldown_rate = - warmup_cooldown_rate(current_epoch, new_rate_activation_epoch); - - // portion of newly not-effective cluster stake I'm entitled to at current epoch - let newly_not_effective_cluster_stake = - prev_cluster_stake.effective as f64 * warmup_cooldown_rate; - let newly_not_effective_stake = - ((weight * newly_not_effective_cluster_stake) as u64).max(1); - - current_effective_stake = - current_effective_stake.saturating_sub(newly_not_effective_stake); - if current_effective_stake == 0 { + // Compute how much of this account's stake cools down in `current_epoch` + let newly_deactivated_stake = calculate_cooldown_allowance( + current_epoch, + remaining_deactivating_stake, + &prev_cluster_stake, + new_rate_activation_epoch, + ); + remaining_deactivating_stake = + remaining_deactivating_stake.saturating_sub(newly_deactivated_stake.max(1)); + + // Stop if we've fully cooled down this account + if remaining_deactivating_stake == 0 { break; } + // Stop when we've reached the time bound for this query if current_epoch >= target_epoch { break; } + + // Advance to the next epoch if we have history, otherwise we can't model further cooldown if let Some(current_cluster_stake) = history.get_entry(current_epoch) { prev_epoch = current_epoch; prev_cluster_stake = current_cluster_stake; } else { + // No more history data, return the best-effort state as of the last known epoch break; } } - // deactivating stake should equal to all of currently remaining effective stake - StakeActivationStatus::with_deactivating(current_effective_stake) + // Report how much stake remains in cooldown at `target_epoch` + StakeActivationStatus::with_deactivating(remaining_deactivating_stake) } else { // no history or I've dropped out of history, so assume fully deactivated StakeActivationStatus::default() @@ -621,11 +629,13 @@ impl Delegation { }) { // target_epoch > self.activation_epoch + // + // We advance epoch-by-epoch from just after the activation epoch up to the target_epoch, + // accumulating (warming up) the account's share of effective stake each epoch, + // potentially rate-limited by cluster history. - // loop from my activation epoch until the target epoch summing up my entitlement - // current effective stake is updated using its previous epoch's cluster stake let mut current_epoch; - let mut current_effective_stake = 0; + let mut activated_stake_amount = 0; loop { current_epoch = prev_epoch + 1; // if there is no activating stake at prev epoch, we should have been @@ -634,40 +644,43 @@ impl Delegation { break; } - // how much of the growth in stake this account is - // entitled to take - let remaining_activating_stake = delegated_stake - current_effective_stake; - let weight = - remaining_activating_stake as f64 / prev_cluster_stake.activating as f64; - let warmup_cooldown_rate = - warmup_cooldown_rate(current_epoch, new_rate_activation_epoch); - - // portion of newly effective cluster stake I'm entitled to at current epoch - let newly_effective_cluster_stake = - prev_cluster_stake.effective as f64 * warmup_cooldown_rate; - let newly_effective_stake = - ((weight * newly_effective_cluster_stake) as u64).max(1); - - current_effective_stake += newly_effective_stake; - if current_effective_stake >= delegated_stake { - current_effective_stake = delegated_stake; + // Calculate how much of this account's remaining stake becomes effective in `current_epoch`. + let remaining_activating_stake = delegated_stake - activated_stake_amount; + let newly_effective_stake = calculate_activation_allowance( + current_epoch, + remaining_activating_stake, + &prev_cluster_stake, + new_rate_activation_epoch, + ); + + // Add the newly effective stake, ensuring at least one lamport activates to prevent getting stuck. + activated_stake_amount += newly_effective_stake.max(1); + + // Stop if we've fully warmed up this account's stake. + if activated_stake_amount >= delegated_stake { + activated_stake_amount = delegated_stake; break; } + // Stop when we've reached the time bound for this query if current_epoch >= target_epoch || current_epoch >= self.deactivation_epoch { break; } + + // Advance to the next epoch if we have history, otherwise we can't model further warmup if let Some(current_cluster_stake) = history.get_entry(current_epoch) { prev_epoch = current_epoch; prev_cluster_stake = current_cluster_stake; } else { + // No more history data, return the best-effort state as of the last known epoch break; } } + // Return the portion that has become effective and the portion still activating ( - current_effective_stake, - delegated_stake - current_effective_stake, + activated_stake_amount, + delegated_stake - activated_stake_amount, ) } else { // no history or I've dropped out of history, so assume fully effective @@ -738,7 +751,9 @@ impl Stake { mod tests { use { super::*, - crate::stake_history::StakeHistory, + crate::{ + stake_history::StakeHistory, warmup_cooldown_allowance::warmup_cooldown_rate_fraction, + }, assert_matches::assert_matches, bincode::serialize, solana_account::{state_traits::StateMut, AccountSharedData, ReadableAccount}, @@ -967,7 +982,8 @@ mod tests { }; // save this off so stake.config.warmup_rate changes don't break this test - let increment = (1_000_f64 * warmup_cooldown_rate(0, None)) as u64; + let (rate_num, rate_den) = warmup_cooldown_rate_fraction(0, None); + let increment = ((1_000u128 * rate_num) / rate_den) as u64; let mut stake_history = StakeHistory::default(); // assert that this stake follows step function if there's no history @@ -1336,6 +1352,7 @@ mod tests { let mut effective = base_stake; let other_activation = 100; let mut other_activations = vec![0]; + let (rate_num, rate_den) = warmup_cooldown_rate_fraction(0, None); // Build a stake history where the test staker always consumes all of the available warm // up and cool down stake. However, simulate other stakers beginning to activate during @@ -1358,7 +1375,7 @@ mod tests { }, ); - let effective_rate_limited = (effective as f64 * warmup_cooldown_rate(0, None)) as u64; + let effective_rate_limited = ((effective as u128) * rate_num / rate_den) as u64; if epoch < stake.deactivation_epoch { effective += effective_rate_limited.min(activating); other_activations.push(0); @@ -1401,7 +1418,8 @@ mod tests { let epochs = 7; // make bootstrap stake smaller than warmup so warmup/cooldownn // increment is always smaller than 1 - let bootstrap = (warmup_cooldown_rate(0, None) * 100.0 / 2.0) as u64; + let (rate_num, rate_den) = warmup_cooldown_rate_fraction(0, None); + let bootstrap = ((100u128 * rate_num) / (2u128 * rate_den)) as u64; let stake_history = create_stake_history_from_delegations(Some(bootstrap), 0..epochs, &delegations, None); let mut max_stake = 0; @@ -1503,13 +1521,10 @@ mod tests { // (0..(total_effective_stake as usize / (delegations.len() * 5))).for_each(|_| eprint("#")); // eprintln(); - assert!( - delta - <= ((prev_total_effective_stake as f64 - * warmup_cooldown_rate(epoch, new_rate_activation_epoch)) - as u64) - .max(1) - ); + let (rate_num, rate_den) = + warmup_cooldown_rate_fraction(epoch, new_rate_activation_epoch); + let max_delta = ((prev_total_effective_stake as u128) * rate_num / rate_den) as u64; + assert!(delta <= max_delta.max(1)); prev_total_effective_stake = total_effective_stake; } @@ -1702,7 +1717,7 @@ mod tests { stake: u64::MAX, activation_epoch: Epoch::MAX, deactivation_epoch: Epoch::MAX, - warmup_cooldown_rate: f64::MAX, + _reserved: [0; 8], }, credits_observed: 1, }, @@ -1724,7 +1739,10 @@ mod tests { } mod deprecated { - use super::*; + use { + super::*, + static_assertions::{assert_eq_align, assert_eq_size}, + }; fn check_borsh_deserialization(stake: StakeState) { let serialized = serialize(&stake).unwrap(); @@ -1770,7 +1788,7 @@ mod tests { stake: u64::MAX, activation_epoch: Epoch::MAX, deactivation_epoch: Epoch::MAX, - warmup_cooldown_rate: f64::MAX, + _reserved: [0; 8], }, credits_observed: 1, }, @@ -1804,7 +1822,7 @@ mod tests { stake: u64::MAX, activation_epoch: Epoch::MAX, deactivation_epoch: Epoch::MAX, - warmup_cooldown_rate: f64::MAX, + _reserved: [0; 8], }, credits_observed: 1, }, @@ -1835,5 +1853,61 @@ mod tests { }) ); } + + /// Contains legacy struct definitions to verify memory layout compatibility. + mod legacy { + use super::*; + + #[derive(borsh::BorshSerialize, borsh::BorshDeserialize)] + #[borsh(crate = "borsh")] + pub struct Delegation { + pub voter_pubkey: Pubkey, + pub stake: u64, + pub activation_epoch: Epoch, + pub deactivation_epoch: Epoch, + pub warmup_cooldown_rate: f64, + } + } + + #[test] + fn test_delegation_struct_layout_compatibility() { + assert_eq_size!(Delegation, legacy::Delegation); + assert_eq_align!(Delegation, legacy::Delegation); + } + + #[test] + #[allow(clippy::used_underscore_binding)] + fn test_delegation_deserialization_from_legacy_format() { + let legacy_delegation = legacy::Delegation { + voter_pubkey: Pubkey::new_unique(), + stake: 12345, + activation_epoch: 10, + deactivation_epoch: 20, + warmup_cooldown_rate: NEW_WARMUP_COOLDOWN_RATE, + }; + + let serialized_data = borsh::to_vec(&legacy_delegation).unwrap(); + + // Deserialize into the NEW Delegation struct + let new_delegation = Delegation::try_from_slice(&serialized_data).unwrap(); + + // Assert that the fields are identical + assert_eq!(new_delegation.voter_pubkey, legacy_delegation.voter_pubkey); + assert_eq!(new_delegation.stake, legacy_delegation.stake); + assert_eq!( + new_delegation.activation_epoch, + legacy_delegation.activation_epoch + ); + assert_eq!( + new_delegation.deactivation_epoch, + legacy_delegation.deactivation_epoch + ); + + // Assert that the `reserved` bytes now contain the raw bits of the old f64 + assert_eq!( + new_delegation._reserved, + NEW_WARMUP_COOLDOWN_RATE.to_le_bytes() + ); + } } } diff --git a/interface/src/warmup_cooldown_allowance.rs b/interface/src/warmup_cooldown_allowance.rs new file mode 100644 index 00000000..e34585d9 --- /dev/null +++ b/interface/src/warmup_cooldown_allowance.rs @@ -0,0 +1,418 @@ +use {crate::stake_history::StakeHistoryEntry, solana_clock::Epoch}; + +type Numerator = u128; +type Denominator = u128; + +pub const PREV_WARMUP_COOLDOWN_RATE_FRACTION: (Numerator, Denominator) = (1, 4); // 25% +pub const CURR_WARMUP_COOLDOWN_RATE_FRACTION: (Numerator, Denominator) = (9, 100); // 9% + +#[inline] +pub fn warmup_cooldown_rate_fraction( + epoch: Epoch, + new_rate_activation_epoch: Option, +) -> (Numerator, Denominator) { + if epoch < new_rate_activation_epoch.unwrap_or(u64::MAX) { + PREV_WARMUP_COOLDOWN_RATE_FRACTION + } else { + CURR_WARMUP_COOLDOWN_RATE_FRACTION + } +} + +/// Calculates the potentially rate-limited stake warmup for a single account in the current epoch. +/// +/// This function allocates a share of the cluster's per-epoch activation allowance +/// proportional to the account's share of the previous epoch's total activating stake. +pub fn calculate_activation_allowance( + current_epoch: Epoch, + account_activating_stake: u64, + prev_epoch_cluster_state: &StakeHistoryEntry, + new_rate_activation_epoch: Option, +) -> u64 { + rate_limited_stake_change( + current_epoch, + account_activating_stake, + prev_epoch_cluster_state.activating, + prev_epoch_cluster_state.effective, + new_rate_activation_epoch, + ) +} + +/// Calculates the potentially rate-limited stake cooldown for a single account in the current epoch. +/// +/// This function allocates a share of the cluster's per-epoch deactivation allowance +/// proportional to the account's share of the previous epoch's total deactivating stake. +pub fn calculate_cooldown_allowance( + current_epoch: Epoch, + account_deactivating_stake: u64, + prev_epoch_cluster_state: &StakeHistoryEntry, + new_rate_activation_epoch: Option, +) -> u64 { + rate_limited_stake_change( + current_epoch, + account_deactivating_stake, + prev_epoch_cluster_state.deactivating, + prev_epoch_cluster_state.effective, + new_rate_activation_epoch, + ) +} + +/// Internal helper for the rate-limited stake change calculation. +fn rate_limited_stake_change( + epoch: Epoch, + account_portion: u64, + cluster_portion: u64, + cluster_effective: u64, + new_rate_activation_epoch: Option, +) -> u64 { + // Early return if there's no stake to change (also prevents divide by zero) + if account_portion == 0 || cluster_portion == 0 || cluster_effective == 0 { + return 0; + } + + let (rate_numerator, rate_denominator) = + warmup_cooldown_rate_fraction(epoch, new_rate_activation_epoch); + + // Calculate this account's proportional share of the network-wide stake change allowance for the epoch. + // Formula: `change = (account_portion / cluster_portion) * (cluster_effective * rate)` + // Where: + // - `(account_portion / cluster_portion)` is this account's share of the pool. + // - `(cluster_effective * rate)` is the total network allowance for change this epoch. + // + // Re-arranged formula to use only integer arithmetic w/ rate fraction: + // `change = (account_portion * cluster_effective * rate_numerator) / (cluster_portion * rate_denominator)` + // + // Using `u128` for the intermediate calculations to prevent overflow. + let numerator = (account_portion as u128) + .checked_mul(cluster_effective as u128) + .and_then(|x| x.checked_mul(rate_numerator)); + let denominator = (cluster_portion as u128).saturating_mul(rate_denominator); + + match numerator { + Some(n) => { + let delta = n.checked_div(denominator).unwrap_or(u128::MAX); + // The calculated delta can be larger than `account_portion` if the network's stake change + // allowance is greater than the total stake waiting to change. In this case, the account's + // entire portion is allowed to change. + if delta >= account_portion as u128 { + account_portion + } else { + // The cast is safe because we have just confirmed that `delta` is less than + // `account_portion`, which is a `u64`. + delta as u64 + } + } + // Overflowing u128 is not a realistic scenario except in tests. However, in that case + // it's reasonable to allow activation/deactivation of the account's entire portion. + None => account_portion, + } +} + +#[cfg(test)] +mod test { + #[allow(deprecated)] + use crate::state::{DEFAULT_WARMUP_COOLDOWN_RATE, NEW_WARMUP_COOLDOWN_RATE}; + use {super::*, proptest::prelude::*}; + + // === Rate selector === + + #[test] + fn rate_fraction_before_activation_epoch_uses_prev_rate() { + let epoch = 9; + let new_rate_activation_epoch = Some(10); + let frac = warmup_cooldown_rate_fraction(epoch, new_rate_activation_epoch); + assert_eq!(frac, PREV_WARMUP_COOLDOWN_RATE_FRACTION); + } + + #[test] + fn rate_fraction_at_or_after_activation_epoch_uses_curr_rate() { + let epoch = 10; + let new_rate_activation_epoch = Some(10); + assert_eq!( + warmup_cooldown_rate_fraction(epoch, new_rate_activation_epoch), + CURR_WARMUP_COOLDOWN_RATE_FRACTION + ); + let epoch2 = 11; + assert_eq!( + warmup_cooldown_rate_fraction(epoch2, new_rate_activation_epoch), + CURR_WARMUP_COOLDOWN_RATE_FRACTION + ); + } + + #[test] + fn rate_fraction_none_activation_epoch_behaves_like_prev_rate() { + let epoch = 123; + let frac = warmup_cooldown_rate_fraction(epoch, None); + assert_eq!(frac, PREV_WARMUP_COOLDOWN_RATE_FRACTION); + } + + // === Activation allowance === + + #[test] + fn activation_zero_cases_return_zero() { + // account_portion == 0 + let prev = StakeHistoryEntry { + activating: 10, + effective: 100, + ..Default::default() + }; + assert_eq!(calculate_activation_allowance(0, 0, &prev, Some(0)), 0); + + // cluster_portion == 0 + let prev = StakeHistoryEntry { + activating: 0, + effective: 100, + ..Default::default() + }; + assert_eq!(calculate_activation_allowance(0, 5, &prev, Some(0)), 0); + + // cluster_effective == 0 + let prev = StakeHistoryEntry { + activating: 10, + effective: 0, + ..Default::default() + }; + assert_eq!(calculate_activation_allowance(0, 5, &prev, Some(0)), 0); + } + + #[test] + fn activation_basic_proportional_prev_rate() { + // cluster_effective = 1000, prev rate = 1/4 => total allowance = 250 + // account share = 100 / 500 -> 1/5 => expected 50 + let current_epoch = 99; + let new_rate_activation_epoch = Some(100); + let prev = StakeHistoryEntry { + activating: 500, + effective: 1000, + ..Default::default() + }; + let result = + calculate_activation_allowance(current_epoch, 100, &prev, new_rate_activation_epoch); + assert_eq!(result, 50); + } + + #[test] + fn activation_caps_at_account_portion_when_network_allowance_is_large() { + // total network allowance enormous relative to waiting stake, should cap to account_portion. + let current_epoch = 99; + let new_rate_activation_epoch = Some(100); // prev rate (1/4) + let prev = StakeHistoryEntry { + activating: 100, // cluster_portion + effective: 1_000_000, // large cluster effective + ..Default::default() + }; + let account_portion = 40; + let result = calculate_activation_allowance( + current_epoch, + account_portion, + &prev, + new_rate_activation_epoch, + ); + assert_eq!(result, account_portion); + } + + #[test] + fn activation_overflow_path_returns_account_portion() { + // Force the u128 multiply to overflow: (u64::MAX * u64::MAX * 9) overflows u128. + // When that happens, the helper returns the full account_portion. + let current_epoch = 0; + let new_rate_activation_epoch = Some(0); // use "current" 9/100 to maximize multiplier + let prev = StakeHistoryEntry { + activating: 1, // non-zero cluster_portion + effective: u64::MAX, // huge cluster_effective + ..Default::default() + }; + let account_portion = u64::MAX; + let result = calculate_activation_allowance( + current_epoch, + account_portion, + &prev, + new_rate_activation_epoch, + ); + assert_eq!(result, account_portion); + } + + // === Cooldown allowance === + + #[test] + fn cooldown_zero_cases_return_zero() { + // account_portion == 0 + let prev = StakeHistoryEntry { + deactivating: 10, + effective: 100, + ..Default::default() + }; + assert_eq!(calculate_cooldown_allowance(0, 0, &prev, Some(0)), 0); + + // cluster_portion == 0 + let prev = StakeHistoryEntry { + deactivating: 0, + effective: 100, + ..Default::default() + }; + assert_eq!(calculate_cooldown_allowance(0, 5, &prev, Some(0)), 0); + + // cluster_effective == 0 + let prev = StakeHistoryEntry { + deactivating: 10, + effective: 0, + ..Default::default() + }; + assert_eq!(calculate_cooldown_allowance(0, 5, &prev, Some(0)), 0); + } + + #[test] + fn cooldown_basic_proportional_curr_rate() { + // cluster_effective = 10_000, curr rate = 9/100 => total allowance = 900 + // account share = 200 / 1000 => expected 180 + let current_epoch = 5; + let new_rate_activation_epoch = Some(5); // current (epoch >= activation) + let prev = StakeHistoryEntry { + deactivating: 1000, + effective: 10_000, + ..Default::default() + }; + let result = + calculate_cooldown_allowance(current_epoch, 200, &prev, new_rate_activation_epoch); + assert_eq!(result, 180); + } + + #[test] + fn cooldown_caps_at_account_portion_when_network_allowance_is_large() { + let current_epoch = 0; + let new_rate_activation_epoch = None; // uses prev (1/4) + let prev = StakeHistoryEntry { + deactivating: 100, + effective: 1_000_000, + ..Default::default() + }; + let account_portion = 70; + let result = calculate_cooldown_allowance( + current_epoch, + account_portion, + &prev, + new_rate_activation_epoch, + ); + assert_eq!(result, account_portion); + } + + // === Symmetry & integer rounding === + + #[test] + fn activation_and_cooldown_are_symmetric_given_same_inputs() { + // With equal cluster_portions and same epoch/rate, the math should match. + let epoch = 42; + let new_rate_activation_epoch = Some(1_000); // prev rate for both calls + let prev = StakeHistoryEntry { + activating: 1_000, + deactivating: 1_000, + effective: 5_000, + }; + let account = 333; + let act = calculate_activation_allowance(epoch, account, &prev, new_rate_activation_epoch); + let cool = calculate_cooldown_allowance(epoch, account, &prev, new_rate_activation_epoch); + assert_eq!(act, cool); + } + + #[test] + fn integer_division_truncation_matches_expected() { + // Float math would yield 90.009, integer math must truncate to 90 + let account_portion = 100; + let cluster_portion = 1000; + let cluster_effective = 10001; + let epoch = 20; + let new_rate_activation_epoch = Some(10); // current 9/100 + + let result = rate_limited_stake_change( + epoch, + account_portion, + cluster_portion, + cluster_effective, + new_rate_activation_epoch, + ); + assert_eq!(result, 90); + } + + // === Property tests: compare the integer refactor vs legacy f64 === + + #[allow(deprecated)] + fn legacy_warmup_cooldown_rate( + current_epoch: Epoch, + new_rate_activation_epoch: Option, + ) -> f64 { + if current_epoch < new_rate_activation_epoch.unwrap_or(u64::MAX) { + DEFAULT_WARMUP_COOLDOWN_RATE + } else { + NEW_WARMUP_COOLDOWN_RATE + } + } + + // The original formula used prior to integer implementation + fn calculate_stake_delta_f64_legacy( + account_portion: u64, + cluster_portion: u64, + cluster_effective: u64, + current_epoch: Epoch, + new_rate_activation_epoch: Option, + ) -> u64 { + if cluster_portion == 0 || account_portion == 0 || cluster_effective == 0 { + return 0; + } + let weight = account_portion as f64 / cluster_portion as f64; + let rate = legacy_warmup_cooldown_rate(current_epoch, new_rate_activation_epoch); + let newly_effective_cluster_stake = cluster_effective as f64 * rate; + (weight * newly_effective_cluster_stake) as u64 + } + + proptest! { + #[test] + fn rate_limited_change_consistent_with_legacy( + account_portion in 0u64..=u64::MAX, + cluster_portion in 0u64..=u64::MAX, + cluster_effective in 0u64..=u64::MAX, + current_epoch in 0u64..=2000, + new_rate_activation_epoch_option in prop::option::of(0u64..=2000), + ) { + let integer_math_result = rate_limited_stake_change( + current_epoch, + account_portion, + cluster_portion, + cluster_effective, + new_rate_activation_epoch_option, + ); + + let float_math_result = calculate_stake_delta_f64_legacy( + account_portion, + cluster_portion, + cluster_effective, + current_epoch, + new_rate_activation_epoch_option, + ).min(account_portion); + + let (rate_num, _rate_den) = + warmup_cooldown_rate_fraction(current_epoch, new_rate_activation_epoch_option); + + // See if the u128 product would overflow: account * effective * rate_num + let would_overflow = (account_portion as u128) + .checked_mul(cluster_effective as u128) + .and_then(|n| n.checked_mul(rate_num)) + .is_none(); + + if account_portion == 0 || cluster_portion == 0 || cluster_effective == 0 { + prop_assert_eq!(integer_math_result, 0); + prop_assert_eq!(float_math_result, 0); + } else if would_overflow { + // In the u128 overflow region, the f64 implementation is guaranteed to be imprecise. + // We only assert that our implementation correctly falls back to account_portion. + prop_assert_eq!(integer_math_result, account_portion); + } else { + prop_assert!(integer_math_result <= account_portion); + prop_assert!(float_math_result <= account_portion); + // The `f64`-based oracle calculation loses precision when `u64` inputs exceed 2^53. + // The new integer-based implementation is more precise using `u128` arithmetic. + // The small difference seen in failing tests is a measure of this legacy inaccuracy. + let diff = integer_math_result.abs_diff(float_math_result) as u128; + prop_assert!(diff <= 3750, "candidate={}, oracle={}, diff={}", integer_math_result, float_math_result, diff); + } + } + } +} diff --git a/program/Cargo.toml b/program/Cargo.toml index 4d1737f6..b1c50386 100644 --- a/program/Cargo.toml +++ b/program/Cargo.toml @@ -19,7 +19,9 @@ solana-program-entrypoint = "3.0.0" solana-program-error = "3.0.0" solana-pubkey = "3.0.0" solana-rent = "3.0.0" -solana-stake-interface = { version = "2", features = ["bincode", "borsh", "sysvar"] } +solana-stake-interface = { path = "../interface", version = "2", features = ["bincode", "borsh", "sysvar"] } +# TODO: Remove when mollusk updates to use latest interface package +solana-stake-interface-legacy = { package = "solana-stake-interface", version = "2", features = ["bincode", "borsh", "sysvar"] } solana-sysvar = "3.0.0" solana-vote-interface = { version = "4.0.4", features = ["bincode"] } diff --git a/program/src/helpers/merge.rs b/program/src/helpers/merge.rs index 84ffa097..eb3906c4 100644 --- a/program/src/helpers/merge.rs +++ b/program/src/helpers/merge.rs @@ -236,7 +236,10 @@ mod tests { solana_account::{state_traits::StateMut, AccountSharedData, ReadableAccount}, solana_pubkey::Pubkey, solana_rent::Rent, - solana_stake_interface::stake_history::{StakeHistory, StakeHistoryEntry}, + solana_stake_interface::{ + stake_history::{StakeHistory, StakeHistoryEntry}, + warmup_cooldown_allowance::warmup_cooldown_rate_fraction, + }, }; #[test] @@ -531,10 +534,10 @@ mod tests { // all paritially activated, transient epochs fail loop { clock.epoch += 1; - let delta = activating.min( - (effective as f64 * warmup_cooldown_rate(clock.epoch, new_rate_activation_epoch)) - as u64, - ); + let (rate_num, rate_den) = + warmup_cooldown_rate_fraction(clock.epoch, new_rate_activation_epoch); + let rate_limited = ((effective as u128) * rate_num / rate_den) as u64; + let delta = activating.min(rate_limited); effective += delta; activating -= delta; stake_history.add( @@ -608,10 +611,10 @@ mod tests { // all transient, deactivating epochs fail loop { clock.epoch += 1; - let delta = deactivating.min( - (effective as f64 * warmup_cooldown_rate(clock.epoch, new_rate_activation_epoch)) - as u64, - ); + let (rate_num, rate_den) = + warmup_cooldown_rate_fraction(clock.epoch, new_rate_activation_epoch); + let rate_limited = ((effective as u128) * rate_num / rate_den) as u64; + let delta = deactivating.min(rate_limited); effective -= delta; deactivating -= delta; stake_history.add( diff --git a/program/tests/interface.rs b/program/tests/interface.rs index 0631e27a..8c31d457 100644 --- a/program/tests/interface.rs +++ b/program/tests/interface.rs @@ -15,11 +15,9 @@ use { solana_stake_interface::{ instruction::{self, LockupArgs}, stake_flags::StakeFlags, - stake_history::{StakeHistory, StakeHistoryEntry}, - state::{ - warmup_cooldown_rate, Authorized, Delegation, Lockup, Meta, Stake, StakeAuthorize, - StakeStateV2, NEW_WARMUP_COOLDOWN_RATE, - }, + stake_history::StakeHistory, + state::{Authorized, Delegation, Lockup, Meta, Stake, StakeAuthorize, StakeStateV2}, + warmup_cooldown_allowance::warmup_cooldown_rate_fraction, }, solana_stake_program::{get_minimum_delegation, id}, solana_svm_log_collector::LogCollector, @@ -100,10 +98,6 @@ const CUSTODIAN_RIGHT: Pubkey = // while also making it easy to write tests involving partial (de)activations // if the warmup/cooldown rate changes, this number must be adjusted const PERSISTENT_ACTIVE_STAKE: u64 = 100 * LAMPORTS_PER_SOL; -#[test] -fn assert_warmup_cooldown_rate() { - assert_eq!(warmup_cooldown_rate(0, Some(0)), NEW_WARMUP_COOLDOWN_RATE); -} // hardcoded for convenience const STAKE_RENT_EXEMPTION: u64 = 2_282_880; @@ -148,12 +142,13 @@ impl Env { assert_eq!(mollusk.sysvars.clock.epoch, EXECUTION_EPOCH); // backfill stake history - let stake_delta_amount = - (PERSISTENT_ACTIVE_STAKE as f64 * NEW_WARMUP_COOLDOWN_RATE).floor() as u64; + let (num, den) = warmup_cooldown_rate_fraction(0, Some(0)); + let stake_delta_amount = ((PERSISTENT_ACTIVE_STAKE as u128 * num) / den) as u64; for epoch in 0..EXECUTION_EPOCH { mollusk.sysvars.stake_history.add( epoch, - StakeHistoryEntry { + // TODO: Remove when mollusk updates to latest solana-stake-interface version + solana_stake_interface_legacy::stake_history::StakeHistoryEntry { effective: PERSISTENT_ACTIVE_STAKE, activating: stake_delta_amount, deactivating: stake_delta_amount, diff --git a/program/tests/stake_instruction.rs b/program/tests/stake_instruction.rs index 8aa9fd5a..030771b2 100644 --- a/program/tests/stake_instruction.rs +++ b/program/tests/stake_instruction.rs @@ -27,10 +27,8 @@ use { }, stake_flags::StakeFlags, stake_history::{StakeHistory, StakeHistoryEntry}, - state::{ - warmup_cooldown_rate, Authorized, Delegation, Lockup, Meta, Stake, StakeAuthorize, - StakeStateV2, - }, + state::{Authorized, Delegation, Lockup, Meta, Stake, StakeAuthorize, StakeStateV2}, + warmup_cooldown_allowance::warmup_cooldown_rate_fraction, MINIMUM_DELINQUENT_EPOCHS_FOR_DEACTIVATION, }, solana_stake_program::{get_minimum_delegation, id}, @@ -6430,10 +6428,10 @@ fn test_merge_active_stake() { if clock.epoch == merge_from_activation_epoch { activating += merge_from_amount; } - let delta = activating.min( - (effective as f64 * warmup_cooldown_rate(clock.epoch, new_warmup_cooldown_rate_epoch)) - as u64, - ); + let (rate_num, rate_den) = + warmup_cooldown_rate_fraction(clock.epoch, new_warmup_cooldown_rate_epoch); + let rate_limited = ((effective as u128) * rate_num / rate_den) as u64; + let delta = activating.min(rate_limited); effective += delta; activating -= delta; stake_history.add( @@ -6482,10 +6480,10 @@ fn test_merge_active_stake() { // active/deactivating and deactivating/inactive mismatches fail loop { clock.epoch += 1; - let delta = deactivating.min( - (effective as f64 * warmup_cooldown_rate(clock.epoch, new_warmup_cooldown_rate_epoch)) - as u64, - ); + let (rate_num, rate_den) = + warmup_cooldown_rate_fraction(clock.epoch, new_warmup_cooldown_rate_epoch); + let rate_limited = ((effective as u128) * rate_num / rate_den) as u64; + let delta = deactivating.min(rate_limited); effective -= delta; deactivating -= delta; if clock.epoch == stake_deactivation_epoch { From 9c0ba5cdc6454c786a27c85380a9d942d301fe50 Mon Sep 17 00:00:00 2001 From: Gabe Rodriguez Date: Tue, 18 Nov 2025 09:05:19 +0100 Subject: [PATCH 2/5] Review updates --- Cargo.lock | 33 +- Cargo.toml | 3 + clients/js/src/generated/types/delegation.ts | 15 +- .../rust/src/generated/types/delegation.rs | 2 +- interface/idl.json | 7 +- interface/src/lib.rs | 2 + interface/src/state.rs | 358 ++++++++++++++++-- interface/src/test_utils.rs | 106 ++++++ interface/src/warmup_cooldown_allowance.rs | 105 +++-- program/Cargo.toml | 2 - program/src/helpers/merge.rs | 12 +- program/tests/interface.rs | 12 +- program/tests/stake_instruction.rs | 12 +- scripts/solana.dic | 1 + 14 files changed, 535 insertions(+), 135 deletions(-) create mode 100644 interface/src/test_utils.rs diff --git a/Cargo.lock b/Cargo.lock index 581f4d32..ae0b44ad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -143,7 +143,7 @@ dependencies = [ "solana-secp256k1-recover 3.0.0", "solana-sha256-hasher 3.0.0", "solana-stable-layout 3.0.0", - "solana-stake-interface 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)", + "solana-stake-interface 2.0.1", "solana-svm-callback", "solana-svm-feature-set", "solana-svm-log-collector", @@ -2849,7 +2849,7 @@ dependencies = [ "solana-rent 3.0.0", "solana-sdk-ids 3.0.0", "solana-slot-hashes 3.0.0", - "solana-stake-interface 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)", + "solana-stake-interface 2.0.1", "solana-stake-program 3.0.10", "solana-svm-callback", "solana-svm-log-collector", @@ -6255,7 +6255,7 @@ dependencies = [ "solana-sbpf", "solana-sdk-ids 3.0.0", "solana-slot-hashes 3.0.0", - "solana-stake-interface 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)", + "solana-stake-interface 2.0.1", "solana-svm-callback", "solana-svm-feature-set", "solana-svm-log-collector", @@ -6317,7 +6317,7 @@ dependencies = [ "solana-sdk-ids 3.0.0", "solana-signer", "solana-stable-layout 3.0.0", - "solana-stake-interface 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)", + "solana-stake-interface 2.0.1", "solana-svm", "solana-svm-log-collector", "solana-svm-timings", @@ -6690,7 +6690,7 @@ dependencies = [ "solana-signer", "solana-slot-hashes 3.0.0", "solana-slot-history 3.0.0", - "solana-stake-interface 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)", + "solana-stake-interface 2.0.1", "solana-stake-program 3.0.10", "solana-svm", "solana-svm-callback", @@ -7175,26 +7175,6 @@ dependencies = [ "test-case", ] -[[package]] -name = "solana-stake-interface" -version = "2.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f6f912ae679b683365348dea482dbd9468d22ff258b554fd36e3d3683c2122e3" -dependencies = [ - "borsh 1.5.7", - "num-traits", - "serde", - "serde_derive", - "solana-clock 3.0.0", - "solana-cpi 3.0.0", - "solana-instruction 3.0.0", - "solana-program-error 3.0.0", - "solana-pubkey 3.0.0", - "solana-system-interface 2.0.0", - "solana-sysvar 3.0.0", - "solana-sysvar-id 3.0.0", -] - [[package]] name = "solana-stake-program" version = "1.0.0" @@ -7228,7 +7208,6 @@ dependencies = [ "solana-signature", "solana-signer", "solana-stake-interface 2.0.1", - "solana-stake-interface 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "solana-svm-log-collector", "solana-system-interface 2.0.0", "solana-sysvar 3.0.0", @@ -7259,7 +7238,7 @@ dependencies = [ "solana-pubkey 3.0.0", "solana-rent 3.0.0", "solana-sdk-ids 3.0.0", - "solana-stake-interface 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)", + "solana-stake-interface 2.0.1", "solana-svm-log-collector", "solana-svm-type-overrides", "solana-sysvar 3.0.0", diff --git a/Cargo.toml b/Cargo.toml index 6b8cd32a..ea6dba20 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,3 +31,6 @@ test = "nightly-2025-02-16" [workspace.metadata.spellcheck] config = "scripts/spellcheck.toml" + +[patch.crates-io] +solana-stake-interface = { path = "interface" } diff --git a/clients/js/src/generated/types/delegation.ts b/clients/js/src/generated/types/delegation.ts index 0e50a564..51c88f2b 100644 --- a/clients/js/src/generated/types/delegation.ts +++ b/clients/js/src/generated/types/delegation.ts @@ -8,10 +8,12 @@ import { combineCodec, + fixDecoderSize, + fixEncoderSize, getAddressDecoder, getAddressEncoder, - getF64Decoder, - getF64Encoder, + getBytesDecoder, + getBytesEncoder, getStructDecoder, getStructEncoder, getU64Decoder, @@ -20,6 +22,7 @@ import { type FixedSizeCodec, type FixedSizeDecoder, type FixedSizeEncoder, + type ReadonlyUint8Array, } from '@solana/kit'; export type Delegation = { @@ -27,7 +30,7 @@ export type Delegation = { stake: bigint; activationEpoch: bigint; deactivationEpoch: bigint; - warmupCooldownRate: number; + reserved: ReadonlyUint8Array; }; export type DelegationArgs = { @@ -35,7 +38,7 @@ export type DelegationArgs = { stake: number | bigint; activationEpoch: number | bigint; deactivationEpoch: number | bigint; - warmupCooldownRate: number; + reserved: ReadonlyUint8Array; }; export function getDelegationEncoder(): FixedSizeEncoder { @@ -44,7 +47,7 @@ export function getDelegationEncoder(): FixedSizeEncoder { ['stake', getU64Encoder()], ['activationEpoch', getU64Encoder()], ['deactivationEpoch', getU64Encoder()], - ['warmupCooldownRate', getF64Encoder()], + ['reserved', fixEncoderSize(getBytesEncoder(), 8)], ]); } @@ -54,7 +57,7 @@ export function getDelegationDecoder(): FixedSizeDecoder { ['stake', getU64Decoder()], ['activationEpoch', getU64Decoder()], ['deactivationEpoch', getU64Decoder()], - ['warmupCooldownRate', getF64Decoder()], + ['reserved', fixDecoderSize(getBytesDecoder(), 8)], ]); } diff --git a/clients/rust/src/generated/types/delegation.rs b/clients/rust/src/generated/types/delegation.rs index a9066b51..697ee7f6 100644 --- a/clients/rust/src/generated/types/delegation.rs +++ b/clients/rust/src/generated/types/delegation.rs @@ -21,5 +21,5 @@ pub struct Delegation { pub stake: u64, pub activation_epoch: u64, pub deactivation_epoch: u64, - pub warmup_cooldown_rate: f64, + pub reserved: [u8; 8], } diff --git a/interface/idl.json b/interface/idl.json index 87cf2042..5e4983c5 100644 --- a/interface/idl.json +++ b/interface/idl.json @@ -959,9 +959,12 @@ } }, { - "name": "warmupCooldownRate", + "name": "_reserved", "type": { - "defined": "f64" + "array": [ + "u8", + 8 + ] } } ] diff --git a/interface/src/lib.rs b/interface/src/lib.rs index 9784ed83..a7d0e72a 100644 --- a/interface/src/lib.rs +++ b/interface/src/lib.rs @@ -12,6 +12,8 @@ pub mod stake_history; pub mod state; #[cfg(feature = "sysvar")] pub mod sysvar; +#[cfg(test)] +mod test_utils; pub mod tools; pub mod warmup_cooldown_allowance; diff --git a/interface/src/state.rs b/interface/src/state.rs index 3b97f3db..1472f234 100644 --- a/interface/src/state.rs +++ b/interface/src/state.rs @@ -12,7 +12,9 @@ use { instruction::LockupArgs, stake_flags::StakeFlags, stake_history::{StakeHistoryEntry, StakeHistoryGetEntry}, - warmup_cooldown_allowance::{calculate_activation_allowance, calculate_cooldown_allowance}, + warmup_cooldown_allowance::{ + calculate_activation_allowance, calculate_deactivation_allowance, + }, }, solana_clock::{Clock, Epoch, UnixTimestamp}, solana_instruction::error::InstructionError, @@ -26,17 +28,14 @@ pub type StakeActivationStatus = StakeHistoryEntry; // epoch. #[deprecated( since = "2.0.1", - note = "Use PREV_WARMUP_COOLDOWN_RATE_FRACTION instead" + note = "Use ORIGINAL_WARMUP_COOLDOWN_RATE_BPS instead" )] pub const DEFAULT_WARMUP_COOLDOWN_RATE: f64 = 0.25; -#[deprecated( - since = "2.0.1", - note = "Use CURR_WARMUP_COOLDOWN_RATE_FRACTION instead" -)] +#[deprecated(since = "2.0.1", note = "Use TOWER_WARMUP_COOLDOWN_RATE_BPS instead")] pub const NEW_WARMUP_COOLDOWN_RATE: f64 = 0.09; pub const DEFAULT_SLASH_PENALTY: u8 = ((5 * u8::MAX as usize) / 100) as u8; -#[deprecated(since = "2.0.1", note = "Use warmup_cooldown_rate_fraction() instead")] +#[deprecated(since = "2.0.1", note = "Use warmup_cooldown_rate_bps() instead")] pub fn warmup_cooldown_rate(current_epoch: Epoch, new_rate_activation_epoch: Option) -> f64 { if current_epoch < new_rate_activation_epoch.unwrap_or(u64::MAX) { DEFAULT_WARMUP_COOLDOWN_RATE @@ -507,7 +506,199 @@ impl Delegation { .effective } - #[allow(clippy::comparison_chain)] + /// Previous implementation that uses floats under the hood to calculate warmup/cooldown + /// rate-limiting. For the purpose of consumers like Agave that need to feature gate the update. + #[deprecated(since = "2.0.1", note = "Use stake() instead")] + pub fn stake_v1_legacy( + &self, + epoch: Epoch, + history: &T, + new_rate_activation_epoch: Option, + ) -> u64 { + self.stake_activating_and_deactivating_v1_legacy(epoch, history, new_rate_activation_epoch) + .effective + } + + /// Previous implementation that uses floats under the hood to calculate warmup/cooldown + /// rate-limiting. For the purpose of consumers like Agave that need to feature gate the update. + #[deprecated( + since = "2.0.1", + note = "Use stake_activating_and_deactivating() instead" + )] + pub fn stake_activating_and_deactivating_v1_legacy( + &self, + target_epoch: Epoch, + history: &T, + new_rate_activation_epoch: Option, + ) -> StakeActivationStatus { + // first, calculate an effective and activating stake + let (effective_stake, activating_stake) = + self.stake_and_activating_v1_legacy(target_epoch, history, new_rate_activation_epoch); + + // then de-activate some portion if necessary + if target_epoch < self.deactivation_epoch { + // not deactivated + if activating_stake == 0 { + StakeActivationStatus::with_effective(effective_stake) + } else { + StakeActivationStatus::with_effective_and_activating( + effective_stake, + activating_stake, + ) + } + } else if target_epoch == self.deactivation_epoch { + // can only deactivate what's activated + StakeActivationStatus::with_deactivating(effective_stake) + } else if let Some((history, mut prev_epoch, mut prev_cluster_stake)) = history + .get_entry(self.deactivation_epoch) + .map(|cluster_stake_at_deactivation_epoch| { + ( + history, + self.deactivation_epoch, + cluster_stake_at_deactivation_epoch, + ) + }) + { + // target_epoch > self.deactivation_epoch + + // loop from my deactivation epoch until the target epoch + // current effective stake is updated using its previous epoch's cluster stake + let mut current_epoch; + let mut current_effective_stake = effective_stake; + loop { + current_epoch = prev_epoch + 1; + // if there is no deactivating stake at prev epoch, we should have been + // fully undelegated at this moment + if prev_cluster_stake.deactivating == 0 { + break; + } + + // I'm trying to get to zero, how much of the deactivation in stake + // this account is entitled to take + let weight = + current_effective_stake as f64 / prev_cluster_stake.deactivating as f64; + let warmup_cooldown_rate = + warmup_cooldown_rate(current_epoch, new_rate_activation_epoch); + + // portion of newly not-effective cluster stake I'm entitled to at current epoch + let newly_not_effective_cluster_stake = + prev_cluster_stake.effective as f64 * warmup_cooldown_rate; + let newly_not_effective_stake = + ((weight * newly_not_effective_cluster_stake) as u64).max(1); + + current_effective_stake = + current_effective_stake.saturating_sub(newly_not_effective_stake); + if current_effective_stake == 0 { + break; + } + + if current_epoch >= target_epoch { + break; + } + if let Some(current_cluster_stake) = history.get_entry(current_epoch) { + prev_epoch = current_epoch; + prev_cluster_stake = current_cluster_stake; + } else { + break; + } + } + + // deactivating stake should equal to all of currently remaining effective stake + StakeActivationStatus::with_deactivating(current_effective_stake) + } else { + // no history or I've dropped out of history, so assume fully deactivated + StakeActivationStatus::default() + } + } + + // returned tuple is (effective, activating) stake + #[deprecated(since = "2.0.1", note = "Use stake_and_activating() instead")] + fn stake_and_activating_v1_legacy( + &self, + target_epoch: Epoch, + history: &T, + new_rate_activation_epoch: Option, + ) -> (u64, u64) { + let delegated_stake = self.stake; + + if self.is_bootstrap() { + // fully effective immediately + (delegated_stake, 0) + } else if self.activation_epoch == self.deactivation_epoch { + // activated but instantly deactivated; no stake at all regardless of target_epoch + // this must be after the bootstrap check and before all-is-activating check + (0, 0) + } else if target_epoch == self.activation_epoch { + // all is activating + (0, delegated_stake) + } else if target_epoch < self.activation_epoch { + // not yet enabled + (0, 0) + } else if let Some((history, mut prev_epoch, mut prev_cluster_stake)) = history + .get_entry(self.activation_epoch) + .map(|cluster_stake_at_activation_epoch| { + ( + history, + self.activation_epoch, + cluster_stake_at_activation_epoch, + ) + }) + { + // target_epoch > self.activation_epoch + + // loop from my activation epoch until the target epoch summing up my entitlement + // current effective stake is updated using its previous epoch's cluster stake + let mut current_epoch; + let mut current_effective_stake = 0; + loop { + current_epoch = prev_epoch + 1; + // if there is no activating stake at prev epoch, we should have been + // fully effective at this moment + if prev_cluster_stake.activating == 0 { + break; + } + + // how much of the growth in stake this account is + // entitled to take + let remaining_activating_stake = delegated_stake - current_effective_stake; + let weight = + remaining_activating_stake as f64 / prev_cluster_stake.activating as f64; + let warmup_cooldown_rate = + warmup_cooldown_rate(current_epoch, new_rate_activation_epoch); + + // portion of newly effective cluster stake I'm entitled to at current epoch + let newly_effective_cluster_stake = + prev_cluster_stake.effective as f64 * warmup_cooldown_rate; + let newly_effective_stake = + ((weight * newly_effective_cluster_stake) as u64).max(1); + + current_effective_stake += newly_effective_stake; + if current_effective_stake >= delegated_stake { + current_effective_stake = delegated_stake; + break; + } + + if current_epoch >= target_epoch || current_epoch >= self.deactivation_epoch { + break; + } + if let Some(current_cluster_stake) = history.get_entry(current_epoch) { + prev_epoch = current_epoch; + prev_cluster_stake = current_cluster_stake; + } else { + break; + } + } + + ( + current_effective_stake, + delegated_stake - current_effective_stake, + ) + } else { + // no history or I've dropped out of history, so assume fully effective + (delegated_stake, 0) + } + } + pub fn stake_activating_and_deactivating( &self, target_epoch: Epoch, @@ -559,12 +750,15 @@ impl Delegation { } // Compute how much of this account's stake cools down in `current_epoch` - let newly_deactivated_stake = calculate_cooldown_allowance( + let newly_deactivated_stake = calculate_deactivation_allowance( current_epoch, remaining_deactivating_stake, &prev_cluster_stake, new_rate_activation_epoch, ); + + // Substract the newly deactivated stake, clamping the per-epoch decrease to at + // least 1 lamport so cooldown always makes progress remaining_deactivating_stake = remaining_deactivating_stake.saturating_sub(newly_deactivated_stake.max(1)); @@ -653,7 +847,7 @@ impl Delegation { new_rate_activation_epoch, ); - // Add the newly effective stake, ensuring at least one lamport activates to prevent getting stuck. + // Add the newly effective stake, clamping the per-epoch increase to at least 1 lamport so warmup always makes progress activated_stake_amount += newly_effective_stake.max(1); // Stop if we've fully warmed up this account's stake. @@ -717,6 +911,17 @@ impl Stake { .stake(epoch, history, new_rate_activation_epoch) } + #[deprecated(since = "2.0.1", note = "Use stake() instead")] + pub fn stake_v1_legacy( + &self, + epoch: Epoch, + history: &T, + new_rate_activation_epoch: Option, + ) -> u64 { + self.delegation + .stake_v1_legacy(epoch, history, new_rate_activation_epoch) + } + pub fn split( &mut self, remaining_stake_delta: u64, @@ -751,9 +956,7 @@ impl Stake { mod tests { use { super::*, - crate::{ - stake_history::StakeHistory, warmup_cooldown_allowance::warmup_cooldown_rate_fraction, - }, + crate::{stake_history::StakeHistory, warmup_cooldown_allowance::warmup_cooldown_rate_bps}, assert_matches::assert_matches, bincode::serialize, solana_account::{state_traits::StateMut, AccountSharedData, ReadableAccount}, @@ -982,8 +1185,8 @@ mod tests { }; // save this off so stake.config.warmup_rate changes don't break this test - let (rate_num, rate_den) = warmup_cooldown_rate_fraction(0, None); - let increment = ((1_000u128 * rate_num) / rate_den) as u64; + let rate_bps = warmup_cooldown_rate_bps(0, None); + let increment = ((1_000u128 * rate_bps as u128) / 10_000) as u64; let mut stake_history = StakeHistory::default(); // assert that this stake follows step function if there's no history @@ -1352,7 +1555,7 @@ mod tests { let mut effective = base_stake; let other_activation = 100; let mut other_activations = vec![0]; - let (rate_num, rate_den) = warmup_cooldown_rate_fraction(0, None); + let rate_bps = warmup_cooldown_rate_bps(0, None); // Build a stake history where the test staker always consumes all of the available warm // up and cool down stake. However, simulate other stakers beginning to activate during @@ -1375,7 +1578,7 @@ mod tests { }, ); - let effective_rate_limited = ((effective as u128) * rate_num / rate_den) as u64; + let effective_rate_limited = ((effective as u128) * rate_bps as u128 / 10_000) as u64; if epoch < stake.deactivation_epoch { effective += effective_rate_limited.min(activating); other_activations.push(0); @@ -1418,8 +1621,8 @@ mod tests { let epochs = 7; // make bootstrap stake smaller than warmup so warmup/cooldownn // increment is always smaller than 1 - let (rate_num, rate_den) = warmup_cooldown_rate_fraction(0, None); - let bootstrap = ((100u128 * rate_num) / (2u128 * rate_den)) as u64; + let rate_bps = warmup_cooldown_rate_bps(0, None); + let bootstrap = ((100u128 * rate_bps as u128) / (2u128 * 10_000)) as u64; let stake_history = create_stake_history_from_delegations(Some(bootstrap), 0..epochs, &delegations, None); let mut max_stake = 0; @@ -1521,9 +1724,9 @@ mod tests { // (0..(total_effective_stake as usize / (delegations.len() * 5))).for_each(|_| eprint("#")); // eprintln(); - let (rate_num, rate_den) = - warmup_cooldown_rate_fraction(epoch, new_rate_activation_epoch); - let max_delta = ((prev_total_effective_stake as u128) * rate_num / rate_den) as u64; + let rate_bps = warmup_cooldown_rate_bps(epoch, new_rate_activation_epoch); + let max_delta = + ((prev_total_effective_stake as u128) * rate_bps as u128 / 10_000) as u64; assert!(delta <= max_delta.max(1)); prev_total_effective_stake = total_effective_stake; @@ -1738,6 +1941,117 @@ mod tests { check_flag(StakeFlags::empty(), 0); } + #[cfg(test)] + #[allow(deprecated)] + mod delegation_prop_tests { + use { + super::*, + crate::{ + stake_history::{StakeHistory, StakeHistoryEntry}, + test_utils::max_ulp_tolerance, + }, + proptest::prelude::*, + solana_pubkey::Pubkey, + }; + + prop_compose! { + fn arbitrary_delegation()( + // This tests is bounded to the range where `f64` can represent every integer exactly. + // Beyond this, integer math and float math diverge considerably. + // This case is covered in `warmup_cooldown_allowance.rs`. + stake in 0u64..=(1u64 << 53) - 1, + activation_epoch in 0u64..=50, + deactivation_offset in 0u64..=50, + ) -> Delegation { + let deactivation_epoch = activation_epoch.saturating_add(deactivation_offset); + + Delegation { + voter_pubkey: Pubkey::new_unique(), + stake, + activation_epoch, + deactivation_epoch, + ..Delegation::default() + } + } + } + + prop_compose! { + fn arbitrary_stake_history(max_epoch: Epoch)( + entries in prop::collection::vec( + ( + 0u64..=max_epoch, + 0u64..=1_000_000_000_000, // effective + 0u64..=1_000_000_000_000, // activating + 0u64..=1_000_000_000_000, // deactivating + ), + 0..=((max_epoch + 1) as usize), + ) + ) -> StakeHistory { + let mut history = StakeHistory::default(); + for (epoch, effective, activating, deactivating) in entries { + history.add( + epoch, + StakeHistoryEntry { + effective, + activating, + deactivating, + }, + ); + } + history + } + } + + proptest! { + #![proptest_config(ProptestConfig::with_cases(10_000))] + + #[test] + fn delegation_stake_matches_legacy_within_tolerance( + delegation in arbitrary_delegation(), + target_epoch in 0u64..=50, + new_rate_activation_epoch_option in prop::option::of(0u64..=50), + stake_history in arbitrary_stake_history(50), + ) { + let new_stake = delegation.stake( + target_epoch, + &stake_history, + new_rate_activation_epoch_option, + ); + let legacy_stake = delegation.stake_v1_legacy( + target_epoch, + &stake_history, + new_rate_activation_epoch_option, + ); + + // neither path should ever exceed the delegated amount. + prop_assert!(new_stake <= delegation.stake); + prop_assert!(legacy_stake <= delegation.stake); + + // If the delegation has no stake, both must be zero. + if delegation.stake == 0 { + prop_assert_eq!(new_stake, 0); + prop_assert_eq!(legacy_stake, 0); + } else { + // Compare with a ULP-based tolerance to account for float vs integer math. + let diff = new_stake.abs_diff(legacy_stake); + let tolerance = max_ulp_tolerance(new_stake, legacy_stake); + + prop_assert!( + diff <= tolerance, + "stake mismatch: new={}, legacy={}, diff={}, tol={}, delegation={:?}, target_epoch={}, new_rate_activation_epoch_option={:?}", + new_stake, + legacy_stake, + diff, + tolerance, + delegation, + target_epoch, + new_rate_activation_epoch_option, + ); + } + } + } + } + mod deprecated { use { super::*, diff --git a/interface/src/test_utils.rs b/interface/src/test_utils.rs new file mode 100644 index 00000000..bb25d618 --- /dev/null +++ b/interface/src/test_utils.rs @@ -0,0 +1,106 @@ +//! Shared utilities for tests only + +/// Calculates the "Unit in the Last Place" (`ULP`) for a `u64` value, which is +/// the gap between adjacent `f64` values at that magnitude. We need this because +/// the prop test compares the integer vs float implementations. Past `2^53`, `f64` +/// can't represent every integer, so the float result can differ by a few `ULPs` +/// even when both are correct. `f64` facts: +/// - `f64` has 53 bits of precision (52 fraction bits plus an implicit leading 1). +/// - For integers `x < 2^53`, every integer is exactly representable (`ULP = 1`). +/// - At and above powers of two, spacing doubles: +/// `[2^53, 2^54) ULP = 2` +/// `[2^54, 2^55) ULP = 4` +/// `[2^55, 2^56) ULP = 8` +fn ulp_of_u64(magnitude: u64) -> u64 { + // Avoid the special zero case by forcing at least 1 + let magnitude_f64 = magnitude.max(1) as f64; + + // spacing to the next representable f64 + let spacing = magnitude_f64.next_up() - magnitude_f64; + + // Map back to integer units, clamp so we never return 0 + spacing.max(1.0) as u64 +} + +/// Compute an absolute tolerance for comparing the integer result to the +/// legacy `f64`-based implementation. +/// +/// Because the legacy path rounds multiple times before the final floor, +/// the integer result can differ from the float version by a small number +/// of `ULPs` ("Unit in the Last Place") even when both are "correct" for +/// their domain. +pub fn max_ulp_tolerance(candidate: u64, oracle: u64) -> u64 { + // Measure ULP at the larger magnitude of the two results + let mag = candidate.max(oracle); + + // Get the ULP spacing + let ulp = ulp_of_u64(mag); + + // Use a 4x ULP tolerance to account for precision error accumulation in the + // legacy `f64` impl: + // - Three `u64` to `f64` conversions + // - One division and two multiplications are rounded + // - The `as u64` cast truncates the final `f64` result + // + // Proptest confirmed these can accumulate to >3 ULPs, so 4x is a safe margin. + ulp.saturating_mul(4) +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn ulp_standard_calc() { + assert_eq!(ulp_of_u64(0), 1); + assert_eq!(ulp_of_u64(1), 1); + assert_eq!(ulp_of_u64((1u64 << 53) - 1), 1); + assert_eq!(ulp_of_u64(1u64 << 53), 2); + assert_eq!(ulp_of_u64(u64::MAX), 4096); + } + + #[test] + fn tolerance_small_magnitudes_use_single_ulp() { + // For magnitudes < 2^53, ULP = 1, so tolerance = 4 * 1 = 4. + assert_eq!(max_ulp_tolerance(0, 0), 4); + assert_eq!(max_ulp_tolerance(0, 1), 4); + assert_eq!(max_ulp_tolerance((1u64 << 53) - 1, 1), 4); + } + + #[test] + fn tolerance_scales_with_magnitude_powers_of_two() { + // Around powers of two, ULP doubles each time, so tolerance (4 * ULP) doubles. + let below_2_53 = max_ulp_tolerance((1u64 << 53) - 1, 0); // ULP = 1 + let at_2_53 = max_ulp_tolerance(1u64 << 53, 0); // ULP = 2 + let at_2_54 = max_ulp_tolerance(1u64 << 54, 0); // ULP = 4 + let at_2_55 = max_ulp_tolerance(1u64 << 55, 0); // ULP = 8 + + assert_eq!(below_2_53, 4); // 4 * 1 + assert_eq!(at_2_53, 8); // 4 * 2 + assert_eq!(at_2_54, 16); // 4 * 4 + assert_eq!(at_2_55, 32); // 4 * 8 + } + + #[test] + fn tolerance_uses_larger_of_two_results_and_is_symmetric() { + let small = 1u64; + let large = 1u64 << 53; // where ULP jumps from 1 to 2 + + // order of (candidate, oracle) shouldn't matter + let ab = max_ulp_tolerance(small, large); + let ba = max_ulp_tolerance(large, small); + assert_eq!(ab, ba); + + // Using (large, large) should give the same tolerance, since it's based on max() + let big_only = max_ulp_tolerance(large, large); + assert_eq!(ab, big_only); + } + + #[test] + fn tolerance_at_u64_max_matches_expected_ulp() { + // From ulp_standard_calc: ulp_of_u64(u64::MAX) == 4096 + // So tolerance = 4 * 4096 = 16384 + assert_eq!(max_ulp_tolerance(u64::MAX, 0), 4096 * 4); + assert_eq!(max_ulp_tolerance(0, u64::MAX), 4096 * 4); + } +} diff --git a/interface/src/warmup_cooldown_allowance.rs b/interface/src/warmup_cooldown_allowance.rs index e34585d9..5d89265f 100644 --- a/interface/src/warmup_cooldown_allowance.rs +++ b/interface/src/warmup_cooldown_allowance.rs @@ -1,20 +1,15 @@ use {crate::stake_history::StakeHistoryEntry, solana_clock::Epoch}; -type Numerator = u128; -type Denominator = u128; - -pub const PREV_WARMUP_COOLDOWN_RATE_FRACTION: (Numerator, Denominator) = (1, 4); // 25% -pub const CURR_WARMUP_COOLDOWN_RATE_FRACTION: (Numerator, Denominator) = (9, 100); // 9% +pub const BASIS_POINTS_PER_UNIT: u64 = 10_000; +pub const ORIGINAL_WARMUP_COOLDOWN_RATE_BPS: u64 = 2_500; // 25% +pub const TOWER_WARMUP_COOLDOWN_RATE_BPS: u64 = 900; // 9% #[inline] -pub fn warmup_cooldown_rate_fraction( - epoch: Epoch, - new_rate_activation_epoch: Option, -) -> (Numerator, Denominator) { +pub fn warmup_cooldown_rate_bps(epoch: Epoch, new_rate_activation_epoch: Option) -> u64 { if epoch < new_rate_activation_epoch.unwrap_or(u64::MAX) { - PREV_WARMUP_COOLDOWN_RATE_FRACTION + ORIGINAL_WARMUP_COOLDOWN_RATE_BPS } else { - CURR_WARMUP_COOLDOWN_RATE_FRACTION + TOWER_WARMUP_COOLDOWN_RATE_BPS } } @@ -41,7 +36,7 @@ pub fn calculate_activation_allowance( /// /// This function allocates a share of the cluster's per-epoch deactivation allowance /// proportional to the account's share of the previous epoch's total deactivating stake. -pub fn calculate_cooldown_allowance( +pub fn calculate_deactivation_allowance( current_epoch: Epoch, account_deactivating_stake: u64, prev_epoch_cluster_state: &StakeHistoryEntry, @@ -69,8 +64,7 @@ fn rate_limited_stake_change( return 0; } - let (rate_numerator, rate_denominator) = - warmup_cooldown_rate_fraction(epoch, new_rate_activation_epoch); + let rate_bps = warmup_cooldown_rate_bps(epoch, new_rate_activation_epoch); // Calculate this account's proportional share of the network-wide stake change allowance for the epoch. // Formula: `change = (account_portion / cluster_portion) * (cluster_effective * rate)` @@ -78,28 +72,23 @@ fn rate_limited_stake_change( // - `(account_portion / cluster_portion)` is this account's share of the pool. // - `(cluster_effective * rate)` is the total network allowance for change this epoch. // - // Re-arranged formula to use only integer arithmetic w/ rate fraction: - // `change = (account_portion * cluster_effective * rate_numerator) / (cluster_portion * rate_denominator)` + // Re-arranged formula to maximize precision: + // `change = (account_portion * cluster_effective * rate_bps) / (cluster_portion * BASIS_POINTS_PER_UNIT)` // // Using `u128` for the intermediate calculations to prevent overflow. let numerator = (account_portion as u128) .checked_mul(cluster_effective as u128) - .and_then(|x| x.checked_mul(rate_numerator)); - let denominator = (cluster_portion as u128).saturating_mul(rate_denominator); + .and_then(|x| x.checked_mul(rate_bps as u128)); + let denominator = (cluster_portion as u128).saturating_mul(BASIS_POINTS_PER_UNIT as u128); match numerator { Some(n) => { - let delta = n.checked_div(denominator).unwrap_or(u128::MAX); + // Safe unwrap as denominator cannot be zero + let delta = n.checked_div(denominator).unwrap(); // The calculated delta can be larger than `account_portion` if the network's stake change // allowance is greater than the total stake waiting to change. In this case, the account's // entire portion is allowed to change. - if delta >= account_portion as u128 { - account_portion - } else { - // The cast is safe because we have just confirmed that `delta` is less than - // `account_portion`, which is a `u64`. - delta as u64 - } + delta.min(account_portion as u128) as u64 } // Overflowing u128 is not a realistic scenario except in tests. However, in that case // it's reasonable to allow activation/deactivation of the account's entire portion. @@ -111,38 +100,38 @@ fn rate_limited_stake_change( mod test { #[allow(deprecated)] use crate::state::{DEFAULT_WARMUP_COOLDOWN_RATE, NEW_WARMUP_COOLDOWN_RATE}; - use {super::*, proptest::prelude::*}; + use {super::*, crate::test_utils::max_ulp_tolerance, proptest::prelude::*}; // === Rate selector === #[test] - fn rate_fraction_before_activation_epoch_uses_prev_rate() { + fn rate_bps_before_activation_epoch_uses_prev_rate() { let epoch = 9; let new_rate_activation_epoch = Some(10); - let frac = warmup_cooldown_rate_fraction(epoch, new_rate_activation_epoch); - assert_eq!(frac, PREV_WARMUP_COOLDOWN_RATE_FRACTION); + let bps = warmup_cooldown_rate_bps(epoch, new_rate_activation_epoch); + assert_eq!(bps, ORIGINAL_WARMUP_COOLDOWN_RATE_BPS); } #[test] - fn rate_fraction_at_or_after_activation_epoch_uses_curr_rate() { + fn rate_bps_at_or_after_activation_epoch_uses_curr_rate() { let epoch = 10; let new_rate_activation_epoch = Some(10); assert_eq!( - warmup_cooldown_rate_fraction(epoch, new_rate_activation_epoch), - CURR_WARMUP_COOLDOWN_RATE_FRACTION + warmup_cooldown_rate_bps(epoch, new_rate_activation_epoch), + TOWER_WARMUP_COOLDOWN_RATE_BPS ); let epoch2 = 11; assert_eq!( - warmup_cooldown_rate_fraction(epoch2, new_rate_activation_epoch), - CURR_WARMUP_COOLDOWN_RATE_FRACTION + warmup_cooldown_rate_bps(epoch2, new_rate_activation_epoch), + TOWER_WARMUP_COOLDOWN_RATE_BPS ); } #[test] - fn rate_fraction_none_activation_epoch_behaves_like_prev_rate() { + fn rate_bps_none_activation_epoch_behaves_like_prev_rate() { let epoch = 123; - let frac = warmup_cooldown_rate_fraction(epoch, None); - assert_eq!(frac, PREV_WARMUP_COOLDOWN_RATE_FRACTION); + let bps = warmup_cooldown_rate_bps(epoch, None); + assert_eq!(bps, ORIGINAL_WARMUP_COOLDOWN_RATE_BPS); } // === Activation allowance === @@ -241,7 +230,7 @@ mod test { effective: 100, ..Default::default() }; - assert_eq!(calculate_cooldown_allowance(0, 0, &prev, Some(0)), 0); + assert_eq!(calculate_deactivation_allowance(0, 0, &prev, Some(0)), 0); // cluster_portion == 0 let prev = StakeHistoryEntry { @@ -249,7 +238,7 @@ mod test { effective: 100, ..Default::default() }; - assert_eq!(calculate_cooldown_allowance(0, 5, &prev, Some(0)), 0); + assert_eq!(calculate_deactivation_allowance(0, 5, &prev, Some(0)), 0); // cluster_effective == 0 let prev = StakeHistoryEntry { @@ -257,7 +246,7 @@ mod test { effective: 0, ..Default::default() }; - assert_eq!(calculate_cooldown_allowance(0, 5, &prev, Some(0)), 0); + assert_eq!(calculate_deactivation_allowance(0, 5, &prev, Some(0)), 0); } #[test] @@ -272,7 +261,7 @@ mod test { ..Default::default() }; let result = - calculate_cooldown_allowance(current_epoch, 200, &prev, new_rate_activation_epoch); + calculate_deactivation_allowance(current_epoch, 200, &prev, new_rate_activation_epoch); assert_eq!(result, 180); } @@ -286,7 +275,7 @@ mod test { ..Default::default() }; let account_portion = 70; - let result = calculate_cooldown_allowance( + let result = calculate_deactivation_allowance( current_epoch, account_portion, &prev, @@ -309,7 +298,8 @@ mod test { }; let account = 333; let act = calculate_activation_allowance(epoch, account, &prev, new_rate_activation_epoch); - let cool = calculate_cooldown_allowance(epoch, account, &prev, new_rate_activation_epoch); + let cool = + calculate_deactivation_allowance(epoch, account, &prev, new_rate_activation_epoch); assert_eq!(act, cool); } @@ -332,7 +322,7 @@ mod test { assert_eq!(result, 90); } - // === Property tests: compare the integer refactor vs legacy f64 === + // === Property tests: compare the integer refactor vs legacy `f64` === #[allow(deprecated)] fn legacy_warmup_cooldown_rate( @@ -364,6 +354,8 @@ mod test { } proptest! { + #![proptest_config(ProptestConfig::with_cases(10_000))] + #[test] fn rate_limited_change_consistent_with_legacy( account_portion in 0u64..=u64::MAX, @@ -388,30 +380,33 @@ mod test { new_rate_activation_epoch_option, ).min(account_portion); - let (rate_num, _rate_den) = - warmup_cooldown_rate_fraction(current_epoch, new_rate_activation_epoch_option); + let rate_bps = + warmup_cooldown_rate_bps(current_epoch, new_rate_activation_epoch_option); - // See if the u128 product would overflow: account * effective * rate_num + // See if the u128 product would overflow: account * effective * rate_bps let would_overflow = (account_portion as u128) .checked_mul(cluster_effective as u128) - .and_then(|n| n.checked_mul(rate_num)) + .and_then(|n| n.checked_mul(rate_bps as u128)) .is_none(); if account_portion == 0 || cluster_portion == 0 || cluster_effective == 0 { prop_assert_eq!(integer_math_result, 0); prop_assert_eq!(float_math_result, 0); } else if would_overflow { - // In the u128 overflow region, the f64 implementation is guaranteed to be imprecise. + // In the u128 overflow region, the `f64` implementation is guaranteed to be imprecise. // We only assert that our implementation correctly falls back to account_portion. prop_assert_eq!(integer_math_result, account_portion); } else { prop_assert!(integer_math_result <= account_portion); prop_assert!(float_math_result <= account_portion); - // The `f64`-based oracle calculation loses precision when `u64` inputs exceed 2^53. - // The new integer-based implementation is more precise using `u128` arithmetic. - // The small difference seen in failing tests is a measure of this legacy inaccuracy. - let diff = integer_math_result.abs_diff(float_math_result) as u128; - prop_assert!(diff <= 3750, "candidate={}, oracle={}, diff={}", integer_math_result, float_math_result, diff); + + let diff = integer_math_result.abs_diff(float_math_result); + let tolerance = max_ulp_tolerance(integer_math_result, float_math_result); + prop_assert!( + diff <= tolerance, + "Test failed: candidate={}, oracle={}, diff={}, tolerance={}", + integer_math_result, float_math_result, diff, tolerance + ); } } } diff --git a/program/Cargo.toml b/program/Cargo.toml index b1c50386..647e9c27 100644 --- a/program/Cargo.toml +++ b/program/Cargo.toml @@ -20,8 +20,6 @@ solana-program-error = "3.0.0" solana-pubkey = "3.0.0" solana-rent = "3.0.0" solana-stake-interface = { path = "../interface", version = "2", features = ["bincode", "borsh", "sysvar"] } -# TODO: Remove when mollusk updates to use latest interface package -solana-stake-interface-legacy = { package = "solana-stake-interface", version = "2", features = ["bincode", "borsh", "sysvar"] } solana-sysvar = "3.0.0" solana-vote-interface = { version = "4.0.4", features = ["bincode"] } diff --git a/program/src/helpers/merge.rs b/program/src/helpers/merge.rs index eb3906c4..1d47dcb3 100644 --- a/program/src/helpers/merge.rs +++ b/program/src/helpers/merge.rs @@ -238,7 +238,7 @@ mod tests { solana_rent::Rent, solana_stake_interface::{ stake_history::{StakeHistory, StakeHistoryEntry}, - warmup_cooldown_allowance::warmup_cooldown_rate_fraction, + warmup_cooldown_allowance::warmup_cooldown_rate_bps, }, }; @@ -534,9 +534,8 @@ mod tests { // all paritially activated, transient epochs fail loop { clock.epoch += 1; - let (rate_num, rate_den) = - warmup_cooldown_rate_fraction(clock.epoch, new_rate_activation_epoch); - let rate_limited = ((effective as u128) * rate_num / rate_den) as u64; + let rate_bps = warmup_cooldown_rate_bps(clock.epoch, new_rate_activation_epoch); + let rate_limited = ((effective as u128) * rate_bps as u128 / 10_000) as u64; let delta = activating.min(rate_limited); effective += delta; activating -= delta; @@ -611,9 +610,8 @@ mod tests { // all transient, deactivating epochs fail loop { clock.epoch += 1; - let (rate_num, rate_den) = - warmup_cooldown_rate_fraction(clock.epoch, new_rate_activation_epoch); - let rate_limited = ((effective as u128) * rate_num / rate_den) as u64; + let rate_bps = warmup_cooldown_rate_bps(clock.epoch, new_rate_activation_epoch); + let rate_limited = ((effective as u128) * rate_bps as u128 / 10_000) as u64; let delta = deactivating.min(rate_limited); effective -= delta; deactivating -= delta; diff --git a/program/tests/interface.rs b/program/tests/interface.rs index 8c31d457..9fe3a4d1 100644 --- a/program/tests/interface.rs +++ b/program/tests/interface.rs @@ -15,9 +15,9 @@ use { solana_stake_interface::{ instruction::{self, LockupArgs}, stake_flags::StakeFlags, - stake_history::StakeHistory, + stake_history::{StakeHistory, StakeHistoryEntry}, state::{Authorized, Delegation, Lockup, Meta, Stake, StakeAuthorize, StakeStateV2}, - warmup_cooldown_allowance::warmup_cooldown_rate_fraction, + warmup_cooldown_allowance::warmup_cooldown_rate_bps, }, solana_stake_program::{get_minimum_delegation, id}, solana_svm_log_collector::LogCollector, @@ -142,13 +142,13 @@ impl Env { assert_eq!(mollusk.sysvars.clock.epoch, EXECUTION_EPOCH); // backfill stake history - let (num, den) = warmup_cooldown_rate_fraction(0, Some(0)); - let stake_delta_amount = ((PERSISTENT_ACTIVE_STAKE as u128 * num) / den) as u64; + let rate_bps = warmup_cooldown_rate_bps(0, Some(0)); + let stake_delta_amount = + ((PERSISTENT_ACTIVE_STAKE as u128 * rate_bps as u128) / 10_000) as u64; for epoch in 0..EXECUTION_EPOCH { mollusk.sysvars.stake_history.add( epoch, - // TODO: Remove when mollusk updates to latest solana-stake-interface version - solana_stake_interface_legacy::stake_history::StakeHistoryEntry { + StakeHistoryEntry { effective: PERSISTENT_ACTIVE_STAKE, activating: stake_delta_amount, deactivating: stake_delta_amount, diff --git a/program/tests/stake_instruction.rs b/program/tests/stake_instruction.rs index 030771b2..187e69f0 100644 --- a/program/tests/stake_instruction.rs +++ b/program/tests/stake_instruction.rs @@ -28,7 +28,7 @@ use { stake_flags::StakeFlags, stake_history::{StakeHistory, StakeHistoryEntry}, state::{Authorized, Delegation, Lockup, Meta, Stake, StakeAuthorize, StakeStateV2}, - warmup_cooldown_allowance::warmup_cooldown_rate_fraction, + warmup_cooldown_allowance::warmup_cooldown_rate_bps, MINIMUM_DELINQUENT_EPOCHS_FOR_DEACTIVATION, }, solana_stake_program::{get_minimum_delegation, id}, @@ -6428,9 +6428,8 @@ fn test_merge_active_stake() { if clock.epoch == merge_from_activation_epoch { activating += merge_from_amount; } - let (rate_num, rate_den) = - warmup_cooldown_rate_fraction(clock.epoch, new_warmup_cooldown_rate_epoch); - let rate_limited = ((effective as u128) * rate_num / rate_den) as u64; + let rate_bps = warmup_cooldown_rate_bps(clock.epoch, new_warmup_cooldown_rate_epoch); + let rate_limited = ((effective as u128) * rate_bps as u128 / 10_000) as u64; let delta = activating.min(rate_limited); effective += delta; activating -= delta; @@ -6480,9 +6479,8 @@ fn test_merge_active_stake() { // active/deactivating and deactivating/inactive mismatches fail loop { clock.epoch += 1; - let (rate_num, rate_den) = - warmup_cooldown_rate_fraction(clock.epoch, new_warmup_cooldown_rate_epoch); - let rate_limited = ((effective as u128) * rate_num / rate_den) as u64; + let rate_bps = warmup_cooldown_rate_bps(clock.epoch, new_warmup_cooldown_rate_epoch); + let rate_limited = ((effective as u128) * rate_bps as u128 / 10_000) as u64; let delta = deactivating.min(rate_limited); effective -= delta; deactivating -= delta; diff --git a/scripts/solana.dic b/scripts/solana.dic index 7c3905d1..990611f3 100644 --- a/scripts/solana.dic +++ b/scripts/solana.dic @@ -25,3 +25,4 @@ unstake unstaked warmup withdrawer +representable From 2bfac63b88cde96e33f68963c21609972117de4c Mon Sep 17 00:00:00 2001 From: Gabe Rodriguez Date: Tue, 25 Nov 2025 11:17:20 +0100 Subject: [PATCH 3/5] Deprecation strategy update --- interface/src/lib.rs | 4 +- interface/src/state.rs | 98 ++++++++++++---------- interface/src/{test_utils.rs => ulp.rs} | 2 +- interface/src/warmup_cooldown_allowance.rs | 2 +- program/src/helpers/delegate.rs | 2 +- program/src/helpers/merge.rs | 2 +- program/src/processor.rs | 14 ++-- program/tests/program_test.rs | 2 +- program/tests/stake_instruction.rs | 13 +-- 9 files changed, 77 insertions(+), 62 deletions(-) rename interface/src/{test_utils.rs => ulp.rs} (98%) diff --git a/interface/src/lib.rs b/interface/src/lib.rs index a7d0e72a..ef6a2ef9 100644 --- a/interface/src/lib.rs +++ b/interface/src/lib.rs @@ -12,9 +12,9 @@ pub mod stake_history; pub mod state; #[cfg(feature = "sysvar")] pub mod sysvar; -#[cfg(test)] -mod test_utils; pub mod tools; +#[cfg(test)] +mod ulp; pub mod warmup_cooldown_allowance; pub mod program { diff --git a/interface/src/state.rs b/interface/src/state.rs index 1472f234..26d7b871 100644 --- a/interface/src/state.rs +++ b/interface/src/state.rs @@ -496,6 +496,9 @@ impl Delegation { self.activation_epoch == u64::MAX } + /// Previous implementation that uses floats under the hood to calculate warmup/cooldown + /// rate-limiting. New `stake_v2()` uses integers (upstream eBPF-compatible). + #[deprecated(since = "2.0.1", note = "Use stake_v2() instead")] pub fn stake( &self, epoch: Epoch, @@ -507,25 +510,12 @@ impl Delegation { } /// Previous implementation that uses floats under the hood to calculate warmup/cooldown - /// rate-limiting. For the purpose of consumers like Agave that need to feature gate the update. - #[deprecated(since = "2.0.1", note = "Use stake() instead")] - pub fn stake_v1_legacy( - &self, - epoch: Epoch, - history: &T, - new_rate_activation_epoch: Option, - ) -> u64 { - self.stake_activating_and_deactivating_v1_legacy(epoch, history, new_rate_activation_epoch) - .effective - } - - /// Previous implementation that uses floats under the hood to calculate warmup/cooldown - /// rate-limiting. For the purpose of consumers like Agave that need to feature gate the update. + /// rate-limiting. New `stake_activating_and_deactivating_v2()` uses integers (upstream eBPF-compatible). #[deprecated( since = "2.0.1", - note = "Use stake_activating_and_deactivating() instead" + note = "Use stake_activating_and_deactivating_v2() instead" )] - pub fn stake_activating_and_deactivating_v1_legacy( + pub fn stake_activating_and_deactivating( &self, target_epoch: Epoch, history: &T, @@ -533,7 +523,7 @@ impl Delegation { ) -> StakeActivationStatus { // first, calculate an effective and activating stake let (effective_stake, activating_stake) = - self.stake_and_activating_v1_legacy(target_epoch, history, new_rate_activation_epoch); + self.stake_and_activating(target_epoch, history, new_rate_activation_epoch); // then de-activate some portion if necessary if target_epoch < self.deactivation_epoch { @@ -612,8 +602,8 @@ impl Delegation { } // returned tuple is (effective, activating) stake - #[deprecated(since = "2.0.1", note = "Use stake_and_activating() instead")] - fn stake_and_activating_v1_legacy( + #[deprecated(since = "2.0.1", note = "Use stake_and_activating_v2() instead")] + fn stake_and_activating( &self, target_epoch: Epoch, history: &T, @@ -699,7 +689,17 @@ impl Delegation { } } - pub fn stake_activating_and_deactivating( + pub fn stake_v2( + &self, + epoch: Epoch, + history: &T, + new_rate_activation_epoch: Option, + ) -> u64 { + self.stake_activating_and_deactivating_v2(epoch, history, new_rate_activation_epoch) + .effective + } + + pub fn stake_activating_and_deactivating_v2( &self, target_epoch: Epoch, history: &T, @@ -707,7 +707,7 @@ impl Delegation { ) -> StakeActivationStatus { // first, calculate an effective and activating stake let (effective_stake, activating_stake) = - self.stake_and_activating(target_epoch, history, new_rate_activation_epoch); + self.stake_and_activating_v2(target_epoch, history, new_rate_activation_epoch); // then de-activate some portion if necessary if target_epoch < self.deactivation_epoch { @@ -791,7 +791,7 @@ impl Delegation { } // returned tuple is (effective, activating) stake - fn stake_and_activating( + fn stake_and_activating_v2( &self, target_epoch: Epoch, history: &T, @@ -901,6 +901,7 @@ pub struct Stake { } impl Stake { + #[deprecated(since = "2.0.1", note = "Use stake_v2() instead")] pub fn stake( &self, epoch: Epoch, @@ -911,15 +912,14 @@ impl Stake { .stake(epoch, history, new_rate_activation_epoch) } - #[deprecated(since = "2.0.1", note = "Use stake() instead")] - pub fn stake_v1_legacy( + pub fn stake_v2( &self, epoch: Epoch, history: &T, new_rate_activation_epoch: Option, ) -> u64 { self.delegation - .stake_v1_legacy(epoch, history, new_rate_activation_epoch) + .stake_v2(epoch, history, new_rate_activation_epoch) } pub fn split( @@ -983,7 +983,11 @@ mod tests { I: Iterator, { stakes.fold(StakeHistoryEntry::default(), |sum, stake| { - sum + stake.stake_activating_and_deactivating(epoch, history, new_rate_activation_epoch) + sum + stake.stake_activating_and_deactivating_v2( + epoch, + history, + new_rate_activation_epoch, + ) }) } @@ -1191,23 +1195,31 @@ mod tests { let mut stake_history = StakeHistory::default(); // assert that this stake follows step function if there's no history assert_eq!( - stake.stake_activating_and_deactivating(stake.activation_epoch, &stake_history, None), + stake.stake_activating_and_deactivating_v2( + stake.activation_epoch, + &stake_history, + None + ), StakeActivationStatus::with_effective_and_activating(0, stake.stake), ); for epoch in stake.activation_epoch + 1..stake.deactivation_epoch { assert_eq!( - stake.stake_activating_and_deactivating(epoch, &stake_history, None), + stake.stake_activating_and_deactivating_v2(epoch, &stake_history, None), StakeActivationStatus::with_effective(stake.stake), ); } // assert that this stake is full deactivating assert_eq!( - stake.stake_activating_and_deactivating(stake.deactivation_epoch, &stake_history, None), + stake.stake_activating_and_deactivating_v2( + stake.deactivation_epoch, + &stake_history, + None + ), StakeActivationStatus::with_deactivating(stake.stake), ); // assert that this stake is fully deactivated if there's no history assert_eq!( - stake.stake_activating_and_deactivating( + stake.stake_activating_and_deactivating_v2( stake.deactivation_epoch + 1, &stake_history, None @@ -1224,7 +1236,7 @@ mod tests { ); // assert that this stake is broken, because above setup is broken assert_eq!( - stake.stake_activating_and_deactivating(1, &stake_history, None), + stake.stake_activating_and_deactivating_v2(1, &stake_history, None), StakeActivationStatus::with_effective_and_activating(0, stake.stake), ); @@ -1239,7 +1251,7 @@ mod tests { ); // assert that this stake is broken, because above setup is broken assert_eq!( - stake.stake_activating_and_deactivating(2, &stake_history, None), + stake.stake_activating_and_deactivating_v2(2, &stake_history, None), StakeActivationStatus::with_effective_and_activating( increment, stake.stake - increment @@ -1258,7 +1270,7 @@ mod tests { ); // assert that this stake is broken, because above setup is broken assert_eq!( - stake.stake_activating_and_deactivating( + stake.stake_activating_and_deactivating_v2( stake.deactivation_epoch + 1, &stake_history, None, @@ -1277,7 +1289,7 @@ mod tests { ); // assert that this stake is broken, because above setup is broken assert_eq!( - stake.stake_activating_and_deactivating( + stake.stake_activating_and_deactivating_v2( stake.deactivation_epoch + 2, &stake_history, None, @@ -1347,7 +1359,7 @@ mod tests { assert_eq!( expected_stakes, (0..expected_stakes.len()) - .map(|epoch| stake.stake_activating_and_deactivating( + .map(|epoch| stake.stake_activating_and_deactivating_v2( epoch as u64, &stake_history, None, @@ -1478,7 +1490,7 @@ mod tests { let calculate_each_staking_status = |stake: &Delegation, epoch_count: usize| -> Vec<_> { (0..epoch_count) .map(|epoch| { - stake.stake_activating_and_deactivating(epoch as u64, &stake_history, None) + stake.stake_activating_and_deactivating_v2(epoch as u64, &stake_history, None) }) .collect::>() }; @@ -1599,7 +1611,7 @@ mod tests { (0, history.deactivating) }; assert_eq!( - stake.stake_activating_and_deactivating(epoch, &stake_history, None), + stake.stake_activating_and_deactivating_v2(epoch, &stake_history, None), StakeActivationStatus { effective: expected_stake, activating: expected_activating, @@ -1631,7 +1643,7 @@ mod tests { for epoch in 0..epochs { let stake = delegations .iter() - .map(|delegation| delegation.stake(epoch, &stake_history, None)) + .map(|delegation| delegation.stake_v2(epoch, &stake_history, None)) .sum::(); max_stake = max_stake.max(stake); min_stake = min_stake.min(stake); @@ -1700,7 +1712,7 @@ mod tests { let mut prev_total_effective_stake = delegations .iter() - .map(|delegation| delegation.stake(0, &stake_history, new_rate_activation_epoch)) + .map(|delegation| delegation.stake_v2(0, &stake_history, new_rate_activation_epoch)) .sum::(); // uncomment and add ! for fun with graphing @@ -1709,7 +1721,7 @@ mod tests { let total_effective_stake = delegations .iter() .map(|delegation| { - delegation.stake(epoch, &stake_history, new_rate_activation_epoch) + delegation.stake_v2(epoch, &stake_history, new_rate_activation_epoch) }) .sum::(); @@ -1948,7 +1960,7 @@ mod tests { super::*, crate::{ stake_history::{StakeHistory, StakeHistoryEntry}, - test_utils::max_ulp_tolerance, + ulp::max_ulp_tolerance, }, proptest::prelude::*, solana_pubkey::Pubkey, @@ -2012,12 +2024,12 @@ mod tests { new_rate_activation_epoch_option in prop::option::of(0u64..=50), stake_history in arbitrary_stake_history(50), ) { - let new_stake = delegation.stake( + let new_stake = delegation.stake_v2( target_epoch, &stake_history, new_rate_activation_epoch_option, ); - let legacy_stake = delegation.stake_v1_legacy( + let legacy_stake = delegation.stake( target_epoch, &stake_history, new_rate_activation_epoch_option, diff --git a/interface/src/test_utils.rs b/interface/src/ulp.rs similarity index 98% rename from interface/src/test_utils.rs rename to interface/src/ulp.rs index bb25d618..04f96d08 100644 --- a/interface/src/test_utils.rs +++ b/interface/src/ulp.rs @@ -1,4 +1,4 @@ -//! Shared utilities for tests only +//! Math utilities for calculating float/int differences /// Calculates the "Unit in the Last Place" (`ULP`) for a `u64` value, which is /// the gap between adjacent `f64` values at that magnitude. We need this because diff --git a/interface/src/warmup_cooldown_allowance.rs b/interface/src/warmup_cooldown_allowance.rs index 5d89265f..00a457e2 100644 --- a/interface/src/warmup_cooldown_allowance.rs +++ b/interface/src/warmup_cooldown_allowance.rs @@ -100,7 +100,7 @@ fn rate_limited_stake_change( mod test { #[allow(deprecated)] use crate::state::{DEFAULT_WARMUP_COOLDOWN_RATE, NEW_WARMUP_COOLDOWN_RATE}; - use {super::*, crate::test_utils::max_ulp_tolerance, proptest::prelude::*}; + use {super::*, crate::ulp::max_ulp_tolerance, proptest::prelude::*}; // === Rate selector === diff --git a/program/src/helpers/delegate.rs b/program/src/helpers/delegate.rs index 2d30671c..96269caf 100644 --- a/program/src/helpers/delegate.rs +++ b/program/src/helpers/delegate.rs @@ -38,7 +38,7 @@ pub(crate) fn redelegate_stake( stake_history: &StakeHistorySysvar, ) -> Result<(), ProgramError> { // If stake is currently active: - if stake.stake( + if stake.stake_v2( epoch, stake_history, PERPETUAL_NEW_WARMUP_COOLDOWN_RATE_EPOCH, diff --git a/program/src/helpers/merge.rs b/program/src/helpers/merge.rs index 1d47dcb3..e6ebc796 100644 --- a/program/src/helpers/merge.rs +++ b/program/src/helpers/merge.rs @@ -43,7 +43,7 @@ impl MergeKind { StakeStateV2::Stake(meta, stake, stake_flags) => { // stake must not be in a transient state. Transient here meaning // activating or deactivating with non-zero effective stake. - let status = stake.delegation.stake_activating_and_deactivating( + let status = stake.delegation.stake_activating_and_deactivating_v2( clock.epoch, stake_history, PERPETUAL_NEW_WARMUP_COOLDOWN_RATE_EPOCH, diff --git a/program/src/processor.rs b/program/src/processor.rs index c046d86d..afed83cb 100644 --- a/program/src/processor.rs +++ b/program/src/processor.rs @@ -469,11 +469,13 @@ impl Processor { let minimum_delegation = crate::get_minimum_delegation(); - let status = source_stake.delegation.stake_activating_and_deactivating( - clock.epoch, - stake_history, - PERPETUAL_NEW_WARMUP_COOLDOWN_RATE_EPOCH, - ); + let status = source_stake + .delegation + .stake_activating_and_deactivating_v2( + clock.epoch, + stake_history, + PERPETUAL_NEW_WARMUP_COOLDOWN_RATE_EPOCH, + ); let is_active = status.effective > 0; @@ -631,7 +633,7 @@ impl Processor { .map_err(to_program_error)?; // if we have a deactivation epoch and we're in cooldown let staked = if clock.epoch >= stake.delegation.deactivation_epoch { - stake.delegation.stake( + stake.delegation.stake_v2( clock.epoch, stake_history, PERPETUAL_NEW_WARMUP_COOLDOWN_RATE_EPOCH, diff --git a/program/tests/program_test.rs b/program/tests/program_test.rs index e1988c7a..db3b0593 100644 --- a/program/tests/program_test.rs +++ b/program/tests/program_test.rs @@ -198,7 +198,7 @@ pub async fn get_effective_stake(banks_client: &mut BanksClient, pubkey: &Pubkey StakeStateV2::Stake(_, stake, _) => { stake .delegation - .stake_activating_and_deactivating(clock.epoch, &stake_history, Some(0)) + .stake_activating_and_deactivating_v2(clock.epoch, &stake_history, Some(0)) .effective } _ => 0, diff --git a/program/tests/stake_instruction.rs b/program/tests/stake_instruction.rs index 187e69f0..d255aaee 100644 --- a/program/tests/stake_instruction.rs +++ b/program/tests/stake_instruction.rs @@ -228,7 +228,7 @@ fn get_active_stake_for_tests( let mut active_stake = 0; for account in stake_accounts { if let Ok(StakeStateV2::Stake(_meta, stake, _stake_flags)) = account.state() { - let stake_status = stake.delegation.stake_activating_and_deactivating( + let stake_status = stake.delegation.stake_activating_and_deactivating_v2( clock.epoch, stake_history, None, @@ -253,7 +253,7 @@ where I: Iterator, { stakes.fold(StakeHistoryEntry::default(), |sum, stake| { - sum + stake.stake_activating_and_deactivating(epoch, history, new_rate_activation_epoch) + sum + stake.stake_activating_and_deactivating_v2(epoch, history, new_rate_activation_epoch) }) } @@ -6446,9 +6446,10 @@ fn test_merge_active_stake() { StakeHistory::id(), create_account_shared_data_for_test(&stake_history), ); - if stake_amount == stake.stake(clock.epoch, &stake_history, new_warmup_cooldown_rate_epoch) + if stake_amount + == stake.stake_v2(clock.epoch, &stake_history, new_warmup_cooldown_rate_epoch) && merge_from_amount - == merge_from_stake.stake( + == merge_from_stake.stake_v2( clock.epoch, &stake_history, new_warmup_cooldown_rate_epoch, @@ -6529,8 +6530,8 @@ fn test_merge_active_stake() { StakeHistory::id(), create_account_shared_data_for_test(&stake_history), ); - if 0 == stake.stake(clock.epoch, &stake_history, new_warmup_cooldown_rate_epoch) - && 0 == merge_from_stake.stake( + if 0 == stake.stake_v2(clock.epoch, &stake_history, new_warmup_cooldown_rate_epoch) + && 0 == merge_from_stake.stake_v2( clock.epoch, &stake_history, new_warmup_cooldown_rate_epoch, From fd2a4a664f7748846b494603a828c9bd31b91544 Mon Sep 17 00:00:00 2001 From: Gabe Rodriguez Date: Tue, 25 Nov 2025 13:17:01 +0100 Subject: [PATCH 4/5] Saturate numerators for safety --- interface/src/warmup_cooldown_allowance.rs | 74 ++++++++++++++-------- 1 file changed, 46 insertions(+), 28 deletions(-) diff --git a/interface/src/warmup_cooldown_allowance.rs b/interface/src/warmup_cooldown_allowance.rs index 00a457e2..43544b9b 100644 --- a/interface/src/warmup_cooldown_allowance.rs +++ b/interface/src/warmup_cooldown_allowance.rs @@ -76,24 +76,20 @@ fn rate_limited_stake_change( // `change = (account_portion * cluster_effective * rate_bps) / (cluster_portion * BASIS_POINTS_PER_UNIT)` // // Using `u128` for the intermediate calculations to prevent overflow. + // If the multiplication would overflow, we saturate to u128::MAX. This ensures + // that even in extreme edge cases, the rate-limiting invariant is maintained + // (fail-safe) rather than bypassing rate limits entirely (fail-open). let numerator = (account_portion as u128) - .checked_mul(cluster_effective as u128) - .and_then(|x| x.checked_mul(rate_bps as u128)); + .saturating_mul(cluster_effective as u128) + .saturating_mul(rate_bps as u128); let denominator = (cluster_portion as u128).saturating_mul(BASIS_POINTS_PER_UNIT as u128); - match numerator { - Some(n) => { - // Safe unwrap as denominator cannot be zero - let delta = n.checked_div(denominator).unwrap(); - // The calculated delta can be larger than `account_portion` if the network's stake change - // allowance is greater than the total stake waiting to change. In this case, the account's - // entire portion is allowed to change. - delta.min(account_portion as u128) as u64 - } - // Overflowing u128 is not a realistic scenario except in tests. However, in that case - // it's reasonable to allow activation/deactivation of the account's entire portion. - None => account_portion, - } + // Safe unwrap as denominator cannot be zero due to early return guards above + let delta = numerator.checked_div(denominator).unwrap(); + // The calculated delta can be larger than `account_portion` if the network's stake change + // allowance is greater than the total stake waiting to change. In this case, the account's + // entire portion is allowed to change. + delta.min(account_portion as u128) as u64 } #[cfg(test)] @@ -200,24 +196,46 @@ mod test { } #[test] - fn activation_overflow_path_returns_account_portion() { - // Force the u128 multiply to overflow: (u64::MAX * u64::MAX * 9) overflows u128. - // When that happens, the helper returns the full account_portion. - let current_epoch = 0; - let new_rate_activation_epoch = Some(0); // use "current" 9/100 to maximize multiplier + fn activation_overflow_scenario_still_rate_limits() { + // Extreme scenario where a single account holding nearly the total supply + // and tries to activate everything at once. Asserting rate limiting is maintained. + let supply_lamports: u64 = 400_000_000_000_000_000; // 400M SOL + let account_portion = supply_lamports; let prev = StakeHistoryEntry { - activating: 1, // non-zero cluster_portion - effective: u64::MAX, // huge cluster_effective + activating: supply_lamports, + effective: supply_lamports, ..Default::default() }; - let account_portion = u64::MAX; + let result = calculate_activation_allowance( - current_epoch, + 100, account_portion, &prev, - new_rate_activation_epoch, + None, // forces 25% rate ); - assert_eq!(result, account_portion); + + // Verify overflow actually occurs in this scenario + let rate_bps = ORIGINAL_WARMUP_COOLDOWN_RATE_BPS; + let would_overflow = (account_portion as u128) + .checked_mul(supply_lamports as u128) + .and_then(|n| n.checked_mul(rate_bps as u128)) + .is_none(); + assert!(would_overflow); + + // The ideal result (with infinite precision) is 25% of the stake. + // 400M * 0.25 = 100M + let ideal_allowance = supply_lamports / 4; + + // With saturation fix: + // Numerator saturates to u128::MAX (≈ 3.4e38) + // Denominator = 4e17 * 10,000 = 4e21 + // Result = 3.4e38 / 4e21 ≈ 8.5e16 (85M SOL) + // 85M is ~21.25% of the stake (fail-safe) + // If we allowed unlocking the full account portion it would have been 100% (fail-open) + assert!(result < account_portion); + + // Assert result is in a reasonable throttling range + assert!(result > 0 && result <= ideal_allowance); } // === Cooldown allowance === @@ -394,8 +412,8 @@ mod test { prop_assert_eq!(float_math_result, 0); } else if would_overflow { // In the u128 overflow region, the `f64` implementation is guaranteed to be imprecise. - // We only assert that our implementation correctly falls back to account_portion. - prop_assert_eq!(integer_math_result, account_portion); + // We only assert that the result does not exceed the account's balance + prop_assert!(integer_math_result <= account_portion); } else { prop_assert!(integer_math_result <= account_portion); prop_assert!(float_math_result <= account_portion); From 9719141dc60e0f03f5865a5091136d3d3dcbebfa Mon Sep 17 00:00:00 2001 From: Gabe Rodriguez Date: Mon, 1 Dec 2025 10:31:32 +0100 Subject: [PATCH 5/5] Update assertions --- interface/src/warmup_cooldown_allowance.rs | 29 ++++++++++++++++------ 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/interface/src/warmup_cooldown_allowance.rs b/interface/src/warmup_cooldown_allowance.rs index 43544b9b..95aa7afd 100644 --- a/interface/src/warmup_cooldown_allowance.rs +++ b/interface/src/warmup_cooldown_allowance.rs @@ -96,7 +96,7 @@ fn rate_limited_stake_change( mod test { #[allow(deprecated)] use crate::state::{DEFAULT_WARMUP_COOLDOWN_RATE, NEW_WARMUP_COOLDOWN_RATE}; - use {super::*, crate::ulp::max_ulp_tolerance, proptest::prelude::*}; + use {super::*, crate::ulp::max_ulp_tolerance, proptest::prelude::*, std::ops::Div}; // === Rate selector === @@ -207,7 +207,7 @@ mod test { ..Default::default() }; - let result = calculate_activation_allowance( + let actual_result = calculate_activation_allowance( 100, account_portion, &prev, @@ -228,14 +228,25 @@ mod test { // With saturation fix: // Numerator saturates to u128::MAX (≈ 3.4e38) + let numerator = (account_portion as u128) + .saturating_mul(supply_lamports as u128) + .saturating_mul(rate_bps as u128); + assert_eq!(numerator, u128::MAX); + // Denominator = 4e17 * 10,000 = 4e21 - // Result = 3.4e38 / 4e21 ≈ 8.5e16 (85M SOL) + let denominator = (supply_lamports as u128).saturating_mul(BASIS_POINTS_PER_UNIT as u128); + assert_eq!(denominator, 4_000_000_000_000_000_000_000); + + // Result = u128::MAX / 4e21 ≈ 8.5e16 (~85M SOL) // 85M is ~21.25% of the stake (fail-safe) // If we allowed unlocking the full account portion it would have been 100% (fail-open) - assert!(result < account_portion); + let expected_result = numerator.div(denominator).min(account_portion as u128) as u64; + assert_eq!(expected_result, 85_070_591_730_234_615); - // Assert result is in a reasonable throttling range - assert!(result > 0 && result <= ideal_allowance); + // Assert actual result is expected + assert_eq!(actual_result, expected_result); + assert!(actual_result < account_portion); + assert!(actual_result <= ideal_allowance); } // === Cooldown allowance === @@ -411,8 +422,10 @@ mod test { prop_assert_eq!(integer_math_result, 0); prop_assert_eq!(float_math_result, 0); } else if would_overflow { - // In the u128 overflow region, the `f64` implementation is guaranteed to be imprecise. - // We only assert that the result does not exceed the account's balance + // In the overflow path, the numerator is `u128::MAX` and is divided then clamped to + // `account_portion`. This math often results in less than `account_portion`, but + // never should exceed it. It may be equal in the case the denominator is small and + // post-division result gets clamped. prop_assert!(integer_math_result <= account_portion); } else { prop_assert!(integer_math_result <= account_portion);