test: add tests and mocks to get 100% coverage

Also some small improvementes to the contract

packages/avatar/contracts/mocks/MockOperatorFilterRegistry.sol

@@ -23,6 +23,8 @@ contract MockOperatorFilterRegistry {
     bool public revertWithAddressFiltered;
 
     bool public revertWithCodeHashFiltered;
+
+    bool public returnFalse;
     /**
      * @notice Restricts method caller to the address or EIP-173 "owner()"
      */
@@ -51,15 +53,37 @@ contract MockOperatorFilterRegistry {
         revertWithCodeHashFiltered = _revertWithCodeHashFiltered;
     }
 
+    function doFalse(bool _returnFalse) external {
+        returnFalse = _returnFalse;
+    }
+
+
     /**
      * @notice Registers an address with the registry. May be called by address itself or by EIP-173 owner.
      */
     function register(address registrant) external onlyAddressOrOwner(registrant) {
-        if (_registrations[registrant] != address(0)) {
-            revert AlreadyRegistered();
-        }
-        _registrations[registrant] = registrant;
-        emit RegistrationUpdated(registrant, true);
+        _register(registrant);
+    }
+
+    /**
+     * @notice Registers an address with the registry and "subscribes" to another address's filtered operators and codeHashes.
+     * @param registrant the address of the contract to check for (usually address(this))
+     * @param subscription address to subscribe to
+     */
+    function registerAndSubscribe(address registrant, address subscription) external onlyAddressOrOwner(registrant) {
+        require(subscription!=address(0), "invalid address");
+        _register(registrant);
+    }
+
+    /**
+     * @notice Registers an address with the registry and copies the filtered operators and codeHashes from another
+     *         address without subscribing.
+     * @param registrant the address of the contract to check for (usually address(this))
+     * @param registrantToCopy address to copy entries from
+     */
+    function registerAndCopyEntries(address registrant, address registrantToCopy) external onlyAddressOrOwner(registrant) {
+        require(registrantToCopy!=address(0), "invalid address");
+        _register(registrant);
     }
 
     /**
@@ -85,6 +109,8 @@ contract MockOperatorFilterRegistry {
             } else if (revertWithCodeHashFiltered) {
                 bytes32 codeHash = operator.codehash;
                 revert CodeHashFiltered(operator, codeHash);
+            } else if (returnFalse) {
+                return false;
             }
         }
         return true;
@@ -96,4 +122,16 @@ contract MockOperatorFilterRegistry {
     function codeHashOf(address a) external view returns (bytes32) {
         return a.codehash;
     }
+
+    /**
+     * @notice Registers an address with the registry. May be called by address itself or by EIP-173 owner.
+     */
+    function _register(address registrant) internal {
+        if (_registrations[registrant] != address(0)) {
+            revert AlreadyRegistered();
+        }
+        _registrations[registrant] = registrant;
+        emit RegistrationUpdated(registrant, true);
+    }
+
 }

packages/avatar/contracts/mocks/NFTCollectionMock.sol

@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity 0.8.26;
+
+import {NFTCollection} from "../nft-collection/NFTCollection.sol";
+
+
+contract NFTCollectionMock is NFTCollection {
+    /// @custom:oz-upgrades-unsafe-allow constructor
+    constructor(address _collectionOwner, address _initialTrustedForwarder)  {
+        _transferOwnership(_collectionOwner);
+        _setTrustedForwarder(_initialTrustedForwarder);
+    }
+
+    function NFTCollection_init(
+        address _collectionOwner,
+        string memory _initialBaseURI,
+        string memory _name,
+        string memory _symbol,
+        address payable _mintTreasury,
+        address _signAddress,
+        address _initialTrustedForwarder,
+        address _allowedToExecuteMint,
+        uint256 _maxSupply,
+        MintingDefaults memory _mintingDefaults
+    ) external {
+        __NFTCollection_init(
+            _collectionOwner,
+            _initialBaseURI,
+            _name,
+            _symbol,
+            _mintTreasury,
+            _signAddress,
+            _initialTrustedForwarder,
+            _allowedToExecuteMint,
+            _maxSupply,
+            _mintingDefaults
+        );
+    }
+
+    /**
+     * @notice ERC2771 compatible msg.sender getter
+     * @return sender msg.sender
+     */
+    function msgSender(address) external view returns (address) {
+        return super._msgSender();
+    }
+
+    /**
+     * @notice ERC2771 compatible msg.data getter
+     * @return msg.data
+     */
+    function msgData(address) external view returns (bytes calldata) {
+        return super._msgData();
+    }
+}

packages/avatar/contracts/mocks/ReenterMock.sol

@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: MIT
+// solhint-disable one-contract-per-file
+pragma solidity 0.8.26;
+
+contract ReenterMock {
+    struct MintArgs {
+        CollectionInterface target;
+        address wallet;
+        uint256 amount;
+        uint256 signatureId;
+        bytes signature;
+    }
+
+    struct AllowedArgs {
+        CollectionInterface target;
+        address minterToken;
+    }
+
+    MintArgs public mintArgs;
+    AllowedArgs public allowedArgs;
+
+    function mintReenter(
+        CollectionInterface _target,
+        address _wallet,
+        uint256 _amount,
+        uint256 _signatureId,
+        bytes calldata _signature
+    ) external {
+        mintArgs = MintArgs({target : _target, wallet : _wallet, amount : _amount, signatureId : _signatureId, signature : _signature});
+        _target.mint(_wallet, _amount, _signatureId, _signature);
+    }
+
+    function setAllowedExecuteMintReenter(CollectionInterface _target, address _minterToken) external {
+        allowedArgs = AllowedArgs({target : _target, minterToken : _minterToken});
+        _target.setAllowedExecuteMint(_minterToken);
+    }
+
+    // reenter setAllowedExecuteMint
+    function decimals() external returns (uint256) {
+        allowedArgs.target.setAllowedExecuteMint(allowedArgs.minterToken);
+        return 12;
+    }
+
+    // reenter mint
+    function transferFrom(
+        address,
+        address,
+        uint256
+    ) external {
+        mintArgs.target.mint(mintArgs.wallet, mintArgs.amount, mintArgs.signatureId, mintArgs.signature);
+    }
+
+}
+
+interface CollectionInterface {
+    function mint(
+        address _wallet,
+        uint256 _amount,
+        uint256 _signatureId,
+        bytes calldata _signature
+    ) external;
+
+    function setAllowedExecuteMint(address _minterToken) external;
+
+}

packages/avatar/contracts/nft-collection/NFTCollection.sol

@@ -514,7 +514,7 @@ IERC4906
      * @custom:event {Transfer}
      * @param wallets list of destination wallets and amounts
      */
-    function batchMint(BatchMintingData[] calldata wallets) external whenNotPaused nonReentrant onlyOwner {
+    function batchMint(BatchMintingData[] calldata wallets) external whenNotPaused onlyOwner {
         require(indexWave > 0, "NFTCollection: contract is not configured");
         uint256 len = wallets.length;
         require(len > 0, "NFTCollection: wallets length cannot be 0");
@@ -948,8 +948,7 @@ IERC4906
     override(ERC2981Upgradeable, ERC721Upgradeable)
     returns (bool)
     {
-        return ERC2981Upgradeable.supportsInterface(interfaceId)
-        || ERC721Upgradeable.supportsInterface(interfaceId);
+        return super.supportsInterface(interfaceId);
     }
 
     /**

packages/avatar/contracts/nft-collection/UpdatableOperatorFiltererUpgradeable.sol

@@ -37,14 +37,12 @@ abstract contract UpdatableOperatorFiltererUpgradeable is Initializable, Context
      * @param registry address of the registry to set
      * @param subscriptionOrRegistrant address to subscribe or copy entries from
      * @param subscribe should it subscribe
-     * @param isContract true if the current value of the registry is a contract
      */
     event ContractRegistered(
         address indexed operator,
         IOperatorFilterRegistry indexed registry,
         address indexed subscriptionOrRegistrant,
-        bool subscribe,
-        bool isContract
+        bool subscribe
     );
 
     /**
@@ -53,6 +51,18 @@ abstract contract UpdatableOperatorFiltererUpgradeable is Initializable, Context
      */
     error OperatorNotAllowed(address operator);
 
+    /**
+     * @notice the registry is not configured
+     * @param operator that does the call
+     */
+    error RegistryNotSet(address operator);
+
+    /**
+     * @notice this contract is already registered
+     * @param operator that does the call
+     */
+    error AlreadyRegistered(address operator);
+
     /**
      * @notice Used in approval operations to check if the operator is allowed to call this contract
      * @param operator The address receiving the approval
@@ -76,22 +86,6 @@ abstract contract UpdatableOperatorFiltererUpgradeable is Initializable, Context
         _;
     }
 
-    /**
-     * @notice initialization function in accordance with the upgradable pattern
-     * @param registry the address of the registry
-     * @param subscriptionOrRegistrantToCopy address to subscribe or copy entries from
-     * @param subscribe should it subscribe
-     */
-    function __UpdatableOperatorFiltererUpgradeable_init(
-        address registry,
-        address subscriptionOrRegistrantToCopy,
-        bool subscribe
-    ) internal onlyInitializing {
-        _setOperatorRegistry(registry);
-        _register(subscriptionOrRegistrantToCopy, subscribe);
-    }
-
-
     /**
      * @notice Register this contract into the registry
      * @param subscriptionOrRegistrantToCopy address to subscribe or copy entries from
@@ -100,20 +94,21 @@ abstract contract UpdatableOperatorFiltererUpgradeable is Initializable, Context
     function _register(address subscriptionOrRegistrantToCopy, bool subscribe) internal {
         IOperatorFilterRegistry registry = operatorFilterRegistry;
         bool isContract = address(registry).code.length > 0;
-        if (isContract) {
-            if (!registry.isRegistered(address(this))) {
-                if (subscribe) {
-                    registry.registerAndSubscribe(address(this), subscriptionOrRegistrantToCopy);
-                } else {
-                    if (subscriptionOrRegistrantToCopy != address(0)) {
-                        registry.registerAndCopyEntries(address(this), subscriptionOrRegistrantToCopy);
-                    } else {
-                        registry.register(address(this));
-                    }
-                }
-            }
+        if (!isContract) {
+            revert RegistryNotSet(_msgSender());
+        }
+        if (registry.isRegistered(address(this))) {
+            revert AlreadyRegistered(_msgSender());
+        }
+
+        if (subscribe) {
+            registry.registerAndSubscribe(address(this), subscriptionOrRegistrantToCopy);
+        } else if (subscriptionOrRegistrantToCopy != address(0)) {
+            registry.registerAndCopyEntries(address(this), subscriptionOrRegistrantToCopy);
+        } else {
+            registry.register(address(this));
         }
-        emit ContractRegistered(_msgSender(), registry, subscriptionOrRegistrantToCopy, subscribe, isContract);
+        emit ContractRegistered(_msgSender(), registry, subscriptionOrRegistrantToCopy, subscribe);
     }
 
     /**

packages/avatar/test/avatar/nft-collection/NFTCollection.burn.test.ts

@@ -142,4 +142,29 @@ describe('NFTCollection burn', function () {
       }
     }
   });
+
+  it('other should fail to burn a token', async function () {
+    const {
+      collectionContractAsOwner,
+      randomWallet,
+      collectionContractAsRandomWallet2,
+    } = await loadFixture(setupNFTCollectionContract);
+    await collectionContractAsOwner.setupWave(100, 10, 20);
+
+    await collectionContractAsOwner.enableBurning();
+
+    // mint 5
+    await collectionContractAsOwner.batchMint([[randomWallet, 5]]);
+
+    const transferEvents = await collectionContractAsOwner.queryFilter(
+      'Transfer'
+    );
+    for (let i = 0; i < transferEvents.length; i++) {
+      const tokenId = transferEvents[i].args.tokenId;
+      // burn
+      await expect(
+        collectionContractAsRandomWallet2.burn(tokenId)
+      ).to.revertedWith('ERC721: caller is not token owner or approved');
+    }
+  });
 });