- 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
- 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
- The router address can be set when creating the SplitSwapStrategy, which now takes an Option<Bytes> as input during initialization (another breaking interface change)
- This change allows us to change our router address in the future with minimal effect on the users. Users should only pass the router address in the split swap initialization if they intend to use their own router for execution.
- This change also means that the router address does not need to be passed with the solution, even when using the executor strategy (which was pointless).
- I thought of having a router_address() method to set this in the encoder builder - but it seemed too messy, since we don't need the router address for execution for example. We would then potentially unnecessarily load and set the default router address when not needed. It is also not even used at the highest level EVMTychoEncoder, so it makes more sense for it to be directly associated with the swap strategy instead.
- Users will now not be able to encode for different routers without re-initializing the strategy. We assumed this use case to be very rare and not worth supporting.
Also:
- Remove amount_out_min. Our reasoning is that we perform high-level final amount checks anyway (and expect users to do the same). Lower-level USV4 min amount checks are redundant.
To achieve this, a small refactor was necessary with the introduction of a SwapGroup object, and a separate method to create a vec of these objects. This then separates all logic related to combining protocol data nicely and cleanly into its own method with its own tests.
TODO:
- Deal with split swaps and add test for split swap scenario
- Clean up all existing PR comments
Return selector in the StrategyEncoder. This is needed for the ExecutorStrategyEncoder
--- don't change below this line ---
ENG-4088 Took 12 minutes
Took 13 seconds
- This way this chain object contains everything we need, we don't need to worry about doing any transformation or calling any supplementary functions inside any of the encoders
- Needed to move our new Chain object to a higher level since this is used in the higher-level encoder traits. This required some weird default values in the constants in order to avoid using alloy's hex literal. I could have instead opted to make Bytes parse a string I think, though this would mean possibly returning an error at the constants level, which is not nice either.
Question:
- Do we want the user to be in charge of passing the native and wrapped token every single time? This may be a bit annoying for the user. For now, I have defaulted to those in constants.rs, this would take 5 mins to remove though if you don't like it, and it would get rid of this complicated bytes initialization.
- Tried not to leave a single public function undocumented, though did not double-document public function with obvious names
- Remove all mentions of the strategy selector in favour of the strategy registry (to avoid confusion)
The strategy works as follows:
- Manage approvals needed
- Compute min amount (if check amount is any):
- if slippage is defined, apply slippage on the expected amount and take the min value between that and the check amount
- if not, it's just the check amount
- Iterate through the swaps
- call the corresponding swap encoder to encode the swap
- add swap header (tokens indexes and split)
- ple encode the swaps
- Add extra inputs (amounts, token addresses, min amount, (un)wrap, number of tokens and receiver)
Misc:
- Move executor address and selector encoding inside the SwapEncoder
- Add default executor_selector to SwapEncoder
- Pass router address inside the SplitSwapStrategyEncoder
- Move Permit2 inside the SplitSwapStrategyEncoder. It is a responsibility and a specificity of the strategy to need permit2 approvals
--- don't change below this line ---
ENG-4081 Took 1 hour 21 minutes