Gas Optimizations

[code423n4]

Gas Report

Table of Contents

• Address mappings can be combined in a single mapping
• Bytes constant are cheaper than string constants
• Caching storage variables in local variables to save gas
• Caching mapping accesses in local variables to save gas
• Calldata instead of memory for RO function parameters
• Clones for cheap contract deployment
• Constructor parameters should be avoided when possible
• Constants can be private
• Event fields are redundant
• Functions with access control cheaper if payable
• Prefix increments
• Storage cheaper than memory
• unchecked arithmetic
• unnecessary computation
• upgrade Solidity compiler version
• use Assembly for simple setters
• writing zero wastes gas

Address mappings can be combined in a single mapping

IMPACT

Combining mappings of address into a single mapping of address to a struct can save a Gssset (20000 gas) operation per mapping combined. This also makes it cheaper for functions reading and writing several of these mappings by saving a Gkeccak256 operation- 30 gas.

PROOF OF CONCEPT

1 instance:

Swivel.sol

mapping (address => uint256) public withdrawals\ mapping (address => uint256) public approvals

TOOLS USED

Manual Analysis

MITIGATION

Combine the address mappings aforementionned in a single address => struct mapping, for instance

- mapping (address => uint256) public withdrawals;
  /// @dev maps a token address to a point in time, a hold, after which an approval can be made
- mapping (address => uint256) public approvals;
+ struct Hold {
+    uint256 withdrawals;
+    uint256 approvals;
+ }
+ mapping (address => Hold) public holds;

Bytes constant are cheaper than string constants

IMPACT

If the string can fit into 32 bytes, then bytes32 is cheaper than string. string is a dynamically sized-type, which has current limitations in Solidity compared to a statically sized variable. This means extra gas spent upon deployment and every time the constant is read.

PROOF OF CONCEPT

Instances:

Swivel.sol

string constant public NAME = 'Swivel Finance';\ string constant public VERSION = '3.0.0';

TOOLS USED

Manual Analysis

MITIGATION

- string constant public NAME = 'Swivel Finance';
- string constant public VERSION = '3.0.0';
+ bytes32 constant public NAME = 'Swivel Finance';
+ bytes32 constant public VERSION = '3.0.0';

Caching storage variables in local variables to save gas

IMPACT

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

In particular, in for loops, when using the length of a storage array as the condition being checked after each loop, caching the array length can yield significant gas savings if the array length is high

PROOF OF CONCEPT

2 instances:

Swivel.sol

scope: setFee()

• feeChange is read twice. Unless feeChange == 0, but given that it is an admin function, it is expected that when the admin invokes this function, all the required conditions are met, hence the state variable will be read twice from storage.

if (feeChange == 0)\ if (block.timestamp < feeChange)

MarketPlace.sol

scope: createMarket()

• swivel is read twice. Unless swivel == address(0), but given that it is an admin function, it is expected that when the admin invokes this function, all the required conditions are met, hence the state variable will be read twice from storage.

if (swivel == address(0))\ (address zct, address tracker) = ICreator(creator).create(p, underAddr, m, c, swivel, n, s, IErc20(underAddr).decimals())

TOOLS USED

Manual Analysis

MITIGATION

cache these storage variables using local variables.

Caching mapping accesses in local variables to save gas

IMPACT

Anytime you are reading from a mapping value more than once, it is cheaper in gas cost to cache it, by saving one gkeccak256 operation - 30 gas.

PROOF OF CONCEPT

8 instances:

Swivel.sol

scope: initiateVaultFillingZcTokenInitiate()

• filled[hash] is read twice:

uint256 amount = a + filled[hash];\ filled[hash] += a;

scope: initiateZcTokenFillingVaultInitiate()

• filled[hash] is read twice:

uint256 amount = a + filled[hash];\ filled[hash] += a;

scope: initiateZcTokenFillingZcTokenExit()

• filled[hash] is read twice:

uint256 amount = a + filled[hash];\ filled[hash] += a;

scope: initiateVaultFillingVaultExit()

• filled[hash] is read twice:

uint256 amount = a + filled[hash];\ filled[hash] += a;

scope: exitZcTokenFillingZcTokenInitiate()

• filled[hash] is read twice:

uint256 amount = a + filled[hash];\ filled[hash] += a;

scope: exitVaultFillingVaultInitiate()

• filled[hash] is read twice:

uint256 amount = a + filled[hash];\ filled[hash] += a;

scope: exitVaultFillingZcTokenExit()

• filled[hash] is read twice:

uint256 amount = a + filled[hash];\ filled[hash] += a;

scope: exitZcTokenFillingVaultExit()

• filled[hash] is read twice:

uint256 amount = a + filled[hash];\ filled[hash] += a;

TOOLS USED

Manual Analysis

MITIGATION

cache these filled[hash] accesses using local variables. Note: this will not save gas on the first call for a specific hash - only on subsequent calls.

With that in mind, considering all these functions will only be called once for a specific hash, you can do the following change to save SLOAD operations:

-uint256 amount = a + filled[hash];

-if (amount > o.premium) { revert Exception(5, amount, o.premium, address(0), address(0)); }
+if (a > o.premium) { revert Exception(5, a, o.premium, address(0), address(0)); }

-filled[hash] += a;
+filled[hash] = a;

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,but it alleviates the compiler from the abi.decode() step that copies each index of the calldata to the memory index, each iteration costing 60 gas.

PROOF OF CONCEPT

2 instances:

Swivel.sol

function setFee(uint16[] memory i, uint16[] memory d)

TOOLS USED

Manual Analysis

MITIGATION

Replace memory with calldata

Clones for cheap contract deployment

IMPACT

There's a way to save a significant amount of gas on deployment using Clones: https://www.youtube.com/watch?v=3Mw-pMmJ7TA .

With the standard way using the new keyword, each contract created contains the entire logic. Using proxies allow only the first implementation to contain the logic, saving deployment costs on subsequent instances deployed.

PROOF OF CONCEPT

Instances:

Creator.sol

address zct = address(new ZcToken(p, u, m, c, marketPlace, n, s, d))\ address tracker = address(new VaultTracker(p, m, c, sw))

TOOLS USED

Manual Analysis

MITIGATION

Use a proxy system, see here for an example.

Constants can be private

IMPACT

Marking constants as private save gas upon deployment, as the compiler does not have to create getter functions for these variables. It is worth noting that a private variable can still be read using either the verified contract source code or the bytecode. For immutable variables written via constructor parameters, you can also look the contract deployment transaction.

PROOF OF CONCEPT

10 instances:

Swivel.sol

uint256 constant public HOLD = 3 days\ uint16 constant public MIN_FEENOMINATOR = 33

Marketplace.sol

address public immutable creator

VaultTracker.sol

address public immutable admin\ address public immutable cTokenAddr\ address public immutable swivel\ uint256 public immutable maturity\ uint8 public immutable protocol

ZcToken.sol

uint8 public immutable protocol\ address public immutable cToken

TOOLS USED

Manual Analysis

MITIGATION

Make these constants private instead of public

Constructor parameters should be avoided when possible

IMPACT

Constructor parameters are expensive. The contract deployment will be cheaper in gas if they are hard coded instead of using constructor parameters.

PROOF OF CONCEPT

4 instances:

Swivel.sol

marketPlace = m\ aaveAddr = a\ feenominators = [200, 600, 400, 200]

MarketPlace.sol

creator = c

TOOLS USED

Manual Analysis

MITIGATION

Hardcode these variables with their initial value instead of writing them during contract deployment with constructor parameters.

Event fields are redundant

PROBLEM

block.timestamp and block.number are added to event information by default, explicitly adding them is a waste of gas.

PROOF OF CONCEPT

1 instance:

MarketPlace.sol

emit Mature(p, u, m, exchangeRate, block.timestamp)

TOOLS USED

Manual Analysis

MITIGATION

Remove the uint256 matured event field, as it always corresponds to block.timestamp.

Functions with access control cheaper if payable

PROBLEM

A function with access control marked as payable will lbe cheaper for legitimate callers: the compiler removes checks for msg.value, saving approximately 20 gas per function call.

PROOF OF CONCEPT

36 instances:

Swivel.sol

function setAdmin(address a) external authorized(admin)\ function scheduleWithdrawal(address e) external authorized(admin)\ function blockWithdrawal(address e) external authorized(admin)\ function withdraw(address e) external authorized(admin)\ function scheduleFeeChange() external authorized(admin)\ function blockFeeChange() external authorized(admin)\ function setFee(uint16[] memory i, uint16[] memory d) external authorized(admin)\ function scheduleApproval(address e) external authorized(admin)\ function blockApproval(address e) external authorized(admin)\ function approveUnderlying(address[] calldata u, address[] calldata c) external authorized(admin)\ function authRedeemZcToken(uint8 p, address u, address c, address t, uint256 a) external authorized(marketPlace)

MarketPlace.sol

function setSwivel(address s) external authorized(admin)\ function setAdmin(address a) external authorized(admin)\ function createMarket(uint8 p,uint256 m,address c,string memory n,string memory s) external authorized(admin)\ function mintZcTokenAddingNotional(uint8 p, address u, uint256 m, address t, uint256 a) external authorized(swivel)\ function burnZcTokenRemovingNotional(uint8 p, address u, uint256 m, address t, uint256 a) external authorized(swivel)\ function authRedeem(uint8 p, address u, uint256 m, address f, address t, uint256 a) public authorized(markets[p][u][m].zcToken)\ function redeemZcToken(uint8 p, address u, uint256 m, address t, uint256 a) external authorized(swivel)\ function redeemVaultInterest(uint8 p, address u, uint256 m, address t) external authorized(swivel)\ function custodialInitiate(uint8 p, address u, uint256 m, address z, address n, uint256 a) external authorized(swivel)\ function custodialExit(uint8 p, address u, uint256 m, address z, address n, uint256 a) external authorized(swivel)\ function p2pZcTokenExchange(uint8 p, address u, uint256 m, address f, address t, uint256 a) external authorized(swivel)\ function p2pVaultExchange(uint8 p, address u, uint256 m, address f, address t, uint256 a) external authorized(swivel)\ function transferVaultNotionalFee(uint8 p, address u, uint256 m, address f, uint256 a) external authorized(swivel)\ function pause(uint8 p, bool b) external authorized(admin)

Creator.sol

function create ( //args ) external authorized(marketPlace)\ function setAdmin(address a) external authorized(admin)\ function setMarketPlace(address m) external authorized(admin)

VaultTracker.sol

function addNotional(address o, uint256 a) external authorized(admin)\ function removeNotional(address o, uint256 a) external authorized(admin)\ function redeemInterest(address o) external authorized(admin)\ function matureVault(uint256 c) external authorized(admin)\ function transferNotionalFrom(address f, address t, uint256 a) external authorized(admin)\ function transferNotionalFee(address f, uint256 a) external authorized(admin)

ZcToken.sol

function burn(address f, uint256 a) external onlyAdmin(address(redeemer))\ function mint(address t, uint256 a) external onlyAdmin(address(redeemer))

TOOLS USED

Manual Analysis

MITIGATION

Remove these event fields.

Immutable variables save storage

PROBLEM

If a variable is set in the constructor and never modified afterwrds, marking it as immutable can save a storage slot - 20,000 gas. This also saves 97 gas on every read access of the variable.

PROOF OF CONCEPT

1 instance:

Swivel.sol

address public aaveAddr\ aaveAddr is set in the constructor and read in two functions, but never modified.

TOOLS USED

Manual Analysis

MITIGATION

Mark aaveAddr as immutable.

Prefix increments

IMPACT

Prefix increments are cheaper than postfix increments - 6 gas. This can mean interesting savings in for loops.

PROOF OF CONCEPT

5 instances:

xTRIBE.sol

unchecked {i++;}\ unchecked {i++;}\ unchecked {i++;}\ unchecked {x++;}\ unchecked {i++;}

TOOLS USED

Manual Analysis

MITIGATION

change variable++ to ++variable.

Storage cheaper than memory

PROBLEM

Reference types cached in memory cost more gas than using storage, as new memory is allocated for these variables, copying data from storage to memory for each field of the struct or array: this means every field of the struct/array is read.

PROOF OF CONCEPT

18 instances:

MarketPlace.sol

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

VaultTracker.sol

Vault memory vlt = vaults[o]\ Vault memory vlt = vaults[o]\ Vault memory vlt = vaults[o]\ Vault memory from = vaults[f]\ Vault memory to = vaults[t]\ Vault memory oVault = vaults[f]\ Vault memory sVault = vaults[swivel]\ Vault memory vault = vaults[o]

TOOLS USED

Manual Analysis

MITIGATION

Use storage instead of memory. Cache any field read more than once onto the stack to avoid unnecessary SLOAD operations.

Unchecked arithmetic

IMPACT

The default "checked" behavior costs more gas when adding/diving/multiplying, because under-the-hood those checks are implemented as a series of opcodes that, prior to performing the actual arithmetic, check for under/overflow and revert if it is detected.

if it can statically be determined there is no possible way for your arithmetic to under/overflow (such as a condition in an if statement), surrounding the arithmetic in an unchecked block will save gas

PROOF OF CONCEPT

Instances:

VaultTracker.sol

vlt.notional -= a\ Because of the check here, it cannot underflow

from.notional -= a\ Because of the check here, it cannot underflow

ZcToken.sol

allowance[holder][msg.sender] -= previewAmount\ Because of the check here, it cannot underflow

allowance[holder][msg.sender] -= principalAmount\ Because of the check here, it cannot underflow

TOOLS USED

Manual Analysis

MITIGATION

Place the arithmetic operations in an unchecked block

Unnecessary computation

IMPACT

Unnecessary stack variables can be removed to save gas

PROOF OF CONCEPT

Instances:

ZcToken.sol

111:             uint256 allowed = allowance[holder][msg.sender];
112:             if (allowed >= previewAmount) {
113:                 revert Approvals(allowed, previewAmount);
114:             }
115:             allowance[holder][msg.sender] -= previewAmount;

132:             uint256 allowed = allowance[holder][msg.sender];
133:             if (allowed >= principalAmount) { revert Approvals(allowed, principalAmount); }
134:             allowance[holder][msg.sender] -= principalAmount

Both allowed are not necessary. They only save gas if the function reverts

TOOLS USED

Manual Analysis

MITIGATION

Remove both allowed and read the storage variables directly in the condition blocks.

Upgrade Solidity compiler version

IMPACT

• 0.8.10 removes contract existence checks if the external call has a return value - 700 gas

PROOF OF CONCEPT

Instances:

ERC20Gauges.sol

pragma solidity ^0.8.4

MITIGATION

Upgrade ZcToken.sol compiler version.

use Assembly for simple setters

IMPACT

Where it does not affect readability, using assembly allows to save gas not only on deployment, but also on function calls. This is the case for instance for simple admin setters.

PROOF OF CONCEPT

Instances:

Swivel.sol

428:   function setAdmin(address a) external authorized(admin) returns (bool) {
429:     admin = a;
430: 
431:     return true;
432:   }

MarketPlace.sol

53:   function setAdmin(address a) external authorized(admin) returns (bool) {
54:     admin = a;
55:     return true;
56:   }

Creator.sol

47:   function setAdmin(address a) external authorized(admin) returns (bool) {
48:     admin = a;
49:     return true;
50:   }

MITIGATION

-  admin = a;
+  assembly {
+    sstore(admin.slot, a)
+  }

writing zero wastes gas

IMPACT

Swivel.sol uses a timelock system for changing the fees, which consists in setting feeChange to zero every time fees are changed, then setting it to block.timestamp + HOLD upon the next scheduled fee change. As SSTORE operations are more expensive when setting a state variable from zero to a non-zero value than setting it from a non-zero value to another non-zero value, gas can be saved by not setting feeChange to zero upon a fee update.

PROOF OF CONCEPT

Instances:

Swivel.sol

• in blockFeeChange: feeChange = 0

• in setFee: feeChange = 0

MITIGATION

Change to feeChange = 1, and change the following line in setFee:

- if (feeChange == 0)
+ if (feeChange == 1)

[robrobbins]

well written ticket

but items either addressed elsewhere or wontfix