More solid checkpoint tests (MystenLabs#2492)

crates/sui-core/src/authority_active.rs

@@ -204,16 +204,15 @@ where
     A: AuthorityAPI + Send + Sync + 'static + Clone,
 {
     pub async fn spawn_all_active_processes(self) {
-        self.spawn_active_processes(true, true, CheckpointProcessControl::default())
+        self.spawn_active_processes(true, Some(CheckpointProcessControl::default()))
             .await
     }
 
     /// Spawn all active tasks.
     pub async fn spawn_active_processes(
         self,
         gossip: bool,
-        checkpoint: bool,
-        checkpoint_process_control: CheckpointProcessControl,
+        checkpoint_process_control: Option<CheckpointProcessControl>,
     ) {
         let active = Arc::new(self);
         // Spawn a task to take care of gossip
@@ -227,8 +226,8 @@ where
         // Spawn task to take care of checkpointing
         let checkpoint_locals = active; // .clone();
         let _checkpoint_join = tokio::task::spawn(async move {
-            if checkpoint {
-                checkpoint_process(&checkpoint_locals, &checkpoint_process_control).await;
+            if let Some(checkpoint) = checkpoint_process_control {
+                checkpoint_process(&checkpoint_locals, &checkpoint).await;
             }
         });
 

crates/sui-core/src/authority_active/checkpoint_driver/tests.rs

@@ -110,7 +110,7 @@ async fn checkpoint_active_flow_crash_client_with_gossip() {
             .unwrap();
             // Spin the gossip service.
             active_state
-                .spawn_active_processes(true, true, CheckpointProcessControl::default())
+                .spawn_active_processes(true, Some(CheckpointProcessControl::default()))
                 .await;
         });
     }
@@ -198,7 +198,7 @@ async fn checkpoint_active_flow_crash_client_no_gossip() {
             .unwrap();
             // Spin the gossip service.
             active_state
-                .spawn_active_processes(false, true, CheckpointProcessControl::default())
+                .spawn_active_processes(false, Some(CheckpointProcessControl::default()))
                 .await;
         });
     }

crates/sui/tests/checkpoints_tests.rs

@@ -24,10 +24,7 @@ use typed_store::Map;
 
 /// Helper function determining whether the checkpoint store of an authority contains the input
 /// transactions' digests.
-fn checkpoint_contains_digests(
-    authority: &AuthorityState,
-    transaction_digests: &HashSet<TransactionDigest>,
-) -> bool {
+fn transactions_in_checkpoint(authority: &AuthorityState) -> HashSet<TransactionDigest> {
     let checkpoints_store = authority.checkpoints().unwrap();
 
     // Get all transactions in the first 10 checkpoints.
@@ -46,7 +43,6 @@ fn checkpoint_contains_digests(
                 .collect::<HashSet<_>>()
         })
         .collect::<HashSet<_>>()
-        .is_superset(transaction_digests)
 }
 
 #[tokio::test]
@@ -152,7 +148,7 @@ async fn end_to_end() {
                 ..CheckpointProcessControl::default()
             };
             active_state
-                .spawn_active_processes(true, true, checkpoint_process_control)
+                .spawn_active_processes(true, Some(checkpoint_process_control))
                 .await
         });
     }
@@ -172,37 +168,38 @@ async fn end_to_end() {
         tokio::time::sleep(Duration::from_millis(5)).await;
     }
 
-    // Wait for the transactions to be executed and end up in a checkpoint. Ensure all authorities
-    // moved to the next checkpoint sequence number.
+    // Wait for the transactions to be executed and end up in a checkpoint.
     loop {
+        // Ensure all submitted transactions are in the checkpoint.
         let ok = handles
             .iter()
-            .map(|authority| {
-                authority
-                    .state()
-                    .checkpoints()
-                    .unwrap()
-                    .lock()
-                    .get_locals()
-                    .next_checkpoint
-            })
-            .all(|sequence| sequence >= 1);
+            .map(|authority| transactions_in_checkpoint(&authority.state()))
+            .all(|digests| digests.is_superset(&transaction_digests));
 
         match ok {
             true => break,
             false => tokio::time::sleep(Duration::from_millis(10)).await,
         }
     }
 
-    // Ensure all submitted transactions are in the checkpoint.
-    for authority in &handles {
-        let ok = checkpoint_contains_digests(&authority.state(), &transaction_digests);
-        assert!(ok);
-    }
+    // Ensure all authorities moved to the next checkpoint sequence number.
+    let ok = handles
+        .iter()
+        .map(|authority| {
+            authority
+                .state()
+                .checkpoints()
+                .unwrap()
+                .lock()
+                .get_locals()
+                .next_checkpoint
+        })
+        .all(|sequence| sequence >= 1);
+    assert!(ok);
 }
 
 #[tokio::test]
-async fn end_to_end_with_shared_objects() {
+async fn checkpoint_with_shared_objects() {
     // Get some gas objects to submit shared-objects transactions.
     let mut gas_objects = test_gas_objects();
 
@@ -232,7 +229,7 @@ async fn end_to_end_with_shared_objects() {
                 ..CheckpointProcessControl::default()
             };
             active_state
-                .spawn_active_processes(true, true, checkpoint_process_control)
+                .spawn_active_processes(true, Some(checkpoint_process_control))
                 .await
         });
     }
@@ -303,31 +300,32 @@ async fn end_to_end_with_shared_objects() {
     transaction_digests.insert(*create_counter_transaction.digest());
     transaction_digests.insert(*increment_counter_transaction.digest());
 
-    // Wait for the transactions to be executed and end up in a checkpoint. Ensure all authorities
-    // moved to the next checkpoint sequence number.
+    // Wait for the transactions to be executed and end up in a checkpoint.
     loop {
+        // Ensure all submitted transactions are in the checkpoint.
         let ok = handles
             .iter()
-            .map(|authority| {
-                authority
-                    .state()
-                    .checkpoints()
-                    .unwrap()
-                    .lock()
-                    .get_locals()
-                    .next_checkpoint
-            })
-            .all(|sequence| sequence >= 2);
+            .map(|authority| transactions_in_checkpoint(&authority.state()))
+            .all(|digests| digests.is_superset(&transaction_digests));
 
         match ok {
             true => break,
             false => tokio::time::sleep(Duration::from_millis(10)).await,
         }
     }
 
-    // Ensure all submitted transactions are in the checkpoint.
-    for authority in &handles {
-        let ok = checkpoint_contains_digests(&authority.state(), &transaction_digests);
-        assert!(ok);
-    }
+    // Ensure all authorities moved to the next checkpoint sequence number.
+    let ok = handles
+        .iter()
+        .map(|authority| {
+            authority
+                .state()
+                .checkpoints()
+                .unwrap()
+                .lock()
+                .get_locals()
+                .next_checkpoint
+        })
+        .all(|sequence| sequence >= 1);
+    assert!(ok);
 }