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 aux to AddressData #98

Merged
merged 3 commits into from
Feb 18, 2022
Merged

Add aux to AddressData #98

merged 3 commits into from
Feb 18, 2022

Conversation

Vectorized
Copy link
Collaborator

@Vectorized Vectorized commented Feb 13, 2022

After adding the burn feature, the AddressData struct still has 64 bits of unused space.

This is free real estate.

This PR adds an aux variable for the AddressData to let users make full use of the extra space,
along with the relevant _getAux and _setAux functions.

This aux can be used for storing miscellaneous things like number of whitelist mint slots used.

Deriving contracts can avoid the overhead of an additional expensive SLOAD and SSTORE with this feature.

If there are no likely plans to use the extra 64 bits of space for future features, we can consider this.

@Vectorized Vectorized changed the title Added aux for AddressData Add aux to AddressData Feb 13, 2022
Copy link
Contributor

@ahbanavi ahbanavi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and yes it's the perfect place for storing whitelist mints.

Copy link
Collaborator

@cygaar cygaar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a couple unit tests to verify the behavior? Should be simple since you're just checking that the setter/getter works.

@cygaar
Copy link
Collaborator

cygaar commented Feb 16, 2022

@Vectorized for some reason you have changes from the past 2 merged PRs in your PR. It might be worth rebasing your code on top of main instead of trying to do a merge commit. Not too sure what happened. Other than that, the PR looks good

@cygaar cygaar merged commit ddd1197 into chiru-labs:main Feb 18, 2022
@Vectorized Vectorized deleted the feature/aux branch April 12, 2022 13:52
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