fix(wallet): H2 empty-asset cap bypass; M16 capMu held across settler round-trip - #29
Merged
Conversation
…pMu during settler calls An x402 contract that leaves Asset empty is denominated in the chain's USDC — the only token the EVM signer will authorize. The cap check compared the raw field, so a USDC-scoped cap never matched an empty-asset contract and the spend went through uncounted. Asset now resolves through NormalizeEVMAsset at the IPC boundary and again in SatisfyEVMOn, so cap accounting and the signer see the same symbol. SettlerTransfer held capMu across the settler round-trip, so a slow or unresponsive settler stalled every other cap-gated path on the wallet. The check now claims its budget as an in-flight reservation under the same lock acquisition, releases the lock for the round-trip, then either promotes the reservation to a recorded spend or returns it. Reservations count toward spentInWindow, so check-and-claim stays atomic and a concurrent caller cannot be granted budget an outstanding transfer already holds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses findings H2 and M16 from the audit register.
H2 — empty-asset x402 contract bypassed the USDC spend cap
pkg/wallet/hooks_evm.goAn x402
payment.Contractthat leavesAssetempty is denominated in the chain's USDC — that is the only tokenEVMMethod.Satisfywill ever authorize (it accepts""and signs against the USDC contract). The cap check compared the raw field, so aSpendCap{Asset: "USDC"}never matched and the spend was neither gated nor recorded.NormalizeEVMAssetresolves the field once, and is applied both at the IPC boundary (walletipc.evmSatisfyHandler) and insideSatisfyEVMOn, so the cap check, the recorded spend and the signer all see the same symbol regardless of caller.M16 —
capMuheld across the settler network round-trippkg/wallet/hooks_settler.goSettlerTransferheldcapMufor the whole settler call, so a slow or unresponsive settler stalled every other cap-gated path on the wallet (Pay,SatisfyEVM, evenSpendCaps()).Simply dropping the lock around the call would reintroduce a TOCTOU — two concurrent transfers could each pass a check against the same budget. Instead the check now claims its budget as an in-flight reservation under the same lock acquisition; the lock is released for the round-trip; then the reservation is either promoted to a recorded spend (success) or returned (failure). Reservations count toward
spentInWindowLocked, so check-and-claim stays atomic.Tests
pkg/wallet/zz_cap_boundary_test.go— all three fail onmain, pass here:SpentInWindowstays responsive while a transfer is outstanding, the in-flight amount gates a concurrent transfer, and a failed transfer returns its reservationNo wire-format or protocol changes.
🤖 Generated with Claude Code