Skip to content

Commit

Permalink
feat: expose guardian expiration (#17)
Browse files Browse the repository at this point in the history
  • Loading branch information
arr00 authored Aug 8, 2024
1 parent bd43733 commit 84dff8d
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 66 deletions.
41 changes: 17 additions & 24 deletions contracts/GovernorBravoDelegate.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

/**
Expand Down
13 changes: 9 additions & 4 deletions contracts/GovernorBravoInterfaces.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
26 changes: 11 additions & 15 deletions test/ForkTestSimulateUpgrade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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,
]);
});
});
53 changes: 30 additions & 23 deletions test/GovernorBravo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand Down Expand Up @@ -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");
});
});

Expand Down

0 comments on commit 84dff8d

Please sign in to comment.