ENS contest - simon135's results

dont use charot pragma because their can be bugs in deployment then when your testing on a diffrent version

https://github.com/code-423n4/2022-07-ens/blob/1772ac3d16c8e84bb9c1e02a0c4e321cd6f5652b/contracts/wrapper/NameWrapper.sol#L2 https://github.com/code-423n4/2022-07-ens/blob/04539b27307abd65068b96dd20fc686b451bf9fc/contracts/dnssec-oracle/BytesUtils.sol#L1

no emit of new owner and older owner its best practice to do so

https://github.com/code-423n4/2022-07-ens/blob/04539b27307abd65068b96dd20fc686b451bf9fc/contracts/dnssec-oracle/Owned.sol#L18-L21 https://github.com/code-423n4/2022-07-ens/blob/04539b27307abd65068b96dd20fc686b451bf9fc/contracts/wrapper/Controllable.sol#L11

and no approve of new owner its best pratice to let the new owner call approve

instead of the owner giveing it to him already becuse if the new owner cant be come new owner (keys missing) then its wasted address and you might have to redeploy. https://github.com/code-423n4/2022-07-ens/blob/04539b27307abd65068b96dd20fc686b451bf9fc/contracts/dnssec-oracle/Owned.sol#L18-L21

typos

instead of : authorised use : authorized https://github.com/code-423n4/2022-07-ens/blob/1772ac3d16c8e84bb9c1e02a0c4e321cd6f5652b/contracts/wrapper/NameWrapper.sol#L329

dont use transfer instead use call

becauser transfer can revert because it only can foward 2300 gas which is msg.sender fallback functions is anything not just an emit it will revert. https://github.com/code-423n4/2022-07-ens/blob/04539b27307abd65068b96dd20fc686b451bf9fc/contracts/ethregistrar/ETHRegistrarController.sol#L204https://github.com/code-423n4/2022-07-ens/blob/04539b27307abd65068b96dd20fc686b451bf9fc/contracts/ethregistrar/ETHRegistrarController.sol#L183

Use Custom Errors instead of Revert Strings to save Gas

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) Source 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 are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries).

./contracts/root/Ownable.sol:10:        require(isOwner(msg.sender));
./contracts/root/Root.sol:26:        require(!locked[label]);
./contracts/utils/LowLevelCallUtils.sol:22:        require(
./contracts/wrapper/BytesUtil.sol:13:        require(offset + len <= self.length);
./contracts/wrapper/BytesUtil.sol:28:            require(offset == self.length - 1, "namehash: Junk at end of name");
./contracts/wrapper/BytesUtil.sol: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(
./contracts/wrapper/ERC1155Fuse.sol:85:        require(
./contracts/wrapper/ERC1155Fuse.sol:107:        require(
./contracts/wrapper/ERC1155Fuse.sol:176:        require(to != address(0), "ERC1155: transfer to the zero address");
./contracts/wrapper/ERC1155Fuse.sol:177:        require(
./contracts/wrapper/ERC1155Fuse.sol:195:        require(
./contracts/wrapper/ERC1155Fuse.sol:199:        require(to != address(0), "ERC1155: transfer to the zero address");
./contracts/wrapper/ERC1155Fuse.sol:200:        require(
./contracts/wrapper/ERC1155Fuse.sol:215:            require(
./contracts/wrapper/ERC1155Fuse.sol:248:        require(owner == address(0), "ERC1155: mint of existing token");
./contracts/wrapper/ERC1155Fuse.sol:249:        require(newOwner != address(0), "ERC1155: mint to the zero address");
./contracts/wrapper/ERC1155Fuse.sol:250:        require(
./contracts/wrapper/ERC1155Fuse.sol:290:        require(
## ++i costs less gas compared to i++ or i += 1
++i costs less gas compared to i++ or i += 1 for unsigned integer, 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
uint256 i = 0; i < depositedTokens.values.length; i++)
https://github.com/code-423n4/2022-07-ens/blob/04539b27307abd65068b96dd20fc686b451bf9fc/contracts/dnssec-oracle/RRUtils.sol#L235
https://github.com/code-423n4/2022-07-ens/blob/04539b27307abd65068b96dd20fc686b451bf9fc/contracts/dnssec-oracle/RRUtils.sol#L250
https://github.com/code-423n4/2022-07-ens/blob/04539b27307abd65068b96dd20fc686b451bf9fc/contracts/dnssec-oracle/RRUtils.sol#L235

make only_owner functions payable to save gas

because there is no check for msg.value =0 so it saves gas

./contracts/registry/FIFSRegistrar.sol:33:    function register(bytes32 label, address owner) public only_owner(label)

https://github.com/code-423n4/2022-07-ens/blob/04539b27307abd65068b96dd20fc686b451bf9fc/contracts/dnssec-oracle/DNSSECImpl.sol#L58-L69

make address 0 into long form

to save gas ex: 0x000000000000 https://github.com/code-423n4/2022-07-ens/blob/04539b27307abd65068b96dd20fc686b451bf9fc/contracts/ethregistrar/ETHRegistrarController.sol#L100

./contracts/wrapper/NameWrapper.sol:132:        if (address(upgradeContract) != address(0)) {
./contracts/wrapper/NameWrapper.sol:139:        if (address(upgradeContract) != address(0)) {
./contracts/wrapper/NameWrapper.sol:318:        if (resolver != address(0)) {
./contracts/wrapper/NameWrapper.sol:661:        if (owner == address(0)) {
./contracts/wrapper/NameWrapper.sol:763:        if (oldWrappedOwner != address(0)) {
./contracts/wrapper/NameWrapper.sol:766:            emit NameUnwrapped(node, address(0));
./contracts/wrapper/NameWrapper.sol:799:        if (address(upgradeContract) == address(0)) {
./contracts/wrapper/NameWrapper.sol:911:        if (resolver != address(0)) {
./contracts/wrapper/NameWrapper.sol:919:        if (newOwner == address(0x0) || newOwner == address(this)) {
./contracts/wrapper/ERC1155Fuse.sol:176:        require(to != address(0), "ERC1155: transfer to the zero address");
./contracts/wrapper/ERC1155Fuse.sol:199:        require(to != address(0), "ERC1155: transfer to the zero address");
./contracts/wrapper/ERC1155Fuse.sol:248:        require(owner == address(0), "ERC1155: mint of existing token");
./contracts/wrapper/ERC1155Fuse.sol:249:        require(newOwner != address(0), "ERC1155: mint to the zero address"

Using bools for storage incurs overhead

// Booleans are more expensive than uint256 or any type that takes up a full // word because each write operation emits an extra SLOAD to first read the // slot's contents, replace the bits taken up by the boolean, and then write // back. This is the compiler's defense against contract upgrades and // pointer aliasing, and it cannot be disabled. https://github.com/code-423n4/2022-07-ens/blob/04539b27307abd65068b96dd20fc686b451bf9fc/contracts/wrapper/Controllable.sol#L7

make 2 require statements instead of && in a require statement to save gas

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

cache array.length into a variable to save gas

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

Unchecking arithmetics operations that can’t underflow/overflow

Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn’t possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked block: Checked or Unchecked Arithmetic. I suggest wrapping with an unchecked block: Same thing with second unchecked because total can't overflow amount cant overflow

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