farming-pool: compute_credits' unchecked i128 multiplication chain traps the whole contract on overflow with no typed error

[prodbycorne]

Problem

compute_credits chains three unchecked i128 multiplications with no bound on any input:

fn compute_credits(
    amount: i128,
    allocation_pct: u32,
    multiplier: u32,
    credit_rate: i128,
    ledgers_elapsed: u32,
) -> i128 {
    compute_total_stake(amount, allocation_pct, multiplier) * credit_rate * ledgers_elapsed as i128
}

amount is user-supplied (bounded only by their token balance, which can be arbitrarily large for high-supply tokens with many decimals), multiplier and credit_rate are admin-controlled with no upper bound at all (assert!(global_multiplier >= 1, ...) and assert!(credit_rate > 0, ...) only check a lower bound), and ledgers_elapsed grows unboundedly the longer a position goes uncheckpointed.

The workspace release profile sets overflow-checks = true (soroban/Cargo.toml), so an overflow does not silently wrap — it traps the whole invocation. That is a meaningful improvement over silent corruption, but it means: any user whose stake.amount * credit_rate * multiplier * elapsed product exceeds i128::MAX (~1.7e38) can no longer call any function that reads their position (get_credits, checkpoint via stake/unstake/set_boost) — every call traps, permanently, with an opaque host error instead of a typed PoolError. There is no checked_mul/checked_add anywhere in the accrual path (confirmed: no checked_ calls exist in farming-pool/src/lib.rs), so this failure mode is unrecoverable without an emergency migration.

This is realistic, not merely theoretical: credit_rate and global_multiplier are both admin-settable to arbitrary values after initialization via set_credit_rate/set_global_multiplier, so an admin fat-fingering a rate (e.g. intending 100 but typing 100_000_000_000) can retroactively brick every large staker's position the next time they interact with the pool.

Impact

• Denial of service / fund lock: an overflowing product permanently traps checkpoint, get_credits, calculate_credits, unstake, and unlock_assets for the affected user — with no recovery path since there is no way to reset stake.amount/credit_rate/start_ledger without the transaction itself succeeding first.
• No typed error surfaced: the failure presents as an opaque Soroban host trap rather than a matchable PoolError variant, making it undiagnosable for integrators without reading the WASM trap output.
• Asymmetric admin risk: a single set_credit_rate/set_global_multiplier call by the admin can retroactively immobilize funds for every large staker in the pool.

Fix

Use checked_mul/checked_add throughout the accrual path and surface a new typed error instead of trapping:

pub enum PoolError {
    // ...
    CreditOverflow = 15,
}

fn compute_credits(
    amount: i128, allocation_pct: u32, multiplier: u32, credit_rate: i128, ledgers_elapsed: u32,
) -> Result<i128, PoolError> {
    let total_stake = compute_total_stake(amount, allocation_pct, multiplier)?;
    total_stake
        .checked_mul(credit_rate)
        .and_then(|v| v.checked_mul(ledgers_elapsed as i128))
        .ok_or(PoolError::CreditOverflow)
}

Propagate Result through checkpoint, checkpoint_position, get_credits, and calculate_credits so an overflowing user can still call unstake/unlock_assets and receive a typed, recoverable error rather than a permanent trap — and consider whether unstake/unlock_assets should fall back to returning principal without the overflowing credit computation in a degraded-but-functional mode.

Acceptance Criteria

• Add PoolError::CreditOverflow (or equivalent) variant.
• compute_credits/compute_total_stake use checked_mul and return Result instead of trapping.
• Add test_get_credits_returns_typed_error_on_overflow constructing a UserStake/GlobalMultiplier/CreditRate combination that overflows i128 and asserting a typed PoolError is returned rather than a panic/trap.
• Add a fuzz or property test sweeping amount, multiplier, credit_rate, and ledgers_elapsed near their extremes to confirm no path silently wraps or traps ungracefully.
• unstake/unlock_assets remain callable (returning principal, at minimum) even when the credit computation for that user would overflow.

Additional Notes

Confirmed against current source: compute_credits (farming-pool/src/lib.rs:138-146) chains compute_total_stake(...) * credit_rate * ledgers_elapsed as i128 with plain * operators; grepping farming-pool/src/lib.rs for checked_ confirms zero occurrences anywhere in the crate. soroban/Cargo.toml's release profile sets overflow-checks = true (confirmed present), so overflow traps rather than wraps — worth restating in the fix comment since it changes the severity framing from "silent corruption" to "permanent DoS," which is the framing the issue already uses.

Edge cases:

• checkpoint (lib.rs:148-162) and checkpoint_position (lib.rs:164-170) both call into accrual math but only the former routes through compute_credits/compute_total_stake — checkpoint_position does its own inline position.amount * position.credit_rate * elapsed as i128 (lib.rs:167) with the exact same unchecked-multiplication risk, but is not mentioned in this issue's acceptance criteria. Since farming-pool: checkpoint_position and calculate_credits duplicate the credit-accrual formula instead of sharing one implementation #63 asks to extract a shared compute_position_credits helper, the overflow fix should target that shared helper too (once farming-pool: checkpoint_position and calculate_credits duplicate the credit-accrual formula instead of sharing one implementation #63 lands) rather than leaving the Position path unchecked while UserStake is fixed — otherwise this issue's fix only closes half of the two structurally-identical accrual paths.
• calculate_credits (lib.rs:305-317) does the identical unchecked multiplication as checkpoint_position for the preview path — same gap as above.
• Interacts with farming-pool: get_credits retroactively reprices a user's entire uncheckpointed accrual window when the admin changes the global multiplier #60: once UserStake gains a multiplier field, the overflow-checked compute_credits needs to accept/pass through that frozen value rather than a live read — implement farming-pool: get_credits retroactively reprices a user's entire uncheckpointed accrual window when the admin changes the global multiplier #60 first (or in the same PR) so the checked-arithmetic signature doesn't need to change twice.
• The suggested PoolError::CreditOverflow = 15 fits the next open discriminant after NoActiveStake = 14; note discriminants 4-12 are unused/reserved in the current PoolError enum (only 1, 2, 3, 13, 14 are defined) — worth a one-line comment on why, or at least confirming the new variant picks an unused value and doesn't assume contiguous numbering.

Implementation sketch: change compute_total_stake/compute_credits (lib.rs:131-146) to return Result<i128, PoolError> using checked_mul; propagate ? through checkpoint (148-162, called from stake/unstake/set_boost), get_credits (568-586), and — if #63's shared-helper refactor lands first — the analogous Position helper used by checkpoint_position/calculate_credits. unlock_assets/unstake should still be able to return principal even if the credit computation overflows, per the issue's last acceptance-criteria bullet — this likely means computing/persisting the credit component in a try-like manner and falling back to 0 or a saturated value with a distinguishable event, rather than aborting the whole withdrawal.

Test plan: test_get_credits_returns_typed_error_on_overflow (construct UserStake{ amount: i128::MAX / 2, credit_rate: i128::MAX / 2, .. } combined with an admin-set large global_multiplier/credit_rate to force overflow, assert Err(PoolError::CreditOverflow) via try_get_credits), plus a property/fuzz test (per issue) sweeping amount, multiplier, credit_rate, ledgers_elapsed near i128::MAX/u32::MAX boundaries. Add test_unstake_still_returns_principal_when_credit_overflow proving the degraded-but-functional withdrawal path works.

Cross-references: #60 (multiplier snapshot changes the same function signatures), #61 (same call chain, sequencing matters for merge order), #63 (shared helper extraction should happen before or alongside this fix so both UserStake and Position paths get overflow protection together, not just one).

[Achievers-sketch]

Experienced Soroban/Rust engineer. I’ll replace unchecked arithmetic with checked operations, propagate typed overflow errors, preserve principal withdrawals, and add comprehensive boundary/property tests