Paladin - Warden Pledges contest - ReyAdmirado's results

QA

1. typo

protocalFeeRatio --> protocolFeeRatio (this is a mistake in a statevar and it has 6 instances)

• WardenPledge.sol#L41

taget --> target

• WardenPledge.sol#L292

balacne --> balance

• WardenPledge.sol#L292

reards --> rewards

• WardenPledge.sol#L339

Minmum --> Minimum (4 instances)

• WardenPledge.sol#L523

Platfrom --> Platform

• WardenPledge.sol#L621

2. 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.

• WardenPledge.sol#L94

• WardenPledge.sol#L96

• WardenPledge.sol#L98

• WardenPledge.sol#L102

• WardenPledge.sol#L105

3. Code Structure Deviates From Best-Practice

The best-practice layout for a contract should follow the following order: state variables, events, modifiers, constructor and functions. Function ordering helps readers identify which functions they can call and find constructor and fallback functions easier. Functions should be grouped according to their visibility and ordered as: constructor, receive function (if exists), fallback function (if exists), external, public, internal, private.

wrong place for struct

• WardenPledge.sol#L279

external should be after each other first then public then internal then private

• WardenPledge.sol#L153
• WardenPledge.sol#L181
• WardenPledge.sol#L222
• WardenPledge.sol#L525

4. Use Underscores for Number Literals

There are multiple occasions where certain numbers have been hardcoded, either in variables or in the code itself. Large numbers can become hard to read.

• WardenPledge.sol#L23
• WardenPledge.sol#L24

5. Constant redefined elsewhere

Consider defining in only one contract so that values cannot become out of sync when only one location is updated. A cheap way to store constants in a single location is to create an internal constant in a library. If the variable is a local cache of another contract’s value, consider making the cache variable internal or private, which will require external users to query the contract with the source of truth, so that callers don’t get out of sync. https://medium.com/coinmonks/gas-cost-of-solidity-library-functions-dbe0cedd4678

• WardenPledge.sol#L22
• WardenPledge.sol#L23
• WardenPledge.sol#L24

6. consider changing this state var to constant to reduce confusion

minDelegationTime state var does not change and its always 1 week, change it to constant to improve readability of the contract

• WardenPledge.sol#L79

7. Missing event emitting

Event is never emitted, consider adding emit in the position intended

IncreasePledgeTargetVotes

• WardenPledge.sol#L96

8. Outdated compiler version

Using old versions of Solidity prevents benefits of bug fixes and newer security checks. Using the latest versions might make contracts susceptible to undiscovered compiler bugs

Gas

1. State variables only set in the constructor should be declared immutable.

Avoids a Gsset (20000 gas) in the constructor, and replaces each Gwarmaccess (100 gas) with a PUSH32 (3 gas).

votingEscrow

• WardenPledge.sol#L60

delegationBoost

• WardenPledge.sol#L62

2. state variable should be declared as constant which saves lots of gas

• WardenPledge.sol#L79

3. state variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second+ access of a state variable within a function. Caching of a state variable replace each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.

mostly the if statements gonna pass, so its better to cache minAmountRewardToken[rewardToken] at this line

• WardenPledge.sol#L312

consider caching pledgeParams.endTimestamp earlier before this line because the if statement mostly gonna pass

• WardenPledge.sol#L380

pledgeParams.rewardToken

• WardenPledge.sol#L394

same here

• WardenPledge.sol#L438

4. Stack variable used as a cheaper cache for a state variable is only used once

If the variable is only accessed once, it’s cheaper to use the state variable directly that one time, and save the 3 gas the extra stack assignment would spend

amount

• WardenPledge.sol#L209

slope

• WardenPledge.sol#L256

totalDelegatedAmount

• WardenPledge.sol#L263

remainingDuration

• WardenPledge.sol#L430

rewardPerVoteDiff

• WardenPledge.sol#L431

creator

• WardenPledge.sol#L458
• WardenPledge.sol#L490

5. Add unchecked {} for subtractions where the operands cannot underflow because of a previous if statement

if(a <= b); x = b - a => if(a <= b); unchecked { x = b - a } this will stop the check for overflow and underflow so it will save gas

endTimestamp is bigger than block.timestamp because its checked in line 229. (endTimestamp got pledgeParams.endTimestamp value in line 233):

• WardenPledge.sol#L237

checked in line 383

• WardenPledge.sol#L385

checked in line 426

• WardenPledge.sol#L430

checked in line 429

• WardenPledge.sol#L431

6. <x> += <y> costs more gas than <x> = <x> + <y> for state variables

Using the addition operator instead of plus-equals saves gas

• WardenPledge.sol#L268

• WardenPledge.sol#L340

• WardenPledge.sol#L401

• WardenPledge.sol#L445

7. Avoid a function use by swapping two sides of || in if statement

There is a chance that the first part will be true so the second part doesn’t need to be checked, so it is better to use the part that we have instead of the part that needs to be called.

• WardenPledge.sol#L383

8. use a more recent version of solidity

Use a solidity version of at least 0.8.13 to get the ability to use using for with a list of free functions

9. usage of uint/int smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size. Each operation involving a uint8 costs an extra 22-28 gas (depending on whether the other operand is also a variable of type uint8) as compared to ones involving uint256, due to the compiler having to clear the higher bits of the memory word before operating on the uint8, as well as the associated stack operations of doing so. Use a larger size then downcast where needed https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Use a larger size then downcast where needed

• WardenPledge.sol#L41