Skip to content

Commit

Permalink
Improve GovernorTimelockControl.state() to detect direct cancel (#2977)
Browse files Browse the repository at this point in the history
  • Loading branch information
Amxx authored Nov 18, 2021
1 parent 6e5bf05 commit a57e638
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 2 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## Unreleased

* `GovernorTimelockControl`: improve the `state()` function to have it reflect cases where a proposal has been canceled directly on the timelock. ([#2977](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2977))

## Unreleased

* `Ownable`: add an internal `_transferOwnership(address)`. ([#2568](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2568))
Expand Down
4 changes: 3 additions & 1 deletion contracts/governance/extensions/GovernorTimelockControl.sol
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,10 @@ abstract contract GovernorTimelockControl is IGovernorTimelock, Governor {
return status;
} else if (_timelock.isOperationDone(queueid)) {
return ProposalState.Executed;
} else {
} else if (_timelock.isOperationPending(queueid)) {
return ProposalState.Queued;
} else {
return ProposalState.Canceled;
}
}

Expand Down
42 changes: 41 additions & 1 deletion test/governance/extensions/GovernorTimelockControl.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const Governor = artifacts.require('GovernorTimelockControlMock');
const CallReceiver = artifacts.require('CallReceiverMock');

contract('GovernorTimelockControl', function (accounts) {
const [ voter ] = accounts;
const [ admin, voter ] = accounts;

const name = 'OZ-Governor';
// const version = '1';
Expand All @@ -33,6 +33,7 @@ contract('GovernorTimelockControl', function (accounts) {
this.receiver = await CallReceiver.new();
// normal setup: governor is proposer, everyone is executor, timelock is its own admin
await this.timelock.grantRole(await this.timelock.PROPOSER_ROLE(), this.mock.address);
await this.timelock.grantRole(await this.timelock.PROPOSER_ROLE(), admin);
await this.timelock.grantRole(await this.timelock.EXECUTOR_ROLE(), constants.ZERO_ADDRESS);
await this.timelock.revokeRole(await this.timelock.TIMELOCK_ADMIN_ROLE(), deployer);
await this.token.mint(voter, tokenSupply);
Expand Down Expand Up @@ -320,6 +321,45 @@ contract('GovernorTimelockControl', function (accounts) {
runGovernorWorkflow();
});

describe('cancel on timelock is forwarded in state', function () {
beforeEach(async function () {
this.settings = {
proposal: [
[ this.receiver.address ],
[ web3.utils.toWei('0') ],
[ this.receiver.contract.methods.mockFunction().encodeABI() ],
'<proposal description>',
],
voters: [
{ voter: voter, support: Enums.VoteType.For },
],
steps: {
queue: { delay: 3600 },
execute: { enable: false },
},
};
});
afterEach(async function () {
const timelockid = await this.timelock.hashOperationBatch(
...this.settings.proposal.slice(0, 3),
'0x0',
this.descriptionHash,
);

expect(await this.mock.state(this.id)).to.be.bignumber.equal(Enums.ProposalState.Queued);

const receipt = await this.timelock.cancel(timelockid, { from: admin });
expectEvent(
receipt,
'Cancelled',
{ id: timelockid },
);

expect(await this.mock.state(this.id)).to.be.bignumber.equal(Enums.ProposalState.Canceled);
});
runGovernorWorkflow();
});

describe('updateTimelock', function () {
beforeEach(async function () {
this.newTimelock = await Timelock.new(3600, [], []);
Expand Down

0 comments on commit a57e638

Please sign in to comment.