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

Add Utils Struct EnumerableMap Bytes32ToUintMap #3416

Merged
merged 13 commits into from May 23, 2022

Conversation

arturictus
Copy link
Contributor

@arturictus arturictus commented May 17, 2022

Adding Bytes32ToUintMap in struct helpers.

PR Checklist

  • Tests
  • Documentation
  • Changelog entry

@Amxx
Copy link
Collaborator

Amxx commented May 18, 2022

Hello @arturictus

Can you tell us more about your usecase for a bytes32 → uint256 map?

@arturictus
Copy link
Contributor Author

arturictus commented May 18, 2022

hello @Amxx,
Of course.
I'm building a booking contract which allows to book one night per year.
The approach I'm taking is:

  • the frontend will supply (uint16 year, uint16 dayOfTheYear [1-366])
  • The bookings and cancellations can not be made to passed dates.
  • Each wallet can only book once per year-dayOfTheYear

I use the map as

  • key: bytes32(abi.encode(year, dayOftheYear))
  • value: timestamp (calculated timestamp to compare to block.timestamp)

This allows me:

  • validate uniqueness of the reservation
  • Validate time constraints with the timestamp
  • create, update and cancel reservations with O(1) complexity
  • Be able to iterate over reservations for calculations (done in the client)

I can imagine other use cases:

  • key: productID, value: price
  • key: saleID, value: time until applicable
  • etc, etc

Happy to hear your thoughts

@Amxx
Copy link
Collaborator

Amxx commented May 18, 2022

Thanks for the details. In your case, I would argue that the key could be a uint corresponding to the day's "timestamp". I would consider that safer than bytes32(abi.encode(year, dayOftheYear)) (you are sure you're not doing a hash here ?)

We also have #3338. We'll discuss new mapping types soon

@arturictus
Copy link
Contributor Author

arturictus commented May 20, 2022

Hi @Amxx,
Thanks for your response.
Answering your comments:

I would argue that the key could be a uint corresponding to the day's "timestamp"

The problem with that is that a day has 86400 seconds which I can not use timestamp for unique key. (there are some workarounds, but I'm not happy with them)

you are sure you're not doing a hash here ?

Still considering to hash it unless I need to get the original key.
The inputs are validated so they are quite limited in range and always fit in bytes32 range.

@Amxx
Copy link
Collaborator

Amxx commented May 20, 2022

Could you please add a changelog entry?

@arturictus
Copy link
Contributor Author

Could you please add a changelog entry?

Yes!, done :)

@Amxx Amxx enabled auto-merge (squash) May 23, 2022 07:53
@Amxx Amxx merged commit de74c8c into OpenZeppelin:master May 23, 2022
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.

None yet

2 participants