Paladin contest - securerodd's results

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.

Paladin Contest

March 31, 2022

@securerodd

Gas Optimizations

1. Use of Unchecked Directive and Caching of Calculated Value Saves 355 Gas

This finding applies to the following five functions:

increaseLockDuration(uint256 duration)

increaseLock(uint256 amount)

stakeAndIncreaseLock(uint256 amount, uint256 duration)

_kick(address user, address kicker

_unlock(address user)

Each function implemented almost the exact same logic to check that a lock exists and then to get the current lock. The logic (taken from increaseLockDuration(uint256 duration)) can be seen below:

require(userLocks[msg.sender].length != 0, "hPAL: No Lock");
// Find the current Lock
uint256 currentUserLockIndex = userLocks[msg.sender].length - 1;

Significant gas can be saved by first caching the length of userlocks[msg.sender] into a local variable, then checking that it is not 0, and using the unchecked directive to decrement it by 1. The suggested replacement logic is seen below:

uint256 currentUserLockIndex = userLocks[msg.sender].length;
require(currentUserLockIndex != 0, "hPAL: No Lock");
// Find the current Lock
unchecked {--currentUserLockIndex;}

Through testing on Remix, this saved 71 gas on increaseLockDuration(uint256 duration). This same logic could be ported over to all 5 functions for ~355 gas savings.