veToken Finance contest - 0xkatana's results

Lock more veAsset permanently.

General Information

Platform: Code4rena

Start Date: 26/05/2022

Pot Size: $75,000 USDT

Total HM: 31

Participants: 71

Period: 7 days

Judge: GalloDaSballo

Total Solo HM: 18

Id: 126

League: ETH

veToken Finance

Findings Distribution

Researcher Performance

Rank: 67/71

Findings: 1

Award: $52.07

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest Page Github Contest Repository Github Findings Repository veToken Finance

Findings Information

🌟 Selected for report: IllIllI

Also found by: 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, Cityscape, Dravee, ElKu, FSchmoede, Funen, GalloDaSballo, Hawkeye, Kaiziron, MiloTruck, Randyyy, RoiEvenHaim, Ruhum, SecureZeroX, SmartSek, TerrierLover, TomJ, Tomio, WatchPug, Waze, _Adam, asutorufos, c3phas, catchup, cogitoergosumsw, delfin454000, ellahi, fatherOfBlocks, gzeon, hansfriese, horsefacts, jonatascm, minhquanym, oyc_109, pauliax, reassor, robee, sach1r0, saian, sashik_eth, simon135, z3s

Awards

52.0655 USDT - $52.07

Labels

bug

G (Gas Optimization)

External Links

Link to GitHub issue

[G-01] Use prefix not postfix in loops

Using a prefix increment (++i) instead of a postfix increment (i++) saves gas for each loop cycle and so can have a big gas impact when the loop executes on a large number of elements.

Recommended Mitigation Steps

Use prefix not postfix to increment in a loop

[G-02] Redundant zero initialization

Solidity does not recognize null as a value, so uint variables are initialized to zero. Setting a uint variable to zero is redundant and can waste gas.

Recommended Mitigation Steps

Remove the redundant zero initialization uint256 i; instead of uint256 i = 0;

[G-03] Split up require statements instead of &&

Combining require statement conditions with && logic uses unnecessary gas. It is better to split up each part of the logical statement into a separate require statements

One example is

require(msg.sender == poolManager && !isShutdown, "!add");

This can be improved to

require(msg.sender == poolManager);
require(!isShutdown, "!add");

Several places had require statements with at least one logical "and". Instead, split into two to save gas https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/Booster.sol#L261 https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/Booster.sol#L262 https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/StashFactory.sol#L87

Recommended Mitigation Steps

Use separate require statements instead of concatenating with &&

[G-04] Cache array length before loop

Caching the array length outside a loop saves reading it on each iteration, as long as the array's length is not changed during the loop. This saves gas.

Recommended Mitigation Steps

Cache the array length before the for loop

[G-05] Use != 0 instead of > 0

Using > 0 uses slightly more gas than using != 0. Use != 0 when comparing uint variables to zero, which cannot hold values below zero

Recommended Mitigation Steps

Replace > 0 with != 0 to save gas

[G-06] Use type(uint).max

The value 0xffffffffffffffffffffffffffff is used but using type(uint112).max uses less gas than this constant value. Source https://forum.openzeppelin.com/t/using-the-maximum-integer-in-solidity/3000/13

Found here https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/helper/FixedPoint.sol#L25

Recommended Mitigation Steps

Use type(uint112).max

[G-07] For loop incrementing can be unsafe

For loops that use i++ do not need to use safemath for this operation because the loop would run out of gas long before this point. Making this addition operation unsafe using unchecked saves gas.

Sample code to make the for loop increment unsafe

for (uint i = 0; i < length; i = unchecked_inc(i)) {
    // do something that doesn't change the value of i
}

function unchecked_inc(uint i) returns (uint) {
    unchecked {
        return i + 1;
    }
}

Idea borrowed from https://gist.github.com/hrkrshnn/ee8fabd532058307229d65dcd5836ddc#the-increment-in-for-loop-post-condition-can-be-made-unchecked

Recommended Mitigation Steps

Make the increment in for loops unsafe to save gas

[G-08] Use iszero assembly for zero checks

Comparing a value to zero can be done using the iszero EVM opcode. This can save gas

Source from t11s https://twitter.com/transmissions11/status/1474465495243898885

Recommended Mitigation Steps

Use the assembly iszero evm opcode to compare values to zero

[G-09] Add payable to functions that won't receive ETH

Identifying a function as payable saves gas. Functions that have a modifier like onlyOwner cannot be called by normal users and will not mistakenly receive ETH. These functions can be payable to save gas.

Recommended Mitigation Steps

Add payable to these functions for gas savings

[G-10] Use internal function in place of modifier

An internal function can save gas vs. a modifier. A modifier inlines the code of the original function but an internal function does not.

Source https://blog.polymath.network/solidity-tips-and-tricks-to-save-gas-and-reduce-bytecode-size-c44580b218e6#dde7

Many modifiers can use this change https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VE3DLocker.sol#L798 https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/BaseRewardPool.sol#L137 https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VE3DRewardPool.sol#L146 https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/Migrations.sol#L8

Recommended Mitigation Steps

Use internal functions in place of modifiers to save gas.

[G-11] Use newer solidity version

A solidity version before 0.8.X is used in some contracts. The latest release of solidity includes changes that can provide gas savings. The improvements include:

Safemath by default from 0.8.0 (can be more gas efficient than some library based safemath).
Low level inliner from 0.8.2, leads to cheaper runtime gas. Especially relevant when the contract has small functions. For example, OpenZeppelin libraries typically have a lot of small helper functions and if they are not inlined, they cost an additional 20 to 40 gas because of 2 extra jump instructions and additional stack operations needed for function calls.
Optimizer improvements in packed structs: Before 0.8.3, storing packed structs, in some cases used an additional storage read operation. After EIP-2929, if the slot was already cold, this means unnecessary stack operations and extra deploy time costs. However, if the slot was already warm, this means additional cost of 100 gas alongside the same unnecessary stack operations and extra deploy time costs.
Custom errors from 0.8.4, leads to cheaper deploy time cost and run time cost. Note: the run time cost is only relevant when the revert condition is met. In short, replace revert strings by custom errors.
Solidity version 0.8.13 can save more gas with Yul IR pipeline

Source https://gist.github.com/hrkrshnn/ee8fabd532058307229d65dcd5836ddc#upgrade-to-at-least-084

Files with older solidity versions: https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/helper/BitMath.sol https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/helper/FullMath.sol https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/helper/FixedPoint.sol https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/helper/Babylonian.sol https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/Migrations.sol

Recommended Mitigation Steps

Use solidity release 0.8.13 with Yul IR pipeline and other improvements for gas savings

[G-12] Use simple comparison in if statement

The comparison operators >= and <= use more gas than >, <, or ==. Replacing the >= and ≤ operators with a comparison operator that has an opcode in the EVM saves gas

The existing code is https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/BaseRewardPool.sol#L329-L336

if (block.timestamp >= periodFinish) {
    rewardRate = reward.div(duration);
} else {
    uint256 remaining = periodFinish.sub(block.timestamp);
    uint256 leftover = remaining.mul(rewardRate);
    reward = reward.add(leftover);
    rewardRate = reward.div(duration);
}

A simple comparison can be used for gas savings by reversing the logic

if (block.timestamp < periodFinish) {
    uint256 remaining = periodFinish.sub(block.timestamp);
    uint256 leftover = remaining.mul(rewardRate);
    reward = reward.add(leftover);
    rewardRate = reward.div(duration);
} else {
    rewardRate = reward.div(duration);
}

Recommended Mitigation Steps

Replace the comparison operator and reverse the logic to save gas using the suggestions above

[G-13] Bitshift for divide by 2

When multiply or dividing by a power of two, it is cheaper to bitshift than to use standard math operations.

There is a divide by 2 operation on these lines https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/helper/FixedPoint.sol#L167 https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VE3DLocker.sol#L428

Recommended Mitigation Steps

Bitshift right by one bit instead of dividing by 2 to save gas

[G-14] Non-public variables save gas

Many constant variables are public, but changing the visibility of these variables to private or internal can save gas.

Recommended Mitigation Steps

Declare some public variables as private or internal to save gas

[G-15] Write contracts in vyper

The contracts are all written entirely in solidity. Writing contracts with vyper instead of solidity can save gas.

Source https://twitter.com/eiber_david/status/1515737811881807876

Recommended Mitigation Steps

Write some or all of the contracts in vyper to save gas

[G-16] Remove SafeMath import

Some contracts are written with solidity 0.8.7. These contracts do not need to import SafeMath because solidity has safemath checks with standard math operators in solidity 0.8.X. Removing this import saves gas on deployment.

Safemath import example in solidity 0.8.7 contract https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/BaseRewardPool.sol#L42

Recommended Mitigation Steps

Remove safemath import in solidity 0.8.X contracts

#0 - GalloDaSballo
2022-07-14T01:45:38Z

Should save 500 - 1000 gas, feels automated and misses the nuance between avoiding storage and memory

veToken Finance contest - 0xkatana's results

Lock more veAsset permanently.

General Information

Findings Distribution

Researcher Performance

External Links

[G-45] Gas Report - $52.07

Findings Information

Awards

Labels

External Links

[G-01] Use prefix not postfix in loops

Recommended Mitigation Steps

[G-02] Redundant zero initialization

Recommended Mitigation Steps

[G-03] Split up require statements instead of &&

Recommended Mitigation Steps

[G-04] Cache array length before loop

Recommended Mitigation Steps

[G-05] Use != 0 instead of > 0

Recommended Mitigation Steps

[G-06] Use type(uint).max

Recommended Mitigation Steps

[G-07] For loop incrementing can be unsafe

Recommended Mitigation Steps

[G-08] Use iszero assembly for zero checks

Recommended Mitigation Steps

[G-09] Add payable to functions that won't receive ETH

Recommended Mitigation Steps

[G-10] Use internal function in place of modifier

Recommended Mitigation Steps

[G-11] Use newer solidity version

Recommended Mitigation Steps

[G-12] Use simple comparison in if statement

Recommended Mitigation Steps

[G-13] Bitshift for divide by 2

Recommended Mitigation Steps

[G-14] Non-public variables save gas

Recommended Mitigation Steps

[G-15] Write contracts in vyper

Recommended Mitigation Steps

[G-16] Remove SafeMath import

Recommended Mitigation Steps

#0 - GalloDaSballo2022-07-14T01:45:38Z

#0 - GalloDaSballo
2022-07-14T01:45:38Z