Swivel v3 contest - _Adam's results

[L01] Add 2 step Changes for Critical Changes

Critical changes such as ownership updates should be a 2 step process to protect against human error. While the errors are unlikely important parts of the contract would become unusable if they occured. Consider changing the following functions to 2 step procedures. Swivel.sol#L428 MarketPlace.sol#L53 Creator.sol#L47

[L02] Unlocked Pragma

Recommend not using a floating pragma and changing to 0.8.13 to be consistent with other contracts. ZcToken.sol#L2

[N01] Open Todos

There are 17 open todos throughout swivel.sol, recommend resolving and removing before deployment.

[G01] Minimising SLOAD's

Whenever referencing a state variable more than once in a function without modifying it, you can save ~97 gas per use by caching the value. (normally 100 gas each use vs 103 gas to SLOAD/MSTORE for the first use and then only 3 gas for further uses)

VaultTracker.sol#L165-L186 - maturityRate is referenced up to 4 times.

[G02] For Loop Optimisations

In for loops pre increments can be used to save a small amount of gas per iteration. I ran a test in remix using a for loop and found the deployment savings of 497 gas and ~5 gas per iteration.

contract Test {
	function loopTest() external {
		for (uint256 i; i < 1; i++) {
		(Deployment cost: 118,408, Cost on function call: 24,532)
		vs
		for (uint256 i; i < 1; ++i) {
		(Deployment cost: 117,911, Cost on function call: 24,527)
		}
	}
}

For loops that can use pre increments: Swivel.sol#L100 Swivel.sol#L269 Swivel.sol#L418 Swivel.sol#L511 Swivel.sol#L564

[G03] State Variables that can be Immutable

State variables that are initialised in the constructor and then never updated anywhere can be changed to immutable. Based on the following test in remix switching to immutable variables can save 26,376 in deployment costs and 2,456 whenever referencing the variable.

contract Test {
	address public aaveAddr; 
	(Deployment Cost: 167,940, Cost on function call: 26,861)
	vs
	address public immutable aaveAddr;
	(Deployment Cost: 141,564, Cost on function call: 24,405)

	constructor(address _aaveAddr) {
		aaveAddr = _aaveAddr;
	} 

	function test() external {
		address testAddress = aaveAddr; 
	}
}

Variables that can be updated: Swivel.sol#L33

[G04] Deleting Mappings is Cheaper than setting to Default Value

Based on this test in remix you can save ~511 gas in deployment costs and ~6 gas on each function call by using delete instead of setting a mapping to the default value.

contract Test {
	mapping (address => uint256) public withdrawals;
	function test(address a) external {
		withdrawals[a] = 0;
		(Deployment cost: 180,368, Execution cost: 27,820)
		vs
		delete withdrawals[a];
		(Deployment cost: 179,857, Execution cost: 27,814)
	}
}

Swivel.sol#L448 Swivel.sol#L464 Swivel.sol#L534 Swivel.sol#L560

[G05] x = x + y is Cheaper than x += y

Based on test in remix you can save ~1,007 gas on deployment and ~15 gas on execution cost if you use x = x + y over x += y. (Is only true for storage variables)

contract Test {
	uint256 x = 1;
	function test() external {
		x += 3; 
		(Deployment Cost: 153,124, Execution Cost: 30,369)
		vs
		x = x + 1;
		(Deployment Cost: 152,117, Execution Cost: 30,354)
	}

}

Instances where x = x + y/x = x - y can be implemented: Swivel.sol#L121 Swivel.sol#L158 Swivel.sol#L193 Swivel.sol#L222 Swivel.sol#L287 Swivel.sol#L318 Swivel.sol#L348 Swivel.sol#L383 ZcToken.sol#L115 ZcToken.sol#L134