Add timestamp based governor with EIP-6372 and EIP-5805#3934
Add timestamp based governor with EIP-6372 and EIP-5805#3934Amxx merged 70 commits intoOpenZeppelin:masterfrom
Conversation
f0ccbfb to
bcedb7e
Compare
| startBlock: web3.utils.toBN(await clockFromReceipt[mode](txPropose.receipt)).add(votingDelay), | ||
| endBlock: web3.utils | ||
| .toBN(await clockFromReceipt[mode](txPropose.receipt)) |
There was a problem hiding this comment.
It looks like we always convert the result of clockFromReceipt[mode] and clock[mode] into BN. Can those helpers return a BN directly?
There was a problem hiding this comment.
There are many places (25 occurences) where we do:
const timepoint = await clockFromReceipt[mode](receipt);
// [...]
expect(await this.token.getPastVotes(holder, timepoint - 1)).to.be.bignumber.equal('0');we would need to to timepoint.subn(1) which affects readability.
There was a problem hiding this comment.
this is to compare to 9 occurences of
.toBN(await clockFromReceipt[mode](...))|
FYI I removed this PR from the milestone because the corresponding issue is already in it. |
🦋 Changeset detectedLatest commit: 8397ce7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
6d243e8 to
21588f0
Compare
|
The storage layout errors we are seeing, which say "Layout could have changed...", are because the compiler is not producing storage layout information so the Upgrades plugin conservatively reports that there could be some error. I've added the layout in the config in 3b591a4. After that finishes producing the new layout artifact, we should merge and run it in this PR. |
|
The diff test coverage is not looking super good. See on Codecov. It seems to be in large parte because |
Co-authored-by: Francisco <[email protected]>
done |
frangio
left a comment
There was a problem hiding this comment.
This is looking good to me.
Documentation still needs work but as discussed elsewhere we can do this in a follow up PR before release.
Co-authored-by: Francisco <[email protected]>
Conflicts: test/governance/Governor.test.js test/governance/extensions/GovernorTimelockCompound.test.js test/governance/extensions/GovernorTimelockControl.test.js test/governance/extensions/GovernorWithParams.test.js test/governance/utils/Votes.behavior.js test/token/ERC20/extensions/ERC20Votes.test.js test/token/ERC20/extensions/ERC20VotesComp.test.js
|
When is this getting released? |
|
@jonwalch in the next minor (4.9) in a few weeks. |
|
Next week. Sorry for the delays, we've taken a lot of time to test and audit. |
|
All good. Thanks for the update! |
Fixes #3081
WIP
This includes a lot of retypes that should be safe, but that must be double-checked, and that the plugin needs to understand.
PR Checklist
clock()Votes&ERC20Votesto use the value returned byclock()Governorto use aclock()functionclock()value from the token contract