diff --git a/CHANGELOG.md b/CHANGELOG.md index 59e5a0912f7..eddab7cc487 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * `AccessControl`: mark `_setupRole(bytes32,address)` as deprecated in favor of `_grantRole(bytes32,address)`. ([#2568](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2568)) * `EIP712`: cache `address(this)` to immutable storage to avoid potential issues if a vanilla contract is used in a delegatecall context. ([#2852](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2852)) * Add internal `_setApprovalForAll` to `ERC721` and `ERC1155`. ([#2834](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2834)) + * `Governor`: shift vote start and end by one block to better match Compound's GovernorBravo and prevent voting at the Governor level if the voting snapshot is not ready. ([#2892](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2892)) ## 4.3.2 (2021-09-14) diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index 741e02f7d98..f1411ffa220 100644 --- a/contracts/governance/Governor.sol +++ b/contracts/governance/Governor.sol @@ -115,9 +115,9 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor { return ProposalState.Executed; } else if (proposal.canceled) { return ProposalState.Canceled; - } else if (proposal.voteStart.isPending()) { + } else if (proposal.voteStart.getDeadline() >= block.number) { return ProposalState.Pending; - } else if (proposal.voteEnd.isPending()) { + } else if (proposal.voteEnd.getDeadline() >= block.number) { return ProposalState.Active; } else if (proposal.voteEnd.isExpired()) { return diff --git a/contracts/governance/IGovernor.sol b/contracts/governance/IGovernor.sol index ee5180dfc2d..90f4549395b 100644 --- a/contracts/governance/IGovernor.sol +++ b/contracts/governance/IGovernor.sol @@ -103,28 +103,31 @@ abstract contract IGovernor is IERC165 { /** * @notice module:core - * @dev block number used to retrieve user's votes and quorum. + * @dev Block number used to retrieve user's votes and quorum. As per Compound's Comp and OpenZeppelin's + * ERC20Votes, the snapshot is performed at the end of this block. Hence, voting for this proposal starts at the + * beginning of the following block. */ function proposalSnapshot(uint256 proposalId) public view virtual returns (uint256); /** * @notice module:core - * @dev timestamp at which votes close. + * @dev Block number at which votes close. Votes close at the end of this block, so it is possible to cast a vote + * during this block. */ function proposalDeadline(uint256 proposalId) public view virtual returns (uint256); /** * @notice module:user-config - * @dev delay, in number of block, between the proposal is created and the vote starts. This can be increassed to + * @dev Delay, in number of block, between the proposal is created and the vote starts. This can be increassed to * leave time for users to buy voting power, of delegate it, before the voting of a proposal starts. */ function votingDelay() public view virtual returns (uint256); /** * @notice module:user-config - * @dev delay, in number of blocks, between the vote start and vote ends. + * @dev Delay, in number of blocks, between the vote start and vote ends. * - * Note: the {votingDelay} can delay the start of the vote. This must be considered when setting the voting + * NOTE: The {votingDelay} can delay the start of the vote. This must be considered when setting the voting * duration compared to the voting delay. */ function votingPeriod() public view virtual returns (uint256); diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js index ed58eea571d..da3e0f1414f 100644 --- a/test/governance/Governor.test.js +++ b/test/governance/Governor.test.js @@ -559,6 +559,10 @@ contract('Governor', function (accounts) { await time.advanceBlockTo(this.snapshot); + expect(await this.mock.state(this.id)).to.be.bignumber.equal(Enums.ProposalState.Pending); + + await time.advanceBlock(); + expect(await this.mock.state(this.id)).to.be.bignumber.equal(Enums.ProposalState.Active); }); runGovernorWorkflow(); diff --git a/test/governance/GovernorWorkflow.behavior.js b/test/governance/GovernorWorkflow.behavior.js index 433a4e6a2fb..70319cd44d3 100644 --- a/test/governance/GovernorWorkflow.behavior.js +++ b/test/governance/GovernorWorkflow.behavior.js @@ -58,7 +58,7 @@ function runGovernorWorkflow () { tryGet(this.settings, 'steps.propose.error') === undefined && tryGet(this.settings, 'steps.propose.noadvance') !== true ) { - await time.advanceBlockTo(this.snapshot); + await time.advanceBlockTo(this.snapshot.addn(1)); } } @@ -92,7 +92,7 @@ function runGovernorWorkflow () { // fast forward if (tryGet(this.settings, 'steps.wait.enable') !== false) { - await time.advanceBlockTo(this.deadline); + await time.advanceBlockTo(this.deadline.addn(1)); } // queue