feat: expose guardian expiration (#17)

compound-finance · Aug 8, 2024 · 84dff8d · 84dff8d
1 parent bd43733
commit 84dff8d
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 66 deletions.
diff --git a/contracts/GovernorBravoDelegate.sol b/contracts/GovernorBravoDelegate.sol
@@ -382,10 +382,14 @@ contract GovernorBravoDelegate is
 
         Proposal storage proposal = proposals[proposalId];
 
+        // 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()
-        ) {
+        if (msg.sender != proposal.proposer && msg.sender != proposalGuardian) {
             require(
                 (comp.getPriorVotes(proposal.proposer, block.number - 1) <
                     proposalThreshold),
@@ -663,17 +667,6 @@ contract GovernorBravoDelegate is
         return (whitelistAccountExpirations[account] > block.timestamp);
     }
 
-    /**
-     * @notice View function which returns the current proposal guardian
-     * @return The address of the current proposal guardian. If the proposal guardian expiry has passed, address(0) is returned.
-     */
-    function proposalGuardian() public view returns (address) {
-        return
-            _proposalGuardianExpiry >= block.timestamp
-                ? _proposalGuardian
-                : address(0);
-    }
-
     /**
      * @notice Admin function for setting the voting delay
      * @param newVotingDelay new voting delay, in blocks
@@ -769,25 +762,25 @@ contract GovernorBravoDelegate is
     }
 
     /**
-     * @notice Admin function for setting the proposalGuardian. ProposalGuardian can cancel proposals
-     * @param account Account to set proposalGuardian to (0x0 to remove proposalGuardian)
-     * @param expiry Timestamp at which the set proposal guardian will expire
+     * @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(address account, uint96 expiry) external {
+    function _setProposalGuardian(
+        ProposalGuardian memory newProposalGuardian
+    ) external {
         require(
             msg.sender == admin,
             "GovernorBravo::_setProposalGuardian: admin only"
         );
 
         emit ProposalGuardianSet(
-            _proposalGuardian,
-            _proposalGuardianExpiry,
-            account,
-            expiry
+            proposalGuardian.account,
+            proposalGuardian.expiration,
+            newProposalGuardian.account,
+            newProposalGuardian.expiration
         );
 
-        _proposalGuardian = account;
-        _proposalGuardianExpiry = expiry;
+        proposalGuardian = newProposalGuardian;
     }
 
     /**

diff --git a/contracts/GovernorBravoInterfaces.sol b/contracts/GovernorBravoInterfaces.sol
@@ -182,11 +182,16 @@ contract GovernorBravoDelegateStorageV2 is GovernorBravoDelegateStorageV1 {
 }
 
 contract GovernorBravoDelegateStorageV3 is GovernorBravoDelegateStorageV2 {
-    /// @notice Address which has the ability to cancel proposals
-    address internal _proposalGuardian;
+    /// @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 Timestamp at which the proposalGuardian stored in the `_proposalGuardian` variable is set to expire
-    uint96 internal _proposalGuardianExpiry;
+    /// @notice Account which has the ability to cancel proposals. This privilege expires at the given expiration timestamp.
+    ProposalGuardian public proposalGuardian;
 }
 
 interface TimelockInterface {

diff --git a/test/ForkTestSimulateUpgrade.ts b/test/ForkTestSimulateUpgrade.ts
@@ -187,24 +187,19 @@ describe("ForkTestSimulateUpgrade", function () {
     );
     const [signer] = await ethers.getSigners();
     const setProposalGuardianSelector = ethers
-      .id("_setProposalGuardian(address,uint96)")
+      .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,
-            (await time.latest()) +
-              6 * 30 * 24 * 60 * 60 /* Expire in 6 months */,
-          ]
-        )
+        .encode(["address", "uint96"], [signer.address, expirationTimestamp])
         .slice(2);
 
-    expect(await governorBravoDelegator.proposalGuardian()).to.equal(
-      ethers.ZeroAddress
-    );
+    expect(await governorBravoDelegator.proposalGuardian()).to.deep.equal([
+      ethers.ZeroAddress,
+      0,
+    ]);
 
     await proposeAndExecute(
       governorBravoDelegator.connect(proposingSigner),
@@ -214,8 +209,9 @@ describe("ForkTestSimulateUpgrade", function () {
       "Set Proposal Guardian"
     );
 
-    expect(await governorBravoDelegator.proposalGuardian()).to.equal(
-      signer.address
-    );
+    expect(await governorBravoDelegator.proposalGuardian()).to.deep.equal([
+      signer.address,
+      expirationTimestamp,
+    ]);
   });
 });
diff --git a/test/GovernorBravo.ts b/test/GovernorBravo.ts
@@ -687,10 +687,10 @@ describe("Governor Bravo", function () {
       const { governorBravo, otherAccount } = await loadFixture(deployFixtures);
       const proposalId = await proposeAndPass(governorBravo);
 
-      await governorBravo._setProposalGuardian(
-        otherAccount,
-        (await time.latest()) + 1000
-      );
+      await governorBravo._setProposalGuardian({
+        account: otherAccount,
+        expiration: (await time.latest()) + 1000,
+      });
       await governorBravo.connect(otherAccount).cancel(proposalId);
     });
 
@@ -1228,42 +1228,49 @@ describe("Governor Bravo", function () {
     it("Set Proposal Guardian: admin only", async function () {
       const { governorBravo, otherAccount } = await loadFixture(deployFixtures);
       await expect(
-        governorBravo
-          .connect(otherAccount)
-          ._setProposalGuardian(otherAccount, (await time.latest()) + 1000)
+        governorBravo.connect(otherAccount)._setProposalGuardian({
+          account: otherAccount,
+          expiration: (await time.latest()) + 1000,
+        })
       ).to.be.revertedWith("GovernorBravo::_setProposalGuardian: admin only");
-      expect(await governorBravo.proposalGuardian()).to.equal(
-        ethers.ZeroAddress
-      );
+      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(otherAccount, expiryTimestamp)
+        governorBravo._setProposalGuardian({
+          account: otherAccount,
+          expiration: expiryTimestamp,
+        })
       )
         .to.emit(governorBravo, "ProposalGuardianSet")
         .withArgs(ethers.ZeroAddress, 0, otherAccount.address, expiryTimestamp);
-      expect(await governorBravo.proposalGuardian()).to.equal(
-        otherAccount.address
-      );
+      expect(await governorBravo.proposalGuardian()).to.deep.equal([
+        otherAccount.address,
+        expiryTimestamp,
+      ]);
     });
 
-    it("Set Proposal Guardian: removed after expiration", async function () {
+    it("Set Proposal Guardian: proposal guardian abilities removed after expiration", async function () {
       const { governorBravo, otherAccount } = await loadFixture(deployFixtures);
-      const expiryTimestamp = (await time.latest()) + 1000;
 
-      await governorBravo._setProposalGuardian(otherAccount, expiryTimestamp);
+      const proposalId = await proposeAndPass(governorBravo);
 
-      expect(await governorBravo.proposalGuardian()).to.equal(
-        otherAccount.address
-      );
+      const expiryTimestamp = (await time.latest()) + 1000;
+      await governorBravo._setProposalGuardian({
+        account: otherAccount,
+        expiration: expiryTimestamp,
+      });
 
       await time.increase(1001);
-      expect(await governorBravo.proposalGuardian()).to.equal(
-        ethers.ZeroAddress
-      );
+      await expect(
+        governorBravo.connect(otherAccount).cancel(proposalId)
+      ).to.be.revertedWith("GovernorBravo::cancel: proposer above threshold");
     });
   });