Neo Tokyo contest - ReyAdmirado's results

A staking contract for the crypto gaming illuminati.

General Information

Platform: Code4rena

Start Date: 08/03/2023

Pot Size: $60,500 USDC

Total HM: 2

Participants: 123

Period: 7 days

Judge: hansfriese

Id: 220

League: ETH

Neo Tokyo

Findings Distribution

Researcher Performance

Rank: 55/123

Findings: 1

Award: $149.89

Gas:
grade-a

🌟 Selected for report: 0

🚀 Solo Findings: 0

1. state variables can be packed into fewer storage slots

If variables occupying the same slot are both written the same function or by the constructor, avoids a separate Gsset (20000 gas). Reads of the variables are also cheaper.

lpLocked(#L511) and LP(#232) can be put together in the same slot if they be declared near each other. they are accessed in the same function only once so mainly saves Gsset of 20000 for a slot and near 40 gas.

2. expressions for constant values such as a call to keccak256(), should use immutable rather than constant

3. Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key’s keccak256 hash (Gkeccak256 - 30 gas) and that calculation’s associated stack operations.

stakedS1(#L372) and stakedS2(#L405) and stakerLPPosition(#L434) can be put together to save lots of gas because they are used in the same function multiple times

_stakerS1Position(#L378) and _stakerS2Position(#L411) can be put together to save lots of gas because they are used in the same function multiple times

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

_stakerS1Position[_staker].length can be cached before #L715 because its used twice (in #L716 and #L717) and the cached version can be used instead

_stakerS2Position[_staker].length can be cached before #L732 because its used twice (in #L733 and #L734) and the cached version can be used instead

stakerLPPosition[msg.sender].multiplier will be checked in #L1144 and if it fails we gonna have another complex SLOAD so we can cache it before the first check and risk losing 3 gas if the first check happens to be true otherwise we reduce a complex SLOAD. cache it before #L1144

cache LP because its being read twice (in #L1137 and #L1170). so cache it before #L1137 to save 97 gas.

stakedCitizen.timelockEndTime is being read twice and should be cached to save gas. first SLOAD is in #L1467 and the second one depending on if the #L1467 condition be true or false will be #L1467 or #L1472. so cache it before #L1467

possibly reduce a complex SLOAD by risking losing 3 gas. caching stakedCitizen.stakedBytes will save gas if the condition happens and if it doesnt we only lose 3 gas for caching. cache before #L1477

possibly reduce a complex SLOAD by risking losing 3 gas. caching stakedCitizen.stakedVaultId will save gas if the condition happens and if it doesnt we only lose 3 gas for caching. cache before #L1482

cache LP because its being read twice (in #L1618 and #L1641). so cache it before #L1618 to save 97 gas.

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

Using the addition operator instead of plus-equals saves gas (13 or 113 each dependant on the usage see here)

6. can make the variable outside the loop to save gas

make the variable outside and only give the value to variable inside

7. using calldata instead of memory for read-only arguments in external functions saves gas

When a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution.

8. Ternary over if ... else

Using ternary operator instead of the if else statement saves gas.

9. before some functions we should check some variables for possible gas save

before transfer we should check for amount being 0 so the function doesnt run when its not gonna do anything

amount

10. instead of calculating a statevar with keccak256() every time the contract is made pre calculate them before and only give the result to a constant

11. Optimize names to save gas

Contracts most called functions could simply save gas by function ordering via Method ID. Calling a function at runtime will be cheaper if the function is positioned earlier in the order (has a relatively lower Method ID) because 22 gas are added to the cost of a function for every position that came before it. The caller can save on gas if you prioritize most called functions.

See more here. you can use this tool to get the optimized version for function and properties signitures

12. Non-strict inequalities are cheaper than strict ones

In the EVM, there is no opcode for non-strict inequalities (>=, <=) and two operations are performed (> + = or < + =). consider replacing >= with the strict counterpart > and add - 1 to the second side

13. Setting the constructor to payable

You can cut out 10 opcodes in the creation-time EVM bytecode if you declare a constructor payable. Making the constructor payable eliminates the need for an initial check of msg.value == 0 and saves 13 gas on deployment with no security risks.

14. instead of assigning to zero use delete

assigning to zero uses more gas than using delete , and they both assign variable to default value. so it is encouraged if the data is no longer needed use delete instead.

5 instances here

3 instances here

1 here

15. part of the code can be pre calculated

just pre calculate the answer of these parts and use the answer inside the code instead. use a comment to show what it was to the readers

_DIVISOR / _DIVISOR can be pre calculated and given to the contract as a constant. this stops one operation

100 * timelockMultiplier / _DIVISOR can just use timelockMultiplier because the other operations result will be 1

100 / _BYTES_PER_POINT can be pre calculated and given to the contract as a constant. this stops one operation

amount * 100 / 1e18 * timelockMultiplier / _DIVISOR we can pre calculate the 100 / 1e18 / _DIVISOR part to reduce use of 2 operations

100 * _DIVISOR can be pre calculated and given to the contract as a constant. this stops one operation

amount * 100 / 1e18 * lpPosition.multiplier / _DIVISOR we can pre calculate the 100 / 1e18 / _DIVISOR part to reduce use of 2 operations

#0 - c4-judge

2023-03-17T04:00:20Z

hansfriese marked the issue as grade-a

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