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.