ethereum-optimism / ethereum-optimism/actions

Sentinel recipient leaks via actions.swap.getQuote — split price quote from executable quote

Open
#435 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.