search

code-423n4 / 2022-03-paladin-findings

0 stars 0 forks source link

QA Report #70

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Issue 1 (Low) - Missing 0 address check for immutable rewardsVault address

In the constructor for HolyPaladinToken.sol, the rewardsVault is set without requiring that the address not be 0. All other addresses in the constructor are properly verified. Moreover, rewardsVault is immutable, so the contract would need to be redeployed.

https://github.com/code-423n4/2022-03-paladin/blob/9c26ec8556298fb1dc3cf71f471aadad3a5c74a0/contracts/HolyPaladinToken.sol#L194

Issue 2 (Low) - Floating Pragma

https://github.com/code-423n4/2022-03-paladin/blob/9c26ec8556298fb1dc3cf71f471aadad3a5c74a0/contracts/PaladinRewardReserve.sol#L2

Both files contain a floating pragma. It is recommended to specify a compiler version to reduce the risk of deploying contracts with different versions, potentially leading to compiler-specific bugs.

Issue 3 (Low) - Use of SafeApprove() deprecated

https://github.com/code-423n4/2022-03-paladin/blob/9c26ec8556298fb1dc3cf71f471aadad3a5c74a0/contracts/PaladinRewardReserve.sol#L31

OpenZeppelin has marked this function as deprecated and recommends using safeIncreaseAllowance(). See following comments:

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/742e85be7c08dff21410ba4aa9c60f6a033befb8/contracts/token/ERC20/utils/SafeERC20.sol#L39-L43

Issue 4 (Non-critical) - Make the emergency logic a modifier

Repeated emergency checks can be made a modifier to reduce code duplication.

Example: https://github.com/code-423n4/2022-03-paladin/blob/9c26ec8556298fb1dc3cf71f471aadad3a5c74a0/contracts/HolyPaladinToken.sol#L254

Kogaroshi commented 2 years ago

QA & gas optimizations changes are done in the PR: https://github.com/PaladinFinance/Paladin-Tokenomics/pull/6 (some changes/tips were implemented, others are noted but won't be applied)