Merge pull request #29 from windranger-io/afk/metadataUpdaterLocker

Metadata updater role and metadata locking
shl0ms · Feb 24, 2022 · b6ff3a2 · b6ff3a2
2 parents e7f7f8f + d14c897
commit b6ff3a2
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 16 deletions.
diff --git a/contracts/TimelockedERC721.sol b/contracts/TimelockedERC721.sol
@@ -16,11 +16,14 @@ contract TimelockedERC721 is
     ERC721EnumerableUpgradeable
 {
     bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
+    bytes32 public constant METADATA_MODIFIER_ROLE = keccak256("METADATA_MODIFIER_ROLE");
 
     mapping(uint256 => bool) public lockedTokens;
 
     uint256 public tokenUnlockTimestamp;
 
+    bool public metadataUpdatable = true;
+
     string private _baseTokenURI;
 
     event TokenUnlockTimestampSet(address adminAddress, uint256 timestamp);
@@ -44,11 +47,27 @@ contract TimelockedERC721 is
         _;
     }
 
+    modifier onlyMetadataModifier() {
+        require(
+            hasRole(METADATA_MODIFIER_ROLE, _msgSender()),
+            "Doesn't have metadata modifier role!"
+        );
+        _;
+    }
+
     modifier futureTimestamp(uint256 timestamp_) {
         require(timestamp_ > block.timestamp, "timestamp must be in future!");
         _;
     }
 
+    modifier metadataLocked() {
+        require(
+            metadataUpdatable,
+            "Metadata cannot be updated!"
+        );
+        _;
+    }
+
     modifier tokenLocked(uint256 tokenId_) {
         if (lockedTokens[tokenId_]) {
             if (block.timestamp >= tokenUnlockTimestamp) {
@@ -80,6 +99,7 @@ contract TimelockedERC721 is
 
         _setupRole(DEFAULT_ADMIN_ROLE, _msgSender());
         _setupRole(MINTER_ROLE, _msgSender());
+        _setupRole(METADATA_MODIFIER_ROLE, _msgSender());
     }
 
     function setTokenUnlockTimestamp(uint256 timestamp_)
@@ -101,10 +121,19 @@ contract TimelockedERC721 is
         emit MinterAdded(minter_);
     }
 
-    function _baseURI() internal view virtual override returns (string memory) {
-        return _baseTokenURI;
+    function addMetadataModifier(address metadataUpdater_) external onlyAdmin {
+        grantRole(METADATA_MODIFIER_ROLE, metadataUpdater_);
+    }
+
+    function updateBaseTokenURI(string memory newBaseTokenURI_) external onlyMetadataModifier metadataLocked {
+        _baseTokenURI = newBaseTokenURI_;
     }
 
+    function lockMetadata() external onlyMetadataModifier {
+        metadataUpdatable = false;
+    }
+
+
     function lockToken(uint256 tokenId) external {
         require(ownerOf(tokenId) == _msgSender(), "token not owned!");
         require(!lockedTokens[tokenId], "token already locked!");
@@ -122,18 +151,6 @@ contract TimelockedERC721 is
         }
     }
 
-    function _beforeTokenTransfer(
-        address from_,
-        address to_,
-        uint256 tokenId_
-    )
-        internal
-        override(ERC721Upgradeable, ERC721EnumerableUpgradeable)
-        tokenLocked(tokenId_)
-    {
-        super._beforeTokenTransfer(from_, to_, tokenId_);
-    }
-
     function supportsInterface(bytes4 interfaceId)
         public
         view
@@ -147,4 +164,20 @@ contract TimelockedERC721 is
     {
         return super.supportsInterface(interfaceId);
     }
+
+    function _beforeTokenTransfer(
+        address from_,
+        address to_,
+        uint256 tokenId_
+    )
+        internal
+        override(ERC721Upgradeable, ERC721EnumerableUpgradeable)
+        tokenLocked(tokenId_)
+    {
+        super._beforeTokenTransfer(from_, to_, tokenId_);
+    }
+
+    function _baseURI() internal view virtual override returns (string memory) {
+        return _baseTokenURI;
+    }
 }
diff --git a/test/TimelockedERC721.test.ts b/test/TimelockedERC721.test.ts
@@ -47,7 +47,7 @@ describe('Timelocked ERC721', () => {
         )
     })
 
-    it('[sanity] admin has DEFAULT_ADMIN_ROLE and MINTER_ROLE', async () => {
+    it('[sanity] admin has DEFAULT_ADMIN_ROLE, MINTER_ROLE, and METADATA_UPDATER_ROLE', async () => {
         expect(
             await timelockedERC721.hasRole(
                 ethers.utils.keccak256(
@@ -56,12 +56,22 @@ describe('Timelocked ERC721', () => {
                 await admin.getAddress()
             )
         )
+
         expect(
             await timelockedERC721.hasRole(
                 ethers.utils.keccak256(ethers.utils.toUtf8Bytes('MINTER_ROLE')),
                 await admin.getAddress()
             )
         )
+
+        expect(
+            await timelockedERC721.hasRole(
+                ethers.utils.keccak256(
+                    ethers.utils.toUtf8Bytes('METADATA_UPDATER_ROLE')
+                ),
+                await admin.getAddress()
+            )
+        )
     })
 
     it('Should be able to mint tokens to other addresses with minter role', async () => {
@@ -75,7 +85,7 @@ describe('Timelocked ERC721', () => {
         }
     })
 
-    it('Should be able to batchMint to an addres', async () => {
+    it('Should be able to batchMint to an address', async () => {
         await timelockedERC721
             .connect(admin)
             .batchMint(await users[1].getAddress(), [90, 91, 92])
@@ -103,6 +113,25 @@ describe('Timelocked ERC721', () => {
         ).to.be.revertedWith("Doesn't have minter role!")
     })
 
+    it('Should be able to update baseURI with metadata updater role', async () => {
+        await timelockedERC721.connect(admin).updateBaseTokenURI('0x000/')
+
+        expect(await timelockedERC721.tokenURI(0)).to.equal('0x000/0')
+    })
+
+    it('Should not be able to update baseURI without metadata updater role', async () => {
+        await expect(
+            timelockedERC721.connect(users[1]).updateBaseTokenURI('0x000/')
+        ).to.be.revertedWith("Doesn't have metadata modifier role!")
+    })
+
+    it('Should not be able to update baseURI with metadata updater role when metadata is locked', async () => {
+        await timelockedERC721.connect(admin).lockMetadata()
+        await expect(
+            timelockedERC721.connect(admin).updateBaseTokenURI('0x')
+        ).to.be.revertedWith('Metadata cannot be updated!')
+    })
+
     it('Should be able to transfer tokens that have never been locked', async () => {
         await timelockedERC721
             .connect(users[0])