Implementation of an address Enumerable Set #2061
Conversation
|
I'm thinking that we could rename this contract to either Set.sol or AddressSet.sol. Future versions of the same contract might be UintSet, IntSet and MultiSet (the latter allowing to store more than one copy of each value). |
|
I would like to see the following names: the file @nventuro will be taking care of this PR since I will be off for a week. Please be patient in case he may have a lot on his plate right now. Thanks for contributing! |
|
Renamed to AddressSet. I assume that we would use overloading to use the same methods but with different structs. An UintSet probably would be useful early on. @nventuro, please provide any feedback, but don't feel rushed. At your pace. |
There was a problem hiding this comment.
This is looking great @albertocuestacanada! I'm looking forward to this being included in the upcoming release 🎉
I left a couple minor comments on the code and tests, once those are sorted out we can take a look at the documentation and then we'll be good to go! Thanks a lot for your work!
|
@albertocuestacanada thank you so much for your work on this! If you don't mind, I'll add some commits myself with minor style adjustments, adding the contract to our documentation, creating the changelog entry, etc: I don't want to bother you with those tasks. I'll create separate commits nonetheless so you can look at the process and do it yourself in the future if you so desire :) |
|
@albertocuestacanada it looks like I don't have permission to push to your fork to update this PR. Could you temporarily me add there as a collaborator so I can push these commits? Thanks! |
|
@nventuro, I just added you as a collaborator. Good job on finding that bug and thanks for the fix 👍 I'll follow your commits with interest. I'm learning a good deal about best practices through this PR. I like that. |
|
I'm glad this is being useful for you too! I think we're pretty much set here. Take a look at the changes and let me know if anything strikes you as odd - if not, we're ready to merge! |
|
You can look at the autogenerated documentation here: https://5e2b115a1b84650008d7b59a--openzeppelin-contracts-docs.netlify.com/contracts/2.x/api/utils#EnumerableSet Our generator seems to not be able to properly handle array return types, I'll open an issue about that. |
|
@nventuro, it is much more readable, especially so the tests 👍 Nothing to object, let's merge. |
|
And we're off! Thank you so much for your help @albertocuestacanada, I hope you can contribute to Contracts again soon 🙌! |
|
Hi @nventuro, I realized that the code used in EnumerableSetMock was unnecessarily verbose, and given that these files are sometimes used as a guide for how to use the contracts, I made a small cosmetic fix. Please have a look and let me know if you want me to do anything like creating an issue: |
|
I realize I'm a little late to the discussion but I have a few libraries I'd be happy to contribute if it helps. They use type-wise variants to cover different key types, similar to the RBAC xxxRole.sol approach. Example implementations for key types: bytes32, address, uint and string which should be sufficient to show how to adapt to any other situation that might be needed. I'm not really sure what the workflow should be in this case, so I'll just make a pull request and see what happens. Let me know if you want me to create an issue or expand the idea with unit tests, etc. |
* Drafted Enumerable.sol. * Drafted test framework. * Tweaked the tests to follow oz structure. * Coded EnumerableSet. * Moved EnumerableSet to `utils`. * Fixed linting. * Improved comments. * Tweaked contract description. * Renamed struct to AddressSet. * Relaxed version pragma to 0.5.0 * Removed events. * Revert on useless operations. * Small comment. * Created AddressSet factory method. * Failed transactions return false. * Transactions now return false on failure. * Remove comments from mock * Rename mock functions * Adapt tests to code style, use test-helpers * Fix bug in remove, improve tests. * Add changelog entry * Add entry on Utils doc * Add optimization for removal of last slot * Update docs * Fix headings of utilities documentation Co-authored-by: Nicolás Venturo <[email protected]>
Fixes #1240
An address Set is a data structure containing a number of unique addresses.
It is possible to add and remove addresses in O(1).
It is also possible to query if the Set contains an address in O(1).
It is possible to retrieve an array with all the addresses in the Set using enumerate. This operation is O(N) where N is the number of addresses in the Set. The order in which the addresses are retrieved is not guaranteed.