-
Notifications
You must be signed in to change notification settings - Fork 75
feat(spoke-pool): Reduce size of state variables #32
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/Optimism_SpokePool.sol
Outdated
| uint64 _depositQuoteTimeBuffer, | ||
| address timerAddress | ||
| address timerAddress, | ||
| uint32 _depositQuoteTimeBuffer |
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.
should we default this to what we use in prod (10 mins) then remove it from the constructor? this is what I did in the hub pool: as much as posible remove the instantiation of constructor variables if we know what they will be in production. thoughts?
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.
yea lets do it
| uint32 public deploymentTime; | ||
|
|
||
| // Any deposit quote times greater than or less than this value to the current contract time is blocked. Forces | ||
| // caller to use an up to date realized fee. | ||
| uint64 public depositQuoteTimeBuffer; | ||
| // caller to use an up to date realized fee. Defaults to 10 minutes. | ||
| uint32 public depositQuoteTimeBuffer = 600; | ||
|
|
||
| // Use count of deposits as unique deposit identifier. | ||
| uint64 public numberOfDeposits; | ||
| uint32 public numberOfDeposits; |
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.
nit: can we move these down below weth, since I assume weth and all of these will be read on each deposit, so we'll end up saving one word's worth a reads by grouping them.
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.
nice done
This PR is cherry-picked directly from contracts-v2/master. Originally authored by Nick & Matt. Signed-off-by: nicholaspai <npai.nyc@gmail.com> Signed-off-by: Matt Rice <matthewcrice32@gmail.com> Co-authored-by: nicholaspai <9457025+nicholaspai@users.noreply.github.com> Co-authored-by: Matt Rice <matthewcrice32@gmail.com>
Percentages and timestamp-based variables can be smaller size to save gas costs for storing data