Skip to content
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

New locker lib: unpacks length and nonzeroDeltaCount #388

Merged
merged 4 commits into from
Nov 9, 2023

Conversation

snreynolds
Copy link
Member

Related Issue

Which issue does this pull request resolve?

Description of changes

@snreynolds snreynolds marked this pull request as ready for review November 7, 2023 15:25
uint256 slot = NONZERO_DELTA_COUNT;
assembly {
let count := tload(slot)
count := sub(count, 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think its worth catching the underflow here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No because we only call this if we've incremented prior so I think its ok and its an internal function

Copy link
Contributor

@hensha256 hensha256 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of things but i love this change with them separated!

contracts/libraries/Lockers.sol Outdated Show resolved Hide resolved

/// @notice This is a temporary library that allows us to use transient storage (tstore/tload)
/// for the lockers array and nonzero delta count.
library Lockers {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be great to see some tests for this library!

@snreynolds snreynolds merged commit f70df7d into transient-storage Nov 9, 2023
2 of 3 checks passed
@snreynolds snreynolds deleted the test-new-locker-lib branch November 9, 2023 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants