Paladin - Warden Pledges contest - Awesome's results

block.timestamp is unreliable

Using block.timestamp as part of the time checks could be slightly modified by miners/validators to favor them in contracts that contain logic heavily dependent on them.

Consider this problem and warn users that a scenario like this could occur. If the change of timestamps will not affect the protocol in any way, consider documenting the reasoning and writing tests enforcing that these guarantees will be preserved even if the code changes in the future.

There are 8 instances of this issue:

Line 229, Line 237, Line 319, Line 380, Line 426, Line 430, Line 463, Line 496

Typos

Stick to the proper spelling otherwise, it will make it decrease readability

There are 9 instances of this issue consider making the following changes:

Change "minmum" to "minimum" Line 523, Line 539, Line 558, Line 568

Change "protocalFeeRatio" to "protocolFeeRatio" Line 71

Change "reards" to "rewards" Line 339

Change "InequalArraySizes" to "UnequalArraySizes" Line 545

Change "taget" to "target" and "balacne" to "balance" Line 292

Change "Platfrom" to "Platform" Line 621-622

Use CamelCase

Consider sticking to using the CamelCase naming convention as it is highly recommended to follow these guidelines to help improve the readability of the source code for example:

Line 117:    event PlatformFeeUpdated(uint256 oldfee, uint256 newFee);

Line 117

Should be refactored to

Line 117:    event PlatformFeeUpdated(uint256 oldFee, uint256 newFee);

Remove Shorthand Addition/Subtraction Assignment

Instead of using the shorthand of addition/subtraction assignment operators (+=, -=) it costs less to remove the shorthand ( x += y same as x = x+y ) saves ~22 gas There are 3 instances of this issue:

Line 445

Line 340

Line 401

as an example Line 445

Line 445:    pledgeAvailableRewardAmounts[pledgeId] += totalRewardAmount;

could be refactored to

Line 445:    pledgeAvailableRewardAmounts[pledgeId] = pledgeAvailableRewardAmounts[pledgeId] + totalRewardAmount;

Function Order Affects Gas Consumption

public/external function names and public member variable names can be optimized to save gas. See this link for an example of how it works. Below are the interfaces/abstract contracts that can be optimized so that the most frequently-called functions use the least amount of gas possible during method lookup. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted

Functions With Access Control Cheaper if Payable

A function with access control marked as payable will be cheaper for legitimate callers: the compiler removes checks for msg.value, saving approximately 20 gas per function call.

There are 10 instances of this issue::

File: WardenPledge.sol Line 541

File: WardenPledge.sol Line 560

File: WardenPledge.sol Line 570

File: WardenPledge.sol Line 585

File: WardenPledge.sol Line 599

File: WardenPledge.sol Line 612

File: WardenPledge.sol Line 625

File: WardenPledge.sol Line 636

File: WardenPledge.sol Line 643

File: WardenPledge.sol Line 653

Uncheck arithmetics operations that can’t underflow/overflow

Solidity version 0.8+ comes with an implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn’t possible, some gas can be saved by using an unchecked block. For example:

Line 445:    pledgeAvailableRewardAmounts[pledgeId] += totalRewardAmount;

Line 445 could be refactored to

Line 445:    unchecked { pledgeAvailableRewardAmounts[pledgeId] += totalRewardAmount; }

Line 340 Line 401

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 declaring 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 incurring 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.

There are 2 instances of this issue:

Line 227

Line 318