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.

Sign up for GitHub

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

Jump to bottom

Extend Checkpoints with new sizes and lookup mechanisms #3589

Merged
merged 53 commits into from
Aug 30, 2022
Merged

Extend Checkpoints with new sizes and lookup mechanisms #3589

merged 53 commits into from
Aug 30, 2022

Conversation

Amxx
Copy link
Collaborator

@Amxx Amxx commented Jul 28, 2022 •
edited
Loading

This PR includes quite a few things:

  • Procedural generation of the Checkpoint library, automating the support for various key/value lengths
    • Checkpoints.Checkpoint224: key is uint32 and value is uint224. Usecase: history checkpoints for voting
    • Checkpoints.Checkpoint160: key is uint96 and value is uint160. Usecase: Address sequence (for NFT batch minting)
  • For each Checkpoint type, functions availables are:
    • latest()
    • push(key, value)
    • lowerLookup(key): returns the c.value of the first checkpoint for which key <= c.key (or 0 if no such checkpoint exists)
    • upperLookup(key): returns the c.value of the last checkpoint for which key >= c.key (or 0 if no such checkpoint exists)
    • upperLookupRecent(key): same as upperLookup but optimised to look in recent checkpoints.
  • Checkpoint.History now uses the Checkpoints.Checkpoint224 structure, with block.number for keys.
    • getAtBlock now uses upperLookupRecent, optimizing for recent value lookup → delayed to further PR

Fixes #3585

PR Checklist

  • Tests
  • Documentation
  • Changelog entry

@Amxx
Copy link
Collaborator Author

Amxx commented Jul 29, 2022

Uses ethereum/solidity#9117 (comment) to skip checks

@Amxx Amxx force-pushed the feature/checkpoint branch from 66a2e82 to 28f6e6f Compare July 29, 2022 08:07
@Amxx Amxx requested a review from frangio July 29, 2022 17:04
@Amxx Amxx mentioned this pull request Jul 29, 2022
Merged
3 tasks
@Amxx Amxx mentioned this pull request Aug 14, 2022
Merged
4 tasks
frangio
frangio reviewed Aug 17, 2022
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
contracts/utils/Checkpoints.sol Show resolved Hide resolved
contracts/utils/Checkpoints.sol Outdated Show resolved Hide resolved
contracts/utils/Checkpoints.sol Outdated Show resolved Hide resolved
contracts/utils/Checkpoints.sol Show resolved Hide resolved
scripts/generate/templates/Checkpoints.js Outdated Show resolved Hide resolved
contracts/utils/Arrays.sol Outdated Show resolved Hide resolved
@frangio frangio changed the title Refactor the Checkpoint library, with new key/value balances and new lookup mechanisms Extend Checkpoints with new sizes and lookup mechanisms Aug 30, 2022
frangio
frangio reviewed Aug 30, 2022
View reviewed changes
Copy link
Contributor

@frangio frangio left a comment

Choose a reason for hiding this comment

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

Mostly minor comments. We can merge after this.

contracts/utils/Checkpoints.sol Outdated Show resolved Hide resolved
scripts/generate/templates/Checkpoints.js Outdated Show resolved Hide resolved
scripts/generate/templates/Checkpoints.js Outdated Show resolved Hide resolved
scripts/generate/templates/Checkpoints.js Outdated Show resolved Hide resolved
scripts/generate/templates/Checkpoints.js Outdated Show resolved Hide resolved
test/utils/Checkpoints.test.js Outdated Show resolved Hide resolved
test/utils/Checkpoints.test.js Outdated Show resolved Hide resolved
test/utils/Checkpoints.test.js Show resolved Hide resolved
test/utils/Checkpoints.test.js Show resolved Hide resolved
scripts/generate/templates/Checkpoints.js Show resolved Hide resolved
Amxx and others added 11 commits August 30, 2022 14:03
@Amxx
improve tests
2d03e54
@Amxx
Merge remote-tracking branch 'amxx/feature/checkpoint' into feature/c…
69ac69e 
…heckpoint
@Amxx @frangio
Update test/utils/Checkpoints.test.js
5661e71 
Co-authored-by: Francisco <[email protected]>
@Amxx @frangio
Apply suggestions from code review
6cd42ce 
Co-authored-by: Francisco <[email protected]>
@Amxx
fix generation
9c84034
@Amxx @frangio
Update scripts/generate/templates/Checkpoints.js
b878b23 
Co-authored-by: Francisco <[email protected]>
@Amxx @frangio
Update scripts/generate/templates/Checkpoints.js
c832e53 
Co-authored-by: Francisco <[email protected]>
@Amxx @frangio
Update scripts/generate/templates/Checkpoints.js
a450545 
Co-authored-by: Francisco <[email protected]>
@Amxx @frangio
Update scripts/generate/templates/Checkpoints.js
96122a3 
Co-authored-by: Francisco <[email protected]>
@Amxx
fix generation
c891f58
@Amxx
add History.getAtRecentBlock & tests
47ed78e
@Amxx Amxx requested a review from frangio August 30, 2022 13:27
@Amxx Amxx enabled auto-merge (squash) August 30, 2022 19:18
@Amxx Amxx disabled auto-merge August 30, 2022 19:32
@Amxx Amxx merged commit 71aaca2 into OpenZeppelin:master Aug 30, 2022
@Amxx Amxx deleted the feature/checkpoint branch August 30, 2022 19:32
ronhuafeng added a commit to ronhuafeng/openzeppelin-contracts that referenced this pull request Sep 9, 2022
@ronhuafeng
Extend Checkpoints with new sizes and lookup mechanisms (OpenZeppelin…
9f9afcf 
…#3589)
@tinchoabbate tinchoabbate mentioned this pull request Sep 22, 2022
Merged
JulissaDantes pushed a commit to JulissaDantes/openzeppelin-contracts that referenced this pull request Nov 3, 2022
@Amxx @JulissaDantes
Extend Checkpoints with new sizes and lookup mechanisms (OpenZeppelin…
1612906 
…#3589)
JulissaDantes pushed a commit to JulissaDantes/openzeppelin-contracts that referenced this pull request Nov 4, 2022
@Amxx @JulissaDantes
Extend Checkpoints with new sizes and lookup mechanisms (OpenZeppelin…
dc62c04 
…#3589)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frangio frangio frangio approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.8
Development

Successfully merging this pull request may close these issues.

Add a Checkpoints lookup that optimizes for recency
2 participants
@Amxx @frangio