Canto v2 contest - TomJ's results

Table of Contents

Low Risk Issues

• Admin Address Change should be a 2-Step Process
• Require should be used instead of Assert
• Immutable addresses should 0-Check

Non-critical Issues

• Require Statements without Descriptive Revert Strings
• Unnecessary Use of Named Returns
• Should Resolve TODOs before Deployment
• Constant Naming Convention
• Define Magic Numbers to Constant
• Use Double-Quotes for Strings
• Best Practices of Source File Layout
• Event is Missing Indexed Fields
• TYPO
• Incomplete NatSpec
• Use fixed compiler versions instead of floating version

 

Low Risk Issues

[L-01] Admin Address Change should be a 2-Step Process

Issue

The function changes an admin account with single process. This can be a concern since an admin role has a high privilege in the contract and mistakenly setting a new admin to an incorrect address will end up losing that privilege.

PoC

https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Note.sol#L21-L27 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/NoteInterest.sol#L109-L114

Mitigation

This can be fixed by implementing 2-step process. We can do this by following. First make the function approve a new address as a pending admin. Next that pending admin has to claim the ownership in a separate transaction to be a new admin.

 

[L-02] Require should be used instead of Assert

Issue

Solidity documents mention that properly functioning code should never reach a failing assert statement and if this happens there is a bug in the contract which should be fixed. Reference: https://docs.soliditylang.org/en/v0.8.15/control-structures.html#panic-via-assert-and-error-via-require

PoC

./BaseV1-periphery.sol:100:        assert(msg.sender == address(wcanto)); // only accept ETH via fallback from the WETH contract
./BaseV1-periphery.sol:245:                assert(amountAOptimal <= amountADesired);
./BaseV1-periphery.sol:291:        assert(wcanto.transfer(pair, amountCANTO));
./BaseV1-periphery.sol:437:        assert(wcanto.transfer(pairFor(routes[0].from, routes[0].to, routes[0].stable), amounts[0]));

Mitigation

Replace assert by require.

 

[L-03] Immutable addresses should 0-Check

Issue

I recommend adding check of 0-address for immutable addresses. Not doing so might lead to non-functional contract when it is updated to 0-address accidentally.

PoC

4 immutable addresses of "USDC", "Comptroller", "factory" and "wcanto" of BaseV1-periphery.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-periphery.sol#L91-L97

91:    constructor(address _factory, address _wcanto, address USDC_, address Comptroller_) {
92:        factory = _factory;
93:        pairCodeHash = IBaseV1Factory(_factory).pairCodeHash();
94:        wcanto = IWCANTO(_wcanto);
95:        USDC = USDC_;
96:        Comptroller = Comptroller_;
97:    }

Mitigation

Add 0-address check for above 4 immutable addresses.

 

Non-critical Issues

[N-01] Require Statements without Descriptive Revert Strings

Issue

It is best practice to include descriptive revert strings for require statement for readability and auditing.

PoC

./Note.sol:22:        require(msg.sender == admin);
./Note.sol:23:        require(address(accountant) == address(0));
./AccountantDelegator.sol:22:        require(admin_ != address(0));
./CNote.sol:74:        require(address(_accountant) != address(0)); //check that the accountant has been set
./CNote.sol:121:        require(address(_accountant) != address(0)); //check that the accountant has been set
./TreasuryDelegator.sol:13:        require(admin_ != address(0));
./Reservoir.sol:74:    require(c >= a, errorMessage);
./Reservoir.sol:79:    require(b <= a, errorMessage);
./Reservoir.sol:90:    require(c / a == b, errorMessage);
./GovernorBravoDelegate.sol:53:        require(proposals[unigovProposal.id].id == 0);
./BaseV1-core.sol:127:        require(_unlocked == 1);
./BaseV1-core.sol:288:        require(!BaseV1Factory(factory).isPaused());
./BaseV1-core.sol:468:        require(token.code.length > 0);
./BaseV1-core.sol:471:        require(success && (data.length == 0 || abi.decode(data, (bool))));
./BaseV1-core.sol:501:        require(msg.sender == pauser);
./BaseV1-core.sol:506:        require(msg.sender == pendingPauser);
./BaseV1-core.sol:511:        require(msg.sender == pauser);
./Comp.sol:276:        require(n < 2**32, errorMessage);
./Comp.sol:281:        require(n < 2**96, errorMessage);
./Comp.sol:287:        require(c >= a, errorMessage);
./Comp.sol:292:        require(b <= a, errorMessage);
./BaseV1-periphery.sol:228:        require(amountADesired >= amountAMin);
./BaseV1-periphery.sol:229:        require(amountBDesired >= amountBMin);
./BaseV1-periphery.sol:309:        require(IBaseV1Pair(pair).transferFrom(msg.sender, pair, liquidity)); // send liquidity to pair
./BaseV1-periphery.sol:474:        require(token.code.length > 0);
./BaseV1-periphery.sol:477:        require(success && (data.length == 0 || abi.decode(data, (bool))));

Mitigation

Add descriptive revert strings to easier understand what the code is trying to do.

 

[N-02] Unnecessary Use of Named Returns

Issue

Several function adds return statement even thought named returns variable are used. Remove unnecessary named returns variable to improve code readability. Also keeping the use of named returns or return statement consistent through out the whole project if possible is recommended.

PoC

1. BaseV1-core.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-core.sol#L141-L143 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-core.sol#L206-L214

2. BaseV1-periphery.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-periphery.sol#L135-L147 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-periphery.sol#L489-L535

3. GovernorAlpha.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/GovernorAlpha.sol#L218-L221

4. GovernorBravoDelegate.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/GovernorBravoDelegate.sol#L102-L105

Mitigation

Remove unused named returns variable and keep the use of named returns or return statement consistent through out the whole project if possible.

 

[N-03] Should Resolve TODOs before Deployment

Issue

Questions/Issues in the code should be resolved before the deployment.

PoC

./Reservoir.sol:49:    uint reservoirBalance_ = token_.balanceOf(address(this)); // TODO: Verify this is a static call

Mitigation

Resolve the question/issue and remove TODO comment from code.

 

[N-04] Constant Naming Convention

Issue

Constants should be named with all capital letters with underscores separating words. Reference: https://docs.soliditylang.org/en/v0.8.15/style-guide.html?highlight=naming#constants

PoC

./GovernorAlpha.sol:6:    string public constant name = "Compound Governor Alpha";
./NoteInterest.sol:20:    uint public constant BlocksPerYear = 5256000;
./GovernorBravoDelegate.sol:9:    string public constant name = "Compound Governor Bravo";
./GovernorBravoDelegate.sol:12:    uint public constant proposalMaxOperations = 10; // 10 actions
./BaseV1-core.sol:43:    uint8 public constant decimals = 18;
./BaseV1-core.sol:72:    uint constant periodSize = 0;
./Comp.sol:6:    string public constant name = "Compound";
./Comp.sol:9:    string public constant symbol = "COMP";
./Comp.sol:12:    uint8 public constant decimals = 18;
./Comp.sol:15:    uint public constant totalSupply = 10000000e18; // 10 million Comp
./TreasuryDelegate.sol:12:    bytes32 constant cantoDenom = keccak256(bytes("CANTO"));
./TreasuryDelegate.sol:13:    bytes32 constant noteDenom = keccak256(bytes("NOTE")); //cache hashed values to reduce unnecessary gas costs

Mitigation

Name the constants with all capital letters with underscores separating words. For examples: NAME, BLOCKS_PER_YEAR, DECIMALS

 

[N-05] Define Magic Numbers to Constant

Issue

It is best practice to define magic numbers to constant rather than just using as a magic number. This improves code readability and maintainability.

PoC

./BaseV1-core.sol:332:        return x0*(y*y/1e18*y/1e18)/1e18+(x0*x0/1e18*x0/1e18)*y/1e18;

https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-core.sol#L332

./BaseV1-core.sol:336:        return 3*x0*(y*y/1e18)/1e18+(x0*x0/1e18*x0/1e18);

https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-core.sol#L336

./BaseV1-periphery.sol:534:        return price * 1e18;

https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-periphery.sol#L534

Mitigation

Define magic numbers to constant.

 

[N-06] Use Double-Quotes for Strings

Issue

Strings should be quoted with double-quotes instead of single-quotes. Solidity documents reference: https://docs.soliditylang.org/en/v0.8.15/style-guide.html?highlight=strings#other-recommendations

PoC

./BaseV1-core.sol:256:        require(liquidity > 0, 'ILM'); // BaseV1: INSUFFICIENT_LIQUIDITY_MINTED
./BaseV1-core.sol:275:        require(amount0 > 0 && amount1 > 0, 'ILB'); // BaseV1: INSUFFICIENT_LIQUIDITY_BURNED
./BaseV1-core.sol:289:        require(amount0Out > 0 || amount1Out > 0, 'IOA'); // BaseV1: INSUFFICIENT_OUTPUT_AMOUNT
./BaseV1-core.sol:291:        require(amount0Out < _reserve0 && amount1Out < _reserve1, 'IL'); // BaseV1: INSUFFICIENT_LIQUIDITY
./BaseV1-core.sol:297:        require(to != _token0 && to != _token1, 'IT'); // BaseV1: INVALID_TO
./BaseV1-core.sol:306:        require(amount0In > 0 || amount1In > 0, 'IIA'); // BaseV1: INSUFFICIENT_INPUT_AMOUNT
./BaseV1-core.sol:312:        require(_k(_balance0, _balance1) >= _k(_reserve0, _reserve1), 'K'); // BaseV1: K
./BaseV1-core.sol:416:        require(deadline >= block.timestamp, 'BaseV1: EXPIRED');
./BaseV1-core.sol:434:        require(recoveredAddress != address(0) && recoveredAddress == owner, 'BaseV1: INVALID_SIGNATURE');
./BaseV1-core.sol:524:        require(tokenA != tokenB, 'IA'); // BaseV1: IDENTICAL_ADDRESSES
./BaseV1-core.sol:526:        require(token0 != address(0), 'ZA'); // BaseV1: ZERO_ADDRESS
./BaseV1-core.sol:527:        require(getPair[token0][token1][stable] == address(0), 'PE'); // BaseV1: PAIR_EXISTS - single check is sufficient

Mitigation

Change above single-quotes to double-quotes.

 

[N-07] Best Practices of Source File Layout

Issue

I recommend following best practices of solidity source file layout for readability. Reference: https://docs.soliditylang.org/en/v0.8.15/style-guide.html#order-of-layout

This best practices is to layout a contract elements in following order: Pragma statements, Import statements, Interfaces, Libraries, Contracts

Inside each contract, library or interface, use the following order: Type declarations, State variables, Events, Modifiers, Functions

PoC

1. CNote.sol

• Modifier placed at the end of contract

2. CToken.sol

• Modifier placed at the end of contract

3. GovernorAlpha.sol

• Interfaces placed after Contracts

4. GovernorBravoInterfaces.sol

• Interfaces placed after Contracts

5. Reservoir.sol

• Import statement placed very end of the layout

Mitigation

I recommend to follow best practice source file layout

 

[N-08] Event is Missing Indexed Fields

Issue

Each event should have 3 indexed fields if there are 3 or more fields.

PoC

./AccountantInterfaces.sol:24:    event AcctInit(address lendingMarketAddress);
./AccountantInterfaces.sol:25:	event AcctSupplied(uint amount, uint err);
./AccountantInterfaces.sol:26:    event AcctRedeemed(uint amount);
./AccountantInterfaces.sol:36:    event NewImplementation(address oldImplementation, address newImplementation);
./GovernorAlpha.sol:116:    event ProposalCreated(uint id, address proposer, address[] targets, uint[] values, string[] signatures, bytes[] calldatas, uint startBlock, uint endBlock, string description);
./GovernorAlpha.sol:119:    event VoteCast(address voter, uint proposalId, bool support, uint votes);
./GovernorAlpha.sol:122:    event ProposalCanceled(uint id);
./GovernorAlpha.sol:125:    event ProposalQueued(uint id, uint eta);
./GovernorAlpha.sol:128:    event ProposalExecuted(uint id);
./NoteInterest.sol:64:    event NewBaseRate(uint oldBaseRateMantissa, uint newBaseRateMantissa);
./NoteInterest.sol:67:    event NewAdjusterCoefficient(uint oldAdjusterCoefficient, uint newAdjusterCoefficient);
./NoteInterest.sol:70:    event NewUpdateFrequency(uint oldUpdateFrequency, uint newUpdateFrequency);
./NoteInterest.sol:73:    event NewInterestParams(uint baserateperblock);
./NoteInterest.sol:76:    event NewPriceOracle(address oldOracle, address newOracle);
./NoteInterest.sol:79:    event NewAdmin(address oldAdmin, address newAdmin);
./CNote.sol:10:    event AccountantSet(address accountant, address accountantPrior);
./GovernorBravoInterfaces.sol:9:    event ProposalCreated(uint id, address proposer, address[] targets, uint[] values, string[] signatures, bytes[] calldatas, uint startBlock, uint endBlock, string description);
./GovernorBravoInterfaces.sol:12:    event ProposalCanceled(uint id);
./GovernorBravoInterfaces.sol:15:    event ProposalQueued(uint id, uint eta);
./GovernorBravoInterfaces.sol:18:    event ProposalExecuted(uint id);
./GovernorBravoInterfaces.sol:21:    event NewImplementation(address oldImplementation, address newImplementation);
./GovernorBravoInterfaces.sol:24:    event ProposalThresholdSet(uint oldProposalThreshold, uint newProposalThreshold);
./GovernorBravoInterfaces.sol:27:    event NewPendingAdmin(address oldPendingAdmin, address newPendingAdmin);
./GovernorBravoInterfaces.sol:30:    event NewAdmin(address oldAdmin, address newAdmin);
./BaseV1-core.sol:90:    event Mint(address indexed sender, uint amount0, uint amount1);
./BaseV1-core.sol:91:    event Burn(address indexed sender, uint amount0, uint amount1, address indexed to);
./BaseV1-core.sol:92-99:    event Swap(address indexed sender, uint amount0In, uint amount1In, uint amount0Out, uint amount1Out, address indexed to);
./BaseV1-core.sol:100:    event Sync(uint reserve0, uint reserve1);
./BaseV1-core.sol:101:    event Claim(address indexed sender, address indexed recipient, uint amount0, uint amount1);
./BaseV1-core.sol:103:    event Transfer(address indexed from, address indexed to, uint amount);
./BaseV1-core.sol:104:    event Approval(address indexed owner, address indexed spender, uint amount);
./BaseV1-core.sol:489:    event PairCreated(address indexed token0, address indexed token1, bool stable, address pair, uint);
./Comp.sol:51:    event DelegateVotesChanged(address indexed delegate, uint previousBalance, uint newBalance);
./Comp.sol:54:    event Transfer(address indexed from, address indexed to, uint256 amount);
./Comp.sol:57:    event Approval(address indexed owner, address indexed spender, uint256 amount);
./TreasuryInterfaces.sol:22:    event NewImplementation(address oldImplementation, address newImplementation);
./TreasuryInterfaces.sol:23:    event Received(address sender, uint amount); 

Mitigation

Use all 3 index fields when possible.

 

[N-09] TYPO

Issue

Some typo was found in the following.

PoC

1. BaseV1-core.sol

Change "obervations" to "observations" https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-core.sol#L64

64:     // Structure to capture time period obervations every 30 minutes, used for local oracles

2. BaseV1-periphery.sol

Change "doesn"t" to "doesn't" https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-periphery.sol#L173 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-periphery.sol#L204 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-periphery.sol#L230

173:        // create the pair if it doesn"t exist yet
204:        // create the pair if it doesn"t exist yet
230:        // create the pair if it doesn"t exist yet

3. CNote.sol

Change "Accounant" to "Accountant" https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/CNote.sol#L24

24:    * @dev return the current address of the Accounant

Change "efore" to "before" https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/CNote.sol#L41

41:     * @dev This function does not accrue efore calculating the exchange rate

4. NoteInterest.sol

Change "irrelevent" to "irrelevant" https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/NoteInterest.sol#L124

124:     * @notice The following parameters are irrelevent for calculating Note's interest rate. They are passed in to align with the standard function definition `getSupplyRate` in InterestRateModel

Mitigation

Change to correct spelling as mentioned in above PoC

 

[N-10] Incomplete NatSpec

Issue

It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces. However there were places where this NatSpec was incomplete.

PoC

1. AccountantDelegate.sol no @param for treasury_ https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Accountant/AccountantDelegate.sol#L11-L16

2. AccountantDelegator.sol no @return
   https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Accountant/AccountantDelegator.sol#L49-L52 no @return https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Accountant/AccountantDelegator.sol#L58-L61 no @return https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Accountant/AccountantDelegator.sol#L74-L79

3. GovernorBravoDelegator.sol no @param for newPendingAdmin https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/GovernorBravoDelegator.sol#L52-L54

Mitigation

Add missing NatSpec mentioned in above PoC.

 

[N-11] Use fixed compiler versions instead of floating version

Issue

It is best practice to lock your pragma instead of using floating pragma. The use of floating pragma has a risk of accidentally get deployed using latest complier which may have higher risk of undiscovered bugs. Reference: https://consensys.github.io/smart-contract-best-practices/development-recommendations/solidity-specific/locking-pragmas/

PoC

./Note.sol:1:pragma solidity ^0.8.10;
./AccountantDelegator.sol:1:pragma solidity ^0.8.10;
./AccountantInterfaces.sol:1:pragma solidity ^0.8.10;
./CToken.sol:2:pragma solidity ^0.8.10;
./GovernorAlpha.sol:2:pragma solidity ^0.8.10;
./NoteInterest.sol:1:pragma solidity ^0.8.10;
./CNote.sol:1:pragma solidity ^0.8.10;
./TreasuryDelegator.sol:1:pragma solidity ^0.8.10;
./Reservoir.sol:2:pragma solidity ^0.8.10;
./GovernorBravoInterfaces.sol:2:pragma solidity ^0.8.10;
./GovernorBravoDelegate.sol:2:pragma solidity ^0.8.10;
./Comp.sol:2:pragma solidity ^0.8.10;
./GovernorBravoDelegator.sol:2:pragma solidity ^0.8.10;
./TreasuryInterfaces.sol:1:pragma solidity ^0.8.10;
./AccountantDelegate.sol:1:pragma solidity ^0.8.10;
./TreasuryDelegate.sol:1:pragma solidity ^0.8.10;

Mitigation

I suggest to lock your pragma and aviod using floating pragma.

// bad
pragma solidity ^0.8.10;

// good
pragma solidity 0.8.10;

 

[G-01] Duplicate require() Checks Should be a Modifier or a Function

Issue

Since below require checks are used more than once, I recommend making these to a modifier or a function.

PoC

./BaseV1-periphery.sol:405:        require(amounts[amounts.length - 1] >= amountOutMin, "BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT");
./BaseV1-periphery.sol:420:        require(amounts[amounts.length - 1] >= amountOutMin, "BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT");
./BaseV1-periphery.sol:435:        require(amounts[amounts.length - 1] >= amountOutMin, "BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT");
./BaseV1-periphery.sol:448:        require(amounts[amounts.length - 1] >= amountOutMin, "BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT");

Mitigation

I recommend making duplicate require statement into modifier or a function.

 

[G-02] Use Calldata instead of Memory for Read Only Function Parameters

Issue

It is cheaper gas to use calldata than memory if the function parameter is read only. Calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory. More details on following link. link: https://docs.soliditylang.org/en/v0.8.15/types.html#data-location

PoC

./AccountantDelegator.sol:80:    function delegateTo(address callee, bytes memory data) internal returns(bytes memory) {
./AccountantDelegator.sol:96:    function delegateToImplementation(bytes memory data) public returns (bytes memory) {
./AccountantDelegator.sol:107:    function delegateToViewImplementation(bytes memory data) public view returns (bytes memory) {
./CToken.sol:28:                        string memory name_,
./CToken.sol:29:                        string memory symbol_,
./GovernorAlpha.sol:136:    function propose(address[] memory targets, uint[] memory values, string[] memory signatures, bytes[] memory calldatas, string memory description) public returns (uint) {
./GovernorAlpha.sol:188:    function _queueOrRevert(address target, uint value, string memory signature, bytes memory data, uint eta) internal {
./TreasuryDelegator.sol:77:    function delegateToImplementation(bytes memory data) public returns (bytes memory) {
./TreasuryDelegator.sol:88:    function delegateToViewImplementation(bytes memory data) public view returns (bytes memory) {
./TreasuryDelegator.sol:105:    function delegateTo(address callee, bytes memory data) internal returns(bytes memory) {
./Reservoir.sol:71:  function add(uint a, uint b, string memory errorMessage) internal pure returns (uint) {
./Reservoir.sol:78:  function sub(uint a, uint b, string memory errorMessage) internal pure returns (uint) {
./Reservoir.sol:84:  function mul(uint a, uint b, string memory errorMessage) internal pure returns (uint) {
./GovernorBravoDelegate.sol:75:    function queueOrRevertInternal(address target, uint value, string memory signature, bytes memory data, uint eta) internal {
./Comp.sol:275:    function safe32(uint n, string memory errorMessage) internal pure returns (uint32) {
./Comp.sol:280:    function safe96(uint n, string memory errorMessage) internal pure returns (uint96) {
./Comp.sol:285:    function add96(uint96 a, uint96 b, string memory errorMessage) internal pure returns (uint96) {
./Comp.sol:291:    function sub96(uint96 a, uint96 b, string memory errorMessage) internal pure returns (uint96) {
./GovernorBravoDelegator.sol:66:    function delegateTo(address callee, bytes memory data) internal {
./BaseV1-periphery.sol:150:    function getAmountsOut(uint amountIn, route[] memory routes) public view returns (uint[] memory amounts) {
./BaseV1-periphery.sol:379:    function _swap(uint[] memory amounts, route[] memory routes, address _to) internal virtual {
./BaseV1-periphery.sol:458:        uint[] memory amounts,
./BaseV1-periphery.sol:538:    function compareStrings(string memory str1, string memory str2) internal pure returns(bool) {

Mitigation

Change memory to calldata

 

[G-03] Use require instead of &&

Issue

When there are multiple conditions in require statement, break down the require statement into multiple require statements instead of using && can save gas.

PoC

./CToken.sol:34:        require(accrualBlockNumber == 0 && borrowIndex == 0, "market may only be initialized once");
./GovernorAlpha.sol:138:        require(targets.length == values.length && targets.length == signatures.length && targets.length == calldatas.length, "GovernorAlpha::propose: proposal function information arity mismatch");
./GovernorAlpha.sol:228:        require(proposalCount >= proposalId && proposalId > 0, "GovernorAlpha::state: invalid proposal id");
./GovernorBravoDelegate.sol:42-45:        require(unigovProposal.targets.length == unigovProposal.values.length && unigovProposal.targets.length == unigovProposal.signatures.length && unigovProposal.targets.length == unigovProposal.calldatas.length, "GovernorBravo::propose: proposal function information arity mismatch");
./BaseV1-core.sol:275:        require(amount0 > 0 && amount1 > 0, 'ILB'); // BaseV1: INSUFFICIENT_LIQUIDITY_BURNED
./BaseV1-core.sol:291:        require(amount0Out < _reserve0 && amount1Out < _reserve1, 'IL'); // BaseV1: INSUFFICIENT_LIQUIDITY
./BaseV1-core.sol:297:        require(to != _token0 && to != _token1, 'IT'); // BaseV1: INVALID_TO
./BaseV1-core.sol:434:        require(recoveredAddress != address(0) && recoveredAddress == owner, 'BaseV1: INVALID_SIGNATURE');
./BaseV1-core.sol:471:        require(success && (data.length == 0 || abi.decode(data, (bool))));
./BaseV1-periphery.sol:123:        require(reserveA > 0 && reserveB > 0, "BaseV1Router: INSUFFICIENT_LIQUIDITY");
./BaseV1-periphery.sol:477:        require(success && (data.length == 0 || abi.decode(data, (bool))));
./BaseV1-periphery.sol:483:        //require(success && (data.length == 0 || abi.decode(data, (bool))), "failing here");

Mitigation

Break down into several require statement. For example,

require(accrualBlockNumber == 0);
require(borrowIndex == 0, "market may only be initialized once");

 

[G-04] Minimize the Number of SLOADs by Caching State Variable

Issue

SLOADs cost 100 gas where MLOADs/MSTOREs cost only 3 gas. Whenever function reads storage value more than once, it should be cached to save gas.

PoC

1. AccountantDelegate.sol

• Cache note of function sweepInterest 2 SLOADs to 1 SLOAD, 1 MSTORE and 1 MLOAD https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Accountant/AccountantDelegate.sol#L83

• Cache cnote of function supplyMarket 3 SLOADs to 1 SLOAD, 1 MSTORE and 2 MLOADs https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Accountant/AccountantDelegate.sol#L50-L53

• Cache cnote of function redeemMarket 3 SLOADs to 1 SLOAD, 1 MSTORE and 2 MLOADs https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Accountant/AccountantDelegate.sol#L64-L68

• Cache cnote of function sweepInterest 5 SLOADs to 1 SLOAD, 1 MSTORE and 4 MLOADs https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Accountant/AccountantDelegate.sol#L80-L101

2. BaseV1-core.sol

• Cache decimals0 and decimals1 of function _getAmountOut 6 SLOADs to 2 SLOAD, 2 MSTORE and 2 MLOADs https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-core.sol#L372-L377

• Cache MINIMUM_LIQUIDITY of function mint 2 SLOADs to 1 SLOAD, 1 MSTORE and 1 MLOAD https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-core.sol#L251-L252

3. BaseV1-periphery.sol

• Cache USDC of function getUnderlyingPrice 5 SLOADs to 1 SLOAD, 1 MSTORE and 4 MLOADs https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-periphery.sol#L522-L531

Mitigation

When certain state variable is read more than once, cache it to local variable to save gas.

 

[G-05] Redundant Use of Variable

Issue

Below has redundant use of variables. Instead of defining a new variable, emit the event first and then update the variable.

PoC

1. _setPendingAdmin function of CToken.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/CToken.sol#L860-L869 Change it to

        // Emit NewPendingAdmin(oldPendingAdmin, newPendingAdmin)
        emit NewPendingAdmin(pendingAdmin, newPendingAdmin);
        // Store pendingAdmin with value newPendingAdmin
        pendingAdmin = newPendingAdmin;
        return NO_ERROR;

2. _acceptAdmin function of CToken.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/CToken.sol#L883-L896 Change it to

        emit NewAdmin(admin, pendingAdmin);
        emit NewPendingAdmin(pendingAdmin, address(0));

        // Store admin with value pendingAdmin
        admin = pendingAdmin;

        // Clear the pending value
        pendingAdmin = payable(address(0));

        return NO_ERROR;

3. _setComptroller function of CToken.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/CToken.sol#L910-L920 Change it to

        // Ensure invoke comptroller.isComptroller() returns true
        require(newComptroller.isComptroller(), "marker method returned false");

        // Emit NewComptroller(oldComptroller, newComptroller)
        emit NewComptroller(comptroller, newComptroller);

        // Set market's comptroller to newComptroller
        comptroller = newComptroller;

        return NO_ERROR;

4. _setReserveFactorFresh function of CToken.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/CToken.sol#L955-L960 Change it to

        emit NewReserveFactor(reserveFactorMantissa, newReserveFactorMantissa);
        reserveFactorMantissa = newReserveFactorMantissa;

        return NO_ERROR;

5. _setInterestRateModelFresh function of CToken.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/CToken.sol#L1111-L1122 Change it to

        // Ensure invoke newInterestRateModel.isInterestRateModel() returns true
        require(newInterestRateModel.isInterestRateModel(), "marker method returned false");

        // Emit NewMarketInterestRateModel(oldInterestRateModel, newInterestRateModel)
        emit NewMarketInterestRateModel(interestRateModel, newInterestRateModel);

        // Set the interest rate model to newInterestRateModel
        interestRateModel = newInterestRateModel;

        return NO_ERROR;

6. initialize function of NoteInterest.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/NoteInterest.sol#L103-L106 Change it to

        emit NewPriceOracle(address(oracle), oracleAddress);
        cNote = CErc20(cnoteAddr);
        oracle = PriceOracle(oracleAddress);

7. _setBaseRatePerYear function of NoteInterest.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/NoteInterest.sol#L168-L170 Change it to

        emit NewBaseRate(baseRatePerYear, newBaseRatePerYear);
        baseRatePerYear = newBaseRateMantissa;

8. _setAdjusterCoefficient function of NoteInterest.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/NoteInterest.sol#L181-L183 Change it to

        emit NewAdjusterCoefficient(adjusterCoefficient, newAdjusterCoefficient);
        adjusterCoefficient = newAdjusterCoefficient;

9. _setUpdateFrequency function of NoteInterest.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/NoteInterest.sol#L194-L196 Change it to

        emit NewUpdateFrequency(updateFrequency, newUpdateFrequency);
        updateFrequency = newUpdateFrequency;

10. setImplementation function of TreasuryDelegator.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Treasury/TreasuryDelegator.sol#L34-L37 Change it to

        emit NewImplementation(implementation, implementation_);
        implementation = implementation_;

11. _setPendingAdmin function of GovernorBravoDelegate.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/GovernorBravoDelegate.sol#L143-L149 Change it to

        // Emit NewPendingAdmin(oldPendingAdmin, newPendingAdmin)
        emit NewPendingAdmin(pendingAdmin, newPendingAdmin);

        // Store pendingAdmin with value newPendingAdmin
        pendingAdmin = newPendingAdmin;

12. _acceptAdmin function of GovernorBravoDelegate.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/GovernorBravoDelegate.sol#L160-L171 Change it to

        emit NewAdmin(admin, pendingAdmin);
        emit NewPendingAdmin(pendingAdmin, address(0));

        // Store admin with value pendingAdmin
        admin = pendingAdmin;

        // Clear the pending value
        pendingAdmin = address(0);

13. _setImplementation function of GovernorBravoDelegator.sol https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/GovernorBravoDelegator.sol#L30-L33 Change it to

        emit NewImplementation(implementation, implementation_);
        implementation = implementation_;

Mitigation

Instead of defining a new variable, emit the event first and then update the variable like shown in above PoC.

 

[G-06] Using Elements Smaller than 32 bytes (256 bits) Might Use More Gas

Issue

Since EVM operates on 32 bytes at a time, it acctually cost more gas to use elements smaller than 32 bytes. Reference: https://docs.soliditylang.org/en/v0.8.15/internals/layout_in_storage.html

PoC

./CToken.sol:30:                        uint8 decimals_,
./CToken.sol:31:                        uint8 stable_
./GovernorAlpha.sol:88:        uint96 votes;
./GovernorAlpha.sol:253:    function castVoteBySig(uint proposalId, bool support, uint8 v, bytes32 r, bytes32 s) public {
./GovernorAlpha.sol:267:        uint96 votes = comp.getPriorVotes(voter, proposal.startBlock);
./GovernorAlpha.sol:331:    function getPriorVotes(address account, uint blockNumber) external view returns (uint96);
./GovernorBravoDelegate.sol:18:    bytes32 public constant BALLOT_TYPEHASH = keccak256("Ballot(uint256 proposalId,uint8 support)");
./BaseV1-core.sol:9:    function decimals() external view returns (uint8);
./BaseV1-core.sol:43:    uint8 public constant decimals = 18;
./BaseV1-core.sol:415:    function permit(address owner, address spender, uint value, uint deadline, uint8 v, bytes32 r, bytes32 s) external {
./Comp.sol:12:    uint8 public constant decimals = 18;
./Comp.sol:18:    mapping (address => mapping (address => uint96)) internal allowances;
./Comp.sol:21:    mapping (address => uint96) internal balances;
./Comp.sol:28:        uint32 fromBlock;
./Comp.sol:29:        uint96 votes;
./Comp.sol:33:    mapping (address => mapping (uint32 => Checkpoint)) public checkpoints;
./Comp.sol:36:    mapping (address => uint32) public numCheckpoints;
./Comp.sol:64:        balances[account] = uint96(totalSupply);
./Comp.sol:87:        uint96 amount;
./Comp.sol:89:            amount = type(uint96).max;
./Comp.sol:116:        uint96 amount = safe96(rawAmount, "Comp::transfer: amount exceeds 96 bits");
./Comp.sol:130:        uint96 spenderAllowance = allowances[src][spender];
./Comp.sol:131:        uint96 amount = safe96(rawAmount, "Comp::approve: amount exceeds 96 bits");
./Comp.sol:133:        if (spender != src && spenderAllowance != type(uint96).max) {
./Comp.sol:134:            uint96 newAllowance = sub96(spenderAllowance, amount, "Comp::transferFrom: transfer amount exceeds spender allowance");
./Comp.sol:161:    function delegateBySig(address delegatee, uint nonce, uint expiry, uint8 v, bytes32 r, bytes32 s) public {
./Comp.sol:177:    function getCurrentVotes(address account) external view returns (uint96) {
./Comp.sol:178:        uint32 nCheckpoints = numCheckpoints[account];
./Comp.sol:189:    function getPriorVotes(address account, uint blockNumber) public view returns (uint96) {
./Comp.sol:192:        uint32 nCheckpoints = numCheckpoints[account];
./Comp.sol:207:        uint32 lower = 0;
./Comp.sol:208:        uint32 upper = nCheckpoints - 1;
./Comp.sol:210:            uint32 center = upper - (upper - lower) / 2; // ceil, avoiding overflow
./Comp.sol:225:        uint96 delegatorBalance = balances[delegator];
./Comp.sol:233:    function _transferTokens(address src, address dst, uint96 amount) internal {
./Comp.sol:244:    function _moveDelegates(address srcRep, address dstRep, uint96 amount) internal {
./Comp.sol:247:                uint32 srcRepNum = numCheckpoints[srcRep];
./Comp.sol:248:                uint96 srcRepOld = srcRepNum > 0 ? checkpoints[srcRep][srcRepNum - 1].votes : 0;
./Comp.sol:249:                uint96 srcRepNew = sub96(srcRepOld, amount, "Comp::_moveVotes: vote amount underflows");
./Comp.sol:254:                uint32 dstRepNum = numCheckpoints[dstRep];
./Comp.sol:255:                uint96 dstRepOld = dstRepNum > 0 ? checkpoints[dstRep][dstRepNum - 1].votes : 0;
./Comp.sol:256:                uint96 dstRepNew = add96(dstRepOld, amount, "Comp::_moveVotes: vote amount overflows");
./Comp.sol:262:    function _writeCheckpoint(address delegatee, uint32 nCheckpoints, uint96 oldVotes, uint96 newVotes) internal {
./Comp.sol:263:      uint32 blockNumber = safe32(block.number, "Comp::_writeCheckpoint: block number exceeds 32 bits");
./Comp.sol:275:    function safe32(uint n, string memory errorMessage) internal pure returns (uint32) {
./Comp.sol:277:        return uint32(n);
./Comp.sol:280:    function safe96(uint n, string memory errorMessage) internal pure returns (uint96) {
./Comp.sol:282:        return uint96(n);
./Comp.sol:285:    function add96(uint96 a, uint96 b, string memory errorMessage) internal pure returns (uint96) {
./Comp.sol:286:        uint96 c = a + b;
./Comp.sol:291:    function sub96(uint96 a, uint96 b, string memory errorMessage) internal pure returns (uint96) {
./BaseV1-periphery.sol:18:    function permit(address owner, address spender, uint value, uint deadline, uint8 v, bytes32 r, bytes32 s) external;
./BaseV1-periphery.sol:22:    function getReserves() external view returns (uint112 _reserve0, uint112 _reserve1, uint32 _blockTimestampLast);
./BaseV1-periphery.sol:34:    function decimals() external view returns (uint8);
./BaseV1-periphery.sol:112:        pair = address(uint160(uint256(keccak256(abi.encodePacked(
./BaseV1-periphery.sol:350:        bool approveMax, uint8 v, bytes32 r, bytes32 s
./BaseV1-periphery.sol:369:        bool approveMax, uint8 v, bytes32 r, bytes32 s
./BaseV1-periphery.sol:491:        uint8 stable;
./BaseV1-periphery.sol:542:    function _returnStableBooleans(uint8 stable) internal pure returns (bool, bool){

Mitigation

I suggest using uint256 instead of anything smaller.

 

[G-07] Boolean Comparisons

Issue

It is more gas expensive to compare boolean with "variable == true" or "variable == false" than directly checking the returned boolean value.

PoC

./GovernorAlpha.sol:266:        require(receipt.hasVoted == false, "GovernorAlpha::_castVote: voter already voted");

Mitigation

Simply check by returned boolean value. Change it to

require(!receipt.hasVoted, "GovernorAlpha::_castVote: voter already voted");

 

[G-08] Both named returns and return statement are used

Issue

In some function return statement are used even though named returns is set. This is redundant because return statement is not needed when using named returns and named returns is not needed when using return statement. Removing unused named returns variable in below code can save gas and improve code readability.

PoC

1, BaseV1-core.sol

Remove returns variable "dec0", "dec1", "r0", "r1", "st", "t0" and "t1"
Link: https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-core.sol#L141-L143

Remove returns variable "amountOut"
Link: https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-core.sol#L206-L214

2. BaseV1-periphery.sol

Remove returns variable "amount" and "stable"
Link: https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-periphery.sol#L135-L147

Remove returns variable "price"
Link: https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-periphery.sol#L489-L535

3. GovernorAlpha.sol

Remove returns variable "targets", "values", "signatures" and "calldatas"
Link: https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/GovernorAlpha.sol#L218-L221

4. GovernorBravoDelegate.sol

Remove returns variable "targets", "values", "signatures" and "calldatas"
Link: https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/GovernorBravoDelegate.sol#L102-L105

Mitigation

Remove unused named returns variable as mentioned in above PoC.

 

[G-09] Constant Value of a Call to keccak256() should Use Immutable

Issue

When using constant it is expected that the value should be converted into a constant value at compile time. However when using a call to keccak256(), the expression is re-calculated each time the constant is referenced. Resulting in costing about 100 gas more on each access to this "constant". link for more details: https://github.com/ethereum/solidity/issues/9232

PoC

./GovernorAlpha.sol:110:    bytes32 public constant DOMAIN_TYPEHASH = keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)");
./GovernorAlpha.sol:113:    bytes32 public constant BALLOT_TYPEHASH = keccak256("Ballot(uint256 proposalId,bool support)");
./GovernorBravoDelegate.sol:15:    bytes32 public constant DOMAIN_TYPEHASH = keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)");
./GovernorBravoDelegate.sol:18:    bytes32 public constant BALLOT_TYPEHASH = keccak256("Ballot(uint256 proposalId,uint8 support)");
./Comp.sol:39:    bytes32 public constant DOMAIN_TYPEHASH = keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)");
./Comp.sol:42:    bytes32 public constant DELEGATION_TYPEHASH = keccak256("Delegation(address delegatee,uint256 nonce,uint256 expiry)");
./TreasuryDelegate.sol:12:    bytes32 constant cantoDenom = keccak256(bytes("CANTO"));
./TreasuryDelegate.sol:13:    bytes32 constant noteDenom = keccak256(bytes("NOTE")); //cache hashed values to reduce unnecessary gas costs

Mitigation

Change the variable from constant to immutable.

 

[G-10] Unnecessary Default Value Initialization

Issue

When variable is not initialized, it will have its default values. For example, 0 for uint, false for bool and address(0) for address. Reference: https://docs.soliditylang.org/en/v0.8.15/control-structures.html#scoping-and-declarations

PoC

./CToken.sol:82:        uint startingAllowance = 0;
./GovernorAlpha.sol:181:        for (uint i = 0; i < proposal.targets.length; i++) {
./GovernorAlpha.sol:197:        for (uint i = 0; i < proposal.targets.length; i++) {
./GovernorAlpha.sol:211:        for (uint i = 0; i < proposal.targets.length; i++) {
./GovernorBravoDelegate.sol:66:        for (uint i = 0; i < newProposal.targets.length; i++) {
./GovernorBravoDelegate.sol:88:        for (uint i = 0; i < proposal.targets.length; i++) {
./BaseV1-core.sol:48:    uint public totalSupply = 0;
./BaseV1-core.sol:72:    uint constant periodSize = 0;
./BaseV1-core.sol:210:        for (uint i = 0; i < _prices.length; i++) {
./BaseV1-core.sol:226:        uint nextIndex = 0;
./BaseV1-core.sol:227:        uint index = 0;
./BaseV1-core.sol:340:        for (uint i = 0; i < 255; i++) {
./Comp.sol:207:        uint32 lower = 0;
./BaseV1-periphery.sol:154:        for (uint i = 0; i < routes.length; i++) {
./BaseV1-periphery.sol:176:        uint _totalSupply = 0;
./BaseV1-periphery.sol:380:        for (uint i = 0; i < routes.length; i++) {

Mitigation

I suggest removing default value initialization. For example,

• uint startingAllowance;
• for (uint i; i < proposal.targets.length; i++) {

 

[G-11] Store Array's Length as a Variable

Issue

By storing an array's length as a variable before the for-loop, can save 3 gas per iteration.

PoC

./GovernorAlpha.sol:181:        for (uint i = 0; i < proposal.targets.length; i++) {
./GovernorAlpha.sol:197:        for (uint i = 0; i < proposal.targets.length; i++) {
./GovernorAlpha.sol:211:        for (uint i = 0; i < proposal.targets.length; i++) {
./GovernorBravoDelegate.sol:66:        for (uint i = 0; i < newProposal.targets.length; i++) {
./GovernorBravoDelegate.sol:88:        for (uint i = 0; i < proposal.targets.length; i++) {
./BaseV1-core.sol:210:        for (uint i = 0; i < _prices.length; i++) {
./BaseV1-periphery.sol:154:        for (uint i = 0; i < routes.length; i++) {
./BaseV1-periphery.sol:380:        for (uint i = 0; i < routes.length; i++) {

Mitigation

Store array's length as a variable before looping it. For example, I suggest changing it to

    length = proposal.targets.length
    for (uint i = 0; i < length; i++) {

 

[G-12] ++i Costs Less Gas than i++

Issue

Prefix increments/decrements (++i or --i) costs cheaper gas than postfix increment/decrements (i++ or i--).

PoC

./GovernorAlpha.sol:152:        proposalCount++;
./GovernorAlpha.sol:181:        for (uint i = 0; i < proposal.targets.length; i++) {
./GovernorAlpha.sol:197:        for (uint i = 0; i < proposal.targets.length; i++) {
./GovernorAlpha.sol:211:        for (uint i = 0; i < proposal.targets.length; i++) {
./GovernorBravoDelegate.sol:66:        for (uint i = 0; i < newProposal.targets.length; i++) {
./GovernorBravoDelegate.sol:88:        for (uint i = 0; i < proposal.targets.length; i++) {
./BaseV1-core.sol:210:        for (uint i = 0; i < _prices.length; i++) {
./BaseV1-core.sol:340:        for (uint i = 0; i < 255; i++) {
./BaseV1-core.sol:430:                keccak256(abi.encode(PERMIT_TYPEHASH, owner, spender, value, nonces[owner]++, deadline))
./Comp.sol:167:        require(nonce == nonces[signatory]++, "Comp::delegateBySig: invalid nonce");
./BaseV1-periphery.sol:154:        for (uint i = 0; i < routes.length; i++) {
./BaseV1-periphery.sol:380:        for (uint i = 0; i < routes.length; i++) {

Mitigation

Change it to postfix increments/decrements. For example,

    for (uint i = 0; i < proposal.targets.length; ++i) {

 

[G-13] != 0 costs less gass than > 0

Issue

!= 0 costs less gas when optimizer is enabled and is used for unsigned integers in "require" statement.

PoC

./CToken.sol:37:        require(initialExchangeRateMantissa > 0, "initial exchange rate must be greater than zero.");
./GovernorAlpha.sol:228:        require(proposalCount >= proposalId && proposalId > 0, "GovernorAlpha::state: invalid proposal id");
./BaseV1-core.sol:256:        require(liquidity > 0, 'ILM'); // BaseV1: INSUFFICIENT_LIQUIDITY_MINTED
./BaseV1-core.sol:275:        require(amount0 > 0 && amount1 > 0, 'ILB'); // BaseV1: INSUFFICIENT_LIQUIDITY_BURNED
./BaseV1-core.sol:289:        require(amount0Out > 0 || amount1Out > 0, 'IOA'); // BaseV1: INSUFFICIENT_OUTPUT_AMOUNT
./BaseV1-core.sol:306:        require(amount0In > 0 || amount1In > 0, 'IIA'); // BaseV1: INSUFFICIENT_INPUT_AMOUNT
./BaseV1-core.sol:468:        require(token.code.length > 0);
./BaseV1-periphery.sol:122:        require(amountA > 0, "BaseV1Router: INSUFFICIENT_AMOUNT");
./BaseV1-periphery.sol:123:        require(reserveA > 0 && reserveB > 0, "BaseV1Router: INSUFFICIENT_LIQUIDITY");
./BaseV1-periphery.sol:474:        require(token.code.length > 0);
./BaseV1-periphery.sol:481:        require(token.code.length > 0, "token code length failure");

Mitigation

I suggest changing > 0 to != 0

    require(initialExchangeRateMantissa != 0, "initial exchange rate must be greater than zero.");

 

[G-14] Keep The Revert Strings of Error Messages within 32 Bytes

Issue

Since each storage slot is size of 32 bytes, error messages that is longer than this will need extra storage slot leading to more gas cost.

PoC

./AccountantDelegator.sol:42:        require(msg.sender == admin, "AccountantDelegator::_setImplementation: admin only");
./AccountantDelegator.sol:43:        require(implementation_ != address(0), "AccountantDelegator::_setImplementation: invalid implementation address");
./CToken.sol:33:        require(msg.sender == admin, "only admin may initialize the market");
./CToken.sol:34:        require(accrualBlockNumber == 0 && borrowIndex == 0, "market may only be initialized once");
./CToken.sol:37:        require(initialExchangeRateMantissa > 0, "initial exchange rate must be greater than zero.");
./CToken.sol:50:        require(err == NO_ERROR, "setting interest rate model failed");
./CToken.sol:485:        require(redeemTokensIn == 0 || redeemAmountIn == 0, "one of redeemTokensIn or redeemAmountIn must be zero");
./CToken.sol:765:        require(amountSeizeError == NO_ERROR, "LIQUIDATE_COMPTROLLER_CALCULATE_AMOUNT_SEIZE_FAILED");
./GovernorAlpha.sol:137:        require(comp.getPriorVotes(msg.sender, sub256(block.number, 1)) > proposalThreshold(), "GovernorAlpha::propose: proposer votes below proposal threshold");
./GovernorAlpha.sol:138:        require(targets.length == values.length && targets.length == signatures.length && targets.length == calldatas.length, "GovernorAlpha::propose: proposal function information arity mismatch");
./GovernorAlpha.sol:139:        require(targets.length != 0, "GovernorAlpha::propose: must provide actions");
./GovernorAlpha.sol:140:        require(targets.length <= proposalMaxOperations(), "GovernorAlpha::propose: too many actions");
./GovernorAlpha.sol:145:          require(proposersLatestProposalState != ProposalState.Active, "GovernorAlpha::propose: one live proposal per proposer, found an already active proposal");
./GovernorAlpha.sol:146:          require(proposersLatestProposalState != ProposalState.Pending, "GovernorAlpha::propose: one live proposal per proposer, found an already pending proposal");
./GovernorAlpha.sol:156:        require(newProposal.id == 0, "GovernorAlpha::propose: ProposalID collsion");
./GovernorAlpha.sol:178:        require(state(proposalId) == ProposalState.Succeeded, "GovernorAlpha::queue: proposal can only be queued if it is succeeded");
./GovernorAlpha.sol:189:        require(!timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))), "GovernorAlpha::_queueOrRevert: proposal action already queued at eta");
./GovernorAlpha.sol:194:        require(state(proposalId) == ProposalState.Queued, "GovernorAlpha::execute: proposal can only be executed if it is queued");
./GovernorAlpha.sol:205:        require(state != ProposalState.Executed, "GovernorAlpha::cancel: cannot cancel executed proposal");
./GovernorAlpha.sol:208:        require(msg.sender == guardian || comp.getPriorVotes(proposal.proposer, sub256(block.number, 1)) < proposalThreshold(), "GovernorAlpha::cancel: proposer above threshold");
./GovernorAlpha.sol:228:        require(proposalCount >= proposalId && proposalId > 0, "GovernorAlpha::state: invalid proposal id");
./GovernorAlpha.sol:258:        require(signatory != address(0), "GovernorAlpha::castVoteBySig: invalid signature");
./GovernorAlpha.sol:263:        require(state(proposalId) == ProposalState.Active, "GovernorAlpha::_castVote: voting is closed");
./GovernorAlpha.sol:266:        require(receipt.hasVoted == false, "GovernorAlpha::_castVote: voter already voted");
./GovernorAlpha.sol:283:        require(msg.sender == guardian, "GovernorAlpha::__acceptAdmin: sender must be gov guardian");
./GovernorAlpha.sol:288:        require(msg.sender == guardian, "GovernorAlpha::__abdicate: sender must be gov guardian");
./GovernorAlpha.sol:293:        require(msg.sender == guardian, "GovernorAlpha::__queueSetTimelockPendingAdmin: sender must be gov guardian");
./GovernorAlpha.sol:298:        require(msg.sender == guardian, "GovernorAlpha::__executeSetTimelockPendingAdmin: sender must be gov guardian");
./NoteInterest.sol:167:        require(msg.sender == admin, "only the admin may set the base rate");
./NoteInterest.sol:180:        require(msg.sender == admin, "only the admin may set the adjuster coefficient");
./NoteInterest.sol:193:        require(msg.sender == admin, "only the admin may set the update frequency");
./CNote.sol:17:        require(msg.sender == admin, "CNote::_setAccountantContract:Only admin may call this function");
./CNote.sol:105:            require(balanceCur == 0, "Accountant has not been correctly supplied");
./CNote.sol:148:        require(token.balanceOf(address(this)) == 0, "cNote::doTransferOut: TransferOut Failed");
./TreasuryDelegator.sol:31:        require(msg.sender == admin, "GovernorBravoDelegator::setImplementation: admin only");
./TreasuryDelegator.sol:32:        require(implementation_ != address(0), "GovernorBravoDelegator::setImplementation: invalid implementation address");
./TreasuryDelegator.sol:121:        require(msg.value == 0, "TreasuryDelegator::fallback:cannot send value to fallback");
./GovernorBravoDelegate.sol:25:        require(address(timelock) == address(0), "GovernorBravo::initialize: can only initialize once");
./GovernorBravoDelegate.sol:26:        require(msg.sender == admin, "GovernorBravo::initialize: admin only");
./GovernorBravoDelegate.sol:27:        require(timelock_ != address(0), "GovernorBravo::initialize: invalid timelock address");
./GovernorBravoDelegate.sol:42-45:        require(unigovProposal.targets.length == unigovProposal.values.length && unigovProposal.targets.length == unigovProposal.signatures.length && unigovProposal.targets.length == unigovProposal.calldatas.length, "GovernorBravo::propose: proposal function information arity mismatch");
./GovernorBravoDelegate.sol:46:        require(unigovProposal.targets.length != 0, "GovernorBravo::propose: must provide actions");
./GovernorBravoDelegate.sol:47:        require(unigovProposal.targets.length <= proposalMaxOperations, "GovernorBravo::propose: too many actions");
./GovernorBravoDelegate.sol:76:        require(!timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))), "GovernorBravo::queueOrRevertInternal: identical proposal action already queued at eta");
./GovernorBravoDelegate.sol:85:        require(state(proposalId) == ProposalState.Queued, "GovernorBravo::execute: proposal can only be executed if it is queued");
./GovernorBravoDelegate.sol:128:        require(msg.sender == admin, "GovernorBravo::_initiate: admin only");
./GovernorBravoDelegate.sol:129:        require(initialProposalId == 0, "GovernorBravo::_initiate: can only initiate once");
./GovernorBravoDelegate.sol:140:        require(msg.sender == admin, "GovernorBravo:_setPendingAdmin: admin only");
./GovernorBravoDelegate.sol:158:        require(msg.sender == pendingAdmin, "GovernorBravo:_acceptAdmin: pending admin only");
./Comp.sol:166:        require(signatory != address(0), "Comp::delegateBySig: invalid signature");
./Comp.sol:167:        require(nonce == nonces[signatory]++, "Comp::delegateBySig: invalid nonce");
./Comp.sol:168:        require(block.timestamp <= expiry, "Comp::delegateBySig: signature expired");
./Comp.sol:190:        require(blockNumber < block.number, "Comp::getPriorVotes: not yet determined");
./Comp.sol:234:        require(src != address(0), "Comp::_transferTokens: cannot transfer from the zero address");
./Comp.sol:235:        require(dst != address(0), "Comp::_transferTokens: cannot transfer to the zero address");
./GovernorBravoDelegator.sol:27:        require(msg.sender == admin, "GovernorBravoDelegator::_setImplementation: admin only");
./GovernorBravoDelegator.sol:28:        require(implementation_ != address(0), "GovernorBravoDelegator::_setImplementation: invalid implementation address");
./GovernorBravoDelegator.sol:40:        require(msg.sender == admin, "GovernorBravoDelegator::_initiateDelegated: admin only");
./GovernorBravoDelegator.sol:48:        require(msg.sender == admin, "GovernorBravoDelegator::_acceptInitialAdminDelegated: admin only");
./GovernorBravoDelegator.sol:56:        require(msg.sender == admin, "GovernorBravoDelegator::_setPendingAdminDelegated: admin only");
./AccountantDelegate.sol:87:		require(cNoteAmt >= noteDiff, "AccountantDelegate::sweepInterest:Error calculating interest to sweep");
./AccountantDelegate.sol:101:		require(cnote.balanceOf(treasury) == 0, "AccountantDelegate::sweepInterestError");
./BaseV1-periphery.sol:104:        require(tokenA != tokenB, "BaseV1Router: IDENTICAL_ADDRESSES");
./BaseV1-periphery.sol:122:        require(amountA > 0, "BaseV1Router: INSUFFICIENT_AMOUNT");
./BaseV1-periphery.sol:123:        require(reserveA > 0 && reserveB > 0, "BaseV1Router: INSUFFICIENT_LIQUIDITY");
./BaseV1-periphery.sol:241:                require(amountBOptimal >= amountBMin, "BaseV1Router: INSUFFICIENT_B_AMOUNT");
./BaseV1-periphery.sol:246:                require(amountAOptimal >= amountAMin, "BaseV1Router: INSUFFICIENT_A_AMOUNT");
./BaseV1-periphery.sol:313:        require(amountA >= amountAMin, "BaseV1Router: INSUFFICIENT_A_AMOUNT");
./BaseV1-periphery.sol:314:        require(amountB >= amountBMin, "BaseV1Router: INSUFFICIENT_B_AMOUNT");
./BaseV1-periphery.sol:405:        require(amounts[amounts.length - 1] >= amountOutMin, "BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT");
./BaseV1-periphery.sol:420:        require(amounts[amounts.length - 1] >= amountOutMin, "BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT");
./BaseV1-periphery.sol:435:        require(amounts[amounts.length - 1] >= amountOutMin, "BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT");
./BaseV1-periphery.sol:448:        require(amounts[amounts.length - 1] >= amountOutMin, "BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT");
./BaseV1-periphery.sol:470:        require(success, "TransferHelper: ETH_TRANSFER_FAILED");

Mitigation

Simply keep the error messages within 32 bytes to avoid extra storage slot cost.

 

[G-15] Use Custom Errors to Save Gas

Issue

Custom errors from Solidity 0.8.4 are cheaper than revert strings. Details are explained here: https://blog.soliditylang.org/2021/04/21/custom-errors/

PoC

./AccountantDelegator.sol:42:        require(msg.sender == admin, "AccountantDelegator::_setImplementation: admin only");
./AccountantDelegator.sol:43:        require(implementation_ != address(0), "AccountantDelegator::_setImplementation: invalid implementation address");
./AccountantDelegator.sol:123:        require(msg.value == 0,"AccountantDelegator:fallback: cannot send value to fallback");
./CToken.sol:33:        require(msg.sender == admin, "only admin may initialize the market");
./CToken.sol:34:        require(accrualBlockNumber == 0 && borrowIndex == 0, "market may only be initialized once");
./CToken.sol:37:        require(initialExchangeRateMantissa > 0, "initial exchange rate must be greater than zero.");
./CToken.sol:42:        require(err == NO_ERROR, "setting comptroller failed");
./CToken.sol:50:        require(err == NO_ERROR, "setting interest rate model failed");
./CToken.sol:349:        require(borrowRateMantissa <= borrowRateMaxMantissa, "borrow rate is absurdly high");
./CToken.sol:485:        require(redeemTokensIn == 0 || redeemAmountIn == 0, "one of redeemTokensIn or redeemAmountIn must be zero");
./CToken.sol:765:        require(amountSeizeError == NO_ERROR, "LIQUIDATE_COMPTROLLER_CALCULATE_AMOUNT_SEIZE_FAILED");
./CToken.sol:768:        require(cTokenCollateral.balanceOf(borrower) >= seizeTokens, "LIQUIDATE_SEIZE_TOO_MUCH");
./CToken.sol:774:            require(cTokenCollateral.seize(liquidator, borrower, seizeTokens) == NO_ERROR, "token seizure failed");
./CToken.sol:912:        require(newComptroller.isComptroller(), "marker method returned false");
./CToken.sol:1114:        require(newInterestRateModel.isInterestRateModel(), "marker method returned false");
./CToken.sol:1154:        require(_notEntered, "re-entered");
./GovernorAlpha.sol:137:        require(comp.getPriorVotes(msg.sender, sub256(block.number, 1)) > proposalThreshold(), "GovernorAlpha::propose: proposer votes below proposal threshold");
./GovernorAlpha.sol:138:        require(targets.length == values.length && targets.length == signatures.length && targets.length == calldatas.length, "GovernorAlpha::propose: proposal function information arity mismatch");
./GovernorAlpha.sol:139:        require(targets.length != 0, "GovernorAlpha::propose: must provide actions");
./GovernorAlpha.sol:140:        require(targets.length <= proposalMaxOperations(), "GovernorAlpha::propose: too many actions");
./GovernorAlpha.sol:145:          require(proposersLatestProposalState != ProposalState.Active, "GovernorAlpha::propose: one live proposal per proposer, found an already active proposal");
./GovernorAlpha.sol:146:          require(proposersLatestProposalState != ProposalState.Pending, "GovernorAlpha::propose: one live proposal per proposer, found an already pending proposal");
./GovernorAlpha.sol:156:        require(newProposal.id == 0, "GovernorAlpha::propose: ProposalID collsion");
./GovernorAlpha.sol:178:        require(state(proposalId) == ProposalState.Succeeded, "GovernorAlpha::queue: proposal can only be queued if it is succeeded");
./GovernorAlpha.sol:189:        require(!timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))), "GovernorAlpha::_queueOrRevert: proposal action already queued at eta");
./GovernorAlpha.sol:194:        require(state(proposalId) == ProposalState.Queued, "GovernorAlpha::execute: proposal can only be executed if it is queued");
./GovernorAlpha.sol:205:        require(state != ProposalState.Executed, "GovernorAlpha::cancel: cannot cancel executed proposal");
./GovernorAlpha.sol:208:        require(msg.sender == guardian || comp.getPriorVotes(proposal.proposer, sub256(block.number, 1)) < proposalThreshold(), "GovernorAlpha::cancel: proposer above threshold");
./GovernorAlpha.sol:228:        require(proposalCount >= proposalId && proposalId > 0, "GovernorAlpha::state: invalid proposal id");
./GovernorAlpha.sol:258:        require(signatory != address(0), "GovernorAlpha::castVoteBySig: invalid signature");
./GovernorAlpha.sol:263:        require(state(proposalId) == ProposalState.Active, "GovernorAlpha::_castVote: voting is closed");
./GovernorAlpha.sol:266:        require(receipt.hasVoted == false, "GovernorAlpha::_castVote: voter already voted");
./GovernorAlpha.sol:283:        require(msg.sender == guardian, "GovernorAlpha::__acceptAdmin: sender must be gov guardian");
./GovernorAlpha.sol:288:        require(msg.sender == guardian, "GovernorAlpha::__abdicate: sender must be gov guardian");
./GovernorAlpha.sol:293:        require(msg.sender == guardian, "GovernorAlpha::__queueSetTimelockPendingAdmin: sender must be gov guardian");
./GovernorAlpha.sol:298:        require(msg.sender == guardian, "GovernorAlpha::__executeSetTimelockPendingAdmin: sender must be gov guardian");
./GovernorAlpha.sol:304:        require(c >= a, "addition overflow");
./GovernorAlpha.sol:309:        require(b <= a, "subtraction underflow");
./NoteInterest.sol:167:        require(msg.sender == admin, "only the admin may set the base rate");
./NoteInterest.sol:180:        require(msg.sender == admin, "only the admin may set the adjuster coefficient");
./NoteInterest.sol:193:        require(msg.sender == admin, "only the admin may set the update frequency");
./CNote.sol:17:        require(msg.sender == admin, "CNote::_setAccountantContract:Only admin may call this function");
./CNote.sol:94:        require(success, "TOKEN_TRANSFER_IN_FAILED");
./CNote.sol:105:            require(balanceCur == 0, "Accountant has not been correctly supplied");
./CNote.sol:147:        require(success, "TOKEN_TRANSFER_OUT_FAILED");
./CNote.sol:148:        require(token.balanceOf(address(this)) == 0, "cNote::doTransferOut: TransferOut Failed");
./CNote.sol:157:            require(_notEntered, "re-entered");
./TreasuryDelegator.sol:31:        require(msg.sender == admin, "GovernorBravoDelegator::setImplementation: admin only");
./TreasuryDelegator.sol:32:        require(implementation_ != address(0), "GovernorBravoDelegator::setImplementation: invalid implementation address");
./TreasuryDelegator.sol:121:        require(msg.value == 0, "TreasuryDelegator::fallback:cannot send value to fallback");
./GovernorBravoDelegate.sol:25:        require(address(timelock) == address(0), "GovernorBravo::initialize: can only initialize once");
./GovernorBravoDelegate.sol:26:        require(msg.sender == admin, "GovernorBravo::initialize: admin only");
./GovernorBravoDelegate.sol:27:        require(timelock_ != address(0), "GovernorBravo::initialize: invalid timelock address");
./GovernorBravoDelegate.sol:42-45:        require(unigovProposal.targets.length == unigovProposal.values.length && unigovProposal.targets.length == unigovProposal.signatures.length && unigovProposal.targets.length == unigovProposal.calldatas.length, "GovernorBravo::propose: proposal function information arity mismatch");
./GovernorBravoDelegate.sol:46:        require(unigovProposal.targets.length != 0, "GovernorBravo::propose: must provide actions");
./GovernorBravoDelegate.sol:47:        require(unigovProposal.targets.length <= proposalMaxOperations, "GovernorBravo::propose: too many actions");
./GovernorBravoDelegate.sol:76:        require(!timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))), "GovernorBravo::queueOrRevertInternal: identical proposal action already queued at eta");
./GovernorBravoDelegate.sol:85:        require(state(proposalId) == ProposalState.Queued, "GovernorBravo::execute: proposal can only be executed if it is queued");
./GovernorBravoDelegate.sol:128:        require(msg.sender == admin, "GovernorBravo::_initiate: admin only");
./GovernorBravoDelegate.sol:129:        require(initialProposalId == 0, "GovernorBravo::_initiate: can only initiate once");
./GovernorBravoDelegate.sol:140:        require(msg.sender == admin, "GovernorBravo:_setPendingAdmin: admin only");
./GovernorBravoDelegate.sol:158:        require(msg.sender == pendingAdmin, "GovernorBravo:_acceptAdmin: pending admin only");
./GovernorBravoDelegate.sol:176:        require(c >= a, "addition overflow");
./GovernorBravoDelegate.sol:181:        require(b <= a, "subtraction underflow");
./BaseV1-core.sol:256:        require(liquidity > 0, 'ILM'); // BaseV1: INSUFFICIENT_LIQUIDITY_MINTED
./BaseV1-core.sol:275:        require(amount0 > 0 && amount1 > 0, 'ILB'); // BaseV1: INSUFFICIENT_LIQUIDITY_BURNED
./BaseV1-core.sol:289:        require(amount0Out > 0 || amount1Out > 0, 'IOA'); // BaseV1: INSUFFICIENT_OUTPUT_AMOUNT
./BaseV1-core.sol:291:        require(amount0Out < _reserve0 && amount1Out < _reserve1, 'IL'); // BaseV1: INSUFFICIENT_LIQUIDITY
./BaseV1-core.sol:297:        require(to != _token0 && to != _token1, 'IT'); // BaseV1: INVALID_TO
./BaseV1-core.sol:306:        require(amount0In > 0 || amount1In > 0, 'IIA'); // BaseV1: INSUFFICIENT_INPUT_AMOUNT
./BaseV1-core.sol:312:        require(_k(_balance0, _balance1) >= _k(_reserve0, _reserve1), 'K'); // BaseV1: K
./BaseV1-core.sol:416:        require(deadline >= block.timestamp, 'BaseV1: EXPIRED');
./BaseV1-core.sol:434:        require(recoveredAddress != address(0) && recoveredAddress == owner, 'BaseV1: INVALID_SIGNATURE');
./BaseV1-core.sol:524:        require(tokenA != tokenB, 'IA'); // BaseV1: IDENTICAL_ADDRESSES
./BaseV1-core.sol:526:        require(token0 != address(0), 'ZA'); // BaseV1: ZERO_ADDRESS
./BaseV1-core.sol:527:        require(getPair[token0][token1][stable] == address(0), 'PE'); // BaseV1: PAIR_EXISTS - single check is sufficient
./Comp.sol:166:        require(signatory != address(0), "Comp::delegateBySig: invalid signature");
./Comp.sol:167:        require(nonce == nonces[signatory]++, "Comp::delegateBySig: invalid nonce");
./Comp.sol:168:        require(block.timestamp <= expiry, "Comp::delegateBySig: signature expired");
./Comp.sol:190:        require(blockNumber < block.number, "Comp::getPriorVotes: not yet determined");
./Comp.sol:234:        require(src != address(0), "Comp::_transferTokens: cannot transfer from the zero address");
./Comp.sol:235:        require(dst != address(0), "Comp::_transferTokens: cannot transfer to the zero address");
./GovernorBravoDelegator.sol:27:        require(msg.sender == admin, "GovernorBravoDelegator::_setImplementation: admin only");
./GovernorBravoDelegator.sol:28:        require(implementation_ != address(0), "GovernorBravoDelegator::_setImplementation: invalid implementation address");
./GovernorBravoDelegator.sol:40:        require(msg.sender == admin, "GovernorBravoDelegator::_initiateDelegated: admin only");
./GovernorBravoDelegator.sol:48:        require(msg.sender == admin, "GovernorBravoDelegator::_acceptInitialAdminDelegated: admin only");
./GovernorBravoDelegator.sol:56:        require(msg.sender == admin, "GovernorBravoDelegator::_setPendingAdminDelegated: admin only");
./AccountantDelegate.sol:87:		require(cNoteAmt >= noteDiff, "AccountantDelegate::sweepInterest:Error calculating interest to sweep");
./AccountantDelegate.sol:101:		require(cnote.balanceOf(treasury) == 0, "AccountantDelegate::sweepInterestError");
./BaseV1-periphery.sol:104:        require(tokenA != tokenB, "BaseV1Router: IDENTICAL_ADDRESSES");
./BaseV1-periphery.sol:106:        require(token0 != address(0), "BaseV1Router: ZERO_ADDRESS");
./BaseV1-periphery.sol:122:        require(amountA > 0, "BaseV1Router: INSUFFICIENT_AMOUNT");
./BaseV1-periphery.sol:123:        require(reserveA > 0 && reserveB > 0, "BaseV1Router: INSUFFICIENT_LIQUIDITY");
./BaseV1-periphery.sol:151:        require(routes.length >= 1, "BaseV1Router: INVALID_PATH");
./BaseV1-periphery.sol:241:                require(amountBOptimal >= amountBMin, "BaseV1Router: INSUFFICIENT_B_AMOUNT");
./BaseV1-periphery.sol:246:                require(amountAOptimal >= amountAMin, "BaseV1Router: INSUFFICIENT_A_AMOUNT");
./BaseV1-periphery.sol:313:        require(amountA >= amountAMin, "BaseV1Router: INSUFFICIENT_A_AMOUNT");
./BaseV1-periphery.sol:314:        require(amountB >= amountBMin, "BaseV1Router: INSUFFICIENT_B_AMOUNT");
./BaseV1-periphery.sol:405:        require(amounts[amounts.length - 1] >= amountOutMin, "BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT");
./BaseV1-periphery.sol:420:        require(amounts[amounts.length - 1] >= amountOutMin, "BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT");
./BaseV1-periphery.sol:433:        require(routes[0].from == address(wcanto), "BaseV1Router: INVALID_PATH");
./BaseV1-periphery.sol:435:        require(amounts[amounts.length - 1] >= amountOutMin, "BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT");
./BaseV1-periphery.sol:446:        require(routes[routes.length - 1].to == address(wcanto), "BaseV1Router: INVALID_PATH");
./BaseV1-periphery.sol:448:        require(amounts[amounts.length - 1] >= amountOutMin, "BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT");
./BaseV1-periphery.sol:470:        require(success, "TransferHelper: ETH_TRANSFER_FAILED");
./BaseV1-periphery.sol:481:        require(token.code.length > 0, "token code length failure");

Mitigation

I suggest implementing custom errors to save gas.