Add transfer in Executor and pass receiver address in encoding
This is done by using the TAKE action instead of TAKE_ALL
--- don't change below this line ---
ENG-4315 Took 1 hour 36 minutes
Took 2 minutes
- Also do some renamings and comment improvements:
transfer method -> type
- Remove integration tests since we no longer support direct calls to USV3 executor, even for testing.
- This needed to be in BalancerV2 so that the executor can also take care of transfers from the user into the tycho router, to avoid having transfer actions mixed between the router and executors
Add transfer in Executor and pass receiver address in encoding
Remove integration test with StETH pool because we don't support it at simulation level and there is something weird with the amounts (that it is not worth it to investigate now)
--- don't change below this line ---
ENG-4315 Took 34 minutes
Took 2 minutes
- This needed to be in Curve so that the executor can also take care of transfers from the user into the tycho router, to avoid having transfer actions mixed between the router and executors
- The out transfer is now a responsibility of the Executors -> remove this from router methods
- Also adding a check that the receiver got the full amount out
- In encoding, if it is the last swap, pass the receiver as the trade receiver and not the router address (fix encoding tests)
- Fixed some solidity tests (after rebasing with a PR that is still open, I will fix them all)
TODO: Adapt curve and uniswap v4 to support this
--- don't change below this line ---
ENG-4315 Took 3 hours 7 minutes
Took 20 minutes
Took 59 seconds
Took 7 minutes
- Renamed ExecutorTransferMethods to TokenTransfer to avoid leaking information about how the transfer happens into the executor. The executor shouldn't care if there are multiple methods or one single method that takes care of everything.
- Also renamed TransferMethod to TransferType to match the rust encoding
- This arbitrary sender should be same, since the sender is always set to be msg.sender from within the contract (no calldata can be sent specifying otherwise). This is just used to circumvent msg.sender being the pool/protocol during callbacks.
- For protocols like Balancer and Curve, which expect funds to be in the router at the time of swap, we must support also transferring funds from the user into the router. Doing this in the router would mean we are dealing with transfers in two different places: in the router main methods and in the executors. To avoid this, we are now performing transfers just in the executors, and two transfer types have been added to support transfers into the router.
TODO:
- Add this for Balancer and Curve (only added for USV4 atm).
- TODO consider renaming TRANSFER_FROM and TRANSFER_PERMIT2 to include "pool" in the name
- Renamed ExecutorTransferMethods to TokenTransfer to avoid leaking information about how the transfer happens into the executor. The executor shouldn't care if there are multiple methods or one single method that takes care of everything.
- Also renamed TransferMethod to TransferType to match the rust encoding
- Properly decode, update tests with proper decoding
- Added test case for each transfer method
- Also fully tested permit2 transferFrom and it works perfectly.
TODO:
- Fix integration tests once encoding is implemented.
- For now, hardcode them to TRANSFER on the rust encoder side. This will be fixed in an upcoming PR.
- Remove the split swap simple route integration test - seemed overkill since simple routes are tested in many other integration tests, the original rust test named doesn't exist anymore, and simple routes should anyway be passing through the single endpoint.
- Properly decode, update tests with proper decoding
- Added test case for each transfer method
- Also fully tested permit2 transferFrom and it works perfectly.
NOTE:
UniswapV3 doesn't support NONE as a transfer method.
TODO:
- Fix integration tests once encoding is implemented.
- Properly decode, update tests with proper decoding
- Added test case for each transfer method
- Also fully tested permit2 transferFrom and it works perfectly.
TODO:
- Fix integration tests once encoding is implemented.
- The tycho router address default is set at the EncoderBuilder level (not inside the strategies)
- Rename TychoCoreChain to TychoCommonChain
- Only take TychoCommonChain as an argument at the outermost level: EncoderBuilder. Everywhere else we use the execution Chain right away
--- don't change below this line ---
ENG-4332 Took 1 hour 10 minutes
We have a trait TychoEncoder and then two implementations: TychoRouterEncoder and TychoExecutorEncoder.
This way we go a level above with the decision if it is a direct execution or if it should use the tycho router.
- Created two builders: one for each tycho encoder
- Delete ExecutorStrategyEncoder and move code straight into the TychoExecutorEncoder
- Add validate_solution to trait TychoEncoder
- Move group_swaps.rs a level up
- Update tests and usage cases
Doing this we get rid of all that weird stuff we were doing before
--- don't change below this line ---
ENG-4306 Took 2 hours 6 minutes
Took 12 seconds
