feat(MayanFacet): route native ETH via swapAndForwardEth (EXSC-364) [MayanFacet v2.0.0, IMayan v1.1.0]#2042
Conversation
Native sends now route through the Mayan forwarder's swapAndForwardEth when MayanData.swapProtocol is set (Mayan's implicit 1:1 ETH->WETH source swap), and keep forwardEth when it is not; ERC20 sends are unchanged. MayanData gains swapProtocol/swapData/middleToken/minMiddleAmount; _parseReceiver is unchanged so on-chain receiver validation still gates every path. Appending struct fields changes both entrypoint selectors, so this is a MAJOR bump (1.3.0 -> 2.0.0). Unblocks BE ticket EXBE-355 by defining the calldata shape. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughMayan bridging now accepts native swap parameters and explicit refund recipients, conditionally routes native assets through Mayan’s swap forwarder, updates deployment and signing metadata, and adds native and ERC20 demo flows with expanded tests. ChangesMayan native swap forwarding
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…XSC-364) Deployed 0x6b380169D4C02446703b97E6c3a5dA514039A897, explorer-verified, diamondCut applied (old selectors removed, new added — confirmed via loupe). Diamond-registry churn from the incomplete staging master log was reverted; only the accurate MayanFacet delta is committed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-364) MayanData gained the swap-layer fields, changing both Mayan entrypoint signatures; regenerate config/clearSigningProposal.json to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/solidity/Facets/MayanFacet.t.sol (1)
619-630: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert
mayanProtocolforwarding too.The recorder stores
lastMayanProtocol, but this test never verifies it. Assert it equalsdata.mayanProtocolto cover every newly forwarded destination argument.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/solidity/Facets/MayanFacet.t.sol` around lines 619 - 630, Extend the assertions in the Mayan forwarding test to verify forwarder.lastMayanProtocol() equals data.mayanProtocol, alongside the existing checks for the other forwarded destination arguments. Use the recorder and input symbols already present in this test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Facets/MayanFacet.sol`:
- Around line 53-67: Add a nonzero refundRecipient field to MayanData and use it
instead of msg.sender for excess-native refund handling in both entrypoints and
for leftover refunds passed to _depositAndSwap. Validate the recipient before
processing, and update the associated ABI metadata and documentation to reflect
the new tuple field.
In `@src/Interfaces/IMayan.sol`:
- Around line 21-29: Add NatSpec documentation above swapAndForwardEth
describing its purpose and include an `@param` entry for amountIn, swapProtocol,
swapData, middleToken, minMiddleAmount, mayanProtocol, and mayanData, with clear
descriptions that identify the trust-sensitive protocol and calldata inputs.
---
Nitpick comments:
In `@test/solidity/Facets/MayanFacet.t.sol`:
- Around line 619-630: Extend the assertions in the Mayan forwarding test to
verify forwarder.lastMayanProtocol() equals data.mayanProtocol, alongside the
existing checks for the other forwarded destination arguments. Use the recorder
and input symbols already present in this test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ae30449d-67c6-46e2-909d-b658c367e6e5
📒 Files selected for processing (9)
config/clearSigningProposal.jsondeployments/arbitrum.diamond.staging.jsondeployments/arbitrum.staging.jsondocs/MayanFacet.mdscript/demoScripts/demoMayan.tssrc/Facets/MayanFacet.solsrc/Interfaces/IMayan.soltest/solidity/Facets/CalldataVerificationFacet.t.soltest/solidity/Facets/MayanFacet.t.sol
Address CodeRabbit finding — interface functions require @notice + @param per [CONV:INTERFACE-NATSPEC]. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
….sender (EXSC-364) Both entrypoints refunded excess native / swap leftovers to msg.sender, which is relayer-unsafe per [CONV:FACET-REFUNDS] (CodeRabbit Major). Add refundRecipient to MayanData (zero-checked, revert InvalidCallData), route refundExcessNative and _depositAndSwap through it. Folds EXSC-606 into this already-breaking ABI change to avoid a third selector churn. Version 2.0.0 -> 3.0.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e PR) Revert the erroneous 2.0.0->3.0.0 bump; version is pure semver and this PR is a single release. Staging redeploys of the same version use an incremented SALT in .env, not a version bump. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…XSC-364) - Rewrite demoMayan.ts to exercise the native ETH -> swapAndForwardEth path with real Mayan data (was ERC20/WETH-only); keep the legacy forwardERC20 path behind MAYAN_DEMO_MODE=erc20 and fix its stale USDC labels + hard-coded minAmount. - Bump @mayanfinance/swap-sdk 8.5.0 -> 15.0.0: the old SDK is rejected by Mayan's quote API (SDK_VERSION_TOO_OLD). getSwapFromEvmTxPayload is now async. - Redeploy MayanFacet v2.0.0 to Arbitrum staging (0x9923249c...) and cut it onto the diamond; the prior staging deploy predated the refundRecipient field, so the branch selectors were never registered. - Record the verified staging tx in the demo header: 0.001 ETH (Arbitrum) -> ~1.8388 USDT (Polygon), Mayan ORDER_SETTLED, source 0xa67bb19b...dd1f6c. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
script/demoScripts/demoMayan.ts (1)
83-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd explicit return types to the new async functions.
Declare
Promise<...>return types for the quote finder, both runners, andmain.As per coding guidelines, “Use explicit return types for functions in TypeScript.”
Also applies to: 127-130, 217-220, 302-302
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@script/demoScripts/demoMayan.ts` around lines 83 - 86, Update findNativeSwapAndForwardEthQuote, the two runner functions, and main with explicit Promise<...> return type annotations, using each function’s actual resolved return value; do not alter their existing behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@script/demoScripts/demoMayan.ts`:
- Around line 310-319: Validate the normalized mode in the demo entrypoint
before creating the provider, signer, or invoking either transaction flow.
Accept only the supported values used by runErc20ForwardErc20 and
runNativeSwapAndForwardEth (erc20 and native); reject any other value, including
typos, with an error rather than defaulting to native.
- Around line 99-116: Update the catch around getSwapFromEvmTxPayload to retain
and surface the failure before continuing, including the relevant route or quote
context in the log. Preserve the existing continue behavior so failed payload
generation still skips to the next candidate instead of reaching the generic “No
native-ETH SWIFT quote” path.
- Around line 249-251: Validate the decoded transaction method immediately after
iface.parseTransaction in the Mayan transaction flow, and reject any method
other than forwardERC20 before accessing parsed.args.amountIn or protocolData.
Keep the existing amount derivation unchanged for valid forwardERC20
transactions.
---
Nitpick comments:
In `@script/demoScripts/demoMayan.ts`:
- Around line 83-86: Update findNativeSwapAndForwardEthQuote, the two runner
functions, and main with explicit Promise<...> return type annotations, using
each function’s actual resolved return value; do not alter their existing
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 63fd72c7-d3ab-4b09-b192-5283ec9f75c0
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
deployments/arbitrum.diamond.staging.jsondeployments/arbitrum.staging.jsonpackage.jsonscript/demoScripts/demoMayan.tssrc/Facets/MayanFacet.sol
The native swapAndForwardEth path does not assume WETH: Mayan selects the Swift v2 order's input token (middleToken) per route/quote — it may be a stablecoin (verified staging run: Arbitrum ETH -> Polygon USDT used USDT as middleToken) — and the source-side conversion is a real DEX swap, not a 1:1 ETH->WETH wrap. Docs only; no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…cs (self-review) Two inline comments still said the native source swap is ETH->WETH; corrected to ETH->middleToken to match the doc fix (Mayan picks the token per route). Also drop an unverified 'SDK v9+' precision from the async note. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Surface getSwapFromEvmTxPayload failures (route + error) instead of an empty catch that folds auth/network errors into the generic no-quote path. - Guard the ERC20 path: reject a decoded method other than forwardERC20 before reading amountIn/protocolData (mirrors the native-path guard). - Reject unsupported MAYAN_DEMO_MODE instead of silently defaulting to native — the script broadcasts a real tx. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| _mayanData.swapProtocol, | ||
| _mayanData.swapData, | ||
| _mayanData.middleToken, | ||
| _mayanData.minMiddleAmount, |
There was a problem hiding this comment.
minMiddleAmount is forwarded straight through as the only slippage guard on the native ETH→middleToken source swap, but the facet never requires it to be > 0. At zero, the forwarder's middleAmount >= minMiddleAmount check is a no-op and the intermediate swap can be fully sandwiched — the order gets created for a dust amount and the user receives almost nothing on the destination chain.
This is a new value-at-risk surface introduced by the swapAndForwardEth path. It's caller/backend-supplied per the aggregator pattern, so this may be an accepted "backend responsibility" — but if so, please state that explicitly in the audit notes, and consider whether a require(minMiddleAmount > 0) (or a receiver-derived floor) is warranted here given the source swap executes with the user's native value.
| "Version": "1.0.0" | ||
| }, | ||
| "0xEB6803A40B8350286e1E8684068E28c5c57a94AE": { | ||
| "0x6b380169D4C02446703b97E6c3a5dA514039A897": { |
There was a problem hiding this comment.
This diamond staging map now lists MayanFacet 2.0.0 at two addresses — 0x6b380169… here and 0x9923249c… a few lines below — while the flat arbitrum.staging.json registers only 0x9923249c…. The same hunk also adds CalldataVerificationFacet / PolymerCCTPFacet / SymbiosisFacet addresses that don't match the flat file's entries for those facets.
The PR body explains a re-cut happened after the refundRecipient field was added, so 0x6b380169… is most likely a stale pre-refundRecipient artifact. Please reconcile the two deployment logs before merge — deploy/verify tooling that iterates the diamond facets map (verify-contracts, multisig-rollout) could otherwise act on a facet address that was never cut into the live diamond, or report a spurious mismatch that blocks the staging rollout.
…ayanfacet-route-between-forwardeth-and-swapandforwardeth # Conflicts: # deployments/arbitrum.diamond.staging.json
Which Linear task belongs to this PR?
https://linear.app/lifi-linear/issue/EXSC-364/sc-mayanfacet-route-between-forwardeth-and-swapandforwardeth-by
Also closes EXSC-606 (route refunds to an explicit
refundRecipientinstead ofmsg.sender) — folded in here to avoid a secondMayanDataABI break, per the refund-routing convention call (agreed by Daniela, Goran, Max).Unblocks BE EXBE-355 — this PR defines the
MayanDatacalldata shape the backend consumes.Why did I implement it this way?
MayanFacetpreviously routed all native-ETH sends through the Mayan forwarder'sforwardEth. A Mayan Swift v2 order takes a specific ERC20 input token, not native ETH, so to offer native ETH as a sending token the forwarder exposesswapAndForwardEth, which performs a source-side swap (viaswapProtocol/swapData) converting the native input into the order's input token (middleToken) before creating the order. ThemiddleTokenis chosen by Mayan per route/quote — it may be WETH or another token such as a stablecoin — and the conversion is a real DEX swap, not a fixed 1:1 wrap. (The verified staging run below used USDT asmiddleTokenfor an ETH→Polygon-USDT route.)Design — extend
MayanData, discriminate at the native branch. Four swap-layer fields are appended toMayanDatafor the native path, plus one refund field (see below). The full struct:_startBridgenative branch:forwardERC20(unchanged)swapProtocol != address(0)→swapAndForwardEth(minAmount, swapProtocol, swapData, middleToken, minMiddleAmount, mayanProtocol, protocolData)swapProtocol == address(0)→forwardEth(retained)The backend selects the path per-quote by populating (or not) the swap-layer fields — the facet needs no knowledge of Mayan's ETH/WETH order semantics.
_parseReceiver/_parseHypercoreReceiverare byte-for-byte unchanged, so on-chain receiver validation (derived fromprotocolData, run before the branch) still gates every path. The new opaqueswapData/swapProtocolonly govern the source-side conversion executed inside the immutable, trustedMAYANforwarder (which itself allowlistsswapProtocol). No EIP-712 gate is required — unlike rule 102's opaque-receiver case, the final receiver stays on-chain-validatable.Refund routing (EXSC-606, folded in). Both entrypoints previously refunded excess native and swap leftovers to
msg.sender. Per.agents/rules/102-facets.md[CONV:FACET-REFUNDS],msg.senderis the relayer (or the Permit2Proxy on gasless flows), never assumed to be the user, so user-owned refunds get stranded. This PR adds an explicitrefundRecipienttoMayanData, zero-address-checked on both entrypoints (revertInvalidCallData), and routes bothrefundExcessNative(payable(refundRecipient))and the_depositAndSwapleftover receiver there. The backend setsrefundRecipientto the owner of the source-side input per order (relayer when the relayer fronts the native, user when the user funds it). Since we were already breaking theMayanDataABI for EXSC-364, folding this in adds one struct field with no additional selector churn.Versioning. Appending struct fields changes both entrypoint selectors, a breaking ABI change for existing callers → MAJOR bump
1.3.0 → 2.0.0(one MAJOR bump covers the whole PR):startBridgeTokensViaMayan0xb621b032 → 0xa2eff2daswapAndStartBridgeTokensViaMayan→ 0x2abf2290IMayan1.0.0 → 1.1.0(genuinely newswapAndForwardEthfunction).Testing.
forge test --match-contract MayanFacet→ 39 pass, incl. new tests for the nativeswapAndForwardEthpath (arg pass-through against a recorder etched onto the realMAYANimmutable), theswapAndStartBridgedouble-swap edge (proves_replaceInputAmountstays skipped for native), a receiver-mismatch revert on the new path, and the EXSC-606 refund tests (zero-refundRecipientreverts on both entrypoints; excess native lands onrefundRecipient, notmsg.sender).Staging E2E (real Mayan data).
script/demoScripts/demoMayan.tswas rewritten to exercise the nativeswapAndForwardEthpath end-to-end with a live Mayan quote (previously it only covered the ERC20 path). This required bumping@mayanfinance/swap-sdk8.5.0 → 15.0.0(the pinned SDK is rejected by Mayan's quote API:SDK_VERSION_TOO_OLD). Verified run on Arbitrum staging: 0.001 ETH → ~1.8388 USDT delivered to the receiver on Polygon, Mayan statusORDER_SETTLED. Source tx:0xa67bb19b…dd1f6c.Review. Local CodeRabbit (
/pr-ready) bypassed per the sanctioned interim (rate limits — see #2020); an independent adversarialcode-reviewerpass was run instead (correctness + rule-102 opaque-swapDatalens;IMayanverified byte-exact against the live forwarder ABI) with no findings. Cloud CodeRabbit + Aikido run on this PR.Deploy. MayanFacet v2.0.0 deployed + verified on the Arbitrum staging diamond at
0x9923249c1f14E4BB68F71C689C3fCC323E5410aCand cut in (selectorsstartBridgeTokensViaMayan 0xa2eff2da,swapAndStartBridgeTokensViaMayan 0x2abf2290,MAYAN() 0xce90a721). Note: an earlier staging deploy in this PR predated therefundRecipientfield, so it carried the pre-refundRecipientselectors; the diamond has now been re-cut to the current build (confirmed via DiamondLoupe).Follow-up. EXSC-604 (signed arbitrary destination-call payloads, EIP-712) has been descoped / cancelled (PR #2043 closed) — it is no longer stacked on this branch. The remaining Mayan hardening item is EXSC-605 (reject a non-empty
customPayloadon the unsigned bridge path), tracked separately; it is a pre-existing surface not widened by this PR.Checklist before requesting a review
/pr-ready(local CodeRabbit) on this branch and resolved (or explicitly documented) all findings — bypassed per the sanctioned interim (rate limits); independent adversarial review done instead, documented aboveChecklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)