farming-pool: no fuzz/boundary test exercises i128 overflow boundaries in compute_credits/compute_total_stake

[prodbycorne]

Problem

compute_credits chains three unchecked i128 multiplications (compute_total_stake(...) * credit_rate * ledgers_elapsed as i128, itself internally computing amount * allocation_pct as i128 / 100 and boosted * multiplier as i128), and there is currently zero test coverage anywhere near the boundaries of i128:

$ grep -n "i128::MAX\|checked_mul\|overflow" soroban/contracts/farming-pool/src/test.rs
# (no matches)

Every existing test in farming-pool/src/test.rs uses small, hand-picked values (amount in the hundreds/thousands, multiplier in 1-10, elapsed in single/double digits), which never come close to exercising the overflow behavior the workspace's overflow-checks = true release profile setting implies should be tested (see the companion "unchecked i128 multiplication chain traps the whole contract on overflow" issue). Since global_multiplier and credit_rate are both admin-settable to arbitrary values post-initialization with no upper bound, and amount is bounded only by a user's token balance, the actual production input space is far larger than anything the test suite currently exercises.

Impact

• The most consequential arithmetic bug class in the contract is completely untested: whether compute_credits/compute_total_stake trap gracefully, trap ungracefully (losing the whole transaction with an opaque host error), or — if checked_mul migration from the companion overflow issue is only partially applied — silently produce a wrong result, is currently unverified by any automated test.
• Regression risk for the overflow-checked fix: once compute_credits/compute_total_stake are migrated to checked_mul (per the companion issue), there is no fuzz/boundary test to prove the fix actually covers the full input space rather than just the specific values chosen ad hoc for a couple of example tests.

Fix

Add targeted boundary-value tests plus a bounded fuzz sweep near the extremes of each input:

#[test]
fn test_compute_credits_near_i128_max_amount_does_not_wrap() {
    // amount chosen so that amount * allocation_pct/100 * multiplier * credit_rate * elapsed
    // is just below i128::MAX, and a slightly larger variant just above it.
    let result = std::panic::catch_unwind(|| {
        compute_credits(i128::MAX / 4, 100, 4, 1, 1)
    });
    // Once the checked_mul migration lands, assert this returns Err(PoolError::CreditOverflow)
    // instead of panicking/trapping.
}

// Plus, if proptest is added per the path-independence issue:
proptest! {
    #[test]
    fn compute_credits_never_wraps_or_panics_silently(
        amount in 1i128..i128::MAX/1000,
        allocation_pct in 1u32..=100,
        multiplier in 1u32..10_000,
        credit_rate in 1i128..1_000_000,
        elapsed in 1u32..1_000_000,
    ) {
        // Assert the function either returns a correct-magnitude result or a typed overflow
        // error — never a value smaller than a naively-scaled lower bound, which would indicate
        // silent wraparound.
    }
}

Acceptance Criteria

• At least one explicit boundary test using values close to i128::MAX for amount/multiplier/credit_rate/ledgers_elapsed combinations.
• A fuzz or property-based sweep across a wide range of (amount, allocation_pct, multiplier, credit_rate, elapsed) combinations asserting no silent wraparound occurs.
• Test(s) reference and validate the fix from the companion "unchecked i128 multiplication chain" issue (i.e. assert a typed error is returned near the overflow boundary, once that fix lands, rather than a panic).
• Tests run within CI's existing cargo test --workspace time budget.

Further Investigation

Why this is a spike: the right shape of the fix this test suite is meant to validate is itself undecided. compute_credits (soroban/contracts/farming-pool/src/lib.rs:138-146) and compute_total_stake (lib.rs:131-136) currently use raw */-// operators on i128, which under this workspace's overflow-checks profile setting either panics (traps the whole transaction with an opaque host error) or wraps silently depending on build profile — the same profile-dependency risk flagged in #82 for the factory's u32 PoolCount. Whether the eventual fix is (a) checked_mul/checked_add returning a new PoolError::CreditOverflow variant, (b) saturating arithmetic that clamps to i128::MAX instead of erroring, or (c) a redesign that keeps intermediate values in a wider/scaled representation to push the overflow boundary out further, is an open design decision this issue's own body defers to a "companion issue" — meaning the tests this issue asks for can't be finalized until that decision is made, and the test design should itself help surface which of (a)/(b)/(c) is preferable by characterizing exactly where today's implementation breaks.

Tradeoff approaches for the boundary-test design

1. Pure characterization tests first (no fix assumed): write #[should_panic] tests that document today's actual behavior (does i128::MAX/4 * 100 / 100 * 4 * 1 * 1 panic via debug-mode overflow-checks, or does the release-profile setting mean the workspace's cargo test run — which uses the dev/test profile, not [profile.release] — actually panics today regardless of the release profile flag?). This is important to verify empirically rather than assumed: confirm via cargo test -p farming-pool whether overflow-checks is even active in the default test profile before designing the "graceful error" tests around an assumption. If it already panics in tests today, that's arguably an existing (if undocumented) DoS/panic-safety gap in its own right.
2. Property-based sweep bounded near realistic production ranges, per the issue's own proptest sketch — cheap to write but only useful once the ranges chosen actually reach the overflow boundary for the current un-fixed arithmetic; today's implementation will hit the panic boundary at fairly modest combined inputs (e.g. amount ~ 10^15, multiplier ~ 10^4, credit_rate ~ 10^6, elapsed ~ 10^6 already exceeds i128::MAX ~ 1.7*10^38 when multiplied together... actually verify the exact magnitude threshold by computation rather than estimating, since i128::MAX is large enough that four chained multiplications of "reasonable-looking" per-admin-settable values may or may not reach it — this needs to be computed precisely as part of the spike, not assumed).
3. Golden/regression boundary tests keyed to the eventual error variant name: once the overflow-issue fix lands (introducing e.g. PoolError::CreditOverflow), add exact boundary tests (i128::MAX / N for a precisely derived N) asserting the typed error is returned at N+1 and a valid result at N. This is the most precise but has to be written after, not concurrently with, the arithmetic fix — sequencing risk if this issue is picked up before its companion overflow-fix issue.

Edge cases

• allocation_pct = 100 combined with very large multiplier (removes the /100 truncation's downward pressure, reaching overflow fastest).
• Negative-adjacent inputs: credit_rate is i128 and only validated > 0 at set_credit_rate/initialize, so a fuzz sweep should also confirm no combination of positive inputs can ever produce a negative result via wraparound (a negative credits balance would be a distinct, worse bug than a panic — silent value corruption rather than a loud failure).
• Interaction with farming-pool: set_global_multiplier and set_credit_rate accept unbounded values with no sanity ceiling, guaranteeing compute_credits overflow at scale #89: once set_global_multiplier/set_credit_rate gain upper ceilings, add a specific test asserting that the maximum allowed multiplier/credit_rate combined with a generous but realistic amount and multi-year elapsed (e.g. ~6.3M ledgers/year at 5s/ledger) still does not overflow — this is the concrete acceptance test tying farming-pool: set_global_multiplier and set_credit_rate accept unbounded values with no sanity ceiling, guaranteeing compute_credits overflow at scale #89's chosen ceiling constants back to this issue's fuzzing.
• Interaction with farming-pool: no property-based test enforces that total accrued credits are independent of checkpoint frequency #75: both issues likely want proptest as a shared dev-dependency; coordinate constants/ranges (e.g. a shared proptest config module) rather than duplicating generator bounds independently in two places.

Test plan

• test_compute_credits_near_i128_max_amount_does_not_wrap (named in the issue body) plus a first characterization test test_compute_credits_current_behavior_at_i128_max_boundary documenting today's actual (panic vs. wrap) behavior before any fix lands.
• compute_credits_never_wraps_or_panics_silently proptest (named in the issue body), gated behind the same proptest dev-dependency added for farming-pool: no property-based test enforces that total accrued credits are independent of checkpoint frequency #75.
• Add test_set_credit_rate_and_multiplier_ceiling_prevents_overflow once farming-pool: set_global_multiplier and set_credit_rate accept unbounded values with no sanity ceiling, guaranteeing compute_credits overflow at scale #89's ceilings exist, computing the actual numeric bound via i128::MAX divided by the chosen ceiling constants and a realistic max elapsed.

[Achievers-sketch]

Experienced Rust and property-testing developer. I’ll add boundary and fuzz tests for i128 overflow, validate checked arithmetic, prevent regressions, and ensure CI stability