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.
Harden arithmetic safety and document the overflow strategy
Description
Cargo.tomlsetsoverflow-checks = trueon the release profile, whilecontracts/escrow/src/lib.rsdeliberately usessaturating_*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
Agentpay-Org/Agentpay-contractsonly.saturating_*) and not relying on the build profile; where overflow must be impossible, document why.docs/escrow/arithmetic.mdcapturing the policy and the meaning of a saturated value to downstream consumers.Suggested execution
git checkout -b security/contracts-24-arithmetic-policycontracts/escrow/src/lib.rs— explicit arithmetic with rationale comments.contracts/escrow/src/test.rs— boundary behaviour per site.docs/escrow/arithmetic.md.///) matching the existing style inlib.rs.Test and commit
cargo fmt --all -- --check,cargo build, andcargo test.cargo testoutput and a short security notes section in the PR description.Example commit message
security: audit and document arithmetic overflow/saturation strategyGuidelines
Community & contribution rewards