Merge pull request #1106 from thesandboxgame/catalyst-audit/L-02-arra…

…y-may-contain-duplicate-value

Catalyst audit: L-02 array may contain duplicate value

packages/asset/contracts/Asset.sol

@@ -336,7 +336,7 @@ contract Asset is
         return TokenIdUtils.isBridged(tokenId);
     }
 
-    /// @notice This function is used to register Asset contract on the Operator Filterer Registry of Opensea.can only be called by admin.
+    /// @notice This function is used to register Asset contract on the Operator Filterer Registry of OpenSea.can only be called by admin.
     /// @dev used to register contract and subscribe to the subscriptionOrRegistrantToCopy's black list.
     /// @param subscriptionOrRegistrantToCopy registration address of the list to subscribe.
     /// @param subscribe bool to signify subscription "true"" or to copy the list "false".

packages/asset/contracts/Catalyst.sol

@@ -30,7 +30,7 @@ import {ICatalyst} from "./interfaces/ICatalyst.sol";
 
 /// @title Catalyst
 /// @author The Sandbox
-/// @notice THis contract manages catalysts which are used to mint new assets.
+/// @notice This contract manages catalysts which are used to mint new assets.
 /// @dev An ERC1155 contract that manages catalysts, extends multiple OpenZeppelin contracts to
 /// provide a variety of features including, AccessControl, URIStorage, Burnable and more.
 /// The contract includes support for meta transactions.
@@ -77,7 +77,7 @@ contract Catalyst is
         address _defaultMinter,
         string[] memory _catalystIpfsCID,
         address _royaltyManager
-    ) public initializer {
+    ) external initializer {
         require(bytes(_baseUri).length != 0, "Catalyst: base uri can't be empty");
         require(_trustedForwarder != address(0), "Catalyst: trusted forwarder can't be zero");
         require(_subscription != address(0), "Catalyst: subscription can't be zero");
@@ -154,7 +154,7 @@ contract Catalyst is
     }
 
     /// @notice Add a new catalyst type, limited to DEFAULT_ADMIN_ROLE only
-    /// @param ipfsCID The royalty bps for the catalyst
+    /// @param ipfsCID The IPFS content identifiers for the catalyst
     function addNewCatalystType(string memory ipfsCID) external onlyRole(DEFAULT_ADMIN_ROLE) {
         require(bytes(ipfsCID).length != 0, "Catalyst: CID can't be empty");
         uint256 newCatId = ++highestTierIndex;
@@ -187,6 +187,7 @@ contract Catalyst is
     function setBaseURI(string memory baseURI) external onlyRole(DEFAULT_ADMIN_ROLE) {
         require(bytes(baseURI).length != 0, "Catalyst: base uri can't be empty");
         _setBaseURI(baseURI);
+        emit BaseURISet(baseURI);
     }
 
     /// @notice returns full token URI, including baseURI and token metadata URI
@@ -223,12 +224,18 @@ contract Catalyst is
         return ERC2771HandlerUpgradeable._msgData();
     }
 
+    /// @dev Sets `baseURI` as the `_baseURI` for all tokens
+    function _setBaseURI(string memory baseURI) internal virtual override {
+        super._setBaseURI(baseURI);
+        emit BaseURISet(baseURI);
+    }
+
     /// @notice Transfers `value` tokens of type `id` from  `from` to `to`  (with safety call).
     /// @param from address from which tokens are transfered.
     /// @param to address to which the token will be transfered.
     /// @param id the token type transfered.
     /// @param value amount of token transfered.
-    /// @param data aditional data accompanying the transfer.
+    /// @param data additional data accompanying the transfer.
     function safeTransferFrom(
         address from,
         address to,
@@ -245,7 +252,7 @@ contract Catalyst is
     /// @param to address to which the token will be transfered.
     /// @param ids ids of each token type transfered.
     /// @param values amount of each token type transfered.
-    /// @param data aditional data accompanying the transfer.
+    /// @param data additional data accompanying the transfer.
     function safeBatchTransferFrom(
         address from,
         address to,
@@ -286,7 +293,7 @@ contract Catalyst is
         return super.supportsInterface(interfaceId);
     }
 
-    /// @notice This function is used to register Catalyst contract on the Operator Filterer Registry of Opensea.can only be called by admin.
+    /// @notice This function is used to register Catalyst contract on the Operator Filterer Registry of OpenSea. Can only be called by admin.
     /// @dev used to register contract and subscribe to the subscriptionOrRegistrantToCopy's black list.
     /// @param subscriptionOrRegistrantToCopy registration address of the list to subscribe.
     /// @param subscribe bool to signify subscription "true"" or to copy the list "false".
@@ -298,10 +305,13 @@ contract Catalyst is
         _registerAndSubscribe(subscriptionOrRegistrantToCopy, subscribe);
     }
 
-    /// @notice sets filter registry address deployed in test
+    /// @notice sets filter registry address
     /// @param registry the address of the registry
     function setOperatorRegistry(address registry) external onlyRole(DEFAULT_ADMIN_ROLE) {
         require(registry != address(0), "Catalyst: registry can't be zero address");
         OperatorFiltererUpgradeable._setOperatorFilterRegistry(registry);
+        emit OperatorRegistrySet(registry);
     }
+
+    uint256[49] private __gap;
 }

packages/asset/contracts/interfaces/ICatalyst.sol

@@ -7,6 +7,8 @@ interface ICatalyst {
     event TrustedForwarderChanged(address indexed newTrustedForwarderAddress);
     event NewCatalystTypeAdded(uint256 catalystId);
     event DefaultRoyaltyChanged(address indexed newDefaultRoyaltyRecipient, uint256 newDefaultRoyaltyAmount);
+    event BaseURISet(string baseURI);
+    event OperatorRegistrySet(address indexed registry);
 
     /// @notice Mints a new token, limited to MINTER_ROLE only
     /// @param to The address that will own the minted token

packages/asset/docs/Asset.md

@@ -176,14 +176,15 @@ Sets a new trusted forwarder for meta-transactions.
 Parameters:
 
 - `trustedForwarder` - The new trusted forwarder.
+
 ### setTokenRoyalties
 
 ```solidity
 function setTokenRoyalties(
         uint256 tokenId,
         address payable recipient,
         address creator
-) external override onlyRole(DEFAULT_ADMIN_ROLE) 
+) external override onlyRole(DEFAULT_ADMIN_ROLE)
 ```
 
 Sets token royalty i.e. the creator splitter address as EIP2981 royalty recipient. deploys a splitter if there is none deployed for a creator. Only admin can call it.
@@ -254,7 +255,6 @@ Parameters:
 
 - `tokenId` - the id of the token.
 
-
 ### isBridged
 
 ```solidity
@@ -275,7 +275,7 @@ function registerAndSubscribe(address subscriptionOrRegistrantToCopy, bool subsc
     onlyRole(DEFAULT_ADMIN_ROLE)
 ```
 
-Used to register and subscribe on the operator filter registry of Opensea
+Used to register and subscribe on the operator filter registry of OpenSea
 
 Parameters:
 
@@ -286,7 +286,7 @@ Parameters:
 
 ```solidity
 function setOperatorRegistry(address registry) external onlyRole(DEFAULT_ADMIN_ROLE)
-```fu
+```
 
 Used to the address of the operator filter registry
 

packages/asset/test/AssetReveal.test.ts

@@ -66,8 +66,8 @@ describe('AssetReveal (/packages/asset/contracts/AssetReveal.sol)', function ()
         newMetadataHashes1,
         [revealHashA]
       );
-      expect(result.events[4].event).to.equal('AssetRevealMint');
-      const newTokenId = result.events[4].args.newTokenIds[0];
+      expect(result.events[3].event).to.equal('AssetRevealMint');
+      const newTokenId = result.events[3].args.newTokenIds[0];
       const revealNonce = await TokenIdUtilsContract.getRevealNonce(newTokenId);
       expect(revealNonce.toString()).to.equal('1');
 
@@ -86,8 +86,8 @@ describe('AssetReveal (/packages/asset/contracts/AssetReveal.sol)', function ()
         [revealHashB]
       );
 
-      expect(result2.events[4].event).to.equal('AssetRevealMint');
-      const newTokenId2 = result2.events[4].args.newTokenIds[0];
+      expect(result2.events[3].event).to.equal('AssetRevealMint');
+      const newTokenId2 = result2.events[3].args.newTokenIds[0];
       const revealNonce2 = await TokenIdUtilsContract.getRevealNonce(
         newTokenId2
       );
@@ -123,8 +123,8 @@ describe('AssetReveal (/packages/asset/contracts/AssetReveal.sol)', function ()
         [revealHashA]
       );
 
-      expect(result.events[4].event).to.equal('AssetRevealMint');
-      const newTokenId = result.events[4].args.newTokenIds[0];
+      expect(result.events[3].event).to.equal('AssetRevealMint');
+      const newTokenId = result.events[3].args.newTokenIds[0];
       const revealNonce = await TokenIdUtilsContract.getRevealNonce(newTokenId);
       expect(revealNonce.toString()).to.equal('1');
 
@@ -143,8 +143,8 @@ describe('AssetReveal (/packages/asset/contracts/AssetReveal.sol)', function ()
         [revealHashB]
       );
 
-      expect(result2.events[3].event).to.equal('AssetRevealMint');
-      const newTokenId2 = result2.events[3].args.newTokenIds[0];
+      expect(result2.events[1].event).to.equal('AssetRevealMint');
+      const newTokenId2 = result2.events[1].args.newTokenIds[0];
       const revealNonce2 = await TokenIdUtilsContract.getRevealNonce(
         newTokenId2
       );
@@ -650,8 +650,8 @@ describe('AssetReveal (/packages/asset/contracts/AssetReveal.sol)', function ()
             [revealHashA]
           );
 
-          expect(result.events[4].event).to.equal('AssetRevealMint');
-          const newTokenId = result.events[4].args.newTokenIds[0];
+          expect(result.events[3].event).to.equal('AssetRevealMint');
+          const newTokenId = result.events[3].args.newTokenIds[0];
           const balance = await AssetContract.balanceOf(
             user.address,
             newTokenId
@@ -684,9 +684,9 @@ describe('AssetReveal (/packages/asset/contracts/AssetReveal.sol)', function ()
             newMetadataHashes,
             [revealHashA]
           );
-          expect(result.events[4].event).to.equal('AssetRevealMint');
-          expect(result.events[4].args['newTokenIds'].length).to.equal(1);
-          const newTokenId = result.events[4].args.newTokenIds[0];
+          expect(result.events[3].event).to.equal('AssetRevealMint');
+          expect(result.events[3].args['newTokenIds'].length).to.equal(1);
+          const newTokenId = result.events[3].args.newTokenIds[0];
           const balance = await AssetContract.balanceOf(
             user.address,
             newTokenId
@@ -720,7 +720,7 @@ describe('AssetReveal (/packages/asset/contracts/AssetReveal.sol)', function ()
             [revealHashA]
           );
 
-          const newTokenId = result.events[4].args.newTokenIds[0];
+          const newTokenId = result.events[3].args.newTokenIds[0];
           const balance = await AssetContract.balanceOf(
             user.address,
             newTokenId
@@ -791,9 +791,8 @@ describe('AssetReveal (/packages/asset/contracts/AssetReveal.sol)', function ()
               revealHashF,
             ]
           );
-
-          expect(result.events[19].event).to.equal('AssetRevealMint');
-          expect(result.events[19].args['newTokenIds'].length).to.equal(6);
+          expect(result.events[13].event).to.equal('AssetRevealMint');
+          expect(result.events[13].args['newTokenIds'].length).to.equal(6);
         });
         it('should set the reveal hash as used after successful mint', async function () {
           const {
@@ -958,7 +957,7 @@ describe('AssetReveal (/packages/asset/contracts/AssetReveal.sol)', function ()
             newMetadataHashes,
             [revealHashA, revealHashB]
           );
-          expect(result.events[7].event).to.equal('AssetRevealMint');
+          expect(result.events[5].event).to.equal('AssetRevealMint');
         });
         it('should emit AssetRevealMint event with correct arguments', async function () {
           const {
@@ -988,9 +987,9 @@ describe('AssetReveal (/packages/asset/contracts/AssetReveal.sol)', function ()
             [revealHashA, revealHashB]
           );
 
-          expect(result.events[7].event).to.equal('AssetRevealMint');
+          expect(result.events[5].event).to.equal('AssetRevealMint');
 
-          const args = result.events[7].args;
+          const args = result.events[5].args;
           const {
             recipient,
             unrevealedTokenId,