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

[WIP] Add Uni V3, stCelo, rstCelo, Algebra #44

Closed
wants to merge 10 commits into from

Conversation

d-mooers
Copy link
Contributor

@d-mooers d-mooers commented Jun 15, 2023

Minima was recently taken offline due to changes in the regulatory landscape in the US. While we await clarity on the licenses required to run a router as a service, some entities on Celo find themselves needing a routing solution to replace Minima. To ensure that the Celo community still has access to a router than can handle most protocols on the network, Ill be adding some additional pairs here.

Minima is a bit different from Swappa, so these pairs will be largely written from scratch (save for the input-output math). Extra care should be taken when using these added pairs until they are fully vetted by community.

  • Uni V3
  • stCelo
  • rstCelo
  • Algebra (Ube V3)

@d-mooers
Copy link
Contributor Author

This will be a pretty big pr - is it preferred for me to break this up into multiple PRs instead?

@d-mooers d-mooers changed the title [WIP] Add Uni V3, stCelo, Algebra, BalancerV2 [WIP] Add Uni V3, stCelo, rstCelo, Algebra Jun 15, 2023
@zviadm
Copy link
Collaborator

zviadm commented Jun 21, 2023

This will be a pretty big pr - is it preferred for me to break this up into multiple PRs instead?

Hey, yes this is a massive diff and is very difficult to review. I think separating it into three PRs for different pairs would be a good start.

I think STCelo stuff should be very easy so i can review those quickly.
I am guessing Uni v3 one is the biggest one so would be best to have a way to review it separately.

@zviadm
Copy link
Collaborator

zviadm commented Jun 21, 2023

Thanks for the massive contribution, I am happy to get these stuff merged quickly, but still want to do at least a reasonable level of review here.

To help out with this, splitting this big diff into separate PRs for each Pair would be a good start. That way I can review and merge easier pairs quickly and spend a bit more time on more complex Uni v3 stuff.

@zviadm
Copy link
Collaborator

zviadm commented Jul 15, 2023

Fyi, I merged in stCelo and UniswapV3 stuff separately.
Just a heads up @d-mooers , there were bugs in both stCelo stuff and in UniswapV3 stuff too. UniswapV3 stuff is still not fully fixed, but I will take another pass at trying to fix it. Just a heads up in case you are using this code somewhere else too.

I don't know what the Algebra stuff is? Does it have any TVL? If it does, can consider merging that in too, otherwise can skip it.

@zviadm zviadm closed this Jul 15, 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.

2 participants