Also added a getForkBlock() on TychoRouterTestSetup.
Moved the router balancer test inside the Balancer file and created a TychoRouterForBalancerV3Test to wrap it
Took 1 hour 10 minutes
Took 21 seconds
Refactor BalancerV3Executor to have an inner _swapCallback method with the real swapping logic. Then we have two external methods:
- handleCallback: called by the router. Here we need to remove the first 68 bytes (4 selector + 32 dataOffset + 32 dataLength)
- swapCallback: called by the Vault directly if we are swapping against the executor directly (no router involved)
Took 1 minute
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
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
- In RestrictTransferFrom:
- Compare tokenIn with tokenIn from storage
- Correct docstrings
- Recompute storage slots with new names
- Rename transferFromNeeded to isTransferFromAllowed
- Don't track amount spent but subtract from amount allowed
- In TychoRouter: Rename transferFromNeeded to isTransferFromAllowed
Took 32 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
For organization (and thus safety) purposes.
Rename to RestrictTransferFrom.sol so that we can perform multiple transfer froms (upto an allowance) in the case of split swaps (where the split is the first swap).
TODO: Fix tests.
- Don't use payable(receiver).transfer(amount) and use OpenZeppelin's Address.sendValue instead
- In Univ4Executor send funds to the poolManager and not msg.sender
- In OneTransferFromOnly:
- rename method name
- don't pass the sender but hardcode it to caller() (msg.sender)
- Move marking the transfer as done up (before we actually transfer) to prevent reentrancy attacks
Took 18 minutes
This needs to be calculated before we perform a transferFrom in the router! This worked before since we were doing the transferFroms always from inside the executors.
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
