Nibbl contest - _Adam's results

[L01] Unlocked Pragma

Recommend not using a floating pragma and changing to 0.8.10 to be consistent with other contracts. AccessControlMechanism.sol#L4

[N01] Repeated Code

The Same require check is repeated 8 times, recommend creating a modifier and using that instead. Basket.sol#L36 Basket.sol#L42 Basket.sol#L53 Basket.sol#L62 Basket.sol#L69 Basket.sol#L79 Basket.sol#L86 Basket.sol#L92

[N02] Incomplete Natspec

NibblVaultFactory.sol#L37 - missing @return NibblVaultFactory.sol#L63 - missing @return NibblVault.sol#L271 - missing @param _ totalSupply NibblVault.sol#L283 - missing @param _ totalSupply NibblVault.sol#L299 - missing @return NibblVault.sol#L334 - missing @param _ totalSupply NibblVault.sol#L346 - missing @param _ totalSupply NibblVault.sol#L361 - missing @return NibblVault.sol#L397 - missing @return NibblVault.sol#L463 - missing @param _ to NibblVault.sol#L473 - missing @return Basket.sol#L34 - missing @param _ to Basket.sol#L51 - missing @param _ to Basket.sol#L60 - missing @param _ to

[G01] Public Functions that can be External

The following functions are never called in their contracts and can be switched to external to save gas: NibblVaultFactory.sol#L64-L69 NibblVaultFactory.sol#L76 NibblVaultFactory.sol#L80 NibblVaultFactory.sol#L88 Twav.sol#L44

[G02] 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: NibblVault.sol#L506 NibblVault.sol#L525 NibblVault.sol#L547 Basket.sol#L43 Basket.sol#L70 Basket.sol#L93

[G03] Pre Increments in Loops

In for loops pre increments can also 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: NibblVault.sol#L506 NibblVault.sol#L525 NibblVault.sol#L547 Basket.sol#L43 Basket.sol#L70 Basket.sol#L93

[G04] Custom Errors

As your using a solidity version > 0.8.4 for most of the contracts you can replace revert strings with custom errors. This will save in deployment costs and runtime costs. I ran a test in remix comparing a revert string vs custom errors and found that 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)

There are 41 revert strings throughout your contracts that can be replaced with custom errors.

[G05] Long Revert Strings

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: NibblVaultFactory.sol#L48-L49 NibblVaultFactory.sol#L107 NibblVaultFactory.sol#L131 NibblVaultFactory.sol#L141 NibblVaultFactory.sol#L149 NibblVaultFactory.sol#L166

[G06] && in Require Statements

If optimising for runtime costs over deployment costs you can seperate && in require functions into 2 parts. I ran a basic test in remix and it cost an extra 234 gas to deploy but will save ~9 gas everytime the require function is called.

contract Test {
	uint256 a = 0;
	uint256 b = 1;

	function test() external {
		require(a == 0 && b > a) 
		(Deployment cost: 123,291, Cost on function call: 29,371)
		vs
		require(a == 0);
		require(b > a);
		(Deployment cost: 123,525, Cost on function call: 29,362)
	}
}

Require statements that can be split up: NibblVaultFactory.sol#L107 NibblVaultFactory.sol#L131 NibblVaultFactory.sol#L149 NibblVaultFactory.sol#L166

[G07] Packing Variables

Moving the uint32 variable primaryReserveRatio down to line 52 will it allow it to be stored with secondaryReserveRatio and use 1 less storage slot (save 20,000 gas). NibblVault.sol#L28

[G08] Calldata Over Memory

When using function arguments that you only need to reference and not make any modifications to it is cheaper to use calldata than memory. Instances where calldata can be used instead of memory: NibblVaultFactory.sol#L41-L42 NibblVaultFactory.sol#L80 NibblVaultFactory.sol#L88 NibblVault.sol#L174-L175 NibblVault.sol#L504 NibblVault.sol#L523 NibblVault.sol#L545 Basket.sol#L41 Basket.sol#L68 Basket.sol#L91 EIP712Base.sol#L15

[G09] Minimize SLOAD's

Whenever referencing a storage variable more than once in a function without modifying it, it is cheaper to cache locally and use that instead. (normally 100 gas each use vs 103 gas to SLOAD/MSTORE for the first use and then only 3 gas for any others)

NibblVaultFactory.sol#L107 can cache basketUpdateTime (save ~97 gas) NibblVaultFactory.sol#L131 can cache feeToUpdateTime (save ~97 gas) NibblVaultFactory.sol#L149 can cache feeAdminUpdateTime (save ~97 gas) NibblVaultFactory.sol#L166 can cache vaultUpdateTime (save ~97 gas) For the previous 4 make sure to only use cached variables when referencing not when deleting. NibblVault.sol#L139 can cache buyoutEndTime (save ~97 gas) Twav.sol#L27-L29 can cache twavObservationsIndex (save ~194 gas) On line 29 ensure the cached value is only used on the right side of the assignment.

[G10] Using smaller than 32 byte uints (uint256) can cost more

As explained in Solidity Docs using elements smaller than 32 bytes can cost more gas. Recommend replacing the follpwing with uint256: Twav.sol#L22 Twav.sol#L37