terminal-fi / swappa

Framework for executing cross-DEX token swaps
MIT License
20 stars 13 forks source link

feat(mento-v2): Integrate mento-v2 #42

Closed ninabarbakadze closed 1 year ago

ninabarbakadze commented 1 year ago

Description

This PR integrates mento-v2 in swappa. This entailed writing a PairMentoV2 smart contract and a ts component following the general Pair interface.

Tested

PairMentoV2.sol and SwappaRouterV1.sol were deployed and swaps were tested on Baklava. Integrated celo-foundry packaged and tested PairMentoV2.sol contract.

In order to easily tests swaps we replaced mainnet.SwappaRouterV1.json address with baklava address - 0xf821B3c10c922C88D12FF5f91Cf2275e0768D7Dc

Issues

zviadm commented 1 year ago

@ninocomputer I just skimmed through some of mento-v2 stuff, it is a bit too complicated so i probably would need to take a bit of time to understand all the complexities.

However, one thing i noticed is that there is some mention of trading limits. Should those also be implemented in addition to buckets to determine OutputAmounts? I havent looked into it in details, but from cursory glance it seems like if trading limits are reached trades can't happen despite current bucket sizes?

ninabarbakadze commented 1 year ago

@ninocomputer I just skimmed through some of mento-v2 stuff, it is a bit too complicated so i probably would need to take a bit of time to understand all the complexities.

However, one thing i noticed is that there is some mention of trading limits. Should those also be implemented in addition to buckets to determine OutputAmounts? I havent looked into it in details, but from cursory glance it seems like if trading limits are reached trades can't happen despite current bucket sizes?

Currently, tradingLimits go into action only during swaps in the Broker not whilst calculating the amountOut. I don't think we'd need to implement tradingLimits logic in the component but do let me know if you think otherwise 🙏🏻

zviadm commented 1 year ago

@ninocomputer I just skimmed through some of mento-v2 stuff, it is a bit too complicated so i probably would need to take a bit of time to understand all the complexities. However, one thing i noticed is that there is some mention of trading limits. Should those also be implemented in addition to buckets to determine OutputAmounts? I havent looked into it in details, but from cursory glance it seems like if trading limits are reached trades can't happen despite current bucket sizes?

Currently, tradingLimits go into action only during swaps in the Broker not whilst calculating the amountOut. I don't think we'd need to implement tradingLimits logic in the component but do let me know if you think otherwise 🙏🏻

If the trade can't actually happen due to tradingLimits, amountOut calculated in Swappa must be 0. Otherwise, once tradingLimit is reached, Swappa might still choose MentoV2 in its trading route, and when you actually try to perform the swap, you are going to get TX failures.