GalloDaSballo
commented
2 years ago [L-01] Inconsistency in event emissions
Agree with the catch, consistency is key
[L-02] Code vs comment conflict: Backwards imp. of “safe”
Nice catch
[L-03] Use of deprecated SafeApprove()
Valid but code is safe
[L-04] Improper ownership transfer
Disagree but valid
[L-05] Missing event emissions on important setter functions
Non-critical
[L-06] Missing address(0) checks on important functions
Valid per industry standard
[N-01] Inconsistency in using modifier
Agree, nice catch
Typo
Agree
This is the type of reports I'd like to see more of, actually specific catches that are custom tailored for the code in scope.
I'll do my best to give extra points to this report which is short, sweet and written by a human being
[L-01] Inconsistency in event emissions
Events are emitted at the beginning vs end of the function execution. Temporary variables are/aren't used in the event emission call.
https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/utils/Preparable.sol#L140-L154
[L-02] Code vs comment conflict: Backwards imp. of “safe”
The "safe" function is not the safe version. The unsafe version is the one that reverts in unsuccessful.
https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/AddressProvider.sol#L307-L327
[L-03] Use of deprecated SafeApprove()
SafeApprove()has been deprecated according to OpenZeppelin.https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/FeeBurner.sol#L118
[L-04] Improper ownership transfer
It is recommended to make ownership transfer a two-step process where you firstly set a pending owner address and then accept the pending owner in a second call.
https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/VestedEscrow.sol#L68-L72
[L-05] Missing event emissions on important setter functions
https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/AmmConvexGauge.sol#L86-L96
[L-06] Missing address(0) checks on important functions
https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/BkdLocker.sol#L227-L232 https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/StakerVault.sol#L322 https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/StakerVault.sol#L381 https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/VestedEscrow.sol#L146 https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/AmmGauge.sol#L131
[N-01] Inconsistency in using modifier
Sometimes modifier is used. Sometimes require statement is used.
https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/KeeperGauge.sol#L57 https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/AmmGauge.sol#L50
[N-02] Typo in "an"
https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/InflationManager.sol#L66