Merge branch 'mraszyk/select_nonterminal_proposal_ids' into 'master'

fix: filter for nonfinal instead of open proposals in NNS governance proposal validation

This MR fixes the uniqueness check for some NNS proposal types (currently only SNS proposals) to consider both open and adopted proposals as these are both nonfinal states that could result in concurrent execution of two proposals of the same type.

In practice, it is unlikely that a new proposal is opened and quickly voted in while another proposal is being adopted, but still this MR makes the uniqueness check more robust against such scenarios. 

See merge request dfinity-lab/public/ic!19084

rs/nns/governance/src/governance.rs

@@ -5028,7 +5028,7 @@ impl Governance {
 
         // Must be unique.
         #[allow(unused_variables)]
-        let other_proposal_ids = self.select_open_proposal_ids(|action| {
+        let other_proposal_ids = self.select_nonfinal_proposal_ids(|action| {
             matches!(action, Action::CreateServiceNervousSystem(_))
         });
 
@@ -5047,15 +5047,27 @@ impl Governance {
         Ok(())
     }
 
-    fn select_open_proposal_ids(&self, action_predicate: impl Fn(&Action) -> bool) -> Vec<u64> {
+    fn select_nonfinal_proposal_ids(&self, action_predicate: impl Fn(&Action) -> bool) -> Vec<u64> {
         self.heap_data
             .proposals
             .values()
             .filter_map(|proposal_data| {
-                // Disregard non-Open proposals.
-                if proposal_data.status() != ProposalStatus::Open {
-                    return None;
-                }
+                // Disregard proposals that are in a final (or Unspecified) state.
+                match proposal_data.status() {
+                    ProposalStatus::Open | ProposalStatus::Adopted => (),
+                    ProposalStatus::Rejected
+                    | ProposalStatus::Executed
+                    | ProposalStatus::Failed => {
+                        return None;
+                    }
+                    ProposalStatus::Unspecified => {
+                        println!(
+                            "{}ERROR: ProposalData had Unspecified status: {:#?}",
+                            LOG_PREFIX, proposal_data
+                        );
+                        return None;
+                    }
+                };
 
                 // Unpack proposal.
                 let action = match &proposal_data.proposal {
@@ -5064,7 +5076,7 @@ impl Governance {
                         ..
                     }) => action,
 
-                    // Ignore proposals not of the same type.
+                    // Ignore proposals with no action.
                     _ => {
                         println!(
                             "{}ERROR: ProposalData had no action: {:#?}",

rs/nns/integration_tests/src/create_service_nervous_system.rs

@@ -115,7 +115,10 @@ fn test_several_proposals() {
     // Step 2.3: This unblocks more proposals from being made.
     execute_proposal(&mut state_machine, ProposalId { id: proposal_id_1 });
 
-    // Step 2.4: Finally, make a third proposal. This should be allowed.
+    // Step 2.4: Wait for proposal_1 to finish executing.
+    nns_wait_for_proposal_execution(&mut state_machine, proposal_id_1);
+
+    // Step 2.5: Finally, make a third proposal. This should now be allowed.
     let response_3 = make_proposal(&mut state_machine, 3);
     let response_3 = match response_3.command {
         Some(manage_neuron_response::Command::MakeProposal(response_3)) => response_3,
@@ -133,9 +136,6 @@ fn test_several_proposals() {
 
     // Step 3: Inspect results.
 
-    // Step 3.0: Wait for proposal_1 to finish executing.
-    nns_wait_for_proposal_execution(&mut state_machine, proposal_id_1);
-
     // Step 3.1: Inspect proposals.
 
     // There should only be two proposals of type CreateServiceNervousSystem.