Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 19 commits into from
Jul 17, 2023

Conversation

ninabarbakadze
Copy link
Contributor

@ninabarbakadze ninabarbakadze commented Mar 15, 2023

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

@ninabarbakadze ninabarbakadze changed the title Feature/add pairmentov2 feat(mento-v2): Integrate mento-v2 Mar 16, 2023
@@ -0,0 +1 @@
{"address":"0xa7cBD42Ee1d9832ac6d15c04Be4E88340dd3951f"}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the same address as baklava, since the contract has not been deployed on mainnet

@ninabarbakadze ninabarbakadze marked this pull request as ready for review March 22, 2023 10:29
@ninabarbakadze ninabarbakadze force-pushed the feature/add-pairmentov2 branch from b1300de to cd68b46 Compare April 17, 2023 14:14
@ninabarbakadze ninabarbakadze force-pushed the feature/add-pairmentov2 branch from cd68b46 to 8ac5f76 Compare April 17, 2023 14:15
@zviadm
Copy link
Collaborator

zviadm commented Apr 17, 2023

@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
Copy link
Contributor Author

@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
Copy link
Collaborator

zviadm commented Apr 19, 2023

@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.

@zviadm zviadm mentioned this pull request Jul 17, 2023
@zviadm zviadm merged commit 328bd0a into terminal-fi:main Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get MentoV2 included in Swappa
2 participants