[client errors] Centralise the per-authority error handling in the sa…

…fe client (MystenLabs#507)

* Added report_client_error
* Use the safe client error reporting in authority aggregator

Co-authored-by: George Danezis <[email protected]>

sui_core/src/authority_aggregator.rs

@@ -167,12 +167,7 @@ where
                         .certified_transaction
                         .ok_or(SuiError::AuthorityInformationUnavailable)?;
 
-                    // Check & Add it to the list of certificates to sync
-                    returned_certificate.check(&self.committee).map_err(|_| {
-                        SuiError::ByzantineAuthoritySuspicion {
-                            authority: source_authority,
-                        }
-                    })?;
+                    // Add it to the list of certificates to sync
                     missing_certificates.push(ConfirmationTransaction::new(returned_certificate));
                 }
             }
@@ -224,17 +219,40 @@ where
             // Note: here we could improve this function by passing into the
             //       `sync_authority_source_to_destination` call a cache of
             //       certificates and parents to avoid re-downloading them.
-            if timeout(
-                timeout_period,
-                self.sync_authority_source_to_destination(
-                    cert.clone(),
-                    source_authority,
-                    destination_authority,
-                ),
-            )
-            .await
-            .is_ok()
-            {
+
+            let logic = async {
+                let res = self
+                    .sync_authority_source_to_destination(
+                        cert.clone(),
+                        source_authority,
+                        destination_authority,
+                    )
+                    .await;
+
+                if let Err(err) = &res {
+                    // We checked that the source authority has all the information
+                    // since the source has signed the certificate. Either the
+                    // source or the destination authority may be faulty.
+
+                    let inner_err = SuiError::PairwiseSyncFailed {
+                        xsource: source_authority,
+                        destination: destination_authority,
+                        tx_digest: cert.certificate.transaction.digest(),
+                        error: Box::new(err.clone()),
+                    };
+
+                    // Report the error to both authority clients.
+                    let source_client = &self.authority_clients[&source_authority];
+                    let destination_client = &self.authority_clients[&destination_authority];
+
+                    source_client.report_client_error(inner_err.clone());
+                    destination_client.report_client_error(inner_err);
+                }
+
+                res
+            };
+
+            if timeout(timeout_period, logic).await.is_ok() {
                 // If the updates suceeds we return, since there is no need
                 // to try other sources.
                 return Ok(());

sui_core/src/safe_client.rs

@@ -165,6 +165,15 @@ impl<C> SafeClient<C> {
 
         Ok(())
     }
+
+    /// This function is used by the higher level authority logic to report an
+    /// error that could be due to this authority.
+    pub fn report_client_error(&self, _error: SuiError) {
+        // TODO: At a minumum we log this error along the authority name, and potentially
+        // in case of strong evidence of byzantine behaviour we could share this error
+        // with the rest of the network, or de-prioritize requests to this authority given
+        // weaker evidence.
+    }
 }
 
 #[async_trait]
@@ -182,7 +191,10 @@ where
             .authority_client
             .handle_transaction(transaction)
             .await?;
-        self.check_transaction_response(digest, &transaction_info)?;
+        if let Err(err) = self.check_transaction_response(digest, &transaction_info) {
+            self.report_client_error(err.clone());
+            return Err(err);
+        }
         Ok(transaction_info)
     }
 
@@ -196,7 +208,11 @@ where
             .authority_client
             .handle_confirmation_transaction(transaction)
             .await?;
-        self.check_transaction_response(digest, &transaction_info)?;
+
+        if let Err(err) = self.check_transaction_response(digest, &transaction_info) {
+            self.report_client_error(err.clone());
+            return Err(err);
+        }
         Ok(transaction_info)
     }
 
@@ -217,7 +233,10 @@ where
             .authority_client
             .handle_object_info_request(request.clone())
             .await?;
-        self.check_object_response(&request, &response)?;
+        if let Err(err) = self.check_object_response(&request, &response) {
+            self.report_client_error(err.clone());
+            return Err(err);
+        }
         Ok(response)
     }
 
@@ -231,7 +250,11 @@ where
             .authority_client
             .handle_transaction_info_request(request)
             .await?;
-        self.check_transaction_response(digest, &transaction_info)?;
+
+        if let Err(err) = self.check_transaction_response(digest, &transaction_info) {
+            self.report_client_error(err.clone());
+            return Err(err);
+        }
         Ok(transaction_info)
     }
 }

sui_types/src/error.rs

@@ -198,6 +198,15 @@ pub enum SuiError {
         "We have received cryptographic level of evidence that authority {authority:?} is faulty in a Byzantine manner."
     )]
     ByzantineAuthoritySuspicion { authority: AuthorityName },
+    #[error(
+        "Sync from authority failed. From {xsource:?} to {destination:?}, digest {tx_digest:?}: {error:?}",
+    )]
+    PairwiseSyncFailed {
+        xsource: AuthorityName,
+        destination: AuthorityName,
+        tx_digest: TransactionDigest,
+        error: Box<SuiError>,
+    },
     #[error("Storage error")]
     StorageError(#[from] typed_store::rocks::TypedStoreError),