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.

Sign up for GitHub

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

Jump to bottom

Clamm #39

Merged
merged 18 commits into from
Jan 16, 2024
Merged

Clamm #39

merged 18 commits into from
Jan 16, 2024

Conversation

ethzoomer
Copy link
Contributor

Adds CLAMM pools to LpSugar. Need to decide how we want to differentiate between v2 and v3 pools before the byData call

@ethzoomer ethzoomer requested a review from stas November 28, 2023 02:26
stas
stas approved these changes Nov 30, 2023
View reviewed changes
Copy link
Collaborator

@stas stas left a comment

Choose a reason for hiding this comment

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

This looks great.

Left some suggestions, otherwise I think we're good to move forward.

@ethzoomer ethzoomer requested a review from stas December 4, 2023 23:20
stas
stas approved these changes Dec 5, 2023
View reviewed changes
Copy link
Collaborator

@stas stas left a comment

Choose a reason for hiding this comment

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

This looks great, left some minor suggestions on optimizations. We can rebase and merge it once it's ready! 👏

@ethzoomer ethzoomer requested a review from stas December 5, 2023 16:52
stas
stas approved these changes Dec 13, 2023
View reviewed changes
Copy link
Collaborator

@stas stas left a comment

Choose a reason for hiding this comment

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

Looks good. I think we can roll it out and prepare for the dapp release using the new data layout 🙏

@stas
Copy link
Collaborator

stas commented Jan 10, 2024

@ethzoomer any reason why we don't add the additional addresses[] to the tokens() function?

@ethzoomer
Copy link
Contributor Author

@ethzoomer any reason why we don't add the additional addresses[] to the tokens() function?

Wdym exactly?

stas
stas reviewed Jan 12, 2024
View reviewed changes
@stas
Copy link
Collaborator

stas commented Jan 15, 2024

@ethzoomer any reason why we don't add the additional addresses[] to the tokens() function?

It's related to the conversation about tokensByAddress() in 908f286

@stas
Copy link
Collaborator

stas commented Jan 15, 2024

@ethzoomer do you think you can deploy this, I'm happy to give it a quick test and merge it.

stas
stas reviewed Jan 16, 2024
View reviewed changes
readme.md Outdated
* `alm_vault` - ALM vault address on v3 if it exists, empty address on v2
* `alm_reserve0` - ALM vault token0 reserves on v3, 0 on v2
* `alm_reserve1` - ALM vault token1 reserves on v3, 0 on v2
* `positions` - a list of account pool position data, it is a struct of type `Position`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just realized we didn't add the Position docs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ty added

@ethzoomer
Copy link
Contributor Author

Put _addresses into tokens(). Added Position info to readme and deployed to mainnet at 0xE180829A166d1e0bec705C1eB25758F645C9E317

@stas
Copy link
Collaborator

stas commented Jan 16, 2024

@ethzoomer I think we can merge this now ⚡

@ethzoomer ethzoomer merged commit f1d3b4d into v2 Jan 16, 2024
0 of 2 checks passed
@stas stas deleted the clamm branch January 16, 2024 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stas stas stas approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ethzoomer @stas