feat: add 1inchswap module#116
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a91409d. Configure here.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a91409d64f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (pendingSwap.orderHash != bytes32(0)) revert SwapAlreadyExecuted(); | ||
| if (orderHash == bytes32(0)) revert InvalidInput(); | ||
|
|
||
| _checkSignatures(EXECUTE_SWAP_SIG, safe, abi.encode(orderHash), signers, signatures); |
There was a problem hiding this comment.
executeSwap only verifies signatures over orderHash, and never checks that this hash represents the previously requested fromToken/toToken/fromAmount/minToAmount. In practice, a compromised backend can get owners to sign an opaque hash for a different order, then the router can pull fromAmount once authorized even if the fill does not match the original intent; settlement checks happen after funds move and cannot undo that trade. Bind execution authorization to the stored pending swap fields (or recompute/verify the exact order payload on-chain) before calling authorizeOrderHash.
Useful? React with 👍 / 👎.
| function revokeOrderHash(bytes32 hash) external { | ||
| if (!isModuleEnabled(msg.sender)) revert OnlyModules(); | ||
| delete _getERC1271Storage().authorizedOrderHashes[hash]; |
There was a problem hiding this comment.
revokeOrderHash is callable only by a currently enabled module. If an active Fusion order exists when governance disables/unwhitelists the 1inch module (a normal emergency response), the module can no longer call Safe cleanup paths, so authorized order hashes (and associated approvals) may remain uncleared. Providing an owner/admin emergency revoke path avoids getting stuck with live authorizations after module disable.
Useful? React with 👍 / 👎.
1 participant
Note
High Risk
High risk because it changes Safe signature-validation surfaces (
checkSignaturesABI, adds ERC-1271) and introduces a new module that can approve/transfer tokens via 1inch and interacts withCashModulewithdrawal locking/cancellation.Overview
Adds
OneInchSwapModuleto execute 1inch swaps from anEtherFiSafe, including an atomic “classic” swap path and a two-step Fusion intent flow (requestSwap/settleSwap) with pending-swap state, router approvals, output checks, andCashModule-driven force-cancel viacancelBridgeByCashModule.Upgrades
EtherFiSafefor 1inch Fusion by adding ERC-1271 (isValidSignature) and exposing the EIP-712getDomainSeparator; it also changescheckSignaturesparameters fromcalldatatomemoryacrossIEtherFiSafe/EtherFiSafeBase/MultiSigto support signature-blob decoding.Adds deployment and Gnosis-transaction generator scripts for deploying the new Safe implementation/module and configuring it as a default module, plus a comprehensive
OneInchSwapModuletest suite;.gitignoreis tweaked to ignore.DS_Storeand.certora_internal/.Reviewed by Cursor Bugbot for commit 0485b49. Bugbot is set up for automated code reviews on this repo. Configure here.