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

feat: add support for proposal types #2

Merged
merged 4 commits into from
Dec 20, 2023

Conversation

0xRaccoon
Copy link
Member

@0xRaccoon 0xRaccoon commented Dec 15, 2023

@@ -32,6 +39,7 @@ abstract contract WonderGovernor is Context, ERC165, EIP712, Nonces, IGovernor,
keccak256('ExtendedBallot(uint256 proposalId,uint8 support,address voter,uint256 nonce,string reason,bytes params)');

struct ProposalCore {
uint8 proposalType;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

beauty <3
is there any difference between placing this first or last?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question; since there is no other uint8 data in the struct, I guess that the members' order won't significantly impact packing efficiency. We should put it in the best logical order for the best readability. What do you think will be a more readable placing proposalType before or after the proposer?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tbh I have no idea. Is there a standard for this? it could be a question for the web3-devs channel

*/
function getVotesWithParams(
address account,
uint8 proposalType,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is proposalType necessary to get the votes?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This one, in its implementation, should query the token to get the votes by proposalType, which data will be in the Votes contract in the
mapping(address delegatee => mapping(uint8 proposalType => Checkpoints.Trace208)) private _delegateCheckpoints; member

0xParticle
0xParticle previously approved these changes Dec 15, 2023
Copy link
Collaborator

@0xParticle 0xParticle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks solid. Added a few minor comments.

@0xRaccoon 0xRaccoon merged commit baa40aa into dev Dec 20, 2023
6 checks passed
@0xRaccoon 0xRaccoon deleted the feat/add-support-for-proposal-types branch December 20, 2023 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants