[checkpoint] Remove AuthenticatedCheckpoint::None (MystenLabs#3439)

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

@@ -360,7 +360,7 @@ where
         responses: Vec<(
             AuthorityName,
             Option<SignedCheckpointProposalSummary>,
-            AuthenticatedCheckpoint,
+            Option<AuthenticatedCheckpoint>,
         )>,
         errors: Vec<(AuthorityName, SuiError)>,
     }
@@ -429,7 +429,7 @@ where
     // Extract the highest checkpoint cert returned.
     let mut highest_certificate_cert: Option<CertifiedCheckpointSummary> = None;
     for state in &final_state.responses {
-        if let AuthenticatedCheckpoint::Certified(cert) = &state.2 {
+        if let Some(AuthenticatedCheckpoint::Certified(cert)) = &state.2 {
             if let Some(old_cert) = &highest_certificate_cert {
                 if cert.summary.sequence_number > old_cert.summary.sequence_number {
                     highest_certificate_cert = Some(cert.clone());
@@ -450,7 +450,7 @@ where
         .responses
         .iter()
         .for_each(|(auth, _proposal, checkpoint)| {
-            if let AuthenticatedCheckpoint::Signed(signed) = checkpoint {
+            if let Some(AuthenticatedCheckpoint::Signed(signed)) = 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 {
@@ -759,7 +759,7 @@ where
         {
             if let AuthorityCheckpointInfo::Proposal { current, previous } = &response.info {
                 // Check if there is a latest checkpoint
-                if let AuthenticatedCheckpoint::Certified(prev) = previous {
+                if let Some(AuthenticatedCheckpoint::Certified(prev)) = previous {
                     if prev.summary.sequence_number >= next_checkpoint_sequence_number {
                         // We are now behind, stop the process
                         break;

crates/sui-core/src/authority_aggregator.rs

@@ -1605,7 +1605,9 @@ where
 
                     if let CheckpointResponse {
                         info:
-                            AuthorityCheckpointInfo::Past(AuthenticatedCheckpoint::Certified(past)),
+                            AuthorityCheckpointInfo::Past(Some(AuthenticatedCheckpoint::Certified(
+                                past,
+                            ))),
                         detail,
                     } = resp
                     {

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

@@ -152,13 +152,7 @@ impl CheckpointStore {
         // Extract the previous checkpoint digest if there is one.
         Ok(if checkpoint_sequence > 0 {
             self.get_checkpoint(checkpoint_sequence - 1)?
-                .map(|prev_checkpoint| match prev_checkpoint {
-                    AuthenticatedCheckpoint::Certified(cert) => cert.summary.digest(),
-                    AuthenticatedCheckpoint::Signed(signed) => signed.summary.digest(),
-                    _ => {
-                        unreachable!();
-                    }
-                })
+                .map(|prev_checkpoint| prev_checkpoint.summary().digest())
         } else {
             None
         })
@@ -311,8 +305,7 @@ impl CheckpointStore {
             .iter()
             .skip_to_last()
             .next()
-            .map(|(_, c)| c)
-            .unwrap_or(AuthenticatedCheckpoint::None);
+            .map(|(_, c)| c);
 
         // Get the current proposal if there is one.
         let current = latest_checkpoint_proposal
@@ -346,23 +339,17 @@ impl CheckpointStore {
         seq: CheckpointSequenceNumber,
     ) -> Result<CheckpointResponse, SuiError> {
         // Get the checkpoint with a given sequence number
-        let checkpoint = self
-            .checkpoints
-            .get(&seq)?
-            .unwrap_or(AuthenticatedCheckpoint::None);
+        let checkpoint = self.checkpoints.get(&seq)?;
 
         // If a checkpoint is found, and if requested, return the list of transaction digest in it.
-        let detail = if let &AuthenticatedCheckpoint::None = &checkpoint {
-            None
-        } else if detail {
-            self.checkpoint_contents.get(&seq)?
-        } else {
-            None
+        let content = match (detail, &checkpoint) {
+            (true, Some(_)) => self.checkpoint_contents.get(&seq)?,
+            _ => None,
         };
 
         Ok(CheckpointResponse {
             info: AuthorityCheckpointInfo::Past(checkpoint),
-            detail,
+            detail: content,
         })
     }
 

crates/sui-core/src/checkpoints/tests/checkpoint_tests.rs

@@ -400,7 +400,7 @@ fn latest_proposal() {
     ));
     if let AuthorityCheckpointInfo::Proposal { current, previous } = response.info {
         assert!(current.is_none());
-        assert!(matches!(previous, AuthenticatedCheckpoint::None));
+        assert!(matches!(previous, None));
     }
 
     // ---
@@ -423,7 +423,7 @@ fn latest_proposal() {
     ));
     if let AuthorityCheckpointInfo::Proposal { current, previous } = response.info {
         assert!(current.is_some());
-        assert!(matches!(previous, AuthenticatedCheckpoint::None));
+        assert!(matches!(previous, None));
 
         let current_proposal = current.unwrap();
         current_proposal
@@ -444,7 +444,7 @@ fn latest_proposal() {
     ));
     if let AuthorityCheckpointInfo::Proposal { current, previous } = response.info {
         assert!(current.is_some());
-        assert!(matches!(previous, AuthenticatedCheckpoint::None));
+        assert!(matches!(previous, None));
 
         let current_proposal = current.unwrap();
         current_proposal
@@ -535,7 +535,10 @@ fn latest_proposal() {
     ));
     if let AuthorityCheckpointInfo::Proposal { current, previous } = response.info {
         assert!(current.is_none());
-        assert!(matches!(previous, AuthenticatedCheckpoint::Signed { .. }));
+        assert!(matches!(
+            previous,
+            Some(AuthenticatedCheckpoint::Signed { .. })
+        ));
     }
 
     // --- TEST 4 ---
@@ -551,7 +554,10 @@ fn latest_proposal() {
     ));
     if let AuthorityCheckpointInfo::Proposal { current, previous } = response.info {
         assert!(current.is_none());
-        assert!(matches!(previous, AuthenticatedCheckpoint::Signed { .. }));
+        assert!(matches!(
+            previous,
+            Some(AuthenticatedCheckpoint::Signed { .. })
+        ));
     }
 
     // ---
@@ -570,7 +576,10 @@ fn latest_proposal() {
     ));
     if let AuthorityCheckpointInfo::Proposal { current, previous } = response.info {
         assert!(current.is_some());
-        assert!(matches!(previous, AuthenticatedCheckpoint::Signed { .. }));
+        assert!(matches!(
+            previous,
+            Some(AuthenticatedCheckpoint::Signed { .. })
+        ));
 
         let current_proposal = current.unwrap();
         current_proposal
@@ -616,18 +625,12 @@ fn set_get_checkpoint() {
 
     // There is no previous checkpoint
     let response = cps1.handle_past_checkpoint(true, 0).unwrap();
-    assert!(matches!(
-        response.info,
-        AuthorityCheckpointInfo::Past(AuthenticatedCheckpoint::None)
-    ));
+    assert!(matches!(response.info, AuthorityCheckpointInfo::Past(None)));
     assert!(response.detail.is_none());
 
     // There is no previous checkpoint
     let response = cps1.handle_past_checkpoint(true, 0).unwrap();
-    assert!(matches!(
-        response.info,
-        AuthorityCheckpointInfo::Past(AuthenticatedCheckpoint::None)
-    ));
+    assert!(matches!(response.info, AuthorityCheckpointInfo::Past(None)));
     assert!(response.detail.is_none());
 
     // ---
@@ -694,17 +697,19 @@ fn set_get_checkpoint() {
     let response = cps1.handle_past_checkpoint(true, 0).unwrap();
     assert!(matches!(
         response.info,
-        AuthorityCheckpointInfo::Past(AuthenticatedCheckpoint::Signed(..))
+        AuthorityCheckpointInfo::Past(Some(AuthenticatedCheckpoint::Signed(..)))
     ));
-    if let AuthorityCheckpointInfo::Past(AuthenticatedCheckpoint::Signed(signed)) = response.info {
+    if let AuthorityCheckpointInfo::Past(Some(AuthenticatedCheckpoint::Signed(signed))) =
+        response.info
+    {
         signed.verify(&committee, response.detail.as_ref()).unwrap();
     }
 
     // Make a certificate
     let mut signed_checkpoint: Vec<SignedCheckpointSummary> = Vec::new();
     for x in [&mut cps1, &mut cps2, &mut cps3] {
         match x.handle_past_checkpoint(true, 0).unwrap().info {
-            AuthorityCheckpointInfo::Past(AuthenticatedCheckpoint::Signed(signed)) => {
+            AuthorityCheckpointInfo::Past(Some(AuthenticatedCheckpoint::Signed(signed))) => {
                 signed_checkpoint.push(signed)
             }
             _ => unreachable!(),
@@ -726,7 +731,7 @@ fn set_get_checkpoint() {
     let response = cps1.handle_past_checkpoint(true, 0).unwrap();
     assert!(matches!(
         response.info,
-        AuthorityCheckpointInfo::Past(AuthenticatedCheckpoint::Certified(..))
+        AuthorityCheckpointInfo::Past(Some(AuthenticatedCheckpoint::Certified(..)))
     ));
 
     // --- TEST 3 ---
@@ -756,7 +761,7 @@ fn set_get_checkpoint() {
     let response = cps4.handle_past_checkpoint(true, 0).unwrap();
     assert!(matches!(
         response.info,
-        AuthorityCheckpointInfo::Past(AuthenticatedCheckpoint::Certified(..))
+        AuthorityCheckpointInfo::Past(Some(AuthenticatedCheckpoint::Certified(..)))
     ));
 }
 
@@ -1814,7 +1819,7 @@ async fn checkpoint_messaging_flow() {
             .expect("No issues");
 
         match &response.info {
-            AuthorityCheckpointInfo::Past(AuthenticatedCheckpoint::Signed(checkpoint)) => {
+            AuthorityCheckpointInfo::Past(Some(AuthenticatedCheckpoint::Signed(checkpoint))) => {
                 signed_checkpoint.push(checkpoint.clone());
                 contents = response.detail.clone();
             }

crates/sui-core/src/safe_client.rs

@@ -386,13 +386,9 @@ where
     fn verify_checkpoint_sequence(
         &self,
         expected_seq: Option<CheckpointSequenceNumber>,
-        checkpoint: &AuthenticatedCheckpoint,
+        checkpoint: &Option<AuthenticatedCheckpoint>,
     ) -> SuiResult {
-        let observed_seq = match checkpoint {
-            AuthenticatedCheckpoint::None => None,
-            AuthenticatedCheckpoint::Signed(s) => Some(*s.summary.sequence_number()),
-            AuthenticatedCheckpoint::Certified(c) => Some(*c.summary.sequence_number()),
-        };
+        let observed_seq = checkpoint.as_ref().map(|c| c.summary().sequence_number);
 
         if let (Some(e), Some(o)) = (expected_seq, observed_seq) {
             fp_ensure!(
@@ -419,21 +415,19 @@ where
 
         // Verify response data was correct for request
         match &req_type {
-            CheckpointRequestType::LatestCheckpointProposal => {
-                if let AuthorityCheckpointInfo::Proposal { previous, .. } = &resp.info {
+            CheckpointRequestType::LatestCheckpointProposal => match &resp.info {
+                AuthorityCheckpointInfo::Proposal { previous, .. } => {
                     self.verify_checkpoint_sequence(None, previous)?;
                     Ok(resp)
-                } else {
-                    Err(SuiError::ByzantineAuthoritySuspicion {
-                        authority: self.address,
-                    })
                 }
-            }
+                _ => Err(SuiError::ByzantineAuthoritySuspicion {
+                    authority: self.address,
+                }),
+            },
             CheckpointRequestType::PastCheckpoint(seq) => {
                 if let AuthorityCheckpointInfo::Past(past) = &resp.info {
                     match past {
-                        AuthenticatedCheckpoint::Signed(_)
-                        | AuthenticatedCheckpoint::Certified(_) => {
+                        Some(_) => {
                             if detail && resp.detail.is_none() {
                                 // peer has the checkpoint, but refused to give us the contents.
                                 // (For AuthorityCheckpointInfo::Proposal, contents are not
@@ -444,7 +438,7 @@ where
                             }
                         }
                         // Checkpoint wasn't found, so detail is obviously not required.
-                        AuthenticatedCheckpoint::None => (),
+                        None => (),
                     }
                     self.verify_checkpoint_sequence(Some(*seq), past)?;
                     Ok(resp)

crates/sui-types/src/messages_checkpoint.rs

@@ -129,11 +129,18 @@ impl CheckpointResponse {
                 if let Some(current) = current {
                     current.verify(committee, self.detail.as_ref())?;
                     // detail pertains to the current proposal, not the previous
-                    previous.verify(committee, None)?;
+                    if let Some(previous) = previous {
+                        previous.verify(committee, None)?;
+                    }
+                }
+                Ok(())
+            }
+            AuthorityCheckpointInfo::Past(ckpt) => {
+                if let Some(ckpt) = ckpt {
+                    ckpt.verify(committee, self.detail.as_ref())?;
                 }
                 Ok(())
             }
-            AuthorityCheckpointInfo::Past(ckpt) => ckpt.verify(committee, self.detail.as_ref()),
         }
     }
 }
@@ -146,22 +153,19 @@ pub enum AuthorityCheckpointInfo {
     // the previous checkpoint.
     Proposal {
         current: Option<SignedCheckpointProposalSummary>,
-        previous: AuthenticatedCheckpoint,
+        previous: Option<AuthenticatedCheckpoint>,
         // Include in all responses the local state of the sequence
         // of transaction to allow followers to track the latest
         // updates.
         // last_local_sequence: TxSequenceNumber,
     },
     // Returns the requested checkpoint.
-    Past(AuthenticatedCheckpoint),
+    Past(Option<AuthenticatedCheckpoint>),
 }
 
 // TODO: Rename to AuthenticatedCheckpointSummary
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub enum AuthenticatedCheckpoint {
-    // No authentication information is available
-    // or checkpoint is not available on this authority.
-    None,
     // The checkpoint with just a single authority
     // signature.
     Signed(SignedCheckpointSummary),
@@ -174,15 +178,13 @@ impl AuthenticatedCheckpoint {
         match self {
             Self::Signed(s) => &s.summary,
             Self::Certified(c) => &c.summary,
-            Self::None => unreachable!(),
         }
     }
 
     pub fn verify(&self, committee: &Committee, detail: Option<&CheckpointContents>) -> SuiResult {
         match self {
             Self::Signed(s) => s.verify(committee, detail),
             Self::Certified(c) => c.verify(committee, detail),
-            Self::None => Ok(()),
         }
     }
 }