FlashloanStrategy Static Analysis Findings

FSG-01S: Illegible Numeric Value Representations

Type	Severity	Location
Code Style		FlashloanStrategy.sol: • I-1: L621 • I-2: L639 • I-3: L687

Description:

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

Example:

Copy
621/ 10_000

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 value literals have been replaced by the BPS_BASIS constant rendering this exhibit no longer applicable.

FSG-02S: Inexistent Sanitization of Input Addresses

Type	Severity	Location
Input Sanitization		FlashloanStrategy.sol:L111-L136

Description:

The linked function(s) accept address arguments yet do not properly sanitize them.

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:

Copy
111constructor(
112    address admin_,
113    SickleFactory sickleFactory_,
114    address aaveV2LendingPool_,
115    address aaveV3LendingPool_,
116    address balancerVault_,
117    address quickswapFactoryAddr_,
118    address uniswapV3FactoryAddr_,
119    address morpho_,
120    bytes4[] memory whitelistedOpsSelectors_,
121    address[] memory correspondingStrategies_
122) Admin(admin_) {
123    sickleFactory = sickleFactory_;
124    aaveV2LendingPool = aaveV2LendingPool_;
125    aaveV3LendingPool = aaveV3LendingPool_;
126    balancerVault = balancerVault_;
127    quickswapFactoryAddr = quickswapFactoryAddr_;
128    uniswapV3FactoryAddr = uniswapV3FactoryAddr_;
129    morpho = morpho_;
130
131    currentFlashloanStatus = FlashloanStatus.FLASHLOAN_UNLOCKED;
132
133    _setWhitelistedFlashloanOpsSelectors(
134        whitelistedOpsSelectors_, correspondingStrategies_
135    );
136}

Recommendation:

We advise some basic sanitization to be put in place by ensuring that each 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.

FSG-03S: Inexistent Visibility Specifiers

Type	Severity	Location
Code Style		FlashloanStrategy.sol: • I-1: L93 • I-2: L94

Description:

The linked variables have no visibility specifier explicitly set.

Example:

Copy
93bytes32 flashloanDataHash;

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.

FSG-04S: Literal Equality of bool Variable

Type	Severity	Location
Gas Optimization		FlashloanStrategy.sol:L224

Description:

The linked bool comparison is performed between a variable and a bool literal.

Example:

Copy
224if (registry.isWhitelistedCaller(msg.sender) == false) {

Recommendation:

We advise the bool variable to be utilized directly either in its negated (!) or original form.

Alleviation (6ab7af3bb495b817ffec469255ea679b1813eecb):

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

FSG-05S: Multiple Top-Level Declarations

Type	Severity	Location
Code Style		FlashloanStrategy.sol:L28, L36

Description:

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

Example:

Copy
28library FlashloanStrategyEvents {
29    event SelectorLinked(bytes4 selector, address strategy);
30}
31
32/// @title FlashloanStrategy contract
33/// @author vfat.tools
34/// @notice Manages approved flashloan providers for the sickle and calls to and
35/// from flashloan providers
36contract FlashloanStrategy is Admin, IFlashLoanRecipient {

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.