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] feat: add poolId -> poolKey mapping #91

Merged
merged 4 commits into from
Jun 28, 2024

Conversation

ChefMist
Copy link
Collaborator

@ChefMist ChefMist commented Jun 27, 2024

This PR adds poolIdToPoolKey mapping.

Context

In order to retrieve the mapping currently, we need to rely on off-chain indexer. However if off-chain indexer has a problem, we cannot retrieve the mapping from poolid -> poolKey.

This PR adds the mapping on-chain. The result is around ~90k more gas for CLPool to store in a pool.initialize.
58,707 -> 149,801

Assume ETH at $5000 and gas at 10 gwei. it will cost 0.0009 more eth = $4.5

@chefburger
Copy link
Collaborator

lgtm, wait and see if we really want to merge it

probably do the same thing for bin pool

@ChefMist
Copy link
Collaborator Author

ChefMist commented Jun 27, 2024

lgtm, wait and see if we really want to merge it

probably do the same thing for bin pool

agree, if we are okay with this, I'll add this on bin pool as well within this PR

@@ -0,0 +1 @@
162167
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

originally was 71,099 so around 90k increase as well

@ChefMist ChefMist changed the title [Discussion] feat: add poolId -> poolKey mapping [Feat] feat: add poolId -> poolKey mapping Jun 27, 2024
@ChefMist
Copy link
Collaborator Author

created #98 to track if this is needed after further developments

@ChefMist ChefMist merged commit 62d1e90 into main Jun 28, 2024
2 checks passed
@ChefMist ChefMist deleted the feat/add-poolId-to-poolKey-map branch June 28, 2024 03:51
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