fix: check for uninitialized voting modules

packages/contracts-bedrock/interfaces/governance/IProposalValidator.sol

@@ -29,6 +29,7 @@ interface IProposalValidator is ISemver {
     error ProposalValidator_ProposalIdMismatch();
     error ProposalValidator_InvalidProposer();
     error ProposalValidator_InvalidProposal();
+    error ProposalValidator_InvalidVotingModule();
 
     event ProposalSubmitted(
         bytes32 indexed proposalHash,

packages/contracts-bedrock/snapshots/abi/ProposalValidator.json

@@ -853,6 +853,11 @@
     "name": "ProposalValidator_InvalidVotingCycle",
     "type": "error"
   },
+  {
+    "inputs": [],
+    "name": "ProposalValidator_InvalidVotingModule",
+    "type": "error"
+  },
   {
     "inputs": [],
     "name": "ProposalValidator_ProposalAlreadyApproved",

packages/contracts-bedrock/snapshots/semver-lock.json

@@ -176,8 +176,8 @@
     "sourceCodeHash": "0x18f43b227decd0f2a895b8b55e23fa6a47706697c272bbf2482b3f912be446e1"
   },
   "src/governance/ProposalValidator.sol:ProposalValidator": {
-    "initCodeHash": "0xb612561f3182537796ec6bae6a15a4f6b9e63d839e051a165f6b81b3f6ce0807",
-    "sourceCodeHash": "0xbfb241a7033264d5b7097a4326a415b53514c4a4dee01430bd092fa6cbf0872b"
+    "initCodeHash": "0xd072e288107128a1b0f26f41c4b257295919777cadfc06514d2a4738fb96e1d1",
+    "sourceCodeHash": "0x725efcbb68cb4a23af492983ba22969ddaa5d472878fb8490a437eaaa88812a8"
   },
   "src/legacy/DeployerWhitelist.sol:DeployerWhitelist": {
     "initCodeHash": "0x53099379ed48b87f027d55712dbdd1da7d7099925426eb0531da9c0012e02c29",

packages/contracts-bedrock/src/governance/ProposalValidator.sol

@@ -86,6 +86,9 @@ contract ProposalValidator is OwnableUpgradeable, ReinitializableBase, ISemver {
     /// @notice Thrown when the proposal is invalid trying to move to vote.
     error ProposalValidator_InvalidProposal();
 
+    /// @notice Thrown when the voting module address is invalid (zero address).
+    error ProposalValidator_InvalidVotingModule();
+
     /*//////////////////////////////////////////////////////////////
                                  EVENTS
     //////////////////////////////////////////////////////////////*/
@@ -332,9 +335,15 @@ contract ProposalValidator is OwnableUpgradeable, ReinitializableBase, ISemver {
         bytes memory proposalVotingModuleData = abi.encode(optimisticSettings);
 
         // Get the optimistic module address from configurator
-        address votingModule = IProposalTypesConfigurator(GOVERNOR.PROPOSAL_TYPES_CONFIGURATOR()).proposalTypes(
-            proposalTypesData[_proposalType].proposalVotingModule
-        ).module;
+        IProposalTypesConfigurator.ProposalType memory proposalTypeConfig = IProposalTypesConfigurator(
+            GOVERNOR.PROPOSAL_TYPES_CONFIGURATOR()
+        ).proposalTypes(proposalTypesData[_proposalType].proposalVotingModule);
+        address votingModule = proposalTypeConfig.module;
+
+        // Validate voting module exists
+        if (bytes(proposalTypeConfig.name).length == 0) {
+            revert ProposalValidator_InvalidVotingModule();
+        }
 
         // Generate unique proposal hash
         proposalHash_ =
@@ -425,9 +434,15 @@ contract ProposalValidator is OwnableUpgradeable, ReinitializableBase, ISemver {
         bytes memory proposalVotingModuleData = abi.encode(options, settings);
 
         // Get the module address from the configurator
-        address votingModule = IProposalTypesConfigurator(GOVERNOR.PROPOSAL_TYPES_CONFIGURATOR()).proposalTypes(
-            proposalTypesData[ProposalType.CouncilMemberElections].proposalVotingModule
-        ).module;
+        IProposalTypesConfigurator.ProposalType memory proposalTypeConfig = IProposalTypesConfigurator(
+            GOVERNOR.PROPOSAL_TYPES_CONFIGURATOR()
+        ).proposalTypes(proposalTypesData[ProposalType.CouncilMemberElections].proposalVotingModule);
+        address votingModule = proposalTypeConfig.module;
+
+        // Validate voting module exists
+        if (bytes(proposalTypeConfig.name).length == 0) {
+            revert ProposalValidator_InvalidVotingModule();
+        }
 
         // Generate unique proposal hash
         proposalHash_ =
@@ -512,9 +527,15 @@ contract ProposalValidator is OwnableUpgradeable, ReinitializableBase, ISemver {
         bytes memory proposalVotingModuleData = abi.encode(options, settings);
 
         // Get the module address from the configurator
-        address votingModule = IProposalTypesConfigurator(GOVERNOR.PROPOSAL_TYPES_CONFIGURATOR()).proposalTypes(
-            proposalTypesData[_proposalType].proposalVotingModule
-        ).module;
+        IProposalTypesConfigurator.ProposalType memory proposalTypeConfig = IProposalTypesConfigurator(
+            GOVERNOR.PROPOSAL_TYPES_CONFIGURATOR()
+        ).proposalTypes(proposalTypesData[_proposalType].proposalVotingModule);
+        address votingModule = proposalTypeConfig.module;
+
+        // Validate voting module exists
+        if (bytes(proposalTypeConfig.name).length == 0) {
+            revert ProposalValidator_InvalidVotingModule();
+        }
 
         // Generate unique proposal hash
         proposalHash_ = _hashProposalWithModule(votingModule, proposalVotingModuleData, keccak256(bytes(_description)));

packages/contracts-bedrock/test/governance/ProposalValidator.t.sol

@@ -485,6 +485,36 @@ contract ProposalValidator_Init is CommonTest {
         );
     }
 
+    /// @notice Helper function to mock proposal types configurator call with changed module
+    function _mockProposalTypesConfiguratorCallWithUninitializedModule(uint8 _votingModuleId) internal {
+        address moduleAddress;
+        if (_votingModuleId == APPROVAL_VOTING_MODULE_ID) {
+            moduleAddress = approvalVotingModule;
+        } else if (_votingModuleId == OPTIMISTIC_VOTING_MODULE_ID) {
+            moduleAddress = optimisticVotingModule;
+        }
+
+        _mockAndExpect(
+            address(governor),
+            abi.encodeCall(IOptimismGovernor.PROPOSAL_TYPES_CONFIGURATOR, ()),
+            abi.encode(proposalTypesConfigurator)
+        );
+
+        _mockAndExpect(
+            address(proposalTypesConfigurator),
+            abi.encodeCall(IProposalTypesConfigurator.proposalTypes, (_votingModuleId)),
+            abi.encode(
+                IProposalTypesConfigurator.ProposalType({
+                    quorum: 0,
+                    approvalThreshold: 0,
+                    name: "",
+                    description: "",
+                    module: address(0)
+                })
+            )
+        );
+    }
+
     /// @notice Initializes the validator
     function _initializeValidator() internal virtual {
         (
@@ -943,6 +973,21 @@ contract ProposalValidator_SubmitUpgradeProposal_TestFail is ProposalValidator_I
         );
     }
 
+    function test_submitUpgradeProposal_invalidVotingModule_reverts() public {
+        uint248 againstThreshold = 5000; // 50%
+        ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType.ProtocolOrGovernorUpgrade;
+        bytes32 attestationUid = _createApprovedProposerAttestation(topDelegate_A, proposalType);
+
+        // Mock configurator to return uninitialized module
+        _mockProposalTypesConfiguratorCallWithUninitializedModule(OPTIMISTIC_VOTING_MODULE_ID);
+
+        vm.expectRevert(ProposalValidator.ProposalValidator_InvalidVotingModule.selector);
+        vm.prank(topDelegate_A);
+        validator.submitUpgradeProposal(
+            againstThreshold, proposalDescription, attestationUid, proposalType, CYCLE_NUMBER
+        );
+    }
+
     function testFuzz_submitUpgradeProposal_duplicateProposal_reverts(uint8 proposalTypeValue) public {
         // Bound proposal type to only upgrade proposals (0 = ProtocolOrGovernorUpgrade, 1 = MaintenanceUpgrade)
         proposalTypeValue = uint8(bound(proposalTypeValue, 0, 1));
@@ -1377,6 +1422,20 @@ contract ProposalValidator_SubmitCouncilMemberElectionsProposal_TestFail is Prop
             criteriaValue, optionDescriptions, proposalDescription, revocableAttestationUid, CYCLE_NUMBER
         );
     }
+
+    function test_submitCouncilMemberElectionsProposal_invalidVotingModule_reverts() public {
+        attestationUid =
+            _createApprovedProposerAttestation(topDelegate_A, ProposalValidator.ProposalType.CouncilMemberElections);
+
+        // Mock configurator to return uninitialized module
+        _mockProposalTypesConfiguratorCallWithUninitializedModule(APPROVAL_VOTING_MODULE_ID);
+
+        vm.expectRevert(ProposalValidator.ProposalValidator_InvalidVotingModule.selector);
+        vm.prank(topDelegate_A);
+        validator.submitCouncilMemberElectionsProposal(
+            criteriaValue, optionDescriptions, proposalDescription, attestationUid, CYCLE_NUMBER
+        );
+    }
 }
 
 /// @title ProposalValidator_SubmitFundingProposal_Test
@@ -1742,6 +1801,21 @@ contract ProposalValidator_SubmitFundingProposal_TestFail is ProposalValidator_I
             CYCLE_NUMBER
         );
     }
+
+    function test_submitFundingProposal_invalidVotingModule_reverts() public {
+        ProposalValidator.ProposalType proposalType = ProposalValidator.ProposalType.GovernanceFund;
+        (string[] memory descriptions, address[] memory recipients, uint256[] memory amounts) =
+            _createMinimalFundingArrays(1);
+
+        // Mock configurator to return uninitialized module
+        _mockProposalTypesConfiguratorCallWithUninitializedModule(APPROVAL_VOTING_MODULE_ID);
+
+        vm.expectRevert(ProposalValidator.ProposalValidator_InvalidVotingModule.selector);
+        vm.prank(user);
+        validator.submitFundingProposal(
+            FUNDING_CRITERIA_VALUE, descriptions, recipients, amounts, description, proposalType, CYCLE_NUMBER
+        );
+    }
 }
 
 /// @title ProposalValidator_ApproveProposal_Test