Skip to content

Add overflow-checked arithmetic verification under the release profile #24

@mikewheeleer

Description

@mikewheeleer

Harden arithmetic safety and document the overflow strategy

Description

Cargo.toml sets overflow-checks = true on the release profile, while contracts/escrow/src/lib.rs deliberately uses saturating_* in the hot paths. The mix of "panic on overflow (checked)" and "silently saturate" is not documented and could surprise auditors — e.g. a future + that should saturate but doesn't, or a saturating path that masks a real bug. This issue audits every arithmetic site and makes the strategy explicit and tested.

Requirements and context

  • Repository scope: Agentpay-Org/Agentpay-contracts only.
  • Inventory every arithmetic operation (usage add, lifetime add, billing mul, any new balance math) and classify each as "must saturate" or "must check/panic", with a rationale comment at each site.
  • Where saturation is intended, ensure it is explicit (saturating_*) and not relying on the build profile; where overflow must be impossible, document why.
  • Add a docs/escrow/arithmetic.md capturing the policy and the meaning of a saturated value to downstream consumers.
  • Add tests proving the chosen behaviour at each boundary.

Suggested execution

  • Fork the repo and create a branch
  • git checkout -b security/contracts-24-arithmetic-policy
  • Implement changes
    • Write code in: contracts/escrow/src/lib.rs — explicit arithmetic with rationale comments.
    • Write comprehensive tests in: contracts/escrow/src/test.rs — boundary behaviour per site.
    • Add documentation: add docs/escrow/arithmetic.md.
    • Include NatSpec-style doc comments (///) matching the existing style in lib.rs.
    • Validate security: no silent wrap, saturation cannot hide accounting errors.
  • Test and commit

Test and commit

  • Run cargo fmt --all -- --check, cargo build, and cargo test.
  • Cover edge cases: max-boundary inputs for each operation, zero operands.
  • Include the full cargo test output and a short security notes section in the PR description.

Example commit message

security: audit and document arithmetic overflow/saturation strategy

Guidelines

  • Minimum 95 percent test coverage for impacted modules.
  • Clear, reviewer-focused documentation.
  • Timeframe: 96 hours.

Community & contribution rewards

  • 💬 Join the AgentPay community on Discord for questions, reviews, and faster merges: https://discord.gg/eXvRKkgcv
  • ⭐ This is a GrantFox OSS / Official Campaign task and may be rewarded. When your PR is merged you'll be prompted to rate the project — if this issue and the maintainers helped you ship, we'd be grateful for a 5-star rating. Clear questions in Discord and tidy, well-tested PRs are the fastest path to a merge and a reward.

Metadata

Metadata

Assignees

No one assigned
    No fields configured for Feature.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions