Conversation
Adds spec documents following x402's scheme spec structure: - scheme_escrow.md: High-level overview, lifecycle, settlement methods - scheme_escrow_evm.md: EVM implementation details, PaymentInfo struct, verification/settlement logic Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The operator can be either a contract or an EOA — show both flows distinctly instead of assuming a single contract-based architecture. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
charge() sends funds directly to receiver but remains refundable within refundExpiry — it is NOT a non-refundable path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Verified against merged PR #16 and AuthCaptureEscrow source: - Nonce derivation: add typehash prefix and payer=address(0) detail - Settlement step 3: collectorData, not signature (matches ABI param name) - Fee system: describe escrow-level validation, not operator-specific Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
e3b08c2 to
9604782
Compare
The exact scheme specs have no diagrams — align escrow specs with the same convention. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
vraspar
left a comment
There was a problem hiding this comment.
Code Review: Add escrow scheme specification docs
Overview
Spec-only PR adding two markdown documents to specs/schemes/escrow/ following x402's scheme spec convention:
scheme_escrow.md— High-level scheme overview (lifecycle, settlement methods, security)scheme_escrow_evm.md— EVM implementation details (PaymentRequirements, verification, settlement, PaymentInfo struct)
294 additions, 0 deletions, pure documentation.
Accuracy Verification
Verified the spec against the actual implementation. The spec is highly accurate — every major technical detail checks out:
| Aspect | Verdict |
|---|---|
| PaymentInfo struct fields/types | Matches PAYMENT_INFO_COMPONENTS |
| Nonce derivation (typehash + payer=address(0) + chainId/escrow outer hash) | Exact match |
| Verification step order (10 steps) | Exact match with verify() implementation |
| Settlement logic (re-verify → determine function → call → wait → return) | Exact match |
EIP-6492 handling (off-chain: parseErc6492Signature; on-chain: raw passthrough) |
Correct |
| Expiry ordering enforcement | Correct |
| Fee system (minFeeBps/maxFeeBps, feeReceiver semantics) | Correct |
Issues
1. Spec documents PR #16 behavior, but targets main
The spec describes settlementMethod, charge() ABI, feeReceiver defaulting to address(0) — all from PR #16. The base branch is main, but these features aren't on main yet.
Risk: If this merges before PR #16, the spec documents behavior that doesn't exist in the codebase.
Fix: Either merge PR #16 first, or note in the PR description that this depends on #16.
2. Missing: ERC-3009 variant not specified
The spec says "ERC-3009 receiveWithAuthorization" in the summary but never explicitly names the EIP-712 primary type used for signing.
The implementation uses ReceiveWithAuthorization (not TransferWithAuthorization). This distinction matters — they have different typehashes. An implementor following just the spec could pick the wrong variant.
Suggested addition to the Appendix or Nonce Derivation section:
The ERC-3009 signature uses
ReceiveWithAuthorizationas the EIP-712 primary type (notTransferWithAuthorization), because the token collector callstoken.receiveWithAuthorization(...).
3. scheme_escrow.md — "Sign" step says "maximum amount" but conflates with server amount
Sign: Client signs an ERC-3009
receiveWithAuthorizationfor the maximum amount
The PaymentRequirements.amount is the server's requested amount. The client sets paymentInfo.maxAmount >= that amount. The ERC-3009 value field equals maxAmount, not the server's amount.
Suggestion: Clarify:
Sign: Client signs an ERC-3009
receiveWithAuthorizationformaxAmount(>= the server's requestedamount)
4. scheme_escrow_evm.md — extra table lists duration fields as uint48
| preApprovalExpirySeconds | No | uint48 | ...
These are seconds-based durations in the off-chain extra config, not raw uint48 timestamps. The implementation type is number in TypeScript. Using uint48 implies a Solidity type (which is what the on-chain PaymentInfo struct uses). Consider using number to avoid confusion between the off-chain config value and the on-chain struct type.
5. scheme_escrow_evm.md — PaymentPayload example shows "extra": {} in accepted
"accepted": {
...
"extra": {}
}In the actual payload, accepted echoes back the full PaymentRequirements including the populated extra with escrow addresses. An empty extra: {} could mislead implementors into thinking they don't need to echo those fields.
6. Minor: Charge path refund asymmetry not documented
The Charge lifecycle correctly says "Operator can refund within refundExpiry" but doesn't mention that on the charge path, only the operator can trigger a refund — unlike the authorize path where the payer can also reclaim() after authorizationExpiry. This asymmetry is worth noting.
Style
The spec follows x402's specs/schemes/<name>/ directory structure correctly (scheme_escrow.md parallels scheme_exact.md, scheme_escrow_evm.md parallels scheme_exact_evm.md). Section structure aligns well with the x402 template. Tables are well-formatted and consistent.
Verdict
Approve with nits. The spec is technically accurate against the implementation. Issue #1 (merge ordering with PR #16) and Issue #2 (ERC-3009 variant) are the most important to address. The rest are documentation clarity improvements.
🤖 Generated with Claude Code
vraspar
left a comment
There was a problem hiding this comment.
Code Review: Add escrow scheme specification docs
Overview
Spec-only PR adding two markdown documents to specs/schemes/escrow/ following x402's scheme spec convention:
scheme_escrow.md— High-level scheme overview (lifecycle, settlement methods, security)scheme_escrow_evm.md— EVM implementation details (PaymentRequirements, verification, settlement, PaymentInfo struct)
294 additions, 0 deletions, pure documentation.
Accuracy Verification
Verified the spec against the actual implementation. The spec is highly accurate — every major technical detail checks out:
| Aspect | Verdict |
|---|---|
| PaymentInfo struct fields/types | Matches PAYMENT_INFO_COMPONENTS |
| Nonce derivation (typehash + payer=address(0) + chainId/escrow outer hash) | Exact match |
| Verification step order (10 steps) | Exact match with verify() implementation |
| Settlement logic (re-verify → determine function → call → wait → return) | Exact match |
EIP-6492 handling (off-chain: parseErc6492Signature; on-chain: raw passthrough) |
Correct |
| Expiry ordering enforcement | Correct |
| Fee system (minFeeBps/maxFeeBps, feeReceiver semantics) | Correct |
Issues
1. Spec documents PR #16 behavior, but targets main
The spec describes settlementMethod, charge() ABI, feeReceiver defaulting to address(0) — all from PR #16. The base branch is main, but these features aren't on main yet.
Risk: If this merges before PR #16, the spec documents behavior that doesn't exist in the codebase.
Fix: Either merge PR #16 first, or note in the PR description that this depends on #16.
2. Missing: ERC-3009 variant not specified
The spec says "ERC-3009 receiveWithAuthorization" in the summary but never explicitly names the EIP-712 primary type used for signing.
The implementation uses ReceiveWithAuthorization (not TransferWithAuthorization). This distinction matters — they have different typehashes. An implementor following just the spec could pick the wrong variant.
Suggested addition to the Appendix or Nonce Derivation section:
The ERC-3009 signature uses
ReceiveWithAuthorizationas the EIP-712 primary type (notTransferWithAuthorization), because the token collector callstoken.receiveWithAuthorization(...).
3. scheme_escrow.md — "Sign" step says "maximum amount" but conflates with server amount
Sign: Client signs an ERC-3009
receiveWithAuthorizationfor the maximum amount
The PaymentRequirements.amount is the server's requested amount. The client sets paymentInfo.maxAmount >= that amount. The ERC-3009 value field equals maxAmount, not the server's amount.
Suggestion: Clarify:
Sign: Client signs an ERC-3009
receiveWithAuthorizationformaxAmount(>= the server's requestedamount)
4. scheme_escrow_evm.md — extra table lists duration fields as uint48
| preApprovalExpirySeconds | No | uint48 | ...
These are seconds-based durations in the off-chain extra config, not raw uint48 timestamps. The implementation type is number in TypeScript. Using uint48 implies a Solidity type (which is what the on-chain PaymentInfo struct uses). Consider using number to avoid confusion between the off-chain config value and the on-chain struct type.
5. scheme_escrow_evm.md — PaymentPayload example shows "extra": {} in accepted
"accepted": {
...
"extra": {}
}In the actual payload, accepted echoes back the full PaymentRequirements including the populated extra with escrow addresses. An empty extra: {} could mislead implementors into thinking they don't need to echo those fields.
6. Minor: Charge path refund asymmetry not documented
The Charge lifecycle correctly says "Operator can refund within refundExpiry" but doesn't mention that on the charge path, only the operator can trigger a refund — unlike the authorize path where the payer can also reclaim() after authorizationExpiry. This asymmetry is worth noting.
Style
The spec follows x402's specs/schemes/<name>/ directory structure correctly (scheme_escrow.md parallels scheme_exact.md, scheme_escrow_evm.md parallels scheme_exact_evm.md). Section structure aligns well with the x402 template. Tables are well-formatted and consistent.
Verdict
Approve with nits. The spec is technically accurate against the implementation. Issue #1 (merge ordering with PR #16) and Issue #2 (ERC-3009 variant) are the most important to address. The rest are documentation clarity improvements.
🤖 Generated with Claude Code
…nd asymmetry
- Specify ReceiveWithAuthorization as EIP-712 primaryType in verification and settlement sections
- Populate extra in PaymentPayload example (was misleading empty {})
- Document charge path refund asymmetry (no payer reclaim, only operator refund)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thanks for the thorough review! Addressed in d09d3f8: Issue 1 — Merge ordering with PR #16: No longer applicable — #16 was merged before this branch was rebased onto main. The spec targets post-#16 main. Issue 2 — ERC-3009 variant: Fixed. Added Issue 3 — "maximum amount" vs server amount: No change. In the implementation Issue 4 — uint48 vs number for duration fields: Keeping Issue 5 — Empty Issue 6 — Charge path refund asymmetry: Fixed. Added note that on the charge path the payer cannot |
- Lifecycle ends at RESOURCE DELIVERED, not [USE] → CAPTURE - Post-settlement operations (capture, refund, void) are commerce-payments concerns, noted as references rather than scheme steps - Use cases reflect fraud protection / refundable payments, not session billing Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Adds specification documents for the
escrowscheme following x402'sspecs/schemes/<name>/structure:scheme_escrow.md— High-level overview: settlement methods (authorize vs charge), lifecycle, relationship toexact, security considerationsscheme_escrow_evm.md— EVM implementation: PaymentRequirements/PaymentPayload examples, verification logic, settlement logic, PaymentInfo struct, expiry ordering, fee systemThese specs document the scheme built on Commerce Payments Protocol and can serve as the basis for a future upstream proposal to coinbase/x402.
Verified against
AuthCaptureEscrow.sol) — PaymentInfo struct, expiry ordering, fee validation all matchPaymentOperator.sol— confirmed 4-paramauthorize()/charge()wrapper matches our OPERATOR_ABI (escrow's rawcharge()takes 6 params but the operator wraps it)Fixes applied during review
payer=address(0)(payer-agnostic) detailsignature→collectorData(matches ABI param name)PaymentOperatornaming — uses generic "operator" throughoutchargedescription: refundable post-settlement, not non-refundable🤖 Generated with Claude Code