QA Report #71
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
Non Critical
Use of Floating Pragma:
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.
Recommended Mitigation Steps:
Lock the pragma version: use
0.8.9
instead of^0.8.0
.Delete unused imports:
Having unused imports make confusion.
Code Style: non-constant should not be named in upper-case:
Non-constant (especially public) variables should not be in
SNAKE_CASE
, or they may be misunderstood as constants.Consider changing to
camelCase
.https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L27
it's value can be changed in:
https://github.com/code-423n4/2022-03-joyn/blob/main/core-contracts/contracts/CoreCollection.sol#L209
Redundant
== true
in require:transfer
return boolean andrequire
expect a boolean. so these== true
can be deleted:https://github.com/code-423n4/2022-03-joyn/blob/main/royalty-vault/contracts/RoyaltyVault.sol#L44
https://github.com/code-423n4/2022-03-joyn/blob/main/royalty-vault/contracts/RoyaltyVault.sol#L48
https://github.com/code-423n4/2022-03-joyn/blob/main/royalty-vault/contracts/RoyaltyVault.sol#L55
Low Risks
Use better reentrancy attack prevention method than gas limit:
https://github.com/code-423n4/2022-03-joyn/blob/main/splits/contracts/Splitter.sol#L255
As mentioned in comments this can lead to limited reentrancy attack. use openzeppelin reentrancyguard here.
this internal function it's not used yet in other parts of code.
but use a reentrancy attack prevention method when you going to use it.
The text was updated successfully, but these errors were encountered: