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.