prePO contest - 0xNazgul's results

Lack of Zero Address Validation in functions

Severity: Low

Description Although most of the functions throughout the codebase properly validate function inputs, there are some instances of functions that do not. Such as severely initialize() functions that do not check for zero address. They are missing in:

In Collateral.sol there is: initialize() is missing zero address check for _newBaseToken and _newTreasury.

InDepositHook.sol there is: constructor() there are no zero address checks for _newAccessController and _newDepositRecord.

PrePOMarketFactory.sol there is: createMarket() there are no zero address checks for _governance and _newCollateral.

SingleStrategyController.sol there is: setVault() there is no zero address check for _newVault.

WithdrawHook.sol there is: constructor() there is no zero address checks for _newDepositRecord.

Recommendation Add in zero address checks to avoid having to waste gas on a redeploy or brick anything.

Initializations May Be Front-Run

Description Low Contracts using initialize patterns, instead of constructors, may be susceptible to front-running if not properly deployed. Many contracts use initialize pattern, instead of constructors, at deployment to initialize key contract variables. If factory patterns are not used to deploy and initialize such contracts atomically or if deployment scripts are not robust enough to prevent front-running of such initialization then it may lead to security concerns. While most of them use OpenZeppelin’s initializable to enforce single initializations, few of them reimplement this functionality instead of using the OpenZeppelin library.

Contracts that use initialize: In Collateral.sol

Recommendation Use a factory pattern that will deploy and initialize atomically to prevent front-running of the initialization, or ensure the deployment scripts are robust in case of a front-running attack.

Catching The Array Length Prior To Loop.

Description One can save gas by caching the array length (in stack) and using that set variable in the loop. They are as follows: In AccountAccessController.sol There is:

allowAccounts() and blockAccounts().

Recommendation Simply do something like so before the for loop: uint length = _accounts.length. Then add length in place of _accounts.length in the for loop.

Pragma Directive Ordering

Description Pragma directive can not go after import directives. This can be found in the following contracts:

CollateralDepositRecord.sol

DepositHook.sol

WithdrawHook.sol

Recommendation Move the pragma directive prior to import directives.

Unused Function Parameter

Description There is an unused function parameter in the following contracts functions: In DepositHook.sol The function hook() unused parameter uint256 _initialAmount remove the _amount variable passed through collataral.sol informational/maybe gas since it cuts byte code?

In WithdrawHook.sol The function hook

Both have an unused parameter of uint256 _initialAmount.

Recommendation In both hook() functions uint256 _initialAmount can be removed to save some gas. Along with the removal of the _amount variable being passed to them through Collateral.sol in the functions deposit() and withdraw().

Function Ordering via Method ID

Description As noted in the "Gas Optimization Notes" there is a list of each contracts most called functions since this is known one could simply save gas by function ordering via Method ID Calling a function at runtime will be cheaper if the function is positioned earlier in the order (has a relatively lower Method ID) because 22 gas are added to the cost of a function for every position that came before it. The caller can save on gas if you prioritize most called functions. One could use This tool to help find alternative function names with lower Method IDs while keeping the original name intact.

Recommendation Find a lower method ID name for the most called functions for example mostCalled() vs. mostCalled_41q() is cheaper by 44 gas.