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 #109

Open
code423n4 opened this issue Jun 18, 2022 · 1 comment
Open

Gas Optimizations #109

code423n4 opened this issue Jun 18, 2022 · 1 comment
Labels
bug Something isn't working G (Gas Optimization) sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons valid

Comments

@code423n4
Copy link
Contributor

  1. Using bools for storage incurs overhead

// Booleans are more expensive than uint256 or any type that takes up a full
// word because each write operation emits an extra SLOAD to first read the
// slot's contents, replace the bits taken up by the boolean, and then write
// back. This is the compiler's defense against contract upgrades and
// pointer aliasing, and it cannot be disabled.
https://github.com/Badger-Finance/vested-aura/blob/a12c5c21ef08808a573a582f8fd259c33cf2e4e3/contracts/MyStrategy.sol#L23-L28

make constants internal to save gas saving 3 gas on not having to make it public

https://github.com/Badger-Finance/vested-aura/blob/a12c5c21ef08808a573a582f8fd259c33cf2e4e3/contracts/MyStrategy.sol#L38-L47

make constant imutable because its on deployment its cheaper to read and its saves 100 gas on read

https://github.com/Badger-Finance/vested-aura/blob/a12c5c21ef08808a573a582f8fd259c33cf2e4e3/contracts/MyStrategy.sol#L38-L47

make governance functions payable

Functions marked as payable are 24 gas cheaper than their counterpart (in non-payable functions, Solidity adds an extra check to ensure msg.value is zero).
When users can't mistakenly send ETH to a function (as an example, when there's an onlyOwner modifier or alike), it is safe to mark it as payable
Instances:
Line:80,87,93,99,108,380

Reduce the size of error messages (Long revert Strings)
Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met. Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.
1 byte for character
https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L186
Save gas by in for loop make i uninitiated instead of zero saving 3 gas
https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L118
Lines:300,317

make ++I instead of I++

++i costs less gas compared to i++ or i += 1
++i costs less gas compared to i++ or i += 1 for unsigned integer, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled. i++ increments i and returns the initial value of i. Which means:
uint i = 1; i++; // == 1 but i == 2
But ++i returns the actual incremented value:
uint i = 1; ++i; // == 2 and i == 2 too, so no need for a temporary variable In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2
Lines:300,317,118

use revert instead of require

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Jun 18, 2022
code423n4 added a commit that referenced this issue Jun 18, 2022
@GalloDaSballo
Copy link
Collaborator

Using bools for storage incurs overhead

True but we won't change the variables for governance values that will rarely if ever change

make constants internal to save gas saving 3 gas on not having to make it public

Disagree in lack of proof, it just saves the cost of the getters

make constant imutable because its on deployment its cheaper to read and its saves 100 gas on read

Not true, if anything it costs more as the immutable functions are inlined at deploy time

make governance functions payable

Dude pls no

Reduce the size of error messages (Long revert Strings)

Ack

## make ++I instead of I++
Saves 5 gas

@GalloDaSballo GalloDaSballo added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Jun 19, 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) sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons valid
Projects
None yet
Development

No branches or pull requests

3 participants