QA Report #146
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
resolved
Finding has been patched by sponsor (sponsor pls link to PR containing fix)
reviewed
Issues that Backd has reviewed (just for internal tracking, can ignore this)
[L-01] Deprecated safeApprove() function
Using this deprecated function can lead to unintended reverts and potentially the locking of funds. A deeper discussion on the deprecation of this function is in OZ issue #2219 (OpenZeppelin/openzeppelin-contracts#2219). The OpenZeppelin ERC20 safeApprove() function has been deprecated, as seen in the comments of the OpenZeppelin code.
As recommended by the OpenZeppelin comment, I suggest replacing safeApprove() with safeIncreaseAllowance() or safeDecreaseAllowance() instead:
[L-02] Immutable addresses should be 0-checked
In the constructors, the solution never checks for
address(0)
when setting the value of immutable variables. I suggest adding those checks.List of immutable variables:
[N-01] Open TODOS
Consider resolving the TODOs before deploying.
[N-02] Code in comment
Consider deleting the following line:
[N-03] Related data should be grouped in a struct
The "LP Fee Distribution"
maps
should be grouped in a struct.From:
To
It would be less error-prone, more readable, and it would be possible to delete all related fields with a simple
delete userSpecificData[address]
.[N-04] Unused named returns
Several functions are declaring named returns but then are using return statements. I suggest choosing only one for readability reasons:
[N-05] It's better to emit after all processing is done
Instances include:
The text was updated successfully, but these errors were encountered: