Total supply exceeds max supply

Critical TC – 01 |  Resolved

There are requirements _tokenIdCounter.current() <= maxSupply in safeMint(), lazyMint() functions. It makes possible to mint one extra token after reaching max supply.

Steps to reproduce: deploy contract with max supply value = 5, mint 5 tokens with any minting function, mint one extra token with safeMint() or lazyMint().

Recommendation:

Change requirements from _tokenIdCounter.current() <= maxSupply to _tokenIdCounter.current() < maxSupply in safeMint() and lazyMint() functions.

Re-Audit:

There is no need to change requirement in the safeMintBatch() function. It causes a new issue when max supply will not be reached with the safeMintBatch() function. Requirement changes should be reverted into:

require(
            _tokenIdCounter.current() + tokenIds.length <= maxSupply,
            "Batch mint would exceed max supply"
        );

‍

‍

Transaction failed with setTokenTransferabilityRange()

Medium TM – 01 |  Resolved

If some tokens from range are burned or just do not exist transaction will be failed for all tokens from range. To prevent transaction failure, nonexistent tokens may be skipped.

Recommendation:

Consider replace requirement into branching:

require(_exists(tokenId), "Token does not exist");
            transferableTokens[tokenId] = transferable;
            emit TokenTransferabilityUpdated(tokenId, transferable);

into:

		if(_exists(tokenId)) {
       transferableTokens[tokenId] = transferable;
       emit TokenTransferabilityUpdated(tokenId, transferable);
            }

The same can be used for the array in the setTokenTransferabilities() function.

Invalid checking of expiry date

Medium TM – 02 |  Resolved

There is incorrect checking of expiry parameter in the lazyMint() function. If there is value less than block.timestamp, it will work but it shouldn't.

Expiry date should be bigger than current timestamp: block.timestamp < expiry. Expiry is the date when the signature is considered invalid.

‍

Recommendation:

Change requirement:

	require(expiry <= block.timestamp + WEEK, "Signature has expired");

into:

	require(expiry > block.timestamp && expiry <= block.timestamp + WEEK, "Signature has expired");

or:

	require(expiry > block.timestamp && expiry <= block.timestamp + 1 weeks, "Signature has expired");

and remove the WEEK variable.

The batchLazyMint() can be called only by owner

Medium TM – 03 |  Resolved

There is a lazyMint() function that can be called by anyone with a signature but the batchLazyMint() function can be called only by the owner. I suppose it also should be called by anyone, at least it is secured with a signature.

Recommendation:

Consider removing the onlyOwner modifier from the batchLazyMint() function.

Floating pragma ^0.8.2

Low ML – 01 |  Resolved

The current version of solc in contract MetaXSeed is ^0.8.2 and it is better to lock the pragma to a specific version.

Contracts should be deployed with the same compiler version and flag that they 
have been tested thoroughly. Locking the pragma helps to ensure that contracts 
do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

Recommendation:

Consider locking pragma to a specific version.

Visibility of function

Low ML – 02 |  Resolved

The functions addSigner(), setGlobalTransferability(), safeMint(), safeMintBatch(), batchLazyMint(), burn(), setTokenURI(), setTokenTransferabilityRange(), setTokenTransferabilityRange(), totalSupply(), tokensOfOwner() of MetaXSeed contract can be marked as external, it will save gas during function execution.

The lazyMint() function may also be marked as external but it is called inside another batchLazyMint() function of contract. There could be a good practice to create an internal _lazyMint() function and call it inside external lazyMint() and batchLazyMint() functions with passed parameters from these functions.

Recommendation:

Consider changing visibility to external if possible, it will bring saving gas during function execution.

Unnecessary import of ECDSA.sol

Low ML – 03 |  Resolved

MetaXSeed.sol imports ECDSA.sol from the openzeppelin library but it is already imported from another imported file - draft-EIP712.sol.

Recommendation:

Consider deleting unused import, it will save gas during deployment.

Unnecessary signature requirement

Low TL – 01 |  Resolved

There is unimportant require(signer != address(0), "Invalid signature") in the lazyMint() function. It could be missed because zero signer (address(0)) cannot be added to the contract: there is the same requirement in the addSigner() function.

Recommendation:

Consider removing the require(signer != address(0), "Invalid signature").

Order of layout

Informational  MI – 01 |  Resolved

The layout contract elements in MetaXSeed are not fully logically grouped.

The contract elements should be grouped and ordered in the following way:

• Pragma statements;
• Import statements;
• Interfaces;
• Libraries;
• Contract.

Inside each contract, library or interface, use the following order:

• Library declarations (using directive);
• Type declarations;
• State variables;
• Events;
• Modifiers;
• Functions.

Ordering helps readers to navigate the code and find the elements more quickly.

Recommendation:

Consider changing the order of layout according to solidity documentation: Order of Layout.

Order of functions

Informational  MI – 02 |  Resolved

The functions in contract MetaXSeed are not grouped according to their visibility and order.

Functions should be grouped according to their visibility and ordered in the following way:

• constructor;
• receive function (if exists);
• fallback function (if exists);
• external;
• public;
• internal;
• private.

Ordering helps readers navigate across the code, identify which functions they can call and find the constructor and fallback definitions easier.

Recommendation:

Consider changing functions order according to solidity documentation: Order of Functions.

Wrong naming convention

Informational  MI – 03 |  Resolved

According to the Solidity Style Guide, internal functions and variables should begin from the underscore. In the MetaXSeed contract there are: tokenIdCounter, SIGNATURE_VERSION, SIGNING_DOMAIN, LAZY_MINT_TYPEHASH, WEEK, which didn’t fit the naming convention.

Recommendation:

Consider following the Solidity Style Guide, and naming conventions.

Re-Audit:

Changes were missed for tokenIdCounter.

Function parameters orders mismatched

Informational  TI – 01 |  Resolved

There are mismatched parameters order in safeMint() and safeMintBatch() functions. It will be more comfortable if functions have logically similar parameters order.

Recommendation:

Change safeMint() function parameters order into:

	function safeMint(
        address to,
        uint256 tokenId,
        string memory tokenURI,
        bool transferable
    )