We don't need to use all the functionalities of the Curve Router, only the swap type 1 (exchange). By bypassing the router we can save gas on 2 token transfers and with smaller calldata
A nice side effect is that the executor is much more readable and understandable now
--- don't change below this line ---
ENG-4305 Took 2 hours 25 minutes
Took 12 seconds
- Needed to take pool code init hash as input param for executors
- Added tests for ethereum. Will test base on-chain.
- Important note: Pancakeswap uses their deployer instead of their factory (this is a different address) for target verification.
- This factory is not the same for Ethereum and Base, so Base txs were failing when verifying pool addresses.
- I've double checked that we don't have this problem for Balancer V2 - the vault address in the same on Base and on Ethereum Mainnet.
- Note: I think we can get the fee straight from the pool... why did we always encode this and send it from the solver? Is this bound to change sometimes?
Changes:
- If the tokenIn is ETH, skip permit2 approval
- Make executors payable: When using delegatecall the executor inherits the execution context of whoever calls it. Our main swap function can accept ETH, it needs to be payable so by consequence the executors also need to be.
- Set uniswap v2 executor in test router
- Add tests for all possible cases of swap
- Add tests for all cases of splitSwap
- Add test functions to handle permit2 and encode swaps
--- don't change below this line ---
ENG-4041 Took 3 hours 50 minutes
Took 49 seconds
Took 14 seconds
I was inspired to do this because, when disabling the slither check for the delegatecall when calling the swap executor, I realized it's not clear from the same contract that we have already checked for contract code existence when setting the executor. This made me feel uneasy, as this contract can then not stand alone and must rely on the higher level contract to safely check for code existence, otherwise the delegatecall is unsafe. Keeping this logic in a separate contract seems error-prone to me, as we may remove the check for code existence without immediately realizing the implications of doing so.
For this reason I have organized it as follows:
- Logic/tests relating to proper roles/access control in the main TychoRouter.
- Lower-level logic/tests that checks contract validity before setting the executor in the SwapExecutionDispatcher
- Moved the deployment method into a test template for organization
- Created skeletons of dispatcher contracts
- Added all possible test cases for thoroughness
