We don't want to be responsible for holding private keys -> the user is the one that should do this outside of tycho-execution
Done:
- Remove signature from EncodedSolution
- Introduce UserTransferType and pass that everywhere instead of is_permit2_active and token_in_already_in_router
- Remove signing from permit2. Added it to the encoding_utils.rs only
- Mark encode_full_calldata as deprecated
- Backwards compatibility: still accept a signer for the encode_full_calldata case
- Update all tests
Took 2 hours 10 minutes
Took 13 minutes
- Remove encode_full_calldata from the TychoEncoder trait
- Make the TychoExecutorEncoder use encode_solutions instead of encode_full_calldata
- Update tycho-encode.rs accordingly
Took 1 hour 3 minutes
Took 12 seconds
This way the user is responsible for encoding the Tycho Router method inputs that are used as guardrails in execution.
Interface changes:
- Create EncodedSolution
- StrategyEncoder
- don't need to know have permit2 or token_in_already_in_router as attributes anymore
- encode_strategy returns EncodedSolution now (no method encoding done here now)
- TychoEncoder
- add encode_solution() method. This is the recommended method for users
- needs to have permit2, token_in_already_in_router and router_address as attributes
- permit creation is made in the router now
Also:
- create encoding_utils.rs
- update all tests
Took 2 hours 42 minutes
Took 3 minutes
Took 13 minutes
This will block an attempt to transfer from the user when we expect the funds to already be in the router.
chores:
- add docs
- in EncodingContext, rename transfer to transfer_type
Took 58 minutes
we will never perform a manual transfer into these protocols, as they require the tokens to be in the router contract in order to perform a transferFrom.
Delete TokenTransfer.sol
Make slither happy
Bugfixes:
- Executors
- Ekubo:
- Fix the POOL_DATA_OFFSET value and remove sender from callback data
- Use SafeERC20
- Maverick and Univ2: Use safeTransfer and not safeTransferFrom
- Univ3: update expected data length
- Univ4: update the selectors (the signature changed)
- Router:
- For split swap we don't need to pass the tokenInReceiver, it should always be the router address
- For single and sequential: change order of the parameters (to be before the permit2 specific objects)
- Encoders:
- Update selector signatures
- For split swap pass the transfer_from (we might not need to if the token in is ETH)
Took 2 hours 51 minutes
The transfer from the user into the router is supposed to happen in the router (we only support this in the executors for callback constrained protocols). This is necessary because of some security concerns that were found in the audit. This way we reduce the space of attack.
- Refactored TransferOptimization not to handle TransferTypes anymore but just return bools.
- Split get_transfer_type into get_transfers and get_in_between_transfer. Updates tests
- Updated the strategies to use this
- Updated function signatures to pass transfer_from and funds_receiver
- Updated SwapEncoders to handle this
- SplitSwapStrategy just assumes all tokens are sent to and from the router at all times
Took 2 hours 46 minutes
- Also added integration test to test the optimizations, where we can see the in and out transfers being optimized if we enable verbose foundry testing
- Fixed typo in swap encoder builder initialization
This way we can automatically replace the calldata when something changes. We don't need to manually replace the string ourselves.
--- don't change below this line ---
ENG-4453 Took 3 hours 26 minutes
- Needed to add ekubo and uniswap v4 to callback-limited protocols.
- I had to bump the fork block in all of our integration tests: The way it was before meant that certain integration tests were using certain executor addresses, and others were using different ones, because of the redeployment. This was a pain to account for on the rust side. Instead, all tests now use an Ekubo-compatible fork block. Values needed to be updated because of price changes between blocks.
Only the first swap of a SwapGroup would actually get a transfer in, so we don't need to do this inside of the swaps loop
Rename in_token and out_token to token_in and token_out in SwapGroup
--- don't change below this line ---
ENG-4446 Took 22 minutes
In both encoding and contracts:
- Make sure that the tests names/encoder match what is happening (we had test_split_... that was only a single swap).
- Made modules inside the encodings tests
- In swap_encoders: there is one module per protocol
- In strategy_encoders: there is one module per strategy and an extra protocol_integration module. Inside the sequential module there is another module called chained_swaps
- In contracts, each strategy has its own file. Integration tests per strategy should also be here. Renamed TychoRouterIntegration to TychoRouterProtocolIntegration. Only protocol integration tests should be in this file
--- don't change below this line ---
ENG-4327 Took 1 hour 13 minutes
- In Tycho Router
- Fix integration tests
- If the trade is cyclical, we can't do the balance check for the correctness of the amountOut transfers
- In encoding,
- if it is a wrap trade, the transfer should be just a normal Transfer (not TransferFrom nor a Permit2Transfer)
- add test names to println! statements
--- don't change below this line ---
ENG-4315 Took 1 hour 0 minutes
Took 30 seconds
Took 21 seconds
Took 16 seconds
Took 17 seconds
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
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
- 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
