Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QA Report #218

Open
code423n4 opened this issue May 24, 2022 · 0 comments
Open

QA Report #218

code423n4 opened this issue May 24, 2022 · 0 comments
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax

Comments

@code423n4
Copy link
Contributor

Low Risk Issues

  1. The function inside "ADMIN" region doesn't have the "onlyOwner" modifier.
    AuraLocker.sol#L239
    You marked the region as "ADMIN" at #L191 but this function doesn't have the "onlyOwner" modifier.
    I think it's not dangerous because both variables are immutable but it would be good to write some comments if you don't add the modifier.

Non-critical Issues

  1. Natspec incomplete
    AuraBalRewardPool.sol#L62 => @param _startDelay
    BoosterOwner.sol#L69 => @param _seal
    CrvDepositor.sol#L47 => #param _daoOperator

  2. Event should use indexed fields if there are three or more fields
    AuraMerkleDrop.sol#L41
    Booster.sol#L72
    Booster.sol#L78
    Booster.sol#L82
    Booster.sol#L84
    RewardFactory.sol#L32

  3. Typo
    I will mark incorrect parts with ""

Aura.sol#L114
// e.g. (new) amount = 1e19 * 950 / 500 = "19e17";

AuraClaimZap.sol#L37

  •       - add option "to" use all funds in wallet
    

AuraVestedEscrow.sol#L94

  • @param _amount "Arrary" of amount of rewardTokens to vest

Booster.sol#L372

  • @notice Shuts down the WHOLE SYSTEM by withdrawing all the LP tokens "ot" here and then allowing
  1. It would be good to check an empty address when you change Owner or FeeManager as you can't
    control the contract forever if you set an empty address by mistake.
    Booster.sol#L128
    You can add below line at #L130.
    require(_owner != address(0), "0 address");

You can modify similarly with others also.
Booster.sol#L138
Booster.sol#L148
Booster.sol#L191
CrvDepositor.sol#L62
CrvDepositor.sol#L67
PoolManagerProxy.sol#L43
PoolManagerProxy.sol#L48
PoolManagerSecondaryProxy.sol#L58
PoolManagerSecondaryProxy.sol#L63
VoterProxy.sol#L73

@code423n4 code423n4 added bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax labels May 24, 2022
code423n4 added a commit that referenced this issue May 24, 2022
@0xMaharishi 0xMaharishi added the duplicate This issue or pull request already exists label May 27, 2022
@dmvt dmvt removed the duplicate This issue or pull request already exists label Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Projects
None yet
Development

No branches or pull requests

3 participants