Skip to content

fix(settlement): checked arithmetic for credited balances#222

Merged
greatest0fallt1me merged 4 commits intoCalloraOrg:mainfrom
Agbasimere:fix/settlement-checked-math
Mar 31, 2026
Merged

fix(settlement): checked arithmetic for credited balances#222
greatest0fallt1me merged 4 commits intoCalloraOrg:mainfrom
Agbasimere:fix/settlement-checked-math

Conversation

@Agbasimere
Copy link
Copy Markdown
Contributor

Closes: #134

Summary

This pull request replaces naive i128 balance updates (using += and -=) with checked arithmetic in both the callora-vault and the newly renamed callora-settlement contract (formerly revenue_pool). This ensures any overflow or underflow results in an immediate panic and transaction revert, preventing silent state corruption.

Key Changes

  • Contract Renaming: Renamed contracts/revenue_pool to contracts/settlement and updated the crate name to callora-settlement.
  • Checked Arithmetic:
    • Replaced meta.balance += amount in vault/src/lib.rs with checked_add().expect("balance overflow").
    • Replaced meta.balance -= amount in vault/src/lib.rs with checked_sub().expect("balance underflow").
    • Replaced total_amount += amount in settlement/src/lib.rs with checked_add().expect("total amount overflow").
  • Boundary Tests: Added explicit overflow and underflow tests to vault/src/test.rs and settlement/src/test.rs to verify panic behavior.
  • Documentation:
    • Created SECURITY.md documenting the checked arithmetic policy.
    • Updated README.md, INVARIANTS.md, and .github/workflows/ci.yml for the contract rename.
  • Workspace Compliance: Ensured overflow-checks = true is explicitly documented and enforced for both dev and release profiles in root Cargo.toml.

Security Notes

All balance mutations now utilize explicit checked arithmetic (checked_add/checked_sub). In the event of an overflow, the contract will panic with a descriptive message, providing an additional layer of security beyond the default workspace policy.

Test Results

  • cargo fmt: Passed.
  • cargo clippy: Passed with zero warnings.
  • cargo test: Added deposit_overflow_panics and batch_distribute_overflow_panics tests; all related tests passing.

@drips-wave
Copy link
Copy Markdown

drips-wave bot commented Mar 30, 2026

@Agbasimere Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@Agbasimere Agbasimere force-pushed the fix/settlement-checked-math branch from 9b451e1 to efe5e1a Compare March 30, 2026 14:51
@Agbasimere Agbasimere force-pushed the fix/settlement-checked-math branch from efe5e1a to 13f601e Compare March 30, 2026 15:10
…ked-math

# Conflicts:
#	Cargo.toml
#	SECURITY.md
#	contracts/settlement/Cargo.toml
#	contracts/settlement/src/lib.rs
#	contracts/settlement/src/test.rs
#	contracts/settlement/src/test_balance.rs
#	contracts/vault/src/lib.rs
#	contracts/vault/src/test.rs
@Agbasimere
Copy link
Copy Markdown
Contributor Author

@greatest0fallt1me Kindly review.

@greatest0fallt1me greatest0fallt1me merged commit 65958d0 into CalloraOrg:main Mar 31, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Settlement: safe i128 balance updates (no silent wrap)

2 participants