Canto contest - _Adam's results

QA01 Unlocked Pragma

Recommend using static solidity version and not floating pragma in the following contracts: Proposal-Store.sol#L3 WETH.sol#L1 GovernorBravoDelegate.sol#L2 Comptroller.sol#L2 AccountantDelegator.sol#L1 AccountantInterfaces.sol#L1 TreasuryDelegator.sol#L1 TreasuryDelegate.sol#L1 TreasuryInterfaces.sol#L1 CNote.sol#L1 NoteInterest.sol#L1

QA02 Open Todos/Old Code

Recommend removing open todos before deploying contract Comptroller.sol#L1232 Comptroller.sol#L1271

This appears to be old code/unused code, recommend removing. NoteInterest.sol#L79-L85 NoteInterest.sol#L93-L94

QA03 address(0) Checks for Immutable Addresses

Recommend adding != address(0) checks for the following 2 variables: BaseV1-periphery.sol#L76 BaseV1-periphery.sol#L78

G01 Initialising to Default Values

When initialising a variable to its default variable, it is cheaper to leave blank. I ran a test in remix that initialises a single variable and got a saving of 2,246 gas.

contract Test {
    uint256 public variable = 0;     (69,312 gas)
    vs
    uint256 public variable;         (67,066 gas)
}

BaseV1-core.sol#L46 - can change to: uint public totalSupply;

G02 Emitting Storage Variables

You can save an SLOAD (~100 gas) by emiting local variables over storage variables when they have the same value.

BaseV1-core.sol#L170 - can emit balance0 & balance1 over reserve0 & reserve0 (save 2 SLOADS) Comptroller.sol#L856 - can emit newCloseFactorMantissa instead of closeFactorMantissa Comptroller.sol#L1045 - can emit newPauseGuardian instead of pauseGuardian NoteInterest.sol#L127 - can emit newBaseRatePerYear instead of baseRatePerYear NoteInterest.sol#L144 - can emit newBaseRateMantissa instead of baseRatePerYear NoteInterest.sol#L144 - can emit newAdjusterCoefficient instead of adjusterCoefficient NoteInterest.sol#L170 - can emit newUpdateFrequency instead of updateFrequency

G03 Variables Can be Immutable/Constant

The following variables are initialised either when created or in the constructor and then never modified and can be changed to immutable/constant to save gas.

Proposal-Store.sol#L35 BaseV1-core.sol#L39-L40 WETH.sol#L6-L8 NoteInterest.sol#L22

G04 For Loop Optimisations

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
		}
	}
}

In for loops pre increments can also be used to save a small amount of gas per iteraition. 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)
		}
	}
}

Looping over memory/storage variable lengths will cost ~3 gas/~100 gas per iteration, i recommend cacheing their value and looping over that instead.

Instances of for loops that can be optimised: BaseV1-core.sol#L207 BaseV1-core.sol#L337 BaseV1-periphery.sol#L136 BaseV1-periphery.sol#L362 GovernorBravoDelegate.sol#L68 GovernorBravoDelegate.sol#L90 Comptroller.sol#L126 Comptroller.sol#L206 Comptroller.sol#L735 Comptroller.sol#L959 Comptroller.sol#L1005 Comptroller.sol#L1106 Comptroller.sol#L1347 Comptroller.sol#L1353 Comptroller.sol#L1359 Comptroller.sol#L1364 Comptroller.sol#L1413

G05 Minimising SLOAD's

You can save 1 SLOAD (~97 gas) per use by cacheing a storage variable that is used more than once.

CNote.sol#L15-L18 - can cache address(_ accountant) CNote.sol#L179-L180 - can cache address(_ accountant)

G06 No Need to check == True in Conditional Statements

You can save some gas (~1000 gas on deployment and ~10 gas on function call, based on remix test) by removing == True from the following locations.
Comptroller.sol#L149 Comptroller.sol#L1053 Comptroller.sol#L1063 Comptroller.sol#L1072 Comptroller.sol#L1081 Comptroller.sol#L1350 Comptroller.sol#L1357 Comptroller.sol#L1456

G07 Custom Errors

As your using a solidity version greater 0.8.4 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)

I recommend replacing all revert strings with custom errors.

G08 Long Revert Strings

If opting not to update revert strings to 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:

BaseV1-periphery.sol#L86 BaseV1-periphery.sol#L104-L105 BaseV1-periphery.sol#L223 BaseV1-periphery.sol#L228 BaseV1-periphery.sol#L295-L296 BaseV1-periphery.sol#L387 BaseV1-periphery.sol#L402 BaseV1-periphery.sol#L417 BaseV1-periphery.sol#L430 BaseV1-periphery.sol#L452 WETH.sol#L29 WETH.sol#L96-L97 GovernorBravoDelegate.sol#L25-L27 GovernorBravoDelegate.sol#L42-L47 GovernorBravoDelegate.sol#L78 GovernorBravoDelegate.sol#L87 GovernorBravoDelegate.sol#L115 GovernorBravoDelegate.sol#L132-L133 GovernorBravoDelegate.sol#L146 GovernorBravoDelegate.sol#L164 Comptroller.sol#L178 Comptroller.sol#L491 Comptroller.sol#L998 Comptroller.sol#L1016 Comptroller.sol#L1051-L1052 Comptroller.sol#L1061-L1062 Comptroller.sol#L1071 Comptroller.sol#L1080 Comptroller.sol#L1089 Comptroller.sol#L1095-L1096 Comptroller.sol#L1411 AccountantDelegator.sol#L43-L44 AccountantDelegator.sol#L124 TreasuryDelegator.sol#L31-L32 TreasuryDelegate.sol#L47 CNote.sol#L16 CNote.sol#L43 CNote.sol#L45 CNote.sol#L54 CNote.sol#L77 CNote.sol#L114 CNote.sol#L130 CNote.sol#L146 CNote.sol#L198 CNote.sol#L214 CNote.sol#L264 CNote.sol#L310 CNote.sol#L330 NoteInterest.sol#L141 NoteInterest.sol#L154 NoteInterest.sol#L167

G09 Uint > 0 Checks in Require Functions

If opting not to use custom errors when checking whether a uint is > 0 in a requrie functions you can save a small amount of gas by replacing with != 0. This is only true if optimiser is turned on and in a require statement. I ran a test in remix with optimisation set to 10,000 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 a uint is compared > 0: BaseV1-core.sol#L253 BaseV1-core.sol#L272 BaseV1-core.sol#L286 BaseV1-core.sol#L303 BaseV1-core.sol#L465 BaseV1-periphery.sol#L104-L105 BaseV1-periphery.sol#L456 BaseV1-periphery.sol#L463

G10 && in Require Functions

If not opting to use custom errors and If optimising for running 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)
	}
}

Instances where require statements can be split into seperate statements:

BaseV1-core.sol#L272 BaseV1-core.sol#L288 BaseV1-core.sol#L294 BaseV1-core.sol#L431 BaseV1-core.sol#L468 BaseV1-periphery.sol#L105 BaseV1-periphery.sol#L459 BaseV1-periphery.sol#L466 GovernorBravoDelegate.sol#L42-L45 GovernorBravoDelegate.sol#L115 GovernorBravoDelegate.sol#L164 Comptroller.sol#L1003 Comptroller.sol#L1411