-
Notifications
You must be signed in to change notification settings - Fork 12.1k
Add Governor extension GovernorNoncesKeyed
to use NoncesKeyed
for vote by sig
#5574
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
Merged
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
e6d91e4
Use `NoncesKeyed` for `Governor` votes by sig with `proposalId` as th…
arr00 da90311
remove `.only`
arr00 34f0b85
simplify `_validateVoteSignature`
arr00 4fe8605
extract into an extension
arr00 ceeefe0
inline raw digest
arr00 4a9a301
rename extension
arr00 057a3ff
update tests
arr00 ba1f08a
add docs
arr00 6da3519
only run on one token type
arr00 d5295cf
Add changeset
arr00 cb82477
Update Governor.sol
Amxx 0e1014e
Update GovernorNoncesKeyed.sol
Amxx 0220d84
Update Governor.sol
Amxx 4b4b43e
fix lint
arr00 15ca4dd
Propose different design for GovernorNoncesKeyed
ernestognw 8f15e99
Suggestions
ernestognw caee631
Apply suggestions from code review
arr00 f335900
Apply suggestions from code review
arr00 2cec0ff
Merge pull request #1 from ernestognw/chore/governor-keyed-nonces-ref…
arr00 d713c04
Merge branch 'master' into feat/governor-keyed-nonces
ernestognw f6d65e7
Nit pragma
ernestognw File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'openzeppelin-solidity': minor | ||
--- | ||
|
||
`GovernorNoncesKeyed`: Extension of `Governor` that adds support for keyed nonces when voting by sig. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
// SPDX-License-Identifier: MIT | ||
|
||
pragma solidity ^0.8.24; | ||
|
||
import {Governor} from "../Governor.sol"; | ||
import {Nonces} from "../../utils/Nonces.sol"; | ||
import {NoncesKeyed} from "../../utils/NoncesKeyed.sol"; | ||
import {SignatureChecker} from "../../utils/cryptography/SignatureChecker.sol"; | ||
|
||
/** | ||
* @dev An extension of {Governor} that extends existing nonce management to use {NoncesKeyed}, where the key is the first 192 bits of the `proposalId`. | ||
* This is useful for voting by signature while maintaining separate sequences of nonces for each proposal. | ||
* | ||
* NOTE: Traditional (un-keyed) nonces are still supported and can continue to be used as if this extension was not present. | ||
*/ | ||
abstract contract GovernorNoncesKeyed is Governor, NoncesKeyed { | ||
function _useCheckedNonce(address owner, uint256 nonce) internal virtual override(Nonces, NoncesKeyed) { | ||
super._useCheckedNonce(owner, nonce); | ||
} | ||
|
||
/** | ||
* @dev Check the signature against keyed nonce and falls back to the traditional nonce. | ||
* | ||
* NOTE: This function won't call `super._validateVoteSig` if the keyed nonce is valid. | ||
* Side effects may be skipped depending on the linearization of the function. | ||
*/ | ||
function _validateVoteSig( | ||
uint256 proposalId, | ||
uint8 support, | ||
address voter, | ||
bytes memory signature | ||
) internal virtual override returns (bool) { | ||
if ( | ||
SignatureChecker.isValidSignatureNow( | ||
voter, | ||
_hashTypedDataV4( | ||
keccak256( | ||
abi.encode(BALLOT_TYPEHASH, proposalId, support, voter, nonces(voter, uint192(proposalId))) | ||
) | ||
), | ||
signature | ||
) | ||
) { | ||
_useNonce(voter, uint192(proposalId)); | ||
return true; | ||
} else { | ||
return super._validateVoteSig(proposalId, support, voter, signature); | ||
} | ||
} | ||
|
||
/** | ||
* @dev Check the signature against keyed nonce and falls back to the traditional nonce. | ||
* | ||
* NOTE: This function won't call `super._validateExtendedVoteSig` if the keyed nonce is valid. | ||
* Side effects may be skipped depending on the linearization of the function. | ||
*/ | ||
function _validateExtendedVoteSig( | ||
uint256 proposalId, | ||
uint8 support, | ||
address voter, | ||
string memory reason, | ||
bytes memory params, | ||
bytes memory signature | ||
) internal virtual override returns (bool) { | ||
if ( | ||
SignatureChecker.isValidSignatureNow( | ||
voter, | ||
_hashTypedDataV4( | ||
keccak256( | ||
abi.encode( | ||
EXTENDED_BALLOT_TYPEHASH, | ||
proposalId, | ||
support, | ||
voter, | ||
nonces(voter, uint192(proposalId)), | ||
keccak256(bytes(reason)), | ||
keccak256(params) | ||
) | ||
) | ||
), | ||
signature | ||
) | ||
) { | ||
_useNonce(voter, uint192(proposalId)); | ||
return true; | ||
} else { | ||
return super._validateExtendedVoteSig(proposalId, support, voter, reason, params, signature); | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
// SPDX-License-Identifier: MIT | ||
|
||
pragma solidity ^0.8.24; | ||
|
||
import {Governor, Nonces} 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 {GovernorProposalGuardian} from "../../governance/extensions/GovernorProposalGuardian.sol"; | ||
import {GovernorNoncesKeyed} from "../../governance/extensions/GovernorNoncesKeyed.sol"; | ||
|
||
abstract contract GovernorNoncesKeyedMock is | ||
GovernorSettings, | ||
GovernorVotesQuorumFraction, | ||
GovernorCountingSimple, | ||
GovernorNoncesKeyed | ||
{ | ||
function proposalThreshold() public view override(Governor, GovernorSettings) returns (uint256) { | ||
return super.proposalThreshold(); | ||
} | ||
|
||
function _validateVoteSig( | ||
uint256 proposalId, | ||
uint8 support, | ||
address voter, | ||
bytes memory signature | ||
) internal virtual override(Governor, GovernorNoncesKeyed) returns (bool) { | ||
return super._validateVoteSig(proposalId, support, voter, signature); | ||
} | ||
|
||
function _validateExtendedVoteSig( | ||
uint256 proposalId, | ||
uint8 support, | ||
address voter, | ||
string memory reason, | ||
bytes memory params, | ||
bytes memory signature | ||
) internal virtual override(Governor, GovernorNoncesKeyed) returns (bool) { | ||
return super._validateExtendedVoteSig(proposalId, support, voter, reason, params, signature); | ||
} | ||
|
||
function _useCheckedNonce(address owner, uint256 nonce) internal virtual override(Nonces, GovernorNoncesKeyed) { | ||
super._useCheckedNonce(owner, nonce); | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -198,31 +198,35 @@ describe('Governor', function () { | |
}); | ||
|
||
describe('vote with signature', function () { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about adding stress test edge cases (e.g. signature replay on multiple proposals) or extend the extension to support batch signatures per user? |
||
it('votes with an EOA signature', async function () { | ||
it('votes with an EOA signature on two proposals', async function () { | ||
await this.token.connect(this.voter1).delegate(this.userEOA); | ||
|
||
const nonce = await this.mock.nonces(this.userEOA); | ||
|
||
// Run proposal | ||
await this.helper.propose(); | ||
await this.helper.waitForSnapshot(); | ||
await expect( | ||
this.helper.vote({ | ||
support: VoteType.For, | ||
voter: this.userEOA.address, | ||
nonce, | ||
signature: signBallot(this.userEOA), | ||
}), | ||
) | ||
.to.emit(this.mock, 'VoteCast') | ||
.withArgs(this.userEOA, this.proposal.id, VoteType.For, ethers.parseEther('10'), ''); | ||
|
||
await this.helper.waitForDeadline(); | ||
await this.helper.execute(); | ||
for (let i = 0; i < 2; i++) { | ||
const nonce = await this.mock.nonces(this.userEOA); | ||
|
||
// After | ||
expect(await this.mock.hasVoted(this.proposal.id, this.userEOA)).to.be.true; | ||
expect(await this.mock.nonces(this.userEOA)).to.equal(nonce + 1n); | ||
// Run proposal | ||
await this.helper.propose(); | ||
await this.helper.waitForSnapshot(); | ||
await expect( | ||
this.helper.vote({ | ||
support: VoteType.For, | ||
voter: this.userEOA.address, | ||
nonce, | ||
signature: signBallot(this.userEOA), | ||
}), | ||
) | ||
.to.emit(this.mock, 'VoteCast') | ||
.withArgs(this.userEOA, this.proposal.id, VoteType.For, ethers.parseEther('10'), ''); | ||
|
||
// After | ||
expect(await this.mock.hasVoted(this.proposal.id, this.userEOA)).to.be.true; | ||
expect(await this.mock.nonces(this.userEOA)).to.equal(nonce + 1n); | ||
|
||
// Update proposal to allow for re-propose | ||
this.helper.description += ' - updated'; | ||
} | ||
|
||
await expect(this.mock.nonces(this.userEOA)).to.eventually.equal(2n); | ||
}); | ||
|
||
it('votes with a valid EIP-1271 signature', async function () { | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.