Governance Dynamic Quorum - Moonwell

ABIEncoderV2 is being stated in a solidity version 0.8+ which is not needed since ABIEncoderV2 is activated by default 0.8+.

Code Location

MoonwellApolloGovernor.sol#L2

MoonwellApolloGovernor.sol

pragma solidity 0.8.10;
pragma experimental ABIEncoderV2;

import "./IERC20.sol";

SOLVED: The Moonwell team solved this issue by removing ABIEncoderV2.

Commit ID: c7da88a3fe3f0062d8a83ba808b648f1da369fec

During the review the newest version of solidity was released with the important bug fixes & Bug.

Code Location

MoonwellApolloGovernor.sol#L2

MoonwellApolloGovernor.sol

pragma solidity 0.8.10;
pragma experimental ABIEncoderV2;

import "./IERC20.sol";

SOLVED: The Moonwell team solved this issue by updating pragma to 0.8.17.

Commit ID: c7da88a3fe3f0062d8a83ba808b648f1da369fec

boolean variable are initialized to a default value of false per Solidity docs. Setting a variable to the default value is unnecessary.

Code Location

MoonwellApolloGovernor.sol#L278

MoonwellApolloGovernor.sol

      Proposal storage newProposal = proposals[proposalCount];
        newProposal.id = proposalCount;
        newProposal.proposer = msg.sender;
        newProposal.eta = 0;
        newProposal.targets = targets;
        newProposal.values = values;
        newProposal.signatures = signatures;
        newProposal.calldatas = calldatas;
        newProposal.startTimestamp = startTimestamp;
        newProposal.endTimestamp = endTimestamp;
        newProposal.startBlock = 0;
        newProposal.forVotes = 0;
        newProposal.againstVotes = 0;
        newProposal.abstainVotes = 0;
        newProposal.totalVotes = 0;
        newProposal.canceled = false;
        newProposal.executed = false;
        newProposal.quorum = currentQuorum;
        newProposal.quorumAdjusted = false;

SOLVED: The Moonwell team solved this issue by removing explicit initialization.

Commit ID: c7da88a3fe3f0062d8a83ba808b648f1da369fec

Events allow capturing the changed parameters so that off-chain tools/interfaces can register such changes with timelocks that allow users to evaluate them and consider if they would like to engage/exit based on how they perceive the changes as affecting the trustworthiness of the protocol or profitability of the implemented financial services. The alternative of directly querying on-chain contract state for such changes is not considered practical for most users/usages. In the implementation, current quorum is not emitted on the proposal generation.

Code Location

MoonwellApolloGovernor.sol#L282

MoonwellApolloGovernor.sol

        Proposal storage newProposal = proposals[proposalCount];
        newProposal.id = proposalCount;
        newProposal.proposer = msg.sender;
        newProposal.eta = 0;
        newProposal.targets = targets;
        newProposal.values = values;
        newProposal.signatures = signatures;
        newProposal.calldatas = calldatas;
        newProposal.startTimestamp = startTimestamp;
        newProposal.endTimestamp = endTimestamp;
        newProposal.startBlock = 0;
        newProposal.forVotes = 0;
        newProposal.againstVotes = 0;
        newProposal.abstainVotes = 0;
        newProposal.totalVotes = 0;
        newProposal.canceled = false;
        newProposal.executed = false;
        newProposal.quorum = currentQuorum;
        newProposal.quorumAdjusted = false;

        latestProposalIds[newProposal.proposer] = proposalCount;

        emit ProposalCreated(newProposal.id, msg.sender, targets, values, signatures, calldatas, startTimestamp, endTimestamp, description);
        return newProposal.id;

SOLVED: The Moonwell team solved this issue by adding current quorum to the event.

Commit ID: c7da88a3fe3f0062d8a83ba808b648f1da369fec

The code sections use i++ which costs more gas than ++i, especially in a loop. Finally, the initialization of i = 0 can be skipped, as 0 is the default value.

Code Location

MoonwellApolloGovernor.sol#L255-L676

MoonwellApolloGovernor.sol

    function getQuorum() public view returns (uint) {
        uint newQuorum = currentQuorum;

        // Start at the high water mark
        for (uint i = lastQuorumAdjustment + 1; i < proposalCount; i++) {
            // Pull state and ignore in flight proposals
            ProposalState proposalState = state(i);
            if (proposalState == ProposalState.Pending || proposalState == ProposalState.Active) {
                continue;
            }

            // Get the proposal
            Proposal storage proposal = proposals[i];

            // Only proceed if quorum for this proposal is not yet taken into account.
            if (!proposal.quorumAdjusted) {
                // If a proposal is canceled, ignore it in quorum calculations.
                if (proposalState == ProposalState.Canceled) {
                    continue;
                }

                // Adjust quorum in accordance with the proposal.
                newQuorum = _calculateNewQuorum(newQuorum, proposal.totalVotes);
            }
        }

        return newQuorum;
    }

SOLVED: The Moonwell team solved this issue with using prefix increment.

Commit ID: c7da88a3fe3f0062d8a83ba808b648f1da369fec

Solidity versions >= 0.8.x perform checked arithmetic by default, so the SafeMath library is unnecessary in most cases.

Code Location

MoonwellApolloGovernor.sol#L704-L713

MoonwellApolloGovernor.sol

    function add256(uint256 a, uint256 b) internal pure returns (uint) {
        uint c = a + b;
        require(c >= a, "addition overflow");
        return c;
    }

    function sub256(uint256 a, uint256 b) internal pure returns (uint) {
        require(b <= a, "subtraction underflow");
        return a - b;
    }

SOLVED: The Moonwell team solved this issue with deleting SafeMath from the contract.

Commit ID: c7da88a3fe3f0062d8a83ba808b648f1da369fec

Some functions are missing @param for some of their parameters. Given that NatSpec is an important part of code documentation, this affects code comprehension, auditability, and usability.

Code Location

MoonwellApolloGovernor.sol#L523

MoonwellApolloGovernor.sol

    function setQuorumCaps(uint newLowerQuorumCap, uint newUpperQuorumCap) external {
        require(msg.sender == address(timelock), "only timelock");

        if (newLowerQuorumCap != lowerQuorumCap) {
            uint oldLowerQuorumCap = lowerQuorumCap;
            lowerQuorumCap = newLowerQuorumCap;
            emit LowerQuorumCapChanged(oldLowerQuorumCap, newLowerQuorumCap);
        }

        if (newUpperQuorumCap != upperQuorumCap) {
            uint oldUpperQuorumCap = upperQuorumCap;
            upperQuorumCap = newUpperQuorumCap;
            emit UpperQuorumCapChanged(oldUpperQuorumCap, newUpperQuorumCap);
        }
    }

SOLVED: The Moonwell team solved this issue by adding natspecs on the functions.

Commit ID: c7da88a3fe3f0062d8a83ba808b648f1da369fec