QA Report

[code423n4]

Paladin Contest

March 31, 2022

@securerodd #

Non-Critical

1. Inconsistent Removal of Bonus Multiplier

The bonus multipliers userCurrentBonusRatio[user] and userBonusRatioDecrease[user] are zeroed out in the _kick(address user, address kicker) and _unlock(address user) functions (where an empty lock is set). These bonus multipliers are not zeroed out in the emergencyWithdraw(uint256 amount, address receiver) function even though an empty lock is set there as well. The logic for this is seen below:

// Remove the bonus multiplier
userCurrentBonusRatio[user] = 0;
userBonusRatioDecrease[user] = 0;

Recommendation: Remove the redundant lines in _kick(address user, address kicker) and _unlock(address user). Currently, before a user is able to claim rewards or perform any action that hits the pre-transfer hook, the _updateUserRewards(address user) function is called. This function will only factor in the locking rewards if the current lock's amount is > 0 and its duration is not equal to 0. Thus, zeroing out the bonus multipliers is unnecessary as long as an empty lock is set. This is already done in all 3 of the aforementioned functions.

[Kogaroshi]

This logic of reset of userCurrentBonusRatio[user] and userBonusRatioDecrease[user] is implemented in case other smart contracts are built on top of hPAL, and might use that system, so we never want to return them a bonusRatio if the Lock was removed. Update in the PR: https://github.com/PaladinFinance/Paladin-Tokenomics/pull/5

[0xean]

duplicate of #27

[0xean]

closing as dupe