Description

The FCNVault contract contains three withdraw() functions that are not used.

The FCNVault contract also contains three redeem() functions. Two of the redeem() functions are wrappers. These are shown below:

function redeem(uint256 shares, address receiver, address owner) public onlyOwner returns (uint256) {
    uint256 assets = convertToAssets(shares);
    if (owner != msg.sender) {
        _spendAllowance(owner, msg.sender, shares);
    }
    _burn(owner, shares);
    emit Withdraw(msg.sender, receiver, owner, assets, shares);
    return assets;
}

function redeem(uint256 shares, address receiver) external onlyOwner returns (uint256) {
    return redeem(shares, receiver, msg.sender);
}

function redeem(uint256 shares) external onlyOwner returns (uint256) {
    return redeem(shares, msg.sender, msg.sender);
}

The last redeem() function is the only one that is called by the FCNProduct contract. This will result in a final call to the first redeem() function with the owner parameter set to msg.sender. This makes the owner != msg.sender if block in the first redeem() function unnecessary.

The FCNProduct contract contains a function called throwError(), which is not used. The contract itself also defines an error named FCNProductError, which is unused.

Impact

Unused or unreachable functionality adds to the complexity of the codebase, which might lead to programmer error in the future.

Recommendations

The withdraw() functions should be removed unless there are future plans to use them in the codebase.

The redeem() function should have the owner != msg.sender if block removed, as the condition will never be true.

The throwError() function and the corresponding FCNProductError should be removed unless there are future plans to use it.

Remediation

The client has remediated this issue by removing the use of the unused FCNProductError and throwError() function in commit 0bb7eb6b↗.

Unused functionality was removed from FCNVault in commit f4a93124↗.