Due to extremely short `votingDelay` and `votingPeriod`, governance is practically impossible.

[c4-submissions]

Lines of code

https://github.com/open-dollar/od-contracts/blob/f4f0246bb26277249c1d5afe6201d4d9096e52e6/src/contracts/gov/ODGovernor.sol#L41 https://github.com/open-dollar/od-contracts/blob/v1.5.5-audit/src/contracts/gov/ODGovernor.sol#L31-L41

Vulnerability details

Impact

It is practically impossible for enough users to vote within a voting period of 4 seconds from the voting start time.

Proof of Concept

The problem lies in the votingDelay and votingPeriod that are set in the ODGovernor.sol this way.

contract ODGovernor is
  Governor,
  GovernorSettings,
  GovernorCompatibilityBravo,
  GovernorVotes,
  GovernorVotesQuorumFraction,
  GovernorTimelockControl
{
...
constructor(
    address _token,
    TimelockController _timelock
  )
    Governor('ODGovernor')//@audit larger voting delay gives users the time to unstake tokens if necessary.
    GovernorSettings(1, 15, 0)//@audit voting period 15blocks too small and no function to update. quorum will never be reached.
    GovernorVotes(IVotes(_token))
    GovernorVotesQuorumFraction(3)
    GovernorTimelockControl(_timelock)
  {}//@audit votingPeriod: how long does a proposal remain open to votes.
...
}

In the constructor above the GovernorSettings constructor is used to set the votingDelay and votingPeriod.

GovernorSettings(initialVotingDelay, initialVotingPeriod, initialProposalThreshold) measured in blocks
GovernorSettings(1, 15, 0)

The voting delay and voting period are measured in blocks. Since this project is built `specifically for Arbitrum, we will consider Arbitrum particularly to set this values.

• The average blocktime on arbitrum is 0.26 seconds.
• so setting 1 voting delay in the code above implies that there is just 0.26 seconds delay from proposal to voting start time.
• And setting 15 voting period above implies that users have just 15 * 0.26 = 3.9 seconds to cast their vote.

It will be more practical to set atleast 1 day voting delay and atleast 1 week voting period as set in OZ governance examples. To convert these times to blocks on Arbitrum based on 0.26 secs avg blocktime: 1 day = 86400 / 0.26 = ~332, 308 blocks per day so 1 week will be 332, 308 * 7 = 2,326,156 blocks per week

so it will be more practical for GovernorSettings parameters to be set this way:

//votingDelay = 1day and votingPeriod = 1week.
GovernorSettings(332308, 2326156, 0); //particulary for Arbitrum based on block numbers.

Calculation reference from OZ docs: https://docs.openzeppelin.com/contracts/4.x/governance#governor

In the inherited OZ's Governor.sol, the voting delay and voting period are used to set each proposal's voting start time and deadline in the propose function this way.

function propose(
        address[] memory targets,
        uint256[] memory values,
        bytes[] memory calldatas,
        string memory description
    ) public virtual override returns (uint256) {
...
 ProposalCore storage proposal = _proposals[proposalId];
 require(proposal.voteStart.isUnset(), "Governor: proposal already exists");
uint64 snapshot = block.number.toUint64() + votingDelay().toUint64();//@audit voting delay.
        uint64 deadline = snapshot + votingPeriod().toUint64();

        proposal.voteStart.setDeadline(snapshot);
        proposal.voteEnd.setDeadline(deadline);
...
}

Tools Used

Openzeppelin Governance docs, OZ's smart contract wizard. https://docs.openzeppelin.com/contracts/4.x/governance#governor

Recommended Mitigation Steps

• Short term: Use Oz's examples but do your calculations based on Arbitrum block time:

  GovernorSettings(332308, 2326156, 0); //particulary for Arbitrum based on block numbers.

  OZ's wizard example: https://wizard.openzeppelin.com/#governor

• Long term: Use latest Oz's from 4.9 and above that have time based voting instead of block number. read here: https://docs.openzeppelin.com/contracts/4.x/governance#disclaimer

Assessed type

Governance

[c4-pre-sort]

raymondfam marked the issue as low quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #73

[c4-judge]

MiloTruck marked the issue as selected for report

[MiloTruck]

The warden has demonstrated how the configured values for GovernorSettings are far too small for any effective governance to take place, since users only have ~4 seconds to cast votes. Therefore, all governor-related functionality in the Vault721 contract will be unaccessible.

Since this only impacts setter functions and does not affect the protocol's core functionality, I believe medium severity is appropriate.

[c4-judge]

MiloTruck marked the issue as satisfactory

[pi0neerpat]

The governance settings currently set are for testing, not production, however finding is valid as this was not explicitly stated before the audit. Additionally, recommendation for both short and long-term solutions is appreciated.