Poor logic in domainSeparator#

Informational

In Minting.sol there is _domainSeparatorV4 function used to get the domain separator, needed for signature verification. On line 71 there is a check that ds.contractInfo.canvasAddress equals ds._CACHED_THIS and block.chainid equals ds._CACHED_CHAIN_ID. If it is true, _CACHED_DOMAIN_SEPARATOR is returned. However, the if statement will always be true because ds._CACHED_THIS and ds._CACHED_CHAIN_ID are set to exactly those values in the initialize function and are never changed. In case that the result would somehow be false, the other line of code that would be executed would not work either, because it needs ds._TYPE_HASH, ds._HASHED_NAME and ds._HASHED_VERSION, which are only set in the initialize function as well. So you are counting on the contract being initialized in both cases. This means that if one option works, the other does as well, and if one does not work, the other does not either. We can see the constructor in Canvas.sol executes the initialize function, so we can be sure that all the needed values are set. Therefore, _CACHED_DOMAIN_SEPARATOR can be returned and there is no need for the if statement.

function initialize(string memory name, string memory version) external {
    Schema.Storage storage ds = canvasStorage();

    bytes32 hashedName = keccak256(bytes(name));
    bytes32 hashedVersion = keccak256(bytes(version));
    bytes32 typeHash = keccak256(
        "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"
    );
    ds._HASHED_NAME = hashedName;
    ds._HASHED_VERSION = hashedVersion;
    ds._CACHED_CHAIN_ID = block.chainid;
    ds._CACHED_DOMAIN_SEPARATOR = _buildDomainSeparator(typeHash, hashedName, hashedVersion);
    ds._CACHED_THIS = ds.contractInfo.canvasAddress;
    ds._TYPE_HASH = typeHash;
}

function _domainSeparatorV4() internal view returns (bytes32) {
    Schema.Storage storage ds = canvasStorage();

    if (ds.contractInfo.canvasAddress == ds._CACHED_THIS && block.chainid == ds._CACHED_CHAIN_ID) { 
      return ds._CACHED_DOMAIN_SEPARATOR;
    } else {
      return _buildDomainSeparator(ds._TYPE_HASH, ds._HASHED_NAME, ds._HASHED_VERSION);
    }
}

Recommendation#

Consider removing the if statement in _domainSeparatorV4 and just return ds._CACHED_DOMAIN_SEPARATOR. Only store the _CACHED_DOMAIN_SEPARATOR variable and remove other variables, because they are not needed.

function initialize(string memory name, string memory version) external {
    Schema.Storage storage ds = canvasStorage();
    ds._CACHED_DOMAIN_SEPARATOR = _buildDomainSeparator(
        keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"),
        keccak256(bytes(name)),
        keccak256(bytes(version))
        );
}

function _domainSeparatorV4() internal view returns (bytes32) {
    return canvasStorage()._CACHED_DOMAIN_SEPARATOR;
}