-
Notifications
You must be signed in to change notification settings - Fork 609
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(CL): Tick Bitmap #3065
feat(CL): Tick Bitmap #3065
Conversation
Marking this R4R so we can start testing finding ticks for dynamic swaps...still need to implement tests. Could be that my approach is off here using 64-bit words or maybe even the translation of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a rudimentary test of this locally and it appears to work as intended, well done!
// Ref: https://uniswapv3book.com/docs/introduction/uniswap-v3/#ticks | ||
// Ref: https://uniswapv3book.com/docs/milestone_2/tick-bitmap-index/#bitmap | ||
type TickBitmap struct { | ||
bitmap map[int16]uint64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can we serialize and store this structure in the state?
Does it make sense to use a "prefix" map that we can store on disk?
e.g. we make a prefix of the format bm< tick index >< key separator>< data >
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are a few ways:
- Collapse the
TickBitmap
into a sorted array of arrays, e.g.[[key1, value1], [key2, value2], ...]
. You then encode this array and store it under some prefix,PrefixByte | Encode(array)
. The words are 64-bits and the possible range of ticks is [−887272, 887272]. So that's roughly 1,774,545 possible ticks which can be presented via the 64-bit words. I didn't do the napkin math, but the map should be pretty small, so this isn't bad at all. - Alternatively, we can take your approach too. I just don't see a need to have individual entries for each k/v pair in state.
But then this raises the question how and when do we load it and then save it? I think we could either lazy-load it or load it at BeginBlock and then commit it at each EndBlock?
@ValarDragon wdyt?
Closes: #3059