From e97dddc1bb19fc06c32eea5e91874d42dedd025d Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Sun, 26 Jul 2026 15:11:52 +0300 Subject: [PATCH] fix(wallet): resolve EVM contract asset before the cap check; free capMu during settler calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- pkg/wallet/hooks_evm.go | 28 ++++- pkg/wallet/hooks_settler.go | 23 +++- pkg/wallet/spendcap.go | 36 ++++++ pkg/wallet/wallet.go | 8 ++ pkg/wallet/zz_cap_boundary_test.go | 169 +++++++++++++++++++++++++++++ pkg/walletipc/dispatcher_evm.go | 3 + 6 files changed, 263 insertions(+), 4 deletions(-) create mode 100644 pkg/wallet/zz_cap_boundary_test.go diff --git a/pkg/wallet/hooks_evm.go b/pkg/wallet/hooks_evm.go index c402371..24aa760 100644 --- a/pkg/wallet/hooks_evm.go +++ b/pkg/wallet/hooks_evm.go @@ -259,25 +259,49 @@ func (w *Wallet) SatisfyEVM(ctx context.Context, c payment.Contract) (payment.Re return w.SatisfyEVMOn(ctx, w.evm.chainID, c) } +// DefaultEVMAsset is the asset an EVM payment contract denominates in +// when it leaves the Asset field empty. The EVM signer only ever +// produces EIP-3009 authorizations against the chain's USDC contract, +// so an unset Asset means USDC. +const DefaultEVMAsset Asset = "USDC" + +// NormalizeEVMAsset resolves a contract's Asset field to the symbol the +// EVM signer actually operates on. An empty field resolves to +// DefaultEVMAsset; anything else is passed through untouched (the +// signer rejects assets it can't produce). Cap accounting and the +// signer must agree on this value, so both sides call through here. +func NormalizeEVMAsset(asset string) Asset { + if asset == "" { + return DefaultEVMAsset + } + return Asset(asset) +} + // SatisfyEVMOn is the multichain entry point. The caller passes the // chain id explicitly — the IPC dispatcher exposes this as an // optional `chain_id` field on wallet.evm.satisfy. Caps are checked // against the same wallet-wide spendLog as SatisfyEVM and Pay so a // multichain wallet can't dodge a cap by switching chains. +// +// The contract's Asset is resolved through NormalizeEVMAsset before +// either the cap check or the signer sees it, so both accounts for the +// same asset regardless of how the caller spelled it. func (w *Wallet) SatisfyEVMOn(ctx context.Context, chainID uint64, c payment.Contract) (payment.Receipt, error) { binding := w.evmByChain[chainID] if binding == nil { return payment.Receipt{}, fmt.Errorf("wallet.evm.satisfy: chain %d not configured", chainID) } + asset := NormalizeEVMAsset(c.Asset) + c.Asset = string(asset) w.capMu.Lock() defer w.capMu.Unlock() - if err := w.checkSpendCapLocked(Asset(c.Asset), Amount(c.Amount)); err != nil { + if err := w.checkSpendCapLocked(asset, Amount(c.Amount)); err != nil { return payment.Receipt{}, err } receipt, err := binding.method.Satisfy(ctx, c) if err != nil { return payment.Receipt{}, err } - w.recordSpendLocked(Asset(c.Asset), Amount(c.Amount)) + w.recordSpendLocked(asset, Amount(c.Amount)) return receipt, nil } diff --git a/pkg/wallet/hooks_settler.go b/pkg/wallet/hooks_settler.go index 37e856b..7e4d8c5 100644 --- a/pkg/wallet/hooks_settler.go +++ b/pkg/wallet/hooks_settler.go @@ -61,16 +61,35 @@ func (w *Wallet) SettlerTransfer( } a := Asset(asset) amt := Amount(amount) + + // Check and claim the budget in one capMu acquisition, then drop + // the lock for the settler round-trip so a slow or hung settler + // doesn't stall every other cap-gated path on this wallet. The + // reservation keeps the check-then-record pair atomic: it already + // counts against the cap, so a concurrent caller can't be granted + // budget this transfer is holding. w.capMu.Lock() - defer w.capMu.Unlock() if err := w.checkSpendCapLocked(a, amt); err != nil { + w.capMu.Unlock() return settlerclient.Transaction{}, err } + reservation := w.reserveSpendLocked(a, amt) + w.capMu.Unlock() + tx, err := w.settler.Transfer(ctx, w.signer, to, asset, amount, memo, expiresIn) + + w.capMu.Lock() + w.releaseReservationLocked(reservation) + if err == nil { + // Settled: convert the reservation into a permanent (and, when + // a cap-state file is configured, durable) record. + w.recordSpendLocked(a, amt) + } + w.capMu.Unlock() + if err != nil { return settlerclient.Transaction{}, err } - w.recordSpendLocked(a, amt) return tx, nil } diff --git a/pkg/wallet/spendcap.go b/pkg/wallet/spendcap.go index db235b2..a13c7f5 100644 --- a/pkg/wallet/spendcap.go +++ b/pkg/wallet/spendcap.go @@ -85,6 +85,8 @@ func (w *Wallet) SpentInWindow(asset Asset, window time.Duration) Amount { } // spentInWindowLocked is the internal accumulator; w.capMu must be held. +// In-flight reservations count alongside settled records so budget that +// has been handed out but not yet confirmed can't be handed out twice. func (w *Wallet) spentInWindowLocked(asset Asset, window time.Duration) Amount { cutoff := w.clock().Add(-window) var total Amount @@ -97,9 +99,43 @@ func (w *Wallet) spentInWindowLocked(asset Asset, window time.Duration) Amount { } total += r.amount } + for _, r := range w.capReservations { + if r.asset != asset { + continue + } + if r.at.Before(cutoff) { + continue + } + total += r.amount + } return total } +// reserveSpendLocked books an in-flight spend and returns its handle. +// Called under the same capMu acquisition as checkSpendCapLocked so the +// check and the claim are one atomic step: the lock can then be dropped +// for a slow remote call without a second caller seeing stale budget. +// Every reservation must be handed back to releaseReservationLocked. +// w.capMu must be held. +func (w *Wallet) reserveSpendLocked(asset Asset, amount Amount) uint64 { + if w.capReservations == nil { + w.capReservations = make(map[uint64]spendRecord) + } + w.capReservationSeq++ + id := w.capReservationSeq + w.capReservations[id] = spendRecord{at: w.clock(), asset: asset, amount: amount} + return id +} + +// releaseReservationLocked drops a reservation booked by +// reserveSpendLocked. Callers that went on to succeed follow it with +// recordSpendLocked, which makes the spend permanent (and durable); +// callers that failed simply release, returning the budget. +// w.capMu must be held. +func (w *Wallet) releaseReservationLocked(id uint64) { + delete(w.capReservations, id) +} + // pruneSpendLogLocked drops records older than the longest configured // window, since anything older can never affect a future cap check. // w.capMu must be held. diff --git a/pkg/wallet/wallet.go b/pkg/wallet/wallet.go index 27969da..fdcb246 100644 --- a/pkg/wallet/wallet.go +++ b/pkg/wallet/wallet.go @@ -105,6 +105,14 @@ type Wallet struct { // each append links to the prior record. Both guarded by capMu. capStateHMACKey []byte capStateLastHMAC []byte + // capReservations holds spends that have passed the cap check but + // whose outcome is not yet known — the remote call is still in + // flight. They count toward spentInWindowLocked so a concurrent + // caller cannot be granted budget an in-flight spend has already + // claimed. capReservationSeq issues the handles. Both guarded by + // capMu. + capReservations map[uint64]spendRecord + capReservationSeq uint64 } // New returns a wallet bound to a pilot address, a signer, and a Store. diff --git a/pkg/wallet/zz_cap_boundary_test.go b/pkg/wallet/zz_cap_boundary_test.go new file mode 100644 index 0000000..2609f8f --- /dev/null +++ b/pkg/wallet/zz_cap_boundary_test.go @@ -0,0 +1,169 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package wallet + +import ( + "context" + "crypto/ed25519" + "crypto/rand" + "errors" + "net" + "testing" + "time" + + "github.com/pilot-protocol/app-store/pkg/payment" + "github.com/pilot-protocol/wallet/pkg/evm" + "github.com/pilot-protocol/wallet/pkg/settlerclient" +) + +// An x402 contract that omits Asset is denominated in the chain's USDC +// — that is the only token the EVM signer will authorize. The cap +// check must resolve it the same way the signer does, otherwise a +// USDC-scoped cap never matches and never fires. +func TestSatisfyEVMEmptyAssetIsCappedAsUSDC(t *testing.T) { + w := newDualWallet(t) + w.SetSpendCaps(SpendCap{Asset: "USDC", Limit: 1_000_000, Window: 24 * time.Hour}) + + to, err := evm.ParseAddress("0x000000000000000000000000000000000000bEEF") + if err != nil { + t.Fatalf("parse addr: %v", err) + } + c := payment.Contract{ + ID: "ctr-empty-asset", + Amount: 5_000_000, // 5x the cap + Asset: "", // omitted → USDC + RecipientAddr: to.Hex(), + ExpiresAt: time.Now().Add(time.Minute), + Nonce: "ctr-empty-asset-nonce", + } + if _, err := w.SatisfyEVM(context.Background(), c); !errors.Is(err, ErrSpendCapExceeded) { + t.Fatalf("over-cap empty-asset satisfy err = %v, want ErrSpendCapExceeded", err) + } + if used := w.SpentInWindow("USDC", 24*time.Hour); used != 0 { + t.Fatalf("rejected satisfy consumed budget: used=%d, want 0", used) + } +} + +// A within-cap empty-asset satisfy must book its spend against USDC so +// the next one sees the reduced budget. +func TestSatisfyEVMEmptyAssetRecordsAgainstUSDC(t *testing.T) { + w := newDualWallet(t) + w.SetSpendCaps(SpendCap{Asset: "USDC", Limit: 1_500_000, Window: 24 * time.Hour}) + + to, err := evm.ParseAddress("0x000000000000000000000000000000000000bEEF") + if err != nil { + t.Fatalf("parse addr: %v", err) + } + mk := func(id string) payment.Contract { + return payment.Contract{ + ID: id, + Amount: 1_000_000, + Asset: "", + RecipientAddr: to.Hex(), + ExpiresAt: time.Now().Add(time.Minute), + Nonce: id, + } + } + if _, err := w.SatisfyEVM(context.Background(), mk("ctr-1")); err != nil { + t.Fatalf("first satisfy: %v", err) + } + if used := w.SpentInWindow("USDC", 24*time.Hour); used != 1_000_000 { + t.Fatalf("spend not booked against USDC: used=%d, want 1000000", used) + } + if _, err := w.SatisfyEVM(context.Background(), mk("ctr-2")); !errors.Is(err, ErrSpendCapExceeded) { + t.Fatalf("second satisfy err = %v, want ErrSpendCapExceeded", err) + } +} + +// hangingSettler accepts connections and never answers, so a call +// stays outstanding until its deadline fires. +func hangingSettler(t *testing.T) string { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + conns := make(chan net.Conn, 8) + go func() { + for { + c, err := ln.Accept() + if err != nil { + return + } + conns <- c + } + }() + t.Cleanup(func() { + _ = ln.Close() + close(conns) + for c := range conns { + _ = c.Close() + } + }) + return ln.Addr().String() +} + +// The settler round-trip must not be made while capMu is held: other +// cap-gated paths (and plain introspection) have to stay responsive +// while a transfer is outstanding. The in-flight amount still has to +// count against the cap, so check-and-claim remains a single atomic +// step and a second transfer can't spend the same budget. +func TestSettlerTransferReleasesCapLockDuringRoundTrip(t *testing.T) { + s, err := NewLocalSigner() + if err != nil { + t.Fatalf("signer: %v", err) + } + w := NewInMemory(addrBob, s) + defer w.Close() + w.SetSpendCaps(SpendCap{Asset: "USDC", Limit: 100, Window: 24 * time.Hour}) + + spub, _, _ := ed25519.GenerateKey(rand.Reader) + w.SetSettler(settlerclient.New(hangingSettler(t), spub)) + + to := make([]byte, ed25519.PublicKeySize) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + inflight := make(chan error, 1) + go func() { + _, err := w.SettlerTransfer(ctx, to, "USDC", 60, "", 0) + inflight <- err + }() + + // Wait for the reservation to be booked, which happens before the + // round-trip starts. SpentInWindow takes capMu, so this also proves + // the lock is available while the call is outstanding. + deadline := time.Now().Add(3 * time.Second) + for { + observed := make(chan Amount, 1) + go func() { observed <- w.SpentInWindow("USDC", 24*time.Hour) }() + var used Amount + select { + case used = <-observed: + case <-time.After(2 * time.Second): + t.Fatal("SpentInWindow blocked: capMu is held across the settler round-trip") + } + if used == 60 { + break + } + if time.Now().After(deadline) { + t.Fatalf("in-flight transfer never reserved its budget: used=%d, want 60", used) + } + time.Sleep(5 * time.Millisecond) + } + + // The outstanding 60 must gate a second transfer: 60+60 > 100. + shortCtx, shortCancel := context.WithTimeout(context.Background(), 2*time.Second) + defer shortCancel() + if _, err := w.SettlerTransfer(shortCtx, to, "USDC", 60, "", 0); !errors.Is(err, ErrSpendCapExceeded) { + t.Fatalf("second transfer err = %v, want ErrSpendCapExceeded (in-flight budget double-spent)", err) + } + + // The first transfer never lands, so its reservation is returned. + if err := <-inflight; err == nil { + t.Fatal("transfer against a silent settler succeeded, want a network error") + } + if used := w.SpentInWindow("USDC", 24*time.Hour); used != 0 { + t.Fatalf("failed transfer kept its reservation: used=%d, want 0", used) + } +} diff --git a/pkg/walletipc/dispatcher_evm.go b/pkg/walletipc/dispatcher_evm.go index 16805bd..1e88001 100644 --- a/pkg/walletipc/dispatcher_evm.go +++ b/pkg/walletipc/dispatcher_evm.go @@ -115,6 +115,9 @@ func evmSatisfyHandler(w *wallet.Wallet) ipc.Handler { return nil, fmt.Errorf("decode satisfy req: %w", err) } chainID := chainOrPrimary(w, inner.ChainID) + // Resolve the contract's asset at the boundary so the cap + // accounting and the signer operate on the same symbol. + inner.Contract.Asset = string(wallet.NormalizeEVMAsset(inner.Contract.Asset)) // Route through Wallet.SatisfyEVMOn so the same rolling-window // spend cap that gates Pay also gates on-chain receipts, even // when the caller targets a non-primary chain.