feat: proposal guardian (#16)

* feat: proposal guardian

* remove `.only`

* create new `proposalGuardian` role

* add tests

* Update comments per recommendation

* feat: proposal guardian expires

* lint `GovernorBravoDelegate`

* feat: expose guardian expiration (#17)

.gitignore

@@ -8,3 +8,4 @@ cache
 artifacts
 yarn-error.log
 .DS_Store
+yarn.lock

.prettierignore

@@ -2,5 +2,10 @@ node_modules
 artifacts
 cache
 coverage*
-contracts*
+contracts/test*
+contracts/SafeMath.sol
+contracts/Timelock.sol
+contracts/Comp.sol
+contracts/GovernorBravoDelegator.sol
+contracts/GovernorBravoInterfaces.sol
 typechain-types

.prettierrc

@@ -0,0 +1,11 @@
+{
+    "plugins": ["prettier-plugin-solidity"],
+    "overrides": [
+      {
+        "files": "*.sol",
+        "options": {
+          "bracketSpacing": true
+        }
+      }
+    ]
+  }

contracts/GovernorBravoDelegate.sol

@@ -1,11 +1,11 @@
 // SPDX-License-Identifier: BSD-3-Clause
 pragma solidity ^0.8.10;
 
-import { GovernorBravoDelegateStorageV2, GovernorBravoEvents, TimelockInterface, CompInterface, GovernorAlphaInterface } from "./GovernorBravoInterfaces.sol";
+import { GovernorBravoDelegateStorageV3, GovernorBravoEvents, TimelockInterface, CompInterface, GovernorAlphaInterface } from "./GovernorBravoInterfaces.sol";
 
 /// @custom:security-contact [email protected]
 contract GovernorBravoDelegate is
-    GovernorBravoDelegateStorageV2,
+    GovernorBravoDelegateStorageV3,
     GovernorBravoEvents
 {
     /// @notice The name of this contract
@@ -157,7 +157,10 @@ contract GovernorBravoDelegate is
         bytes32 r,
         bytes32 s
     ) public returns (uint) {
-        require(proposalId == proposalCount + 1, "GovernorBravo::proposeBySig: invalid proposal id");
+        require(
+            proposalId == proposalCount + 1,
+            "GovernorBravo::proposeBySig: invalid proposal id"
+        );
         address signatory;
         {
             bytes32 domainSeparator = keccak256(
@@ -368,7 +371,7 @@ contract GovernorBravoDelegate is
     }
 
     /**
-     * @notice Cancels a proposal only if sender is the proposer, or proposer delegates dropped below proposal threshold
+     * @notice Cancels a proposal if sender is the proposer or proposalGuardian. Can be called by anyone if the proposer delegates dropped below proposal threshold.
      * @param proposalId The id of the proposal to cancel
      */
     function cancel(uint proposalId) external {
@@ -379,21 +382,25 @@ contract GovernorBravoDelegate is
 
         Proposal storage proposal = proposals[proposalId];
 
-        // Proposer can cancel
-        if (msg.sender != proposal.proposer) {
-            // Whitelisted proposers can't be canceled for falling below proposal threshold
+        // ProposalGuardian is only active if the expiration is in the future
+        address proposalGuardian = proposalGuardian.expiration >=
+            block.timestamp
+            ? proposalGuardian.account
+            : address(0);
+
+        // Proposer or proposalGuardian can cancel
+        if (msg.sender != proposal.proposer && msg.sender != proposalGuardian) {
+            require(
+                (comp.getPriorVotes(proposal.proposer, block.number - 1) <
+                    proposalThreshold),
+                "GovernorBravo::cancel: proposer above threshold"
+            );
+            // Only whitelist guardian can cancel proposals from whitelisted proposers with delegations below the proposal threshold
             if (isWhitelisted(proposal.proposer)) {
                 require(
-                    (comp.getPriorVotes(proposal.proposer, block.number - 1) <
-                        proposalThreshold) && msg.sender == whitelistGuardian,
+                    msg.sender == whitelistGuardian,
                     "GovernorBravo::cancel: whitelisted proposer"
                 );
-            } else {
-                require(
-                    (comp.getPriorVotes(proposal.proposer, block.number - 1) <
-                        proposalThreshold),
-                    "GovernorBravo::cancel: proposer above threshold"
-                );
             }
         }
 
@@ -653,7 +660,7 @@ contract GovernorBravoDelegate is
 
     /**
      * @notice View function which returns if an account is whitelisted
-     * @param account Account to check white list status of
+     * @param account Account to check whitelist status of
      * @return If the account is whitelisted
      */
     function isWhitelisted(address account) public view returns (bool) {
@@ -754,6 +761,28 @@ contract GovernorBravoDelegate is
         emit WhitelistGuardianSet(oldGuardian, whitelistGuardian);
     }
 
+    /**
+     * @notice Admin function for setting the proposalGuardian. ProposalGuardian can cancel all proposals.
+     * @param newProposalGuardian The account and expiration for the new proposal guardian.
+     */
+    function _setProposalGuardian(
+        ProposalGuardian memory newProposalGuardian
+    ) external {
+        require(
+            msg.sender == admin,
+            "GovernorBravo::_setProposalGuardian: admin only"
+        );
+
+        emit ProposalGuardianSet(
+            proposalGuardian.account,
+            proposalGuardian.expiration,
+            newProposalGuardian.account,
+            newProposalGuardian.expiration
+        );
+
+        proposalGuardian = newProposalGuardian;
+    }
+
     /**
      * @notice Initiate the GovernorBravo contract
      * @dev Admin only. Sets initial proposal id which initiates the contract, ensuring a continuous proposal id count

contracts/GovernorBravoInterfaces.sol

@@ -1,7 +1,6 @@
 // SPDX-License-Identifier: BSD-3-Clause
 pragma solidity ^0.8.10;
 
-
 contract GovernorBravoEvents {
     /// @notice An event emitted when a new proposal is created
     event ProposalCreated(uint id, address proposer, address[] targets, uint[] values, string[] signatures, bytes[] calldatas, uint startBlock, uint endBlock, string description);
@@ -48,6 +47,13 @@ contract GovernorBravoEvents {
 
     /// @notice Emitted when the whitelistGuardian is set
     event WhitelistGuardianSet(address oldGuardian, address newGuardian);
+
+    /// @notice Emitted when the proposalGuardian is set
+    event ProposalGuardianSet(
+        address oldProposalGuardian, 
+        uint96 oldProposalGuardianExpiry, 
+        address newProposalGuardian, 
+        uint newProposalGuardianExpiry);
 }
 
 contract GovernorBravoDelegatorStorage {
@@ -61,15 +67,13 @@ contract GovernorBravoDelegatorStorage {
     address public implementation;
 }
 
-
 /**
  * @title Storage for Governor Bravo Delegate
  * @notice For future upgrades, do not change GovernorBravoDelegateStorageV1. Create a new
  * contract which implements GovernorBravoDelegateStorageV1 and following the naming convention
  * GovernorBravoDelegateStorageVX.
  */
 contract GovernorBravoDelegateStorageV1 is GovernorBravoDelegatorStorage {
-
     /// @notice The delay before voting on a proposal may take place, once proposed, in blocks
     uint public votingDelay;
 
@@ -97,7 +101,6 @@ contract GovernorBravoDelegateStorageV1 is GovernorBravoDelegatorStorage {
     /// @notice The latest proposal for each proposer
     mapping (address => uint) public latestProposalIds;
 
-
     struct Proposal {
         /// @notice Unique id for looking up a proposal
         uint id;
@@ -178,6 +181,19 @@ contract GovernorBravoDelegateStorageV2 is GovernorBravoDelegateStorageV1 {
     address public whitelistGuardian;
 }
 
+contract GovernorBravoDelegateStorageV3 is GovernorBravoDelegateStorageV2 {
+    /// @notice The address and expiration of the proposal guardian.
+    struct ProposalGuardian {
+        // Address of the `ProposalGuardian`
+        address account;
+        // Timestamp at which the guardian loses the ability to cancel proposals
+        uint96 expiration;
+    }
+
+    /// @notice Account which has the ability to cancel proposals. This privilege expires at the given expiration timestamp.
+    ProposalGuardian public proposalGuardian;
+}
+
 interface TimelockInterface {
     function delay() external view returns (uint);
     function GRACE_PERIOD() external view returns (uint);

test/ForkTestSimulateUpgrade.ts

@@ -11,6 +11,7 @@ import {
   reset,
   impersonateAccount,
   loadFixture,
+  time,
 } from "@nomicfoundation/hardhat-network-helpers";
 
 describe("ForkTestSimulateUpgrade", function () {
@@ -30,7 +31,7 @@ describe("ForkTestSimulateUpgrade", function () {
       "0xc0Da02939E1441F497fd74F78cE7Decb17B66529"
     );
     const proposingSigner = await ethers.getSigner(
-      "0x2775b1c75658Be0F640272CCb8c72ac986009e38"
+      "0xF977814e90dA44bFA03b6295A0616a897441aceC"
     );
     await hardhat.network.provider.send("hardhat_setBalance", [
       proposingSigner.address,
@@ -109,6 +110,9 @@ describe("ForkTestSimulateUpgrade", function () {
     expect(await governorBravoDelegator.timelock()).to.equal(
       "0x6d903f6003cca6255D85CcA4D3B5E5146dC33925"
     );
+    expect(await governorBravoDelegator.whitelistGuardian()).to.equal(
+      "0xbbf3f1421D886E9b2c5D716B5192aC998af2012c"
+    );
   });
 
   it("Grant COMP proposal", async function () {
@@ -176,4 +180,38 @@ describe("ForkTestSimulateUpgrade", function () {
       .to.emit(governorBravoDelegator, "VoteCast")
       .withArgs(signer.address, proposalId, 1, BigInt("1000"), "Great Idea!");
   });
+
+  it("Set proposal guardian", async function () {
+    const { governorBravoDelegator, proposingSigner } = await loadFixture(
+      deployFixtures
+    );
+    const [signer] = await ethers.getSigners();
+    const setProposalGuardianSelector = ethers
+      .id("_setProposalGuardian((address,uint96))")
+      .substring(0, 10);
+    const expirationTimestamp = (await time.latest()) + 6 * 30 * 24 * 60 * 60; // Expire in 6 months
+    const setProposalGuardianData =
+      setProposalGuardianSelector +
+      ethers.AbiCoder.defaultAbiCoder()
+        .encode(["address", "uint96"], [signer.address, expirationTimestamp])
+        .slice(2);
+
+    expect(await governorBravoDelegator.proposalGuardian()).to.deep.equal([
+      ethers.ZeroAddress,
+      0,
+    ]);
+
+    await proposeAndExecute(
+      governorBravoDelegator.connect(proposingSigner),
+      [await governorBravoDelegator.getAddress()],
+      [0],
+      [setProposalGuardianData],
+      "Set Proposal Guardian"
+    );
+
+    expect(await governorBravoDelegator.proposalGuardian()).to.deep.equal([
+      signer.address,
+      expirationTimestamp,
+    ]);
+  });
 });

test/GovernorBravo.ts

@@ -683,6 +683,17 @@ describe("Governor Bravo", function () {
       await governorBravo.connect(otherAccount).cancel(proposalId);
     });
 
+    it("Happy path: guardian can cancel", async function () {
+      const { governorBravo, otherAccount } = await loadFixture(deployFixtures);
+      const proposalId = await proposeAndPass(governorBravo);
+
+      await governorBravo._setProposalGuardian({
+        account: otherAccount,
+        expiration: (await time.latest()) + 1000,
+      });
+      await governorBravo.connect(otherAccount).cancel(proposalId);
+    });
+
     it("Error: above threshold", async function () {
       const { governorBravo, otherAccount } = await loadFixture(deployFixtures);
       const proposalId = await proposeAndPass(governorBravo);
@@ -740,28 +751,6 @@ describe("Governor Bravo", function () {
           "GovernorBravo::cancel: whitelisted proposer"
         );
       });
-
-      it("Error: whitelisted proposer above threshold", async function () {
-        const { governorBravo, owner, otherAccount, comp } = await loadFixture(
-          deployFixtures
-        );
-
-        await governorBravo._setWhitelistAccountExpiration(
-          otherAccount,
-          (await time.latest()) + 1000
-        );
-        const proposalId = await propose(governorBravo.connect(otherAccount));
-        await comp.transfer(
-          otherAccount,
-          BigInt("100000") * BigInt("10") ** BigInt("18")
-        );
-        await comp.connect(otherAccount).delegate(otherAccount);
-
-        await governorBravo._setWhitelistGuardian(owner);
-        await expect(governorBravo.cancel(proposalId)).to.be.revertedWith(
-          "GovernorBravo::cancel: whitelisted proposer"
-        );
-      });
     });
   });
 
@@ -1235,6 +1224,56 @@ describe("Governor Bravo", function () {
     });
   });
 
+  describe("Proposal Guardian", function () {
+    it("Set Proposal Guardian: admin only", async function () {
+      const { governorBravo, otherAccount } = await loadFixture(deployFixtures);
+      await expect(
+        governorBravo.connect(otherAccount)._setProposalGuardian({
+          account: otherAccount,
+          expiration: (await time.latest()) + 1000,
+        })
+      ).to.be.revertedWith("GovernorBravo::_setProposalGuardian: admin only");
+      expect(await governorBravo.proposalGuardian()).to.deep.equal([
+        ethers.ZeroAddress,
+        0,
+      ]);
+    });
+
+    it("Set Proposal Guardian: happy path", async function () {
+      const { governorBravo, otherAccount } = await loadFixture(deployFixtures);
+      const expiryTimestamp = (await time.latest()) + 1000;
+      await expect(
+        governorBravo._setProposalGuardian({
+          account: otherAccount,
+          expiration: expiryTimestamp,
+        })
+      )
+        .to.emit(governorBravo, "ProposalGuardianSet")
+        .withArgs(ethers.ZeroAddress, 0, otherAccount.address, expiryTimestamp);
+      expect(await governorBravo.proposalGuardian()).to.deep.equal([
+        otherAccount.address,
+        expiryTimestamp,
+      ]);
+    });
+
+    it("Set Proposal Guardian: proposal guardian abilities removed after expiration", async function () {
+      const { governorBravo, otherAccount } = await loadFixture(deployFixtures);
+
+      const proposalId = await proposeAndPass(governorBravo);
+
+      const expiryTimestamp = (await time.latest()) + 1000;
+      await governorBravo._setProposalGuardian({
+        account: otherAccount,
+        expiration: expiryTimestamp,
+      });
+
+      await time.increase(1001);
+      await expect(
+        governorBravo.connect(otherAccount).cancel(proposalId)
+      ).to.be.revertedWith("GovernorBravo::cancel: proposer above threshold");
+    });
+  });
+
   describe("Whitelist", function () {
     it("Set whitelist guardian: admin only", async function () {
       const { governorBravo, otherAccount } = await loadFixture(deployFixtures);