Skip to content

feat: freeze NFT Metadata #506

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

Merged
merged 8 commits into from
Sep 27, 2023
Merged

Conversation

WhiteOakKong
Copy link
Contributor

@WhiteOakKong WhiteOakKong commented Sep 26, 2023

Updating the existing NFTMetadata extension to allow for the freezing of individual token URIs.

In addition to the updated NFTMetadata extension, the extension was added to the prebuilt contract TokenERC721.

The internal function _canFreezeMetadata and _canSetMetadata are virtual to remain unopinionated, but the implementation in TokenERC721 allows the DEFAULT_ADMIN_ROLE to update the metadata and the owner of the individual token to freeze the metadata.

Additional minor updates to LoyaltyCard.sol were made to fit the changes to NFTMetadata extension.

The NFTMetadata extension has full test coverage.

WhiteOakKong and others added 5 commits September 26, 2023 10:04
update NFTMetadata extension,
add extension to prebuilt TokenERC721.
update LoyaltyCard to fit updated extension.
add test file for NFTMetadata (no test written yet)
- add testing coverage for NFTMetadata extension
- Move URIFrozen to NFTMetadata extension
- update INFTMetadata interface
based on feedback:
- replace freezeTokenURI with freezeMetadata
- make metadata freeze apply to all tokens
- add METADATA_ROLE to TokenERC721
- adjust TokenERC721 implementation such that METADATA_ROLE is required to set and freeze metadata.
@WhiteOakKong WhiteOakKong marked this pull request as ready for review September 27, 2023 03:30
@nkrishang nkrishang merged commit b4dd853 into thirdweb-dev:main Sep 27, 2023
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