Putty contest - _Adam's results

[N01] Recommend constant variables over magic numbers

Constant variables instead of magic numbers can help keep the code easier to read and maintain. PuttyV2.sol#L287 - recommend adding constant MAX_DURATION PuttyV2.sol#L499 - recommend adding constant FEE_DENOMINATOR

[G01] Function argument require statements should come first

Require checks that involve function arguments/constants should come before any that involve state variables. That way if they fail you are saving having to load any state variables (100 gas for an SLOAD)

PuttyV2.sol#L281 - Recommend moving this require statement down to after the require statement on line 298 PuttyV2.sol#L401- Recommend moving this require statement down to after the require statement on line 406

[G02] Public Function that can be External

The following functions are never called in their contracts and can be switched to external to save gas:

PuttyV2.sol#L389 PuttyV2.sol#L466 PuttyV2.sol#L546-L550 PuttyV2.sol#L573-L577 PuttyV2.sol#L753

[G03] Minimize Loading Storage Variables

Whenever referencing a storage variable more than once in a function without modifying 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)

PuttyV2.sol#L498-L499 - can cache fee to save ~97 gas

[G04] Unchecked Increments in Loops

When incrementing i in for loops there is no chance of overflow so unchecked can be used to save gas. I ran a simple test in remix and found deployment savings of 31,653 gas and on each function call saved ~141 gas per iteration.

contract Test {
	function loopTest() external {
		for (uint256 i; i < 1; ++i) {
		Deployment Cost: 125,637, Cost on function call: 24,601
		vs
		for (uint256 i; i < 1; ) {
		// for loop body
		unchecked { ++i }
		Deployment Cost: 93,984, Cost on function call: 24,460
		}
	}
}

For loops that can use unchecked increments: PuttyV2.sol#L556 PuttyV2.sol#L594 PuttyV2.sol#L611 PuttyV2.sol#L627 PuttyV2.sol#L637 PuttyV2.sol#L647 PuttyV2.sol#L658 PuttyV2.sol#L670 PuttyV2.sol#L728 PuttyV2.sol#L742

[G05] Pre Increments in Loops

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: PuttyV2.sol#L556 PuttyV2.sol#L594 PuttyV2.sol#L611 PuttyV2.sol#L627 PuttyV2.sol#L637 PuttyV2.sol#L647 PuttyV2.sol#L658 PuttyV2.sol#L670 PuttyV2.sol#L728 PuttyV2.sol#L742

[G06] Custom Errors

As your using a solidity version > 0.8.4 you can replace revert strings with custom errors. This will save in deployment costs and runtime costs. Based on a test in remix, replacing a single revert string with a custom error saved 12,404 gas in deployment cost and 86 gas on each function call.

contract Test {
	uint256 a;
	function check() external {
		require(a != 0, "check failed");
	}
}   (Deployment cost: 114,703, Cost on Function call: 23,392)
vs 
contract Test {
	uint256 a;
	error checkFailed();
	function check() external {
		if (a != 0) revert checkFailed();
	}
}   (Deployment cost: 102,299, Cost on Function call: 23,306)

Instances where custom errors can be implemented: PuttyV2.sol#L214 PuttyV2.sol#L241 PuttyV2.sol#L278-L293 PuttyV2.sol#L297-L298 PuttyV2.sol#L329 PuttyV2.sol#L353 PuttyV2.sol#L395-L401 PuttyV2.sol#L405-L406 PuttyV2.sol#L429 PuttyV2.sol#L470 PuttyV2.sol#L475 PuttyV2.sol#L481 PuttyV2.sol#L527 PuttyV2.sol#L551-L552 PuttyV2.sol#L598-L599 PuttyV2.sol#L765 PuttyV2Nft.sol#L12-L13 PuttyV2Nft.sol#L26-L31 PuttyV2Nft.sol#L41

[G07] Uint > 0 Checks in Require Statements

When checking whether a uint is > 0 in a require statement (with optimiser on) you can save a small amount of gas by replacing with != 0. I ran a test in remix and found the savings for a single occurance is 632 in deployment cost and 6 gas on each function call.

contract Test {
	uint256 a;
	function check() external {
		require(a > 0); 
		(Deployment cost: 79,763, Cost on function call: 23,305)	
		vs
		require(a != 0); 
		(Deployment cost: 79,331, Cost on function call: 23,299)	
	}
}

Instances where uint != 0 can be used: PuttyV2.sol#L293 PuttyV2.sol#L598-L599

[G08] Store Multiple Related mappings into a Struct

When using multiple related mappings they can be combined into a mapping => struct. This will allow the use of less storage slots and save in deployment costs (saving ~62,051 gas based on the following remix test). This will also result in cheaper reads and writes going forward.

contract Test {
	mapping(uint256 => uint256) public positionExpirations;
	mapping(uint256 => bool) public exercisedPositions;
	mapping(uint256 => uint256[]) public positionFloorAssetTokenIds;
	(Deployment Cost: 219,235)
	vs
	struct positionInfo {
		uint256 expirations;
		bool exercised;
		uint256[] floorAssetTokenIds;
	}

	mapping(uint256 => positionInfo) public positions;
	(Deployment Cost: 157,184)
}

Related Mappings that can be combined in a Struct: PuttyV2.sol#L145-L164