Skip to content

feat(treasury): fix multi-token deposit balance accounting gap - #510

Merged
misrasamuelisiguzor-oss merged 2 commits into
WHEELBACK:mainfrom
AMV-AUTOS:feat/treasury-multi-token-accounting-audit
Aug 29, 2026
Merged

feat(treasury): fix multi-token deposit balance accounting gap#510
misrasamuelisiguzor-oss merged 2 commits into
WHEELBACK:mainfrom
AMV-AUTOS:feat/treasury-multi-token-accounting-audit

Conversation

@AMV-AUTOS

@AMV-AUTOS AMV-AUTOS commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Closes Add multi-token treasury support with a documented per-token accounting model #448.
  • Audited contracts/treasury/src/deposits.rs and settlements.rs as requested. Confirmed a genuine gap, not just a hypothetical one: DataKey::Balance was keyed only by holder address (Balance(Address)), so deposit/batch_deposit/withdraw/get_balance pooled deposits from every concurrently-allowlisted token into one shared balance per address — e.g. depositing 100 USDC then 100 EURC read back as a combined 200, and a withdrawal of either token drew from that mixed total. TokenAllowlist already permits multiple simultaneously-allowed tokens, so this was reachable in normal use.
  • Fixed by re-keying storage as DataKey::Balance(holder, token_contract) in crates/multisig/src/lib.rs, and updating deposit_one/withdraw/get_balance in deposits.rs to read/write the per-(holder, token) bucket. get_balance's public signature now takes an explicit token_contract parameter.
  • Note: execute_settlement/partially_execute_settlement never touch this ledger — they pay merchants directly out of the treasury's on-chain token balance via token::Client::transfer, so the deposit ledger and the settlement flow are independent accounting paths by design. Documented this and the verified per-token isolation in contracts/treasury/README.md.
  • Updated the existing call sites in treasury_deposit_withdraw_roundtrip_test.rs and reentrancy_suite/{deposit,withdraw}.rs to the new get_balance signature so the crate keeps compiling; no new test coverage was added per the task scope for this batch.

Test plan

  • Manual review: confirmed DataKey::Balance is only referenced within contracts/treasury/src/deposits.rs, so the storage-key shape change is contained
  • Manual review: confirmed all existing get_balance/DataKey::Balance call sites across the repo were updated to the new two-argument shape
  • cargo test --all, cargo clippy -- -D warnings, cargo fmt --all -- --check (not run locally — no Rust toolchain available in this environment; CI will verify)

Closes WHEELBACK#448.

Audited contracts/treasury/src/deposits.rs and settlements.rs. Found a
genuine gap: DataKey::Balance was keyed only by holder address
(Balance(Address)), not by token contract, so deposit/batch_deposit/
withdraw/get_balance silently pooled deposits from every
concurrently-allowlisted token into one shared balance per address —
depositing 100 USDC then 100 EURC read back as 200, and a withdrawal of
either token drew from that mixed total. TokenAllowlist already permits
multiple simultaneously-allowed tokens, so this was reachable in normal
use, not just a hypothetical.

Fixed by re-keying storage as DataKey::Balance(holder, token_contract) in
crates/multisig/src/lib.rs, and updating deposit_one/withdraw/get_balance
in deposits.rs to read/write the per-(holder, token) bucket. get_balance's
public signature now takes an explicit token_contract parameter; updated
the existing call sites in treasury_deposit_withdraw_roundtrip_test.rs and
reentrancy_suite/{deposit,withdraw}.rs to match, without adding new test
coverage. Documented the verified per-token isolation, and the fact that
execute_settlement/partially_execute_settlement never touch this ledger
(they pay merchants straight out of the treasury's on-chain token
balance), in contracts/treasury/README.md.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NpeEK1u9YTwWabX64LfM43
@drips-wave

drips-wave Bot commented Aug 29, 2026

Copy link
Copy Markdown

@AMV-AUTOS 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

@misrasamuelisiguzor-oss
misrasamuelisiguzor-oss merged commit 7766fbb into WHEELBACK:main Aug 29, 2026
5 of 14 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.

Add multi-token treasury support with a documented per-token accounting model

2 participants