code-423n4 / 2022-02-redacted-cartel-findings

0 stars 0 forks source link

Gas Optimizations #53

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

1. Loops can be more efficient

Impact

The local variable used as for loop index need not be initialized to 0 because the default value is 0. Avoiding this anti-pattern can save a few opcodes and therefore a tiny bit of gas.

Proof of Concept

for (uint256 i = 0; i < distributions.length; i++) {

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/BribeVault.sol#L269

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/RewardDistributor.sol#L82

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/RewardDistributor.sol#L103

Remix

Recommended Mitigation Steps

Remove explicit 0 initialization of for loop index variable.

2. Long Revert Strings

Impact

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition has been met.

Proof of Concept

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/BribeVault.sol#L265

There are several other places throughout the codebase where the same optimization can be used.

Tools

https://planetcalc.com/9029/

Recommended Mitigation Steps

Shorten the revert strings to fit in 32 bytes.

3. > 0 can be replaced with != 0 for gas optimisation

Vulnerability details

Impact

!= 0 is a cheaper operation compared to > 0, when dealing with uint.

Proof of Concept

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/BribeVault.sol#L279 There are several other places throughout the codebase where the same optimization can be used.

Tools

Remix

Recommended Mitigation Steps

4. Upgrade pragma to at least 0.8.4

Impact

Using newer compiler versions and the optimizer gives gas optimizations and additional safety checks are available for free. The advantages of versions 0.8.* over <0.8.0 are: • Safemath by default from 0.8.0 (can be more gas efficient than 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. https://github.com/code-423n4/2021-10-mochi-findings/issues/34

Tools

Remix

Recommended Mitigation Steps

Consider to upgrade pragma to at least 0.8.4.

5. 10 ** 18 can be changed to 1e18

Vulnerability details

Impact

10 ** 18 can be changed to 1e18 and save some gas

Proof of Concept

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/ThecosomataETH.sol#L103 https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/ThecosomataETH.sol#L108

Tools

Remix

Recommended Mitigation Steps

6. Gas optimization on 2**256 - 1

Vulnerability details

Impact

when using 2**256 - 1 it takes more gas, than using type(uint256).max

Proof of Concept

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/ThecosomataETH.sol#L68-L69

Tools

Remix

Recommended Mitigation Steps

Use type(uint256).max

7. Change string to byteX if possible

Vulnerability details

Impact

In the Constants library, declaring the constants with type bytes32 can save gas.

Proof of Concept

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/TokemakBribe.sol#L31

Tools

https://medium.com/layerx/how-to-reduce-gas-cost-in-solidity-f2e5321e0395#2a78

Recommended Mitigation Steps

8. Variable is set on initialization, doesn't change afterwards and should be immutable

Vulnerability details

Impact

Proof of Concept

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/TokemakBribe.sol#L28 https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/BribeVault.sol#L23-L25

Tools

Remix

Recommended Mitigation Steps

9. Checking non-zero value can avoid an external call to save gas

Vulnerability details

Impact

Checking if _amount > 0 before can save gas by avoiding the external call in such situations.

Proof of Concept

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/RewardDistributor.sol#L179-L181 https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/BribeVault.sol#L283 https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/BribeVault.sol#L296

Tools

Remix

Recommended Mitigation Steps

10. Caching variables

Impact

Some of the variable can be cached to slightly reduce gas usage.

Proof of Concept

BTRFLY, TREASURY, WETH can be cached. https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/ThecosomataETH.sol#L124-L155

Tools

Remix

Recommended Mitigation Steps

Consider caching those variable for read and make sure write back to storage.

11. Use of constant keccak variables results in extra hashing (and so gas).

Impact

Increase gas costs on all onlyAdmin operations

Proof of Concept

The DEPOSITOR_ROLE variable is marked as constant: https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/BribeVault.sol#L27

bytes32 public constant DEPOSITOR_ROLE = keccak256("DEPOSITOR_ROLE");

This results in the keccak operation being performed whenever the variable is used, increasing gas costs relative to just storing the output hash. Changing to immutable will only perform hashing on contract deployment which will save gas. See: ethereum/solidity#9232 (comment)

Tools

Remix

Recommended Mitigation Steps

Change the variable to be immutable rather than constant

12. Prefix increments are cheaper than postfix increments

Impact

There is no risk of overflow caused by increamenting the iteration index in for loops. Increments perform overflow checks that are not necessary in this case.

Proof of Concept

for (uint256 i = 0; i < distributions.length; i++) {

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/BribeVault.sol#L269

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/RewardDistributor.sol#L82

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/RewardDistributor.sol#L103

Tools

Remix

Recommended Mitigation Steps

Surround the increment expressions with an unchecked { ... } block to avoid the default overflow checks

13. Shifting instead of dividing

Vulnerability details

Impact

Instead of dividing _index / 256 , it is more efficient to shift by 16 dividing (x / (2 ** 16) = x >> 16)

Proof of Concept

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/RewardDistributor.sol#L201 https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/RewardDistributor.sol#L135

uint256 claimedGroup = _index / 256;// 256 = 2 ** 16

Change to:

uint256 claimedGroup = _index > > 16;

Tools

Remix

Recommended Mitigation Steps

drahrealm commented 2 years ago

Duplicate with previous similar gas optimization tips

GalloDaSballo commented 2 years ago

The hashing finding has been disproven

Not convinced that converting a string to bytes would actually save gas (doc linked is 3 years old, pre-history in solidity times)

Other findings are valid and the report is pretty good I thnk this warden and others have used a tool as some findings are literally copy pasted

GalloDaSballo commented 2 years ago

6/10 a lot of findings, but really feels like a copy paste job