Pack Governor's ProposalCore into a single slot #4268
Conversation
🦋 Changeset detectedLatest commit: 802d4d8 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 |
contracts/governance/Governor.sol
Outdated
| @@ -161,7 +156,8 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive | |||
| * @dev See {IGovernor-state}. | |||
| */ | |||
| function state(uint256 proposalId) public view virtual override returns (ProposalState) { | |||
| ProposalCore storage proposal = _proposals[proposalId]; | |||
| // ProposalCore is just one slot | |||
There was a problem hiding this comment.
What's the intention with this comment? Is it that we're reading it into memory because it's one slot and that will be more efficient? If so, let's write that explicitly.
There was a problem hiding this comment.
Yes, that is the intention.
| voteStart: SafeCast.toUint48(snapshot), | ||
| voteDuration: SafeCast.toUint32(duration), |
There was a problem hiding this comment.
Are we documenting these bounds anywhere?
There was a problem hiding this comment.
Documentation added in IGovernor (in the natspec for votingDelay and votingPeriod)
|
Let's change GovernorSettings in here as well. |
Co-authored-by: Ernesto García <ernestognw@gmail.com>
|
LGTM except for the comments above. |
Co-authored-by: Francisco <fg@frang.io>
Co-authored-by: Ernesto García <ernestognw@gmail.com> Co-authored-by: Francisco <fg@frang.io>
Fixes #3208
Fixes LIB-803
Note: we could merge
canceledandexecutedinto a single byte using an enum. That would increase the space for duration touint40.This is purely internal, no interface change:
PR Checklist
npx changeset add)