ENS contest - Bnke0x0's results

[L-01] require() should be used instead of assert():-

  1. File: 2022-07-ens/contracts/dnssec-oracle/RRUtils.sol (line 22):

      `assert(idx < self.length);`          

  2. File: 2022-07-ens/contracts/dnssec-oracle/RRUtils.sol (line 52):

      `assert(offset < self.length);`

[L-02] SPDX license identifiers missing:-

  1. File: 2022-07-ens/contracts/dnssec-oracle/BytesUtils.sol

            

  2. File: 2022-07-ens/contracts/dnssec-oracle/RRUtils.sol

      

  3. File: 2022-07-ens/contracts/dnssec-oracle/SHA1.sol



  4. File: 2022-07-ens/contracts/dnssec-oracle/Owned.sol

            

  5. File: 2022-07-ens/contracts/dnssec-oracle/algorithms/Algorithm.sol

      

  6. File: 2022-07-ens/contracts/dnssec-oracle/digests/Digest.sol



  7. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol

            

  8. File: 2022-07-ens/contracts/ethregistrar/IETHRegistrarController.sol

      

  9. File: 2022-07-ens/contracts/registry/ReverseRegistrar.sol



  10. File: 2022-07-ens/contracts/registry/IReverseRegistrar.sol

            

  11. File: 2022-07-ens/contracts/wrapper/IMetadataService.sol

      

  12. File: 2022-07-ens/contracts/registry/ENS.sol      

[L-03] Missing checks for address(0x0) when assigning values to address state variables:-

  1. File: 2022-07-ens/contracts/dnssec-oracle/Owned.sol (line 19):

      `owner = newOwner;`          

  2. File: 2022-07-ens/contracts/wrapper/Controllable.sol (line 12):

      `controllers[controller] = active;`      

[L-04] MOpen TODOS (Code architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment):-

  1. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 238):

      `// TODO: Check key isn't expired, unless updating key itself`          

  2. File: 2022-07-ens/contracts/wrapper/Controllable.sol (line 12):

      `controllers[controller] = active;`      

[L-05] abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256():-

  1. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 255):

      `bytes32 nodehash = keccak256(abi.encodePacked(ETH_NODE, label));`           

[L-06] address.call{value:x}() should be used instead of payable.transfer():-

  1. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 183-184):

      `payable(msg.sender).transfer(
            msg.value - (price.base + price.premium)`          

  2. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 204):

      `payable(msg.sender).transfer(msg.value - price.base);`                        

[L-07] Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions:-

  1. File: 22022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 17):

      `contract ETHRegistrarController is Ownable, IETHRegistrarController {`          

  2. File: 2022-07-ens/contracts/registry/ReverseRegistrar.sol (line 18):

      `contract ReverseRegistrar is Ownable, Controllable, IReverseRegistrar {`            

  3. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 14):

      `abstract contract ERC1155Fuse is ERC165, IERC1155, IERC1155MetadataURI {`          

  4. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 27-33):

      `contract NameWrapper is
  Ownable,
  ERC1155Fuse,
  INameWrapper,
  Controllable,
  IERC721Receiver
    {`  

  5. File: 2022-07-ens/contracts/wrapper/Controllable.sol (line 6):

      `contract Controllable is Ownable {`          

[N-01] Adding a return statement when the function defines a named return variable, is redundant:-

  1. File: 2022-07-ens/contracts/dnssec-oracle/BytesUtils.sol (line 247):

      `return ret;`          

  2. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 97):

      `return proof;`            

[N-02] require()/revert() statements should have descriptive reason strings:-

  1. File: 2022-07-ens/contracts/dnssec-oracle/BytesUtils.sol (line 12):

      `require(offset + len <= self.length);`          

  2. File: 2022-07-ens/contracts/dnssec-oracle/BytesUtils.sol (line 146):

      `require(idx + 2 <= self.length);`            

  3. File: 2022-07-ens/contracts/dnssec-oracle/BytesUtils.sol (line 159):

      `require(idx + 4 <= self.length);`          

  4. File: 2022-07-ens/contracts/dnssec-oracle/BytesUtils.sol (line 172):

      `require(idx + 32 <= self.length);`  

  5. File: 2022-07-ens/contracts/dnssec-oracle/BytesUtils.sol (line 185):

      `require(idx + 20 <= self.length);`

  6. File: 2022-07-ens/contracts/dnssec-oracle/BytesUtils.sol (line 199):

      `require(len <= 32);`          

  7. File: 2022-07-ens/contracts/dnssec-oracle/BytesUtils.sol (line 200):

      `require(idx + len <= self.length);`            

  8. File: 2022-07-ens/contracts/dnssec-oracle/BytesUtils.sol (line 235):

      `require(offset + len <= self.length);`          

  9. File: 2022-07-ens/contracts/dnssec-oracle/BytesUtils.sol (line 262):

      `require(len <= 52);`  

  10. File: 2022-07-ens/contracts/dnssec-oracle/BytesUtils.sol (line 268):

      `require(char >= 0x30 && char <= 0x7A);`          

  11. File: 2022-07-ens/contracts/dnssec-oracle/BytesUtils.sol (line 270):

      ` require(decoded <= 0x20)`          

  12. File: 2022-07-ens/contracts/dnssec-oracle/Owned.sol (line 10):

      `require(msg.sender == owner);`            

  13. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 57):

      `require(_maxCommitmentAge > _minCommitmentAge);`          

  14. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 121):

      `require(commitments[commitment] + maxCommitmentAge < block.timestamp);`  

  15. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 246):

      `require(duration >= MIN_REGISTRATION_DURATION);`

  16. File: 2022-07-ens/contracts/wrapper/BytesUtil.sol (line 13):

      `require(offset + len <= self.length);`          

[N-03] Use a more recent version of solidity (Use a solidity version of at least 0.8.12 to get string.concat() to be used instead of abi.encodePacked(<str>,<str>)):-

  1. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 1):

      `pragma solidity >=0.8.4;`          

  2. File: 2022-07-ens/contracts/registry/ReverseRegistrar.sol (line 1):

      `pragma solidity >=0.8.4;`            

  3. File: 2022-07-ens/contracts/wrapper/BytesUtil.sol (line 2):

      `pragma solidity >=0.8.4;`          

  4. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol(line 2):

      `pragma solidity >=0.8.4;`  

[N-04] Event is missing indexed fields (Each event should use three indexed fields if there are three or more fields):-

  1. File: 2022-07-ens/contracts/dnssec-oracle/DNSSEC.sol (line 14-15):

      ` event AlgorithmUpdated(uint8 id, address addr);
event DigestUpdated(uint8 id, address addr);`          

  2. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 34-47):

      `event NameRegistered(
    string name,
    bytes32 indexed label,
    address indexed owner,
    uint256 baseCost,
    uint256 premium,
    uint256 expires
);
event NameRenewed(
    string name,
    bytes32 indexed label,
    uint256 cost,
    uint256 expires
);`            

  3. File: 2022-07-ens/contracts/ethregistrar/IBaseRegistrar.sol (line 9-19):

      `event NameMigrated(
    uint256 indexed id,
    address indexed owner,
    uint256 expires
);
event NameRegistered(
    uint256 indexed id,
    address indexed owner,
    uint256 expires
);
event NameRenewed(uint256 indexed id, uint256 expires);`          

  4. File: 2022-07-ens/contracts/wrapper/INameWrapper.sol(line 19-29):

      `event NameWrapped(
    bytes32 indexed node,
    bytes name,
    address owner,
    uint32 fuses,
    uint64 expiry
);

event NameUnwrapped(bytes32 indexed node, address owner);

event FusesSet(bytes32 indexed node, uint32 fuses, uint64 expiry);`            

  3. File: 2022-07-ens/contracts/registry/ENS.sol (line 5):

      `event NewOwner(bytes32 indexed node, bytes32 indexed label, address owner);`          

  4. File: 2022-07-ens/contracts/registry/ENS.sol (line 17-21):

      `event ApprovalForAll(
    address indexed owner,
    address indexed operator,
    bool approved
);`

[N-05] Use a more recent version of solidity (Use a solidity version of at least 0.8.13 to get the ability to use using for with a list of free functions):-

  1. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 2):

      `pragma solidity >=0.8.4;`          

  2. File: 2022-07-ens/contracts/dnssec-oracle/RRUtils.sol (line 1):

      `pragma solidity >=0.8.4;`            

  3. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 1):

      `pragma solidity >=0.8.4;`          

  4. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 2):

      `pragma solidity >=0.8.4;`     

  5. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 2):

      `pragma solidity >=0.8.4;`                     

[N-06] public functions not called by the contract should be declared external instead:-

  1. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 58-61):

      `function setAlgorithm(uint8 id, Algorithm algo) public owner_only {
    algorithms[id] = algo;
    emit AlgorithmUpdated(id, address(algo));
}`          

  2. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 69-72):

      ` function setDigest(uint8 id, Digest digest) public owner_only {
    digests[id] = digest;
    emit DigestUpdated(id, address(digest));
}`            

  3. File: 2022-07-ens/contracts/dnssec-oracle/Owned.sol (line 18-20):

      `function setOwner(address newOwner) public owner_only {
    owner = newOwner;
}`          

  4. File: 2022-07-ens/contracts/registry/ReverseRegistrar.sol (line 51-57):

      `function setDefaultResolver(address resolver) public override onlyOwner {
    require(
        address(resolver) != address(0),
        "ReverseRegistrar: Resolver address must not be 0"
    );
    defaultResolver = NameResolver(resolver);
}`     

  5. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 105-110):

      `function setMetadataService(IMetadataService _newMetadataService)
    public
    onlyOwner
{
    metadataService = _newMetadataService;
}`                               

  6. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 128-131):

      `function setUpgradeContract(INameWrapperUpgrade _upgradeAddress)
    public
    onlyOwner
{`          

  7. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 335-339):

      `function unwrapETH2LD(
    bytes32 labelhash,
    address newRegistrant,
    address newController
) public override onlyTokenOwner(_makeNode(ETH_NODE, labelhash)) {`            

  8. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 356-360):

      `function unwrap(
    bytes32 parentNode,
    bytes32 labelhash,
    address newController
) public override onlyTokenOwner(_makeNode(parentNode, labelhash)) {`          

  9. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 373-378):

      `function setFuses(bytes32 node, uint32 fuses)
    public
    onlyTokenOwner(node)
    operationAllowed(node, CANNOT_BURN_FUSES)
    returns (uint32)
{`     

  10. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 504-515):

      `function setSubnodeOwner(
    bytes32 parentNode,
    string calldata label,
    address newOwner,
    uint32 fuses,
    uint64 expiry
)
    public
    onlyTokenOwner(parentNode)
    canCallSetSubnodeOwner(parentNode, keccak256(bytes(label)))
    returns (bytes32 node)
{`       

  11. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 584-631):

      `    function setRecord(
    bytes32 node,
    address owner,
    address resolver,
    uint64 ttl
)
    public
    override
    onlyTokenOwner(node)
    operationAllowed(
        node,
        CANNOT_TRANSFER | CANNOT_SET_RESOLVER | CANNOT_SET_TTL
    )
{
    ens.setRecord(node, address(this), resolver, ttl);
    (address oldOwner, , ) = getData(uint256(node));
    _transfer(oldOwner, owner, uint256(node), 1, "");
}

/**
 * @notice Sets resolver contract in the registry
 * @param node namehash of the name
 * @param resolver the resolver contract
 */

function setResolver(bytes32 node, address resolver)
    public
    override
    onlyTokenOwner(node)
    operationAllowed(node, CANNOT_SET_RESOLVER)
{
    ens.setResolver(node, resolver);
}

/**
 * @notice Sets TTL in the registry
 * @param node namehash of the name
 * @param ttl TTL in the registry
 */

function setTTL(bytes32 node, uint64 ttl)
    public
    override
    onlyTokenOwner(node)
    operationAllowed(node, CANNOT_SET_TTL)
{
    ens.setTTL(node, ttl);
}`     

[N-07] Unused file:-

  1. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 1):

      `// SPDX-License-Identifier: MIT`          

  2. File: 2022-07-ens/contracts/dnssec-oracle/DNSSEC.sol (line 1):

      `// SPDX-License-Identifier: MIT`            

  3. File: 2022-07-ens/contracts/wrapper/BytesUtil.sol (line 1):

      `// SPDX-License-Identifier: MIT`          

  4. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 1):

      `// SPDX-License-Identifier: MIT`     

  5. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 1):

      `// SPDX-License-Identifier: MIT`     

  6. File: 2022-07-ens/contracts/wrapper/INameWrapper.sol (line 1):

      `// SPDX-License-Identifier: MIT`          

  7. File: 2022-07-ens/contracts/wrapper/Controllable.sol (line 1):

      `// SPDX-License-Identifier: MIT`            

  8. File: 2022-07-ens/contracts/wrapper/INameWrapperUpgrade.sol (line 1):

      `// SPDX-License-Identifier: MIT`          

  9. File: 2022-07-ens/contracts/resolvers/Resolver.sol (line 1):          

[G-01] x = x + y is cheaper than x += y:-

  1. File: 2022-07-ens/contracts/dnssec-oracle/RRUtils.sol (line 250):

      `counts -= 1;`          

[G-02] <array>.length should not be looked up in every loop of a for-loop:-

  1. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 93):

      `for(uint i = 0; i < input.length; i++) {`          

  2. File: 2022-07-ens/contracts/dnssec-oracle/RRUtils.sol (line 310):

      `for(uint i = 0; i < data.length + 31; i += 32) {`

  3. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 256):

      `for (uint256 i = 0; i < data.length; i++) {`          

  4. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 92):

      `for (uint256 i = 0; i < accounts.length; ++i) {`          

  5. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 205):

      `for (uint256 i = 0; i < ids.length; ++i) {`          

[G-03] ++i costs less gas than i++, especially when it’s used in for-loops (--i/i-- too):-

  1. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 93):

      `for(uint i = 0; i < input.length; i++) {`          

  2. File: 2022-07-ens/contracts/dnssec-oracle/RRUtils.sol (line 310):

      `for(uint i = 0; i < data.length + 31; i += 32) {`

  3. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 256):

      `for (uint256 i = 0; i < data.length; i++) {`          

  4. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 92):

      `for (uint256 i = 0; i < accounts.length; ++i) {`          

  5. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 205):

      `for (uint256 i = 0; i < ids.length; ++i) {`          

[G-04] require()/revert() strings longer than 32 bytes cost extra gas:-

  1. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 327):

      `revert("ERC1155: transfer to non ERC1155Receiver implementer");`          

  2. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 354):

      `revert("ERC1155: ERC1155Receiver rejected tokens");`

  3. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 359):

      `revert("ERC1155: transfer to non ERC1155Receiver implementer");`          

  4. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 99-102):

      `require(
            resolver != address(0),
            "ETHRegistrarController: resolver is required when data is supplied"
        );`          

  5. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 137-140):

      `require(
        msg.value >= (price.base + price.premium),
        "ETHRegistrarController: Not enough ether provided"
    );`          

  6. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 196-199):

      `require(
        msg.value >= price.base,
        "ETHController: Not enough Ether provided for renewal"
    );`          

  7. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 232-242):

      `require(
        commitments[commitment] + minCommitmentAge <= block.timestamp,
        "ETHRegistrarController: Commitment is not valid"
    );

    // If the commitment is too old, or the name is registered, stop
    require(
        commitments[commitment] + maxCommitmentAge > block.timestamp,
        "ETHRegistrarController: Commitment has expired"
    );
    require(available(name), "ETHRegistrarController: Name is unavailable");`

  8. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 259-262):

      `require(
            txNamehash == nodehash,
            "ETHRegistrarController: Namehash on record do not match the name being registered"
        );`        

  9. File: 2022-07-ens/contracts/registry/ReverseRegistrar.sol (line 41-47):

      `require(
        addr == msg.sender ||
            controllers[msg.sender] ||
            ens.isApprovedForAll(addr, msg.sender) ||
            ownsContract(addr),
        "ReverseRegistrar: Caller is not a controller or authorised by address or the address itself"
    );`          

  10. File: 2022-07-ens/contracts/registry/ReverseRegistrar.sol (line 52-55):

      `require(
        address(resolver) != address(0),
        "ReverseRegistrar: Resolver address must not be 0"
    );`

  11. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 60-62):

      `require(
        account != address(0),
        "ERC1155: balance query for the zero address"
    );`          

  12. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 85-88):

      `require(
        accounts.length == ids.length,
        "ERC1155: accounts and ids length mismatch"
    );`

  13. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 176-180):

      `require(to != address(0), "ERC1155: transfer to the zero address");
    require(
        from == msg.sender || isApprovedForAll(from, msg.sender),
        "ERC1155: caller is not owner nor approved"
    );`          

  14. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 195-203):

      `require(
        ids.length == amounts.length,
        "ERC1155: ids and amounts length mismatch"
    );
    require(to != address(0), "ERC1155: transfer to the zero address");
    require(
        from == msg.sender || isApprovedForAll(from, msg.sender),
        "ERC1155: transfer caller is not owner nor approved"
    );`          

  15. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 215-218):

      `require(
            amount == 1 && oldOwner == from,
            "ERC1155: insufficient balance for transfer"
        );`          

  16. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 248-253):

      `require(owner == address(0), "ERC1155: mint of existing token");
    require(newOwner != address(0), "ERC1155: mint to the zero address");
    require(
        newOwner != address(this),
        "ERC1155: newOwner cannot be the NameWrapper contract"
    );`          

  17. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 290-293):

      `require(
        amount == 1 && oldOwner == from,
        "ERC1155: insufficient balance for transfer"
    );`

  18. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 17):

      `2022-07-ens/contracts/wrapper/Controllable.sol`        

[G-05] It costs more gas to initialize variables to zero than to let the default of zero be applied:-

  1. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 93):

      `for(uint i = 0; i < input.length; i++) {`          

  2. File: 2022-07-ens/contracts/dnssec-oracle/RRUtils.sol (line 310):

      `for(uint i = 0; i < data.length + 31; i += 32) {`

  3. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 256):

      `for (uint256 i = 0; i < data.length; i++) {`          

  4. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 92):

      `for (uint256 i = 0; i < accounts.length; ++i) {`          

  5. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 205):

      `for (uint256 i = 0; i < ids.length; ++i) {`  

[G-06] && operator can use more gas:-

  1. File: 2022-07-ens/contracts/dnssec-oracle/BytesUtils.sol (line 268):

      `require(char >= 0x30 && char <= 0x7A);`          

  2. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 215-218):

      `require(
            amount == 1 && oldOwner == from,
            "ERC1155: insufficient balance for transfer"
        );`

  

[G-07] Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead:-

  1. File: 2022-07-ens/contracts/dnssec-oracle/BytesUtils.sol (line 265):

      `uint8 decoded;`          

  2. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 21-24):

      `uint16 constant DNSCLASS_IN = 1;

       uint16 constant DNSTYPE_DS = 43;
       uint16 constant DNSTYPE_DNSKEY = 48;`

  3. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 28-35):

      `error InvalidLabelCount(bytes name, uint labelsExpected);
error SignatureNotValidYet(uint32 inception, uint32 now);
error SignatureExpired(uint32 expiration, uint32 now);
error InvalidClass(uint16 class);
error InvalidRRSet();
error SignatureTypeMismatch(uint16 rrsetType, uint16 sigType);
error InvalidSignerName(bytes rrsetName, bytes signerName);
error InvalidProofType(uint16 proofType);`          

  4. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 39-40):

      `mapping (uint8 => Algorithm) public algorithms;
mapping (uint8 => Digest) public digests;`          

  5. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 58):

      `function setAlgorithm(uint8 id, Algorithm algo) public owner_only {` 

  6. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 69):

      `function setDigest(uint8 id, Digest digest) public owner_only {`

  7. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 126-127):

      `if(!RRUtils.serialNumberGte(rrset.expiration, uint32(now))) {
        revert SignatureExpired(rrset.expiration, uint32(now));`          

  8. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 132-133):

      `if(!RRUtils.serialNumberGte(uint32(now), rrset.inception)) {
        revert SignatureNotValidYet(rrset.inception, uint32(now));`          

  9. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 147):

      `function validateRRs(RRUtils.SignedSet memory rrset, uint16 typecovered) internal pure returns (bytes memory name) {`             

  10. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 256):

      `uint16 computedkeytag = keyrdata.computeKeytag();`

  11. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 302):

      `iuint16 keytag = keyrdata.computeKeytag();`          

  12. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 335):

      `function verifyDSHash(uint8 digesttype, bytes memory data, bytes memory digest) internal view returns (bool) {`          

  13. File: 2022-07-ens/contracts/dnssec-oracle/RRUtils.sol (line 73-79):

      `        uint16 typeCovered;
    uint8 algorithm;
    uint8 labels;
    uint32 ttl;
    uint32 expiration;
    uint32 inception;
    uint16 keytag;`          

  14. File: 2022-07-ens/contracts/dnssec-oracle/RRUtils.sol (line 107-109):

      `uint16 dnstype;
    uint16 class;
    uint32 ttl;`

  15. File: 2022-07-ens/contracts/dnssec-oracle/RRUtils.sol (line 187-189):

      `uint16 flags;
    uint8 protocol;
    uint8 algorithm;`          

  16. File: 2022-07-ens/contracts/dnssec-oracle/RRUtils.sol (line 206-208):

      `uint16 keytag;
    uint8 algorithm;
    uint8 digestType;`          

  17. File: 2022-07-ens/contracts/dnssec-oracle/RRUtils.sol (line 266):

      `function serialNumberGte(uint32 i1, uint32 i2) internal pure returns(bool) ` 

  18. File: 2022-07-ens/contracts/dnssec-oracle/RRUtils.sol (line 234):

      `return uint16(ac1);`

  19. File: 2022-07-ens/contracts/dnssec-oracle/DNSSEC.sol (line 14-15):

      `event AlgorithmUpdated(uint8 id, address addr);
event DigestUpdated(uint8 id, address addr);`

  20. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 94-95):

      ` uint32 fuses,
    uint64 wrapperExpiry`          

  21. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 133-134):

      `uint32 fuses,
    uint64 wrapperExpiry`

  22. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 20-21):

      `uint32,
    uint64`          

  23. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 34-35):

      `uint32,
    uint64`        

  23. File: 2022-07-ens/contracts/wrapper/BytesUtil.sol (line 43):

      `uint len = uint(uint8(self[idx]));`                

  24. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 238):

      `function _canTransfer(uint32 fuses) internal virtual returns (bool);`          

  25. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 281):

      `(address oldOwner, uint32 fuses, uint64 expiry) = getData(id);`

                    

[G-08] Use a more recent version of solidity:-

  1. File: 2022-07-ens/contracts/dnssec-oracle/BytesUtils.sol (line 1):

      `pragma solidity ^0.8.4;`          

  2. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 2):

      `pragma solidity ^0.8.4;`

  3. File: 2022-07-ens/contracts/dnssec-oracle/RRUtils.sol (line 1):

      `pragma solidity ^0.8.4;`          

  4. File: 2022-07-ens/contracts/dnssec-oracle/SHA1.sol (line 1):

      `pragma solidity ^0.8.4;`          

  5. File: 2022-07-ens/contracts/dnssec-oracle/Owned.sol (line 1):

      `pragma solidity ^0.8.4;` 

  6. File: 2022-07-ens/contracts/dnssec-oracle/DNSSEC.sol (line 2):

      `pragma solidity ^0.8.4;`

  7. File: 2022-07-ens/contracts/dnssec-oracle/algorithms/Algorithm.sol (line 1):

      `pragma solidity ^0.8.4;`          

  8. File: 2022-07-ens/contracts/dnssec-oracle/digests/Digest.sol (line 1):

      `pragma solidity ^0.8.4;`          

  9. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol(line 1):

      `pragma solidity ^0.8.4;`             

  10. File: 2022-07-ens/contracts/ethregistrar/IETHRegistrarController.sol (line 1):

      `pragma solidity ^0.8.4;`

  11. File: 2022-07-ens/contracts/ethregistrar/StringUtils.sol (line 1):

      `pragma solidity ^0.8.4;`          

  12. File: 2022-07-ens/contracts/registry/ReverseRegistrar.sol (line 1):

      `pragma solidity ^0.8.4;`          

  13. File: 22022-07-ens/contracts/registry/IReverseRegistrar.sol (line 1):

      `pragma solidity ^0.8.4;`          

  14. File: 2022-07-ens/contracts/wrapper/BytesUtil.sol (line 2):

      `pragma solidity ^0.8.4;`

  15. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 2):

      `pragma solidity ^0.8.4;`          

  16. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 2):

      `pragma solidity ^0.8.4;`          

  17. File: 2022-07-ens/contracts/wrapper/INameWrapper.sol (line 2):

      `pragma solidity ^0.8.4;` 

  18. File: 2022-07-ens/contracts/wrapper/Controllable.sol (line 2):

      `pragma solidity ^0.8.4;`

  19. File: 2022-07-ens/contracts/wrapper/IMetadataService.sol (line 1):

      `pragma solidity ^0.8.4;`

  20. File: 2022-07-ens/contracts/wrapper/INameWrapperUpgrade.sol (line 2):

      `pragma solidity ^0.8.13;`          

  21. File: 2022-07-ens/contracts/registry/ENS.sol (line 1):

      `pragma solidity ^0.8.4;`

  22. File: 2022-07-ens/contracts/resolvers/Resolver.sol (line 2):

      `pragma solidity ^0.8.4;`               

[G-09] Using private rather than public for constants, saves gas:-

  1. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 22-23):

      `bytes32 private constant ETH_NODE =
    0x93cdeb708b7545dc668eb9280176169d1c33cfd8ed6f04690a0bcc88a93fc4ae;`          

[G-10] Functions guaranteed to revert when called by normal users can be marked payable:-

1. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 58-61):

   function setAlgorithm(uint8 id, Algorithm algo) public owner_only { algorithms[id] = algo; emit AlgorithmUpdated(id, address(algo)); }

2. File: 2022-07-ens/contracts/dnssec-oracle/DNSSECImpl.sol (line 69-72):

   function setDigest(uint8 id, Digest digest) public owner_only { digests[id] = digest; emit DigestUpdated(id, address(digest)); }

3. File: 2022-07-ens/contracts/dnssec-oracle/Owned.sol (line 18-20):

   function setOwner(address newOwner) public owner_only { owner = newOwner; }

4. File: 2022-07-ens/contracts/registry/ReverseRegistrar.sol (line 51-57):

   function setDefaultResolver(address resolver) public override onlyOwner { require( address(resolver) != address(0), "ReverseRegistrar: Resolver address must not be 0" ); defaultResolver = NameResolver(resolver); }

5. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 105-110):

   function setMetadataService(IMetadataService _newMetadataService) public onlyOwner { metadataService = _newMetadataService; }

6. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 128-131):

   function setUpgradeContract(INameWrapperUpgrade _upgradeAddress) public onlyOwner {

7. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 335-339):

   function unwrapETH2LD( bytes32 labelhash, address newRegistrant, address newController ) public override onlyTokenOwner(_makeNode(ETH_NODE, labelhash)) {

8. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 356-360):

   function unwrap( bytes32 parentNode, bytes32 labelhash, address newController ) public override onlyTokenOwner(_makeNode(parentNode, labelhash)) {

9. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 373-378):

   function setFuses(bytes32 node, uint32 fuses) public onlyTokenOwner(node) operationAllowed(node, CANNOT_BURN_FUSES) returns (uint32) {

10. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 504-515):

    function setSubnodeOwner( bytes32 parentNode, string calldata label, address newOwner, uint32 fuses, uint64 expiry ) public onlyTokenOwner(parentNode) canCallSetSubnodeOwner(parentNode, keccak256(bytes(label))) returns (bytes32 node) {

11. File: 2022-07-ens/contracts/wrapper/NameWrapper.sol (line 584-631):

    ` function setRecord( bytes32 node, address owner, address resolver, uint64 ttl ) public override onlyTokenOwner(node) operationAllowed( node, CANNOT_TRANSFER | CANNOT_SET_RESOLVER | CANNOT_SET_TTL ) { ens.setRecord(node, address(this), resolver, ttl); (address oldOwner, , ) = getData(uint256(node)); _transfer(oldOwner, owner, uint256(node), 1, ""); }

/**

• @notice Sets resolver contract in the registry
• @param node namehash of the name
• @param resolver the resolver contract */

function setResolver(bytes32 node, address resolver) public override onlyTokenOwner(node) operationAllowed(node, CANNOT_SET_RESOLVER) { ens.setResolver(node, resolver); }

/**

• @notice Sets TTL in the registry
• @param node namehash of the name
• @param ttl TTL in the registry */

function setTTL(bytes32 node, uint64 ttl) public override onlyTokenOwner(node) operationAllowed(node, CANNOT_SET_TTL) { ens.setTTL(node, ttl); }`

[G-11] Don’t compare boolean expressions to boolean literals ( if (<x> == true) => if (<x>), if (<x> == false) => if (!<x>)):-

  1. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 259-262):

      `require(
            txNamehash == nodehash,
            "ETHRegistrarController: Namehash on record do not match the name being registered"
        );`        

  2. File: 2022-07-ens/contracts/registry/ReverseRegistrar.sol (line 41-47):

      `require(
        addr == msg.sender ||
            controllers[msg.sender] ||
            ens.isApprovedForAll(addr, msg.sender) ||
            ownsContract(addr),
        "ReverseRegistrar: Caller is not a controller or authorised by address or the address itself"
    );`          
      

  3. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 85-88):

      `require(
        accounts.length == ids.length,
        "ERC1155: accounts and ids length mismatch"
    );`

  4. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 176-180):

      `require(to != address(0), "ERC1155: transfer to the zero address");
    require(
        from == msg.sender || isApprovedForAll(from, msg.sender),
        "ERC1155: caller is not owner nor approved"
    );`          

  5. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 195-203):

      `require(
        ids.length == amounts.length,
        "ERC1155: ids and amounts length mismatch"
    );
    require(
        from == msg.sender || isApprovedForAll(from, msg.sender),
        "ERC1155: transfer caller is not owner nor approved"
    );`          

  6. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 215-218):

      `require(
            amount == 1 && oldOwner == from,
            "ERC1155: insufficient balance for transfer"
        );`          

  7. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 248-253):

      `require(owner == address(0), "ERC1155: mint of existing token");`          

  8. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 290-293):

      `require(
        amount == 1 && oldOwner == from,
        "ERC1155: insufficient balance for transfer"
    );`         

[G-12] Use custom errors rather than revert()/require() strings to save deployment gas:-

  1. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 327):

      `revert("ERC1155: transfer to non ERC1155Receiver implementer");`          

  2. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 354):

      `revert("ERC1155: ERC1155Receiver rejected tokens");`

  3. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 359):

      `revert("ERC1155: transfer to non ERC1155Receiver implementer");`          

  4. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 99-102):

      `require(
            resolver != address(0),
            "ETHRegistrarController: resolver is required when data is supplied"
        );`          

  5. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 137-140):

      `require(
        msg.value >= (price.base + price.premium),
        "ETHRegistrarController: Not enough ether provided"
    );`          

  6. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 196-199):

      `require(
        msg.value >= price.base,
        "ETHController: Not enough Ether provided for renewal"
    );`          

  7. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 232-242):

      `require(
        commitments[commitment] + minCommitmentAge <= block.timestamp,
        "ETHRegistrarController: Commitment is not valid"
    );

    // If the commitment is too old, or the name is registered, stop
    require(
        commitments[commitment] + maxCommitmentAge > block.timestamp,
        "ETHRegistrarController: Commitment has expired"
    );
    require(available(name), "ETHRegistrarController: Name is unavailable");`

  8. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 259-262):

      `require(
            txNamehash == nodehash,
            "ETHRegistrarController: Namehash on record do not match the name being registered"
        );`        

  9. File: 2022-07-ens/contracts/registry/ReverseRegistrar.sol (line 41-47):

      `require(
        addr == msg.sender ||
            controllers[msg.sender] ||
            ens.isApprovedForAll(addr, msg.sender) ||
            ownsContract(addr),
        "ReverseRegistrar: Caller is not a controller or authorised by address or the address itself"
    );`          

  10. File: 2022-07-ens/contracts/registry/ReverseRegistrar.sol (line 52-55):

      `require(
        address(resolver) != address(0),
        "ReverseRegistrar: Resolver address must not be 0"
    );`

  11. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 60-62):

      `require(
        account != address(0),
        "ERC1155: balance query for the zero address"
    );`          

  12. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 85-88):

      `require(
        accounts.length == ids.length,
        "ERC1155: accounts and ids length mismatch"
    );`

  13. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 176-180):

      `require(to != address(0), "ERC1155: transfer to the zero address");
    require(
        from == msg.sender || isApprovedForAll(from, msg.sender),
        "ERC1155: caller is not owner nor approved"
    );`          

  14. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 195-203):

      `require(
        ids.length == amounts.length,
        "ERC1155: ids and amounts length mismatch"
    );
    require(to != address(0), "ERC1155: transfer to the zero address");
    require(
        from == msg.sender || isApprovedForAll(from, msg.sender),
        "ERC1155: transfer caller is not owner nor approved"
    );`          

  15. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 215-218):

      `require(
            amount == 1 && oldOwner == from,
            "ERC1155: insufficient balance for transfer"
        );`          

  16. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 248-253):

      `require(owner == address(0), "ERC1155: mint of existing token");
    require(newOwner != address(0), "ERC1155: mint to the zero address");
    require(
        newOwner != address(this),
        "ERC1155: newOwner cannot be the NameWrapper contract"
    );`          

  17. File: 2022-07-ens/contracts/wrapper/ERC1155Fuse.sol (line 290-293):

      `require(
        amount == 1 && oldOwner == from,
        "ERC1155: insufficient balance for transfer"
    );`

  18. File: 2022-07-ens/contracts/ethregistrar/ETHRegistrarController.sol (line 17):

      `2022-07-ens/contracts/wrapper/Controllable.sol`