Skip to content

Conversation

@0xChin
Copy link
Member

@0xChin 0xChin commented Jul 24, 2025

No description provided.

@0xChin 0xChin requested a review from 0xOneTony July 24, 2025 14:56
@0xChin 0xChin self-assigned this Jul 24, 2025
@0xOneTony 0xOneTony mentioned this pull request Jul 24, 2025
//////////////////////////////////////////////////////////////*/

/// @notice The Optimism Governor contract that will handle the voting phase.
IOptimismGovernor public immutable GOVERNOR;
Copy link
Member

Choose a reason for hiding this comment

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

Should we also do the same for the governor and move to initialize?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not really sure, it could somehow make sense to change the schema uid if the resolver changes by any chance
but I wonder in which context another governor would be deployed given that it is a proxy, and in case a new one is deployed, how compatible would it be with the existing validator

@0xChin 0xChin changed the base branch from chore/pp-fixed-version to sc-feat/permissionless-proposals July 24, 2025 15:07
@0xChin 0xChin changed the base branch from sc-feat/permissionless-proposals to fix/ir-findings July 24, 2025 15:20
"slot": "104",
"type": "mapping(bytes32 => struct ProposalValidator.ProposalData)"
"slot": "106",
"type": "mapping(uint256 => struct ProposalValidator.ProposalData)"
Copy link

Choose a reason for hiding this comment

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

Bug: Storage Corruption in Contract Upgrade

Critical storage layout corruption in the ProposalValidator contract. New state variables approvedProposerAttestationSchemaUid and topDelegatesAttestationSchemaUid were introduced as stored variables (previously immutable), inserting at slots 101 and 102. This shifts existing storage variables: proposalDistributionThreshold (101->103), votingCycles (102->104), proposalTypesData (103->105), and _proposals (104->106). Furthermore, the _proposals mapping's key type changed from bytes32 to uint256. These breaking changes cause data corruption and render existing state (including all prior proposals) inaccessible for deployed proxies, making the contract unusable after upgrade.

Locations (3)

Fix in CursorFix in Web

@0xChin 0xChin merged commit 2499ac1 into fix/ir-findings Jul 24, 2025
3 checks passed
0xOneTony pushed a commit that referenced this pull request Jul 29, 2025
* chore: use fixed variable for contract version

* refactor: rename proposalHash to proposalId for governor consistency

* chore: move attestation schemas uids to initialize function

* chore: specify target modules in submit functions

* refactor: proper naming for modules settings

* fix: pre-pr
0xOneTony pushed a commit that referenced this pull request Jul 29, 2025
* chore: use fixed variable for contract version

* refactor: rename proposalHash to proposalId for governor consistency

* chore: move attestation schemas uids to initialize function

* chore: specify target modules in submit functions

* refactor: proper naming for modules settings

* fix: pre-pr
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