Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas Optimizations #53

Open
code423n4 opened this issue Feb 17, 2022 · 3 comments
Open

Gas Optimizations #53

code423n4 opened this issue Feb 17, 2022 · 3 comments
Labels
bug Something isn't working G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

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.
code-423n4/2021-10-mochi-findings#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

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Feb 17, 2022
code423n4 added a commit that referenced this issue Feb 17, 2022
@drahrealm drahrealm added the duplicate This issue or pull request already exists label Feb 19, 2022
@drahrealm
Copy link
Collaborator

Duplicate with previous similar gas optimization tips

@GalloDaSballo
Copy link
Collaborator

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
Copy link
Collaborator

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

@CloudEllie CloudEllie reopened this Mar 25, 2022
@CloudEllie CloudEllie removed the duplicate This issue or pull request already exists label Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working G (Gas Optimization)
Projects
None yet
Development

No branches or pull requests

4 participants