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

✨ EnumerableKeyValueSetLib #1315

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

0xLightt
Copy link

Description

This pull request introduces an optimized version of EnurableMapLib designed for scenarios where both the key and value can fit into a single slot. Currently, it supports address keys mapped to uint96 values.

If this functionality is deemed useful, I am open to adding more tests and functions to ensure it aligns with the existing EnurableMapLib. Any input or feedback would be greatly appreciated!

It is also possible to add a new entry to prep, allowing for combinations that fit within a single slot.

Checklist

  • Ran forge fmt?
  • Ran forge test?

@Vectorized
Copy link
Owner

This is tempting.

Lemme marinate.

@Vectorized
Copy link
Owner

@0xLightt Are you using this in your project or planning to use this in an upcoming project?

@Vectorized
Copy link
Owner

Taste wise, I think this should be AddressToUint96Map in EnumerableMapLib.

Another problem is the audit has concluded, and EnumerableSet is kind of a hard file. I can activate full-effort God-eye to ensure that there's no bug, but you know... public perception. Not everyone trusts my spidersense.

@0xLightt
Copy link
Author

I am not using exactly this. It's slightly modified because we only have one instance per contract and don't have any zero addresses or values. So we can skip the whole zero sentinel logic and also just have 3 functions: 'values' and both 'addOrUpdate' and 'remove' functions that return the previous values. I will gladly share it, but it's too tailored to add to solady.

Appreciate the naming suggestion! ❤️

It goes without saying, but it's totally up to you. In my opinion, if there is to be an address to uint96 enumerable map it will eventually be optimized to use 1 slot. If the risk or effort isn't worth optimizing it at the moment, then I think that what already exists is more than enough.

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.

2 participants