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

Open
code423n4 opened this issue Jul 15, 2022 · 1 comment
Open

Gas Optimizations #174

code423n4 opened this issue Jul 15, 2022 · 1 comment
Labels
bug Something isn't working G (Gas Optimization) resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix)

Comments

@code423n4
Copy link
Contributor

Gas Report

1. State variables only set in constructor should be immutable

address public aaveAddr; // TODO immutable?

File: Swivel.sol#L33

In this instance I'm not sure why the function setSwivel() needs to exist. I believe this function can be removed and change swivel to immutable and set in the constructor.

function setSwivel(address s) external authorized(admin) returns (bool) {
    if (swivel != address(0)) { revert Exception(20, 0, 0, swivel, address(0));  }

    swivel = s;
    return true;
  }

MarketPlace.sol#L45-50

2. ++i costs less gas than i++

All instances are in for-loops

unchecked {i++;}

File: Swivel.sol#L100

File: Swivel.sol#L269

File: Swivel.sol#L418

File: Swivel.sol#L511

File: Swivel.sol#L564

3. x += y costs more gas than x = x + y for state variables

filled[hash] += a;

File: Swivel.sol#L121

File: Swivel.sol#L158

File: Swivel.sol#L193

File: Swivel.sol#L222

File: Swivel.sol#L287

File: Swivel.sol#L318

File: Swivel.sol#L348

File: Swivel.sol#L383

4. State variables set with literals in the constructor are cheaper if set when initialized.

feenominators = [200, 600, 400, 200];

File: Swivel.sol#73

5. Internal functions only called once can be inlined to save gas.

This does have a significant impact on readability in this case but still is a valid gas oppitization.

initiateVaultFillingZcTokenInitiate()

File: Swivel.sol#111

initiateZcTokenFillingVaultInitiate()

File: Swivel.sol#151

initiateZcTokenFillingZcTokenExit()

File: Swivel.sol#186

initiateVaultFillingVaultExit()

File: Swivel.sol#215

exitZcTokenFillingZcTokenInitiate()

File: Swivel.sol#280

exitVaultFillingVaultInitiate()

File: Swivel.sol#311

exitVaultFillingZcTokenExit()

File: Swivel.sol#341

exitZcTokenFillingVaultExit()

File: Swivel.sol#376

6. Using private rather than public for constants, saves gas.

These values can be read from source code if needed.

Gas saved here is on deployment and potentially on every function in the contract as one less methed id is added to the method table reducing checks.

string constant public NAME = 'Swivel Finance';

File: Swivel.sol#25

string constant public VERSION = '3.0.0';

File: Swivel.sol#26

uint256 constant public HOLD = 3 days;

File: Swivel.sol#27

uint16 constant public MIN_FEENOMINATOR = 33;

File: Swivel.sol#35

7. Functions guaranteed to revert when called by normal users can be marked payable

This saves gas by removing the msg.value > 0 check.

This appears to be pretty common at this point so I don't believe it degrades readability.

setAdmin()

File: Swivel.sol#428

scheduleWithdrawal()

File: Swivel.sol#436

blockWithdrawal()

File: Swivel.sol#447

withdraw()

File: Swivel.sol#457

scheduleFeeChange()

File: Swivel.sol#473

blockFeeChange()

File: Swivel.sol#483

setFee()

File: Swivel.sol#495

scheduleApproval()

File: Swivel.sol#522

blockApproval()

File: Swivel.sol#533

approveUnderlying()

File: Swivel.sol#544

create()

File: Creator.sol#30

setAdmin()

File: Creator.sol#47

setMarketPlace()

File: Creator.sol#54

setSwivel()

File: MarketPlace.sol#45

8. Caching struct to memory and not reading all members is more expensive then directly reading from storage location.

Vault memory vault = vaults[o];

File: VaultTracker.sol#L244

Market memory market = markets[p][u][m];

File: MarketPlace.sol#L91

File: MarketPlace.sol#L116

File: MarketPlace.sol#L132

File: MarketPlace.sol#L149

File: MarketPlace.sol#L177

File: MarketPlace.sol#L216

File: MarketPlace.sol#L228

File: MarketPlace.sol#L248

File: MarketPlace.sol#L266

In this case just market.zcToken can be cached instead of the whole struct.

File: MarketPlace.sol#L284

9. Order if statements that can cause revert to be as cheap as possible.

This means don't perform unnecessary actions before a if revert statement unless that action is need in the if statement.

In this example only from is needed for the if statement so to should be cached to memory after the if to save gas on a revert.

Vault memory from = vaults[f];
Vault memory to = vaults[t];

if (a > from.notional) { revert Exception(31, a, from.notional, address(0), address(0)); }

File: VaultTracker.sol#L155-158

10. Perform unnecessary actions before potential arithmetic overflow/underflow wastes will waste gas when encountering revert from overflow/underflow.

Since solidity 0.8.0 overflow/underflow protection has been built in. Solidity will now trigger a revert if a overflow/underflow is dectected.

to can be defined on line 179

Vault memory to = vaults[t];

File: VaultTracker.sol#L156

11. uints/ints smaller than 32 bytes can be more expensive.

The EVM has a 32 bytes word size so ints smaller than this require more operations to interct with.

uint8 public immutable protocol;

File: VaultTracker.sol#L26

uint16 constant public MIN_FEENOMINATOR = 33;

File: Swivel.sol#L35

12. When arguments are read-only on external functions, the data location should be calldata

function setFee(uint16[] memory i, uint16[] memory d) external authorized(admin) returns (bool)

File: Swivel.sol#L495

13. Not using initialized return variable wastes gas.

  function authRedeem(uint8 p, address u, uint256 m, address f, address t, uint256 a) public authorized(markets[p][u][m].zcToken) returns (uint256 underlyingAmount) {

File: MarketPlace.sol#L148

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

  1. addressing

  2. partially addressing. any cached struct that has less that 2 fields referenced was changed. there were 2 instances of that.

9: addressing

others were dupes or wontfix

@robrobbins robrobbins added the resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) label Aug 31, 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) resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix)
Projects
None yet
Development

No branches or pull requests

2 participants