code-423n4 / 2022-03-biconomy-findings

0 stars 0 forks source link

Gas Optimizations #155

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Public Functions That Can Be Set To External

Description Several functions across multiple contracts are marked public and can be marked external to save gas. They are as follows:

In LPToken.sol there are: setSvgHelper(), getAllNftIdsByUser(), tokenURI and supportsInterface().

In TokenManager.sol there are: getEquilibriumFee(), getMaxFee(), getTokensInfo(), getDepositConfig(), and getTransferConfig().

In SvgHelperBase.sol there are: setTokenDecimals(), getAttributes(), getDescription() and getTokenSvg().

In ExecutorManager.sol there are: getExecutorStatus() and getAllExecutors().

In LiquidityFarming.sol there are: setRewardPerSecond(), getNftIdsStaked() and getRewardRatePerSecond()

In LiquidityPool.sol there are: setTrustedForwarder(), getExecutorManager(), setLiquidityProviders() and setExecutorManager().

In LiquidityProviders.sol there are: getTotalReserveByToken(), getSuppliedLiquidityByToken(), getCurrentLiquidity(), increaseCurrentLiquidity(), decreaseCurrentLiquidity() and getFeeAccumulatedOnNft().

Recommendation Switch all of these functions from public to external to save gas on deployment.

Catching The Array Length Prior To Loop.

Description One can save gas by caching the array length (in stack) and using that set variable in the loop. They are as follows:

In ExecutorManager.sol there are: addExecutors(), removeExecutors().

In WhitelistPeriodManager.sol there is: setIsExcludedAddressStatus().

Recommendation For addExecutors(), removeExecutors() simply do something like so: uint length = executorArray.length; As for setIsExcludedAddressStatus() simply do something like so: uint length = _addresses.length;

Rearrangement of Order of Execution.

Description In LiquidityPool.sol By moving require(receiver != address(0), "Receiver address cannot be 0"); and require(amount != 0, "Amount cannot be 0"); to the start of the function. The function will revert before even reaching the more costly require.

Recommendation: Move require(receiver != address(0), "Receiver address cannot be 0"); and require(amount != 0, "Amount cannot be 0"); to the start of these functions depositErc20(), depositNative() and sendFundsToUser().

pauliax commented 2 years ago

1 is not valid, see: https://ethereum.stackexchange.com/a/107939/17387 2 is not valid, the variables come from calldata, not storage.