ethereum-optimism / ethereum-optimism/actions
Sentinel recipient leaks via actions.swap.getQuote — split price quote from executable quote
- Dominant language
- TypeScript
- Stars
- 32
- Forks
- 26
- Avg merge
- 10h 20m
- Merged PRs (30d)
- 16
Description
Surfaced during review of #434.
## Problem
Quotes returned from \`actions.swap.getQuote(...)\` (no wallet bound) carry \`recipient = 0x0000000000000000000000000000000000000001\` (the Universal Router \`msg.sender\` sentinel) when no recipient is passed. After #434 these quotes throw at \`wallet.swap.execute\` because the sentinel won't match any wallet's address — but the type lies: \`SwapQuote.recipient: Address\` (required) suggests \"ready to execute\" while the value is actually un-executable.
This is a footgun for downstream consumers that read \`quote.recipient\` for display, accounting, analytics, or any non-execute purpose. They'd see and trust the sentinel as a real address.
There's also an encoder-level concern: Velodrome v2/leaf routers encode \`recipient\` directly into calldata. With the sentinel default, calldata for those chains has \`address(1)\` baked in. Today this is harmless because #434's strict check at \`wallet.swap.execute\` blocks submission. A defense-in-depth encoder guard (see review notes) was attempted but breaks price-only quote UX since v2/leaf chains can't produce a usable quote without a real recipient — pointing at the same root cause.
## Options
### A. Make \`recipient\` required on \`SwapQuoteParams\`
Caller passes a real address (their wallet, a UI placeholder they control, etc.). Quotes always carry a \"real\" recipient. \`actions.swap.getQuote\` becomes a thin wrapper requiring callers to think about who the quote is for.
Pros: smallest type change, eliminates the sentinel field-value mismatch.
Cons: API friction for pure price-display use cases.
### B. Split the type
\`PriceQuote\` (no recipient, returned by \`actions.swap.getQuote\`) vs \`SwapQuote\` (recipient required, returned by \`wallet.swap.getQuote\`). \`wallet.swap.execute\` only accepts \`SwapQuote\`.
Pros: type system encodes the distinction; \`PriceQuote\` is structurally un-executable; price-only consumers have a smaller surface.
Cons: larger refactor; affects routing code that compares quotes across providers.
### C. Router-type-aware recipient defaults in providers
Each provider's \`_getQuote\` decides the right \"no-recipient-supplied\" default based on its encoding. Universal-Router-only providers (Uniswap V4, Velodrome universal/cl) keep the sentinel. Velodrome v2/leaf throws at quote time when no recipient is supplied. Allows defense-in-depth at the encoder.
Pros: fixes the encoder safety hole as a side effect.
Cons: \`actions.swap.getQuote\` partially diverges by chain — surprising.
## Recommendation
**B** longer-term, **A** as a faster path. The current shape leaks an internal sentinel into a public field; either approach makes the type honest.
Encoder defense-in-depth follows naturally from B/A — once \`actions.swap.getQuote\` is forced to receive a real recipient, the encoder can refuse the sentinel on v2/leaf paths.
## Related
- #434 (PR — strict recipient match at execute, the change that surfaced this)
Contributor guide
Research direction
Start at the actions.swap.getQuote, wallet.swap.getQuote, and wallet.swap.execute entry points, then trace provider _getQuote implementations and the Velodrome v2/leaf encoders. Resolve whether the API should use option A or B, and verify that price-only quotes cannot expose an executable-looking recipient while executable quotes remain valid and encoder handling is safe.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- api, backend-api-design
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100