feat: add LI.FI SWAP validation, APPROVAL spender whitelist, and comprehensive tests for RocketPool validator#14
Open
ajag408 wants to merge 1 commit intoeng-1293-implement-rocketpool-shield-validatorfrom
Conversation
|
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: Organization 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)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stacked on #13. Adds the SWAP transaction type to the RocketPool Shield validator and tightens the APPROVAL flow with a spender whitelist. This completes the exit-path validation for
ethereum-eth-reth-staking— the user's rETH unstake now has full coverage: APPROVAL → SWAP → funds received.What changed
APPROVAL spender whitelist
PR #13 intentionally left the
approve()spender unvalidated because it comes dynamically from the LI.FI/v1/quoteAPI. On-chain investigation (Etherscan ERC20 transfer data for rETH) confirmed there are exactly two LI.FI contracts used as approval targets on Ethereum mainnet:0x1231DEB6f5749EF6cE6943a275A1D3E7486F4EaE0x89c6340B1a1f4b25D36cd8B063D49045caF3f818Both use the Diamond Proxy pattern (EIP-2535) — addresses are stable across upgrades. Approvals to any other spender are now blocked.
SWAP transaction validation (exit step 1)
Validates the LI.FI swap that converts rETH → ETH after the approval. Supports two call paths:
Direct Diamond calls —
tx.tois the Diamond,tx.datais the swap calldata directly.Permit2 Proxy wrapped calls —
tx.tois the Permit2 Proxy,tx.dataencodes one of three forwarding functions (callDiamondWithPermit2,callDiamondWithPermit2Witness,callDiamondWithEIP2612Signature) that embed the Diamond swap calldata as an innerbytesparameter. The validator extracts this inner calldata before validation.Validation checks (in order)
tx.from == userAddresschainId == 1tx.toinLIFI_CONTRACTStx.value == 0tx.datapresent and >= 10 chars_receiver == userAddressCheck 8 is the critical security property. Without it, an attacker intercepting the LI.FI API response could modify
_receiverto redirect the user's swap output through the legitimate Diamond contract. The_receiverparameter sits atargs[3]across all 7 swap functions — verified by ABI analysis and confirmed stable across multiple versions of the LI.FI contract types.Whitelisted Diamond swap functions
swapTokensGenericswapTokensSingleV3ERC20ToERC20swapTokensSingleV3ERC20ToNativeswapTokensSingleV3NativeToERC20swapTokensMultipleV3ERC20ToERC20swapTokensMultipleV3ERC20ToNativeswapTokensMultipleV3NativeToERC20Any calldata with an unrecognized selector is blocked (fail-closed). These 7 signatures have been stable in the LI.FI Diamond ABI since 2023.
Files changed
rocketpool.validator.tsLIFI_CONTRACTS,LIFI_SWAP_ABI,PERMIT2_PROXY_ABI, spender whitelist invalidateApproval,validateSwap,extractDiamondCalldata,validateLifiSwapReceiverrocketpool.validator.test.tsTest coverage (51 tests, all passing)
callDiamondWithPermit2,callDiamondWithPermit2Witness,callDiamondWithEIP2612Signature), unknown contract, no calldata, unknown selector, wrong receiver, ETH value, wrong user, wrong network, Permit2 wrong inner receiver, Permit2 garbage inner calldata, Permit2 unparseable outer calldataDesign notes