Paladin - Warden Pledges contest - c3phas's results

A governance lending protocol transforming users voting power into a new money lego.

General Information

Platform: Code4rena

Start Date: 27/10/2022

Pot Size: $33,500 USDC

Total HM: 8

Participants: 96

Period: 3 days

Judge: kirk-baird

Total Solo HM: 1

Id: 176

League: ETH

Paladin

Findings Distribution

Researcher Performance

Rank: 27/96

Findings: 2

Award: $180.63

QA:
grade-b
Gas:
grade-a

🌟 Selected for report: 1

πŸš€ Solo Findings: 0

QA

Missing checks for address(0x0) when assigning values to address state variables

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L137-L140

File: /contracts/WardenPledge.sol
137:        votingEscrow = IVotingEscrow(_votingEscrow);
138:        delegationBoost = IBoostV2(_delegationBoost);
140:        chestAddress = _chestAddress;

Constants should be defined rather than using magic numbers

There are several occurrences of literal values with unexplained meaning .Literal values in the codebase without an explained meaning make the code harder to read, understand and maintain, thus hindering the experience of developers, auditors and external contributors alike.

Developers should define a constant variable for every magic value used , giving it a clear and self-explanatory name.

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L626

File: /contracts/WardenPledge.sol

//@audit: 500
626:        if(newFee > 500) revert Errors.InvalidValue();

Typos - see audit tags

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L292

File: /contracts/WardenPledge.sol

//@audit: **balacne** instead of **balance*
292:    * @param targetVotes Maximum taget of votes to have (own balacne + delegation) for the receiver

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L365

File: /contracts/WardenPledge.sol

//@audit: **ot** instead of **to**
295:    * @param maxTotalRewardAmount Maximum total reward amount allowed ot be pulled by this contract
296:    * @param maxFeeAmount Maximum feeamount allowed ot be pulled by this contract
365:    * @param maxTotalRewardAmount Maximum added total reward amount allowed ot be pulled by this contract
366:    * @param maxFeeAmount Maximum fee amount allowed ot be pulled by this contract
411:    * @param maxTotalRewardAmount Maximum added total reward amount allowed ot be pulled by this contract
412:    * @param maxFeeAmount Maximum fee amount allowed ot be pulled by this contract

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L523

File: /contracts/WardenPledge.sol

//@audit: **Minmum** instead of **Minimum**
523:    * @param minRewardPerSecond Minmum amount of reward per vote per second for the token

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L539

File: /contracts/WardenPledge.sol

//@audit: **Minmum** instead of **Minimum**
539:    * @param minRewardsPerSecond Minmum amount of reward per vote per second for each token in the list

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L558

File: /contracts/WardenPledge.sol

//@audit: **Minmum** instead of **Minimum**
558:    * @param minRewardPerSecond Minmum amount of reward per vote per second for the token

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L568

File: /contracts/WardenPledge.sol
//@audit: **Minmum** instead of **Minimum**
568:    * @param minRewardPerSecond Minmum amount of reward per vote per second for the token

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L621-L622

File: /contracts/WardenPledge.sol

//@audit: Platfrom instead of Platform
621:    * @notice Updates the Platfrom fees BPS ratio
622:     * @dev Updates the Platfrom fees BPS ratio

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L650

File: /contracts/WardenPledge.sol
//@audit: **tof** instead of **of**
650:    * @param token Address tof the EC2O token

Natspec is incomplete

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L125-L136

File: /contracts/WardenPledge.sol

//@audit: Missing @param _chestAddress

125:    /**
126:    * @dev Creates the contract, set the given base parameters
127:    * @param _votingEscrow address of the voting token to delegate
128:    * @param _delegationBoost address of the contract handling delegation
129:    * @param _minTargetVotes min amount of veToken to target in a Pledge
130:    */
131:    constructor(
132:        address _votingEscrow,
133:        address _delegationBoost,
134:        address _chestAddress,
135:        uint256 _minTargetVotes
136:    ) {

Event is missing indexed fields

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L85-L92

File: /contracts/WardenPledge.sol
85:    event NewPledge(
86:        address creator,
87:        address receiver,
88:        address rewardToken,
89:        uint256 targetVotes,
90:        uint256 rewardPerVote,
91:        uint256 endTimestamp
92:    );


115:    event ChestUpdated(address oldChest, address newChest);
116:    /** @notice Event emitted when xx */
117:    event PlatformFeeUpdated(uint256 oldfee, uint256 newFee);
118:    /** @notice Event emitted when xx */
119:    event MinTargetUpdated(uint256 oldMinTarget, uint256 newMinTargetVotes);

The nonReentrant modifier should occur before all other modifiers

This is a best-practice to protect against reentrancy in other modifiers

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L195

File: /contracts/WardenPledge.sol
195:    function pledge(uint256 pledgeId, uint256 amount, uint256 endTimestamp) external whenNotPaused nonReentrant {

206:    function pledgePercent(uint256 pledgeId, uint256 percent, uint256 endTimestamp) external whenNotPaused nonReentrant {

299:    function createPledge(
300:        address receiver,
301:        address rewardToken,
302:        uint256 targetVotes,
303:        uint256 rewardPerVote, // reward/veToken/second
304:        uint256 endTimestamp,
305:        uint256 maxTotalRewardAmount,
306:        uint256 maxFeeAmount
307:    ) external whenNotPaused nonReentrant returns(uint256){


368:    function extendPledge(
369:        uint256 pledgeId,
370:        uint256 newEndTimestamp,
371:        uint256 maxTotalRewardAmount,
372:        uint256 maxFeeAmount
373:    ) external whenNotPaused nonReentrant {


414:    function increasePledgeRewardPerVote(
415:        uint256 pledgeId,
416:        uint256 newRewardPerVote,
417:        uint256 maxTotalRewardAmount,
418:        uint256 maxFeeAmount
419:    ) external whenNotPaused nonReentrant {


456:    function retrievePledgeRewards(uint256 pledgeId, address receiver) external whenNotPaused nonReentrant {


488:    function closePledge(uint256 pledgeId, address receiver) external whenNotPaused nonReentrant {

Code Structure Deviates From Best-Practice

Order of Functions

Some constructs deviate from this recommended best-practice: External/public functions are mixed with internal/private ones Ordering helps readers identify which functions they can call and to find the constructor and fallback definitions easier.

Functions should be grouped according to their visibility and ordered:-

  • constructor
  • receive function (if exists)
  • fallback function (if exists)
  • external
  • public
  • internal
  • private

Our code does not seem to follow this recommendations . We have public functions declared before external functions

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L153

File: /contracts/WardenPledge.sol
153:    function pledgesIndex() public view returns(uint256){

163:    function getUserPledges(address user) external view returns(uint256[] memory){

Internal functions are also mixed with external functions rather than be ordered accordingly

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L222

Recommendation: Consider adopting recommended best-practice for code structure and layout. See Documentation

#0 - c4-judge

2022-11-12T00:58:07Z

kirk-baird marked the issue as grade-b

Awards

160.9924 USDC - $160.99

Labels

bug
G (Gas Optimization)
high quality report
grade-a
selected for report
G-37

External Links

FINDINGS

NB: Some functions have been truncated where neccessary to just show affected parts of the code.

Throught the report some places might be denoted with audit tags to show the actual place affected.

Using immutable on variables that are only set in the constructor and never after

Use immutable if you want to assign a permanent value at construction. Use constants if you already know the permanent value. Both get directly embedded in bytecode, saving SLOAD. Variables only set in the constructor and never edited afterwards should be marked as immutable, as it would avoid the expensive storage-writing operation in the constructor (around 20 000 gas per variable) and replace the expensive storage-reading operations (around 2100 gas per reading) to a less expensive value reading (3 gas)

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L60

File: /contracts/WardenPledge.sol
60:    IVotingEscrow public votingEscrow;

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L62

File: /contracts/WardenPledge.sol
62:    IBoostV2 public delegationBoost;

Use constants for variables whose value is known beforehand and is never changed

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L79

File: /contracts/WardenPledge.sol
79:    uint256 public minDelegationTime = 1 weeks;
diff --git a/contracts/WardenPledge.sol b/contracts/WardenPledge.sol
index beb990d..642a848 100644
--- a/contracts/WardenPledge.sol
+++ b/contracts/WardenPledge.sol
@@ -76,7 +76,7 @@ contract WardenPledge is Ownable, Pausable, ReentrancyGuard {
     uint256 public minTargetVotes;

     /** @notice Minimum delegation time, taken from veBoost contract */
-    uint256 public minDelegationTime = 1 weeks;
+    uint256 public constant minDelegationTime = 1 weeks;

Cache storage values in memory to minimize SLOADs

The code can be optimizhttps://github.com/code-423n4/2022-08-fiatdao-findings/blob/main/data/GalloDaSballo-G.mded by minimizing the number of SLOADs.

SLOADs are expensive (100 gas after the 1st one) compared to MLOADs/MSTOREs (3 gas each). Storage values read multiple times should instead be cached in memory the first time (costing 1 SLOAD) and then read from this cache to avoid multiple SLOADs.

NB: *Some functions have been truncated where neccessary to just show affected parts of the code

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L222-L274

WardenPledge.sol._pledge(): delegationBoost should be cached(Saves 4 SLOADs ~394 gas)

File: /contracts/WardenPledge.sol
222:    function _pledge(uint256 pledgeId, address user, uint256 amount, uint256 endTimestamp) internal {

240:        delegationBoost.checkpoint_user(user); //@audit: 1st SLOAD
241:        if(delegationBoost.allowance(user, address(this)) < amount) revert Errors.InsufficientAllowance(); //@audit: 2nd SLOAD
242:        if(delegationBoost.delegable_balance(user) < amount) revert Errors.CannotDelegate(); //@audit: 3rd SLOAD

245:        if(delegationBoost.adjusted_balance_of(pledgeParams.receiver) + amount > pledgeParams.targetVotes) revert Errors.TargetVotesOverflow(); //@audit: 4th SLOAD

248:        delegationBoost.boost( //@audit: 5th SLOAD
            pledgeParams.receiver,
            amount,
            endTimestamp,
            user
        );
diff --git a/contracts/WardenPledge.sol b/contracts/WardenPledge.sol
index beb990d..5b3d1bd 100644
--- a/contracts/WardenPledge.sol
+++ b/contracts/WardenPledge.sol
@@ -237,15 +237,16 @@ contract WardenPledge is Ownable, Pausable, ReentrancyGuard {
         uint256 boostDuration = endTimestamp - block.timestamp;

         // Check that the user has enough boost delegation available & set the correct allowance to this contract
-        delegationBoost.checkpoint_user(user);
-        if(delegationBoost.allowance(user, address(this)) < amount) revert Errors.InsufficientAllowance();
-        if(delegationBoost.delegable_balance(user) < amount) revert Errors.CannotDelegate();
+         IBoostV2 _delegationBoost = delegationBoost;
+        _delegationBoost.checkpoint_user(user);
+        if(_delegationBoost.allowance(user, address(this)) < amount) revert Errors.InsufficientAllowance();
+        if(_delegationBoost.delegable_balance(user) < amount) revert Errors.CannotDelegate();

         // Check that this will not go over the Pledge target of votes
-        if(delegationBoost.adjusted_balance_of(pledgeParams.receiver) + amount > pledgeParams.targetVotes) revert Errors.TargetVotesOverflow();
+        if(_delegationBoost.adjusted_balance_of(pledgeParams.receiver) + amount > pledgeParams.targetVotes) revert Errors.TargetVotesOverflow();

         // Creates the DelegationBoost
-        delegationBoost.boost(
+        _delegationBoost.boost(
             pledgeParams.receiver,
             amount,
             endTimestamp,

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L222-L274

WardenPledge.sol._pledge(): pledgeAvailableRewardAmounts[pledgeId] should be cached(saves 1 SLOAD ~97 gas)

File: /contracts/WardenPledge.sol
    function _pledge(uint256 pledgeId, address user, uint256 amount, uint256 endTimestamp) internal {


267:        if(rewardAmount > pledgeAvailableRewardAmounts[pledgeId]) revert Errors.RewardsBalanceTooLow(); //@audit: 1st access
268:        pledgeAvailableRewardAmounts[pledgeId] -= rewardAmount; //@audit: 2nd access
diff --git a/contracts/WardenPledge.sol b/contracts/WardenPledge.sol
index beb990d..2bb2cd2 100644
--- a/contracts/WardenPledge.sol
+++ b/contracts/WardenPledge.sol
@@ -263,9 +263,10 @@ contract WardenPledge is Ownable, Pausable, ReentrancyGuard {
         uint256 totalDelegatedAmount = ((bias * boostDuration) + bias) / 2;
         // Then we can calculate the total amount of rewards for this Boost
         uint256 rewardAmount = (totalDelegatedAmount * pledgeParams.rewardPerVote) / UNIT;
+        uint _pledgeAvailableRewardAmounts = pledgeAvailableRewardAmounts[pledgeId];

-        if(rewardAmount > pledgeAvailableRewardAmounts[pledgeId]) revert Errors.RewardsBalanceTooLow();
-        pledgeAvailableRewardAmounts[pledgeId] -= rewardAmount;
+        if(rewardAmount > _pledgeAvailableRewardAmounts) revert Errors.RewardsBalanceTooLow();
+        pledgeAvailableRewardAmounts[pledgeId] = _pledgeAvailableRewardAmounts - rewardAmount;

         // Send the rewards to the user
         IERC20(pledgeParams.rewardToken).safeTransfer(user, rewardAmount);

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L299-L358

WardenPledge.sol.createPledge(): minAmountRewardToken[rewardToken] should be cached(saves 1 SLOAD ~97 gas) - happy path

File: /contracts/WardenPledge.sol
299:    function createPledge(

312:        if(minAmountRewardToken[rewardToken] == 0) revert Errors.TokenNotWhitelisted(); //@audit: 1st access
313:        if(rewardPerVote < minAmountRewardToken[rewardToken]) revert Errors.RewardPerVoteTooLow(); //@audit: 2nd access
diff --git a/contracts/WardenPledge.sol b/contracts/WardenPledge.sol
index beb990d..247e5f8 100644
--- a/contracts/WardenPledge.sol
+++ b/contracts/WardenPledge.sol
@@ -309,8 +309,9 @@ contract WardenPledge is Ownable, Pausable, ReentrancyGuard {

         if(receiver == address(0) || rewardToken == address(0)) revert Errors.ZeroAddress();
         if(targetVotes < minTargetVotes) revert Errors.TargetVoteUnderMin();
-        if(minAmountRewardToken[rewardToken] == 0) revert Errors.TokenNotWhitelisted();
-        if(rewardPerVote < minAmountRewardToken[rewardToken]) revert Errors.RewardPerVoteTooLow();
+        uint256 _minAmountRewardToken = minAmountRewardToken[rewardToken]
+        if(_minAmountRewardToken == 0) revert Errors.TokenNotWhitelisted();
+        if(rewardPerVote < _minAmountRewardToken) revert Errors.RewardPerVoteTooLow();

         if(endTimestamp == 0) revert Errors.NullEndTimestamp();
         if(endTimestamp != _getRoundedTimestamp(endTimestamp)) revert Errors.InvalidEndTimestamp();

require() or revert() statements that check input arguments should be at the top of the function

Checks that involve constants should come before checks that involve state variables, function calls, and calculations. By doing these checks first, the function is able to revert before wasting a Gcoldsload (2100 gas) in a function that may ultimately revert in the unhappy case.

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L222-L274

File: /contracts/WardenPledge.sol
222:    function _pledge(uint256 pledgeId, address user, uint256 amount, uint256 endTimestamp) internal {
223:        if(pledgeId >= pledgesIndex()) revert Errors.InvalidPledgeID();
224:        if(amount == 0) revert Errors.NullValue();
diff --git a/contracts/WardenPledge.sol b/contracts/WardenPledge.sol
index beb990d..dfd3ff4 100644
--- a/contracts/WardenPledge.sol
+++ b/contracts/WardenPledge.sol
@@ -220,8 +220,9 @@ contract WardenPledge is Ownable, Pausable, ReentrancyGuard {
     * @param endTimestamp End of delegation
     */
     function _pledge(uint256 pledgeId, address user, uint256 amount, uint256 endTimestamp) internal {
-        if(pledgeId >= pledgesIndex()) revert Errors.InvalidPledgeID();
         if(amount == 0) revert Errors.NullValue();
+        if(pledgeId >= pledgesIndex()) revert Errors.InvalidPledgeID();

         // Load Pledge parameters & check the Pledge is still active
         Pledge memory pledgeParams = pledges[pledgeId];

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L456-L480

File: /contracts/WardenPledge.sol
456:    function retrievePledgeRewards(uint256 pledgeId, address receiver) external whenNotPaused nonReentrant {
457:        if(pledgeId >= pledgesIndex()) revert Errors.InvalidPledgeID();
458:        address creator = pledgeOwner[pledgeId];
459:        if(msg.sender != creator) revert Errors.NotPledgeCreator();
460:        if(receiver == address(0)) revert Errors.ZeroAddress();
diff --git a/contracts/WardenPledge.sol b/contracts/WardenPledge.sol
index beb990d..9c82ad9 100644
--- a/contracts/WardenPledge.sol
+++ b/contracts/WardenPledge.sol
@@ -454,10 +454,11 @@ contract WardenPledge is Ownable, Pausable, ReentrancyGuard {
     * @param receiver Address to receive the remaining rewards
     */
     function retrievePledgeRewards(uint256 pledgeId, address receiver) external whenNotPaused nonReentrant {
+        if(receiver == address(0)) revert Errors.ZeroAddress();
         if(pledgeId >= pledgesIndex()) revert Errors.InvalidPledgeID();
         address creator = pledgeOwner[pledgeId];
         if(msg.sender != creator) revert Errors.NotPledgeCreator();
-        if(receiver == address(0)) revert Errors.ZeroAddress();
+

         Pledge storage pledgeParams = pledges[pledgeId];
         if(pledgeParams.endTimestamp > block.timestamp) revert Errors.PledgeNotExpired();

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L488-L515

File: /contracts/WardenPledge.sol
488:    function closePledge(uint256 pledgeId, address receiver) external whenNotPaused nonReentrant {
489:        if(pledgeId >= pledgesIndex()) revert Errors.InvalidPledgeID();
490:        address creator = pledgeOwner[pledgeId];
491:        if(msg.sender != creator) revert Errors.NotPledgeCreator();
492:        if(receiver == address(0)) revert Errors.ZeroAddress();
diff --git a/contracts/WardenPledge.sol b/contracts/WardenPledge.sol
index beb990d..c06f2ee 100644
--- a/contracts/WardenPledge.sol
+++ b/contracts/WardenPledge.sol
@@ -486,10 +486,11 @@ contract WardenPledge is Ownable, Pausable, ReentrancyGuard {
     * @param receiver Address to receive the remaining rewards
     */
     function closePledge(uint256 pledgeId, address receiver) external whenNotPaused nonReentrant {
+        if(receiver == address(0)) revert Errors.ZeroAddress();
         if(pledgeId >= pledgesIndex()) revert Errors.InvalidPledgeID();
         address creator = pledgeOwner[pledgeId];
         if(msg.sender != creator) revert Errors.NotPledgeCreator();
-        if(receiver == address(0)) revert Errors.ZeroAddress();

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L525-L533

File: /contracts/WardenPledge.sol
525:    function _addRewardToken(address token, uint256 minRewardPerSecond) internal {
526:        if(minAmountRewardToken[token] != 0) revert Errors.AlreadyAllowedToken();
527:        if(token == address(0)) revert Errors.ZeroAddress();
528:        if(minRewardPerSecond == 0) revert Errors.NullValue();
diff --git a/contracts/WardenPledge.sol b/contracts/WardenPledge.sol
index beb990d..71d0087 100644
--- a/contracts/WardenPledge.sol
+++ b/contracts/WardenPledge.sol
@@ -523,10 +523,10 @@ contract WardenPledge is Ownable, Pausable, ReentrancyGuard {
     * @param minRewardPerSecond Minmum amount of reward per vote per second for the token
     */
     function _addRewardToken(address token, uint256 minRewardPerSecond) internal {
-        if(minAmountRewardToken[token] != 0) revert Errors.AlreadyAllowedToken();
         if(token == address(0)) revert Errors.ZeroAddress();
         if(minRewardPerSecond == 0) revert Errors.NullValue();

+        if(minAmountRewardToken[token] != 0) revert Errors.AlreadyAllowedToken();
+
         minAmountRewardToken[token] = minRewardPerSecond;

         emit NewRewardToken(token, minRewardPerSecond);

Using storage instead of memory for structs/arrays saves gas

When fetching data from a storage location, assigning the data to a memory variable causes all fields of the struct/array to be read from storage, which incurs a Gcoldsload (2100 gas) for each field of the struct/array. If the fields are read from the new memory variable, they incur an additional MLOAD rather than a cheap stack read. Instead of declearing the variable with the memory keyword, declaring the variable with the storage keyword and caching any fields that need to be re-read in stack variables, will be much cheaper, only incuring the Gcoldsload for the fields actually read. The only time it makes sense to read the whole struct/array into a memory variable, is if the full struct/array is being returned by the function, is being passed to a function that requires memory, or if the array/struct is being read from another memory array/struct

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L227

File: /contracts/WardenPledge.sol
227:        Pledge memory pledgeParams = pledges[pledgeId];

Using unchecked blocks to save gas

Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn’t possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked block see resource

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L268

File: /contracts/WardenPledge.sol
268:        pledgeAvailableRewardAmounts[pledgeId] -= rewardAmount;

The operation pledgeAvailableRewardAmounts[pledgeId] -= rewardAmount; cannot underflow due to the check on Line 267 that ensures that pledgeAvailableRewardAmounts[pledgeId] is greater than rewardAmount before perfoming the arithmetic operation

#0 - c4-judge

2022-11-12T00:57:21Z

kirk-baird marked the issue as grade-a

#1 - c4-judge

2022-11-12T00:57:49Z

kirk-baird marked the issue as selected for report

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax Β© 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter