QA Report

[code423n4]

Lack of empty address checks

The following methods have a lack checks if the received argument is an address, it's good practice in order to reduce human error to check that the address specified in the constructor or initialize is different than address(0).

Source code:

• CollateralAdapter.sol#L36
• CollateralAdapter.sol#L44-L46
• ConvexCurveLPVault.sol#L37
• GeneralVault.sol#L62
• YieldManager.sol#L61
• YieldManager.sol#L73

Change important values without emitting an event

When features are modified that affect the project's economics or ecology, it's critical to send out an event so that users and dapps can react appropriately.

Source code:

• CollateralAdapter.sol#L44-L46

Wrong type used decimals uint8

The ERC20 standard establishes that the decimals method is of type uint8, however a variable of type uint256 is used to handle it.

Source code:

• ConvexCurveLPVault.sol#L124
• GeneralVault.sol#L122

As stated in the OpenZeppelin source comments, the OpenZeppeling ERC20 safeApprove() function has been deprecated.

Using this deprecated function could result in accidental reverts and possibly fund locking. The deprecation of this function is discussed in greater depth in the OZ issue #2219.

As suggested by the OpenZeppelin comment, replace safeApprove() with safeIncreaseAllowance() or safeDecreaseAllowance() instead.

Source code:

• ConvexCurveLPVault.sol#L141-L146
• LidoVault.sol#L102

The variable _assetsCount in the YieldManager contract could be out of sync with the total assets.

The number could be different if the owner sent the same _asset twice because the assets were not checked to see if they were already in _assetsList.

Source code:

• YieldManager.sol#L75

unsafe approve

It was found some approve without checking the boolean result, ERC20 standard specify that the token can return false if this call was not made, so it's mandatory to check the result of these methods.

Source code:

• YieldManager.sol#L221

[HickupHH3]

Low issues: safeApprove(), _assetsCount out of sync, unsafe approve
NC issues: Empty address checks, event emission,
Invalid: wrong decimals used