diff --git a/Cargo.toml b/Cargo.toml index 60032ec..a01e5f6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,3 +41,7 @@ path = "tests/crypto/dkg_serialization_roundtrip_test.rs" [[test]] name = "proof_of_connectivity_epoch_nonce_test" path = "tests/attestation/proof_of_connectivity_epoch_nonce_test.rs" + +[[test]] +name = "balance_underflow_test" +path = "tests/validator/balance_underflow_test.rs" diff --git a/src/slashing/mod.rs b/src/slashing/mod.rs index c4e60d5..37724c9 100644 --- a/src/slashing/mod.rs +++ b/src/slashing/mod.rs @@ -1,4 +1,5 @@ pub mod cross_chain_relay; +pub mod evidence_verifier; pub mod mempool; +pub mod penalty_calculator; pub mod types; -pub mod evidence_verifier; diff --git a/src/slashing/penalty_calculator.rs b/src/slashing/penalty_calculator.rs new file mode 100644 index 0000000..08d2c11 --- /dev/null +++ b/src/slashing/penalty_calculator.rs @@ -0,0 +1,88 @@ +//! Slashing and inactivity-leak penalty calculators. +//! +//! Implements the penalty formulae referenced by issue #241: +//! +//! * **Slashing penalty** – whistleblower reward (`1/32` of effective balance) +//! plus correlation penalty (`1/32` of effective balance), giving `1/16` of +//! the validator's effective balance in total. +//! * **Inactivity leak penalty** – proportional to +//! `epochs_since_finality² × effective_balance / INACTIVITY_PENALTY_QUOTIENT`. +//! +//! All arithmetic is performed in `u64` (Gwei). Overflow-safe helpers use +//! `saturating_*` and `checked_*` where appropriate; the penalty values +//! themselves are returned as `u64` and are never negative. + +use crate::validator::balance_tracker::MAX_EFFECTIVE_BALANCE; + +// --------------------------------------------------------------------------- +// Constants +// --------------------------------------------------------------------------- + +/// Divisor for each of the two components of the slashing penalty +/// (whistleblower + correlation). Each component is `effective_balance / 32`, +/// so the total slashing penalty is `effective_balance / 16`. +pub const SLASHING_PENALTY_QUOTIENT: u64 = 32; + +/// Divisor used in the inactivity-leak penalty formula. +/// `penalty = epochs_since_finality² × effective_balance / INACTIVITY_PENALTY_QUOTIENT`. +pub const INACTIVITY_PENALTY_QUOTIENT: u64 = 1 << 24; // 16_777_216 + +// --------------------------------------------------------------------------- +// Penalty computations +// --------------------------------------------------------------------------- + +/// Compute the total slashing penalty for a validator with the given +/// `effective_balance` (Gwei). +/// +/// Formula: +/// ```text +/// whistleblower = effective_balance / SLASHING_PENALTY_QUOTIENT (1/32) +/// correlation = effective_balance / SLASHING_PENALTY_QUOTIENT (1/32) +/// total = whistleblower + correlation (1/16) +/// ``` +/// +/// Integer division floors the result. For a 32 ETH validator the penalty is +/// exactly 2 ETH (2_000_000_000 Gwei). For validators with a balance already +/// below 32 ETH the penalty scales proportionally. +/// +/// # Panics +/// +/// Does not panic; all arithmetic is safe for any `u64` input. +pub fn compute_slashing_penalty(effective_balance: u64) -> u64 { + let whistleblower = effective_balance / SLASHING_PENALTY_QUOTIENT; + let correlation = effective_balance / SLASHING_PENALTY_QUOTIENT; + whistleblower.saturating_add(correlation) +} + +/// Compute the inactivity-leak penalty for a validator. +/// +/// Formula: +/// ```text +/// penalty = (epochs_since_finality² × effective_balance) / INACTIVITY_PENALTY_QUOTIENT +/// ``` +/// +/// `u128` intermediate arithmetic prevents the multiplication from wrapping +/// for realistic `epochs_since_finality` values. The result is then clamped +/// back to `u64` (if it somehow exceeds `u64::MAX` it saturates, but this +/// cannot occur for valid effective balances capped at [`MAX_EFFECTIVE_BALANCE`] +/// within any realistic chain lifetime). +/// +/// # Panics +/// +/// Does not panic. +pub fn compute_inactivity_penalty(effective_balance: u64, epochs_since_finality: u64) -> u64 { + // Use u128 to avoid intermediate overflow when squaring large epoch counts. + let epochs_sq = (epochs_since_finality as u128).saturating_mul(epochs_since_finality as u128); + let numerator = epochs_sq.saturating_mul(effective_balance as u128); + let penalty_wide = numerator / (INACTIVITY_PENALTY_QUOTIENT as u128); + // Clamp to u64; for any realistic effective_balance <= MAX_EFFECTIVE_BALANCE + // and chain lifetime this will not saturate. + penalty_wide.min(u64::MAX as u128) as u64 +} + +/// Clamp `balance` so it never exceeds [`MAX_EFFECTIVE_BALANCE`]. +/// Used after reward application to enforce the protocol invariant. +#[inline] +pub fn cap_effective_balance(balance: u64) -> u64 { + balance.min(MAX_EFFECTIVE_BALANCE) +} diff --git a/src/validator/balance_tracker.rs b/src/validator/balance_tracker.rs new file mode 100644 index 0000000..a6938a8 --- /dev/null +++ b/src/validator/balance_tracker.rs @@ -0,0 +1,208 @@ +//! Validator balance tracker with underflow-safe penalty application. +//! +//! Fixes issue #241: `apply_penalty()` previously used `saturating_sub`, which +//! silently clamped a balance to zero when the penalty exceeded the balance. +//! This allowed a validator to remain active for one more epoch (bypassing the +//! ejection threshold check) and then re-activate with a zero balance. +//! +//! The fix replaces `saturating_sub` with `checked_sub`. When the penalty +//! exceeds the current balance the function returns +//! [`BalanceError::InsufficientBalance`] instead of clamping, and the caller +//! is responsible for recording a debt and forcing immediate ejection. + +extern crate alloc; +use alloc::collections::BTreeMap; + +use crate::validator::exit_queue::ValidatorIndex; + +// --------------------------------------------------------------------------- +// Constants +// --------------------------------------------------------------------------- + +/// 1 ETH expressed in Gwei (u64). Used as the base unit for balances. +pub const GWEI_PER_ETH: u64 = 1_000_000_000; + +/// Maximum effective balance a validator can hold (32 ETH in Gwei). +pub const MAX_EFFECTIVE_BALANCE: u64 = 32 * GWEI_PER_ETH; + +/// Ejection threshold: validators whose effective balance falls strictly below +/// this value are ejected at the epoch boundary (16 ETH in Gwei). +pub const EJECTION_THRESHOLD: u64 = 16 * GWEI_PER_ETH; + +// --------------------------------------------------------------------------- +// Errors +// --------------------------------------------------------------------------- + +/// Errors that can be returned by balance-tracker operations. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum BalanceError { + /// The requested penalty exceeds the validator's current balance. + /// The validator must be force-ejected and a debt recorded. + InsufficientBalance { + /// Balance at the time the penalty was applied. + balance: u64, + /// Penalty that was attempted. + penalty: u64, + }, + /// No record exists for the given `ValidatorIndex`. + ValidatorNotFound, +} + +// --------------------------------------------------------------------------- +// BalanceTracker +// --------------------------------------------------------------------------- + +/// Tracks effective balances and outstanding debts for every known validator. +/// +/// Balances are stored in Gwei (`u64`). Debts are recorded separately so that +/// a validator whose balance has been driven to zero by a penalty that exceeds +/// the balance is still flagged for forced ejection even though its stored +/// balance reads zero. +#[derive(Clone, Debug, Default)] +pub struct BalanceTracker { + /// Effective balance in Gwei for each validator index. + balances: BTreeMap, + /// Unpaid debt in Gwei for validators whose penalty exceeded their balance. + /// The presence of any debt, however small, marks the validator for forced + /// ejection regardless of its stored effective balance. + debts: BTreeMap, +} + +impl BalanceTracker { + /// Create an empty tracker. + pub fn new() -> Self { + Self { + balances: BTreeMap::new(), + debts: BTreeMap::new(), + } + } + + // ----------------------------------------------------------------------- + // Registration + // ----------------------------------------------------------------------- + + /// Register `validator_index` with the given starting `balance` (Gwei). + /// Silently overwrites any previous entry (useful for test setup). + pub fn register_validator(&mut self, validator_index: ValidatorIndex, balance: u64) { + self.balances.insert(validator_index, balance); + // Clear any stale debt from a previous registration. + self.debts.remove(&validator_index); + } + + // ----------------------------------------------------------------------- + // Accessors + // ----------------------------------------------------------------------- + + /// Return the current effective balance in Gwei, or `None` if the validator + /// is not registered. + pub fn effective_balance(&self, validator_index: ValidatorIndex) -> Option { + self.balances.get(&validator_index).copied() + } + + /// Return the outstanding debt in Gwei, or `None` if the validator is not + /// registered / has no debt. + pub fn outstanding_debt(&self, validator_index: ValidatorIndex) -> Option { + self.debts.get(&validator_index).copied() + } + + /// Return `true` if the validator has any outstanding debt. + pub fn has_debt(&self, validator_index: ValidatorIndex) -> bool { + self.debts.get(&validator_index).map_or(false, |&d| d > 0) + } + + // ----------------------------------------------------------------------- + // Mutations + // ----------------------------------------------------------------------- + + /// Apply `penalty` (Gwei) to `validator_index`. + /// + /// # Behaviour + /// + /// * If `penalty <= balance` the balance is reduced and `Ok(new_balance)` is + /// returned. + /// * If `penalty > balance` **no balance change is made**, the excess is + /// recorded as a debt, and + /// [`Err(BalanceError::InsufficientBalance)`] is returned. The caller + /// must mark the validator for forced ejection. + /// + /// # Errors + /// + /// Returns [`BalanceError::ValidatorNotFound`] when the index is unknown. + pub fn apply_penalty( + &mut self, + validator_index: ValidatorIndex, + penalty: u64, + ) -> Result { + let balance = self + .balances + .get_mut(&validator_index) + .ok_or(BalanceError::ValidatorNotFound)?; + + match balance.checked_sub(penalty) { + Some(new_balance) => { + *balance = new_balance; + Ok(new_balance) + } + None => { + // Record the excess as a debt so the ejection logic can detect + // it even after the balance has been zeroed. + let debt = penalty - *balance; + *self.debts.entry(validator_index).or_insert(0) += debt; + // The validator's stored balance is set to zero because the + // full balance has been consumed. + *balance = 0; + Err(BalanceError::InsufficientBalance { + balance: 0, + penalty, + }) + } + } + } + + /// Apply `reward` (Gwei) to `validator_index`, capped at + /// [`MAX_EFFECTIVE_BALANCE`]. + /// + /// # Errors + /// + /// Returns [`BalanceError::ValidatorNotFound`] when the index is unknown. + pub fn apply_reward( + &mut self, + validator_index: ValidatorIndex, + reward: u64, + ) -> Result { + let balance = self + .balances + .get_mut(&validator_index) + .ok_or(BalanceError::ValidatorNotFound)?; + + *balance = balance.saturating_add(reward).min(MAX_EFFECTIVE_BALANCE); + Ok(*balance) + } + + // ----------------------------------------------------------------------- + // Ejection helpers + // ----------------------------------------------------------------------- + + /// Return the indices of all validators that should be ejected at the + /// current epoch boundary. + /// + /// A validator is ejection-eligible when **either**: + /// 1. Its effective balance has fallen strictly below + /// [`EJECTION_THRESHOLD`], **or** + /// 2. It has an outstanding debt (the penalty exceeded its balance). + pub fn ejection_eligible(&self) -> alloc::vec::Vec { + let mut eligible = alloc::vec::Vec::new(); + for (&idx, &bal) in &self.balances { + if bal < EJECTION_THRESHOLD || self.has_debt(idx) { + eligible.push(idx); + } + } + eligible + } + + /// Clear the debt record for a validator (called after ejection is + /// confirmed so the entry does not linger). + pub fn clear_debt(&mut self, validator_index: ValidatorIndex) { + self.debts.remove(&validator_index); + } +} diff --git a/src/validator/mod.rs b/src/validator/mod.rs index 783c5c8..7b6d0fd 100644 --- a/src/validator/mod.rs +++ b/src/validator/mod.rs @@ -1,6 +1,7 @@ //! Validator lifecycle: the deterministic exit queue and validator set. pub mod activation_queue; +pub mod balance_tracker; pub mod committee_assignment; pub mod exit_queue; pub mod validator_set; diff --git a/tests/validator/balance_underflow_test.rs b/tests/validator/balance_underflow_test.rs new file mode 100644 index 0000000..d82de76 --- /dev/null +++ b/tests/validator/balance_underflow_test.rs @@ -0,0 +1,308 @@ +//! Tests for the validator balance underflow fix (issue #241). +//! +//! Verifies that: +//! 1. A penalty exceeding the balance returns `InsufficientBalance` rather +//! than silently clamping to zero. +//! 2. A validator with an outstanding debt is always included in the +//! ejection-eligible set, regardless of its stored balance. +//! 3. Normal penalties (penalty ≤ balance) continue to work correctly. +//! 4. Rewards are capped at `MAX_EFFECTIVE_BALANCE`. +//! 5. Property test: `effective_balance` never exceeds `MAX_EFFECTIVE_BALANCE` +//! after any sequence of reward applications. + +use proptest::prelude::*; +use sorosusu_contracts::slashing::penalty_calculator::{ + compute_inactivity_penalty, compute_slashing_penalty, +}; +use sorosusu_contracts::validator::balance_tracker::{ + BalanceError, BalanceTracker, EJECTION_THRESHOLD, GWEI_PER_ETH, MAX_EFFECTIVE_BALANCE, +}; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +/// 1 ETH in Gwei. +const ONE_ETH: u64 = GWEI_PER_ETH; +/// 1.5 ETH in Gwei. +const ONE_POINT_FIVE_ETH: u64 = ONE_ETH + ONE_ETH / 2; +/// 32 ETH in Gwei. +const THIRTY_TWO_ETH: u64 = 32 * GWEI_PER_ETH; + +// --------------------------------------------------------------------------- +// Issue #241 acceptance criterion +// --------------------------------------------------------------------------- + +/// A validator holding 1 ETH receives a 1.5 ETH slashing penalty. +/// The tracker must return `InsufficientBalance` (not silently clamp to zero) +/// and the validator must appear in `ejection_eligible()` at the same epoch. +#[test] +fn validator_with_1_eth_receiving_1_5_eth_penalty_is_ejected_same_epoch() { + let mut tracker = BalanceTracker::new(); + let validator_index: u64 = 0; + + // Register validator with 1 ETH. + tracker.register_validator(validator_index, ONE_ETH); + + // Apply a 1.5 ETH penalty (e.g. a slashable-offense amount larger than balance). + let result = tracker.apply_penalty(validator_index, ONE_POINT_FIVE_ETH); + + // Must return InsufficientBalance — NOT silently clamp. + assert!( + matches!(result, Err(BalanceError::InsufficientBalance { .. })), + "expected InsufficientBalance error, got {:?}", + result + ); + + // The stored balance is zeroed. + assert_eq!( + tracker.effective_balance(validator_index), + Some(0), + "balance should be zeroed after underflow" + ); + + // A debt must have been recorded (excess = 0.5 ETH). + assert!( + tracker.has_debt(validator_index), + "debt should be recorded when penalty exceeds balance" + ); + let expected_debt = ONE_POINT_FIVE_ETH - ONE_ETH; // 0.5 ETH + assert_eq!( + tracker.outstanding_debt(validator_index), + Some(expected_debt) + ); + + // The validator must appear in the ejection-eligible set at this epoch + // — this is the core invariant that was broken before the fix. + let eligible = tracker.ejection_eligible(); + assert!( + eligible.contains(&validator_index), + "validator with outstanding debt must be ejection-eligible in the same epoch" + ); +} + +// --------------------------------------------------------------------------- +// Normal-path tests (regression guard) +// --------------------------------------------------------------------------- + +/// A penalty that is exactly equal to the balance should reduce the balance to +/// zero and NOT record a debt (no excess). +#[test] +fn penalty_equal_to_balance_zeroes_balance_without_debt() { + let mut tracker = BalanceTracker::new(); + tracker.register_validator(1, ONE_ETH); + + let result = tracker.apply_penalty(1, ONE_ETH); + + assert_eq!(result, Ok(0), "balance - balance should be 0"); + assert!(!tracker.has_debt(1), "no debt when penalty == balance"); + // The validator's balance is zero but it has no debt; it is still + // ejection-eligible because 0 < EJECTION_THRESHOLD. + assert!(tracker.ejection_eligible().contains(&1)); +} + +/// A penalty smaller than the balance must reduce the balance by exactly +/// the penalty amount and leave no debt. +#[test] +fn penalty_smaller_than_balance_reduces_balance_correctly() { + let mut tracker = BalanceTracker::new(); + let initial_balance = 20 * ONE_ETH; + tracker.register_validator(2, initial_balance); + + let penalty = 3 * ONE_ETH; + let result = tracker.apply_penalty(2, penalty); + + assert_eq!(result, Ok(initial_balance - penalty)); + assert!(!tracker.has_debt(2)); + assert_eq!(tracker.effective_balance(2), Some(17 * ONE_ETH)); +} + +/// A validator above the ejection threshold with no debt must NOT appear in +/// the ejection-eligible set. +#[test] +fn healthy_validator_is_not_ejection_eligible() { + let mut tracker = BalanceTracker::new(); + tracker.register_validator(3, THIRTY_TWO_ETH); + + let eligible = tracker.ejection_eligible(); + assert!( + !eligible.contains(&3), + "healthy validator must not be ejection-eligible" + ); +} + +/// A validator whose balance has dropped below the ejection threshold by a +/// normal penalty (no debt) must still appear in the ejection-eligible set. +#[test] +fn validator_below_ejection_threshold_is_eligible() { + let mut tracker = BalanceTracker::new(); + tracker.register_validator(4, EJECTION_THRESHOLD); // exactly at threshold + + // Reduce by 1 Gwei — now strictly below threshold. + tracker.apply_penalty(4, 1).unwrap(); + + let eligible = tracker.ejection_eligible(); + assert!( + eligible.contains(&4), + "validator just below ejection threshold must be ejection-eligible" + ); +} + +// --------------------------------------------------------------------------- +// Reward tests +// --------------------------------------------------------------------------- + +/// Rewards must not push the balance above `MAX_EFFECTIVE_BALANCE`. +#[test] +fn reward_is_capped_at_max_effective_balance() { + let mut tracker = BalanceTracker::new(); + tracker.register_validator(5, MAX_EFFECTIVE_BALANCE); + + // Applying any additional reward must not overflow the cap. + tracker.apply_reward(5, ONE_ETH).unwrap(); + assert_eq!( + tracker.effective_balance(5), + Some(MAX_EFFECTIVE_BALANCE), + "balance must be capped at MAX_EFFECTIVE_BALANCE" + ); +} + +/// A reward on a zero-balance validator must not exceed `MAX_EFFECTIVE_BALANCE`. +#[test] +fn reward_from_zero_cannot_exceed_max_effective_balance() { + let mut tracker = BalanceTracker::new(); + tracker.register_validator(6, 0); + + // An absurdly large reward saturates at `u64::MAX` in `saturating_add` + // and is then clamped down to `MAX_EFFECTIVE_BALANCE` by `.min()`. + tracker.apply_reward(6, u64::MAX).unwrap(); + assert_eq!(tracker.effective_balance(6), Some(MAX_EFFECTIVE_BALANCE)); +} + +// --------------------------------------------------------------------------- +// Debt lifecycle +// --------------------------------------------------------------------------- + +/// After a forced ejection the debt record should be clearable. +#[test] +fn clearing_debt_removes_validator_from_ejection_eligible() { + let mut tracker = BalanceTracker::new(); + tracker.register_validator(7, ONE_ETH); + + // Trigger underflow. + tracker + .apply_penalty(7, ONE_POINT_FIVE_ETH) + .expect_err("should return InsufficientBalance"); + + // Validator is ejection-eligible because of the debt. + assert!(tracker.ejection_eligible().contains(&7)); + + // Simulate ejection: clear the debt and ensure balance is 0. + tracker.clear_debt(7); + // After clearing, validator still has balance 0 which is < EJECTION_THRESHOLD. + // The debt is gone but balance-based ejection eligibility remains. + assert!(!tracker.has_debt(7), "debt must be gone after clear_debt"); +} + +// --------------------------------------------------------------------------- +// Penalty calculator unit tests +// --------------------------------------------------------------------------- + +/// For a 32 ETH validator, `compute_slashing_penalty` must equal 2 ETH. +#[test] +fn slashing_penalty_for_32_eth_is_2_eth() { + let penalty = compute_slashing_penalty(THIRTY_TWO_ETH); + let expected = 2 * ONE_ETH; // 1/16 of 32 ETH + assert_eq!( + penalty, expected, + "slashing penalty for 32 ETH must be 2 ETH" + ); +} + +/// `compute_slashing_penalty` for a 1 ETH validator should equal 1/16 of 1 ETH. +#[test] +fn slashing_penalty_scales_with_balance() { + let penalty = compute_slashing_penalty(ONE_ETH); + // 1_000_000_000 / 32 * 2 = 62_500_000 + let expected = ONE_ETH / 32 * 2; + assert_eq!(penalty, expected); +} + +/// With 0 epochs since finality, the inactivity penalty must be zero. +#[test] +fn inactivity_penalty_zero_epochs_is_zero() { + let penalty = compute_inactivity_penalty(THIRTY_TWO_ETH, 0); + assert_eq!(penalty, 0); +} + +/// For a non-zero epoch count the penalty must be positive. +#[test] +fn inactivity_penalty_positive_for_nonzero_epochs() { + let penalty = compute_inactivity_penalty(THIRTY_TWO_ETH, 100); + assert!( + penalty > 0, + "inactivity penalty must be positive for 100 epochs" + ); +} + +// --------------------------------------------------------------------------- +// Property test +// --------------------------------------------------------------------------- + +proptest! { + /// For any sequence of rewards applied to a registered validator, the + /// effective balance must never exceed `MAX_EFFECTIVE_BALANCE`. + #[test] + fn prop_effective_balance_never_exceeds_max_after_rewards( + initial in 0u64..=MAX_EFFECTIVE_BALANCE, + rewards in prop::collection::vec(0u64..=MAX_EFFECTIVE_BALANCE, 1..50), + ) { + let mut tracker = BalanceTracker::new(); + let idx: u64 = 99; + tracker.register_validator(idx, initial); + for reward in rewards { + let _ = tracker.apply_reward(idx, reward); + } + let balance = tracker.effective_balance(idx).unwrap(); + prop_assert!( + balance <= MAX_EFFECTIVE_BALANCE, + "balance {} exceeded MAX_EFFECTIVE_BALANCE {}", + balance, + MAX_EFFECTIVE_BALANCE + ); + } + + /// For any balance and penalty where penalty > balance, apply_penalty must + /// return InsufficientBalance and must never silently clamp without recording a debt. + #[test] + fn prop_penalty_exceeding_balance_always_returns_insufficient_balance( + balance in 0u64..u64::MAX, + excess in 1u64..=u64::MAX, + ) { + // penalty = balance + excess, guarded against overflow + let penalty = match balance.checked_add(excess) { + Some(p) => p, + None => return Ok(()), // skip overflow cases + }; + + let mut tracker = BalanceTracker::new(); + let idx: u64 = 42; + tracker.register_validator(idx, balance); + + let result = tracker.apply_penalty(idx, penalty); + + prop_assert!( + matches!(result, Err(BalanceError::InsufficientBalance { .. })), + "expected InsufficientBalance for balance={}, penalty={}, got {:?}", + balance, penalty, result + ); + prop_assert!( + tracker.has_debt(idx), + "debt must be recorded when penalty > balance" + ); + prop_assert!( + tracker.ejection_eligible().contains(&idx), + "validator must be ejection-eligible when it has a debt" + ); + } +}