Skip to content
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

Use NoncesKeyed in Governor for Voting by Sig #5565

Open
arr00 opened this issue Mar 7, 2025 · 2 comments · May be fixed by #5574
Open

Use NoncesKeyed in Governor for Voting by Sig #5565

arr00 opened this issue Mar 7, 2025 · 2 comments · May be fixed by #5574

Comments

@arr00
Copy link
Contributor

arr00 commented Mar 7, 2025

🧐 Motivation

Using a nonce in addition to proposal id in governor makes it hard to have a functional vote by sig service. This is because votes by sig must have a sequential nonce with no gaps or repeats--if a user decides to take one of their votes by sig on-chain, all other pending votes they have are invalidated. This isn't the desired flow and it isn't necessary.

📝 Details

_hashTypedDataV4(keccak256(abi.encode(BALLOT_TYPEHASH, proposalId, support, voter, _useNonce(voter)))),

This line asserts that votes by sig have sequential nonces which isn't actually necessary. The OZ governor introduces voting by nonce as it is possible that a user would vote multiple times for a single proposal id (so the proposal id can't be the only source of entropy).

It is, however, useful to ensure that a sequence binds together votes on a specific proposal. We should use NoncesKeyed with the key being the proposal id and allow for the 0 key to continue working (no breaking changes).

@arr00 arr00 changed the title Governor Should Use NoncesKeyed for Voting by Sig Use NoncesKeyed in Governor for Voting by Sig Mar 7, 2025
@tushar994
Copy link

I would like to implement this, This is the way I think it should be implemented, based on the information in the issue.

In order to maintain backward compatibility, we will leave the castVoteBySig and the castVoteWithReasonAndParamsBySig function untouched.

We will create two new functions that use Nonces keyed by the proposal id.

  • castVoteBySigWithNonceKeyedByProposalId
  • castVoteWithReasonAndParamsBySigWithNonceKeyedByProposalId

we'll create two new constants

bytes32 public constant BALLOT_TYPEHASH_NONCE_KEYED_BY_PROPOSAL_ID =
        keccak256("Ballot(uint256 proposalId,uint8 support,address voter,uint256 nonceKeyedByProposalId)");
bytes32 public constant EXTENDED_BALLOT_TYPEHASH_NONCE_KEYED_BY_PROPOSAL_ID =
        keccak256(
            "ExtendedBallot(uint256 proposalId,uint8 support,address voter,uint256 nonceKeyedByProposalId,string reason,bytes params)"
        );

we'll use these as prefixes to the signature data for our two new functions.

we'll replace Nonces with NoncesKeyed in the contract, and use 0 keys in the legacy functions, and the proposal id as the key in the new functions.

please let me know if that sounds good, ill make a PR if it does. thank you!!

@arr00
Copy link
Contributor Author

arr00 commented Mar 11, 2025

Thanks for your interest here @tushar994! I only saw your comment after creating a PR myself for this issue--feel free to review it (#5574).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants