Rubicon contest - 0xkatana's results

An order book protocol for Ethereum, built on L2s.

General Information

Platform: Code4rena

Start Date: 23/05/2022

Pot Size: $50,000 USDC

Total HM: 44

Participants: 99

Period: 5 days

Judge: hickuphh3

Total Solo HM: 11

Id: 129

League: ETH

Rubicon

Findings Distribution

Researcher Performance

Rank: 85/99

Findings: 1

Award: $50.15

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest Page Github Contest Repository Github Findings Repository Rubicon

Findings Information

🌟 Selected for report: IllIllI

Also found by: 0x1f8b, 0x4non, 0xDjango, 0xNazgul, 0xf15ers, 0xkatana, Chom, DavidGialdi, Dravee, ElKu, FSchmoede, Fitraldys, Funen, GimelSec, JC, Kaiziron, MaratCerby, Metatron, MiloTruck, Picodes, Randyyy, RoiEvenHaim, SmartSek, Tomio, UnusualTurtle, WatchPug, Waze, _Adam, antonttc, asutorufos, berndartmueller, blackscale, blockdev, c3phas, catchup, csanuragjain, defsec, delfin454000, ellahi, fatherOfBlocks, gzeon, hansfriese, ilan, joestakey, minhquanym, oyc_109, pauliax, pedroais, reassor, rfa, rotcivegaf, sach1r0, samruna, sashik_eth, simon135, z3s

Awards

50.1514 USDC - $50.15

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(_bathAssetAddress != address(0) && _bathQuoteAddress != address(0), "tokenToBathToken error");

This can be improved to

require(_bathAssetAddress != address(0));
require(_bathQuoteAddress != address(0));

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.

This was found in many places https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/rubiconPools/BathToken.sol#L635 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/rubiconPools/BathPair.sol#L311 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/rubiconPools/BathPair.sol#L582 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/RubiconRouter.sol#L169 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/RubiconRouter.sol#L227

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(uint256).max

The value 2**256 - 1 is used for some variables but using type(uint256).max uses less gas than this constant value. Source https://forum.openzeppelin.com/t/using-the-maximum-integer-in-solidity/3000/13

There are five places where this change can save gas https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/rubiconPools/BathToken.sol#L214 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/rubiconPools/BathToken.sol#L256 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/rubiconPools/BathToken.sol#L421 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/rubiconPools/BathToken.sol#L451 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/RubiconRouter.sol#L157

Recommended Mitigation Steps

Use type(uint256).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] Save gas with unchecked

Use unchecked math when there is no overflow risk to save gas. Before index is decreased in remove it is checked for zero condition. This means index will not underflow and can be unchecked.

These subtractions do not need to be checked for underflows because they are in if statements or while loops that confirm the underflow will not happen https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/RubiconMarket.sol#L852 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/RubiconMarket.sol#L891 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/RubiconMarket.sol#L917 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/RubiconMarket.sol#L941

Recommended Mitigation Steps

Add unchecked around math that can't overflow for gas savings. In Solidity before 0.8.0, use the normal math operators instead of safe math functions.

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

Identifying a function as payable saves gas. Functions that have a modifier like onlyOwner or auth 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-11] 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

Recommended Mitigation Steps

Use internal functions in place of modifiers to save gas.

[G-12] Use newer solidity version

A solidity version before 0.8.X is used. The latest release of solidity includes changes that can provide gas savings. The improvements include: The advantages of versions 0.8.* over <0.8.0 are:

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

Recommended Mitigation Steps

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

[G-13] Use simple comparison in trinary logic

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-rubicon/blob/main/contracts/RubiconMarket.sol#L62 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/RubiconMarket.sol#L70

return x >= y ? x : y;

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

return x < y ? y : x;

Other places with this https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/RubiconMarket.sol#L58 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/RubiconMarket.sol#L66

Recommended Mitigation Steps

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

[G-14] 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-rubicon/blob/main/contracts/RubiconMarket.sol#L849

if (pay_amt >= offers[offerId].buy_amt) {
    //If amount to sell is higher or equal than current offer amount to buy
    fill_amt = add(fill_amt, offers[offerId].pay_amt); //Add amount bought to acumulator
    pay_amt = sub(pay_amt, offers[offerId].buy_amt); //Decrease amount to sell
    take(bytes32(offerId), uint128(offers[offerId].pay_amt)); //We take the whole offer
} else {
    // if lower
    uint256 baux = rmul(
        pay_amt * 10**9,
        rdiv(offers[offerId].pay_amt, offers[offerId].buy_amt)
    ) / 10**9;
    fill_amt = add(fill_amt, baux); //Add amount bought to acumulator
    take(bytes32(offerId), uint128(baux)); //We take the portion of the offer that we need
    pay_amt = 0; //All amount is sold
}

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

if (pay_amt < offers[offerId].buy_amt) {
    // if lower
    uint256 baux = rmul(
        pay_amt * 10**9,
        rdiv(offers[offerId].pay_amt, offers[offerId].buy_amt)
    ) / 10**9;
    fill_amt = add(fill_amt, baux); //Add amount bought to acumulator
    take(bytes32(offerId), uint128(baux)); //We take the portion of the offer that we need
    pay_amt = 0; //All amount is sold
} else {
    //If amount to sell is higher or equal than current offer amount to buy
    fill_amt = add(fill_amt, offers[offerId].pay_amt); //Add amount bought to acumulator
    pay_amt = sub(pay_amt, offers[offerId].buy_amt); //Decrease amount to sell
    take(bytes32(offerId), uint128(offers[offerId].pay_amt)); //We take the whole offer
}

Other places with this https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/RubiconMarket.sol#L888

Recommended Mitigation Steps

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

[G-15] 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-rubicon/blob/main/contracts/RubiconMarket.sol#L77 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/RubiconMarket.sol#L81 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/RubiconMarket.sol#L85 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/RubiconMarket.sol#L89

Recommended Mitigation Steps

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

[G-16] Non-public variables save gas

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

https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/peripheral_contracts/TokenWithFaucet.sol#L16 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/peripheral_contracts/SafeMathE.sol#L23 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/peripheral_contracts/SafeMathE.sol#L24 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/rubiconPools/BathToken.sol#L70 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/RubiconMarket.sol#L73 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/RubiconMarket.sol#L74

Recommended Mitigation Steps

Declare some public variables as private or internal to save gas

[G-17] Use calldata instead of memory for function arguments

Using calldata instead of memory for function arguments saves gas sometimes. This can happen when a function is called externally and the memory array values are kept in calldata and copied to memory during ABI decoding (using the opcode calldataload and mstore). If the array is used in a for loop, arr[i] accesses the value in memory using a mload. If calldata is used instead, then instead of going via memory, the value is directly read from calldata using calldataload. That is, there are no intermediate memory operations that carries this value.

Many cases of function arguments using memory instead of calldata can use this improvement to save gas https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/rubiconPools/BathPair.sol#L413-L417 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/rubiconPools/BathPair.sol#L464-L469 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/rubiconPools/BathPair.sol#L578

Source https://gist.github.com/hrkrshnn/ee8fabd532058307229d65dcd5836ddc#use-calldata-instead-of-memory-for-function-parameters

Recommended Mitigation Steps

Change function arguments from memory to calldata

[G-18] 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 doggo demonstrates https://twitter.com/fubuloubu/status/1528179581974417414?t=-hcq_26JFDaHdAQZ-wYxCA&s=19

Recommended Mitigation Steps

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

Rubicon contest - 0xkatana's results

An order book protocol for Ethereum, built on L2s.

General Information

Findings Distribution

Researcher Performance

External Links

[G-06] Gas Report - $50.15

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(uint256).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] Save gas with unchecked

Recommended Mitigation Steps

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

Recommended Mitigation Steps

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

Recommended Mitigation Steps

[G-12] Use newer solidity version

Recommended Mitigation Steps

[G-13] Use simple comparison in trinary logic

Recommended Mitigation Steps

[G-14] Use simple comparison in if statement

Recommended Mitigation Steps

[G-15] Bitshift for divide by 2

Recommended Mitigation Steps

[G-16] Non-public variables save gas

Recommended Mitigation Steps

[G-17] Use calldata instead of memory for function arguments

Recommended Mitigation Steps

[G-18] Write contracts in vyper

Recommended Mitigation Steps