QA Report

[code423n4]

[NC - 01] - Missing comments

The code is super well documented, expect from a few functions which lacks NatSpec comments.

Code: https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVaultFactory.sol#L76 https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVaultFactory.sol#L80 https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVaultFactory.sol#L88

[NC - 02] - Incorrect comment https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L297

buy happens on secondary curve and primary curve both -> buy happens on both secondary curve and primary curve

[Low - 01] UPDATE_TIME is not changeable

UPDATE_TIME is not changeable, but is not a constant either. It seems, as everything is pausable, that it is not a real issue that it is not changeable, although it seems desirable as 2 days is quite long and prevents the team from being reactive during certain periods.

A commun process would be to start with a small timelock as pieces may be moving in the first place and progressively extend it.

[Low - 02] - Initialize implementations

Implementations contract should always be initialized. First because it leads in past to the UUPS proxy hack, and then because you don’t want a third party to use the implementation contract and play with it.

Code: https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L173 https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/Basket.sol#L23

Mitigation step:

Just add a void constructor with the initializer modifier:

/// @custom:oz-upgrades-unsafe-allow constructor
constructor() initializer {}


[HardlyDifficult]

Helpful suggestions