Optimize TickBitmap.flipTick using assembly
#653
Conversation
| vm.expectRevert(abi.encodeWithSelector(TickBitmap.TickMisaligned.selector, tick, tickSpacing)); | ||
| bitmap.flipTick(tick, tickSpacing); | ||
| } else { | ||
| bool initialized = isInitialized(tick, tickSpacing); |
There was a problem hiding this comment.
i think initialized is always going to be false because the bitmap is empty at the beginning. So this test is only testing false->true flipping.
Should be enough just to add another 2 lines where you call flip again and check it
test/libraries/TickBitmap.t.sol
Outdated
| function isInitialized(int24 tick, int24 tickSpacing) internal view returns (bool) { | ||
| unchecked { | ||
| int24 compressed = tick / tickSpacing; | ||
| if (tick < 0 && tick % tickSpacing != 0) compressed--; // round towards negative infinity |
There was a problem hiding this comment.
why do you have to subtract 1? why cant it just be the solidity that was previously in TickBitmap?
(int16 wordPos, uint8 bitPos) = position(tick / tickSpacing);
There was a problem hiding this comment.
That's the first two lines of nextInitializedTickWithinOneWord, which I factored out to a compress function in #654.
There was a problem hiding this comment.
but for flipTick theres a requirement that (tick % tickSpacing != 0) otherwise it reverts, so this line feels out of place here? Not sure what I'm missing
There was a problem hiding this comment.
Do you mean isInitialized should return false instead of rounding if tick % tickSpacing != 0?
A new fuzz test function `test_fuzz_flipTick` has been added to the TickBitmap library for improved testing accuracy. Additionally, the `isInitialized` function has been re-implemented to enhance its precision and correctness.
shuhuiluo
force-pushed
the
tickbitmap-fliptick
branch
from
76d1dd9
to
ab3c411
Compare
A refactoring was done in the TickBitmap test file which includes changing import paths and renaming a test function. A new fuzz test case on testing the next initialized tick within one word, but on an empty bitmap, was also added. This contributes to verifying the accuracy of tick operations under more extensive scenarios.
Related Issue
Which issue does this pull request resolve?
Description of changes
This pull request is a part of #560 in an effort to ease the review process. It focuses on the
flipTickfunction in theTickBitmaplibrary.