Canto v2 contest - rfa's results

QA

[QA-1] Wrong argument for error SenderNotCNote()

https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Accountant/AccountantDelegate.sol#L51 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Accountant/AccountantDelegate.sol#L65

Sender address was expected to be inputed as the argument. But in the current implementation, the note address is inputed as the argument

RECOMMENDED MITIGATION STEP Change address(note) to msg.sender

[QA-2] _accountant address is allowed to do reentrancy

https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/CNote.sol#L156-L157

Removing the check that msg.sender != _accountant can save gas (by reducing action in code), beside it also can prevent any security issue by including _accountant to the validation step

GAS

[G-1] Better way to return err

https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Accountant/AccountantDelegate.sol#L49-L55

Declaring de var in returns() without using statement value can save 13 execution gas fee

RECOMMENDED MITIGATION

    function supplyMarket(uint amount) external override returns(uint err) {
		if (msg.sender != address(cnote)) {
			revert SenderNotCNote(address(cnote));
		}
		err =  cnote.mint(amount);
		emit AcctSupplied(amount, uint(err));
		// @audit-info: remove return statement here
     }

[G-2] revert string > 32 bytes

https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/CNote.sol#L148 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/Comp.sol#L190 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/Comp.sol#L234-L235 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/GovernorAlpha.sol#L145-L146

Strings are broken into 32 byte chunks for operations. Revert error strings over 32 bytes therefore consume extra gas than shorter strings

[G-3] Using uint8(1) and uint8(2) for nonreentrant modifier

https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/CNote.sol#L155-L162

Using uint8 instead of bool as variable type can save 386 execution gas fee per nonReentrant modifier call

RECOMMENDED MITIGATION STEP

uint8 _notEntered = 1;

    modifier nonReentrant() {
        require(_notEntered == 1);
        _notEntered = 2;
        _;
        _notEntered = 1;
    }

[G-4] Unnecessary balanceCur MSTORE

https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/CNote.sol#L103

balanceCur is just called once in the function. We can do SLOAD directly. Remove L103

[G-5] Unnecessary check for overflow/underflow

https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/Comp.sol#L285-L294

Using function to validate that the result of math operation won't underflow/overflow in solidity >=0.8.0 its gas consuming. I recommend to add/sub the value without check (without add96/sub96 function), or use unchecked for operation

MITIGATION STEP

    function add96(uint96 a, uint96 b, string memory errorMessage) internal pure returns (uint96 c) {
        unchecked{ c = a + b;} //@audit: Declare c in returns()
        require(c >= a, errorMessage);
	//@audit: remove return statement
    }

    function sub96(uint96 a, uint96 b, string memory errorMessage) internal pure returns (uint96) {
        require(b <= a, errorMessage);
        unchecked{return a - b;}
    }

[G-6] Tight Proposal struct variable packing

https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/GovernorAlpha.sol#L35-L77

By placing bool (which) has 1 bytes size next to address (20 bytes size) can save 1 slot at the storage

RECOMMENDED MITIGATION STEP

    struct Proposal {
        /// @notice Unique id for looking up a proposal
        uint id;

        /// @notice Creator of the proposal
        address proposer;

        /// @notice Flag marking whether the proposal has been canceled
        bool canceled;

        /// @notice Flag marking whether the proposal has been executed
        bool executed;

        /// @notice The timestamp that the proposal will be available for execution, set once the vote succeeds
        uint eta;

        /// @notice the ordered list of target addresses for calls to be made
        address[] targets;

        /// @notice The ordered list of values (i.e. msg.value) to be passed to the calls to be made
        uint[] values;

        /// @notice The ordered list of function signatures to be called
        string[] signatures;

        /// @notice The ordered list of calldata to be passed to each call
        bytes[] calldatas;

        /// @notice The block at which voting begins: holders must delegate their votes prior to this block
        uint startBlock;

        /// @notice The block at which voting ends: votes must be cast prior to this block
        uint endBlock;

        /// @notice Current number of votes in favor of this proposal
        uint forVotes;

        /// @notice Current number of votes in opposition to this proposal
        uint againstVotes;

        /// @notice Receipts of ballots for the entire set of voters
        mapping (address => Receipt) receipts;
    }

We can move canceled and executed below proposer var

[G-7] Avoid too many proposalCount SLOAD

https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/GovernorAlpha.sol#L152-L153

By using prefix increment and do the operation while assigning proposalId can save 111 execution gas

RECOMMENDED MITIGATION STEP

        uint proposalId = ++proposalCount;