V2ToV3RolloverBase Static Analysis Findings

VTR-01S: Redundant Variable Assignment

Type	Severity	Location
Gas Optimization		V2ToV3RolloverBase.sol:L49

Description:

The linked variable is assigned to redundantly to the default value of the relevant data type (i.e. uint256 assigned to 0, address assigned to address(0) etc.).

Example:

Copy
49bool public paused = false;

Recommendation:

We advise the assignment to be safely omitted optimizing the codebase.

Alleviation:

The redundant variable assignment has been safely omitted.

VTR-02S: Inexistent Sanitization of Input Addresses

Type	Severity	Location
Input Sanitization		V2ToV3RolloverBase.sol:L51-L60

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
51constructor(IVault _vault, OperationContracts memory _opContracts) {
52    // Set Balancer vault address
53    VAULT = _vault;
54
55    // Set lending protocol contract references
56    feeControllerV3 = IFeeController(_opContracts.feeControllerV3);
57    originationControllerV3 = IOriginationController(_opContracts.originationControllerV3);
58    loanCoreV3 = ILoanCore(_opContracts.loanCoreV3);
59    borrowerNoteV3 = IERC721(_opContracts.borrowerNoteV3);
60}

Recommendation:

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

Alleviation:

All input addresses of the V2ToV3RolloverBase::constructor are adequately sanitized as non-zero, preventing the contract from being misconfigured during its deployment and thus addressing this exhibit.

VTR-03S: Improper Invocation of EIP-20 transfer

Type	Severity	Location
Standard Conformity		V2ToV3RolloverBase.sol:L195

Description:

The linked statement does not properly validate the returned bool of the EIP-20 standard transfer function. As the standard dictates, callers must not assume that false is never returned.

Impact:

If the code mandates that the returned bool is true, this will cause incompatibility with tokens such as USDT / Tether as no such bool is returned to be evaluated causing the check to fail at all times. On the other hand, if the token utilized can return a false value under certain conditions but the code does not validate it, the contract itself can be compromised as having received / sent funds that it never did.

Example:

Copy
195token.transfer(to, balance);

Recommendation:

Since not all standardized tokens are EIP-20 compliant (such as Tether / USDT), we advise a safe wrapper library to be utilized instead such as SafeERC20 by OpenZeppelin to opportunistically validate the returned bool only if it exists.

Alleviation:

The SafeERC20::safeTransfer function is now properly utilized in place of the ERC20::transfer function invocation, ensuring that the asset is safely transferred to the VAULT and thus alleviating this exhibit.