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)
2022-03-paladin gas optimization
1 use cache for userLocks[user].length and delete lastUserLockIndex. userLocks[user].length will be called twice in _availableBalanceOf. You can use cache to save gas costs. In addition lastUserLockIndex will be used only one time in this function, so you can delete it.
For example unstake() from Avg 165354 to 165311 only with this change according to hardhat-gas-reporter.
https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L707-L711
function _availableBalanceOf(address user) internal view returns(uint256) { uint256 userLockLength = userLocks[user].length; if(userLocks[user].length == 0) return balanceOf(user); return balanceOf(user) - uint256(userLocks[user][userLockLength - 1].amount); }
2 use cache for userLocks[msg.sender].length and delete currentUserLockIndex in increaseLock. userLocks[msg.sender].length will be called twice in increaseLock. To avoid extra sload, you can use cache for it. In addition to that, currentUserLockIndex will be used only one time in the function. You can delete it to save gas costs.
increaseLock() from Avg 141196 to 141005 only with this change according to hardhat-gas-reporter.
https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L286-L289
function increaseLock(uint256 amount) external { if(emergency) revert EmergencyBlock(); uint256 userLockLength = userLocks[msg.sender].length; require(userLockLength != 0, "hPAL: No Lock"); // Find the current Lock UserLock storage currentUserLock = userLocks[msg.sender][userLockLength -1]; // Update user rewards before any change on their balance (staked and locked) _updateUserRewards(msg.sender); // Call the _lock method with the current duration, and the new amount _lock(msg.sender, amount, currentUserLock.duration, LockAction.INCREASE_AMOUNT); }
3 use cache for userLocks[msg.sender].length and delete currentUserLockIndex in increaseLockDuration. userLocks[msg.sender].length will be called twice inincreaseLockDuration. To avoid extra sload, you can use cache for it. In addition to that, currentUserLockIndex will be used only one time in the function. You can delete it to save gas costs.
increaseLockDuration() from Avg 141196 to 141005 only with this change according to hardhat-gas-reporter.
https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L270-L273
function increaseLockDuration(uint256 duration) external { if(emergency) revert EmergencyBlock(); uint256 userLockLength = userLocks[msg.sender].length; require(userLockLength != 0, "hPAL: No Lock"); UserLock storage currentUserLock = userLocks[msg.sender][userLockLength - 1]; // Update user rewards before any change on their balance (staked and locked) _updateUserRewards(msg.sender); // Call the _lock method with the current amount, and the new duration _lock(msg.sender, currentUserLock.amount, duration, LockAction.INCREASE_DURATION); }
4 use cache for checkpoints[delegatee] in _writeCheckpoint. In _writeCheckpoint checkpoints[delegatee] will be called many times. You can save gas with cache for it.
For example delegate() from Avg 132372 to 132223 only with this change according to hardhat-gas-reporter.
https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L1047-L1057
function _writeCheckpoint(address delegatee, uint256 newVotes) internal { Checkpoint[] storage delegateeCheckPoints = checkpoints[delegatee]; // write a new checkpoint for an user uint pos = delegateeCheckPoints.length;
if (pos > 0 && delegateeCheckPoints[pos - 1].fromBlock == block.number) { delegateeCheckPoints[pos - 1].votes = safe224(newVotes); } else { uint32 blockNumber = safe32(block.number); delegateeCheckPoints.push(Checkpoint(blockNumber, safe224(newVotes))); } }
5 check input validation and rewardsLastUpdate[user] earlier in _updateUserRewards. The input user and rewardsLastUpdate[user] will be checked after calling _updateRewardState(). both checkings must be placed at the beginning of this function to save gas in case of executing these rejections.
https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L859-L861
6 Use cache for userLocks[user] and userLocks[user].length _kick. userLocks[user] is used many times in the function. You can use cache to save gas.
kick() from Avg 231670 to 231316 only with this change according to hardhat-gas-reporter.
https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L1272-L1293
UserLock[] storage _userLocks = userLocks[user]; uint256 userLocksLength = _userLocks.length; require(userLocksLength > 0, "hPAL: No Lock");
// Get the user to kick current Lock // and calculate the end of the Lock uint256 currentUserLockIndex = userLocksLength - 1; UserLock storage currentUserLock = _userLocks[currentUserLockIndex]; uint256 userCurrentLockEnd = currentUserLock.startTimestamp + currentUserLock.duration;
require(block.timestamp > userCurrentLockEnd, "hPAL: Not expired"); require(currentUserLock.amount > 0, "hPAL: No Lock");
require(block.timestamp > userCurrentLockEnd + UNLOCK_DELAY, "hPAL: Not kickable");
// Remove amount from total locked supply currentTotalLocked -= currentUserLock.amount; totalLocks.push(TotalLock( safe224(currentTotalLocked), safe32(block.number) ));
// Set an empty Lock for the user userLocks.push(UserLock(
7 use cache for userLocks[msg.sender], userLocks[msg.sender].length, and userLocks[msg.sender][currentUserLockIndex] are called several times in stakeAndIncreaseLock. stakeAndIncreaseLock can be written with caches like the following to save gas.
stakeAndIncreaseLock() from Avg 227758 to 227326 only with this change according to hardhat-gas-reporter.
https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L346-L365
function stakeAndIncreaseLock(uint256 amount, uint256 duration) external returns(uint256) { if(emergency) revert EmergencyBlock(); UserLock[] storage _userLocks = userLocks[msg.sender]; uint256 _userLocksLength = _userLocks.length; require(_userLocksLength != 0, "hPAL: No Lock"); // Find the current Lock UserLock storage _userLock = userLocks[msg.sender][_userLocksLength -1]; uint256 previousLockAmount = _userLock.amount; // Stake the new amount uint256 stakedAmount = _stake(msg.sender, amount); // No need to update user rewards since it's done through the _stake() method if(delegates[msg.sender] == address(0)){ _delegate(msg.sender, msg.sender); } // Then update the lock with the new increased amount if(duration == _userLock.duration) { _lock(msg.sender, previousLockAmount + amount, duration, LockAction.INCREASE_AMOUNT); } else { _lock(msg.sender, previousLockAmount + amount, duration, LockAction.LOCK); } return stakedAmount; }