diff --git a/contracts/governance/extensions/GovernorProposalGuardian.sol b/contracts/governance/extensions/GovernorProposalGuardian.sol new file mode 100644 index 00000000000..d74f64c13cf --- /dev/null +++ b/contracts/governance/extensions/GovernorProposalGuardian.sol @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {Governor} from "../Governor.sol"; + +/** + * @dev Extension of {Governor} which adds a proposal guardian that can cancel proposals at any stage of their lifecycle. + * + * Note: if the proposal guardian is not configured, then proposers take this role for their proposals. + */ +abstract contract GovernorProposalGuardian is Governor { + address private _proposalGuardian; + + event ProposalGuardianSet(address oldProposalGuardian, address newProposalGuardian); + + /** + * @dev Override {IGovernor-cancel} that implements the extended cancellation logic. + * * proposal guardian can cancel any proposal at any point in the lifecycle. + * * if no proposal guardian is set, proposer can cancel their proposals at any point in the lifecycle. + * * if the proposal guardian is set, proposer keep their ability to cancel during the pending stage (default behavior). + */ + function cancel( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) public virtual override returns (uint256) { + address caller = _msgSender(); + address authority = proposalGuardian(); + + if (authority == address(0)) { + // if there is no proposal guardian + // ... only the proposer can cancel + // ... no restriction on when the proposer can cancel + uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash); + address proposer = proposalProposer(proposalId); + if (caller != proposer) revert GovernorOnlyProposer(caller); + return _cancel(targets, values, calldatas, descriptionHash); + } else if (authority == caller) { + // if there is a proposal guardian, and the caller is the proposal guardian + // ... just cancel + return _cancel(targets, values, calldatas, descriptionHash); + } else { + // if there is a proposal guardian, and the caller is not the proposal guardian + // ... apply default behavior + return super.cancel(targets, values, calldatas, descriptionHash); + } + } + + /** + * @dev Getter that returns the address of the proposal guardian. + */ + function proposalGuardian() public view virtual returns (address) { + return _proposalGuardian; + } + + /** + * @dev Update the proposal guardian's address. This operation can only be performed through a governance proposal. + * + * Emits a {ProposalGuardianSet} event. + */ + function setProposalGuardian(address newProposalGuardian) public virtual onlyGovernance { + _setProposalGuardian(newProposalGuardian); + } + + /** + * @dev Internal setter for the proposal guardian. + * + * Emits a {ProposalGuardianSet} event. + */ + function _setProposalGuardian(address newProposalGuardian) internal virtual { + emit ProposalGuardianSet(_proposalGuardian, newProposalGuardian); + _proposalGuardian = newProposalGuardian; + } +} diff --git a/contracts/governance/extensions/GovernorSecurityCouncil.sol b/contracts/governance/extensions/GovernorSecurityCouncil.sol deleted file mode 100644 index c6fe639c416..00000000000 --- a/contracts/governance/extensions/GovernorSecurityCouncil.sol +++ /dev/null @@ -1,76 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.20; - -import {Governor} from "../Governor.sol"; - -/** - * @dev Extension of {Governor} for adds a security council that can cancel proposals at any stage of their lifecycle. - * - * Note: if the council is not configure, then proposers take over the - */ -abstract contract GovernorSecurityCouncil is Governor { - address private _council; - - event CouncilSet(address oldCouncil, address newCouncil); - - /** - * @dev Override {IGovernor-cancel} that implements the extended cancellation logic. - * * council can cancel any proposal at any point in the lifecycle. - * * if no council is set, proposer can cancel their proposals at any point in the lifecycle. - * * if the council is set, proposer keep their ability to cancel during the pending stage (default behavior). - */ - function cancel( - address[] memory targets, - uint256[] memory values, - bytes[] memory calldatas, - bytes32 descriptionHash - ) public virtual override returns (uint256) { - address caller = _msgSender(); - address authority = council(); - - if (authority == address(0)) { - // if there is no council - // ... only the proposer can cancel - // ... no restriction on when the proposer can cancel - uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash); - address proposer = proposalProposer(proposalId); - if (caller != proposer) revert GovernorOnlyProposer(caller); - return _cancel(targets, values, calldatas, descriptionHash); - } else if (authority == caller) { - // if there is a council, and the caller is the council - // ... just cancel - return _cancel(targets, values, calldatas, descriptionHash); - } else { - // if there is a council, and the caller is not the council - // ... apply default behavior - return super.cancel(targets, values, calldatas, descriptionHash); - } - } - - /** - * @dev Getter that returns the address of the council - */ - function council() public view virtual returns (address) { - return _council; - } - - /** - * @dev Update the council's address. This operation can only be performed through a governance proposal. - * - * Emits a {CouncilSet} event. - */ - function setCouncil(address newCouncil) public virtual onlyGovernance { - _setCouncil(newCouncil); - } - - /** - * @dev Internal setter for the council. - * - * Emits a {CouncilSet} event. - */ - function _setCouncil(address newCouncil) internal virtual { - emit CouncilSet(_council, newCouncil); - _council = newCouncil; - } -} diff --git a/contracts/mocks/governance/GovernorSecurityCouncilMock.sol b/contracts/mocks/governance/GovernorProposalGuardianMock.sol similarity index 77% rename from contracts/mocks/governance/GovernorSecurityCouncilMock.sol rename to contracts/mocks/governance/GovernorProposalGuardianMock.sol index 9cb6d0151da..8252a30a21a 100644 --- a/contracts/mocks/governance/GovernorSecurityCouncilMock.sol +++ b/contracts/mocks/governance/GovernorProposalGuardianMock.sol @@ -6,13 +6,13 @@ import {Governor} from "../../governance/Governor.sol"; import {GovernorSettings} from "../../governance/extensions/GovernorSettings.sol"; import {GovernorCountingSimple} from "../../governance/extensions/GovernorCountingSimple.sol"; import {GovernorVotesQuorumFraction} from "../../governance/extensions/GovernorVotesQuorumFraction.sol"; -import {GovernorSecurityCouncil} from "../../governance/extensions/GovernorSecurityCouncil.sol"; +import {GovernorProposalGuardian} from "../../governance/extensions/GovernorProposalGuardian.sol"; -abstract contract GovernorSecurityCouncilMock is +abstract contract GovernorProposalGuardianMock is GovernorSettings, GovernorVotesQuorumFraction, GovernorCountingSimple, - GovernorSecurityCouncil + GovernorProposalGuardian { function proposalThreshold() public view override(Governor, GovernorSettings) returns (uint256) { return super.proposalThreshold(); @@ -23,7 +23,7 @@ abstract contract GovernorSecurityCouncilMock is uint256[] memory values, bytes[] memory calldatas, bytes32 descriptionHash - ) public override(Governor, GovernorSecurityCouncil) returns (uint256) { + ) public override(Governor, GovernorProposalGuardian) returns (uint256) { return super.cancel(targets, values, calldatas, descriptionHash); } } diff --git a/test/governance/extensions/GovernorSecurityCouncil.test.js b/test/governance/extensions/GovernorProposalGuardian.test.js similarity index 82% rename from test/governance/extensions/GovernorSecurityCouncil.test.js rename to test/governance/extensions/GovernorProposalGuardian.test.js index 39faa9b1786..00230970cf8 100644 --- a/test/governance/extensions/GovernorSecurityCouncil.test.js +++ b/test/governance/extensions/GovernorProposalGuardian.test.js @@ -11,7 +11,7 @@ const TOKENS = [ { Token: '$ERC20VotesTimestampMock', mode: 'timestamp' }, ]; -const name = 'Security Council Governor'; +const name = 'Proposal Guardian Governor'; const version = '1'; const tokenName = 'MockToken'; const tokenSymbol = 'MTKN'; @@ -20,14 +20,14 @@ const votingDelay = 4n; const votingPeriod = 16n; const value = ethers.parseEther('1'); -describe('GovernorSecurityCouncil', function () { +describe.only('GovernorProposalGuardian', function () { for (const { Token, mode } of TOKENS) { const fixture = async () => { const [owner, proposer, voter1, voter2, voter3, voter4, other] = await ethers.getSigners(); const receiver = await ethers.deployContract('CallReceiverMock'); const token = await ethers.deployContract(Token, [tokenName, tokenSymbol, tokenName, version]); - const mock = await ethers.deployContract('$GovernorSecurityCouncilMock', [ + const mock = await ethers.deployContract('$GovernorProposalGuardianMock', [ name, // name votingDelay, // initialVotingDelay votingPeriod, // initialVotingPeriod @@ -73,17 +73,17 @@ describe('GovernorSecurityCouncil', function () { expect(await this.mock.votingPeriod()).to.equal(votingPeriod); }); - describe('set council', function () { + describe('set proposal guardian', function () { it('from governance', async function () { const governorSigner = await ethers.getSigner(this.mock.target); - await expect(this.mock.connect(governorSigner).setCouncil(this.voter1.address)) - .to.emit(this.mock, 'CouncilSet') + await expect(this.mock.connect(governorSigner).setProposalGuardian(this.voter1.address)) + .to.emit(this.mock, 'ProposalGuardianSet') .withArgs(ethers.ZeroAddress, this.voter1.address); - expect(this.mock.council()).to.eventually.equal(this.voter1.address); + expect(this.mock.proposalGuardian()).to.eventually.equal(this.voter1.address); }); it('from non-governance', async function () { - await expect(this.mock.setCouncil(this.voter1.address)) + await expect(this.mock.setProposalGuardian(this.voter1.address)) .to.be.revertedWithCustomError(this.mock, 'GovernorOnlyExecutor') .withArgs(this.owner.address); }); @@ -91,19 +91,19 @@ describe('GovernorSecurityCouncil', function () { describe('cancel proposal during active state', function () { beforeEach(async function () { - await this.mock.$_setCouncil(this.voter1.address); + await this.mock.$_setProposalGuardian(this.voter1.address); await this.helper.connect(this.proposer).propose(); await this.helper.waitForSnapshot(1n); expect(this.mock.state(this.proposal.id)).to.eventually.equal(ProposalState.Active); }); - it('from council', async function () { + it('from proposal guardian', async function () { await expect(this.helper.connect(this.voter1).cancel()) .to.emit(this.mock, 'ProposalCanceled') .withArgs(this.proposal.id); }); - it('from proposer when council is non-zero', async function () { + it('from proposer when proposal guardian is non-zero', async function () { await expect(this.helper.connect(this.proposer).cancel()) .to.be.revertedWithCustomError(this.mock, 'GovernorUnexpectedProposalState') .withArgs( @@ -113,8 +113,8 @@ describe('GovernorSecurityCouncil', function () { ); }); - it('from proposer when council is zero', async function () { - await this.mock.$_setCouncil(ethers.ZeroAddress); + it('from proposer when proposal guardian is zero', async function () { + await this.mock.$_setProposalGuardian(ethers.ZeroAddress); await expect(this.helper.connect(this.proposer).cancel()) .to.emit(this.mock, 'ProposalCanceled') .withArgs(this.proposal.id);