Unnecessary complexity in _tokenRestrictions structure
Description
The MightyNetERC721RestrictedRegistry contract tracks restricted tokens by hashing a token's tokenContract address and tokenId value together, resulting in a tokenHash that is then stored in the contract.
For instance, in the isRestricted function:
bytes32 tokenHash = keccak256(abi.encodePacked(tokenContract, tokenId));Then, tokenHash is used as a key for the _tokenRestrictions mapping to account for restricted tokens.
mapping(bytes32 => address) private _tokenRestrictions;Together, they are used in the form _tokenRestrictions[tokenHash] multiple times in the contract.
Impact
This adds unnecessary complexity in terms of maintainability and readability of the contract code. Additionally, the current implementation consumes slightly more gas than is needed.
Recommendations
We recommend using a traditional nested mapping in order to improve the maintainability, readability, and gas efficiency of the contract:
mapping(address => mapping(uint256 => address)) private _tokenRestrictions;Then, the state of a given token can be accessed with _tokenRestrictions[tokenContract][tokenId].
Remediation
might Bear Games acknowledges this. They have replaced their previously implemented _tokenRestrictions structure with a more efficient one as per our recommendation.
The following is theire provided response:
We have made this change to MightyNetERC721RestrictedRegistryV2.