ENS contest - 0xf15ers's results

1. Use bytes.concat instead of abi.encodePacked

• Since 0.8.4 it is possible to use bytes.concat, which expects literals, bytes, and bytesNN inputs, and is aimed to replace most use cases of abi.encodePacked. It is more expressive, avoids the complex rules of abi.encodePacked, and the latter is expected to be phased out in the future.

2. Use of pragma experimental ABIEncoderV2 is not necessary in later version (since 0.7.4)

• https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/dnssec-oracle/DNSSEC.sol#L3

3. floating pragma

• Using a unlocked pragma might result in contract being deployed with a version it was not tested with, and might result in bugs and unwanted behaviour.
• https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/dnssec-oracle/DNSSEC.sol#L2

4. Interfaces funtions are implicitly virtual

• https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/dnssec-oracle/digests/Digest.sol#L13
• No need to mark the function as virtual

5. SPDX license identifier not provided in source file

• https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/dnssec-oracle/RRUtils.sol#L1

6. public Visibiitliy can be ignored for constructor

• https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/dnssec-oracle/Owned.sol#L18

7. Use two-step process for changing critical roles

• Since critical roles may sometimes never be recovered if set to a wrong address, it is good practice to implement two step process where the new address needs to claim the role after the role is set. That way there is no chance of having the role set to a dead address.
• https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/dnssec-oracle/Owned.sol#L19

8. Missing Address(0) checks while setting new address

• It is a good practice to check for zero address before setting global variables in constructors.
• https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/wrapper/NameWrapper.sol#L54
• https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/dnssec-oracle/Owned.sol#L19

9. now is a builtin symbol

• Although now is deprecated, it is recommened to use other than built-in symbol as a variable.
• https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/dnssec-oracle/DNSSECImpl.sol#L91