feat: add credential validation and settlement APIs#597
Conversation
commit: |
effcf69 to
0e08e87
Compare
fdef078 to
e4a6b90
Compare
e4a6b90 to
1a46b24
Compare
tempoxyz-bot
left a comment
There was a problem hiding this comment.
👁️ Cyclops Review
PR #597 preserves the existing settlement path, but the new advisory Tempo validation surface has two actionable security gaps. Details are inline.
Reviewer Callouts
- ⚡ Advisory API semantics: Downstream integrations may use
validateCredentialfor pre-auth/risk workflows; document exactly what it guarantees and do not present it as proof of payment until replay and sender-authentication gaps are closed. - ⚡ Future split-method hooks: No current method defines both
validateand method-levelsettle, but future methods that do must keepsettle()independently sound and not rely onvalidate()side effects. - ⚡ Validation result/error contract:
Validation.sourceechoescredential.source, andvalidateCredential()can surface raw RPC/viem errors; consumers should use method-authenticated fields and handle non-PaymentErrorfailures.
| throw new MismatchError('Only Tempo (0x76/0x78) transactions are supported.', {}) | ||
|
|
||
| const client = await getClient({ chainId }) | ||
| const transaction = Transaction.deserialize(serializedTransaction) |
There was a problem hiding this comment.
🚨 [SECURITY] Tempo pull validation trusts unauthenticated deserialized senders
This branch treats Transaction.deserialize(serializedTransaction).from plus signature presence as authenticated identity and later returns it as details.sender. Tempo envelopes can carry an explicit sender slot in both 0x78 and 0x76 forms; the parser populates from from that slot and only recovers from the appended signature if from is absent. A challenge-bound transaction can therefore validate while naming a victim sender even though the raw sender signature was produced by someone else.
Recommended Fix: Recover/verify the sender signature against the exact submitted envelope before trusting transaction.from; reject explicit sender slots that are not cryptographically bound, and do not treat parser output or eth_call as proof of signature ownership.
| return request as unknown as z.output<typeof Methods.charge.schema.request> | ||
| })() | ||
| const chainId = resolvedRequest.methodDetails?.chainId ?? request.chainId | ||
| async validate({ credential, request }) { |
There was a problem hiding this comment.
🚨 [SECURITY] Tempo validation reports consumed or un-settleable credentials as valid
The new non-mutating validation hook never reads the replay markers that settlement writes for hash, proof, and pull transaction credentials. A credential that has already been settled can still return a successful validateCredential() result even though settleCredential() would reject it as used. The sponsored pull branch also only performs static call/fee-token checks and skips settlement's sponsored-sender reservation, sponsor policy/filling, and pre-broadcast simulation.
Recommended Fix: Keep validation non-mutating but read replay keys with Store.get before success, and share read-only/pure sponsored-transaction policy checks plus non-mutating simulation where possible. If a full settleability check cannot be done, return an explicit lower-confidence/shape-only validation result instead of unconditional success.
25a5330 to
e15a622
Compare
Motivation
Some integrations need a way to inspect whether a payment credential currently matches a challenge before deciding to accept it and ultimately settle.
Validation should not reserve replay state, sign fee-payer transactions, broadcast payments, or otherwise mutate payment state. Settlement remains the authoritative path and re-runs validation before consuming the credential.
This ~somewhat mirrors the x402 verify flow, and also has the benefit of making this code more explicit.
Summary
validateCredentialfor non-mutating credential pre-checks.settleCredentialfor explicit credential consumption and settlement.verifyCredentialas the backwards-compatible settlement path.Motivation
Key design considerations
verifyCredentialand receive the same receipt-oriented behavior.verifysupport: