FeeCalculatorFacet Static Analysis Findings

FCF-01S: Data Location Optimization

Description:

The signatures member of the setServiceFee function is set as memory while the function is external.

Example:

Copy
47function setServiceFee(uint256 _serviceFee, bytes[] memory signatures)
48    onlyValidSignatures(signatures.length)
49    external
50    override
51{
52    bytes32 ethHash = computeFeeUpdateMessage(_serviceFee);
53    LibDiamond.validateSignatures(ethHash, signatures);
54    LibFeeCalculator.Storage storage fcs = LibFeeCalculator.feeCalculatorStorage();
55    LibDiamond.DiamondStorage storage ds = LibDiamond.diamondStorage();
56    fcs.serviceFee = _serviceFee;
57    emit ServiceFeeSet(msg.sender, _serviceFee);
58    ds.administrativeNonce.increment();
59}

Recommendation:

We advise the array to be set as calldata greatly optimizing the gas cost of the function.

Alleviation:

The _signatures member of the function was properly set to calldata.

FCF-02S: Unused Return Value

Description:

The ERC20 transfer performed by claim does not evaluate the return bool value.

Example:

Copy
103function claim() external override onlyMember {
104    LibRouter.Storage storage rs = LibRouter.routerStorage();
105    uint256 claimableAmount = LibFeeCalculator.claimReward(msg.sender);
106    IERC20(rs.albtToken).transfer(msg.sender, claimableAmount);
107    emit Claim(msg.sender, claimableAmount);
108}

Recommendation:

Although the ALBT implementation would never return a false boolean under any circumstance, it is still best practice to use a safe wrapper implementation of ERC20, such as SafeERC20, to ensure return values are properly evaluated.

Alleviation:

The SafeERC20 implementation by OpenZeppelin was properly imported into the codebase and utilized for all linked transfer and / or transferFrom invocations.