Skip to content

Procedurally generate EnumerableSet and EnumerableMap#3429

Merged
Amxx merged 6 commits into
OpenZeppelin:masterfrom
Amxx:feature/procedural/enumerable
Aug 19, 2022
Merged

Procedurally generate EnumerableSet and EnumerableMap#3429
Amxx merged 6 commits into
OpenZeppelin:masterfrom
Amxx:feature/procedural/enumerable

Conversation

@Amxx
Copy link
Copy Markdown
Collaborator

@Amxx Amxx commented May 23, 2022

PR Checklist

  • Tests
  • Documentation
  • Changelog entry

@Amxx Amxx marked this pull request as draft May 23, 2022 10:14
@Amxx Amxx added this to the 4.8 milestone Jun 7, 2022
@frangio
Copy link
Copy Markdown
Contributor

frangio commented Jul 21, 2022

Now that we have gas reporting, with this PR it should be easy to compare the cost of our current approach (cast everything to a Bytes32Set) versus duplicating the code for every type.

@Amxx
Copy link
Copy Markdown
Collaborator Author

Amxx commented Jul 25, 2022

We discussed that, and casting, while not being a breaking change on the solidity side would not be supported by the upgrades plugin.

I would do this PR in 4.X, and then do the change for casting (depending on the gas outcome) in 5.0

@Amxx Amxx marked this pull request as ready for review July 25, 2022 19:54
Copy link
Copy Markdown
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.

LGTM

@Amxx Amxx merged commit 17bc2da into OpenZeppelin:master Aug 19, 2022
@Amxx Amxx deleted the feature/procedural/enumerable branch August 19, 2022 12:12
@nventuro
Copy link
Copy Markdown
Contributor

I love everything about this

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