-
Notifications
You must be signed in to change notification settings - Fork 75
improve: Change pool rebalance leaf count type to uint8 #64
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
Conversation
contracts/HubPool.sol
Outdated
| // Number of pool rebalance leaves to execute in the poolRebalanceRoot. After this number | ||
| // of leaves are executed, a new root bundle can be proposed | ||
| uint64 unclaimedPoolRebalanceLeafCount; | ||
| uint8 unclaimedPoolRebalanceLeafCount; |
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.
reordered for style from small to big
chrismaree
left a comment
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.
o damn. good catch.
mrice32
left a comment
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.
Looks good!
contracts/HubPool.sol
Outdated
| uint64 unclaimedPoolRebalanceLeafCount; | ||
| uint8 unclaimedPoolRebalanceLeafCount; | ||
| // When root bundle challenge period passes and this root bundle becomes executable. | ||
| uint64 requestExpirationTimestamp; |
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.
If we change this to a uint32 (which is safe until the year 2106), we should be able to group these variables with proposer and proposerBondRepaid, saving us a storage slot.
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.
sure
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.
Changed to uint32 and copied and pasted these two vars below proposerBondRepaid. Is that what you had in mind?
mrice32
left a comment
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.
LGTM!
Max leaf count can only be uint8 to be consistent with size of
RootBundle.claimedBitMapand is the same size passed intoproposeRootBundle, so this is likely just fixing a mistake.