From afae950745d5b161b74581c8b86fc886504073a7 Mon Sep 17 00:00:00 2001 From: Cyber-Mitch Date: Fri, 17 Jul 2026 01:16:14 +0100 Subject: [PATCH] fix(farming-pool): add derived value ceilings to multiplier and credit-rate setters (closes #89) --- soroban/contracts/farming-pool/src/lib.rs | 69 +++++++++- soroban/contracts/farming-pool/src/test.rs | 140 +++++++++++++++++++- soroban/contracts/farming-pool/src/types.rs | 2 + 3 files changed, 205 insertions(+), 6 deletions(-) diff --git a/soroban/contracts/farming-pool/src/lib.rs b/soroban/contracts/farming-pool/src/lib.rs index be5ada0..cd62cce 100644 --- a/soroban/contracts/farming-pool/src/lib.rs +++ b/soroban/contracts/farming-pool/src/lib.rs @@ -10,6 +10,52 @@ use types::{BoostConfig, DataKey, Position, UserStake}; const USER_TTL_THRESHOLD: u32 = 518_400; const USER_TTL_EXTEND_TO: u32 = 1_036_800; +/// Sanity ceilings on `global_multiplier` and `credit_rate` (see #89). +/// +/// `compute_credits` computes +/// `compute_total_stake(amount, allocation_pct, multiplier) * credit_rate * ledgers_elapsed`, +/// and `compute_total_stake` reduces to exactly `amount * multiplier` at +/// `allocation_pct = 100` (its worst case: `boosted = amount`, `principal = 0`). +/// The `/100` division in `compute_total_stake` therefore does *not* loosen +/// this bound at the boundary — the naive product below is tight, not a +/// conservative over-estimate. +/// +/// Worst-case overflow chain: +/// +/// ```text +/// amount_max * multiplier_max * credit_rate_max * elapsed_max <= i128::MAX / 16 +/// ``` +/// +/// Inputs, chosen and justified independently of the multiplier/credit-rate +/// ceilings themselves: +/// - `amount_max = 10^18` — 100 billion whole tokens at Stellar's standard +/// 7-decimal ("stroop") convention. Far above any realistic pool TVL, but +/// many orders of magnitude below `i128::MAX` (~1.7 x 10^38). +/// - `elapsed_max = 63_072_000` ledgers — ~10 years at 5s/ledger +/// (`10 * 365 * 24 * 3600 / 5`), a multi-year operational horizon between +/// checkpoints. +/// - Headroom factor of 16x, i.e. the worst-case product must not exceed +/// `i128::MAX / 16`, leaving ample margin beyond the bare non-overflow +/// requirement. +/// +/// Solving for `multiplier_max * credit_rate_max`: +/// `(i128::MAX / 16) / (amount_max * elapsed_max) ≈ 1.686 x 10^11`. +/// +/// Chosen ceilings (round, human-readable, at or below the derived bound): +/// - `MAX_GLOBAL_MULTIPLIER = 1_000` +/// - `MAX_CREDIT_RATE = 100_000_000` (10^8) +/// - product = 10^11, comfortably under the 1.686 x 10^11 budget. +/// +/// Verification: `amount_max * multiplier_max * credit_rate_max * elapsed_max` +/// = `10^18 * 1_000 * 10^8 * 63_072_000` ≈ `6.307 x 10^36`, versus +/// `i128::MAX ≈ 1.701 x 10^38` — a headroom ratio of ~27x, comfortably +/// exceeding the required 16x. (For reference, the earlier sketch pair of +/// 1_000 / 1_000_000_000 gives a worst case of ≈ 6.307 x 10^37, which fits +/// under raw `i128::MAX` but only with ~2.7x headroom — it does not survive +/// this derivation's 16x margin, hence `MAX_CREDIT_RATE` here is 10x smaller.) +const MAX_GLOBAL_MULTIPLIER: u32 = 1_000; +const MAX_CREDIT_RATE: i128 = 100_000_000; + fn bump_instance(env: &Env) { env.storage() .instance() @@ -174,6 +220,10 @@ pub struct FarmingPool; #[contractimpl] impl FarmingPool { + /// Initialize the pool. `global_multiplier` and `credit_rate` are bounded + /// by `MAX_GLOBAL_MULTIPLIER`/`MAX_CREDIT_RATE` — see #89 for the + /// overflow-safety derivation shared with `set_global_multiplier` and + /// `set_credit_rate`. pub fn initialize( env: Env, admin: Address, @@ -185,8 +235,13 @@ impl FarmingPool { if env.storage().instance().has(&DataKey::Admin) { return Err(PoolError::AlreadyInitialized); } - assert!(global_multiplier >= 1, "multiplier must be >= 1"); - assert!(credit_rate > 0, "credit_rate must be positive"); + // Ceilings mirror `set_global_multiplier`/`set_credit_rate` — see #89. + if !(1..=MAX_GLOBAL_MULTIPLIER).contains(&global_multiplier) { + return Err(PoolError::InvalidGlobalMultiplier); + } + if credit_rate <= 0 || credit_rate > MAX_CREDIT_RATE { + return Err(PoolError::InvalidCreditRate); + } env.storage().instance().set(&DataKey::Admin, &admin); env.storage() @@ -494,10 +549,14 @@ impl FarmingPool { ) } + /// Set the global credit multiplier. Rejects 0 and anything above + /// `MAX_GLOBAL_MULTIPLIER` — see #89 for the overflow-safety derivation. pub fn set_global_multiplier(env: Env, multiplier: u32) -> Result<(), PoolError> { require_initialized(&env)?; get_admin(&env)?.require_auth(); - assert!(multiplier >= 1, "multiplier must be >= 1"); + if !(1..=MAX_GLOBAL_MULTIPLIER).contains(&multiplier) { + return Err(PoolError::InvalidGlobalMultiplier); + } bump_instance(&env); env.storage() @@ -510,10 +569,12 @@ impl FarmingPool { Ok(()) } + /// Set the credit accrual rate. Rejects non-positive values and anything + /// above `MAX_CREDIT_RATE` — see #89 for the overflow-safety derivation. pub fn set_credit_rate(env: Env, new_rate: i128) -> Result<(), PoolError> { require_initialized(&env)?; get_admin(&env)?.require_auth(); - if new_rate <= 0 { + if new_rate <= 0 || new_rate > MAX_CREDIT_RATE { return Err(PoolError::InvalidCreditRate); } bump_instance(&env); diff --git a/soroban/contracts/farming-pool/src/test.rs b/soroban/contracts/farming-pool/src/test.rs index 95f4704..87093bc 100644 --- a/soroban/contracts/farming-pool/src/test.rs +++ b/soroban/contracts/farming-pool/src/test.rs @@ -321,6 +321,22 @@ fn test_set_credit_rate_rejects_zero_with_typed_error() { assert!(matches!(result, Err(Ok(PoolError::InvalidCreditRate)))); } +// ── #89: value ceilings on set_global_multiplier / set_credit_rate ────────── + +#[test] +fn test_set_credit_rate_rejects_above_ceiling() { + let t = setup(2, 1); + let result = t.client.try_set_credit_rate(&(MAX_CREDIT_RATE + 1)); + assert!(matches!(result, Err(Ok(PoolError::InvalidCreditRate)))); +} + +#[test] +fn test_set_credit_rate_accepts_exactly_the_ceiling() { + let t = setup(2, 1); + t.client.set_credit_rate(&MAX_CREDIT_RATE); + assert_eq!(t.client.credit_rate(), MAX_CREDIT_RATE); +} + #[test] fn test_set_credit_rate_requires_admin_auth() { let (env, contract_id, client, _admin, user) = setup_without_mocked_auth(); @@ -414,10 +430,130 @@ fn test_admin_multiplier_change_applies_from_next_checkpoint() { } #[test] -#[should_panic(expected = "multiplier must be >= 1")] fn test_admin_multiplier_rejects_zero() { + // Updated for #89: the old bare `assert!` (matched via `should_panic`) + // was replaced with a typed `PoolError::InvalidGlobalMultiplier` return, + // so this now asserts via `try_set_global_multiplier` like the rest of + // the typed-error suite instead of panic-message matching. + let t = setup(2, 1); + let result = t.client.try_set_global_multiplier(&0u32); + assert!(matches!( + result, + Err(Ok(PoolError::InvalidGlobalMultiplier)) + )); +} + +#[test] +fn test_set_global_multiplier_rejects_above_ceiling() { + let t = setup(2, 1); + let result = t + .client + .try_set_global_multiplier(&(MAX_GLOBAL_MULTIPLIER + 1)); + assert!(matches!( + result, + Err(Ok(PoolError::InvalidGlobalMultiplier)) + )); +} + +#[test] +fn test_set_global_multiplier_accepts_exactly_the_ceiling() { let t = setup(2, 1); - t.client.set_global_multiplier(&0u32); + t.client.set_global_multiplier(&MAX_GLOBAL_MULTIPLIER); + t.client.stake(&t.user, &1_000); + t.client.set_boost(&t.user, &50u32); + let cfg = t.client.get_boost_config(&t.user).unwrap(); + assert_eq!(cfg.multiplier, MAX_GLOBAL_MULTIPLIER); +} + +#[test] +fn test_initialize_rejects_global_multiplier_above_ceiling() { + let env = Env::default(); + env.mock_all_auths(); + let admin = Address::generate(&env); + let token_admin = Address::generate(&env); + let asset = env.register_stellar_asset_contract_v2(token_admin); + let contract_id = env.register(FarmingPool, ()); + let client = FarmingPoolClient::new(&env, &contract_id); + + let result = client.try_initialize( + &admin, + &asset.address(), + &(MAX_GLOBAL_MULTIPLIER + 1), + &1i128, + &0u32, + ); + assert!(matches!( + result, + Err(Ok(PoolError::InvalidGlobalMultiplier)) + )); +} + +#[test] +fn test_initialize_rejects_credit_rate_above_ceiling() { + let env = Env::default(); + env.mock_all_auths(); + let admin = Address::generate(&env); + let token_admin = Address::generate(&env); + let asset = env.register_stellar_asset_contract_v2(token_admin); + let contract_id = env.register(FarmingPool, ()); + let client = FarmingPoolClient::new(&env, &contract_id); + + let result = client.try_initialize( + &admin, + &asset.address(), + &2u32, + &(MAX_CREDIT_RATE + 1), + &0u32, + ); + assert!(matches!(result, Err(Ok(PoolError::InvalidCreditRate)))); +} + +/// Ties the #89 fix to its own derivation: at both ceilings simultaneously +/// (`MAX_GLOBAL_MULTIPLIER`, `MAX_CREDIT_RATE`), full boost allocation (100%, +/// `compute_total_stake`'s worst case), the derivation's `amount_max`, and +/// its `elapsed_max` (~10 years of ledgers), the boost-path credit preview +/// (`get_credits`, which exercises the exact `compute_total_stake` / +/// `compute_credits` chain the derivation bounds) must not overflow/panic and +/// must equal the worst-case product computed by the same formula. This is a +/// single deterministic boundary point, not a fuzz/property suite (#75/#76 +/// remain out of scope). +#[test] +fn test_compute_credits_no_overflow_at_ceilings() { + // Matches the doc comment on MAX_GLOBAL_MULTIPLIER/MAX_CREDIT_RATE. + const AMOUNT_MAX: i128 = 1_000_000_000_000_000_000; // 10^18 + const ELAPSED_MAX: u32 = 63_072_000; // ~10 years at 5s/ledger + + let env = Env::default(); + env.mock_all_auths(); + + let admin = Address::generate(&env); + let user = Address::generate(&env); + + let token_admin = Address::generate(&env); + let asset = env.register_stellar_asset_contract_v2(token_admin); + let token_sac = StellarAssetClient::new(&env, &asset.address()); + token_sac.mint(&user, &AMOUNT_MAX); + + let contract_id = env.register(FarmingPool, ()); + let client = FarmingPoolClient::new(&env, &contract_id); + client.initialize( + &admin, + &asset.address(), + &MAX_GLOBAL_MULTIPLIER, + &MAX_CREDIT_RATE, + &0u32, + ); + + client.stake(&user, &AMOUNT_MAX); + client.set_boost(&user, &100u32); + advance_ledgers(&env, ELAPSED_MAX); + + // No panic/overflow trap here is itself the assertion; also check the + // value is exactly the worst-case product the derivation computed. + let credits = client.get_credits(&user); + let expected = + AMOUNT_MAX * MAX_GLOBAL_MULTIPLIER as i128 * MAX_CREDIT_RATE * ELAPSED_MAX as i128; + assert_eq!(credits, expected); } #[test] diff --git a/soroban/contracts/farming-pool/src/types.rs b/soroban/contracts/farming-pool/src/types.rs index 8d37e93..1931756 100644 --- a/soroban/contracts/farming-pool/src/types.rs +++ b/soroban/contracts/farming-pool/src/types.rs @@ -10,6 +10,8 @@ pub enum PoolError { InvalidCreditRate = 3, NotPaused = 13, NoActiveStake = 14, + /// `global_multiplier` was 0 or exceeded `MAX_GLOBAL_MULTIPLIER`. See #89. + InvalidGlobalMultiplier = 15, } /// Per-user boost configuration returned by `get_boost_config`.