Gas Optimizations

[code423n4]

gas

1 Unnecessary require()

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L183 transferOwnership() function was validating that _admin != address(0). I recommend to remove L183 or call _transferOwnerShip() instead of transferOwnership

2 != is more effective than >

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L229 Using != 0 is more gas effective > 0

3 Using SafeERC20 Lib for pal token

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L13 Using SafeERC20.function for pal token is unnecessary. Using just transfer and transferFrom from ERC20.function is gas saving

4 Unnecessary burnAmountx variable declaration in _unstake function

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L1090 Instead of using ternary operation, using if() statement can save gas, then just store the value amount to burn to amount var:

If(amount > userAvailableBalance){
    amount = userAvailableBalance;
}
_burn(user, amount);
pal.safeTransfer(receiver, amount);
emit Unstake(user, amount);
return amount

5 Declaration bool with default value

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L103 By not declaring that emergency = false, the value will still == false. So delete the value set at L103

6 Unnecessary rewardLastUpdate[user] == block.timestamp

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L861 The only way to update rewardLastUpdate[user]is in the same function (_updateUserReward()). Therefore, the condition rewardLastUpdate[user] == block.timestamp is an edge case (its quite hard to have exact the same block.timestamp since it is updated per second), and have no security risk.

7 Using if() statement instead of ternary operation

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L1206 By using if() statement, MSTORE when the condition == false will prevented and also reducing gas cost

If(action == LockAction.INCREASE_AMOUNT) startTimestamp = currentUserLock.startTimestamp;

8 Prevent too many SLOAD and using if instead of ternary operation

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L388 By chacing claimableReward[msg.sender] to claimAmount and using if condition to set claimAmount = amount (also prevent MSTORE) can be a lot efficient:

uint claimAmount = claimableRewards[msg.sender];

if(amount < claimAmount)claimAmount = amount;

9 Gas improvement on calling SafeERC20.function

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L13 In case SafeERC20 is used. By removing L13 and call SafeERC20.function directly in line can save 15 gas per call. For instance:

SafeERC20.safeTransfer(pal, msg.sender, burnAmount)

10 Using storage instead of memory to declare struct

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L650 Instead of chasing UserLock in memory, read it directly from storage can save gas (if struct var amount > how many time it was called, better using storage pointer)

UserLock storage pastLock = _getPastLock(user, blockNumber);

11 Unnecessary votes variable declaration

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L646 votes is merely called once in the getPastVotes function. Avoiding unnecessary MSTORE and pass it directly to L645 can save gas:

Return _getPastVotes(user, blockNumber) + bonusVotes;

[Kogaroshi]

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)