Unable to set the minimum proposal threshold

Summary

Collections with high total supply NFT will not be able to set the minimum proposal threshold.

Vulnerability Detail

The protocol assures that the proposalThresholdBPS can be setted at least = 1, which means that at least 1 basis point is enough to reach the minimum proposal threshold:

/// @notice The minimum setable proposal threshold
    uint256 public constant MIN_PROPOSAL_THRESHOLD_BPS = 1;  // 1 basis point or 0.01%

/**
     * @notice Admin function for setting the proposal threshold basis points
     * @dev newProposalThresholdBPS must be in [`MIN_PROPOSAL_THRESHOLD_BPS`,`MAX_PROPOSAL_THRESHOLD_BPS`]
     * @param newProposalThresholdBPS new proposal threshold
     */
    function _setProposalThresholdBPS(uint256 newProposalThresholdBPS) external onlyAdmin {
        require(
            newProposalThresholdBPS >= MIN_PROPOSAL_THRESHOLD_BPS &&
                newProposalThresholdBPS <= MAX_PROPOSAL_THRESHOLD_BPS,
            'NounsDAO::_setProposalThreshold: invalid proposal threshold bps'
        );
        //...
    }

/// @notice The number of votes needed to create a proposal at the time of proposal creation. *DIFFERS from GovernerBravo
        uint256 proposalThreshold;

However, collections with at least 20000 NFTs in total supply can’t set proposal threshold = 1. If totalSupply = 20000, with the lowest settings (ds().proposalThresholdBPS = 1), it would returns (20000 * 1) / 10000 = 2:

temp.propThreshold = proposalThreshold(ds, temp.adjustedTotalSupply);

    /**
     * @notice Current proposal threshold using Noun Total Supply
     * Differs from `GovernerBravo` which uses fixed amount
     */
    function proposalThreshold(
        NounsDAOTypes.Storage storage ds,
        uint256 adjustedTotalSupply
    ) internal view returns (uint256) {
        return bps2Uint(ds.proposalThresholdBPS, adjustedTotalSupply); 
    }

    function bps2Uint(uint256 bps, uint256 number) internal pure returns (uint256) {
        return (number * bps) / 10000;
    }

And it can’t be lowered more. Moreover, even if proposal threshold = 2, proposal in proposeBySigs() function will revert even if votes = 2, not lower than temp.propThreshold, which defies logic of protocol's proposal thresholds.

temp.propThreshold = proposalThreshold(ds, temp.adjustedTotalSupply);

if (votes <= temp.propThreshold) revert VotesBelowProposalThreshold();  // if votes = 2 revert, must be at least 3

All of this breaks the protocol's internal logic and cannot be design choise.

Impact

Collections with at least 20000 NFTs in total supply will not be able to set the minimum proposal threshold to lowest values.

Code Snippet

[https://github.com/sherlock-audit/2024-03-nouns-dao-2/blob/main/nouns-monorepo/packages/nouns-contracts/contracts/governance/NounsDAOProposals.sol#L993]() [https://github.com/sherlock-audit/2024-03-nouns-dao-2/blob/main/nouns-monorepo/packages/nouns-contracts/contracts/governance/NounsDAOProposals.sol#L935]() [https://github.com/sherlock-audit/2024-03-nouns-dao-2/blob/main/nouns-monorepo/packages/nouns-contracts/contracts/governance/NounsDAOAdmin.sol#L189]()

Tool used

Manual Review

Recommendation

Increase division to a more precise value such as 1e18 to allow collections with high total supply NFT to always set any allowed threshold, even 1:

function bps2Uint(uint256 bps, uint256 number) internal pure returns (uint256) {
-       return (number * bps) / 10000;
+       return (number * bps) / 1e18;
    }

And proposal must not revert if votes = propThreshold, not below threshold:

-     if (votes <= propThreshold) revert VotesBelowProposalThreshold();
+     if (votes < propThreshold) revert VotesBelowProposalThreshold();

sherlock-audit / 2024-03-nouns-dao-2-judging

DenTonylifer - Unable to set the minimum proposal threshold #3