Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: proposal guardian #16

Merged
merged 8 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ cache
artifacts
yarn-error.log
.DS_Store
yarn.lock
7 changes: 6 additions & 1 deletion .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
11 changes: 11 additions & 0 deletions .prettierrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"plugins": ["prettier-plugin-solidity"],
"overrides": [
{
"files": "*.sol",
"options": {
"bracketSpacing": true
}
}
]
}
61 changes: 45 additions & 16 deletions contracts/GovernorBravoDelegate.sol
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 {
Expand All @@ -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"
);
}
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
24 changes: 20 additions & 4 deletions contracts/GovernorBravoInterfaces.sol
Original file line number Diff line number Diff line change
@@ -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);
Expand Down Expand Up @@ -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 {
Expand All @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
40 changes: 39 additions & 1 deletion test/ForkTestSimulateUpgrade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
reset,
impersonateAccount,
loadFixture,
time,
} from "@nomicfoundation/hardhat-network-helpers";

describe("ForkTestSimulateUpgrade", function () {
Expand All @@ -30,7 +31,7 @@ describe("ForkTestSimulateUpgrade", function () {
"0xc0Da02939E1441F497fd74F78cE7Decb17B66529"
);
const proposingSigner = await ethers.getSigner(
"0x2775b1c75658Be0F640272CCb8c72ac986009e38"
"0xF977814e90dA44bFA03b6295A0616a897441aceC"
);
await hardhat.network.provider.send("hardhat_setBalance", [
proposingSigner.address,
Expand Down Expand Up @@ -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 () {
Expand Down Expand Up @@ -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,
]);
});
});
83 changes: 61 additions & 22 deletions test/GovernorBravo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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"
);
});
});
});

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