fix(swaps): send the initiating EOA as the 1inch origin - #201
fix(swaps): send the initiating EOA as the 1inch origin#201haydenshively wants to merge 2 commits into
Conversation
Unanchored, `.claude/worktrees/` matches that segment at any depth, so knip run from inside a nested worktree treats the whole tree as gitignored, finds no entry points, and reports every file unused — failing the pre-commit hook for any worktree based on main. Git behavior is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
0505b71 to
6bc4a8c
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bc4a8cc15
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| from: params.executor, | ||
| origin: params.executor, | ||
| receiver: params.executor, |
There was a problem hiding this comment.
Send the initiating EOA as origin
When either liquidator enables 1inch via ONEINCH_API_KEY, this request now omits a parameter that the v6.1 OpenAPI schema declares required, so /swap can reject every firm quote before returning calldata. The cited Go SDK behavior does not establish equivalence: its non-omitempty tag sends origin= even when unset, whereas this object removes the query key entirely. Thread the signer EOA into QuoteParameters and send it as origin rather than making the supported 1inch path depend on undocumented missing-parameter behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed, and fixed in 86afec04 — the EOA is now threaded through QuoteParameters.initiatingEoa and sent as origin.
You were right on the specific logical hole: I cited the Go SDK's non-omitempty tag as evidence that omitting the key is safe, when it actually shows the SDK sends origin= — a different request from the one deletion produces. That inference doesn't survive contact with the tag it rests on.
The deciding factor was the failure mode rather than the schema question, which I still can't settle without a key: http-client.ts:137-141 maps any non-429/401/403 4xx to no_route, so a schema rejection would be indistinguishable from a genuine no-route and would sideline 1inch silently rather than loudly. Threading the EOA is correct under every reading of the schema — presence-required, EOA-validated, or ignored — so it's the only option that doesn't gamble.
The adapter now fails loud (api_error, before any HTTP call) when initiatingEoa is absent, matching the tokenInDecimals precedent in liquidswap.ts. Both liquidators pass their existing signer.account.address.
1inch Classic Swap distinguishes the caller (`from`) from the initiating EOA (`origin`); the adapter sent the Executor as both, and the Executor is a contract by construction. Thread the liquidator EOA — the address that really originates the transaction — through `QuoteParameters.initiatingEoa` and send that instead. `origin` is omitted from the v6.1 quickstart and from the Go SDK's own validation, but its swagger-generated field carries no `omitempty` — the same shape as the genuinely-required `from` and `slippage` — so the schema appears to require it. Deleting the parameter would bet the venue on undocumented behavior for a request `http-client` classifies as `no_route` on any 4xx, which is indistinguishable from a real no-route and would sideline 1inch silently. The adapter now fails loud when the EOA is absent instead. Both venue assertions become exact: `toMatchObject` ignored the extra key, which is why the wrong `origin` went unnoticed since #24. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
6bc4a8c to
86afec0
Compare
Closes BOTS-100.
What
packages/swaps/src/venues/oneinch.tssent the Executor address as bothfromandoriginon the Classic Swap v6.1/swaprequest.fromis right — the Executor is what calls the router.originis 1inch's initiating EOA, and the Executor is a contract by construction, so we were sending a value the API does not expect.This threads the liquidator EOA — the address that genuinely originates the transaction (EOA → Executor → router) — through a new optional
QuoteParameters.initiatingEoa, and sends that asorigin.Why not simply delete
originThat was this PR's first approach, and it was wrong. The case for deletion rested on 1inch's own Go SDK:
GetSwapParams.Validate()requiresSrc/Dst/Amount/From/Slippageand never mentionsOrigin, theexamples/swapprogram omits it, and the v6.1 quickstart documents onlysrc,dst,amount,from,slippage,disableEstimate,allowPartialFill.But the swagger-generated struct is
Origin string \url:"origin" json:"origin"`with **no**omitempty— the same shape as the genuinely-requiredFromandSlippage, where truly-optionalReceiverandFeeboth have it. So the OpenAPI spec appears to declareoriginrequired. And the "1inch's own SDK omits it" inference does not hold: precisely *because* that tag lacksomitempty, the Go SDK emits a bareorigin=` rather than dropping the key, which is a different request from the one deletion would produce. (Thanks to the Codex review for catching that my own supporting evidence didn't support the conclusion.)Threading the EOA is the only option that is correct under every reading of the schema — presence-required, EOA-validated, or ignored entirely — and it is what BOTS-100 originally proposed. Sending a truthful
origincosts one optional field and two call sites; guessing wrong costs the venue.Why guessing wrong is expensive
packages/swaps/src/http-client.ts:137-141classifies any non-429/401/403 4xx as'no_route'. If the schema wins and/swapstarts rejecting the request, that rejection is indistinguishable from a genuine no-route: 1inch would quietly fall out of the venue rotation and drive position backoff instead of failing loud. Not a loss — the min-out floor and the router pin are untouched — but a silently degraded liquidator, which is the failure mode this repo's conventions push hardest against.Rather than default-and-continue, the adapter now fails loud (
api_error) wheninitiatingEoais absent, matching the existingtokenInDecimalsprecedent inliquidswap.ts:26-32. A caller that forgets to thread it gets a visiblequote.failed, not a silent omission.Shape
QuoteParameters.initiatingEoa?: Address— optional, so every other venue and their callers/tests are unaffected.composeMultiVenueQuotingtakesinitiatingEoaalongsideexecutorand threads it intofirmQuoteVenue.initiatingEoa: eoa, which eachindex.tsalready had assigner.account.address.PriceParametersis untouched — the indicative/quoteprobe has no taker.Tests
toMatchObjectignored the extraoriginkey, which is why the wrong value survived since #24. Both venue assertions are now exacttoStrictEqualover the full search-param set, plus the endpoint.toStrictEqualrather thantoEqualbecausetoEqualignores explicitly-undefinedproperties, soorigin: undefinedwould have slipped past the very check this adds — verified it fails both ways.New case:
fails loud rather than quoting without an initiating EOAassertsapi_errorand that no HTTP call was made.Risk
Funds-adjacent but narrow: this changes one query parameter on a venue request and adds a required input to the quoting seam. No calldata, ABI, target address, sizing, or slippage bound moves —
minReturn,ONEINCH_ROUTER, and thetx.topin are all as they were. 1inch remains opt-in viaONEINCH_API_KEY, and that key is unset in every environment we run, so nothing in production changes behavior today.Still worth doing before 1inch is enabled anywhere: one live
/swapcall with a real key, to confirm the request shape end to end. That is the check neither I nor the reviewers could run.Second commit (unrelated, required)
chore(repo): anchor the worktrees ignore rule for knipanchors/.claude/worktrees/in.gitignore. Unanchored, that pattern matches the segment at any depth, sopnpm kniprun from inside a nested worktree sees the whole tree as gitignored, finds no entry points, and reports every file unused — failing the pre-commit hook for any worktree based onmain. Git behavior is identical.The same one-line change already exists on the branch behind #200, byte-identical. Note #200 also adds two entries a couple of lines below
.claude/agent-memory/, so the two PRs overlap within one diff hunk — whichever lands second needs a trivial rebase. Happy to split this into its own PR and rebase both #200 and #201 onto it if that is cleaner.Checks
pnpm --filter @repo/swaps run typecheck, both liquidatorspnpm lintpnpm formatpnpm knipmainfrom any worktree)pnpm testThe 4 remaining test files are fork/e2e suites needing
RPC_URL_8453, unset locally and provided in CI.🤖 Generated with Claude Code