QA Report

[code423n4]

Missing Equivalence Checks in Setters

Severity: Low
Context: DNSSECImpl.sol#L58-L72, ReverseRegistrar.sol#L51-L57

Description:
Setter functions are missing checks to validate if the new value being set is the same as the current value already set in the contract. Such checks will showcase mismatches between on-chain and off-chain states.

Recommendation:
This may hinder detecting discrepancies between on-chain and off-chain states leading to flawed assumptions of on-chain state and protocol behavior.

Commented Out Code

Severity: Informational
Context: ReverseRegistrar.sol#L16

Description:
There is commented code that makes the code messy and unneeded.

Recommendation:
Remove the commented out code.

TODOs Left In The Code

Severity: Informational
Context: DNSSECImpl.sol#L238

Description:
There should never be any TODOs in the code when deploying.

Recommendation:
Finish the TODOs before deploying.

Missing or Incomplete NatSpec

Severity: Informational
Context: SHA1.sol, Owned.sol, DNSSEC.sol, ETHRegistrarController.sol, ERC1155Fuse.sol, Controllable.sol`

Description:
Some functions are missing @notice/@dev NatSpec comments for the function, @param for all/some of their parameters and @return for return values. Given that NatSpec is an important part of code documentation, this affects code comprehension, auditability and usability.

Recommendation:
Add in full NatSpec comments for all functions to have complete code documentation for future use.

Spelling Errors

Severity: Informational
Context: BytesUtils.sol#L35 (codepoints => code points), RRUtils.sol#L282 (bitshifting => bit shifting), ReverseRegistrar.sol#L40 (authorised => authorized), ReverseRegistrar.sol#L46 (authorised => authorized), ReverseRegistrar.sol#L80 (authorised => authorized), ReverseRegistrar.sol#L126 (authorised => authorized), ReverseRegistrar.sol#L156 (optimised => optimized), NameWrapper.sol#L152 (Unauthorised => Unauthorized), NameWrapper.sol#L202 (authorised => authorized), NameWrapper.sol#L241 (authorised => authorized), NameWrapper.sol#L266 (authorised => authorized), NameWrapper.sol#L282 (_normaliseExpiry => _normalizeExpiry), NameWrapper.sol#L289 (authorised => authorized), NameWrapper.sol#L315 (Unauthorised => Unauthorized), NameWrapper.sol#L370 (PARENT_CANOT_CONTROL => PARENT_CANNOT_CONTROL), NameWrapper.sol#L381 (Unauthorised => Unauthorized), NameWrapper.sol#L482 (_normaliseExpiry => _normalizeExpiry), NameWrapper.sol#L518 (_getDataAndNormaliseExpiry => _getDataAndNormalizeExpiry), NameWrapper.sol#L534 (regsitry => registry), NameWrapper.sol#L554 (_getDataAndNormaliseExpiry => _getDataAndNormalizeExpiry), NameWrapper.sol#L825 (_getDataAndNormaliseExpiry => _getDataAndNormalizeExpiry), NameWrapper.sol#L842 (_normaliseExpiry => _normalizeExpiry), NameWrapper.sol#L846 (_getETH2LDDataAndNormaliseExpiry => _getETH2LDDataAndNormalizeExpiry), NameWrapper.sol#L863 (_normaliseExpiry => _normalizeExpiry), NameWrapper.sol#L867 (_normaliseExpiry => _normalizeExpiry), NameWrapper.sol#L897 (_getETH2LDDataAndNormaliseExpiry => _getETH2LDDataAndNormalizeExpiry)

Description:
Spelling errors in comments can cause confusion to both users and developers.

Recommendation:
Check all misspellings to ensure they are corrected.

Use of ABIEncoderV2 With 0.8+

Severity: Informational
Context: DNSSECImpl.sol, DNSSEC.sol

Description:
ABIEncoderV2 is being stated in a solidity version 0.8+ which is not needed since ABIEncoderV2 is activated by default 0.8+.

Recommendation:
remove pragma experimental ABIEncoderV2;.

Missing SPDX License Identifier

Severity: Informational
Context: IBaseRegistrar.sol

Description:
This contract is missing the SPDX license Identifier before the pragma version.

Recommendation:
Consider adding the SPDX license.

Missing Pragma Version

Severity: Informational
Context: IBaseRegistrar.sol

Description:
This contract is missing the pragma version.

Recommendation:
Consider adding the pragma version.

Floating Pragma

Severity: Informational
Context: All Contracts

Description:
Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

Recommendation:
Lock the pragma version.

Older Version Pragma

Severity: Informational
Context: All Contracts

Description:
Using very old versions of Solidity prevents benefits of bug fixes and newer security checks. Using the latest versions might make contracts susceptible to undiscovered compiler bugs.

Recommendation:
Consider using the most recent version.

[dmvt]

British English spellings should not be considered spelling errors.