search

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

0 stars 0 forks source link

QA Report #115

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

QA reports (low/non-critical)

Contest: Velodrome

Autor: Rotcivegaf

Scope:

Native Token:

Pair:

Emissions:

Voting:

Governance:

Redemption(WeVE -> VELO):

:star: = Areas of concern :heavy_check_mark: = Audited Contract :white_check_mark: = Semi-Audited Contract(< 100%)

Non-critical

[N-01] Use standard libraries

Use @openzeppelin or @rari-capital/solmate to clarify code avoid mistake and don't repeat logic, PLEASE

[N-02] Use the same indentation for all files

Some files different spaces identation like Bribe.sol

[N-03] Missing error messages in require statements

[N-03] Reuse code

There are many function what used in many contracts like _safeTransfer or _safeTransferFrom, or even the ReentrancyGuard logic, move the repeated logic to a contract/library and heredit/use of it

[N-04] Assert to require

Use require statement instead of assert, and add a error message

[N-05] Event is missing indexed fields

Each event should use three indexed fields if there are three or more fields

Gauge.sol:

RewardsDistributor.sol:

Voter.sol:

VotingEscrow.sol:

PairFactory.sol:

[N-06] Function view to pure

In Bribe.sol, L35-L49, getEpochStart: function state mutability can be restricted to pure

[N-07] Function _clearApproval not emit the Approval event in VotingEscrow.sol, L246

Low

[L-01] OPEN TODOs

There are open TODOs, fix they

VotingEscrow.sol, lines 314, 465, 524: what means "add delegates" Minter.sol, L11: "decide on whether to abstract from VELO or not. currently it's only somewhat abstracted (e.g. L38)" VelodromeLibrary.sol, L09: "make modifiable?"

[L-02] Remove decimals

VotingEscrow.sol, L125: The ERC721 standard not use a decimals variable, this is used in ERC20 standard

[L-03] Miss _from and _to not address(0) require

VotingEscrow.sol, L301: The function _transferFrom should have a _from/_to not address(0) check

function _transferFrom(
    address _from,
    address _to,
    uint _tokenId,
    address _sender
) internal {
    require(_from != address(0), "ERC20: transfer from the zero address");
    require(_to != address(0), "ERC20: transfer to the zero address");

...

[L-04] The Minter not implements Velo functions

The Minter contract not implements the setMinter and setRedemptionReceiver function from the Velo contract Remove this function or implements in the Minter contract

GalloDaSballo commented 2 years ago

[N-01] Use standard libraries

Disagree with the finding

[N-02] Use the same indentation for all files

Valid NC

[N-03] Missing error messages in require statements

Valid NC

[N-03] Reuse code

Valid NC

[N-04] Assert to require

Valid Low

[N-05] Event is missing indexed fields

Valid NC

[N-06] Function view to pure

Valid NC

 [N-07] Function _clearApproval not emit the Approval event in VotingEscrow.sol, L246

Valid NC

[L-01] OPEN TODOs

Valid NC

[L-02] Remove decimals

Valid NC (dead code)

[L-03] Miss _from and _to not address(0) require

Valid Low

 [L-04] The Minter not implements Velo functions

Disagree as those are meant to be used one-off by msg.sender (deployer EOA)

Neat short and sweet report

2 L, 8 NC