Small improvements to checkpoint (MystenLabs#2829)

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

@@ -344,9 +344,10 @@ where
         .iter()
         .for_each(|(auth, _proposal, checkpoint)| {
             if let AuthenticatedCheckpoint::Signed(signed) = checkpoint {
-                // We check this signature is higher than the highest known checkpoint.
+                // We are interested in this signed checkpoint only if it is
+                // newer than the highest known cert checkpoint.
                 if let Some(newest_checkpoint) = &highest_certificate_cert {
-                    if newest_checkpoint.summary.sequence_number > signed.summary.sequence_number {
+                    if newest_checkpoint.summary.sequence_number >= signed.summary.sequence_number {
                         return;
                     }
                 }
@@ -385,8 +386,6 @@ where
             }
         });
 
-    // Examine whether we should start the next checkpoint by looking at whether we have
-    // >2/3 of validators proposing a new checkpoint.
     let next_proposal_sequence_number = highest_certificate_cert
         .as_ref()
         .map(|cert| cert.summary.sequence_number + 1)
@@ -493,13 +492,13 @@ where
 
     for seq in full_sync_start..latest_known_checkpoint.summary.sequence_number {
         debug!("Full Sync ({name:?}): {seq:?}");
-        let (past, _contents) =
+        let (past, contents) =
             get_one_checkpoint(net.clone(), seq, true, &available_authorities).await?;
 
         if let Err(err) =
             checkpoint_db
                 .lock()
-                .process_checkpoint_certificate(&past, &_contents, &net.committee)
+                .process_checkpoint_certificate(&past, &contents, &net.committee)
         {
             warn!("Sync Err: {err:?}");
         }

crates/sui-core/src/checkpoints/mod.rs

@@ -392,13 +392,14 @@ impl CheckpointStore {
             return Ok(());
         }
 
-        // Is this the next expected certificate?
+        // Is this the next expected checkpoint?
         fp_ensure!(
             self.next_checkpoint() == checkpoint_sequence_number,
             SuiError::GenericAuthorityError {
                 error: format!(
-                    "Unexpected certificate, expected next seq={}",
-                    self.next_checkpoint()
+                    "Unexpected checkpoint, expected next seq={}, provided seq={}",
+                    self.next_checkpoint(),
+                    checkpoint_sequence_number,
                 ),
             }
         );
@@ -624,15 +625,6 @@ impl CheckpointStore {
         let our_proposal = locals.current_proposal.as_ref().unwrap();
 
         if let Ok(Some(contents)) = self.reconstruct_contents(committee, our_proposal) {
-            // Here we check, and ensure, all transactions are processed before we
-            // move to sign the checkpoint.
-            if !self
-                .all_checkpoint_transactions_executed(&contents)
-                .map_err(FragmentInternalError::Error)?
-            {
-                return Ok(false);
-            }
-
             let previous_digest = self
                 .get_prev_checkpoint_digest(next_sequence_number)
                 .map_err(FragmentInternalError::Error)?;
@@ -813,7 +805,7 @@ impl CheckpointStore {
 
     // Helper read functions
 
-    /// Return the seq number of the last checkpoint we have recorded.
+    /// Return the seq number of the next checkpoint.
     pub fn next_checkpoint(&mut self) -> CheckpointSequenceNumber {
         self.get_locals().next_checkpoint
     }
@@ -907,23 +899,13 @@ impl CheckpointStore {
         &self,
         transactions: &CheckpointContents,
     ) -> SuiResult<bool> {
-        let new_transactions = self
+        // TODO: What mechanisms are there to ensure these not-yet-executed transactions
+        // will eventually be executed?
+        Ok(self
             .extra_transactions
             .multi_get(transactions.transactions.iter())?
-            .into_iter()
-            .zip(transactions.transactions.iter())
-            .filter_map(
-                |(opt_seq, tx)| {
-                    if opt_seq.is_none() {
-                        Some(*tx)
-                    } else {
-                        None
-                    }
-                },
-            )
-            .count();
-
-        Ok(new_transactions == 0)
+            .iter()
+            .all(|opt| opt.is_some()))
     }
 
     #[cfg(test)]