code-423n4 / 2022-05-backd-findings

0 stars 0 forks source link

QA Report #173

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago
  1. Title : Comment was not the same as actual code

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/AddressProvider.sol#L112-L129

In the function of removepool(). Comment was used to said that :

     * @return `true` if successful. 

but in actual code was :

        return removed;

So it can be changed as it should be.

Tool Used

Manual Review

Another Occurances

It happen too in this code : 1.) AddressProvider.sol Lines247-261 2). LpGauge.sol Lines.48-62

  1. Title : Redundant Code _prepare

This code was redundant and it could be deleted for better code since it has

1 and 2 with the same fn()

    /**
     * @notice Same as `_prepare(bytes32,uint256,uint256)` but uses a default delay
     */
    function _prepare(bytes32 key, uint256 value) internal returns (bool) {
        return _prepare(key, value, _MIN_DELAY);
    }

Tool Used

Manual Review

  1. Title : NatSpec is incomplete

1.) File : contracts/AddressProvider.sol (Lines.77-87)

Missing @return

    /**
     * @notice Adds action.
     * @param action Address of action to add.
     */
    function addAction(address action) external override onlyGovernance returns (bool) {
        bool result = _actions.add(action);
        if (result) {
            emit ActionListed(action);
        }
        return result;
    }

2.) File : contracts/BkdLocker.sol (Lines.77-83)

Missing @return

     /**
     * @notice Lock gov. tokens.
     * @dev The amount needs to be approved in advance.
     */
    function lock(uint256 amount) external override {
        return lockFor(msg.sender, amount);
    }

3.) File : contracts/Controller.sol (Lines.78-84)

Missing @param amount

    /**
     * @notice Prepares the minimum amount of staked BKD required by a keeper
     */
    function prepareKeeperRequiredStakedBKD(uint256 amount) external override onlyGovernance {
        require(addressProvider.getBKDLocker() != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);
        _prepare(_KEEPER_REQUIRED_STAKED_BKD, amount);
    }

4.) File : contracts/Controller.sol (Lines.100-108)

Missing @param keeper

    /**
     * @notice Returns true if the given keeper has enough staked BKD to execute actions
     */
    function canKeeperExecuteAction(address keeper) external view override returns (bool) {
        uint256 requiredBKD = getKeeperRequiredStakedBKD();
        return
            requiredBKD == 0 ||
            IERC20(addressProvider.getBKDLocker()).balanceOf(keeper) >= requiredBKD;
    }

5.) File : contracts/Controller.sol (Lines.118-130)

Missing @param payer

     * @return the total amount of ETH require by `payer` to cover the fees for
     * positions registered in all actions
     */
    function getTotalEthRequiredForGas(address payer) external view override returns (uint256) {
        // solhint-disable-previous-line ordering
        uint256 totalEthRequired;
        address[] memory actions = addressProvider.allActions();
        uint256 numActions = actions.length;
        for (uint256 i; i < numActions; i = i.uncheckedInc()) {
            totalEthRequired += IAction(actions[i]).getEthRequiredForGas(payer);
        }
        return totalEthRequired;
    }

6.) File : contracts/utils/Preparable.sol (Lines.50-55)

    /**
     * @notice Same as `_prepare(bytes32,uint256,uint256)` but uses a default delay
     */
    function _prepare(bytes32 key, uint256 value) internal returns (bool) {
        return _prepare(key, value, _MIN_DELAY);
    }

7.) File : contracts/utils/Preparable.sol (Lines.74-79)

     /**
     * @notice Same as `_prepare(bytes32,address,uint256)` but uses a default delay
     */
    function _prepare(bytes32 key, address value) internal returns (bool) {
        return _prepare(key, value, _MIN_DELAY);
    }
  1. Title : Typo Comment

1.) File : contracts/BkdLocker.sol (Line.174)

invlude change to include

     * @dev This does not invlude the gov. tokens queued for withdrawal.

2.) File : contracts/AddressProvider.sol (Line.237)

feeze change into freeze

     * @param key Key to feeze
  1. Title : simplify the number of _MAX_SUPPLY

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/utils/CvxMintAmount.sol#L12

  uint256 private constant _MAX_SUPPLY = 100000000 * 1e18; //100 mil max supply

changed to :

uint256 private constant _MAX_SUPPLY =  1e26 //100mil
GalloDaSballo commented 2 years ago

Title : Comment was not the same as actual code

Disagree as true is the value returned on success If anything just use @dev tag

Title : NatSpec is incomplete

Very confident natspec doesn't require all fields, else the compiler would warn

1.) File : contracts/BkdLocker.sol (Line.174)

Agree with typos and with the refactoring, although it won't save gas

Overall a pretty small and concise report