Omniscia vfat Audit

NftSettingsRegistry Static Analysis Findings

NftSettingsRegistry Static Analysis Findings

NSR-01S: Illegible Numeric Value Representations

TypeSeverityLocation
Code StyleNftSettingsRegistry.sol:
I-1: L83
I-2: L84
I-3: L85

Description:

The linked representations of numeric literals are sub-optimally represented decreasing the legibility of the codebase.

Example:

contracts/NftSettingsRegistry.sol
83uint256 constant MAX_SLIPPAGE_BP = 500;

Recommendation:

To properly illustrate each value's purpose, we advise the following guidelines to be followed. For values meant to depict fractions with a base of 1e18, we advise fractions to be utilized directly (i.e. 1e17 becomes 0.1e18) as they are supported. For values meant to represent a percentage base, we advise each value to utilize the underscore (_) separator to discern the percentage decimal (i.e. 10000 becomes 100_00, 300 becomes 3_00 and so on). Finally, for large numeric values we simply advise the underscore character to be utilized again to represent them (i.e. 1000000 becomes 1_000_000).

Alleviation (6ab7af3bb495b817ffec469255ea679b1813eecb):

The vfat team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase.

NSR-02S: Inexistent Sanitization of Input Address

Description:

The linked function accepts an address argument yet does not properly sanitize it.

Impact:

The presence of zero-value addresses, especially in constructor implementations, can cause the contract to be permanently inoperable. These checks are advised as zero-value inputs are a common side-effect of off-chain software related bugs.

Example:

contracts/NftSettingsRegistry.sol
91constructor(
92 SickleFactory _factory
93) {
94 factory = _factory;
95}

Recommendation:

We advise some basic sanitization to be put in place by ensuring that the address specified is non-zero.

Alleviation (6ab7af3bb495b817ffec469255ea679b1813eecb):

The vfat team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase.

NSR-03S: Inexistent Visibility Specifiers

TypeSeverityLocation
Code StyleNftSettingsRegistry.sol:
I-1: L83
I-2: L84
I-3: L85
I-4: L86
I-5: L87
I-6: L97

Description:

The linked variables have no visibility specifier explicitly set.

Example:

contracts/NftSettingsRegistry.sol
83uint256 constant MAX_SLIPPAGE_BP = 500;

Recommendation:

We advise them to be set so to avoid potential compilation discrepancies in the future as the current behaviour is for the compiler to assign one automatically which may deviate between pragma versions.

Alleviation (6ab7af3bb495b817ffec469255ea679b1813eecb):

The vfat team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase.

NSR-04S: Multiple Top-Level Declarations

Description:

The referenced file contains multiple top-level declarations that decrease the legibility of the codebase.

Example:

contracts/NftSettingsRegistry.sol
27interface IPreviousAutomation {
28 function rewardAutomation(
29 address user
30 ) external returns (RewardBehavior);
31 function harvestTokensOut(
32 address user
33 ) external returns (address);
34}
35
36interface IPreviousNftSettingsRegistry {

Recommendation:

We advise all highlighted top-level declarations to be split into their respective code files, avoiding unnecessary imports as well as increasing the legibility of the codebase.

Alleviation (6ab7af3bb495b817ffec469255ea679b1813eecb):

The vfat team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase.