Backd contest - Dravee's results

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/actions/topup/TopUpAction.sol#L291 https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/pool/EthPool.sol#L30 https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/strategies/BkdEthCvx.sol#L77 https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/strategies/BkdEthCvx.sol#L93 https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/strategies/BkdEthCvx.sol#L117 https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/vault/EthVault.sol#L29 https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/vault/EthVault.sol#L37 https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/vault/VaultReserve.sol#L81

Vulnerability details

This is a classic Code4rena issue:

• https://github.com/code-423n4/2021-04-meebits-findings/issues/2
• https://github.com/code-423n4/2021-10-tally-findings/issues/20
• https://github.com/code-423n4/2022-01-openleverage-findings/issues/75

Impact

The use of the deprecated transfer() function for an address will inevitably make the transaction fail when:

1. The claimer smart contract does not implement a payable function.
2. The claimer smart contract does implement a payable fallback which uses more than 2300 gas unit.
3. The claimer smart contract implements a payable fallback function that needs less than 2300 gas units but is called through proxy, raising the call's gas usage above 2300.

Additionally, using higher than 2300 gas might be mandatory for some multisig wallets.

Impacted lines:

backd/contracts/pool/EthPool.sol:
  30:         to.transfer(amount);

backd/contracts/strategies/BkdEthCvx.sol:
   77:             payable(vault).transfer(amount);
   93:         payable(vault).transfer(amount);
  117:         payable(vault).transfer(underlyingBalance);

backd/contracts/vault/EthVault.sol:
  29:         payable(to).transfer(amount); 
  37:         payable(addressProvider.getTreasury()).transfer(amount);  

backd/contracts/vault/VaultReserve.sol:
  81:             payable(msg.sender).transfer(amount);

Recommended Mitigation

I recommend using call() instead of transfer()

transfer method is used several places inside the codebase. Since transfer method uses 2300 gas stipend which is not adjustable,it may likely to get broken in future in case of hardforks causing gas price changes or when calling a contract's fallback function.
[Reference Link -1](https://swcregistry.io/docs/SWC-134), [Reference Link -2](https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/)

Originally submitted by warden Dravee in #146, duplicate of #178 related to the use of safeApprove. This is upgraded from a QA report to standalone issue because it correctly described the revert when trying to call safeApprove on non-zero allowance. QA report that only describe safeApprove as deprecated and unable to identify the revert problem are intentionally not upgraded. Duplicate of #180.

Deprecated safeApprove() function

Using this deprecated function can lead to unintended reverts and potentially the locking of funds. A deeper discussion on the deprecation of this function is in OZ issue #2219 (OpenZeppelin/openzeppelin-contracts#2219). The OpenZeppelin ERC20 safeApprove() function has been deprecated, as seen in the comments of the OpenZeppelin code.

As recommended by the OpenZeppelin comment, I suggest replacing safeApprove() with safeIncreaseAllowance() or safeDecreaseAllowance() instead:

backd/contracts/CvxCrvRewardsLocker.sol:
  53:         IERC20(CRV).safeApprove(CRV_DEPOSITOR, type(uint256).max);
  56:         IERC20(CVX_CRV).safeApprove(CVX_CRV_STAKING, type(uint256).max);
  59:         IERC20(CRV).safeApprove(CVX_CRV_CRV_CURVE_POOL, type(uint256).max);
  62:         IERC20(CVX).safeApprove(CVX_LOCKER, type(uint256).max);

backd/contracts/actions/topup/TopUpAction.sol:
   50:             IERC20(token).safeApprove(stakerVaultAddress, depositAmount);
  847:         IERC20(depositToken).safeApprove(feeHandler, feeAmount);
  908:         IERC20(token).safeApprove(spender, type(uint256).max);

backd/contracts/actions/topup/handlers/AaveHandler.sol:
  53:         IERC20(underlying).safeApprove(address(lendingPool), amount);

backd/contracts/actions/topup/handlers/CompoundHandler.sol:
   71:             IERC20(underlying).safeApprove(address(ctoken), amount);
  120:             IERC20(underlying).safeApprove(address(ctoken), debt);

backd/contracts/pool/LiquidityPool.sol:
  721:         IERC20(lpToken_).safeApprove(staker_, type(uint256).max);

backd/contracts/strategies/BkdEthCvx.sol:
  43:         IERC20(lp_).safeApprove(address(_BOOSTER), type(uint256).max);

backd/contracts/strategies/BkdTriHopCvx.sol:
   71:         IERC20(underlying_).safeApprove(curveHopPool_, type(uint256).max);
   72:         IERC20(hopLp_).safeApprove(curvePool_, type(uint256).max);
   73:         IERC20(lp_).safeApprove(address(_BOOSTER), type(uint256).max);
  129:         IERC20(hopLp).safeApprove(curvePool_, 0);
  130:         IERC20(hopLp).safeApprove(curvePool_, type(uint256).max);
  131:         IERC20(lp_).safeApprove(address(_BOOSTER), 0);
  132:         IERC20(lp_).safeApprove(address(_BOOSTER), type(uint256).max);

backd/contracts/strategies/ConvexStrategyBase.sol:
  107:         _CRV.safeApprove(address(_strategySwapper), type(uint256).max);
  108:         _CVX.safeApprove(address(_strategySwapper), type(uint256).max);
  109:         _WETH.safeApprove(address(_strategySwapper), type(uint256).max);
  279:         IERC20(token_).safeApprove(address(_strategySwapper), 0);
  280:         IERC20(token_).safeApprove(address(_strategySwapper), type(uint256).max);

backd/contracts/strategies/StrategySwapper.sol:
  209:         IERC20(token_).safeApprove(spender_, type(uint256).max);

backd/contracts/vault/Erc20Vault.sol:
  21:         IERC20(underlying_).safeApprove(address(reserve), type(uint256).max);
  22:         IERC20(underlying_).safeApprove(_pool, type(uint256).max);

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/oracles/ChainlinkOracleProvider.sol#L55-L58

Vulnerability details

Impact

Here, latestRoundData() is missing an additional validation to ensure that the round is complete.

Proof of Concept

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/oracles/ChainlinkOracleProvider.sol#L55-L58

File: ChainlinkOracleProvider.sol
51:     function getPriceUSD(address asset) public view override returns (uint256) {
...
55:         (, int256 answer, , uint256 updatedAt, ) = AggregatorV2V3Interface(feed).latestRoundData();
56: 
57:         require(block.timestamp <= updatedAt + stalePriceDelay, Error.STALE_PRICE);
58:         require(answer >= 0, Error.NEGATIVE_PRICE);

Tools Used

Manual code review. Chainlink best practices.

Recommended Mitigation Steps

Consider adding missing checks.

As an example:

        (uint80 roundID, int256 answer, , uint256 updatedAt, uint80 answeredInRound) = AggregatorV2V3Interface(feed).latestRoundData(); //@audit The updatedAt data feed property is the timestamp of an answered round and answeredInRound is the round it was updated in
        
        require(updatedAt > 0, Error.STALE_PRICE); //@audit A timestamp with zero value means the round is not complete and should not be used.
        require(answer > 0, Error.NEGATIVE_PRICE);
        require(answeredInRound >= roundID, Error.STALE_PRICE); //@audit If answeredInRound is less than roundId, the answer is being carried over. If answeredInRound is equal to roundId, then the answer is fresh.

• [L-01] Deprecated safeApprove() function
• [L-02] Immutable addresses should be 0-checked
• [N-01] Open TODOS
• [N-02] Code in comment
• [N-03] Related data should be grouped in a struct
• [N-04] Unused named returns
• [N-05] It's better to emit after all processing is done

[L-01] Deprecated safeApprove() function

Using this deprecated function can lead to unintended reverts and potentially the locking of funds. A deeper discussion on the deprecation of this function is in OZ issue #2219 (OpenZeppelin/openzeppelin-contracts#2219). The OpenZeppelin ERC20 safeApprove() function has been deprecated, as seen in the comments of the OpenZeppelin code.

As recommended by the OpenZeppelin comment, I suggest replacing safeApprove() with safeIncreaseAllowance() or safeDecreaseAllowance() instead:

backd/contracts/CvxCrvRewardsLocker.sol:
  53:         IERC20(CRV).safeApprove(CRV_DEPOSITOR, type(uint256).max);
  56:         IERC20(CVX_CRV).safeApprove(CVX_CRV_STAKING, type(uint256).max);
  59:         IERC20(CRV).safeApprove(CVX_CRV_CRV_CURVE_POOL, type(uint256).max);
  62:         IERC20(CVX).safeApprove(CVX_LOCKER, type(uint256).max);

backd/contracts/actions/topup/TopUpAction.sol:
   50:             IERC20(token).safeApprove(stakerVaultAddress, depositAmount);
  847:         IERC20(depositToken).safeApprove(feeHandler, feeAmount);
  908:         IERC20(token).safeApprove(spender, type(uint256).max);

backd/contracts/actions/topup/handlers/AaveHandler.sol:
  53:         IERC20(underlying).safeApprove(address(lendingPool), amount);

backd/contracts/actions/topup/handlers/CompoundHandler.sol:
   71:             IERC20(underlying).safeApprove(address(ctoken), amount);
  120:             IERC20(underlying).safeApprove(address(ctoken), debt);

backd/contracts/pool/LiquidityPool.sol:
  721:         IERC20(lpToken_).safeApprove(staker_, type(uint256).max);

backd/contracts/strategies/BkdEthCvx.sol:
  43:         IERC20(lp_).safeApprove(address(_BOOSTER), type(uint256).max);

backd/contracts/strategies/BkdTriHopCvx.sol:
   71:         IERC20(underlying_).safeApprove(curveHopPool_, type(uint256).max);
   72:         IERC20(hopLp_).safeApprove(curvePool_, type(uint256).max);
   73:         IERC20(lp_).safeApprove(address(_BOOSTER), type(uint256).max);
  129:         IERC20(hopLp).safeApprove(curvePool_, 0);
  130:         IERC20(hopLp).safeApprove(curvePool_, type(uint256).max);
  131:         IERC20(lp_).safeApprove(address(_BOOSTER), 0);
  132:         IERC20(lp_).safeApprove(address(_BOOSTER), type(uint256).max);

backd/contracts/strategies/ConvexStrategyBase.sol:
  107:         _CRV.safeApprove(address(_strategySwapper), type(uint256).max);
  108:         _CVX.safeApprove(address(_strategySwapper), type(uint256).max);
  109:         _WETH.safeApprove(address(_strategySwapper), type(uint256).max);
  279:         IERC20(token_).safeApprove(address(_strategySwapper), 0);
  280:         IERC20(token_).safeApprove(address(_strategySwapper), type(uint256).max);

backd/contracts/strategies/StrategySwapper.sol:
  209:         IERC20(token_).safeApprove(spender_, type(uint256).max);

backd/contracts/vault/Erc20Vault.sol:
  21:         IERC20(underlying_).safeApprove(address(reserve), type(uint256).max);
  22:         IERC20(underlying_).safeApprove(_pool, type(uint256).max);

[L-02] Immutable addresses should be 0-checked

In the constructors, the solution never checks for address(0) when setting the value of immutable variables. I suggest adding those checks.

List of immutable variables:

backd/contracts/BkdLocker.sol:
  41:     IERC20 public immutable govToken;

backd/contracts/Controller.sol:
  19:     IAddressProvider public immutable override addressProvider;

backd/contracts/GasBank.sol:
   9:     IController public immutable controller;
  10:     IAddressProvider public immutable addressProvider;

backd/contracts/StakerVault.sol:
  43:     IController public immutable controller;

backd/contracts/access/Authorization.sol:
  7:     IRoleManager internal immutable __roleManager;

backd/contracts/access/RoleManager.sol:
  23:     IAddressProvider public immutable addressProvider;

backd/contracts/actions/topup/TopUpAction.sol:
  156:     IController public immutable controller;
  157:     IAddressProvider public immutable addressProvider;

backd/contracts/actions/topup/TopUpActionFeeHandler.sol:
  31:     address public immutable actionContract;
  32:     IController public immutable controller;

backd/contracts/actions/topup/TopUpKeeperHelper.sol:
  18:     ITopUpAction private immutable _topupAction;

backd/contracts/actions/topup/handlers/AaveHandler.sol:
  21:     ILendingPool public immutable lendingPool;
  22:     IWETH public immutable weth;

backd/contracts/actions/topup/handlers/CompoundHandler.sol:
  35:     Comptroller public immutable comptroller;
  36:     ICTokenRegistry public immutable cTokenRegistry;

backd/contracts/actions/topup/handlers/CTokenRegistry.sol:
  9:     Comptroller public immutable comptroller;

backd/contracts/oracles/ChainlinkUsdWrapper.sol:
  27:     IChainlinkOracle private immutable _ethOracle =
  29:     IChainlinkOracle private immutable _oracle;
  30:     uint8 private immutable _decimals;

backd/contracts/pool/LiquidityPool.sol:
   65:     IController public immutable controller;
   66:     IAddressProvider public immutable addressProvider;

backd/contracts/pool/PoolFactory.sol:
  64:     IController public immutable controller;
  65:     IAddressProvider public immutable addressProvider;

backd/contracts/strategies/BkdTriHopCvx.sol:
  19:     ICurveSwapEth public immutable curveHopPool; // Curve Pool to use for Hops
  20:     IERC20 public immutable hopLp; // Curve Hop Pool LP Token
  21:     uint256 public immutable curveHopIndex; // Underlying index in Curve Pool

backd/contracts/strategies/ConvexStrategyBase.sol:
  43:     IStrategySwapper internal immutable _strategySwapper;
  50:     address public immutable vault; // Backd Vault

backd/contracts/strategies/StrategySwapper.sol:
  28:     IAddressProvider internal immutable _addressProvider; // Address provider used for getting oracle provider

backd/contracts/vault/Vault.sol:
  48:     IController public immutable controller;
  49:     IAddressProvider public immutable addressProvider;
  50:     IVaultReserve public immutable reserve;

[N-01] Open TODOS

Consider resolving the TODOs before deploying.

actions/topup/TopUpAction.sol:713:        // TODO: add constant gas consumed for transfer and tx prologue
strategies/ConvexStrategyBase.sol:4:// TODO Add validation of curve pools
strategies/ConvexStrategyBase.sol:5:// TODO Test validation

[N-02] Code in comment

Consider deleting the following line:

File: ILendingPool.sol
408:     // function getAddressesProvider() external view returns (ILendingPoolAddressesProvider);

[N-03] Related data should be grouped in a struct

The "LP Fee Distribution" maps should be grouped in a struct.

From:

File: BkdLocker.sol
25:     // User-specific data
26:     mapping(address => uint256) public balances;
27:     mapping(address => uint256) public boostFactors;
28:     mapping(address => uint256) public lastUpdated;
29:     mapping(address => WithdrawStash[]) public stashedGovTokens;
30:     mapping(address => uint256) public totalStashed;

To

    struct UserSpecificData {
        uint256 balances;
        uint256 boostFactors;
        uint256 lastUpdated;
        WithdrawStash[] stashedGovTokens;
        uint256 totalStashed;
    }
    
    mapping(address => UserSpecificData) public userSpecificData;

It would be less error-prone, more readable, and it would be possible to delete all related fields with a simple delete userSpecificData[address].

[N-04] Unused named returns

Several functions are declaring named returns but then are using return statements. I suggest choosing only one for readability reasons:

backd/contracts/actions/topup/TopUpAction.sol:
  497:         returns (address[] memory users, uint256 nextCursor) //@audit unused named returns
  761:     ) public view override returns (uint256 healthFactor) { //@audit unused named returns

backd/contracts/pool/PoolFactory.sol:
  155:     ) external onlyGovernance returns (Addresses memory addrs) { //@audit unused named returns
  216:         return addrs; //@audit unused named returns

backd/contracts/strategies/StrategySwapper.sol:
  301:         returns (uint256 wethIndex_, uint256 tokenIndex_) //@audit unused named returns
  332:         returns (uint256 minAmountOut) //@audit unused named returns

[N-05] It's better to emit after all processing is done

Instances include:

File: Vault.sol
565:     function _activateStrategy() internal returns (bool) {
566:         IStrategy strategy = getStrategy();
567:         if (address(strategy) == address(0)) return false;
568: 
569:         strategyActive = true;
570:         emit StrategyActivated(address(strategy));
571:         _deposit();
572:         return true;
573:     }

Table of Contents:

• Unchecking arithmetics operations that can't underflow/overflow
• No need to explicitly initialize variables with default values
• Tighly pack storage variables
• Caching storage values in memory
• > 0 is less efficient than != 0 for unsigned integers (with proof)
• >= is cheaper than >
• An array's length should be cached to save gas in for-loops
• ++i costs less gas compared to i++ or i += 1
• Increments can be unchecked
• Reduce the size of error messages (Long revert Strings)
• Use Custom Errors instead of Revert Strings to save Gas

Unchecking arithmetics operations that can't underflow/overflow

Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn't possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked block: https://docs.soliditylang.org/en/v0.8.10/control-structures.html#checked-or-unchecked-arithmetic

I suggest wrapping with an unchecked block here (see @audit tags for more details):

backd/contracts/BkdLocker.sol:
  139:             i = i - 1;  //@audit gas: should be unchecked (can't underflow)

backd/contracts/GasBank.sol:
  48:             _withdrawFrom(account, payable(account), currentBalance - ethRequired, currentBalance); //@audit gas: should be unchecked (can't underflow)
  76:             require(currentBalance - amount >= ethRequired, Error.NOT_ENOUGH_FUNDS); //@audit gas: should be unchecked (can't underflow)

backd/contracts/StakerVault.sol:
  112:         balances[msg.sender] -= amount; //@audit gas: should be unchecked (can't underflow)
  163:         uint256 allowanceNew = startingAllowance - amount; //@audit gas: should be unchecked (can't underflow)
  164:         uint256 srcTokensNew = srcTokens - amount; //@audit gas: should be unchecked (can't underflow)
  231:             actionLockedBalances[account] -= amount; //@audit gas: should be unchecked (can't underflow)

backd/contracts/actions/topup/TopUpAction.sol:
  501:         if (howMany >= length - cursor) { //@audit gas: should be unchecked (can't underflow)
  502:             howMany = length - cursor; //@audit gas: should be unchecked (can't underflow)

backd/contracts/pool/LiquidityPool.sol:
  446:             staker.unstakeFor(msg.sender, msg.sender, redeemLpTokens - lpBalance_); //@audit gas: should be unchecked (can't underflow)

backd/contracts/strategies/BkdEthCvx.sol:
  83:         uint256 requiredUnderlyingAmount = amount - underlyingBalance; //@audit gas: should be unchecked (can't underflow)

backd/contracts/strategies/BkdTriHopCvx.sol:
  181:         uint256 requiredUnderlyingAmount = amount - underlyingBalance; //@audit gas: should be unchecked (can't underflow)

backd/contracts/vault/Vault.sol:
  440:             waitingForRemovalAllocated = _waitingForRemovalAllocated - withdrawn;  //@audit gas: should be unchecked (can't underflow)
  444:             uint256 profit = withdrawn - allocated; //@audit gas: should be unchecked (can't underflow)
  452:             allocated -= withdrawn; //@audit gas: should be unchecked (can't underflow)
  482:         if (totalDebt <= netUnderlying) return netUnderlying - totalDebt; //@audit gas: should be unchecked (can't underflow)
  591:             uint256 profit = allocatedUnderlying - amountAllocated; //@audit gas: should be unchecked (can't underflow)
  595:                     profit -= currentDebt; //@audit gas: should be unchecked (can't underflow)
  600:                 currentDebt -= profit; //@audit gas: should be unchecked (can't underflow)
  605:             uint256 loss = amountAllocated - allocatedUnderlying; //@audit gas: should be unchecked (can't underflow)
  756:             return allocated - withdrawn; //@audit gas: should be unchecked (can't underflow)

backd/contracts/vault/VaultReserve.sol:
  77:         _balances[msg.sender][token] -= amount;  //@audit gas: should be unchecked (can't underflow)

No need to explicitly initialize variables with default values

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address...). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

As an example: for (uint256 i = 0; i < numIterations; ++i) { should be replaced with for (uint256 i; i < numIterations; ++i) {

Instances include:

access/RoleManager.sol:80:        for (uint256 i = 0; i < roles.length; i++) {
actions/topup/handlers/CompoundHandler.sol:135:        for (uint256 i = 0; i < assets.length; i++) {
actions/topup/handlers/CTokenRegistry.sol:61:        for (uint256 i = 0; i < ctokens.length; i++) {
actions/topup/TopUpAction.sol:188:        for (uint256 i = 0; i < protocols.length; i++) {
actions/topup/TopUpAction.sol:452:        uint256 totalEthRequired = 0;
actions/topup/TopUpAction.sol:456:        for (uint256 i = 0; i < length; i++) {
actions/topup/TopUpAction.sol:479:        for (uint256 i = 0; i < length; i++) {
actions/topup/TopUpAction.sol:506:        for (uint256 i = 0; i < howMany; i++) {
actions/topup/TopUpAction.sol:891:        for (uint256 i = 0; i < length; i++) {
actions/topup/TopUpKeeperHelper.sol:43:            for (uint256 i = 0; i < users.length; i++) {
actions/topup/TopUpKeeperHelper.sol:46:                for (uint256 j = 0; j < positions.length; j++) {
actions/topup/TopUpKeeperHelper.sol:72:        for (uint256 i = 0; i < keys.length; i++) {
actions/topup/TopUpKeeperHelper.sol:93:        for (uint256 i = 0; i < length; i++) {
actions/topup/TopUpKeeperHelper.sol:165:        for (uint256 i = 0; i < length; i++) {
pool/LiquidityPool.sol:483:        uint256 currentFeeRatio = 0;
strategies/ConvexStrategyBase.sol:313:        for (uint256 i = 0; i < _rewardTokens.length(); i++) {
strategies/ConvexStrategyBase.sol:380:        for (uint256 i = 0; i < _rewardTokens.length(); i++) {
vault/EthVault.sol:9:    address private constant _UNDERLYING = address(0);
vault/Vault.sol:42:    uint256 internal constant _INITIAL_PERFORMANCE_FEE = 0;
vault/Vault.sol:135:            uint256 allocatedUnderlying = 0;
vault/Vault.sol:583:        uint256 strategistShare = 0;
BkdLocker.sol:133:        uint256 totalAvailableToWithdraw = 0;
BkdLocker.sol:310:            for (uint256 i = 0; i < length; i++) {
Controller.sol:114:        uint256 totalEthRequired = 0;
Controller.sol:117:        for (uint256 i = 0; i < numActions; i++) {
CvxCrvRewardsLocker.sol:43:    int128 private constant _CRV_INDEX = 0;
StakerVault.sol:144:        uint256 startingAllowance = 0;
StakerVault.sol:260:        for (uint256 i = 0; i < actions.length; i++) {

I suggest removing explicit initializations for default values.

Tighly pack storage variables

In VaultStorage.sol, the variables can be tightly packed from:

File: VaultStorage.sol
11:     address public pool; //@audit gas: 20 bytes
12: 
13:     uint256 public totalDebt; //@audit gas: 32 bytes
14:     bool public strategyActive; //@audit gas: 1 byte => can be tightly packed by being moved up

to

File: VaultStorage.sol
11:     address public pool; //@audit gas: 20 bytes
12: 
13:     bool public strategyActive; //@audit gas: 1 byte => 1 SLOT saved
14:     uint256 public totalDebt; //@audit gas: 32 bytes

Which would save 1 storage slot.

Caching storage values in memory

The code can be optimized by minimising the number of SLOADs. SLOADs are expensive (100 gas) compared to MLOADs/MSTOREs (3 gas). Here, storage values should get cached in memory (see the @audit tags for further details):

backd/contracts/StakerVault.sol:
  324:         require(IERC20(token).balanceOf(msg.sender) >= amount, Error.INSUFFICIENT_BALANCE);//@audit cache token
  331:         uint256 oldBal = IERC20(token).balanceOf(address(this));//@audit use cache token
  334:             ILiquidityPool pool = controller.addressProvider().getPoolForToken(token);//@audit use cache token
  338:         IERC20(token).safeTransferFrom(msg.sender, address(this), amount);//@audit use cache token
  339:         uint256 staked = IERC20(token).balanceOf(address(this)) - oldBal;//@audit use cache token
  365:         ILiquidityPool pool = controller.addressProvider().getPoolForToken(token); //@audit cache token
  376:         uint256 oldBal = IERC20(token).balanceOf(address(this)); //@audit use cache token
  382:         IERC20(token).safeTransfer(dst, amount);//@audit use cache token
  384:         uint256 unstaked = oldBal - IERC20(token).balanceOf(address(this));//@audit use cache token

backd/contracts/pool/LiquidityPool.sol:
  156:             msg.sender == address(lpToken) || msg.sender == address(staker), //@audit gas: cache lpToken
  160:             addressProvider.isStakerVault(to, address(lpToken)) || //@audit gas: use cache lpToken
  161:             addressProvider.isStakerVault(from, address(lpToken)) || //@audit gas: use cache lpToken
  442:             lpBalance_ + staker.balanceOf(msg.sender) >= redeemLpTokens, //@audit gas: cache staker
  446:             staker.unstakeFor(msg.sender, msg.sender, redeemLpTokens - lpBalance_); //@audit gas: use cache staker

backd/contracts/vault/Vault.sol:
  658:         uint256 newDebt = _handleExcessDebt(totalDebt); //@audit gas: use cache of totalDebt (variable currentDebt)

> 0 is less efficient than != 0 for unsigned integers (with proof)

!= 0 costs less gas compared to > 0 for unsigned integers in require statements with the optimizer enabled (6 gas)

Proof: While it may seem that > 0 is cheaper than !=, this is only true without the optimizer enabled and outside a require statement. If you enable the optimizer at 10k AND you're in a require statement, this will save gas. You can see this tweet for more proofs: https://twitter.com/gzeon/status/1485428085885640706

I suggest changing > 0 with != 0 here:

actions/topup/TopUpAction.sol:210:        require(record.singleTopUpAmount > 0, Error.INVALID_AMOUNT);
actions/topup/TopUpAction.sol:554:        require(position.totalTopUpAmount > 0, Error.INSUFFICIENT_BALANCE);
actions/topup/TopUpActionFeeHandler.sol:123:        require(totalClaimable > 0, Error.NOTHING_TO_CLAIM);
pool/LiquidityPool.sol:401:        require(_depositCap > 0, Error.INVALID_AMOUNT);
pool/LiquidityPool.sol:471:        require(underlyingAmount > 0, Error.INVALID_AMOUNT);
pool/LiquidityPool.sol:473:        require(lpToken_.balanceOf(account) > 0, Error.INSUFFICIENT_BALANCE);
pool/LiquidityPool.sol:549:        require(redeemLpTokens > 0, Error.INVALID_AMOUNT);
vault/Vault.sol:164:        require(amount > 0, Error.INVALID_AMOUNT);
BkdLocker.sol:90:        require(amount > 0, Error.INVALID_AMOUNT);
BkdLocker.sol:91:        require(totalLockedBoosted > 0, Error.NOT_ENOUGH_FUNDS);
BkdLocker.sol:136:        require(length > 0, "No entries");

Also, please enable the Optimizer.

>= is cheaper than >

Strict inequalities (>) are more expensive than non-strict ones (>=). This is due to some supplementary checks (ISZERO, 3 gas)

I suggest using >= instead of > to avoid some opcodes here:

actions/topup/TopUpAction.sol:48:            depositAmount = depositAmount > amountLeft ? amountLeft : depositAmount;
actions/topup/TopUpAction.sol:60:            uint256 availableFunds = balance < allowance ? balance : allowance;
vault/Vault.sol:780:        upperBound = upperBound > ScaledMath.ONE ? ScaledMath.ONE : upperBound;
vault/Vault.sol:781:        uint256 lowerBound = bound > targetAllocation ? 0 : targetAllocation - bound;

An array's length should be cached to save gas in for-loops

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.

Caching the array length in the stack saves around 3 gas per iteration.

Here, I suggest storing the array's length in a variable before the for-loop, and use it instead:

access/RoleManager.sol:80:        for (uint256 i = 0; i < roles.length; i++) {
actions/topup/handlers/CompoundHandler.sol:135:        for (uint256 i = 0; i < assets.length; i++) {
actions/topup/handlers/CTokenRegistry.sol:61:        for (uint256 i = 0; i < ctokens.length; i++) {
actions/topup/TopUpAction.sol:188:        for (uint256 i = 0; i < protocols.length; i++) {
actions/topup/TopUpKeeperHelper.sol:43:            for (uint256 i = 0; i < users.length; i++) {
actions/topup/TopUpKeeperHelper.sol:46:                for (uint256 j = 0; j < positions.length; j++) {
actions/topup/TopUpKeeperHelper.sol:72:        for (uint256 i = 0; i < keys.length; i++) {
strategies/ConvexStrategyBase.sol:313:        for (uint256 i = 0; i < _rewardTokens.length(); i++) {
strategies/ConvexStrategyBase.sol:380:        for (uint256 i = 0; i < _rewardTokens.length(); i++) {
StakerVault.sol:260:        for (uint256 i = 0; i < actions.length; i++) {

++i costs less gas compared to i++ or i += 1

++i costs less gas compared to i++ or i += 1 for unsigned integer, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.

i++ increments i and returns the initial value of i. Which means:

uint i = 1;  
i++; // == 1 but i == 2  

But ++i returns the actual incremented value:

uint i = 1;  
++i; // == 2 and i == 2 too, so no need for a temporary variable  

In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2

Instances include:

access/RoleManager.sol:80:        for (uint256 i = 0; i < roles.length; i++) {
actions/topup/handlers/CompoundHandler.sol:135:        for (uint256 i = 0; i < assets.length; i++) {
actions/topup/handlers/CTokenRegistry.sol:61:        for (uint256 i = 0; i < ctokens.length; i++) {
actions/topup/TopUpAction.sol:188:        for (uint256 i = 0; i < protocols.length; i++) {
actions/topup/TopUpAction.sol:456:        for (uint256 i = 0; i < length; i++) {
actions/topup/TopUpAction.sol:479:        for (uint256 i = 0; i < length; i++) {
actions/topup/TopUpAction.sol:506:        for (uint256 i = 0; i < howMany; i++) {
actions/topup/TopUpAction.sol:891:        for (uint256 i = 0; i < length; i++) {
actions/topup/TopUpKeeperHelper.sol:43:            for (uint256 i = 0; i < users.length; i++) {
actions/topup/TopUpKeeperHelper.sol:46:                for (uint256 j = 0; j < positions.length; j++) {
actions/topup/TopUpKeeperHelper.sol:50:                    topupsAdded++;
actions/topup/TopUpKeeperHelper.sol:72:        for (uint256 i = 0; i < keys.length; i++) {
actions/topup/TopUpKeeperHelper.sol:93:        for (uint256 i = 0; i < length; i++) {
actions/topup/TopUpKeeperHelper.sol:165:        for (uint256 i = 0; i < length; i++) {
strategies/ConvexStrategyBase.sol:313:        for (uint256 i = 0; i < _rewardTokens.length(); i++) {
strategies/ConvexStrategyBase.sol:380:        for (uint256 i = 0; i < _rewardTokens.length(); i++) {
BkdLocker.sol:310:            for (uint256 i = 0; i < length; i++) {
Controller.sol:117:        for (uint256 i = 0; i < numActions; i++) {
StakerVault.sol:260:        for (uint256 i = 0; i < actions.length; i++) {

I suggest using ++i instead of i++ to increment the value of an uint variable.

Increments can be unchecked

In Solidity 0.8+, there's a default overflow check on unsigned integers. It's possible to uncheck this in for-loops and save some gas at each iteration, but at the cost of some code readability, as this uncheck cannot be made inline.

ethereum/solidity#10695

Instances include:

access/RoleManager.sol:80:        for (uint256 i = 0; i < roles.length; i++) {
actions/topup/handlers/CompoundHandler.sol:135:        for (uint256 i = 0; i < assets.length; i++) {
actions/topup/handlers/CTokenRegistry.sol:61:        for (uint256 i = 0; i < ctokens.length; i++) {
actions/topup/TopUpAction.sol:188:        for (uint256 i = 0; i < protocols.length; i++) {
actions/topup/TopUpAction.sol:456:        for (uint256 i = 0; i < length; i++) {
actions/topup/TopUpAction.sol:479:        for (uint256 i = 0; i < length; i++) {
actions/topup/TopUpAction.sol:506:        for (uint256 i = 0; i < howMany; i++) {
actions/topup/TopUpAction.sol:891:        for (uint256 i = 0; i < length; i++) {
actions/topup/TopUpKeeperHelper.sol:43:            for (uint256 i = 0; i < users.length; i++) {
actions/topup/TopUpKeeperHelper.sol:46:                for (uint256 j = 0; j < positions.length; j++) {
actions/topup/TopUpKeeperHelper.sol:72:        for (uint256 i = 0; i < keys.length; i++) {
actions/topup/TopUpKeeperHelper.sol:93:        for (uint256 i = 0; i < length; i++) {
actions/topup/TopUpKeeperHelper.sol:165:        for (uint256 i = 0; i < length; i++) {
strategies/ConvexStrategyBase.sol:313:        for (uint256 i = 0; i < _rewardTokens.length(); i++) {
strategies/ConvexStrategyBase.sol:380:        for (uint256 i = 0; i < _rewardTokens.length(); i++) {
BkdLocker.sol:310:            for (uint256 i = 0; i < length; i++) {
Controller.sol:117:        for (uint256 i = 0; i < numActions; i++) {
StakerVault.sol:260:        for (uint256 i = 0; i < actions.length; i++) {

The code would go from:

for (uint256 i; i < numIterations; i++) {  
 // ...  
}  

to:

for (uint256 i; i < numIterations;) {  
 // ...  
 unchecked { ++i; }  
}  

The risk of overflow is inexistant for a uint256 here.

Reduce the size of error messages (Long revert Strings)

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

Revert string > 32 bytes:

vault/Vault.sol:764:            "sum of strategist fee and reserve fee should be below 1" 

I suggest shortening the revert strings to fit in 32 bytes, or using custom errors as described next.

Use Custom Errors instead of Revert Strings to save Gas

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met)

Source: https://blog.soliditylang.org/2021/04/21/custom-errors/:

Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them.

Custom errors are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries).

Instances include:

181 results - 28 files

backd/contracts/AddressProvider.sol:
   64:         require(!_whiteListedFeeHandlers.contains(feeHandler), Error.ADDRESS_WHITELISTED);
   70:         require(_whiteListedFeeHandlers.contains(feeHandler), Error.ADDRESS_NOT_WHITELISTED);
   96:         require(pool != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);
  100:         require(poolToken != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);
  170:         require(_addressKeyMetas.contains(key), Error.ADDRESS_DOES_NOT_EXIST);
  179:         require(!checkExists || _addressKeyMetas.contains(key), Error.ADDRESS_DOES_NOT_EXIST);
  188:         require(exists, Error.ADDRESS_DOES_NOT_EXIST);
  230:         require(!meta.frozen, Error.ADDRESS_FROZEN);
  231:         require(meta.freezable, Error.INVALID_ARGUMENT);
  249:         require(!meta.frozen, Error.ADDRESS_FROZEN);
  259:         require(!meta.frozen, Error.ADDRESS_FROZEN);
  284:         require(token != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);
  285:         require(!_stakerVaults.contains(token), Error.STAKER_VAULT_EXISTS);
  314:         require(exists, Error.ADDRESS_NOT_FOUND);
  417:         require(!_addressKeyMetas.contains(key), Error.INVALID_ARGUMENT);
  423:         require(_addressKeyMetas.set(key, meta.toUInt()), Error.INVALID_ARGUMENT);

backd/contracts/BkdLocker.sol:
   58:         require(currentUInts256[_START_BOOST] == 0, Error.CONTRACT_INITIALIZED);
   90:         require(amount > 0, Error.INVALID_AMOUNT);
   91:         require(totalLockedBoosted > 0, Error.NOT_ENOUGH_FUNDS);
  118:         require(
  136:         require(length > 0, "No entries");
  207:         require(

backd/contracts/Controller.sol:
  32:         require(address(inflationManager) == address(0), Error.ADDRESS_ALREADY_SET);
  33:         require(_inflationManager != address(0), Error.INVALID_ARGUMENT);
  80:         require(addressProvider.getBKDLocker() != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);

backd/contracts/CvxCrvRewardsLocker.sol:
   83:         require(
  135:             require(!prepareWithdrawal, Error.PREPARED_WITHDRAWAL);

backd/contracts/GasBank.sol:
  42:         require(
  68:         require(currentBalance >= amount, Error.NOT_ENOUGH_FUNDS);
  69:         require(
  76:             require(currentBalance - amount >= ethRequired, Error.NOT_ENOUGH_FUNDS);
  91:         require(success, Error.FAILED_TRANSFER);

backd/contracts/LpToken.sol:
  22:         require(msg.sender == minter, Error.UNAUTHORIZED_ACCESS);
  34:         require(_minter != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);

backd/contracts/StakerVault.sol:
   61:         require(address(_controller) != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);
   70:         require(currentAddresses[_LP_GAUGE] == address(0), Error.ROLE_EXISTS);
   93:         require(msg.sender == address(controller.inflationManager()), Error.UNAUTHORIZED_ACCESS);
  106:         require(msg.sender != account, Error.SELF_TRANSFER_NOT_ALLOWED);
  107:         require(balances[msg.sender] >= amount, Error.INSUFFICIENT_BALANCE);
  139:         require(src != dst, Error.SAME_ADDRESS_NOT_ALLOWED);
  150:         require(startingAllowance >= amount, Error.INSUFFICIENT_BALANCE);
  153:         require(srcTokens >= amount, Error.INSUFFICIENT_BALANCE);
  203:         require(controller.addressProvider().isAction(msg.sender), Error.UNAUTHORIZED_ACCESS);
  224:         require(controller.addressProvider().isAction(msg.sender), Error.UNAUTHORIZED_ACCESS);
  324:         require(IERC20(token).balanceOf(msg.sender) >= amount, Error.INSUFFICIENT_BALANCE);
  340:         require(staked == amount, Error.INVALID_AMOUNT);
  367:         require(
  371:         require(balances[src] >= amount, Error.INSUFFICIENT_BALANCE);

backd/contracts/access/AuthorizationBase.sol:
  17:         require(_roleManager().hasRole(role, msg.sender), Error.UNAUTHORIZED_ACCESS);
  25:         require(_roleManager().hasRole(Roles.GOVERNANCE, msg.sender), Error.UNAUTHORIZED_ACCESS);
  33:         require(_roleManager().hasAnyRole(role1, role2, msg.sender), Error.UNAUTHORIZED_ACCESS);
  45:         require(

backd/contracts/access/RoleManager.sol:
   26:         require(hasRole(Roles.GOVERNANCE, msg.sender), Error.UNAUTHORIZED_ACCESS);
   44:         require(getRoleMemberCount(Roles.GOVERNANCE) > 1, Error.CANNOT_REVOKE_ROLE);
  110:         require(role != Roles.GOVERNANCE, Error.CANNOT_REVOKE_ROLE);
  111:         require(hasRole(role, account), Error.INVALID_ARGUMENT);

backd/contracts/actions/topup/TopUpAction.sol:
   67:         require(amountLeft == 0, Error.INSUFFICIENT_UPDATE_BALANCE);
   98:         require(swapper != address(0), Error.SWAP_PATH_NOT_FOUND);
  185:         require(protocols.length == handlers.length, Error.INVALID_ARGUMENT);
  209:         require(_supportedProtocols.contains(protocol), Error.PROTOCOL_NOT_FOUND);
  210:         require(record.singleTopUpAmount > 0, Error.INVALID_AMOUNT);
  211:         require(record.threshold > ScaledMath.ONE, Error.INVALID_AMOUNT);
  212:         require(record.singleTopUpAmount <= record.totalTopUpAmount, Error.INVALID_AMOUNT);
  213:         require(
  217:         require(_isSwappable(record.depositToken, record.actionToken), Error.SWAP_PATH_NOT_FOUND);
  218:         require(isUsable(record.depositToken), Error.TOKEN_NOT_USABLE);
  224:         require(msg.value >= gasDeposit, Error.VALUE_TOO_LOW_FOR_GAS);
  282:         require(position.threshold != 0, Error.NO_POSITION_EXISTS);
  328:         require(newActionFee <= _MAX_ACTION_FEE, Error.INVALID_AMOUNT);
  359:         require(
  546:         require(controller.canKeeperExecuteAction(msg.sender), Error.NOT_ENOUGH_BKD_STAKED);
  553:         require(position.threshold != 0, Error.NO_POSITION_EXISTS);
  554:         require(position.totalTopUpAmount > 0, Error.INSUFFICIENT_BALANCE);
  560:         require(vars.userFactor < position.threshold, Error.INSUFFICIENT_THRESHOLD);
  575:         require(
  583:         require(
  676:         require(vars.success && abi.decode(vars.topupResult, (bool)), Error.TOP_UP_FAILED);
  723:         require(
  928:         require(!ensureExists || handler != address(0), Error.PROTOCOL_NOT_FOUND);

backd/contracts/actions/topup/TopUpActionFeeHandler.sol:
   54:         require(keeperFee + treasuryFee <= ScaledMath.ONE, Error.INVALID_AMOUNT);
   67:         require(getKeeperGauge(lpToken) == address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);
   68:         require(_keeperGauge != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);
   87:         require(msg.sender == actionContract, Error.UNAUTHORIZED_ACCESS);
  123:         require(totalClaimable > 0, Error.NOTHING_TO_CLAIM);
  151:         require(newKeeperFee <= ScaledMath.ONE, Error.INVALID_AMOUNT);
  161:         require(
  196:         require(newTreasuryFee <= ScaledMath.ONE, Error.INVALID_AMOUNT);
  206:         require(

backd/contracts/actions/topup/handlers/AaveHandler.sol:
  51:         require(reserve.aTokenAddress != address(0), Error.UNDERLYING_NOT_SUPPORTED);

backd/contracts/actions/topup/handlers/CompoundHandler.sol:
   74:         require(err == 0, Error.FAILED_MINT);
   80:         require(success, Error.FAILED_TRANSFER);
  123:         require(err == 0, Error.FAILED_REPAY_BORROW);
  141:             require(oErr == 0, Error.FAILED_METHOD_CALL);
  148:             require(vars.oraclePriceMantissa != 0, Error.FAILED_METHOD_CALL);

backd/contracts/oracles/ChainlinkOracleProvider.sol:
  31:         require(feed != previousFeed, Error.INVALID_ARGUMENT);
  41:         require(stalePriceDelay_ >= 1 hours, Error.INVALID_ARGUMENT);
  53:         require(feed != address(0), Error.ASSET_NOT_SUPPORTED);
  57:         require(block.timestamp <= updatedAt + stalePriceDelay, Error.STALE_PRICE);
  58:         require(answer >= 0, Error.NEGATIVE_PRICE);

backd/contracts/pool/Erc20Pool.sol:
  20:         require(underlying_ != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);
  30:         require(msg.value == 0, Error.INVALID_VALUE);

backd/contracts/pool/EthPool.sol:
  25:         require(msg.sender == from, Error.INVALID_SENDER);
  26:         require(msg.value == amount, Error.INVALID_AMOUNT);

backd/contracts/pool/LiquidityPool.sol:
   76:         require(address(_controller) != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);
  136:         require(address(lpToken) == address(0), Error.ADDRESS_ALREADY_SET);
  137:         require(ILpToken(_lpToken).minter() == address(this), Error.INVALID_MINTER);
  155:         require(
  179:         require(_newRatio <= ScaledMath.ONE, Error.INVALID_AMOUNT);
  208:         require(newRatio <= (ScaledMath.DECIMAL_SCALE * 50) / 100, Error.INVALID_AMOUNT);
  331:         require(address(staker) == address(0), Error.ADDRESS_ALREADY_SET);
  333:         require(stakerVault != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);
  387:         require(isCapped(), Error.NOT_CAPPED);
  399:         require(isCapped(), Error.NOT_CAPPED);
  400:         require(depositCap != _depositCap, Error.SAME_AS_CURRENT);
  401:         require(_depositCap > 0, Error.INVALID_AMOUNT);
  441:         require(
  471:         require(underlyingAmount > 0, Error.INVALID_AMOUNT);
  473:         require(lpToken_.balanceOf(account) > 0, Error.INSUFFICIENT_BALANCE);
  517:             require(
  525:         require(mintedLp >= minTokenAmount, Error.INVALID_AMOUNT);
  549:         require(redeemLpTokens > 0, Error.INVALID_AMOUNT);
  551:         require(lpToken_.balanceOf(msg.sender) >= redeemLpTokens, Error.INSUFFICIENT_BALANCE);
  562:         require(redeemUnderlying >= minRedeemAmount, Error.NOT_ENOUGH_FUNDS_WITHDRAWN);
  811:         require(maxFee >= minFee, Error.INVALID_AMOUNT);
  812:         require(maxFee <= _MAX_WITHDRAWAL_FEE, Error.INVALID_AMOUNT);

backd/contracts/pool/PoolFactory.sol:
  159:         require(vars.poolImplementation != address(0), Error.INVALID_POOL_IMPLEMENTATION);
  162:         require(vars.lpTokenImplementation != address(0), Error.INVALID_LP_TOKEN_IMPLEMENTATION);
  165:         require(vars.vaultImplementation != address(0), Error.INVALID_VAULT_IMPLEMENTATION);
  170:         require(
  180:             require(
  184:             require(lpTokenArgs.decimals == 18, Error.INVALID_DECIMALS);

backd/contracts/strategies/BkdTriHopCvx.sol:
  133:         require(_deposit(), Error.DEPOSIT_FAILED);
  147:         require(msg.value == 0, Error.INVALID_VALUE);

backd/contracts/strategies/ConvexStrategyBase.sol:
   76:         require(msg.sender == vault, Error.UNAUTHORIZED_ACCESS);
  117:         require(!isShutdown, Error.STRATEGY_SHUT_DOWN);
  144:         require(
  197:         require(crvCommunityReserveShare_ <= ScaledMath.ONE, Error.INVALID_AMOUNT);
  198:         require(communityReserve != address(0), "Community reserve must be set");
  214:         require(cvxCommunityReserveShare_ <= ScaledMath.ONE, Error.INVALID_AMOUNT);
  215:         require(communityReserve != address(0), "Community reserve must be set");
  260:         require(msg.sender == _strategist, Error.UNAUTHORIZED_ACCESS);
  273:         require(

backd/contracts/strategies/StrategySwapper.sol:
   69:             require(sent, "failed to send eth");
  110:         require(slippageTolerance_ <= ScaledMath.ONE, Error.INVALID_SLIPPAGE_TOLERANCE);
  111:         require(slippageTolerance_ > 0.8e18, Error.INVALID_SLIPPAGE_TOLERANCE);
  123:         require(token_ != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);
  124:         require(curvePool_ != address(curvePools[token_]), Error.SAME_ADDRESS_NOT_ALLOWED);
  139:         require(token_ != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);

backd/contracts/utils/Pausable.sol:
  10:         require(!isPaused, Error.CONTRACT_PAUSED);
  15:         require(_isAuthorizedToPause(msg.sender), Error.UNAUTHORIZED_PAUSE);

backd/contracts/utils/Preparable.sol:
   28:         require(deadlines[key] == 0, Error.DEADLINE_NOT_ZERO);
   29:         require(delay >= _MIN_DELAY, Error.DELAY_TOO_SHORT);
   86:         require(deadlines[key] != 0, Error.DEADLINE_NOT_ZERO);
   98:         require(deadlines[key] != 0, Error.DEADLINE_NOT_ZERO);
  110:         require(block.timestamp >= deadline, Error.DEADLINE_NOT_REACHED);
  111:         require(deadline != 0, Error.DEADLINE_NOT_SET);

backd/contracts/vault/Erc20Vault.sol:
  20:         require(underlying_ != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);

backd/contracts/vault/Vault.sol:
   53:         require(msg.sender == pool, Error.UNAUTHORIZED_ACCESS);
   58:         require(
   66:         require(
   88:         require(_debtLimit <= ScaledMath.ONE, Error.INVALID_AMOUNT);
   89:         require(_targetAllocation <= ScaledMath.ONE, Error.INVALID_AMOUNT);
   90:         require(_bound <= MAX_DEVIATION_BOUND, Error.INVALID_AMOUNT);
  164:         require(amount > 0, Error.INVALID_AMOUNT);
  165:         require(IPausable(pool).isPaused(), Error.POOL_NOT_PAUSED);
  167:         require(amount <= reserveBalance_, Error.INSUFFICIENT_BALANCE);
  194:         require(currentAddresses[_STRATEGY_KEY] == address(0), Error.ADDRESS_ALREADY_SET);
  195:         require(strategy_ != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);
  198:         require(IStrategy(strategy_).strategist() != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);
  264:         require(newPerformanceFee <= MAX_PERFORMANCE_FEE, Error.INVALID_AMOUNT);
  392:         require(newBound <= MAX_DEVIATION_BOUND, Error.INVALID_AMOUNT);
  429:         require(exists, Error.STRATEGY_DOES_NOT_EXIST);
  762:         require(

backd/contracts/vault/VaultReserve.sol:
  28:         require(_roleManager().hasRole(Roles.VAULT, msg.sender), Error.UNAUTHORIZED_ACCESS);
  51:             require(msg.value == amount, Error.INVALID_AMOUNT);
  59:         require(received >= amount, Error.INVALID_AMOUNT);
  73:         require(canWithdraw(msg.sender), Error.RESERVE_ACCESS_EXCEEDED);
  75:         require(accountBalance >= amount, Error.INSUFFICIENT_BALANCE);

backd/interfaces/vendor/ExponentialNoError.sol:
  83:         require(n < 2**224, errorMessage);
  88:         require(n < 2**32, errorMessage);

backd/libraries/EnumerableMapping.sol:
  104:         require(value != 0 || _contains(map, key), "EnumerableMap: nonexistent key");

I suggest replacing revert strings with custom errors.