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

Open
code423n4 opened this issue Apr 27, 2022 · 0 comments
Open

Gas Optimizations #74

code423n4 opened this issue Apr 27, 2022 · 0 comments
Labels
bug Something isn't working G (Gas Optimization) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Gas Report

Table of Contents

Caching storage variables in memory to save gas

IMPACT

Anytime you are reading from storage more than once, it is cheaper in gas cost to cache the variable in memory: a SLOAD cost 100gas, while MLOAD and MSTORE cost 3 gas.

PROOF OF CONCEPT

Instances include:

FlywheelCore.sol

scope: setFlywheelRewards()

  • flywheelRewards is read twice:
FlywheelCore.sol:166
FlywheelCore.sol:168

scope: accrueStrategy()

  • flywheelBooster is read twice:
FlywheelCore.sol:220
FlywheelCore.sol:221

scope: accrueUser()

  • flywheelBooster is read twice:
FlywheelCore.sol:258
FlywheelCore.sol:259

TOOLS USED

Manual Analysis

MITIGATION

cache these storage variables in memory

Calldata instead of memory for RO function parameters

PROBLEM

If a reference type function parameter is read-only, it is cheaper in gas to use calldata instead of memory.
Calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory.

Try to use calldata as a data location because it will avoid copies and also makes sure that the data cannot be modified.

PROOF OF CONCEPT

Instances include:

FlywheelCore.sol

scope: accrueStrategy()

FlywheelCore.sol:210: RewardsState memory state

scope: accrueUser()

FlywheelCore.sol:241: RewardsState memory state

FlywheelGaugeRewards.sol

scope: _queueRewards()

FlywheelGaugeRewards.sol:180: address[] memory gauges

TOOLS USED

Manual Analysis

MITIGATION

Replace memory with calldata

Comparison Operators

IMPACT

In the EVM, there is no opcode for >= or <=.
When using greater than or equal, two operations are performed: > and =.

Using strict comparison operators hence saves gas

PROOF OF CONCEPT

Instances include:

FlywheelGaugeRewards.sol

FlywheelGaugeRewards.sol:107
FlywheelGaugeRewards.sol:139
FlywheelGaugeRewards.sol:154
FlywheelGaugeRewards.sol:163
FlywheelGaugeRewards.sol:200

ERC20Gauges.sol

ERC20Gauges.sol:259

ERC20MultiVotes.sol

ERC20MultiVotes.sol:379

TOOLS USED

Manual Analysis

MITIGATION

Replace <= with <, and >= with >. Do not forget to increment/decrement the compared variable

example:

-cycle - block.timestamp <= incrementFreezeWindow;
+cycle - block.timestamp < incrementFreezeWindow + 1;

However, if 1 is negligible compared to the value of the variable, we can omit the increment.

example:

-cycle - block.timestamp <= incrementFreezeWindow;
+cycle - block.timestamp < incrementFreezeWindow;

Custom Errors

IMPACT

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) while providing the same amount of information, as explained here

Custom errors are defined using the error statement

PROOF OF CONCEPT

Instances include:

FlywheelCore.sol

FlywheelCore.sol:147

FlywheelGaugeRewards.sol

FlywheelGaugeRewards.sol:114
FlywheelGaugeRewards.sol:153
FlywheelGaugeRewards.sol:154
FlywheelGaugeRewards.sol:195
FlywheelGaugeRewards.sol:200

ERC20Gauges.sol

ERC20Gauges.sol:345

ERC20MultiVotes.sol

ERC20MultiVotes.sol:266
ERC20MultiVotes.sol:352
ERC20MultiVotes.sol:379
ERC20MultiVotes.sol:392
ERC20MultiVotes.sol:393

TOOLS USED

Manual Analysis

MITIGATION

Replace require and revert statements with custom errors.

For instance, in FlywheelGaugeRewards.sol:

Replace

require(newRewards <= type(uint112).max);

with

if (newRewards > type(uint112).max) {
		revert IsNotSafeCast(newRewards);
}

and define the custom error in the contract

error IsNotSafeCast(uint256 newRewards);

Default value initialization

IMPACT

If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type).
Explicitly initializing it with its default value is an anti-pattern and wastes gas.

PROOF OF CONCEPT

Instances include:

xTribe.sol

xTribe.sol:95: uint256 i = 0;

FlywheelGaugeRewards.sol

FlywheelGaugeRewards.sol:189: uint256 i = 0;

ERC20Gauges.sol

ERC20Gauges.sol:134: uint256 i = 0;
ERC20Gauges.sol:184: uint256 i = 0;
ERC20Gauges.sol:307: uint256 i = 0;
ERC20Gauges.sol:384: uint256 i = 0;
ERC20Gauges.sol:564: uint256 i = 0;

ERC20MultiVotes.sol

ERC20MultiVotes.sol:79: uint256 low = 0;
ERC20MultiVotes.sol:346: uint256 i = 0;

TOOLS USED

Manual Analysis

MITIGATION

Remove explicit initialization for default values.

Prefix increments

IMPACT

Prefix increments are cheaper than postfix increments.

PROOF OF CONCEPT

Instances include:

xTRIBE.sol

xTRIBE.sol:99

FlywheelGaugeRewards.sol

FlywheelGaugeRewards.sol:189

ERC20Gauges.sol

ERC20Gauges.sol:137
ERC20Gauges.sol:187
ERC20Gauges.sol:314
ERC20Gauges.sol:391
ERC20Gauges.sol:576

ERC20MultiVotes.sol

ERC20MultiVotes.sol:346
ERC20MultiVotes.sol:392

TOOLS USED

Manual Analysis

MITIGATION

change variable++ to ++variable.

Shifting cheaper than division

IMPACT

A division by 2 can be calculated by shifting one to the right. While the DIV opcode uses 5 gas, the SHR opcode only uses 3 gas. Furthermore, Solidity's division operation also includes a division-by-0 prevention which is bypassed using shifting.

PROOF OF CONCEPT

Instances include:

ERC20MultiVotes.sol

ERC20MultiVotes:94: return (a & b) + (a ^ b) / 2;

TOOLS USED

Manual Analysis

MITIGATION

-return (a & b) + (a ^ b) / 2;
+return (a & b) + (a ^ b) >> 1;

Unnecessary computation

IMPACT

When emitting an event that includes a new and an old value, it is cheaper in gas to avoid caching the old value in memory. Instead, emit the event, then save the new value in storage.

PROOF OF CONCEPT

Instances include:

ERC20Gauges.sol

ERC20Gauges.sol:506

ERC20MultiVotes.sol

ERC20MultiVotes.sol:118

TOOLS USED

Manual Analysis

MITIGATION

Replace

uint256 oldMax = maxDelegates;
maxDelegates = newMax;
emit MaxDelegatesUpdate(oldMax, newMax);

with

emit MaxDelegatesUpdate(maxDelegates, newMax);
maxDelegates = newMax;
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Apr 27, 2022
code423n4 added a commit that referenced this issue Apr 27, 2022
@thomas-waite thomas-waite added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Jul 14, 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 confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

2 participants