feat(MayanFacet): signed arbitrary destination-call payloads (EXSC-604)#2043
Conversation
…(EXSC-604) Stacked on EXSC-364. Relaxes doesNotContainDestinationCalls on both entrypoints and gates hasDestinationCall==true behind a backend EIP-712 signature over the BridgeData fields + keccak256(protocolData); the signed path skips _parseReceiver (receiver becomes signer-attested), consumes transactionId (CEI) for replay protection, and binds chainid + diamond via the domain separator. Plain bridges (hasDestinationCall==false) stay permissionless and unchanged. Reuses the shipped UnitFacet/NEARIntentsFacet pattern (solady ECDSA, immutable BACKEND_SIGNER, governance-only rotation). Version 2.0.0 -> 3.0.0 (constructor + ABI change). config/mayan.json backendSigner is intentionally the zero address so any deploy fails the constructor zero-check until BE provisions a DEDICATED Mayan signer key (must NOT reuse Unit's). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Keep the demo literal in sync with the 9-field MayanData struct. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Test Coverage ReportLine Coverage: 90.14% (3450 / 3827 lines) |
…-and-swapandforwardeth' into feature/exsc-604-sc-mayanfacet-support-arbitrary-destination-call-payloads
…ta (EXSC-604) MayanData gained signature+deadline; regenerate config/clearSigningProposal.json. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…anData order and set version 3.0.0
…C-604) Per decision to reuse the existing shared signer key rather than a dedicated one: config/mayan.json.backendSigner now holds the shared value (matches config/global.json and config/unit.json). Removes the earlier zero-address fail-safe placeholder; no separate key provisioning is needed before deploy. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Closing: we decided not to implement arbitrary destination-call support on Mayan Swift v2 for now (EXSC-604 → Canceled). The signed-payload mechanism worked, but the blocker is destination-call failure handling: Mayan Swift has no LI.FI Receiver in the flow, so our usual 'deliver at least the bridged token to the user on the destination if the destination swap fails' guarantee does not apply. A failed/unfulfilled Swift order refunds to the source trader after the deadline, and what happens when the opaque customPayload destination call itself reverts is undocumented in Mayan's public docs and unverified. Not shipping an opaque arbitrary-call surface with unclear failure semantics. EXSC-364 (#2042, ETH routing) is unaffected and stays open — this PR was only stacked on it. Can reopen if we revisit. |
Which Linear task belongs to this PR?
https://linear.app/lifi-linear/issue/EXSC-604/sc-mayanfacet-support-arbitrary-destination-call-payloads-swift-v2
Why did I implement it this way?
MayanFacet forbade destination calls (
doesNotContainDestinationCallson both entrypoints). Mayan Swift v2 orders can carry acustomPayloadthat makes the destination executor perform an arbitrary call, whose target/calldata cannot be validated on-chain againstbridgeData.receiver. Per.agents/rules/102-facets.md(opaque calldata flows), the sanctioned mitigation is a backend EIP-712 signature committing to theBridgeDatafields plus a hash of the opaque calldata — exactly the pattern already shipped inUnitFacet/NEARIntentsFacet, which this reuses verbatim.Design:
hasDestinationCall == true→ verify a backend signature at the top of each entrypoint (before any deposit/swap), over{transactionId, receiver (NON_EVM-aware), minAmount, sendingAssetId, destinationChainId, hasDestinationCall, mayanProtocol, keccak256(protocolData), deadline}. On this path the receiver is signer-attested;_parseReceiveris skipped.transactionIdis consumed CEI (marked used before the external Mayan call) for replay protection; the domain separator bindsblock.chainid+address(this)(diamond, under delegatecall).hasDestinationCall == false→ unchanged, permissionless_parseReceiverpath. No signature required; existing traffic is not gated.MayanDataappendsbytes signature+uint256 deadline(after 364's four fields; 364 order untouched). ImmutableBACKEND_SIGNERset in the constructor; rotation is a governed facet-redeploy +diamondCut(no owner setter).solady/utils/ECDSA(reverts on malformed sig — no silentaddress(0))._parseReceiver,_parseHypercoreReceiver,_normalizeAmount,_replaceInputAmount, and 364's native-swap branch are untouched.Version:
2.0.0 → 3.0.0(constructor-signature + ABI change — MAJOR).Security review: an independent adversarial pass (forgery / replay / ECDSA / branch-integrity / coexistence / storage / test-quality lenses, cross-checked against the Unit/NEAR references) found no critical or high-severity issue. Two informational notes: (1) the native source-swap params (
swapProtocol/swapData/middleToken/minMiddleAmount) are intentionally not in the signed struct — not theft-exploitable, since the destination call lives in the committedprotocolDataand only the submitter's ownmsg.valueis at stake; (2) signature malleability is harmless because replay keys ontransactionId(in the digest), not the signature bytes.Testing:
forge test --match-contract MayanFacet→ 55 pass, incl. H1–H6 happy paths and N1–N10 negative guards (forged payload incl. customPayload-only tamper, absent/wrong signer, receiver/route mismatch, expired, same- and cross-chain replay, flag-flip both directions, malformed sig). Clean under the solc-0.8.17/london floor build (no stack-too-deep).Backend signer
config/mayan.json.backendSignerreuses the shared LI.FI backend signer — the same key Unit/NEAR use (config/global.json) — per an explicit decision to reuse the existing signer rather than provision a dedicated one. No separate key setup is required before deploy. (Tests use a stub signer and are independent of the config value.)Follow-ups (separate tickets, out of scope here)
customPayloadsmuggling hardening (design OQ-3).refundExcessNative(msg.sender)→ explicitrefundRecipient(design OQ-4; pre-existing).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 security review done instead, documented aboveChecklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)