Reasons for using regular approvals instead of permit2:
- More simplicity, simpler setup
- Unfortunately we won't be able to "expire" approvals, which is a risk we may be okay with taking - since one advantage of this is that we will be able save on gas by not approving the reactor every single time (is this our intention?)
TODO:
- Do we really need input tokens to pass through our router?
- Handle native ETH (double check how this is transferred in the reactor)
- locked-ether: We do have a withdraw method. Not sure why it doesn't register with slither. Already tried renaming.
- low-level-calls: This low level call is integral to the design of our filler contract.=
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
- 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 problem was that the pool manager was expecting an ABI encoded result to be returned and we were not returning that (we were returning just a result)
Special thanks to Max for figuring this out
Took 31 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
