Skip to content

Audit and harden payout/stake arithmetic in BetManager against overflow #588

@greatest0fallt1me

Description

@greatest0fallt1me

Description

The fee module is disciplined about checked arithmetic, but the betting/payout math is not. In contracts/predictify-hybrid/src/bets.rs, calculate_bet_payout performs raw i128 multiplication and division when computing the distributable pool and per-winner share, and place_bet mutates market.total_staked with a raw +=. While [profile.release] sets overflow-checks = true (workspace Cargo.toml), relying on panics for fund math is fragile and yields opaque failures instead of typed errors. This issue audits and converts the hot paths to checked arithmetic with explicit errors.

Requirements and context

  • Raw arithmetic to address in bets.rs: the payout block in calculate_bet_payout (bets.rs:636), e.g. summary.total_pool * fee_percentage / 10_000, total_pool - fee, distributable_pool / num_winners, and the final / total_bets_on_outcome (the checked_mul there is followed by an unchecked divide).
  • place_bet (bets.rs:240) uses market.total_staked += amount; (raw); note place_bets (bets.rs:329) already uses checked_add, so align place_bet with it.
  • Helper functions calculate_implied_probability and calculate_payout_multiplier in bets.rs also use raw * 100 / ....
  • Follow the established pattern in fees.rs (checked_fee_add, checked_mul_div_floor -> Error::FeeArithmeticOverflow); reuse or mirror it and guard divide-by-zero (num_winners/total_bets_on_outcome == 0).
  • Non-functional: behavior for normal values must be unchanged; only overflow/zero-divisor cases change from panic to typed Error.

Acceptance criteria

  • All raw i128 arithmetic in calculate_bet_payout is replaced with checked operations returning a typed error.
  • place_bet uses checked addition for total_staked, matching place_bets.
  • Divide-by-zero on winner/outcome counts is guarded and returns a typed error, not a panic.
  • Probability/multiplier helpers use checked math or documented saturating behavior.
  • Property/unit tests assert no overflow panics for boundary inputs (large pools, single winner, many bettors) and correct values for ordinary inputs.
  • cargo fmt, cargo clippy, and cargo test pass.

Suggested execution

1. Fork the repo and create a branchgit checkout -b feature/checked-bet-arithmetic.
2. Implement changescontracts/predictify-hybrid/src/bets.rs (payout/stake math); optionally factor a shared checked-math helper.
3. Write/extend tests — extend contracts/predictify-hybrid/src/property_based_tests.rs (proptest is a dev-dependency) and bet_tests.rs.
4. Test and commit

cargo fmt --all -- --check
cargo clippy --all-targets --all-features -- -D warnings
cargo test -p predictify-hybrid
stellar contract build --verbose

Example commit message

security: convert bet payout/stake math to checked arithmetic with typed errors

Guidelines

≥90% coverage on the arithmetic branches, including overflow/zero-divisor paths. Add doc-comments describing failure modes and update docs/contracts/FEES.md/docs/security/SECURITY_CONSIDERATIONS.md where payout math is described. Timeframe: 96 hours.

Metadata

Metadata

Assignees

No one assigned

    Labels

    GRANTFOX OSSGrantFox OSS programMAYBE REWARDEDGrantFox — potentially rewardedOFFICIAL CAMPAIGNGrantFox official campaignadvancedHigh complexity / deep contextsecuritySecurity hardeningsmart-contractSoroban / Rust contract work

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions