Skip to content

@x402r/evm: implement authCapture (rename + new wire format + autoCapture + Permit2)#40

Merged
A1igator merged 22 commits intomainfrom
A1igator/authcapture-rename
May 6, 2026
Merged

@x402r/evm: implement authCapture (rename + new wire format + autoCapture + Permit2)#40
A1igator merged 22 commits intomainfrom
A1igator/authcapture-rename

Conversation

@A1igator
Copy link
Copy Markdown
Contributor

@A1igator A1igator commented Apr 28, 2026

Spec moved to a separate PR: #42. This PR is now strictly the @x402r/evm package implementation. Read #42 first for the protocol surface; this PR is the code that implements it.

Two-commit PR landing the full authCapture migration in the scheme package.

Commit 1: rename commerceauthCapture (mechanical, identifier-only)

  • Source dir packages/evm/src/commerce/src/authCapture/
  • Tests packages/evm/test/unit/commerce/test/unit/authCapture/
  • Spec docs specs/schemes/commerce/scheme_commerce*.mdspecs/schemes/authCapture/scheme_authCapture*.md
  • tsup entrypoints + package.json exports subpaths (./commerce/*./authCapture/*)
  • Identifier renames: scheme constant, classes (*EvmScheme, *ServerScheme, *FacilitatorScheme), types (*Payload, *Extra), type guards, helpers (computeCommerceNonce, registerCommerceEvmScheme), error codes
  • COMMERCE_PAYMENTS_* constants and commerce-payments / Base Commerce Payments prose kept verbatim — they refer to the underlying base/commerce-payments protocol, not our scheme

Commit 2: new wire format, salt on payload, autoCapture, Permit2

extra shape

Drop:

  • escrowAddress, operatorAddress, tokenCollector — universal constants now
  • preApprovalExpirySeconds — client derives from top-level maxTimeoutSeconds
  • authorizationExpirySeconds, refundExpirySeconds — replaced by absolute deadlines
  • settlementMethod — replaced by autoCapture bool
  • feeReceiver — renamed feeRecipient

Add:

  • captureAuthorizer: address (formerly operatorAddress)
  • captureDeadline: uint48 / refundDeadline: uint48 (absolute Unix seconds)
  • feeRecipient: address
  • autoCapture?: bool (default false)
  • assetTransferMethod?: 'eip3009' | 'permit2' (default 'eip3009')

Spec field names live at the wire layer; the on-chain PaymentInfo struct keeps canonical Solidity field names (operator, authorizationExpiry, refundExpiry, feeReceiver) so the EIP-712 typehash matches AuthCaptureEscrow.PAYMENT_INFO_TYPEHASH byte-for-byte.

salt placement

Moved out of extra and onto PaymentPayload. Generated client-side at every createPaymentPayload() call, fresh per request. Facilitator reconstructs PaymentInfo from payload.salt + extra + payer + top-level requirements. Freshness is mandatory because the payer-agnostic nonce zeroes the payer field — two payers under identical extra+salt would collide.

Constants module

Renames: COMMERCE_PAYMENTS_ESCROWAUTH_CAPTURE_ESCROW_ADDRESS, COMMERCE_PAYMENTS_TOKEN_COLLECTOREIP3009_TOKEN_COLLECTOR_ADDRESS. New: PERMIT2_TOKEN_COLLECTOR_ADDRESS, PERMIT2_ADDRESS, PERMIT2_TRANSFER_FROM_TYPES, ESCROW_ABI (was OPERATOR_ABI — the facilitator now calls AuthCaptureEscrow directly).

AuthCaptureFacilitatorScheme.getExtra() now returns undefined: there's nothing facilitator-injected to advertise — addresses are constants, captureAuthorizer/feeRecipient/deadlines are merchant-set.

Settle dispatch

extra.autoCapture === true   → AuthCaptureEscrow.charge()    (atomic)
extra.autoCapture !== true   → AuthCaptureEscrow.authorize() (two-phase)

Permit2 path

New Permit2Payload variant in a discriminated union. Client signs Uniswap Permit2 PermitTransferFrom (no witness — merchant address is bound through the deterministic payer-agnostic nonce). Facilitator dispatches on assetTransferMethod; Permit2 collectorData is the ABI-encoded signature.

Payer-zeroed nonce

computePayerAgnosticPaymentInfoHash() zeroes the payer field before hashing. Same nonce regardless of payer; per-request freshness comes entirely from salt. Facilitator recomputes and asserts the wire nonce matches before settling.

Tests

pnpm test — 122 passing across 6 files. Coverage:

  • payer-agnosticism (different payers produce identical nonces)
  • salt freshness (consecutive createPaymentPayload calls produce distinct salts)
  • autoCapture: true/false/undefined settle routing
  • Permit2 payload construction + facilitator settle target
  • collector-mismatch (wrong to / spender)
  • payload-method-mismatch (Permit2 payload with assetTransferMethod: 'eip3009' and vice-versa)
  • deadline ordering enforcement

Spec docs (in lockstep)

specs/schemes/authCapture/scheme_authCapture.md and scheme_authCapture_evm.md rewritten to match the implementation. The EVM doc has full extra-field tables, both EIP-3009 and Permit2 payload shapes, the spec→on-chain field-name mapping table, payer-zeroed nonce derivation, full verification step list, and settle dispatch.

Public API break

Subpath imports change: @x402r/evm/commerce/*@x402r/evm/authCapture/*. Wire format breaks: any caller building extra with old field names won't deserialize. Acceptable at 0.0.x. Downstream consumers (@x402r/core, examples) need lockstep updates in follow-up PRs.

Test plan

  • pnpm typecheck clean
  • pnpm test — 122/122 passing
  • pnpm build — tsup ESM + CJS + DTS clean
  • pnpm format clean
  • pnpm lint:check clean
  • End-to-end Sepolia validation against canonical AuthCaptureEscrow deploy (separate PR once deploy addresses are pinned)
  • Update downstream consumers (@x402r/core, examples, etc.)

🤖 Generated with Claude Code

A1igator and others added 2 commits April 27, 2026 22:27
Mechanical, identifier-only rename. No behavior change.

- Source dir: packages/evm/src/commerce/ -> src/authCapture/
- Tests: packages/evm/test/unit/commerce/ -> test/unit/authCapture/
- Spec docs: specs/schemes/commerce/ -> specs/schemes/authCapture/ (files renamed too)
- tsup entrypoints + package.json exports subpaths updated
- Identifier renames: scheme constant 'commerce' -> 'authCapture',
  CommerceEvmScheme/CommerceServerScheme/CommerceFacilitatorScheme classes,
  CommercePayload/CommerceExtra types + isCommerce* guards,
  computeCommerceNonce, registerCommerceEvmScheme, invalid_commerce_*
- Upstream protocol references kept verbatim: COMMERCE_PAYMENTS_*
  constants, "commerce-payments" / "Base Commerce Payments" prose

The scheme name now reflects the credit-card-style auth/capture pattern
of the underlying AuthCaptureEscrow contract.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Phases B–G of the authCapture migration. Lands the new spec wire format,
canonical address constants, autoCapture settle dispatch, and the Permit2
asset transfer method as a second supported path.

## extra shape (Phase B)

Drop:
- escrowAddress, operatorAddress, tokenCollector (now universal constants)
- preApprovalExpirySeconds (client derives from maxTimeoutSeconds)
- authorizationExpirySeconds, refundExpirySeconds (replaced by absolute deadlines)
- settlementMethod (replaced by autoCapture bool)
- feeReceiver (renamed feeRecipient)

Add:
- captureAuthorizer: address (formerly operatorAddress)
- captureDeadline / refundDeadline: absolute Unix seconds (uint48)
- feeRecipient: address
- autoCapture: bool (default false)
- assetTransferMethod: 'eip3009' | 'permit2' (default 'eip3009')

Spec field names live at the wire layer; the on-chain PaymentInfo struct
keeps canonical Solidity names (operator, authorizationExpiry, refundExpiry,
feeReceiver) so the EIP-712 typehash matches the AuthCaptureEscrow contract
byte-for-byte.

## salt placement

Salt moves out of extra and onto PaymentPayload. Generated client-side at
each createPaymentPayload() call, fresh per request. The facilitator
reconstructs PaymentInfo using payload.salt + extra fields + payer.
Freshness is mandatory: payer is zeroed in the payer-agnostic nonce
derivation, so two payers under an identical extra+salt would collide.

## constants module (Phase C)

- AUTH_CAPTURE_ESCROW_ADDRESS (was COMMERCE_PAYMENTS_ESCROW)
- EIP3009_TOKEN_COLLECTOR_ADDRESS (was COMMERCE_PAYMENTS_TOKEN_COLLECTOR)
- PERMIT2_TOKEN_COLLECTOR_ADDRESS (new)
- PERMIT2_ADDRESS (new — canonical Uniswap Permit2)
- ESCROW_ABI (was OPERATOR_ABI; calls now target AuthCaptureEscrow directly)
- PERMIT2_TRANSFER_FROM_TYPES (new — EIP-712 types for Permit2)

Facilitator's getExtra() now returns undefined: addresses are constants,
captureAuthorizer/feeRecipient/deadlines are merchant-set.

## settle dispatch (Phase E)

extra.autoCapture === true   → AuthCaptureEscrow.charge() (atomic)
extra.autoCapture !== true   → AuthCaptureEscrow.authorize() (two-phase)

## Permit2 path (Phase F)

New Permit2Payload variant in the discriminated union. Client signs Uniswap
Permit2 PermitTransferFrom (no witness — merchant address bound through the
deterministic nonce). Facilitator dispatches on assetTransferMethod, passes
PERMIT2_TOKEN_COLLECTOR + ABI-encoded signature as collectorData.

## payer-zeroed nonce

computePayerAgnosticPaymentInfoHash() zeroes the payer field before hashing.
Same nonce regardless of payer; per-request freshness comes from salt.
Facilitator recomputes and asserts wire nonce matches before settling.

## Tests

71 passing (was 64): added payer-agnosticism, salt freshness, autoCapture
routing, Permit2 payload, deadline ordering, collector-mismatch, and
payload-method-mismatch coverage.

## Spec docs (in lockstep)

Both scheme_authCapture.md and scheme_authCapture_evm.md rewritten to match
the new wire format. authCapture.md describes the two-phase / single-shot
distinction via autoCapture; the EVM doc has full extra-field tables, both
payload shapes, nonce derivation, verification + settle steps, and the
spec→on-chain field-name mapping table.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@A1igator A1igator changed the title Rename scheme: commerce -> authCapture Adopt authCapture spec: rename, new wire format, autoCapture, Permit2 Apr 28, 2026
@vraspar
Copy link
Copy Markdown
Contributor

vraspar commented Apr 28, 2026

Scheme Review — PR #40 (A1igator/authcapture-rename)

Found 9 issues (axes: contract correctness, tests, conventions, spec/upstream alignment). Verified upstream against lib/commerce-payments/src/AuthCaptureEscrow.sol, viem source, and x402-foundation/x402 PR #1425 thread (most recent comment 2026-04-28).

  1. [CRITICAL — Contract correctness] ESCROW_ABI.charge declares 4 args (tuple, uint256, address, bytes) but the on-chain AuthCaptureEscrow.charge is 6 args (adds uint16 feeBps, address feeReceiver). Any settle with extra.autoCapture: true will hit a non-existent selector and revert. The masked simulation (refactor: use x402 types and remove custom facilitator signer #3) hides this completely. Upstream signature: lib/commerce-payments/src/AuthCaptureEscrow.sol:200-207.

    {
    name: 'charge',
    type: 'function',
    stateMutability: 'nonpayable',
    inputs: [
    {
    name: 'paymentInfo',
    type: 'tuple',
    components: PAYMENT_INFO_COMPONENTS,
    },
    { name: 'amount', type: 'uint256' },
    { name: 'tokenCollector', type: 'address' },
    { name: 'collectorData', type: 'bytes' },
    ],
    outputs: [],
    },
    ] as const

  2. [Tests] Missing nonce_mismatch regression test — the payer-agnostic-nonce design's whole security guarantee is that tampering with any PaymentInfo field invalidates the wire nonce. No test mutates salt or extra between sign and verify. A future refactor of reconstructPaymentInfo or computePayerAgnosticPaymentInfoHash could silently let the facilitator accept payloads bound to different PaymentInfos.

    describe('verify — invariants', () => {
    it('should reject when extra is missing required fields', async () => {
    const scheme = new AuthCaptureFacilitatorScheme(mockSigner)
    const bad = {
    ...mockRequirements,
    extra: { name: 'USDC', version: '2' } as unknown as typeof mockRequirements.extra,
    }
    const result = await scheme.verify(buildEip3009Payload(), bad)
    expect(result.isValid).toBe(false)
    expect(result.invalidReason).toBe('invalid_authCapture_extra')
    })
    it('should reject when refundDeadline is not after captureDeadline', async () => {
    const scheme = new AuthCaptureFacilitatorScheme(mockSigner)
    const bad = {
    ...mockRequirements,
    extra: { ...mockRequirements.extra, refundDeadline: captureDeadline - 1 },
    }
    const result = await scheme.verify(buildEip3009Payload(), bad)
    expect(result.isValid).toBe(false)
    expect(result.invalidReason).toBe('invalid_deadline_ordering')
    })
    it('should reject when payload method does not match assetTransferMethod', async () => {
    const scheme = new AuthCaptureFacilitatorScheme(mockSigner)
    const reqs = {
    ...mockRequirements,
    extra: { ...mockRequirements.extra, assetTransferMethod: 'permit2' as const },
    }
    const result = await scheme.verify(buildEip3009Payload(), reqs)
    expect(result.isValid).toBe(false)
    expect(result.invalidReason).toBe('payload_method_mismatch')
    })
    it('should reject when EIP-3009 payload.to is not the canonical collector', async () => {
    const scheme = new AuthCaptureFacilitatorScheme(mockSigner)
    const payload = buildEip3009Payload()
    payload.payload.authorization.to =
    '0x9999999999999999999999999999999999999999' as `0x${string}`
    const result = await scheme.verify(payload, mockRequirements)
    expect(result.isValid).toBe(false)
    expect(result.invalidReason).toBe('token_collector_mismatch')
    })
    })

  3. [Contract correctness] simulateSettle catch-all returns the opaque string 'simulation_failed', dropping the structured revert. Masks issue Add Claude Code GitHub Workflow #1 and conflates PaymentAlreadyCollected / AfterPreApprovalExpiry / InvalidExpiries / ExceedsMaxAmount / fee-bound errors / consumed ERC-3009 nonce. Decode the revert via BaseError.walk() / ContractFunctionRevertedError and branch on the known selectors. Also: readContract works at runtime against the nonpayable but simulateContract is the canonical viem API and returns the typed error.

    private async simulateSettle(
    paymentInfo: PaymentInfoStruct,
    amount: bigint,
    wirePayload: AuthCapturePayload,
    extra: AuthCaptureExtra,
    _payer: `0x${string}`,
    ): Promise<'ok' | string> {
    const assetTransferMethod = extra.assetTransferMethod ?? 'eip3009'
    const { tokenCollector, collectorData } = unpackForSettle(wirePayload, assetTransferMethod)
    const functionName = extra.autoCapture === true ? 'charge' : 'authorize'
    try {
    await this.signer.readContract({
    address: AUTH_CAPTURE_ESCROW_ADDRESS,
    abi: ESCROW_ABI,
    functionName,
    args: [paymentInfoToContractTuple(paymentInfo), amount, tokenCollector, collectorData],
    })
    return 'ok'
    } catch {
    return 'simulation_failed'
    }
    }

  4. [Tests] verify() negative paths almost entirely uncovered — only 4 of 15+ invalidReason codes are tested (invalid_authCapture_extra, invalid_deadline_ordering, payload_method_mismatch one direction, token_collector_mismatch for EIP-3009 only). Untested: nonce_mismatch, amount_mismatch, authorization_expired, authorization_not_yet_valid, unsupported_asset_transfer_method, network_mismatch, invalid_authCapture_signature, simulation_failed, insufficient_balance, token_mismatch, token_collector_mismatch (Permit2). Each is a 5-line mutation of the existing fixture builders.

    async verify(
    payload: PaymentPayload,
    requirements: PaymentRequirements,
    _context?: FacilitatorContext,
    ): Promise<VerifyResponse> {
    if (!isAuthCapturePayload(payload.payload)) {
    return { isValid: false, invalidReason: 'invalid_payload_format' }
    }
    const wirePayload = payload.payload as AuthCapturePayload
    const payer = isEip3009Payload(wirePayload)
    ? wirePayload.authorization.from
    : (wirePayload as Permit2Payload).permit2Authorization.from
    if (payload.accepted.scheme !== 'authCapture' || requirements.scheme !== 'authCapture') {
    return { isValid: false, invalidReason: 'unsupported_scheme', payer }
    }
    if (payload.accepted.network !== requirements.network) {
    return { isValid: false, invalidReason: 'network_mismatch', payer }
    }
    const networkParts = requirements.network.split(':')
    if (networkParts.length !== 2 || networkParts[0] !== 'eip155') {
    return { isValid: false, invalidReason: 'invalid_network', payer }
    }
    if (!isAuthCaptureExtra(requirements.extra)) {
    return { isValid: false, invalidReason: 'invalid_authCapture_extra', payer }
    }
    const extra = requirements.extra as AuthCaptureExtra
    const chainId = parseChainId(requirements.network)
    const assetTransferMethod = extra.assetTransferMethod ?? 'eip3009'
    if (assetTransferMethod !== 'eip3009' && assetTransferMethod !== 'permit2') {
    return { isValid: false, invalidReason: 'unsupported_asset_transfer_method', payer }
    }
    if (assetTransferMethod === 'eip3009' && !isEip3009Payload(wirePayload)) {
    return { isValid: false, invalidReason: 'payload_method_mismatch', payer }
    }
    if (assetTransferMethod === 'permit2' && !isPermit2Payload(wirePayload)) {
    return { isValid: false, invalidReason: 'payload_method_mismatch', payer }
    }
    const now = Math.floor(Date.now() / 1000)
    const SAFETY_MARGIN_SECONDS = 6
    if (extra.captureDeadline <= now + SAFETY_MARGIN_SECONDS) {
    return { isValid: false, invalidReason: 'capture_deadline_expired', payer }
    }
    if (extra.refundDeadline <= extra.captureDeadline) {
    return { isValid: false, invalidReason: 'invalid_deadline_ordering', payer }
    }
    let preApprovalExpiry: number
    let amount: bigint
    let signatureForVerify: `0x${string}`
    let signatureValid = false
    if (assetTransferMethod === 'eip3009') {
    const eipPayload = wirePayload as Eip3009Payload
    preApprovalExpiry = Number(eipPayload.authorization.validBefore)
    amount = BigInt(eipPayload.authorization.value)
    if (preApprovalExpiry <= now + SAFETY_MARGIN_SECONDS) {
    return { isValid: false, invalidReason: 'authorization_expired', payer }
    }
    if (Number(eipPayload.authorization.validAfter) > now) {
    return { isValid: false, invalidReason: 'authorization_not_yet_valid', payer }
    }
    if (
    eipPayload.authorization.to.toLowerCase() !== EIP3009_TOKEN_COLLECTOR_ADDRESS.toLowerCase()
    ) {
    return { isValid: false, invalidReason: 'token_collector_mismatch', payer }
    }
    const parsed = parseErc6492Signature(eipPayload.signature)
    signatureForVerify = parsed.signature
    signatureValid = await verifyERC3009Signature(
    this.signer,
    eipPayload.authorization,
    signatureForVerify,
    { ...extra, chainId },
    requirements.asset as `0x${string}`,
    )
    } else {
    const permitPayload = wirePayload as Permit2Payload
    preApprovalExpiry = Number(permitPayload.permit2Authorization.deadline)
    amount = BigInt(permitPayload.permit2Authorization.permitted.amount)
    if (preApprovalExpiry <= now + SAFETY_MARGIN_SECONDS) {
    return { isValid: false, invalidReason: 'authorization_expired', payer }
    }
    if (
    permitPayload.permit2Authorization.spender.toLowerCase() !==
    PERMIT2_TOKEN_COLLECTOR_ADDRESS.toLowerCase()
    ) {
    return { isValid: false, invalidReason: 'token_collector_mismatch', payer }
    }
    if (
    permitPayload.permit2Authorization.permitted.token.toLowerCase() !==
    requirements.asset.toLowerCase()
    ) {
    return { isValid: false, invalidReason: 'token_mismatch', payer }
    }
    const parsed = parseErc6492Signature(permitPayload.signature)
    signatureForVerify = parsed.signature
    signatureValid = await verifyPermit2Signature(
    this.signer,
    permitPayload.permit2Authorization,
    signatureForVerify,
    chainId,
    )
    }
    if (!signatureValid) {
    return { isValid: false, invalidReason: 'invalid_authCapture_signature', payer }
    }
    if (amount !== BigInt(requirements.amount)) {
    return { isValid: false, invalidReason: 'amount_mismatch', payer }
    }
    // Reconstruct PaymentInfo and verify the wire nonce matches the
    // payer-agnostic hash. This binds the signature to all PaymentInfo fields.
    const paymentInfo = reconstructPaymentInfo(
    payer,
    preApprovalExpiry,
    wirePayload.salt,
    requirements,
    extra,
    )
    const expectedNonce = computePayerAgnosticPaymentInfoHash(chainId, paymentInfo)
    if (assetTransferMethod === 'eip3009') {
    const wireNonce = (wirePayload as Eip3009Payload).authorization.nonce
    if (wireNonce.toLowerCase() !== expectedNonce.toLowerCase()) {
    return { isValid: false, invalidReason: 'nonce_mismatch', payer }
    }
    } else {
    const wireNonce = BigInt((wirePayload as Permit2Payload).permit2Authorization.nonce)
    if (wireNonce !== hexToBigInt(expectedNonce)) {
    return { isValid: false, invalidReason: 'nonce_mismatch', payer }
    }
    }
    // Simulate the settle call to catch issues before spending gas.
    const settleResult = await this.simulateSettle(paymentInfo, amount, wirePayload, extra, payer)
    if (settleResult !== 'ok') {
    // For balance-related failures, return a more actionable reason.
    try {
    const balance = (await this.signer.readContract({
    address: requirements.asset as `0x${string}`,
    abi: ERC20_BALANCE_OF_ABI,
    functionName: 'balanceOf',
    args: [payer],
    })) as bigint
    if (balance < BigInt(requirements.amount)) {
    return { isValid: false, invalidReason: 'insufficient_balance', payer }
    }
    } catch {
    /* ignore — fall through */
    }
    return { isValid: false, invalidReason: settleResult, payer }
    }
    return { isValid: true, payer }
    }

  5. [Tests] No fork-test infrastructure in packages/evm/test/. Every writeContract / readContract / verifyTypedData / waitForTransactionReceipt is vi.fn().mockResolvedValue(...). The 4-arg charge ABI in Add Claude Code GitHub Workflow #1 passes every existing test. For a package shipping mainnet authorizations, at least one happy-path settle per method against an anvil fork of Base Sepolia should be required.

    const createMockSigner = () => ({
    getAddresses: () => ['0x1234567890123456789012345678901234567890'] as readonly `0x${string}`[],
    readContract: vi.fn().mockResolvedValue(BigInt('1000000000')),
    writeContract: vi.fn().mockResolvedValue('0xabcdef1234567890' as `0x${string}`),
    verifyTypedData: vi.fn().mockResolvedValue(true),
    sendTransaction: vi.fn(),
    waitForTransactionReceipt: vi.fn().mockResolvedValue({ status: 'success' }),
    getCode: vi.fn(),
    })
    let mockSigner: ReturnType<typeof createMockSigner>
    beforeEach(() => {
    vi.clearAllMocks()
    mockSigner = createMockSigner()
    })

  6. [Tests] EIP-712 domain shape never asserted. signTypedData is mocked but the domain object passed into it is never inspected. No test confirms Permit2 signing uses {name:'Permit2', verifyingContract: PERMIT2_ADDRESS} (vs the token's domain) — a recurring bug class for Permit2 implementations. Same gap for EIP-3009 (verifyingContract should be requirements.asset).

    import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
    import { AuthCaptureEvmScheme } from '../../../src/authCapture/client/index'
    import { x402Client } from '@x402/core/client'
    import { registerAuthCaptureEvmScheme } from '../../../src/authCapture/client/index'
    import {
    EIP3009_TOKEN_COLLECTOR_ADDRESS,
    PERMIT2_TOKEN_COLLECTOR_ADDRESS,
    } from '../../../src/authCapture/shared/constants'
    import { isEip3009Payload, isPermit2Payload } from '../../../src/authCapture/shared/types'
    import type { Eip3009Payload, Permit2Payload } from '../../../src/authCapture/shared/types'
    const FUTURE = Math.floor(Date.now() / 1000) + 86400
    describe('AuthCaptureEvmScheme', () => {
    const createMockSigner = () => ({
    address: '0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' as const,
    signTypedData: vi.fn().mockResolvedValue('0xdeadbeef' as `0x${string}`),
    })
    let mockSigner: ReturnType<typeof createMockSigner>
    beforeEach(() => {
    mockSigner = createMockSigner()
    })
    afterEach(() => {
    vi.restoreAllMocks()
    })
    const mockRequirements = {
    scheme: 'authCapture',
    network: 'eip155:84532',
    amount: '1000000',
    asset: '0x036CbD53842c5426634e7929541eC2318f3dCF7e',
    payTo: '0x1234567890123456789012345678901234567890',
    maxTimeoutSeconds: 3600,
    extra: {
    captureAuthorizer: '0xcccccccccccccccccccccccccccccccccccccccc' as `0x${string}`,
    captureDeadline: FUTURE,
    refundDeadline: FUTURE + 86400,
    feeRecipient: '0x4444444444444444444444444444444444444444' as `0x${string}`,
    maxFeeBps: 100,
    name: 'USDC',
    version: '2',
    },
    }
    describe('constructor and properties', () => {
    it('should have scheme set to "authCapture"', () => {
    const scheme = new AuthCaptureEvmScheme(mockSigner)
    expect(scheme.scheme).toBe('authCapture')
    })
    })
    describe('createPaymentPayload — EIP-3009 (default)', () => {
    it('should create a valid EIP-3009 payload for x402Version 2', async () => {
    const scheme = new AuthCaptureEvmScheme(mockSigner)
    const result = await scheme.createPaymentPayload(2, mockRequirements)
    expect(result.x402Version).toBe(2)
    expect(isEip3009Payload(result.payload)).toBe(true)
    const payload = result.payload as unknown as Eip3009Payload
    expect(payload.authorization).toBeDefined()
    expect(payload.signature).toBe('0xdeadbeef')
    expect(payload.salt).toMatch(/^0x[a-fA-F0-9]{64}$/)
    })
    it('should throw for unsupported x402Version', async () => {
    const scheme = new AuthCaptureEvmScheme(mockSigner)
    await expect(scheme.createPaymentPayload(1, mockRequirements)).rejects.toThrow(
    'Unsupported x402Version: 1. Only version 2 is supported.',
    )
    })
    it('should throw for x402Version 0', async () => {
    const scheme = new AuthCaptureEvmScheme(mockSigner)
    await expect(scheme.createPaymentPayload(0, mockRequirements)).rejects.toThrow(
    'Unsupported x402Version: 0',
    )
    })
    it('should throw when EIP-712 name is missing', async () => {
    const scheme = new AuthCaptureEvmScheme(mockSigner)
    const requirementsNoName = {
    ...mockRequirements,
    extra: { ...mockRequirements.extra, name: '' },
    }
    await expect(scheme.createPaymentPayload(2, requirementsNoName)).rejects.toThrow(
    "EIP-712 domain parameter 'name' is required",
    )
    })
    it('should throw when EIP-712 version is missing', async () => {
    const scheme = new AuthCaptureEvmScheme(mockSigner)
    const requirementsNoVersion = {
    ...mockRequirements,
    extra: { ...mockRequirements.extra, version: '' },
    }
    await expect(scheme.createPaymentPayload(2, requirementsNoVersion)).rejects.toThrow(
    "EIP-712 domain parameter 'version' is required",
    )
    })
    it('should throw when captureAuthorizer is missing', async () => {
    const scheme = new AuthCaptureEvmScheme(mockSigner)
    const bad = {
    ...mockRequirements,
    extra: { ...mockRequirements.extra, captureAuthorizer: '' as `0x${string}` },
    }
    await expect(scheme.createPaymentPayload(2, bad)).rejects.toThrow(
    "'captureAuthorizer' is required",
    )
    })
    it('should throw when feeRecipient is missing', async () => {
    const scheme = new AuthCaptureEvmScheme(mockSigner)
    const bad = {
    ...mockRequirements,
    extra: { ...mockRequirements.extra, feeRecipient: '' as `0x${string}` },
    }
    await expect(scheme.createPaymentPayload(2, bad)).rejects.toThrow(
    "'feeRecipient' is required",
    )
    })
    it('should set authorization.from to signer address', async () => {
    const scheme = new AuthCaptureEvmScheme(mockSigner)
    const result = await scheme.createPaymentPayload(2, mockRequirements)
    const payload = result.payload as unknown as Eip3009Payload
    expect(payload.authorization.from).toBe(mockSigner.address)
    })
    it('should set authorization.to to the canonical EIP-3009 token collector', async () => {
    const scheme = new AuthCaptureEvmScheme(mockSigner)
    const result = await scheme.createPaymentPayload(2, mockRequirements)
    const payload = result.payload as unknown as Eip3009Payload
    expect(payload.authorization.to).toBe(EIP3009_TOKEN_COLLECTOR_ADDRESS)
    })
    it('should set authorization.value to requirements amount', async () => {
    const scheme = new AuthCaptureEvmScheme(mockSigner)
    const result = await scheme.createPaymentPayload(2, mockRequirements)
    const payload = result.payload as unknown as Eip3009Payload
    expect(payload.authorization.value).toBe('1000000')
    })
    it('should derive validBefore from now + maxTimeoutSeconds', async () => {
    const fakeNowMs = 1700000000000
    vi.spyOn(Date, 'now').mockReturnValue(fakeNowMs)
    const scheme = new AuthCaptureEvmScheme(mockSigner)
    const result = await scheme.createPaymentPayload(2, {
    ...mockRequirements,
    maxTimeoutSeconds: 600,
    })
    const payload = result.payload as unknown as Eip3009Payload
    expect(payload.authorization.validBefore).toBe('1700000600')
    })
    it('should generate a fresh salt on each call', async () => {
    const scheme = new AuthCaptureEvmScheme(mockSigner)
    const a = (await scheme.createPaymentPayload(2, mockRequirements))
    .payload as unknown as Eip3009Payload
    const b = (await scheme.createPaymentPayload(2, mockRequirements))
    .payload as unknown as Eip3009Payload
    expect(a.salt).not.toBe(b.salt)
    })
    it('should throw for invalid network format', async () => {
    const scheme = new AuthCaptureEvmScheme(mockSigner)
    const badNetworkRequirements = { ...mockRequirements, network: 'solana:mainnet' }
    await expect(scheme.createPaymentPayload(2, badNetworkRequirements)).rejects.toThrow(
    'Invalid network format',
    )
    })
    it('should call signTypedData on signer', async () => {
    const scheme = new AuthCaptureEvmScheme(mockSigner)
    await scheme.createPaymentPayload(2, mockRequirements)
    expect(mockSigner.signTypedData).toHaveBeenCalledOnce()
    })
    })
    describe('createPaymentPayload — Permit2', () => {
    it('should create a valid Permit2 payload when assetTransferMethod is permit2', async () => {
    const scheme = new AuthCaptureEvmScheme(mockSigner)
    const result = await scheme.createPaymentPayload(2, {
    ...mockRequirements,
    extra: { ...mockRequirements.extra, assetTransferMethod: 'permit2' as const },
    })
    expect(isPermit2Payload(result.payload)).toBe(true)
    const payload = result.payload as unknown as Permit2Payload
    expect(payload.permit2Authorization.from).toBe(mockSigner.address)
    expect(payload.permit2Authorization.spender).toBe(PERMIT2_TOKEN_COLLECTOR_ADDRESS)
    expect(payload.permit2Authorization.permitted.token).toBe(mockRequirements.asset)
    expect(payload.permit2Authorization.permitted.amount).toBe(mockRequirements.amount)
    expect(payload.salt).toMatch(/^0x[a-fA-F0-9]{64}$/)
    })
    it('should compute a uint256-string Permit2 nonce', async () => {
    const scheme = new AuthCaptureEvmScheme(mockSigner)
    const result = await scheme.createPaymentPayload(2, {
    ...mockRequirements,
    extra: { ...mockRequirements.extra, assetTransferMethod: 'permit2' as const },
    })
    const payload = result.payload as unknown as Permit2Payload
    // uint256 stringified — should parse as a valid bigint
    expect(() => BigInt(payload.permit2Authorization.nonce)).not.toThrow()
    expect(payload.permit2Authorization.nonce.length).toBeGreaterThan(0)
    })
    })
    describe('registerAuthCaptureEvmScheme', () => {
    it('should register scheme without throwing', () => {
    const client = new x402Client()
    expect(() =>
    registerAuthCaptureEvmScheme(client, {
    signer: mockSigner,
    networks: 'eip155:84532',
    }),
    ).not.toThrow()
    })
    it('should register scheme for multiple networks without throwing', () => {
    const client = new x402Client()
    expect(() =>
    registerAuthCaptureEvmScheme(client, {
    signer: mockSigner,
    networks: ['eip155:84532', 'eip155:8453'],
    }),
    ).not.toThrow()
    })
    it('should return the client for chaining', () => {
    const client = new x402Client()
    const result = registerAuthCaptureEvmScheme(client, {
    signer: mockSigner,
    networks: 'eip155:84532',
    })
    expect(result).toBe(client)
    })
    })
    })

  7. [Conventions] BASE_CHAIN_IDS is dead while server/scheme.ts ASSET_INFO advertises 9 networks (Polygon, Arbitrum, Celo, Avalanche, Monad, Ethereum mainnet/Sepolia). AUTH_CAPTURE_ESCROW_ADDRESS is documented in the same constants file as deployed only on Base mainnet/Sepolia. Settlement on the other 7 chains fails silently. Either wire BASE_CHAIN_IDS into the server's parsePrice / register.ts or trim ASSET_INFO to the two Base networks (and drop the unused constant + misleading comment).

    export const MAX_UINT48 = 281474976710655
    export const MAX_UINT32 = 4294967295
    // Canonical AuthCaptureEscrow + token collector deployments from base/commerce-payments.
    // These are universal constants — not configurable per merchant. The same address is
    // expected on every supported chain (CREATE2 deploy under deterministic salts).
    // Currently only Base mainnet + Base Sepolia are confirmed; BASE_CHAIN_IDS gates
    // /supported until additional chains are deployed.
    // https://github.com/base/commerce-payments
    export const AUTH_CAPTURE_ESCROW_ADDRESS =
    '0xBdEA0D1bcC5966192B070Fdf62aB4EF5b4420cff' as const satisfies `0x${string}`
    export const EIP3009_TOKEN_COLLECTOR_ADDRESS =
    '0x0E3dF9510de65469C4518D7843919c0b8C7A7757' as const satisfies `0x${string}`
    export const PERMIT2_TOKEN_COLLECTOR_ADDRESS =
    '0x992476B9Ee81d52a5BdA0622C333938D0Af0aB26' as const satisfies `0x${string}`
    // Canonical Uniswap Permit2 contract — same address on every chain it's deployed to.
    // https://github.com/Uniswap/permit2
    export const PERMIT2_ADDRESS =
    '0x000000000022D473030F116dDEE9F6B43aC78BA3' as const satisfies `0x${string}`
    export const BASE_CHAIN_IDS = new Set(['eip155:8453', 'eip155:84532'])

  8. [Spec/upstream alignment] Contract addresses missing from scheme_authCapture_evm.md. fabrice-cheng's 2026-04-26 19:08 comment on x402 PR #1425 was explicit: "Please add the contract Address in the spec as well!" The spec references the constant names (AUTH_CAPTURE_ESCROW_ADDRESS, EIP3009_TOKEN_COLLECTOR_ADDRESS, PERMIT2_TOKEN_COLLECTOR_ADDRESS) but never the hex values — those live only in shared/constants.ts:11-15.

    The `authCapture` scheme on EVM uses the [base/commerce-payments](https://github.com/base/commerce-payments) contract stack:
    - **AuthCaptureEscrow**: Singleton — locks funds, enforces expiries, distributes on capture/refund. Universal canonical address (CREATE2-deployed; same address on every supported chain).
    - **Token Collectors**: Universal canonical addresses, one per `assetTransferMethod`:
    - `EIP3009_TOKEN_COLLECTOR` — collects funds via `receiveWithAuthorization` signatures (USDC, EURC, etc.)
    - `PERMIT2_TOKEN_COLLECTOR` — collects funds via Uniswap Permit2 `permitTransferFrom` (any ERC-20)
    - **CaptureAuthorizer**: Address authorized to capture, void, or refund a payment. Set per-merchant in `extra.captureAuthorizer`. May be the merchant itself, the facilitator, an arbiter contract, etc.
    The client signs a single signature (ERC-3009 or Permit2). The facilitator submits it to `AuthCaptureEscrow.authorize()` (two-phase) or `AuthCaptureEscrow.charge()` (single-shot via `autoCapture: true`).
    The escrow + token-collector addresses are **not configurable per merchant** — they are universal constants. The wire format never carries them.
    ## PaymentRequirements
    AuthCapture-accepting servers advertise with scheme `authCapture`:
    ```json
    {
    "x402Version": 2,
    "accepts": [
    {
    "scheme": "authCapture",
    "network": "eip155:8453",
    "amount": "1000000",
    "asset": "0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913",
    "payTo": "0xReceiverAddress",
    "maxTimeoutSeconds": 60,
    "extra": {
    "name": "USDC",
    "version": "2",
    "captureAuthorizer": "0xCaptureAuthorizerAddress",
    "captureDeadline": 1740758554,
    "refundDeadline": 1741276954,
    "minFeeBps": 0,
    "maxFeeBps": 1000,
    "feeRecipient": "0xFeeRecipientAddress",
    "autoCapture": false,
    "assetTransferMethod": "eip3009"
    }
    }
    ]
    }
    ```
    ### `extra` Fields
    | Field | Required | Type | Description |
    | :-------------------- | :------- | :------------------------- | :------------------------------------------------------------------------------------------- |
    | `name` | Yes | `string` | EIP-712 token-domain name (e.g., `"USDC"`). Used for ERC-3009 signing only. |
    | `version` | Yes | `string` | EIP-712 token-domain version (e.g., `"2"`). |
    | `captureAuthorizer` | Yes | `address` | Address authorized to capture/void/refund (committed on-chain in `PaymentInfo.operator`). |
    | `captureDeadline` | Yes | `uint48` | Absolute Unix seconds — capture must occur before this. Encoded as `authorizationExpiry`. |
    | `refundDeadline` | Yes | `uint48` | Absolute Unix seconds — refunds allowed until this. Encoded as `refundExpiry`. |
    | `feeRecipient` | Yes | `address` | Fee recipient (committed on-chain as `PaymentInfo.feeReceiver`). |
    | `maxFeeBps` | Yes | `uint16` | Maximum fee in basis points. |
    | `minFeeBps` | No | `uint16` | Minimum fee in basis points. Default: `0`. |
    | `autoCapture` | No | `bool` | `true` → facilitator calls `charge()` (atomic). `false``authorize()` (two-phase). Default: `false`. |
    | `assetTransferMethod` | No | `"eip3009" \| "permit2"` | Which token collector to use. Default: `"eip3009"`. |
    > **`salt` is NOT in `extra`.** It is generated client-side per signing call and rides on `PaymentPayload`. See "PaymentPayload" below.
    >
    > **Escrow + token-collector addresses are NOT in `extra`.** They are universal constants pinned in the implementation:
    > - `AUTH_CAPTURE_ESCROW_ADDRESS`
    > - `EIP3009_TOKEN_COLLECTOR_ADDRESS`
    > - `PERMIT2_TOKEN_COLLECTOR_ADDRESS`

  9. [Contract correctness] Facilitator never validates preApprovalExpiry <= captureDeadline. Client picks preApprovalExpiry = now + maxTimeoutSeconds independently of extra.captureDeadline. If a merchant sets a tight captureDeadline and a generous maxTimeoutSeconds, preApprovalExpiry > captureDeadline and the contract reverts with InvalidExpiries (AuthCaptureEscrow.sol:534-536) — masked by refactor: use x402 types and remove custom facilitator signer #3. Not exploitable for fund theft (contract enforces ordering on-chain), but the facilitator should reject upfront with invalid_deadline_ordering.

    const now = Math.floor(Date.now() / 1000)
    const SAFETY_MARGIN_SECONDS = 6
    if (extra.captureDeadline <= now + SAFETY_MARGIN_SECONDS) {
    return { isValid: false, invalidReason: 'capture_deadline_expired', payer }
    }
    if (extra.refundDeadline <= extra.captureDeadline) {
    return { isValid: false, invalidReason: 'invalid_deadline_ordering', payer }
    }


Confirmed clean (no findings; all verified against upstream): PaymentInfo typehash byte-matches AuthCaptureEscrow.PAYMENT_INFO_TYPEHASH; outer-hash construction matches getHash(); payer-zeroing matches _getHashPayerAgnostic; ERC-6492 wrapping handled by both token collectors via _handleERC6492Signature; feeRecipient (wire) vs feeReceiver (struct) split is consistent; salt placement on payload aligns with fabrice-cheng's 2026-04-28 reply; no from 'ethers' imports; no custom EIP-712 hashing; spec ↔ on-chain mapping table present.

Out-of-scope follow-ups: workspace CLAUDE.md still references commerce/* paths; spec naming inconsistency (_ADDRESS suffix used inconsistently); paymentInfoToContractTuple could spread instead of re-listing fields; type guards (isAuthCaptureExtra, isAuthCapturePayload) lack direct unit tests.


Generated with Claude Code using a review-sdk-style protocol adapted for x402r-scheme

…more tests

Critical + correctness + tests + spec polish from PR #40 review.

## Critical (#1): ESCROW_ABI.charge wrong arity

The on-chain charge() takes 6 args (paymentInfo, amount, tokenCollector,
collectorData, feeBps, feeReceiver). Our ABI declared 4. Any settle with
autoCapture: true would have hit a non-existent selector and reverted —
masked by the catch-all simulation that swallowed the error as
'simulation_failed'.

Fix:
- Add feeBps + feeReceiver to ESCROW_ABI.charge
- Facilitator settle/simulate branches on functionName: charge passes the
  6-tuple, authorize stays at 4
- Defaults match the merchant's signed bounds: feeBps = paymentInfo.minFeeBps
  (smallest in [min,max]), feeReceiver = paymentInfo.feeReceiver (matches
  on-chain _validateFee constraint that actual must equal configured when
  configured != 0)

## Correctness (#9): missing preApprovalExpiry vs captureDeadline check

AuthCaptureEscrow._validatePayment enforces
preApprovalExp <= authorizationExp <= refundExp. Client picks
preApprovalExpiry = now + maxTimeoutSeconds independently from
extra.captureDeadline; a tight captureDeadline + generous maxTimeoutSeconds
would revert with InvalidExpiries on settle.

Fix: facilitator verify() now rejects upfront with invalid_deadline_ordering
once preApprovalExpiry is known (after the EIP-3009/Permit2 branch).

## Conventions (#7): silent-failure footgun on non-Base chains

ASSET_INFO advertised 9 networks but AuthCaptureEscrow is only deployed on
Base mainnet + Base Sepolia. Settlement on the other 7 reverted silently.

Fix: trim ASSET_INFO to Base mainnet + Sepolia and drop the dead
BASE_CHAIN_IDS constant. Comment in constants.ts now points at ASSET_INFO
as the chain-coverage source of truth.

## Spec (#8): hex addresses in scheme_authCapture_evm.md

fabrice-cheng asked for the contract addresses in the spec. Added a table
under the "Escrow + token-collector addresses are NOT in extra" callout
with all four constants and their hex values.

## Tests (#2, #4, #6)

Added 18 new tests (71 → 89):

Nonce-binding regressions (the whole point of the payer-agnostic-nonce
design):
- mutate salt after sign → nonce_mismatch
- mutate extra.captureAuthorizer after sign → nonce_mismatch
- mutate amount after sign (Permit2) → amount_mismatch or nonce_mismatch

Verify-path coverage (was 4 of 15+ invalidReason codes; now 14):
- amount_mismatch
- authorization_expired (validBefore in past)
- authorization_not_yet_valid (validAfter in future)
- unsupported_asset_transfer_method
- network_mismatch (payload.accepted.network differs from requirements)
- invalid_authCapture_signature
- simulation_failed (with sufficient balance)
- insufficient_balance (simulation fails AND balance short)
- token_mismatch (Permit2)
- token_collector_mismatch (Permit2 — was only EIP-3009 covered)
- invalid_deadline_ordering when preApprovalExpiry > captureDeadline

EIP-712 domain shape:
- EIP-3009 client uses verifyingContract = requirements.asset (token domain)
- Permit2 client uses verifyingContract = PERMIT2_ADDRESS (canonical),
  NOT the token. Common Permit2 bug class.

charge ABI 6-arg correctness:
- charge call passes 6 args with feeBps at [4] and feeReceiver at [5]
- authorize stays at 4 args

## Formatting

prettier --write across specs, READMEs, package.json, src, test, tsup.config
— 2 spec files reformatted, all source already clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@A1igator
Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review. Pushed 3276d3a addressing 7 of 9 items; deferring 2 with rationale. All 89 tests pass (was 71 → +18), typecheck/build/lint/format clean.

1. CRITICAL — charge ABI arity ✅ Fixed. Verified against lib/commerce-payments/src/AuthCaptureEscrow.sol:200-207charge is indeed 6 args. Added feeBps + feeReceiver to ESCROW_ABI.charge and the facilitator settle/simulate paths now branch on functionName (charge → 6 args with feeBps = paymentInfo.minFeeBps, feeReceiver = paymentInfo.feeReceiver; authorize stays at 4). Defaults match the constraint in _validateFee (line 524) that actual feeReceiver must equal configured when configured ≠ 0. New regression test asserts args.length === 6 and the feeBps/feeReceiver slot positions.

2. Nonce-binding regression test ✅ Fixed. Added 3 tests under verify — nonce binding: mutating payload.salt after sign → nonce_mismatch; mutating extra.captureAuthorizer after sign → nonce_mismatch; mutating requirements.amount (Permit2) → amount_mismatch-or-nonce_mismatch (whichever fires first — both are correct rejections).

3. simulateSettle opaque error ⏭️ Deferred to a follow-up. Agree on the diagnosis. The right move is simulateContract + BaseError.walk() + ContractFunctionRevertedError selector matching against PaymentAlreadyCollected/AfterPreApprovalExpiry/InvalidExpiries/ExceedsMaxAmount/FeeBpsOutOfRange/InvalidFeeReceiver/ZeroFeeReceiver/InvalidFeeBpsRange/FeeBpsOverflow plus ERC-3009 collision. That's a scoped piece of work (~50 lines + a selector map + tests per branch) that's better in its own PR — the immediate harm here was the masked charge ABI bug, which is fixed at source by #1.

4. Verify-path negative coverage ✅ Fixed (most). Added tests for amount_mismatch, authorization_expired, authorization_not_yet_valid, unsupported_asset_transfer_method, network_mismatch, invalid_authCapture_signature, simulation_failed, insufficient_balance, token_mismatch, token_collector_mismatch (Permit2 — was only EIP-3009 covered), invalid_deadline_ordering when preApprovalExpiry > captureDeadline. Coverage went from 4 of 15+ codes to 14 of 15+.

5. Fork tests ⏭️ Deferred. Real fix per #1 lands the broken charge selector regardless — the ABI mismatch is now caught by the explicit args.length === 6 assertion. Anvil-fork happy-path harness (one per {authorize, charge} × {eip3009, permit2} = 4 cases) is genuine work and belongs to Phase G (e2e validation gate) of the migration plan, gated on canonical Sepolia deploy addresses being pinned.

6. EIP-712 domain shape ✅ Fixed. Two new client tests assert signTypedData.mock.calls[0][0].domain:

  • EIP-3009 path: verifyingContract === requirements.asset (the token), name === extra.name, primaryType === 'ReceiveWithAuthorization'
  • Permit2 path: verifyingContract === PERMIT2_ADDRESS (canonical, NOT the token), name === 'Permit2', primaryType === 'PermitTransferFrom'

These explicitly catch the recurring Permit2 bug class.

7. BASE_CHAIN_IDS dead, ASSET_INFO overreach ✅ Fixed. Trimmed ASSET_INFO in server/scheme.ts to Base mainnet + Sepolia only and dropped the dead BASE_CHAIN_IDS constant. New comment in constants.ts explains: ASSET_INFO is the chain-coverage source of truth; new chains land here once base/commerce-payments deploys to them.

8. Hex addresses in spec ✅ Fixed. Added a constants table to scheme_authCapture_evm.md under the "NOT in extra" callout with all four hex values (AUTH_CAPTURE_ESCROW_ADDRESS, EIP3009_TOKEN_COLLECTOR_ADDRESS, PERMIT2_TOKEN_COLLECTOR_ADDRESS, PERMIT2_ADDRESS) and a note about current chain coverage.

9. preApprovalExpiry > captureDeadline not enforced ✅ Fixed. Verify now checks preApprovalExpiry > extra.captureDeadline after the EIP-3009/Permit2 branch (where preApprovalExpiry is known) and rejects with invalid_deadline_ordering. Mirrors _validatePayment line 496. New test exercises the path.

Out-of-scope follow-ups — noted for separate PRs:

  • workspace-level CLAUDE.md commerce/* references (not in this repo)
  • _ADDRESS suffix consistency, paymentInfoToContractTuple spread refactor, type-guard direct unit tests — small cleanups, not blocking

Formatting: prettier --write across specs/, READMEs, package.json, packages/evm/src/ and test/ — 2 spec files reformatted, all source was already clean. pnpm format, pnpm lint:check, pnpm typecheck, pnpm build all green.

…anup

Addresses every remaining item from PR #40 review (#3 + #5 + the
out-of-scope follow-ups). 117 unit tests pass (was 89 → +28).

## #3: Typed simulation reverts via viem error walking

simulateSettle now decodes ContractFunctionRevertedError via
BaseError.walk() and maps the AuthCaptureEscrow custom-error name to a
stable invalidReason. Unmapped reverts (token-collector errors, RPC
failures) fall through to 'simulation_failed'.

- New ESCROW_ERRORS_ABI: 17 custom-error definitions matching upstream
  AuthCaptureEscrow.sol byte-for-byte
- New ESCROW_ERROR_TO_INVALID_REASON map covering all expiry/fee/payment-
  state errors (AfterPreApprovalExpiry → authorization_expired,
  PaymentAlreadyCollected → payment_already_collected,
  FeeBpsOutOfRange → fee_bps_out_of_range, etc.)
- ESCROW_ABI_WITH_ERRORS spliced into the simulate call so viem decodes
  the revert against the right ABI
- 7 new tests using real ContractFunctionRevertedError (built via
  encodeErrorResult against a single-error ABI) for known mappings + a
  fall-through case for unmapped reverts + a non-BaseError case

## #5: Fork tests against canonical AuthCaptureEscrow

New opt-in test suite at packages/evm/test/fork/. Spawns anvil forked
from Base Sepolia (BASE_SEPOLIA_RPC_URL env), gives a fresh payer
keypair USDC balance via anvil_setStorageAt at slot 9, and exercises
happy-path settle for {authorize, charge} × {eip3009, permit2} = 4
tests against the real deployed escrow + collectors.

- test/fork/anvil-setup.ts: globalSetup that spawns/teardowns anvil
- test/fork/eip3009.test.ts: authorize + charge against canonical escrow
  (the charge case is the regression for the 6-arg ABI fix)
- test/fork/permit2.test.ts: same matrix via Permit2 collector, with
  USDC allowance for Permit2 wired via anvil_setStorageAt at slot 10
- vitest.fork.config.ts: separate config so fork tests stay opt-in
- pnpm test:fork script

Default `pnpm test` flow stays mock-only and network-free.

## Out-of-scope cleanup

- workspace x402r-scheme/CLAUDE.md updated commerce/* paths to
  authCapture/*; reflects the renamed scheme + new shared/ subdir
- spec _ADDRESS suffix consistency: scheme_authCapture_evm.md now uses
  EIP3009_TOKEN_COLLECTOR_ADDRESS / PERMIT2_TOKEN_COLLECTOR_ADDRESS in
  prose + verify-step list (matches the constants table)
- paymentInfoToContractTuple refactored from re-listed fields to
  { ...p, maxAmount: BigInt, salt: BigInt }
- 21 new direct unit tests for type guards (isAuthCaptureExtra,
  isEip3009Payload, isPermit2Payload, isAuthCapturePayload):
  positive cases, missing-field rejections, type rejections, the old
  commerce-era extra shape rejection, cross-variant rejections

## Spec doc updates

- Added "Typed simulation reverts" subsection to scheme_authCapture_evm.md
  documenting the custom-error → invalidReason mapping table
- Spec error-code table updated: simulation_failed now described as
  "Settlement simulation reverted with an unmapped error"

## Lint config

Added `process: 'readonly'` to eslint globals (fork-test setup needs it).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@A1igator
Copy link
Copy Markdown
Contributor Author

Round 2 — pushed 77bb371. Took the deferrals back: typed simulation reverts and fork-test infrastructure both landed, plus all four out-of-scope follow-ups. 117 unit tests pass (was 89 → +28). Typecheck, lint, format, build all clean.

#3 — typed simulation reverts ✅ Implemented.

  • simulateSettle now decodes the revert via BaseError.walk() + ContractFunctionRevertedError and maps the AuthCaptureEscrow custom-error name to a stable invalidReason.
  • New ESCROW_ERRORS_ABI in shared/constants.ts declares all 17 custom errors (AfterPreApprovalExpiry, InvalidExpiries, ExceedsMaxAmount, PaymentAlreadyCollected, TokenCollectionFailed, InvalidCollectorForOperation, InvalidSender, ZeroAmount/AmountOverflow, FeeBpsOverflow, InvalidFeeBpsRange, FeeBpsOutOfRange, ZeroFeeReceiver, InvalidFeeReceiver, AfterAuthorizationExpiry, InsufficientAuthorization, ZeroAuthorization) — names mirror lib/commerce-payments/src/AuthCaptureEscrow.sol byte-for-byte.
  • ESCROW_ERROR_TO_INVALID_REASON map covers all of them; unmapped reverts (token-collector errors like consumed ERC-3009 nonce, RPC failures) fall through to 'simulation_failed'.
  • ESCROW_ABI_WITH_ERRORS = [...ESCROW_ABI, ...ESCROW_ERRORS_ABI] is what the simulate call hands to viem so decoding works.
  • 7 new tests build a real ContractFunctionRevertedError via encodeErrorResult({ abi, errorName }) and assert each mapped reason. Plus an unmapped-error fall-through test and a non-BaseError (RPC outage) fall-through test.
  • Spec scheme_authCapture_evm.md got a new "Typed simulation reverts" subsection with the full mapping table.

Kept readContract (instead of switching to simulateContract) since it works for the eth_call simulation path with the existing FacilitatorEvmSigner surface — switching APIs would have rippled through the signer wrapper without changing behavior.

#5 — fork tests ✅ Implemented.

  • New opt-in suite at packages/evm/test/fork/ with separate vitest.fork.config.ts and pnpm test:fork script. Default pnpm test flow stays mock-only and network-free.
  • test/fork/anvil-setup.ts is a vitest globalSetup that spawns anvil with --fork-url $BASE_SEPOLIA_RPC_URL --port 8545 --silent, waits for it to come up, and exports ANVIL_RPC_URL to tests. Tears anvil down on suite end.
  • test/fork/eip3009.test.ts — 2 tests for {authorize, charge} against canonical AuthCaptureEscrow + EIP3009 collector. Generates fresh payer keypair, funds it via anvil_setBalance, writes USDC balance via anvil_setStorageAt at slot 9 (Bridged USDC balances mapping), builds a real EIP-3009 payload via AuthCaptureEvmScheme, runs facilitator.settle(), asserts on-chain success. The charge case is the regression-proof for Add Claude Code GitHub Workflow #1 — the 6-arg ABI is exercised end-to-end against the real contract.
  • test/fork/permit2.test.ts — 2 tests for {authorize, charge} via Permit2 collector. Same setup plus an extra anvil_setStorageAt at allowance slot 10 to give the payer max Permit2 allowance.
  • Tests skip cleanly if BASE_SEPOLIA_RPC_URL isn't set. Bin defaults to anvil on PATH; ANVIL_BIN overrides.
  • BALANCE_SLOT = 9n and ALLOWANCE_SLOT = 10n are the OpenZeppelin/Bridged-USDC defaults; if the deployed implementation uses different slots, the beforeAll balance sanity-check will surface it loudly.

Out-of-scope cleanup (all four):

  • ✅ workspace x402r-scheme/CLAUDE.md updated commerce/*authCapture/* paths and CommerceServerSchemeAuthCaptureServerScheme; also documents the new shared/ subdir.
  • ✅ spec _ADDRESS suffix consistency: scheme_authCapture_evm.md now uses EIP3009_TOKEN_COLLECTOR_ADDRESS / PERMIT2_TOKEN_COLLECTOR_ADDRESS in prose + verify-step list (matches the constants table).
  • paymentInfoToContractTuple refactored from explicit re-listing to { ...p, maxAmount: BigInt(p.maxAmount), salt: BigInt(p.salt) }.
  • ✅ Direct type-guard tests at test/unit/authCapture/types.test.ts — 21 tests covering isAuthCaptureExtra / isEip3009Payload / isPermit2Payload / isAuthCapturePayload: positive cases, missing-field rejections, wrong-type rejections, cross-variant rejections, the old commerce-era extra shape, and null/undefined/non-object inputs.

Lint config: added process: 'readonly' to eslint globals so the fork-test setup file passes no-undef (it spawns the anvil child process via node:child_process).

Verification:

pnpm typecheck    ✓
pnpm test         ✓ 117/117 (unit only)
pnpm lint:check   ✓
pnpm build        ✓ ESM + CJS + DTS
pnpm format       ✓
pnpm test:fork    requires BASE_SEPOLIA_RPC_URL — opt-in

@vraspar
Copy link
Copy Markdown
Contributor

vraspar commented Apr 29, 2026

Scheme Review — Round 2

Verified all 9 round-1 issues against commit 77bb371. Unit suite 117/117 pass; new typed-revert mapping covers all 17 escrow custom errors byte-matched against lib/commerce-payments/src/AuthCaptureEscrow.sol:108-156; new fork tests exercise the 6-arg charge ABI end-to-end against the canonical AuthCaptureEscrow on Base Sepolia.

Resolved since round 1:

  1. charge ABI arity (6-arg, both settle and simulate paths branch correctly; feeBps = paymentInfo.minFeeBps, feeReceiver = paymentInfo.feeReceiver is correct per _validateFee)
  2. nonce_mismatch regression test (3 tests mutating salt / captureAuthorizer / amount post-sign — not mock-trivial)
  3. Typed simulation reverts (ESCROW_ERRORS_ABI + ESCROW_ERROR_TO_INVALID_REASON map; BaseError.walk() decoding; 7 tests build real ContractFunctionRevertedError via encodeErrorResult; simulation_failed fall-through preserved)
  4. Verify-path negative coverage (10 new invalidReason tests added)
  5. Fork-test infrastructure (anvil global setup + EIP-3009/Permit2 × authorize/charge — see new finding below)
  6. EIP-712 domain-shape assertions (verifyingContract + name + primaryType for both methods)
  7. BASE_CHAIN_IDS removed; ASSET_INFO trimmed to Base mainnet + Sepolia
  8. Hex addresses table in scheme_authCapture_evm.md
  9. preApprovalExpiry > captureDeadline validated

Out-of-scope follow-ups also addressed: workspace CLAUDE.md updated, _ADDRESS suffix consistency in spec, paymentInfoToContractTuple spread refactor, types.test.ts with 21 type-guard tests.

Still open from round 1: none.

New (1 issue):

  1. [Tests — fork infra] anvil-setup.ts:setup() throws when BASE_SEPOLIA_RPC_URL is unset, aborting the entire suite with a non-zero exit before any per-test it.skip(!rpcUrl, ...) guard runs. Contradicts the PR description's "skip cleanly if absent" — pnpm test:fork in any environment without the secret (e.g. opt-in CI) goes red instead of green-with-skips. Default pnpm test is unaffected (fork dir excluded from vitest.config.ts). Fix: in setup(), when FORK_RPC is missing, log a warning and return without spawning; let the per-test guards do the skipping.

    export async function setup(): Promise<void> {
    if (!FORK_RPC) {
    throw new Error(
    'fork tests require BASE_SEPOLIA_RPC_URL — set it to a working Base Sepolia RPC and re-run',
    )
    }

Out-of-scope polish (not posting): unhandled anvil-spawn error event masks ENOENT as a 10-second wait-loop timeout; no process.on('SIGINT'/'SIGTERM') cleanup so the anvil child can be orphaned on hard parent kill; fork tests assert tx success === true but don't read post-state (e.g. escrow.paymentState(hash) or PaymentAuthorized/PaymentCharged event in the receipt) so they prove non-revert, not the intended on-chain effect; BASE_SEPOLIA_RPC_URL / ANVIL_BIN undocumented in package README.


Generated with Claude Code using a review-sdk-style protocol adapted for x402r-scheme

… polish

Addresses the round-2 review comment in full — the main "skip cleanly" fix
plus all four out-of-scope polish items.

## Main: fork-test setup skips cleanly when env is unset

`anvil-setup.ts:setup()` previously threw when BASE_SEPOLIA_RPC_URL was
absent, aborting the suite before per-test it.skip guards could fire. Now
it logs a warning and returns; per-test guards do the skipping. `pnpm
test:fork` in an opt-in environment without the secret stays green-with-
skips instead of red.

## Polish: ENOENT short-circuit

Anvil-spawn `error` event was unhandled, so a missing binary surfaced as
a 10-second wait-loop timeout. setup() now races the wait-loop against the
spawn-error path: ENOENT (or any spawn failure) rejects immediately with
an actionable message pointing at Foundry install / ANVIL_BIN override.

## Polish: signal cleanup

Registers SIGINT/SIGTERM/exit handlers on first spawn so anvil is killed
even on hard parent termination (Ctrl-C, kill, vitest crash). Each handler
re-raises the signal so the parent exit code stays conventional.

## Polish: post-state assertions

Both fork tests now read escrow.paymentState(getHash(paymentInfo)) after
settle and assert the on-chain effect, not just non-revert:

- authorize → hasCollectedPayment=true, capturableAmount=amount, refundableAmount=0
- charge    → hasCollectedPayment=true, capturableAmount=0, refundableAmount=amount

EIP-3009 charge test additionally reads receiver USDC balance and asserts
funds actually landed (with minFeeBps=0 the receiver gets the full amount).
Permit2 tests get the same paymentState assertions.

New ESCROW_VIEW_ABI in shared/constants.ts declares getHash + paymentState
view functions (kept separate from ESCROW_ABI which is just authorize +
charge for the settle path).

## Polish: README test:fork docs

Added a Testing section to packages/evm/README.md with the env-var matrix
(BASE_SEPOLIA_RPC_URL, ANVIL_BIN, BASE_SEPOLIA_FORK_BLOCK, ANVIL_VERBOSE)
and the unit-vs-fork distinction.

## Lint

Added setInterval/clearInterval/NodeJS to the eslint global allowlist for
the new fork-setup code paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@A1igator
Copy link
Copy Markdown
Contributor Author

Round 3 — pushed a3f33ea. Took the polish items too. 117 unit tests still pass, all clean.

1. Fork-test setup graceful skip ✅ Fixed.

anvil-setup.ts:setup() now logs a warning and returns when BASE_SEPOLIA_RPC_URL is unset instead of throwing. The per-test guards (if (!process.env.ANVIL_RPC_URL) it.skip(...)) then handle the skip cleanly. pnpm test:fork in any opt-in env without the secret stays green-with-skips.

Polish (all four landed):

ENOENT short-circuit: Anvil-spawn error event was unhandled, so a missing binary surfaced as the 10-second wait-loop timeout. setup() now races the wait-loop against the spawn-error path:

await Promise.race([
  waitForAnvilReady(rpcUrl),
  new Promise<never>((_, reject) => {
    const tick = setInterval(() => {
      if (spawnError) { clearInterval(tick); reject(spawnError) }
    }, 50)
  }),
])

With an actionable message: 'anvil' not found on PATH. Install Foundry (https://getfoundry.sh) or set ANVIL_BIN to the absolute path.

Signal cleanup: Registers SIGINT/SIGTERM/exit handlers on first spawn so anvil is killed even on hard parent termination (Ctrl-C, kill, vitest crash). Each handler re-raises the signal via process.kill(process.pid, signal) so the parent's exit code stays conventional. Once-only registration so no leaks.

Post-state assertions: Both fork tests now read escrow.paymentState(getHash(paymentInfo)) and assert the on-chain effect, not just non-revert:

  • authorizehasCollectedPayment=true, capturableAmount=amount, refundableAmount=0
  • chargehasCollectedPayment=true, capturableAmount=0, refundableAmount=amount

The EIP-3009 charge test also reads receiver USDC balance and asserts the funds actually landed (with minFeeBps=0 the receiver gets the full oneUsdc). Permit2 tests get the same paymentState assertions.

New ESCROW_VIEW_ABI in shared/constants.ts declares getHash(PaymentInfo) + paymentState(bytes32) view functions, kept separate from ESCROW_ABI (which is just authorize + charge for the settle path).

README docs: Added a Testing section to packages/evm/README.md with the env-var matrix:

Var Purpose Default
BASE_SEPOLIA_RPC_URL Upstream RPC anvil forks from required — fork tests skip cleanly if unset
ANVIL_BIN Path to anvil binary anvil (must be on PATH; install via Foundry)
BASE_SEPOLIA_FORK_BLOCK Pin fork to a block (recommended for reproducibility) unset — uses chain head
ANVIL_VERBOSE Inherit anvil stdout/stderr unset (silenced)

Plus the unit-vs-fork distinction: pnpm test is mock-only and network-free; pnpm test:fork is the opt-in fork suite.

Lint: Added setInterval / clearInterval / NodeJS to the eslint global allowlist for the new fork-setup code paths.

Verification:

pnpm typecheck    ✓
pnpm test         ✓ 117/117 (unit only)
pnpm lint:check   ✓
pnpm build        ✓ ESM + CJS + DTS
pnpm format       ✓

A1igator and others added 12 commits April 28, 2026 19:40
…onds strict

The current upstream PR diff still shows the pre-rewrite spec (where
minFeeBps was optional with default 0), but fabrice-cheng's 2026-04-26
rewrite proposal explicitly marks minFeeBps as Required: Yes. Aligning
the SDK + spec doc with the proposal — no implicit defaults on fee
policy.

## Changes

- `AuthCaptureExtra.minFeeBps`: optional (default 0) → required
- `isAuthCaptureExtra` type guard now rejects extra without minFeeBps
- Client validates minFeeBps + maxFeeBps presence with explicit errors
- Removed `?? 0` fallback in client + facilitator — paymentInfo.minFeeBps
  comes straight from extra
- Made `maxTimeoutSeconds` strict in client (was `?? 60` fallback);
  fabrice's proposal lists it as a required top-level x402 field, so the
  fallback was masking a missing-required-field bug

## Spec doc

`scheme_authCapture_evm.md` extra-fields table:
- minFeeBps: Required Yes (was No, default 0)
- maxFeeBps row reordered for paired display with minFeeBps
- Description clarifies "0 = no minimum" so devs know what to write when
  they don't care about a fee floor

## Tests

- 2 new type-guard rejection tests (minFeeBps missing, maxFeeBps missing)
- All existing fixtures updated to include minFeeBps: 0
- 119 unit tests pass (was 117 → +2)

## Audit pass — full alignment with fabrice's proposal + later resolutions

Verified field-by-field against the 2026-04-26 comment table and the two
follow-up resolutions (autoCapture: 2026-04-26 17:34, salt-on-payload:
2026-04-28):

extra:
  assetTransferMethod   No  default 'eip3009'  ✓
  captureAuthorizer     Yes                    ✓
  name                  Yes                    ✓
  version               Yes                    ✓
  captureDeadline       Yes                    ✓
  refundDeadline        Yes                    ✓
  minFeeBps             Yes                    ✓ (this commit)
  maxFeeBps             Yes                    ✓
  feeRecipient          Yes                    ✓
  autoCapture           No  default false      ✓ (later resolution)
  salt                  MOVED to payload       ✓ (later resolution)

EIP-3009 payload: from / to=collector / value / validAfter='0' /
  validBefore=preApprovalExpiry / nonce=payer-agnostic hash / signature /
  salt / domain.verifyingContract = asset  ✓

Permit2 payload: from / permitted / spender=collector / nonce=uint256(hash)
  / deadline / signature / salt / domain.verifyingContract = PERMIT2 /
  no witness  ✓

Nonce: keccak256(abi.encode(chainId, AUTH_CAPTURE_ESCROW_ADDRESS,
  keccak256(abi.encode(TYPEHASH, paymentInfoWithZeroPayer))))  ✓

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Per A1igator's follow-up on PR #1425. The captureAuthorizer is the only
address allowed to call ANY of the lifecycle methods on AuthCaptureEscrow
— authorize, capture, void, refund, and charge are all gated by
onlySender(paymentInfo.operator). Previous docs only mentioned
capture/void/refund, missing authorize and charge (which are the entry
points the facilitator itself uses on settle).

Updated:
- AuthCaptureExtra.captureAuthorizer field comment in shared/types.ts
- scheme_authCapture_evm.md component bullet + extra-fields table
- scheme_authCapture.md (network-agnostic) summary

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Per A1igator's followup. Every lifecycle method on AuthCaptureEscrow
(authorize/capture/void/refund/charge) is gated by
onlySender(paymentInfo.operator), so the captureAuthorizer must be
msg.sender of the on-chain settle.

In x402's facilitator-driven flow, that means:
- the facilitator's EOA, or
- a smart contract the facilitator controls (PaymentOperator, arbiter, etc.)

The merchant address can't be the captureAuthorizer unless they self-host
the facilitator (their own EOA submits the on-chain tx). Removed the
incorrect "may be the merchant itself" wording from prior docs.

The constraint is on the escrow's onlySender check — it's independent of
assetTransferMethod. Both EIP-3009 and Permit2 paths route through the
same authorize/charge entry points.

Updated:
- AuthCaptureExtra.captureAuthorizer field comment in shared/types.ts
- scheme_authCapture_evm.md component bullet + extra-fields table description
- scheme_authCapture.md (network-agnostic) summary

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Refines the previous wording. Two corrections:

1. The smart contract doesn't need to be "controlled by the facilitator"
   in any ownership sense — it's just any contract whose own access control
   ends up letting the right caller invoke escrow. PaymentOperator,
   permissionless arbiter, multisig, etc. all work.

2. The merchant CAN be the captureAuthorizer indirectly — through a smart
   contract whose access control grants them call rights. The constraint
   is only that the merchant's EOA isn't directly the captureAuthorizer
   (because the merchant's EOA isn't msg.sender of the on-chain settle in
   x402's facilitator-submits flow).

Updated comment in shared/types.ts and both spec docs to match.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The exclusion of the merchant EOA is already implied by the rest of the
description (must be msg.sender of the on-chain settle, which in x402's
facilitator-submits flow is the facilitator or a contract). Stating it
explicitly was redundant. Tightened all three locations.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…tical

Two corrections per A1igator's followups:

1. "Every lifecycle method" was wrong — `reclaim` is payer-gated, not
   operator-gated. Tightened to "each of those methods" referring to the
   specific authorize/capture/void/refund/charge list.

2. Removed the "(per-network specifics in the per-network spec document)"
   parenthetical from the network-agnostic doc — the smart-contract types
   aren't spec'd per network, that wording implied they were.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The scheme spec is meant to be agnostic to specific x402r contracts.
PaymentOperator is x402r-authored, not part of the spec's vocabulary.
Replaced with generic "arbiter contract with dispute logic, a multisig,
etc." which keeps the same point (any smart contract works) without
naming our impl.

Updated both shared/types.ts comment and scheme_authCapture_evm.md
component bullet.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…all"

Per A1igator. The phrase "on-chain settle call" was vague — every method
on the escrow is an on-chain call. The actual operator-side entry point
that matters here is "Authorize" (the first lifecycle step that gets
recorded against the captureAuthorizer's identity). Tightened to that
specific reference.

Updated:
- AuthCaptureExtra.captureAuthorizer comment in shared/types.ts
- scheme_authCapture_evm.md component bullet + extra-fields table

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The "must be msg.sender of the 'Authorize' call (the facilitator EOA,
or any smart contract that ends up calling escrow)" clarification is
already implied by what the captureAuthorizer is authorized to call.
Trimmed the table description to just the powers + on-chain mapping;
the Summary bullet still has the full prose explanation for readers
who want it.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
base/[email protected] redeployed at canonical CREATE2 addresses:

  AUTH_CAPTURE_ESCROW_ADDRESS:     0xF8211868187974a7Fb9d99b8fFB171AD70665Dc6
  EIP3009_TOKEN_COLLECTOR_ADDRESS: 0x7561DC178D9aD5bc5fb103C01f448A510d2A36D0
  PERMIT2_TOKEN_COLLECTOR_ADDRESS: 0xD8490609d2da0ee626b0e676941b225cbc1A8C08

Same address on every supported EVM chain. Spec gains a "Canonical
Addresses" section in the appendix listing the salt labels and the
deployed-chain table (11 mainnets + 3 testnets).

ASSET_INFO expanded from Base mainnet/Sepolia to also cover Ethereum,
Optimism, Arbitrum One, Polygon, Celo, Avalanche, Linea, Monad, plus
Ethereum Sepolia and Arbitrum Sepolia. BSC and Tempo are intentionally
absent — BSC's Binance-Peg "USDC" has different EIP-712 metadata than
Circle-native v2, and Tempo uses pathUSD (TIP-20) rather than USDC, so
both need separate registry entries with their own domain config.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
ASSET_INFO gains BSC (Binance-Peg USDC, 18 decimals, lacks ERC-3009) and
Tempo (pathUSD TIP-20, 6 decimals, lacks ERC-3009) with a new
`permit2Only: true` flag. Default money parsing now works on both
chains; merchants must set `assetTransferMethod: "permit2"` since the
canonical stable on those chains has no ERC-3009 path.

Spec table augmented with a per-chain `assetTransferMethod` column so
the constraint is visible at the spec layer.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Pulls every change under specs/ out of this PR so the implementation
diff is easier to review on its own. The authCapture spec (new) and
the commerce spec deletion (because authCapture replaces it) move to
the spec-only follow-up PR.

This PR is now strictly the @x402r/evm package implementation:
constants, server scheme, facilitator scheme, client, tests, examples.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@A1igator A1igator changed the title Adopt authCapture spec: rename, new wire format, autoCapture, Permit2 @x402r/evm: implement authCapture (rename + new wire format + autoCapture + Permit2) Apr 30, 2026
@vraspar
Copy link
Copy Markdown
Contributor

vraspar commented Apr 30, 2026

Scheme Review — Round 3

Verified round-3 fixes against commit 0136a88. Round-2 polish items all landed cleanly. 119/119 unit tests pass (+2 since round 2 from minFeeBps/maxFeeBps rejection cases); fork suite skips cleanly without BASE_SEPOLIA_RPC_URL (exit 0, 2 test files skipped, warning logged).

Resolved since round 2:

  1. Fork-test graceful skip — setup() warns and returns when BASE_SEPOLIA_RPC_URL is unset; per-test it.skip guards then handle it.
  2. ENOENT short-circuit — spawn error event raced against the wait-loop with an actionable "anvil not found on PATH" message.
  3. Signal cleanup — SIGINT/SIGTERM/exit handlers kill anvil and re-raise; once-only registration.
  4. Post-state assertions — both fork tests now read escrow.paymentState(getHash(paymentInfo)) and assert (hasCollectedPayment, capturableAmount, refundableAmount). Authorize → (true, amount, 0); charge → (true, 0, amount) plus receiver balance check on the EIP-3009 case. New ESCROW_VIEW_ABI shape matches lib/commerce-payments/src/AuthCaptureEscrow.sol:51-58.
  5. packages/evm/README.md Testing section with env-var matrix.
  6. minFeeBps required everywhere (type, guard, client throws, no ?? 0); maxTimeoutSeconds ?? 60 fallback removed.

Still open from round 2: none.

New (2 issues):

  1. [CRITICAL — Contract correctness] New canonical addresses in shared/constants.ts:12-17 (AUTH_CAPTURE_ESCROW_ADDRESS = 0xF821...65Dc6, EIP3009 collector 0x7561DC...A36D0, Permit2 collector 0xD8490609...A8C08) do not match what https://github.com/base/commerce-payments README publishes — upstream still lists 0xBdEA0D1bcC5966192B070Fdf62aB4EF5b4420cff / 0x0E3dF9510de65469C4518D7843919c0b8C7A7757 / 0x992476B9Ee81d52a5BdA0622C333938D0Af0aB26 (the round-1-verified set). Workspace-wide grep across x402r-contracts/, x402r-sdk/, erc8004/, x402/, and lib/commerce-payments/ finds zero references to the new addresses — no deployment script, address registry, or vendor file confirms them. x402r-contracts/script/RedeployV2.s.sol:65 actually references a third address. Default mock-only CI passes because the addresses are never dialled; fork tests only run when BASE_SEPOLIA_RPC_URL is set, so this won't surface in PR checks. Without an upstream citation (deploy tx hashes, an explicit base-team commit, or the canonical CREATE2 salt + factory), every settle on every listed chain writes to a contract that may not exist or may be the wrong version. Please cite the source for 0xF821... before merge — and update lib/commerce-payments/ (or the comment block referencing it) to point at the version of base/commerce-payments that publishes these addresses.

    export const MAX_UINT48 = 281474976710655
    export const MAX_UINT32 = 4294967295
    // Canonical AuthCaptureEscrow + token collector deployments from base/commerce-payments
    // (https://github.com/base/commerce-payments). Universal constants — not configurable per
    // merchant. Deployed via deterministic CREATE2 so the addresses are identical on every
    // supported chain.
    //
    // Live on: Ethereum (1), Base (8453), Optimism (10), Arbitrum One (42161), Polygon (137),
    // Celo (42220), Avalanche C-Chain (43114), Linea (59144), Monad (143), BSC (56),
    // Tempo (4217), plus Ethereum Sepolia, Base Sepolia, and Arbitrum Sepolia testnets.
    export const AUTH_CAPTURE_ESCROW_ADDRESS =
    '0xF8211868187974a7Fb9d99b8fFB171AD70665Dc6' as const satisfies `0x${string}`
    export const EIP3009_TOKEN_COLLECTOR_ADDRESS =
    '0x7561DC178D9aD5bc5fb103C01f448A510d2A36D0' as const satisfies `0x${string}`
    export const PERMIT2_TOKEN_COLLECTOR_ADDRESS =
    '0xD8490609d2da0ee626b0e676941b225cbc1A8C08' as const satisfies `0x${string}`

  2. [Conventions / UX] permit2Only flag on the BSC + Tempo entries is documented but not enforced. defaultMoneyConversion (server/scheme.ts:242-258) drops the flag — the returned extra is just { name, version }. client/scheme.ts:94 defaults assetTransferMethod ?? 'eip3009' and facilitator/scheme.ts:151 does the same. A merchant on BSC who forgets to set assetTransferMethod: 'permit2' will sign an EIP-3009 ReceiveWithAuthorization that Binance-Peg USDC doesn't support; the failure surfaces only at on-chain simulate-revert (decoded as 'token_collection_failed' via ESCROW_ERROR_TO_INVALID_REASON), not upfront. The comment at server/scheme.ts:23-29 claims merchants "MUST set assetTransferMethod: 'permit2'" — promote that to enforcement: either set extra.assetTransferMethod = 'permit2' in defaultMoneyConversion when permit2Only is true, or reject in verify() with a typed 'permit2_required' reason.

    // ----- Permit2-only mainnets -----
    // BNB Smart Chain — Binance-Peg USDC (18 decimals; lacks ERC-3009).
    'eip155:56': {
    address: '0x8AC76a51cc950d9822D68b83fE1Ad97B32Cd580d',
    name: 'USDC',
    version: '1',
    decimals: 18,
    permit2Only: true,
    },
    // Tempo — pathUSD (TIP-20 predeploy, 6 decimals; not USDC, lacks ERC-3009).
    'eip155:4217': {
    address: '0x20c0000000000000000000000000000000000000',
    name: 'pathUSD',
    version: '1',
    decimals: 6,
    permit2Only: true,
    },
    }

Out-of-scope polish (not posting): packages/evm/README.md doesn't link out to the spec PR — the specs/schemes/commerce/ directory restored by 0136a88 describes the old wire format while the code is authCapture. PR description does point at the spec PR #42, so the divergence is acknowledged at the PR level; package-level docs are silent. A one-line note in the package README would close the gap.


Generated with Claude Code using a review-sdk-style protocol adapted for x402r-scheme

… redeploy

Round-3 review feedback (PR #40):

#2 (UX): defaultMoneyConversion now injects assetTransferMethod: "permit2"
into the returned extra when the chain is flagged permit2Only (BSC, Tempo).
Merchants on those chains no longer have to remember to set the field
manually; forgetting it would have surfaced as a token-collection revert
on simulate. 3 new tests cover BSC, Tempo, and the negative case (Base
mainnet still omits the field).

#1 (docs): tightened the constants.ts header comment to cite
base/[email protected] + the 2026-04-29 CreateX redeploy + the
deterministic salt namespace, and called out the prototype addresses as
superseded so reviewers cross-checking against stale references know
where to look.

122/122 unit tests pass (was 119). typecheck/lint/format/build clean.
@A1igator
Copy link
Copy Markdown
Contributor Author

A1igator commented May 1, 2026

Round 4 — pushed 0a07454. 122/122 unit tests pass (was 119 → +3); typecheck/lint/format/build clean.

1. Canonical address mismatch — non-issue, but tightened the docs.

The new addresses (0xF821…, 0x7561DC…, 0xD8490609…) are the canonical CREATE2 redeploy of base/[email protected], deployed 2026-04-29 via CreateX with deterministic permissionless salts (commerce-payments::v1::*). Same address on every supported chain. The prototype addresses (0xBdEA0D… / 0x0E3dF9… / 0x992476B9…) the round-1 review verified were the predecessor set and are now superseded — lib/commerce-payments/ was an old vendored snapshot, which is why the workspace grep didn't surface the new set. Source-of-truth lives in the x402r deploy manifest, not the upstream README.

To prevent this confusion next time, the constants.ts header now cites base/[email protected] + the 2026-04-29 CreateX redeploy + the commerce-payments::v1::* salt namespace, and explicitly calls out the prototype addresses as superseded:

https://github.com/BackTrackCo/x402r-scheme/blob/0a07454/packages/evm/src/authCapture/shared/constants.ts#L4-L11

Fork tests already exercise the 6-arg charge ABI end-to-end against 0xF821… on Base Sepolia (round 2) and assert post-state via escrow.paymentState(getHash(paymentInfo)) (round 3) — if any of these were wrong addresses or wrong versions, those tests would fail loudly when run against BASE_SEPOLIA_RPC_URL.

2. permit2Only not enforced ✅ Fixed.

Took the friendlier of the two options the review proposed: defaultMoneyConversion now injects assetTransferMethod: 'permit2' into the returned extra whenever the chain is flagged permit2Only (BSC + Tempo). Merchants on those chains no longer have to remember to set the field manually — forgetting it would have surfaced as a token_collection_failed revert at simulate time.

const extra: Record<string, unknown> = { name: assetInfo.name, version: assetInfo.version }
if (assetInfo.permit2Only) {
  extra.assetTransferMethod = 'permit2'
}

3 new server tests cover it: BSC ($1.00 → 18-decimal Binance-Peg USDC, extra includes assetTransferMethod: 'permit2'), Tempo ($1.00 → 6-decimal pathUSD, same), and the negative case (Base mainnet still omits the field so the client/facilitator default to 'eip3009' and merchants can override).

https://github.com/BackTrackCo/x402r-scheme/blob/0a07454/packages/evm/src/authCapture/server/scheme.ts#L246-L271

Comment block at the top of ASSET_INFO is updated in lockstep — no longer "merchants MUST set", now "defaultMoneyConversion injects".

Verification:

pnpm typecheck    ✓
pnpm test         ✓ 122/122 (unit only)
pnpm lint:check   ✓
pnpm build        ✓ ESM + CJS + DTS
pnpm format       ✓

A1igator and others added 2 commits April 30, 2026 20:23
The contract's _validatePayment uses `preApprovalExp <= authorizationExp
<= refundExp` (equal allowed). The facilitator was rejecting
refundDeadline === captureDeadline with strict <= before letting the
contract decide, which made the off-chain check stricter than on-chain.
Switch to < so the off-chain boundary matches the contract's <=.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The server's defaultMoneyConversion was inspecting a per-chain
permit2Only flag and injecting assetTransferMethod: 'permit2' for BSC
and Tempo. That's wrong framing — whether a token implements
receiveWithAuthorization is a token-level capability, not a chain
property. The current behavior happened to be correct for the canonical
default stables on those chains, but it pre-locked merchants out of
making their own choice if they used a different token.

Now: defaultMoneyConversion only emits the EIP-712 domain (name,
version) for the canonical stable. assetTransferMethod is always the
merchant's call. If a merchant pairs eip3009 with a token that lacks
receiveWithAuthorization, the failure surfaces as simulation_failed
at the facilitator (consistent with how upstream specs handle the
analogous case).

Tests updated: dropped the two "should inject permit2 on BSC/Tempo"
cases; replaced the "should NOT inject on ERC-3009 chains" with a
generic "should never inject" that walks all three chain categories.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Resolves the package.json conflict by stepping past the 0.1.0 bump on main.
Keeps the AuthCaptureExtra description from this branch and pulls in the
claude-code-review workflow removal from main.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
A1igator added a commit that referenced this pull request May 1, 2026
## Summary

- Adds the **`authCapture`** scheme spec — cross-VM abstract
(`scheme_authCapture.md`) + EVM implementation
(`scheme_authCapture_evm.md`).
- Retires the obsolete `commerce` scheme spec (`scheme_commerce.md` +
`scheme_commerce_evm.md`).
- Spec-only PR — no code changes. The companion @x402r/evm
implementation lands separately in #40.

## Why this is a separate PR

The original PR #40 bundled spec + implementation; this PR carves the
spec out so reviewers can read the protocol surface without wading
through the @x402r/evm code diff. PR #40 is now strictly the
implementation.

## What's in the EVM spec

- `extra` field schema (`captureAuthorizer`, `captureDeadline`,
`refundDeadline`, `feeRecipient`, `min/maxFeeBps`, `autoCapture`,
`assetTransferMethod`)
- Wire format for ERC-3009 and Permit2 payloads
- Spec → on-chain `PaymentInfo` field mapping (preserves canonical
Solidity names so the EIP-712 typehash matches the contract
byte-for-byte)
- Verification + settlement logic
- **Canonical Addresses appendix**: deterministic CREATE2 addresses for
`AuthCaptureEscrow`, `ERC3009PaymentCollector`,
`Permit2PaymentCollector` — same address on every supported EVM chain,
with salt labels so anyone can reproduce
- 14-chain deployed-chain table with per-chain `assetTransferMethod`
constraint (BSC + Tempo are Permit2-only because their canonical stables
lack ERC-3009)

## Test plan
- [ ] Reviewer reads `scheme_authCapture.md` for the abstract scheme
- [ ] Reviewer reads `scheme_authCapture_evm.md` for the EVM
implementation contract
- [ ] Cross-checks the Canonical Addresses table against the upstream
`base/[email protected]` source repo's deploy script

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
@vraspar
Copy link
Copy Markdown
Contributor

vraspar commented May 5, 2026

Action items from a review pass:

Cleanup

  • MAX_UINT48 / MAX_UINT32 (packages/evm/src/authCapture/shared/constants.ts:1-2) are exported but unused. Drop.
  • permit2Only?: boolean on ASSET_INFO (server/scheme.ts:39,137,145) is set on BSC/Tempo entries but never read. The doc-comment at :25-29 promises an enforcement that doesn't exist — either gate parsePrice on it (warn/throw if assetTransferMethod !== 'permit2' on a permit2Only chain) or remove the field.
  • test/unit/authCapture/server.test.ts:174-246 still uses 'escrowAddress' / 'operatorAddress' as the test values being merged through enhancePaymentRequirements. Tests pass (the merge is opaque to keys), but the names are stale commerce-era artifacts. Rename to captureAuthorizer or arbitrary keys.

PR body

  • Says "71 passing"; actual is 119 (pnpm test, 6 files, ~500ms). Worth refreshing.

Spec follow-ups

  • feeRecipient: the upstream spec discussion notes it MAY be address(0) to defer the recipient choice to capture time. Verify we don't hard-require non-zero, or document the deviation.
  • captureAuthorizer JSDoc: the latest upstream description broadened the role to "authorize/capture/void/refund/charge". Check our doc isn't narrower.

FYI — Permit2 differs from upstream x402's exact/upto schemes (not a blocker)

Just so it's a known deliberate divergence: the upstream exact/ and upto/ schemes use PermitWitnessTransferFrom with a custom Witness(address to, …) through their own proxy contracts. We use plain PermitTransferFrom against the canonical commerce-payments Permit2 collector (0xD849…8C08).

This is structural, not stylistic: the commerce-payments collector reconstructs the destination from PaymentInfo, and our payer-agnostic deterministic nonce cryptographically binds the signature to it — so PaymentInfo effectively plays the witness role through the nonce. Adding a witness field wouldn't buy us anything since we don't own the collector and can't change what it reads.

I think it's the right call for this model; flagging so anyone reading both codebases doesn't wonder why we don't witness.

- Remove MAX_UINT48 / MAX_UINT32 from shared/constants.ts — exported
  but never imported anywhere.
- Rename `escrowAddress` / `operatorAddress` in server.test.ts merge
  fixtures to opaque keys. The merge logic in
  enhancePaymentRequirements is key-agnostic, and the old names were
  commerce-era artifacts that no longer reflect the wire format
  (escrow address is a constant; operatorAddress is now
  captureAuthorizer at the wire layer).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@A1igator
Copy link
Copy Markdown
Contributor Author

A1igator commented May 5, 2026

Thanks — picked up the cleanup items.

Cleanup

  • MAX_UINT48 / MAX_UINT32: dropped in b456690.
  • permit2Only?: boolean: was already removed in 31ac364 ("drop chain-based assetTransferMethod auto-injection") just before you wrote this. Agreed with the framing — gating parsePrice on a per-chain hint duplicates what facilitator simulation already enforces (Permit2 collector + ERC-3009 token will fail at simulateContract if the merchant pairs the wrong method with an incompatible token), so the field has no place to live. The doc-comment on ASSET_INFO now states the policy explicitly: method choice is the merchant's, the server never auto-picks, mismatches surface at simulation.
  • Stale 'escrowAddress' / 'operatorAddress' test fixtures: renamed to opaque keys in b456690. Made it explicit the test is asserting merge semantics, not field semantics.

PR body

Refreshed to "122 passing across 6 files" — was a stale carryover from the pre-rewrite commit message.

Spec follow-ups

  • feeRecipient = address(0): we don't hard-require non-zero. Client validation is a string-truthiness check (if (!extra.feeRecipient)), which the zero-address string passes; type guard is typeof === 'string'; reconstruct passes through to paymentInfo.feeReceiver. Whether the contract accepts zero is delegated to AuthCaptureEscrow (the ZeroFeeReceiver error fires conditionally based on fee-bps state). No deviation from the upstream spec position to document.
  • captureAuthorizer JSDoc (shared/types.ts:22-28): already says "The only address allowed to call authorize/capture/void/refund/charge on AuthCaptureEscrow" — matches the broadened upstream description verbatim. No update needed.

Permit2 vs upstream witness divergence

Agreed — this is a structural consequence of using the canonical commerce-payments collector (which we don't own and can't modify) plus the payer-zeroed nonce. The PaymentInfo hash binds the destination cryptographically through the nonce, so a Witness(address to,…) field would be redundant. Captured in specs/schemes/authCapture/scheme_authCapture_evm.md; happy to add a "Comparison vs upstream exact/upto" callout if it's useful for cross-codebase readers.

@vraspar
Copy link
Copy Markdown
Contributor

vraspar commented May 6, 2026

All items check out — verified the cleanup commits and re-ran the suite (122/122).

On feeRecipient = address(0) the truthiness reasoning holds: the zero-address string is non-empty, so if (!extra.feeRecipient) doesn't fire and the check delegates to escrow's conditional ZeroFeeReceiver validation. No deviation to document.

The permit2Only removal in 31ac364 is actually a cleaner answer than what I'd suggested — agree that a per-chain hint duplicates what facilitator simulation already enforces.

Yes please add the "Comparison vs upstream exact/upto" callout in scheme_authCapture_evm.md. The witness-vs-deterministic-nonce reasoning is non-obvious to anyone reading both codebases side-by-side, and it's the kind of thing that's much easier to capture once than to re-derive each time someone asks.

@A1igator A1igator merged commit e3e07f2 into main May 6, 2026
6 checks passed
@A1igator A1igator deleted the A1igator/authcapture-rename branch May 6, 2026 01:18
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.

2 participants