ENS contest - MiloTruck's results

• Low Issues
  • Use of assert() statements
• Non-Critical Issues
  • Duplicated require() checks should be refactored to a modifier or function
  • Missing error message in require statements
  • Use of block.timestamp
  • Floating compiler versions
  • event is missing indexed fields

Low Issues

Use of assert() statements

Contracts use assert() statements in multiple places. As seen in Solidity's documentation:

Assert should only be used to test for internal errors, and to check invariants. Properly functioning code should never create a Panic, not even on invalid external input. If this happens, then there is a bug in your contract which you should fix. Language analysis tools can evaluate your contract to identify the conditions and function calls which will cause a Panic.

Consider using require() statements or if statements with revert() instead.

Instances where assert() is used:

contracts/dnssec-oracle/RRUtils.sol:
  22:        assert(idx < self.length);
  52:        assert(offset < self.length);

Non-Critical Issues

Duplicated require() checks should be refactored to a modifier or function

If the same require() statement that is used to validate function parameters is present across multiple functions in a contract, it should be rewritten into modifier if possible. This would help to reduce code deployment size, which saves gas, and improve code readability.

The following modifier is present in keccak() and substring() in BytesUtils.sol:

contracts/dnssec-oracle/BytesUtils.sol:
  12:        require(offset + len <= self.length);
 235:        require(offset + len <= self.length);

The following modifier is present in safeTransferFrom() and safeBatchTransferFrom() in ERC1155Fuse.sol:

contracts/wrapper/ERC1155Fuse.sol:
 176:        require(to != address(0), "ERC1155: transfer to the zero address");
 199:        require(to != address(0), "ERC1155: transfer to the zero address");

Missing error message in require statements

It is recommended to have descriptive error messages for all require statements to provide users with information should a transaction fail.

Consider adding an error message to the following require statements:

contracts/dnssec-oracle/Owned.sol:
  10:        require(msg.sender == owner);

contracts/dnssec-oracle/BytesUtils.sol:
  12:        require(offset + len <= self.length);
 146:        require(idx + 2 <= self.length);
 159:        require(idx + 4 <= self.length);
 172:        require(idx + 32 <= self.length);
 185:        require(idx + 20 <= self.length);
 199:        require(len <= 32);
 200:        require(idx + len <= self.length);
 235:        require(offset + len <= self.length);
 262:        require(len <= 52);
 268:        require(char >= 0x30 && char <= 0x7A);
 270:        require(decoded <= 0x20);

contracts/ethregistrar/ETHRegistrarController.sol:
  57:        require(_maxCommitmentAge > _minCommitmentAge);
 121:        require(commitments[commitment] + maxCommitmentAge < block.timestamp);
 246:        require(duration >= MIN_REGISTRATION_DURATION);

contracts/wrapper/BytesUtil.sol:
  13:        require(offset + len <= self.length);

Use of block.timestamp

Block timestamps have historically been used for a variety of applications, such as entropy for random numbers (see the Entropy Illusion for further details), locking funds for periods of time, and various state-changing conditional statements that are time-dependent. Miners have the ability to adjust timestamps slightly, which can prove to be dangerous if block timestamps are used incorrectly in smart contracts.

Recommended Mitigation Steps
Block timestamps should not be used for entropy or generating random numbers — i.e., they should not be the deciding factor (either directly or through some derivation) for winning a game or changing an important state.

Time-sensitive logic is sometimes required; e.g., for unlocking contracts (time-locking), completing an ICO after a few weeks, or enforcing expiry dates. It is sometimes recommended to use block.number and an average block time to estimate times; with a 10 second block time, 1 week equates to approximately, 60480 blocks. Thus, specifying a block number at which to change a contract state can be more secure, as miners are unable to easily manipulate the block number.

Instances where block.timestamp is used:

contracts/dnssec-oracle/DNSSECImpl.sol:
  81:        return verifyRRSet(input, block.timestamp);

contracts/ethregistrar/ETHRegistrarController.sol:
 121:        require(commitments[commitment] + maxCommitmentAge < block.timestamp);
 122:        commitments[commitment] = block.timestamp;
 233:        commitments[commitment] + minCommitmentAge <= block.timestamp,
 239:        commitments[commitment] + maxCommitmentAge > block.timestamp,

contracts/wrapper/ERC1155Fuse.sol:
 144:        if (block.timestamp > expiry) {

Floating compiler versions

Non-library/interface files should use fixed compiler versions, not floating ones:

contracts/dnssec-oracle/Owned.sol:
   1:        pragma solidity ^0.8.4;

contracts/dnssec-oracle/DNSSECImpl.sol:
   2:        pragma solidity ^0.8.4;

contracts/ethregistrar/ETHRegistrarController.sol:
   1:        pragma solidity >=0.8.4;

contracts/wrapper/Controllable.sol:
   2:        pragma solidity ^0.8.4;

contracts/wrapper/NameWrapper.sol:
   2:        pragma solidity ^0.8.4;

event is missing indexed fields

Each event should use three indexed fields if there are three or more fields:

contracts/ethregistrar/IBaseRegistrar.sol:
   9:        event NameMigrated(
  10:            uint256 indexed id,
  11:            address indexed owner,
  12:            uint256 expires
  13:        );

  14:        event NameRegistered(
  15:            uint256 indexed id,
  16:            address indexed owner,
  17:            uint256 expires
  18:        );

contracts/ethregistrar/ETHRegistrarController.sol:
  34:        event NameRegistered(
  35:            string name,
  36:            bytes32 indexed label,
  37:            address indexed owner,
  38:            uint256 baseCost,
  39:            uint256 premium,
  40:            uint256 expires
  41:        );

  42:        event NameRenewed(
  43:            string name,
  44:            bytes32 indexed label,
  45:            uint256 cost,
  46:            uint256 expires
  47:        );

contracts/registry/ENS.sol:
   5:        event NewOwner(bytes32 indexed node, bytes32 indexed label, address owner);

  17:        event ApprovalForAll(
  18:            address indexed owner,
  19:            address indexed operator,
  20:            bool approved
  21:        );

contracts/wrapper/INameWrapper.sol:
  19:        event NameWrapped(
  20:            bytes32 indexed node,
  21:            bytes name,
  22:            address owner,
  23:            uint32 fuses,
  24:            uint64 expiry
  25:        );

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

• Gas Report
  • For-loops: Index initialized with default value
  • For-Loops: Cache array length outside of loops
  • For-Loops: Index increments can be left unchecked
  • Arithmetics: ++i costs less gas compared to i++ or i += 1
  • Arithmetics: Use != 0 instead of > 0 for unsigned integers
  • Arithmetics: Use Shift Right/Left instead of Division/Multiplication if possible
  • Visibility: Consider declaring constants as non-public to save gas
  • Visibility: public functions can be set to external
  • Errors: Reduce the length of error messages (long revert strings)
  • Errors: Use multiple require statements instead of &&
  • Errors: Use custom errors instead of revert strings
  • Unnecessary initialization of variables with default values
  • Unnecessary definition of variables
  • Variables declared as constant are expressions, not constants

Gas Report

For-loops: Index initialized with default value

Uninitialized uint variables are assigned with a default value of 0.

Thus, in for-loops, explicitly initializing an index with 0 costs unnecesary gas. For example, the following code:

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

can be changed to:

for (uint256 i; i < length; ++i) {

Consider declaring the following lines without explicitly setting the index to 0:

contracts/dnssec-oracle/RRUtils.sol:
 310:        for(uint i = 0; i < data.length + 31; i += 32) {

contracts/dnssec-oracle/DNSSECImpl.sol:
  93:        for(uint i = 0; i < input.length; i++) {

contracts/dnssec-oracle/BytesUtils.sol:
  56:        for (uint idx = 0; idx < shortest; idx += 32) {
 266:        for(uint i = 0; i < len; i++) {

contracts/ethregistrar/ETHRegistrarController.sol:
 256:        for (uint256 i = 0; i < data.length; i++) {

contracts/wrapper/ERC1155Fuse.sol:
  92:        for (uint256 i = 0; i < accounts.length; ++i) {
 205:        for (uint256 i = 0; i < ids.length; ++i) {

For-Loops: Cache array length outside of loops

Reading an array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.

Caching the array length in the stack saves around 3 gas per iteration.

For example:

for (uint256 i; i < arr.length; ++i) {}

can be changed to:

uint256 len = arr.length;
for (uint256 i; i < len; ++i) {}

Consider making the following change to these lines:

contracts/dnssec-oracle/DNSSECImpl.sol:
  93:        for(uint i = 0; i < input.length; i++) {

contracts/ethregistrar/ETHRegistrarController.sol:
 256:        for (uint256 i = 0; i < data.length; i++) {

contracts/wrapper/ERC1155Fuse.sol:
  92:        for (uint256 i = 0; i < accounts.length; ++i) {
 205:        for (uint256 i = 0; i < ids.length; ++i) {

For-Loops: Index increments can be left unchecked

From Solidity v0.8 onwards, all arithmetic operations come with implicit overflow and underflow checks.

In for-loops, as it is impossible for the index to overflow, it can be left unchecked to save gas every iteration.

For example, the code below:

for (uint256 i; i < numIterations; ++i) {  
    // ...  
}  

can be changed to:

for (uint256 i; i < numIterations;) {  
    // ...  
    unchecked { ++i; }  
}  

Consider making the following change to these lines:

contracts/dnssec-oracle/DNSSECImpl.sol:
  93:        for(uint i = 0; i < input.length; i++) {

contracts/dnssec-oracle/BytesUtils.sol:
 266:        for(uint i = 0; i < len; i++) {
 313:        for(uint256 idx = off; idx < off + len; idx++) {

contracts/ethregistrar/ETHRegistrarController.sol:
 256:        for (uint256 i = 0; i < data.length; i++) {

contracts/ethregistrar/StringUtils.sol:
  14:        for(len = 0; i < bytelength; len++) {

contracts/wrapper/ERC1155Fuse.sol:
  92:        for (uint256 i = 0; i < accounts.length; ++i) {
 205:        for (uint256 i = 0; i < ids.length; ++i) {

Arithmetics: ++i costs less gas compared to i++ or i += 1

++i costs less gas compared to i++ or i += 1 for unsigned integers, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.

i++ increments i and returns the initial value of i. Which means:

uint i = 1;  
i++; // == 1 but i == 2  

But ++i returns the actual incremented value:

uint i = 1;  
++i; // == 2 and i == 2 too, so no need for a temporary variable  

In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2, thus it costs more gas.

The same logic applies for --i and i--.

Consider using ++i instead of i++ or i += 1 in the following instances:

contracts/dnssec-oracle/RRUtils.sol:
  58:        count += 1;
 235:        counts--;
 241:        othercounts--;
 250:        counts -= 1;

contracts/dnssec-oracle/DNSSECImpl.sol:
  93:        for(uint i = 0; i < input.length; i++) {

contracts/dnssec-oracle/BytesUtils.sol:
 266:        for(uint i = 0; i < len; i++) {
 292:        bitlen -= 1;
 313:        for(uint256 idx = off; idx < off + len; idx++) {

contracts/ethregistrar/ETHRegistrarController.sol:
 256:        for (uint256 i = 0; i < data.length; i++) {

contracts/ethregistrar/StringUtils.sol:
  14:        for(len = 0; i < bytelength; len++) {
  17:        i += 1;

Arithmetics: Use != 0 instead of > 0 for unsigned integers

uint will never go below 0. Thus, > 0 is gas inefficient in comparisons as checking if != 0 is sufficient and costs less gas.

Consider changing > 0 to != 0 in these lines:

contracts/dnssec-oracle/RRUtils.sol:
 245:        while (counts > 0 && !self.equals(off, other, otheroff)) {

contracts/ethregistrar/ETHRegistrarController.sol:
  98:        if (data.length > 0) {

contracts/wrapper/BytesUtil.sol:
  44:        if(len > 0) {

Arithmetics: Use Shift Right/Left instead of Division/Multiplication if possible

A division/multiplication by any number x being a power of 2 can be calculated by shifting log2(x) to the right/left.

While the DIV opcode uses 5 gas, the SHR opcode only uses 3 gas. Furthermore, Solidity's division operation also includes a division-by-0 prevention which is bypassed using shifting.

For example, the following code:

uint256 b = a / 2;
uint256 c = a / 4;
uint256 d = a * 8;

can be changed to:

uint256 b = a >> 1;
uint256 c = a >> 2;
uint256 d = a << 3;

Consider making this change to the following lines:

contracts/dnssec-oracle/RRUtils.sol:
 316:        uint unused = 256 - (data.length - i) * 8;

contracts/dnssec-oracle/BytesUtils.sol:
  69:        mask = ~(2 ** (8 * (32 - shortest + idx)) - 1);

Visibility: Consider declaring constants as non-public to save gas

If a constant is not used outside of its contract, declaring it as private or internal instead of public can save gas.

Consider changing the visibility of the following from public to internal or private:

contracts/ethregistrar/ETHRegistrarController.sol:
  21:        uint256 public constant MIN_REGISTRATION_DURATION = 28 days;

Visibility: public functions can be set to external

Calls to external functions are cheaper than public functions. Thus, if a function is not used internally in any contract, it should be set to external to save gas and improve code readability.

Consider changing following functions from public to external:

contracts/dnssec-oracle/Owned.sol:
  18:        function setOwner(address newOwner) public owner_only {

contracts/dnssec-oracle/DNSSECImpl.sol:
  58:        function setAlgorithm(uint8 id, Algorithm algo) public owner_only {
  69:        function setDigest(uint8 id, Digest digest) public owner_only {

contracts/ethregistrar/ETHRegistrarController.sol:
 210:        function withdraw() public {

contracts/wrapper/Controllable.sol:
  11:        function setController(address controller, bool active) onlyOwner() public {

contracts/wrapper/NameWrapper.sol:
 105:        function setMetadataService(IMetadataService _newMetadataService)
 106:            public
 107:            onlyOwner
 108:        {

 128:        function setUpgradeContract(INameWrapperUpgrade _upgradeAddress)
 129:            public
 130:            onlyOwner
 131:        {

 373:        function setFuses(bytes32 node, uint32 fuses)
 374:            public
 375:            onlyTokenOwner(node)
 376:            operationAllowed(node, CANNOT_BURN_FUSES)
 377:            returns (uint32)
 378:        {

 401:        function upgradeETH2LD(
 402:            string calldata label,
 403:            address wrappedOwner,
 404:            address resolver
 405:        ) public {

 429:        function upgrade(
 430:            bytes32 parentNode,
 431:            string calldata label,
 432:            address wrappedOwner,
 433:            address resolver
 434:        ) public {

 456:        function setChildFuses(
 457:            bytes32 parentNode,
 458:            bytes32 labelhash,
 459:            uint32 fuses,
 460:            uint64 expiry
 461:        ) public {

 504:        function setSubnodeOwner(
 505:            bytes32 parentNode,
 506:            string calldata label,
 507:            address newOwner,
 508:            uint32 fuses,
 509:            uint64 expiry
 510:        )
 511:            public
 512:            onlyTokenOwner(parentNode)
 513:            canCallSetSubnodeOwner(parentNode, keccak256(bytes(label)))
 514:            returns (bytes32 node)
 515:        {

 539:        function setSubnodeRecord(
 540:            bytes32 parentNode,
 541:            string memory label,
 542:            address newOwner,
 543:            address resolver,
 544:            uint64 ttl,
 545:            uint32 fuses,
 546:            uint64 expiry
 547:        )
 548:            public
 549:            onlyTokenOwner(parentNode)
 550:            canCallSetSubnodeOwner(parentNode, keccak256(bytes(label)))
 551:        {

Errors: Reduce the length of error messages (long revert strings)

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

In these instances, consider shortening the revert strings to fit within 32 bytes, or using custom errors:

contracts/ethregistrar/ETHRegistrarController.sol:
  99:        require(
 100:            resolver != address(0),
 101:            "ETHRegistrarController: resolver is required when data is supplied"
 102:        );

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

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

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

 238:        require(
 239:            commitments[commitment] + maxCommitmentAge > block.timestamp,
 240:            "ETHRegistrarController: Commitment has expired"
 241:        );

 242:        require(available(name), "ETHRegistrarController: Name is unavailable");

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

contracts/registry/ReverseRegistrar.sol:
  41:        require(
  42:            addr == msg.sender ||
  43:                controllers[msg.sender] ||
  44:                ens.isApprovedForAll(addr, msg.sender) ||
  45:                ownsContract(addr),
  46:            "ReverseRegistrar: Caller is not a controller or authorised by address or the address itself"
  47:        );

  52:        require(
  53:            address(resolver) != address(0),
  54:            "ReverseRegistrar: Resolver address must not be 0"
  55:        );

contracts/wrapper/Controllable.sol:
  17:        require(controllers[msg.sender], "Controllable: Caller is not a controller");

contracts/wrapper/ERC1155Fuse.sol:
  60:        require(
  61:            account != address(0),
  62:            "ERC1155: balance query for the zero address"
  63:        );

  85:        require(
  86:            accounts.length == ids.length,
  87:            "ERC1155: accounts and ids length mismatch"
  88:        );

 107:        require(
 108:            msg.sender != operator,
 109:            "ERC1155: setting approval status for self"
 110:        );

 176:        require(to != address(0), "ERC1155: transfer to the zero address");

 177:        require(
 178:            from == msg.sender || isApprovedForAll(from, msg.sender),
 179:            "ERC1155: caller is not owner nor approved"
 180:        );

 195:        require(
 196:            ids.length == amounts.length,
 197:            "ERC1155: ids and amounts length mismatch"
 198:        );

 199:        require(to != address(0), "ERC1155: transfer to the zero address");

 200:        require(
 201:            from == msg.sender || isApprovedForAll(from, msg.sender),
 202:            "ERC1155: transfer caller is not owner nor approved"
 203:        );

 215:        require(
 216:            amount == 1 && oldOwner == from,
 217:            "ERC1155: insufficient balance for transfer"
 218:        );

 249:        require(newOwner != address(0), "ERC1155: mint to the zero address");

 250:        require(
 251:            newOwner != address(this),
 252:            "ERC1155: newOwner cannot be the NameWrapper contract"
 253:        );

 290:        require(
 291:            amount == 1 && oldOwner == from,
 292:            "ERC1155: insufficient balance for transfer"
 293:        );

Errors: Use multiple require statements instead of &&

Instead of using a single require statement with the && operator, using multiple require statements would help to save runtime gas cost. However, note that this results in a higher deployment gas cost, which is a fair trade-off.

A require statement can be split as such:

// Original code:
require(a && b, 'error');

// Changed to:
require(a, 'error: a');
require(b, 'error: b');

Instances where multiple require statements should be used:

contracts/dnssec-oracle/BytesUtils.sol:
 268:        require(char >= 0x30 && char <= 0x7A);

contracts/wrapper/ERC1155Fuse.sol:
 215:        require(
 216:            amount == 1 && oldOwner == from,
 217:            "ERC1155: insufficient balance for transfer"
 218:        );

 290:        require(
 291:            amount == 1 && oldOwner == from,
 292:            "ERC1155: insufficient balance for transfer"
 293:        );

Errors: Use custom errors instead of revert strings

Since Solidity v0.8.4, custom errors should be used instead of revert strings due to:

• Cheaper deployment cost
• Lower runtime cost upon revert

Taken from Custom Errors in Solidity:

Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them.

Custom errors can be defined using of the error statement, both inside or outside of contracts.

Instances where custom errors can be used instead:

contracts/dnssec-oracle/Owned.sol:
  10:        require(msg.sender == owner);

contracts/dnssec-oracle/RRUtils.sol:
 307:        require(data.length <= 8192, "Long keys not permitted");

contracts/dnssec-oracle/BytesUtils.sol:
  12:        require(offset + len <= self.length);
 146:        require(idx + 2 <= self.length);
 159:        require(idx + 4 <= self.length);
 172:        require(idx + 32 <= self.length);
 185:        require(idx + 20 <= self.length);
 199:        require(len <= 32);
 200:        require(idx + len <= self.length);
 235:        require(offset + len <= self.length);
 262:        require(len <= 52);
 268:        require(char >= 0x30 && char <= 0x7A);
 270:        require(decoded <= 0x20);

contracts/ethregistrar/ETHRegistrarController.sol:
  57:        require(_maxCommitmentAge > _minCommitmentAge);

  99:        require(
 100:            resolver != address(0),
 101:            "ETHRegistrarController: resolver is required when data is supplied"
 102:        );

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

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

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

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

 238:        require(
 239:            commitments[commitment] + maxCommitmentAge > block.timestamp,
 240:            "ETHRegistrarController: Commitment has expired"
 241:        );

 242:        require(available(name), "ETHRegistrarController: Name is unavailable");
 246:        require(duration >= MIN_REGISTRATION_DURATION);

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

contracts/registry/ReverseRegistrar.sol:
  41:        require(
  42:            addr == msg.sender ||
  43:                controllers[msg.sender] ||
  44:                ens.isApprovedForAll(addr, msg.sender) ||
  45:                ownsContract(addr),
  46:            "ReverseRegistrar: Caller is not a controller or authorised by address or the address itself"
  47:        );

  52:        require(
  53:            address(resolver) != address(0),
  54:            "ReverseRegistrar: Resolver address must not be 0"
  55:        );

contracts/wrapper/BytesUtil.sol:
  13:        require(offset + len <= self.length);
  28:        require(offset == self.length - 1, "namehash: Junk at end of name");
  42:        require(idx < self.length, "readLabel: Index out of bounds");

contracts/wrapper/Controllable.sol:
  17:        require(controllers[msg.sender], "Controllable: Caller is not a controller");

contracts/wrapper/ERC1155Fuse.sol:
  60:        require(
  61:            account != address(0),
  62:            "ERC1155: balance query for the zero address"
  63:        );

  85:        require(
  86:            accounts.length == ids.length,
  87:            "ERC1155: accounts and ids length mismatch"
  88:        );

 107:        require(
 108:            msg.sender != operator,
 109:            "ERC1155: setting approval status for self"
 110:        );

 176:        require(to != address(0), "ERC1155: transfer to the zero address");

 177:        require(
 178:            from == msg.sender || isApprovedForAll(from, msg.sender),
 179:            "ERC1155: caller is not owner nor approved"
 180:        );

 195:        require(
 196:            ids.length == amounts.length,
 197:            "ERC1155: ids and amounts length mismatch"
 198:        );

 199:        require(to != address(0), "ERC1155: transfer to the zero address");

 200:        require(
 201:            from == msg.sender || isApprovedForAll(from, msg.sender),
 202:            "ERC1155: transfer caller is not owner nor approved"
 203:        );

 215:        require(
 216:            amount == 1 && oldOwner == from,
 217:            "ERC1155: insufficient balance for transfer"
 218:        );

 248:        require(owner == address(0), "ERC1155: mint of existing token");
 249:        require(newOwner != address(0), "ERC1155: mint to the zero address");

 250:        require(
 251:            newOwner != address(this),
 252:            "ERC1155: newOwner cannot be the NameWrapper contract"
 253:        );

 290:        require(
 291:            amount == 1 && oldOwner == from,
 292:            "ERC1155: insufficient balance for transfer"
 293:        );

Unnecessary initialization of variables with default values

Uninitialized variables are assigned with a default value depending on its type:

• uint: 0
• bool: false
• address: address(0)

Thus, explicitly initializing a variable with its default value costs unnecesary gas. For example, the following code:

bool b = false;
address c = address(0);
uint256 a = 0;

can be changed to:

uint256 a;
bool b;
address c;

Consider declaring the following lines without explicitly setting a value:

contracts/dnssec-oracle/RRUtils.sol:
  50:        uint count = 0;
  63:        uint constant RRSIG_TYPE = 0;
 181:        uint constant DNSKEY_FLAGS = 0;
 200:        uint constant DS_KEY_TAG = 0;

contracts/dnssec-oracle/BytesUtils.sol:
 264:        uint ret = 0;

contracts/ethregistrar/StringUtils.sol:
  12:        uint i = 0;

contracts/wrapper/INameWrapper.sol:
  16:        uint32 constant CAN_DO_EVERYTHING = 0;

Unnecessary definition of variables

Some variables are defined even though they are only used once in their respective functions. Not defining these variables and directly using the expressions on the right can help to reduce gas cost and contract size.

Instances include:

contracts/ethregistrar/ETHRegistrarController.sol:
 258:        bytes32 txNamehash = bytes32(data[i][4:36]);

contracts/wrapper/NameWrapper.sol:
 193:        bytes memory name = names[node];

contracts/wrapper/ERC1155Fuse.sol:
 207:        uint256 amount = amounts[i];

Variables declared as constant are expressions, not constants

Due to how constant variables are implemented (replacements at compile-time), an expression assigned to a constant variable is recomputed each time that the variable is used, which wastes some gas.

If the variable was immutable instead: the calculation would only be done once at deploy time (in the constructor), and then the result would be saved and read directly at runtime rather than being recalculated.

See: ethereum/solidity#9232:

Consequences: each usage of a “constant” costs ~100 gas more on each access (it is still a little better than storing the result in storage, but not much). since these are not real constants, they can’t be referenced from a real constant environment (e.g. from assembly, or from another library)

contracts/wrapper/NameWrapper.sol:
  47:        uint64 private constant MAX_EXPIRY = type(uint64).max;

Change these expressions from constant to immutable and implement the calculation in the constructor. Alternatively, hardcode these values in the constants and add a comment to say how the value was calculated.