code-423n4 / 2022-07-swivel-findings

0 stars 1 forks source link

Gas Optimizations #174

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

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

robrobbins commented 2 years ago
  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