chore(portfolio-contract): swap reward tokens to USDC via 1inch by rabi-siddique · Pull Request #12706 · Agoric/agoric-sdk

rabi-siddique · 2026-06-04T10:14:34Z

closes: https://linear.app/agoric/issue/PAK-462/ymax-contract-should-be-able-to-swap-reward-tokens-to-usdc-on-1inch

Description

Adds a contract-side mechanism to swap reward tokens to USDC on EVM chains via 1Inch, as part of the reward-token auto-claiming cycle.

Security Considerations

This introduces a new authority: from the portfolio's own remote account, the contract grants the configured 1inch router an ERC20 approval and calls its swap() entry point. It does not forward an opaque planner-supplied calldata blob — the planner supplies the swap as decomposed named fields and the contract reconstructs the swap() calldata itself, pinning the fund-safety fields it controls:

srcToken = the reward token being approved,
dstToken = USDC,
dstReceiver = this portfolio's own remote account,
amount = the approved amountIn, and
minReturnAmount = the movement amount (the USDC floor, which Zoe validates).

These are correct by construction, so proceeds can only ever arrive as USDC in our own account, pulling no more than approved, and the router reverts unless at least minReturnAmount reaches dstReceiver. The target is pinned to the configured router and the selector to 1inch V6 swap(). executor, srcReceiver, and data are opaque route internals, trusted only because the planner produced them — they cannot redirect funds given the pinned fields. The allowance is reset to 0 after the swap so the router keeps no residual grant.

Testing Considerations

Unit tests coverage added.
A contract-level test chaining a claim → swap step will be added with the claim-support PR (claims aren't available here to drive that flow end-to-end).
1inch has no testnet support, so end-to-end validation requires main0 testing before rolling-out on main-1.

Upgrade Considerations

Deployment must be sequenced contract-first: the contract must support the new swap step before the planner produces one. The contract upgrade is backward-compatible — it adds handling for MovementDesc.swap but never requires it — so it can ship ahead of the planner and simply stays inert until the planner is upgraded. The reverse order would let the planner submit swaps an older contract can't process.

Ordering

ymax0 (main-0) contract upgrade first — adds swap-step support; no behavior change until its planner produces swaps.
ymax0 planner deployment — starts calling the 1inch Swap API and submitting swap requests (MovementDesc.swap); validate the full reward-token → USDC swap flow end-to-end against the upgraded contract.
ymax1 (main-1) contract upgrade, then its planner — only after end-to-end verification on ymax0 looks good.

dckc

if a more substantive review is needed from me, we should talk

dckc · 2026-06-05T15:44:34Z

  await documentStorageSchema(t, storage, docOpts);
 });

+test('swap reward token to USDC via 1inch', async t => {


I trust that in due course, there will be a contract-level feature test that shows how end-users use this new code.

I expect a contract level test to be somewhat representative, and chain a claim and a swap step. I think it's ok to postpone until the claim PR introduces such a test.

mhofman

I think we still have some bits to iron out on the design, but the overall direction looks good so far.

mhofman · 2026-06-05T16:44:42Z

Out of scope for this PR, but we should probably refactor some of this to use the newer tools we have to generate call data.

More details for the future: what I'm thinking about is better separation of concerns. Generating the call info (aka target + encoded calldata, but also gasLimit and sent value for router calls) that the messaging layers care about can be layered on top of a more basic tool that builds the encoded calldata from abi / function signature. I think makeEvmContract and contractWithCallMetadata (or something like it) builds that separation, but I haven't tried to adopt it throughout the existing codebase.

rabi-siddique · 2026-06-08T05:13:52Z

codegen CI failure:
https://github.com/Agoric/agoric-sdk/actions/runs/27113973754/job/80017490932?pr=12706

frazarshad · 2026-06-08T10:07:16Z

  functionSignature,
  args,
  abi,
+  data,


nit: data seems a bit too generic. maybe something more specific like encodedData would better clarify the purpose of this arg

frazarshad · 2026-06-08T11:06:26Z

i believe an update to privateArgs-ymax1.json and privateArgs-ymax0.json would be required as well

Good catch - updated both privateArgs-ymax0.json and privateArgs-ymax1.json with oneInchRouter on all 5 chains.

frazarshad · 2026-06-08T11:07:13Z

    aaveUSDC: aaveUsdcAddresses.testnet.Base,
    aaveRewardsController: aaveRewardsControllerAddresses.testnet.Base,
    walletHelper: walletHelperAddresses.testnet.Base,
+    oneInchRouter,


i thought testnet didnt have a working oneInch contract?

## What Pin `packages/orchestration`'s chain-info codegen to a specific cosmos [chain-registry](https://github.com/cosmos/chain-registry) commit so it is deterministic, and refresh the committed chain info. ## Why The `test-codegen` job runs `scripts/verify-codegen-idempotence.mjs`, which runs `yarn codegen` in every package and fails on any resulting diff. Orchestration's codegen fetched live data from chain-registry `master`, so it was **not** a pure function of the repo: whenever upstream added IBC connections/channels, the regenerated `src/fetched-chain-info.js` diverged from the committed file and the check failed — on **unrelated** PRs, repo-wide, until someone refreshed. This is currently breaking CI on unrelated work, e.g. the `test-codegen` failure in #12706: https://github.com/Agoric/agoric-sdk/actions/runs/27113973754/job/80017490932?pr=12706 ## How - **Pin the fetch** to a specific chain-registry commit (the `CHAIN_REGISTRY_COMMIT` constant in `scripts/fetch-chain-info.ts`). `yarn codegen` fetches that immutable commit, so output is reproducible and upstream drift can no longer break CI. Fetching is still fine; drifting is not. - **`yarn codegen --refresh`** re-pins to the latest chain-registry commit (rewriting the constant in place) and regenerates, so updates are adopted deliberately. A refresh is a one-line bump to the constant plus the regenerated output. - Commit the latest chain info refreshed from the registry. - Document the refresh workflow in the package README ("Chain info" section). - Make the idempotence check's failure message explain exactly what to do, and note that codegen pulling external data must pin its source. ## Testing - `yarn codegen` (no args) reproduces `src/fetched-chain-info.js` byte-for-byte (idempotent) and leaves the script untouched. - `yarn codegen --refresh` re-pins and regenerates. - `tsc` and eslint pass on the changed files. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

mhofman

Overall looks good, but I think there is some cleanup remaining after the switch to recomposing the call data.

Also one question about which details should be published.

mhofman · 2026-06-09T06:31:50Z

More details for the future: what I'm thinking about is better separation of concerns. Generating the call info (aka target + encoded calldata, but also gasLimit and sent value for router calls) that the messaging layers care about can be layered on top of a more basic tool that builds the encoded calldata from abi / function signature. I think makeEvmContract and contractWithCallMetadata (or something like it) builds that separation, but I haven't tried to adopt it throughout the existing codebase.

mhofman

The swap params should not be plumbed through the context. I also think the minReturnAmount is extraneous (I do not see a way this could be anything but the USDC amount expected to be received in the step).

Besides that looks great.

mhofman · 2026-06-10T00:19:25Z

  await documentStorageSchema(t, storage, docOpts);
 });

+test('swap reward token to USDC via 1inch', async t => {


I expect a contract level test to be somewhat representative, and chain a claim and a swap step. I think it's ok to postpone until the claim PR introduces such a test.

mhofman · 2026-06-10T01:38:09Z

Security Considerations

This introduces a new authority: the contract grants the 1inch router an ERC20 approval and forwards planner-supplied calldata to it from the portfolio's remote account. The call target is pinned to the configured router, but the router is a generic swap engine, so the calldata is not trusted blindly - it is decoded and validated on-chain before the approval and forward.

Out of date, we recompose now.

Testing Considerations

Please note that a contract level test will be added once claim support is there.

Upgrade Considerations

Deployment must be sequenced; the contract can't process swap requests until the planner is capable of producing them, so the planner ships first.

I don't understand this. If anything I believe it should be the opposite. The contract must be deployed first to support new steps that the planner may generate.

mhofman

🎉

mhofman · 2026-06-10T03:46:55Z

+      dest: AnyString<AssetPlaceRef>(),
+    },
+    {
+      phases: M.recordOf(M.string(), M.arrayOf(M.string())),


did we really not have this in the type guard before? I guess it's not used but in tests...

Yes - it was never in the shape on master (just { how, amount, src, dest }), even though phases is published. FlowStepsShape is only used in offer-shapes.test.ts; publishStatus doesn't validate against it at runtime, and the fixtures never included phases, so the gap went unnoticed.

rabi-siddique force-pushed the rs-swap-reward-tokens branch 9 times, most recently from 147f6dd to 72ff993 Compare June 5, 2026 09:53

rabi-siddique marked this pull request as ready for review June 5, 2026 10:25

rabi-siddique requested review from dckc, frazarshad, gibson042 and michaelfig June 5, 2026 10:28

dckc reviewed Jun 5, 2026

View reviewed changes

mhofman reviewed Jun 5, 2026

View reviewed changes

rabi-siddique requested a review from mhofman June 8, 2026 05:16

frazarshad reviewed Jun 8, 2026

View reviewed changes

turadg mentioned this pull request Jun 8, 2026

chore(orchestration): make chain-info codegen deterministic #12714

Merged

mhofman reviewed Jun 9, 2026

View reviewed changes

rabi-siddique requested a review from mhofman June 9, 2026 07:55

mhofman reviewed Jun 10, 2026

View reviewed changes

mhofman mentioned this pull request Jun 10, 2026

feat: create claim rewards step #12718

Open

mhofman reviewed Jun 10, 2026

View reviewed changes

Comment thread packages/portfolio-contract/test/portfolio.flows.test.ts Outdated

Comment thread packages/portfolio-contract/test/portfolio.flows.test.ts Outdated

rabi-siddique requested a review from mhofman June 10, 2026 03:42

mhofman approved these changes Jun 10, 2026

View reviewed changes

rabi-siddique force-pushed the rs-swap-reward-tokens branch from 7249712 to 72e0062 Compare June 10, 2026 04:01

rabi-siddique added 4 commits June 10, 2026 09:16

chore(portfolio-api): add 1inch swap params to MovementDesc and FlowStep

162e13f

chore(portfolio-contract): swap reward tokens to USDC via 1inch

ef43980

test(portfolio-contract): add 1inch reward-token swap flow

b895ee1

chore(portfolio-deploy): configure 1inch AggregationRouterV6 address

4a2c5e2

rabi-siddique force-pushed the rs-swap-reward-tokens branch from 72e0062 to 4a2c5e2 Compare June 10, 2026 04:19

rabi-siddique added the automerge:rebase Automatically rebase updates, then merge label Jun 10, 2026

mergify Bot merged commit 2da8e5c into master Jun 10, 2026
115 checks passed

mergify Bot deleted the rs-swap-reward-tokens branch June 10, 2026 04:53

Conversation

rabi-siddique commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Security Considerations

Testing Considerations

Upgrade Considerations

Ordering

Uh oh!

dckc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhofman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rabi-siddique commented Jun 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhofman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mhofman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mhofman commented Jun 10, 2026

Security Considerations

Testing Considerations

Upgrade Considerations

Uh oh!

Uh oh!

Uh oh!

mhofman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rabi-siddique commented Jun 4, 2026 •

edited

Loading