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

Support more efficient merkle proofs through calldata #3200

Merged
merged 9 commits into from
Jun 1, 2022
Merged

Support more efficient merkle proofs through calldata #3200

merged 9 commits into from
Jun 1, 2022

Conversation

Cr0wn-Gh0ul
Copy link
Contributor

Fixes #3198

PR Checklist

  • Tests
  • Documentation
  • Changelog entry

@Cr0wn-Gh0ul Cr0wn-Gh0ul mentioned this pull request Feb 18, 2022
Closed
@frangio
Copy link
Contributor

frangio commented Feb 18, 2022

This PR as proposed is a breaking change. I was proposing that we should have a new set of functions where the arguments are calldata. Though I see now that this would result in a lot of code duplication. It's not a lot of code so it may be worth doing anyway.

@frangio frangio added this to the 4.7 milestone Apr 8, 2022
@frangio frangio changed the title Change MerkleProof.sol to use calldata instead of memory for proof arg Support more efficient merkle proofs through calldata May 26, 2022
@frangio
Copy link
Contributor

frangio commented May 26, 2022

This is close to done but we want to rename verifyCall to verifyCalldata, processProofCall to processProofCalldata.

@frangio frangio enabled auto-merge (squash) June 1, 2022 20:35
@frangio frangio merged commit 4fc9fd3 into OpenZeppelin:master Jun 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Amxx Amxx Amxx left review comments

@frangio frangio frangio approved these changes

Assignees

@Amxx Amxx

Labels
None yet
Projects
None yet
Milestone
4.7
Development

Successfully merging this pull request may close these issues.

Merkle Proof gas cost may be reduced in some cases.
3 participants
@Cr0wn-Gh0ul @frangio @Amxx