feat: freeze NFT Metadata (#506)

* feat: freeze NFT metadata

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 for NFTMetadata

- add testing coverage for NFTMetadata extension
- Move URIFrozen to NFTMetadata extension
- update INFTMetadata interface

* rework metadata extension

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.

* Update URI structure of TokenERC721

* Fix review comments + lint + additional tests

* add NFTMetadata extension to TokenERC1155 + tests

* lint

Author: WhiteOakKong

contracts/extension/NFTMetadata.sol

@@ -4,14 +4,16 @@ pragma solidity ^0.8.0;
 import "./interface/INFTMetadata.sol";
 
 abstract contract NFTMetadata is INFTMetadata {
-    mapping(uint256 => string) private _tokenURI;
+    bool public uriFrozen;
+
+    mapping(uint256 => string) internal _tokenURI;
 
     /// @notice Returns the metadata URI for a given NFT.
     function _getTokenURI(uint256 _tokenId) internal view virtual returns (string memory) {
         return _tokenURI[_tokenId];
     }
 
-    /// @notice SEts the metadata URI for a given NFT.
+    /// @notice Sets the metadata URI for a given NFT.
     function _setTokenURI(uint256 _tokenId, string memory _uri) internal virtual {
         require(bytes(_uri).length > 0, "NFTMetadata: empty metadata.");
         _tokenURI[_tokenId] = _uri;
@@ -21,10 +23,19 @@ abstract contract NFTMetadata is INFTMetadata {
 
     /// @notice Sets the metadata URI for a given NFT.
     function setTokenURI(uint256 _tokenId, string memory _uri) public virtual {
-        require(_canSetMetadata(), "Not authorized to set metadata");
+        require(_canSetMetadata(), "NFTMetadata: not authorized to set metadata.");
+        require(!uriFrozen, "NFTMetadata: metadata is frozen.");
         _setTokenURI(_tokenId, _uri);
     }
 
+    function freezeMetadata() public virtual {
+        require(_canFreezeMetadata(), "NFTMetadata: not authorized to freeze metdata");
+        uriFrozen = true;
+        emit MetadataFrozen();
+    }
+
     /// @dev Returns whether metadata can be set in the given execution context.
     function _canSetMetadata() internal view virtual returns (bool);
+
+    function _canFreezeMetadata() internal view virtual returns (bool);
 }

contracts/extension/interface/INFTMetadata.sol

@@ -4,6 +4,14 @@ pragma solidity ^0.8.0;
 import "../../eip/interface/IERC4906.sol";
 
 interface INFTMetadata is IERC4906 {
+    /// @dev This event emits when the metadata of all tokens are frozen.
+    /// While not currently supported by marketplaces, this event allows
+    /// future indexing if desired.
+    event MetadataFrozen();
+
     /// @notice Sets the metadata URI for a given NFT.
     function setTokenURI(uint256 _tokenId, string memory _uri) external;
+
+    /// @notice Freezes the metadata URI for a given NFT.
+    function freezeMetadata() external;
 }

contracts/prebuilts/loyalty/LoyaltyCard.sol

@@ -350,6 +350,11 @@ contract LoyaltyCard is
         return hasRole(METADATA_ROLE, _msgSender());
     }
 
+    /// @dev Returns whether metadata can be frozen in the given execution context.
+    function _canFreezeMetadata() internal view virtual override returns (bool) {
+        return hasRole(METADATA_ROLE, _msgSender());
+    }
+
     /// @dev Returns whether operator restriction can be set in the given execution context.
     function _canSetOperatorRestriction() internal virtual override returns (bool) {
         return hasRole(DEFAULT_ADMIN_ROLE, _msgSender());

contracts/prebuilts/token/TokenERC1155.sol

@@ -21,6 +21,8 @@ import "../../extension/interface/IPrimarySale.sol";
 import "../../extension/interface/IRoyalty.sol";
 import "../../extension/interface/IOwnable.sol";
 
+import "../../extension/NFTMetadata.sol";
+
 // Token
 import "@openzeppelin/contracts-upgradeable/token/ERC1155/ERC1155Upgradeable.sol";
 
@@ -59,7 +61,8 @@ contract TokenERC1155 is
     AccessControlEnumerableUpgradeable,
     DefaultOperatorFiltererUpgradeable,
     ERC1155Upgradeable,
-    ITokenERC1155
+    ITokenERC1155,
+    NFTMetadata
 {
     using ECDSAUpgradeable for bytes32;
     using StringsUpgradeable for uint256;
@@ -82,6 +85,8 @@ contract TokenERC1155 is
     bytes32 private constant TRANSFER_ROLE = keccak256("TRANSFER_ROLE");
     /// @dev Only MINTER_ROLE holders can sign off on `MintRequest`s.
     bytes32 private constant MINTER_ROLE = keccak256("MINTER_ROLE");
+    /// @dev Only METADATA_ROLE holders can update NFT metadata.
+    bytes32 private constant METADATA_ROLE = keccak256("METADATA_ROLE");
 
     /// @dev Max bps in the thirdweb system
     uint256 private constant MAX_BPS = 10_000;
@@ -119,8 +124,6 @@ contract TokenERC1155 is
     /// @dev Mapping from mint request UID => whether the mint request is processed.
     mapping(bytes32 => bool) private minted;
 
-    mapping(uint256 => string) private _tokenURI;
-
     /// @dev Token ID => total circulating supply of tokens with that ID.
     mapping(uint256 => uint256) public totalSupply;
 
@@ -173,6 +176,9 @@ contract TokenERC1155 is
         _setupRole(MINTER_ROLE, _defaultAdmin);
         _setupRole(TRANSFER_ROLE, _defaultAdmin);
         _setupRole(TRANSFER_ROLE, address(0));
+
+        _setupRole(METADATA_ROLE, _defaultAdmin);
+        _setRoleAdmin(METADATA_ROLE, METADATA_ROLE);
     }
 
     ///     =====   Public functions  =====
@@ -388,8 +394,7 @@ contract TokenERC1155 is
         uint256 _amount
     ) internal {
         if (bytes(_tokenURI[_tokenId]).length == 0) {
-            require(bytes(_uri).length > 0, "empty uri.");
-            _tokenURI[_tokenId] = _uri;
+            _setTokenURI(_tokenId, _uri);
         }
 
         _mint(_to, _tokenId, _amount, "");
@@ -584,6 +589,16 @@ contract TokenERC1155 is
         return hasRole(DEFAULT_ADMIN_ROLE, _msgSender());
     }
 
+    /// @dev Returns whether metadata can be set in the given execution context.
+    function _canSetMetadata() internal view virtual override returns (bool) {
+        return hasRole(METADATA_ROLE, _msgSender());
+    }
+
+    /// @dev Returns whether metadata can be frozen in the given execution context.
+    function _canFreezeMetadata() internal view virtual override returns (bool) {
+        return hasRole(METADATA_ROLE, _msgSender());
+    }
+
     function _msgSender()
         internal
         view

contracts/prebuilts/token/TokenERC721.sol

@@ -21,6 +21,9 @@ import "../../extension/interface/IPrimarySale.sol";
 import "../../extension/interface/IRoyalty.sol";
 import "../../extension/interface/IOwnable.sol";
 
+//Extensions
+import "../../extension/NFTMetadata.sol";
+
 // Token
 import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol";
 
@@ -61,7 +64,8 @@ contract TokenERC721 is
     AccessControlEnumerableUpgradeable,
     DefaultOperatorFiltererUpgradeable,
     ERC721EnumerableUpgradeable,
-    ITokenERC721
+    ITokenERC721,
+    NFTMetadata
 {
     using ECDSAUpgradeable for bytes32;
     using StringsUpgradeable for uint256;
@@ -78,6 +82,8 @@ contract TokenERC721 is
     bytes32 private constant TRANSFER_ROLE = keccak256("TRANSFER_ROLE");
     /// @dev Only MINTER_ROLE holders can sign off on `MintRequest`s.
     bytes32 private constant MINTER_ROLE = keccak256("MINTER_ROLE");
+    /// @dev Only METADATA_ROLE holders can update NFT metadata.
+    bytes32 private constant METADATA_ROLE = keccak256("METADATA_ROLE");
 
     /// @dev Max bps in the thirdweb system
     uint256 private constant MAX_BPS = 10_000;
@@ -109,9 +115,6 @@ contract TokenERC721 is
     /// @dev Mapping from mint request UID => whether the mint request is processed.
     mapping(bytes32 => bool) private minted;
 
-    /// @dev Mapping from tokenId => URI
-    mapping(uint256 => string) private uri;
-
     /// @dev Token ID => royalty recipient and bps for token
     mapping(uint256 => RoyaltyInfo) private royaltyInfoForToken;
 
@@ -151,6 +154,10 @@ contract TokenERC721 is
         _owner = _defaultAdmin;
         _setupRole(DEFAULT_ADMIN_ROLE, _defaultAdmin);
         _setupRole(MINTER_ROLE, _defaultAdmin);
+
+        _setupRole(METADATA_ROLE, _defaultAdmin);
+        _setRoleAdmin(METADATA_ROLE, METADATA_ROLE);
+
         _setupRole(TRANSFER_ROLE, _defaultAdmin);
         _setupRole(TRANSFER_ROLE, address(0));
     }
@@ -182,7 +189,7 @@ contract TokenERC721 is
 
     /// @dev Returns the URI for a tokenId
     function tokenURI(uint256 _tokenId) public view override returns (string memory) {
-        return uri[_tokenId];
+        return _tokenURI[_tokenId];
     }
 
     /// @dev Lets an account with MINTER_ROLE mint an NFT.
@@ -320,7 +327,7 @@ contract TokenERC721 is
         nextTokenIdToMint += 1;
 
         require(bytes(_uri).length > 0, "empty uri.");
-        uri[tokenIdToMint] = _uri;
+        _setTokenURI(tokenIdToMint, _uri);
 
         _safeMint(_to, tokenIdToMint);
 
@@ -464,6 +471,16 @@ contract TokenERC721 is
         return hasRole(DEFAULT_ADMIN_ROLE, _msgSender());
     }
 
+    /// @dev Returns whether metadata can be set in the given execution context.
+    function _canSetMetadata() internal view virtual override returns (bool) {
+        return hasRole(METADATA_ROLE, _msgSender());
+    }
+
+    /// @dev Returns whether metadata can be frozen in the given execution context.
+    function _canFreezeMetadata() internal view virtual override returns (bool) {
+        return hasRole(METADATA_ROLE, _msgSender());
+    }
+
     function supportsInterface(bytes4 interfaceId)
         public
         view

src/test/LoyaltyCard.t.sol

@@ -302,6 +302,39 @@ contract LoyaltyCardTest is BaseTest {
         loyaltyCard.mintWithSignature(_mintrequest, _signature);
     }
 
+    /*///////////////////////////////////////////////////////////////
+                        Unit tests: `setTokenURI`
+    //////////////////////////////////////////////////////////////*/
+
+    function test_setTokenURI_state() public {
+        string memory uri = "uri_string";
+
+        vm.prank(signer);
+        loyaltyCard.setTokenURI(0, uri);
+
+        string memory _tokenURI = loyaltyCard.tokenURI(0);
+
+        assertEq(_tokenURI, uri);
+    }
+
+    function test_setTokenURI_revert_NotAuthorized() public {
+        string memory uri = "uri_string";
+
+        vm.expectRevert("NFTMetadata: not authorized to set metadata.");
+        vm.prank(address(0x1));
+        loyaltyCard.setTokenURI(0, uri);
+    }
+
+    function test_setTokenURI_revert_Frozen() public {
+        string memory uri = "uri_string";
+
+        vm.startPrank(signer);
+        loyaltyCard.freezeMetadata();
+
+        vm.expectRevert("NFTMetadata: metadata is frozen.");
+        loyaltyCard.setTokenURI(0, uri);
+    }
+
     /*///////////////////////////////////////////////////////////////
                         Audit fixes tests
     //////////////////////////////////////////////////////////////*/

src/test/sdk/extension/NFTMetadata.t.sol

@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.11;
+
+import "@std/Test.sol";
+import "@ds-test/test.sol";
+
+import { NFTMetadata } from "contracts/extension/NFTMetadata.sol";
+
+contract NFTMetadataHarness is NFTMetadata {
+    address private authorized;
+
+    constructor() {
+        authorized = msg.sender;
+    }
+
+    function _canSetMetadata() internal view override returns (bool) {
+        if (msg.sender == authorized) return true;
+        return false;
+    }
+
+    function _canFreezeMetadata() internal view override returns (bool) {
+        if (msg.sender == authorized) return true;
+        return false;
+    }
+
+    function getTokenURI(uint256 _tokenId) external view returns (string memory) {
+        return _getTokenURI(_tokenId);
+    }
+
+    function URIStatus() external view returns (bool) {
+        return uriFrozen;
+    }
+
+    function supportsInterface(bytes4 interfaceId) external view override returns (bool) {}
+}
+
+contract ExtensionNFTMetadata is DSTest, Test {
+    NFTMetadataHarness internal ext;
+
+    function setUp() public {
+        ext = new NFTMetadataHarness();
+    }
+
+    /*///////////////////////////////////////////////////////////////
+                        Unit tests: `setTokenURI`
+    //////////////////////////////////////////////////////////////*/
+
+    function test_setTokenURI_state() public {
+        string memory uri = "test";
+        ext.setTokenURI(0, uri);
+        assertEq(ext.getTokenURI(0), uri);
+
+        string memory uri2 = "test2";
+        ext.setTokenURI(0, uri2);
+        assertEq(ext.getTokenURI(0), uri2);
+    }
+
+    function test_setTokenURI_revert_notAuthorized() public {
+        vm.startPrank(address(0x1));
+        string memory uri = "test";
+        vm.expectRevert("NFTMetadata: not authorized to set metadata.");
+        ext.setTokenURI(1, uri);
+    }
+
+    function test_setTokenURI_revert_emptyMetadata() public {
+        string memory uri = "";
+        vm.expectRevert("NFTMetadata: empty metadata.");
+        ext.setTokenURI(1, uri);
+    }
+
+    function test_setTokenURI_revert_frozen() public {
+        ext.freezeMetadata();
+        string memory uri = "test";
+        vm.expectRevert("NFTMetadata: metadata is frozen.");
+        ext.setTokenURI(2, uri);
+    }
+
+    /*///////////////////////////////////////////////////////////////
+                        Unit tests: `freezeMetadata`
+    //////////////////////////////////////////////////////////////*/
+
+    function test_freezeMetadata_state() public {
+        ext.freezeMetadata();
+        assertEq(ext.URIStatus(), true);
+    }
+
+    function test_freezeMetadata_revert_notAuthorized() public {
+        vm.startPrank(address(0x1));
+        vm.expectRevert("NFTMetadata: not authorized to freeze metdata");
+        ext.freezeMetadata();
+    }
+}

src/test/token/TokenERC1155.t.sol

@@ -903,4 +903,37 @@ contract TokenERC1155Test is BaseTest {
         vm.prank(address(0x1));
         tokenContract.setContractURI("");
     }
+
+    /*///////////////////////////////////////////////////////////////
+                        Unit tests: setTokenURI
+    //////////////////////////////////////////////////////////////*/
+
+    function test_setTokenURI_state() public {
+        string memory uri = "uri_string";
+
+        vm.prank(deployerSigner);
+        tokenContract.setTokenURI(0, uri);
+
+        string memory _tokenURI = tokenContract.uri(0);
+
+        assertEq(_tokenURI, uri);
+    }
+
+    function test_setTokenURI_revert_NotAuthorized() public {
+        string memory uri = "uri_string";
+
+        vm.expectRevert("NFTMetadata: not authorized to set metadata.");
+        vm.prank(address(0x1));
+        tokenContract.setTokenURI(0, uri);
+    }
+
+    function test_setTokenURI_revert_Frozen() public {
+        string memory uri = "uri_string";
+
+        vm.startPrank(deployerSigner);
+        tokenContract.freezeMetadata();
+
+        vm.expectRevert("NFTMetadata: metadata is frozen.");
+        tokenContract.setTokenURI(0, uri);
+    }
 }