Improve documentation of various governance aspects (#3161)

Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
(cherry picked from commit 85566fae)

contracts/governance/Governor.sol

@@ -41,8 +41,9 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor {
     mapping(uint256 => ProposalCore) private _proposals;
 
     /**
-     * @dev Restrict access to governor executing address. Some module might override the _executor function to make
-     * sure this modifier is consistant with the execution model.
+     * @dev Restrict access of functions to the governance executor, which may be the Governor itself or a timelock
+     * contract, as specified by {_executor}. This generally means that function with this modifier must be voted on and
+     * executed through the governance protocol.
      */
     modifier onlyGovernance() {
         require(_msgSender() == _executor(), "Governor: onlyGovernance");

contracts/governance/extensions/GovernorTimelockCompound.sol

@@ -224,14 +224,16 @@ abstract contract GovernorTimelockCompound is IGovernorTimelock, Governor {
 
     /**
      * @dev Public endpoint to update the underlying timelock instance. Restricted to the timelock itself, so updates
-     * must be proposed, scheduled and executed using the {Governor} workflow.
+     * must be proposed, scheduled, and executed through governance proposals.
      *
-     * For security reason, the timelock must be handed over to another admin before setting up a new one. The two
+     * For security reasons, the timelock must be handed over to another admin before setting up a new one. The two
      * operations (hand over the timelock) and do the update can be batched in a single proposal.
      *
      * Note that if the timelock admin has been handed over in a previous operation, we refuse updates made through the
      * timelock if admin of the timelock has already been accepted and the operation is executed outside the scope of
      * governance.
+
+     * CAUTION: It is not recommended to change the timelock while there are other queued governance proposals.
      */
     function updateTimelock(ICompoundTimelock newTimelock) external virtual onlyGovernance {
         _updateTimelock(newTimelock);

contracts/governance/extensions/GovernorTimelockControl.sol

@@ -16,9 +16,10 @@ import "../TimelockController.sol";
  * the assets and permissions must be attached to the {TimelockController}. Any asset sent to the {Governor} will be
  * inaccessible.
  *
- * WARNING: Setting up the TimelockController to have additional proposers besides the governor introduces the risk that
- * approved governance proposals could be blocked by the other proposers, effectively executing a Denial of Service attack,
- * and therefore blocking access to governance-controlled assets.
+ * WARNING: Setting up the TimelockController to have additional proposers besides the governor is very risky, as it
+ * grants them powers that they must be trusted or known not to use: 1) {onlyGovernance} functions like {relay} are
+ * available to them through the timelock, and 2) approved governance proposals can be blocked by them, effectively
+ * executing a Denial of Service attack. This risk will be mitigated in a future release.
  *
  * _Available since v4.3._
  */
@@ -147,7 +148,9 @@ abstract contract GovernorTimelockControl is IGovernorTimelock, Governor {
 
     /**
      * @dev Public endpoint to update the underlying timelock instance. Restricted to the timelock itself, so updates
-     * must be proposed, scheduled and executed using the {Governor} workflow.
+     * must be proposed, scheduled, and executed through governance proposals.
+     *
+     * CAUTION: It is not recommended to change the timelock while there are other queued governance proposals.
      */
     function updateTimelock(TimelockController newTimelock) external virtual onlyGovernance {
         _updateTimelock(newTimelock);

contracts/governance/extensions/GovernorVotesQuorumFraction.sol

@@ -16,26 +16,61 @@ abstract contract GovernorVotesQuorumFraction is GovernorVotes {
 
     event QuorumNumeratorUpdated(uint256 oldQuorumNumerator, uint256 newQuorumNumerator);
 
+    /**
+     * @dev Initialize quorum as a fraction of the token's total supply.
+     *
+     * The fraction is specified as `numerator / denominator`. By default the denominator is 100, so quorum is
+     * specified as a percent: a numerator of 10 corresponds to quorum being 10% of total supply. The denominator can be
+     * customized by overriding {quorumDenominator}.
+     */
     constructor(uint256 quorumNumeratorValue) {
         _updateQuorumNumerator(quorumNumeratorValue);
     }
 
+    /**
+     * @dev Returns the current quorum numerator. See {quorumDenominator}.
+     */
     function quorumNumerator() public view virtual returns (uint256) {
         return _quorumNumerator;
     }
 
+    /**
+     * @dev Returns the quorum denominator. Defaults to 100, but may be overridden.
+     */
     function quorumDenominator() public view virtual returns (uint256) {
         return 100;
     }
 
+    /**
+     * @dev Returns the quorum for a block number, in terms of number of votes: `supply * numerator / denominator`.
+     */
     function quorum(uint256 blockNumber) public view virtual override returns (uint256) {
         return (token.getPastTotalSupply(blockNumber) * quorumNumerator()) / quorumDenominator();
     }
 
+    /**
+     * @dev Changes the quorum numerator.
+     *
+     * Emits a {QuorumNumeratorUpdated} event.
+     *
+     * Requirements:
+     *
+     * - Must be called through a governance proposal.
+     * - New numerator must be smaller or equal to the denominator.
+     */
     function updateQuorumNumerator(uint256 newQuorumNumerator) external virtual onlyGovernance {
         _updateQuorumNumerator(newQuorumNumerator);
     }
 
+    /**
+     * @dev Changes the quorum numerator.
+     *
+     * Emits a {QuorumNumeratorUpdated} event.
+     *
+     * Requirements:
+     *
+     * - New numerator must be smaller or equal to the denominator.
+     */
     function _updateQuorumNumerator(uint256 newQuorumNumerator) internal virtual {
         require(
             newQuorumNumerator <= quorumDenominator(),

docs/modules/ROOT/pages/governance.adoc

@@ -248,7 +248,7 @@ contract MyGovernor is Governor, GovernorCompatibilityBravo, GovernorVotes, Gove
 
 It is good practice to add a timelock to governance decisions. This allows users to exit the system if they disagree with a decision before it is executed. We will use OpenZeppelin’s TimelockController in combination with the GovernorTimelockControl module.
 
-IMPORTANT: When using a timelock, it is the timelock that will execute proposals and thus the timelock that should hold any funds, ownership, and access control roles. Funds in the Governor contract are not currently retrievable when using a timelock! (As of version 4.3 there is a caveat when using the Compound Timelock: ETH in the timelock is not easily usable, so it is recommended to manage ERC20 funds only in this combination until a future version resolves the issue.)
+IMPORTANT: When using a timelock, it is the timelock that will execute proposals and thus the timelock that should hold any funds, ownership, and access control roles. Before version 4.5 there was no way to recover funds in the Governor contract when using a timelock! Before version 4.3, when using the Compound Timelock, ETH in the timelock was not easily accessible.
 
 TimelockController uses an AccessControl setup that we need to understand in order to set up roles.
 
@@ -294,7 +294,9 @@ This will create a new proposal, with a proposal id that is obtained by hashing
 
 === Cast a Vote
 
-Once a proposal is active, stakeholders can cast their vote. This is done through a function in the Governor contract that users can invoke directly from a governance UI such as Tally.
+Once a proposal is active, delegates can cast their vote. Note that it is delegates who carry voting power: if a token holder wants to participate, they can set a trusted representative as their delegate, or they can become a delegate themselves by self-delegating their voting power.
+
+Votes are cast by interacting with the Governor contract through the `castVote` family of functions. Voters would generally invoke this from a governance UI such as Tally.
 
 image::tally-vote.png[Voting in Tally]