QA Report

[code423n4]

Summary

We list 3 low-critical findings and 1 non-critical finding here:

• (Low) Rewrite _updateDropPerSecond to avoid loss of precision
• (Low) Rewrite _getUserAccruedRewards to avoid loss of precision
• (Low) Lock pragma to ensure compiler version
• (Non) _getNewReceiverCooldown revert when divided by zero

In summary of recommended security practices, it's better to rewrite formulas to avoid loss of precision, lock pragma version, and check division by zero.

(Low) Rewrite _updateDropPerSecond to avoid loss of precision

Impact

In _updateDropPerSecond:

        uint256 dropDecreasePerMonth = (startDropPerSecond - endDropPerSecond) / (dropDecreaseDuration / MONTH);

dropDecreasePerMonth may suffer from loss of precision due to ordering of operations.

Proof of Concept

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L729

Tools Used

vim, ganache-cli

Recommended Mitigation Steps

rewrite to avoid loss of precision:

        uint256 dropDecreasePerMonth = (startDropPerSecond - endDropPerSecond) * MONTH / dropDecreaseDuration;

(Low) Rewrite _getUserAccruedRewards to avoid loss of precision

Impact

In _getUserAccruedRewards:

        lockingRewards = (userLockedBalance * ((indexDiff * vars.periodBonusRatio) / UNIT)) / UNIT;

lockingRewards may suffer from loss of precision.

Proof of Concept

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L842

Tools Used

vim, ganache-cli

Recommended Mitigation Steps

rewrite to avoid loss of precision:

        lockingRewards = userLockedBalance * indexDiff * vars.periodBonusRatio / UNIT / UNIT;

(Low) Lock pragma to ensure compiler version

Impact

Floating pragma may cause unexpected compilation time behaviour and introduce unintended bugs.

Proof of Concept

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

Tools Used

vim, ganache-cli

Recommended Mitigation Steps

Don't use ^, lock pragma to ensure compiler version. e.g. pragma solidity 0.8.4;

(Non) _getNewReceiverCooldown revert when divided by zero

Impact

_getNewReceiverCooldown reverts on transferring 0 amount to someone with 0 balance due to division by 0.

Proof of Concept

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L1108

Tools Used

vim, ganache-cli

Recommended Mitigation Steps

Explicitly check (amount + receiverBalance) should not equal to 0.

[Kogaroshi]

PR with the changes for better precision: https://github.com/PaladinFinance/Paladin-Tokenomics/pull/9