-
Notifications
You must be signed in to change notification settings - Fork 197
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
bitwise optimizations #120
Conversation
cedb045
to
d865412
Compare
} | ||
|
||
/// @notice Checks whether a nonce is taken and sets the bit at the bit position in the bitmap at the word position | ||
/// @param from The address to use the nonce at | ||
/// @param nonce The nonce to spend | ||
function _useUnorderedNonce(address from, uint256 nonce) internal { | ||
(uint256 wordPos, uint256 bitPos) = bitmapPositions(nonce); | ||
uint256 bitmap = nonceBitmap[from][wordPos]; | ||
uint256 bit = 1 << bitPos; | ||
uint256 flipped = nonceBitmap[from][wordPos] ^= bit; |
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.
woah this is so smart. love it
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.
Smart, yes, but this approach runs counter to the Checks-Effects-Interactions pattern by switching the order of the checks and the effects.
It's also not super intuitive what the order of operations is - I opened an issue about this: #217
test/NonceBitmap.t.sol
Outdated
@@ -13,6 +13,12 @@ contract NonceBitmapTest is Test { | |||
permit2 = new MockPermit2(); | |||
} | |||
|
|||
function testNonceLLL() public { |
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.
what is LLL?
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.
oops lemme remove
No description provided.