Fractional v2 contest - _Adam's results

[L01] Use of transfer is not Recommended

If msg.sender is a contract account the transfer can fail due to either failure to implement a payable fallback function or the function call costs exceeding the limit of 2300 gas. I recommend using call() instead of transfer(). Migration.sol#L325

[L02] 2 Step Ownership Changes

Critical changes such as ownership changes 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: FERC1155.sol#L229-L236 Vault.sol#L93-L97

[N01] Open Todos

Recommend removing open todos before deployment MerkleBase.sol#L24-L25

[G01] Custom Errors

There are a couple of files that have yet to be upgraded to custom errors. Based on the following test in remix you can save 12,404 in deployment costs per custom error 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: FERC1155.sol#L263-L268 FERC1155.sol#L275-L286 FERC1155.sol#L297 MerkleBase.sol#L62 MerkleBase.sol#L78

[G02] Shorten Revert Messages

If you opt not to use custom errors keeping revert strings <= 32 bytes in length will save gas. I ran a test in remix and found the savings for a single short revert string vs long string to be 9,377 gas in deployment cost and 18 gas on function call.

contract Test {
	uint256 a;
	function check() external {
		require(a != 0, "short error message"); 
		(Deployment cost: 114,799, Cost on function call: 23,392)	
		vs 
		require(a != 0, "A longer Error Message over 32 bytes in     
        length"); 
		(Deployment cost: 124,176, Cost on function call: 23,410)	
	}
}

I recommend shortenning the following revert strings to < 32 bytes in length: MerkleBase.sol#L62 MerkleBase.sol#L78

[G03] Loop Optimisation

When incrementing i in for loops there is no chance of overflow so unchecked can be used to save gas. Pre increments can also be used to save a small amount of gas per iteration (~5 gas). I ran a simple test in remix and found deployment savings of 31,901 gas and on each function call saved ~144 gas per iteration.

contract Test {
	function loopTest() external {
		for (uint256 i; i < 1; i++) {
		Deployment Cost: 125,885, Cost on function call: 24,604
		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/pre increments: Vault.sol#L104

[G04] State Variables that can be Immutable

Variables that are initialised in the constructor and then never modified 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 supply; 
	(Deployment Cost: 167,940, Cost on function call: 26,861)
	vs
	address public immutable supply;
	(Deployment Cost: 141,564, Cost on function call: 24,405)

	constructor(address _supply) {
		supply = _supply;
	} 

	function test() external {
		address testAddress = supply; // to test referencing gas costs
	}
}

Variables that can be changed to immutable: Buyout.sol#L29-L33 - registry, supply & transfer are all set in the constructor and never modified. Minter.sol#L14 - supply is never modified Migration.sol#L37-L39 - buyout & registry are never modified VaultFactory.sol#L15 - implementation is never modified

[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: FERC1155.sol#L62 FERC1155.sol#L86 Buyout.sol#L139 Buyout.sol#L176 Migration.sol#L123-L124 Migration.sol#L134-L135 Migration.sol#L156 Migration.sol#L160

[G06] Minimise 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)

FERC1155.sol#L246-L247 - can cache royaltyPercent[_ id])(save ~94 gas) FERC1155.sol#L303-L305 - can cache _ controller (save ~94 gas) Buyout.sol#L476-L477 - can cache supply (save ~94 gas) Buyout.sol#L482-L501 - can cache transfer (save ~679 gas) Migration.sol#L81-L95 - can cache registry (save ~94 gas)

[G07] 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)
	}
}

Instances where mappings are being set to their default value: Vault.sol#L105 Migration.sol#L161 Migration.sol#L323 Migration.sol#L157 Migration.sol#L310

[G08] Functions that can be Payable

Functions that will always revert when regular users call them (such as those that can only be called by owner/controller) can be marked payable to save a small amount of gas (~24 Gas when function is called based on remix test)

FERC1155.sol#L205-L207 FERC1155.sol#L198 FERC1155.sol#L217-L221 FERC1155.sol#L229-L231 Vault.sol#L73 Vault.sol#L86 Vault.sol#L93 Vault.sol#L101

[G09] Public Function that can be External

The following functions are never called in their contracts and can be switched from public to external to save gas: MerkleBase.sol#L43-L47 MerkleBase.sol#L61 MerkleBase.sol#L73-L74 Metadata.sol#L36 SelfPermit.sol#L18-L26 SelfPermit.sol#L46-L53