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

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

QA Report #326

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

Comments

@code423n4
Copy link
Contributor

2022-05-aura

1 follow the naming convention.

According to the solidity doc, constants should be named with all capital letters with underscores separating words.

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/Aura.sol#L29
https://github.com/code-423n4/2022-05-aura/blob/main/contracts/AuraStakingProxy.sol#L45

uint256 public constant TOTAL_CLIFFS = 500;
uint256 public constant DENOMINATOR = 10000;

2 Lock pragmas to specific compiler version.

Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

pragma solidity 0.8.11;

3 Use modifiers only for checks.

The code inside a modifier is usually executed before the function body, so any state changes or external calls will violate the Checks-Effects-Interactions pattern. Moreover, these statements may also remain unnoticed by the developer, as the code for modifier may be far from the function declaration.

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/AuraBalRewardPool.sol#L89-L97

Define updateReward as function and use it in stake, stakeFor, withdraw, and getReward after require statements.

4 emit block.timestamp too in Forwarded.

At the end of forward in AuraPenaltyForwarder, the Forwarded event will be emitted. Block.timestamp may be added into the event as the extra information.

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/AuraPenaltyForwarder.sol#L56

event Forwarded(uint256 amount, uint256 lastDistributed);

emit Forwarded(bal, block.timestamp);

5 _getReward in ExtraRewardsDistributor must be internal.

The visibility of _getReward is public. However, there are 2 other external functions(getReward) for this functionality. _getReward can and must be internal.

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/ExtraRewardsDistributor.sol#L141-L145

function _getReward(address _account, address _token, uint256 _startIndex) internal

6 missing validation check for array length. The length of the following arrays must be checked if the both have same length or not.

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/AuraVestedEscrow.sol#L96

require(_recipient.length == _amount.length, “error message”);

7 wrong description in the comment.

According to the comment on the _claim, it seems that the claimable reward must be always locked. However, it can be also transferred to the _recipient. It depends on the param _lock. For example, the claimable reward will be transferred if this function will be called from the cancel.

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/AuraVestedEscrow.sol#L175

@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 25, 2022
code423n4 added a commit that referenced this issue May 25, 2022
@0xMaharishi 0xMaharishi added the duplicate This issue or pull request already exists label May 28, 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 duplicate This issue or pull request already exists 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

2 participants