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 #207

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

QA Report #207

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

Incorrect Fee Calculations

CrvDepositor @notice states lockIncentive is set to 1% initially however when the actual calculations are done due to the FEE_DENOMINATOR being set to 10,000 and lockIncentive having a max value of 30, the fee will be only 0.1-0.3%.

CrvDepositor.sol#L184

Proof of Concept with current values:
lockIncentive = 10
FEE_DENOMINATOR = 10,000

If depositFor is called with an _amount of 1,000 CRV

callIncentive = _amount.mul(lockIncentive).div(FEE_DENOMINATOR)
= 1,000 * 10 / 10,000
= 1 CRV

Recommend changing lockIncentive to 100 and the require check on line 75 to _lockIncentive <= 300.

Inconsistent inflationProtectionTime

AuraMinter.sol#L23

inflationProtectionTime is set to block.timestamp + 156 weeks when @notice states inflation will be protected for 4 years.

I couldn’t find which is the intended value in the docs, but recommed either updating the @notice to 3 years or change the value on line 23 to 208 weeks.

safeApprove() Has Been Deprecated

As per Openzepplin safeApprove is deprecated in favor of using safeIncreaseAllowance whenever increasing allowance.

Recommend replacing all the following safeApprove calls that aren’t setting to 0 with safeIncreaseAllowance.

AuraLocker.sol#L240-L241
AuraBalRewardPool.sol#L75
AuraClaimZap.sol#L98-L105
AuraMerkleDrop.sol#L131-L132
AuraPenaltyForwarder.sol#L41
AuraStakingProxy.sol#L147-L151
AuraStakingProxy.sol#L215-L216
AuraVestedEscrow.sol#L186
BalLiquidityProvider.sol#L59-L60
CrvDepositorWrapper.sol#L52-L53
BaseRewardPool4626.sol#L40
Booster.sol#L422-L423
CrvDepositor.sol#L199-L200
VoterProxy.sol#L176-L177
VoterProxy.sol#L193-L194
VoterProxy.sol#L244-L245
VoterProxy.sol#L255-L256

Unlocked Pragma Versions

Non Library/Interface files should not use unlocked pragma versions for production, recommend removing ^.

All files in /main/contracts/ with the exception of CrvDepositerWrapper.sol are currently using floating versions.

Incomplete Natspec

The following 2 functions appear to have their natspec completed but are each missing a parameter.

AuraBalRewardPool.sol#L54-L61
Natspec is missing @param _startDelay description

CrvDepositor.sol#L164
Natspec is missing @param _to description

@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