add a description field in TooManyIncorrectAuthorities (MystenLabs#3906)

* amplify TooManyAuthorityFailure * decompose remote faucet response for better debuggability * use &'static str
bulentbozdag · Aug 12, 2022 · 4558de4 · 4558de4
1 parent 03beb9d
commit 4558de4
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 16 deletions.
diff --git a/crates/sui-cluster-test/src/faucet.rs b/crates/sui-cluster-test/src/faucet.rs
@@ -93,19 +93,24 @@ impl FaucetClient for RemoteFaucetClient {
             .json(&map)
             .send()
             .await
-            .unwrap()
-            .json::<FaucetResponse>()
-            .await
+            .unwrap_or_else(|e| panic!("Failed to talk to remote faucet {:?}: {:?}", gas_url, e));
+        let full_bytes = response.bytes().await.unwrap();
+        let faucet_response: FaucetResponse = serde_json::from_slice(&full_bytes)
+            .map_err(|e| anyhow::anyhow!("json deser failed with bytes {:?}: {e}", full_bytes))
             .unwrap();
 
-        if let Some(error) = response.error {
+        if let Some(error) = faucet_response.error {
             panic!("Failed to get gas tokens with error: {}", error)
         }
 
         sleep(Duration::from_secs(2)).await;
 
-        let gas_coins =
-            into_gas_coin_with_owner_check(response.transferred_gas_objects, address, client).await;
+        let gas_coins = into_gas_coin_with_owner_check(
+            faucet_response.transferred_gas_objects,
+            address,
+            client,
+        )
+        .await;
 
         let minimum_coins = minimum_coins.unwrap_or(5);
 

diff --git a/crates/sui-core/src/authority_active/checkpoint_driver/mod.rs b/crates/sui-core/src/authority_active/checkpoint_driver/mod.rs
@@ -374,6 +374,7 @@ where
                         if state.bad_weight > validity {
                             return Err(SuiError::TooManyIncorrectAuthorities {
                                 errors: state.errors,
+                                action: "get_latest_checkpoint_from_all",
                             });
                         }
                     }

diff --git a/crates/sui-core/src/authority_aggregator.rs b/crates/sui-core/src/authority_aggregator.rs
@@ -767,6 +767,8 @@ where
         timeout_each_authority: Duration,
         // When to give up on the attempt entirely.
         timeout_total: Option<Duration>,
+        // The behavior that authorities expect to perform, used for logging and error
+        description: &'static str,
     ) -> Result<S, SuiError>
     where
         FMap: Fn(AuthorityName, SafeClient<A>) -> AsyncResult<'a, S, SuiError> + Send + Clone + 'a,
@@ -792,6 +794,7 @@ where
                             .iter()
                             .map(|(a, b)| (*a, b.clone()))
                             .collect(),
+                        action: description,
                     }
                 }
             })?
@@ -871,6 +874,7 @@ where
                                             response.err().map(|err| (name, err))
                                         })
                                         .collect(),
+                                    action: "get_object_by_id",
                                 });
                             }
                         }
@@ -1027,6 +1031,7 @@ where
                                 if state.bad_weight > validity {
                                     return Err(SuiError::TooManyIncorrectAuthorities {
                                         errors: state.errors,
+                                        action: "get_all_owned_objects",
                                     });
                                 }
                             }
@@ -1704,6 +1709,7 @@ where
             |_, client| Box::pin(async move { client.handle_checkpoint(request.clone()).await }),
             self.timeouts.serial_authority_request_timeout,
             timeout_total,
+            "handle_checkpoint_request",
         )
         .await
     }
@@ -1743,6 +1749,7 @@ where
             },
             self.timeouts.serial_authority_request_timeout,
             timeout_total,
+            "get_certified_checkpoint",
         )
         .await
     }
@@ -1777,6 +1784,7 @@ where
             },
             self.timeouts.serial_authority_request_timeout,
             timeout_total,
+            "handle_cert_info_request",
         )
         .await
     }
@@ -1821,6 +1829,7 @@ where
             },
             self.timeouts.serial_authority_request_timeout,
             timeout_total,
+            "handle_transaction_and_effects_info_request",
         )
         .await
     }
@@ -1930,6 +1939,7 @@ where
             .true_effects
             .ok_or(SuiError::TooManyIncorrectAuthorities {
                 errors: final_state.errors,
+                action: "execute_cert_to_true_effects",
             })
             .tap_err(|e| info!(?digest, "execute_cert_to_true_effects failed: {}", e))
     }

diff --git a/crates/sui-core/src/epoch/reconfiguration.rs b/crates/sui-core/src/epoch/reconfiguration.rs
@@ -361,6 +361,7 @@ where
                             if state.bad_weight > validity {
                                 return Err(SuiError::TooManyIncorrectAuthorities {
                                     errors: state.errors,
+                                    action: "wait_for_epoch_cert",
                                 });
                             }
                         }
@@ -377,6 +378,7 @@ where
         } else {
             Err(SuiError::TooManyIncorrectAuthorities {
                 errors: final_state.errors,
+                action: "wait_for_epoch_cert",
             })
         }
     }

diff --git a/crates/sui-core/src/unit_tests/authority_aggregator_tests.rs b/crates/sui-core/src/unit_tests/authority_aggregator_tests.rs
@@ -381,9 +381,12 @@ async fn test_map_reducer() {
             0usize,
             |_name, _client| Box::pin(async move { Ok(()) }),
             |_accumulated_state, _authority_name, _authority_weight, _result| {
-                Box::pin(
-                    async move { Err(SuiError::TooManyIncorrectAuthorities { errors: vec![] }) },
-                )
+                Box::pin(async move {
+                    Err(SuiError::TooManyIncorrectAuthorities {
+                        errors: vec![],
+                        action: "",
+                    })
+                })
             },
             Duration::from_millis(1000),
         )
@@ -399,8 +402,10 @@ async fn test_map_reducer() {
             0usize,
             |_name, _client| {
                 Box::pin(async move {
-                    let res: Result<usize, SuiError> =
-                        Err(SuiError::TooManyIncorrectAuthorities { errors: vec![] });
+                    let res: Result<usize, SuiError> = Err(SuiError::TooManyIncorrectAuthorities {
+                        errors: vec![],
+                        action: "",
+                    });
                     res
                 })
             },
@@ -451,9 +456,12 @@ async fn test_map_reducer() {
                 })
             },
             |_accumulated_state, _authority_name, _authority_weight, _result| {
-                Box::pin(
-                    async move { Err(SuiError::TooManyIncorrectAuthorities { errors: vec![] }) },
-                )
+                Box::pin(async move {
+                    Err(SuiError::TooManyIncorrectAuthorities {
+                        errors: vec![],
+                        action: "",
+                    })
+                })
             },
             Duration::from_millis(10),
         )
@@ -1002,6 +1010,7 @@ async fn test_quorum_once_with_timeout() {
             },
             Duration::from_millis(authority_request_timeout),
             Some(Duration::from_millis(30 * 50)),
+            "test",
         )
         .await
         .unwrap();

diff --git a/crates/sui-faucet/src/faucet/simple_faucet.rs b/crates/sui-faucet/src/faucet/simple_faucet.rs
@@ -103,7 +103,6 @@ impl SimpleFaucet {
                 break;
             }
         }
-        debug!("Planning to use coins: {:?}", coins);
         coins
     }
 
@@ -115,6 +114,8 @@ impl SimpleFaucet {
     ) -> Result<Vec<(TransactionDigest, ObjectID, u64, ObjectID)>, FaucetError> {
         let number_of_coins = amounts.len();
         let coins = self.select_coins(number_of_coins).await;
+        debug!(recipient=?to, ?uuid, "Planning to use coins: {:?}", coins);
+
         let futures: Vec<_> = coins
             .iter()
             .zip(amounts)
@@ -247,6 +248,14 @@ impl Faucet for SimpleFaucet {
 
         let results = self.transfer_gases(amounts, recipient, id).await?;
 
+        if results.len() != amounts.len() {
+            return Err(FaucetError::Transfer(format!(
+                "Requested {} coins but only got {}",
+                amounts.len(),
+                results.len()
+            )));
+        }
+
         let elapsed = timer.stop_and_record();
 
         info!(uuid = ?id, ?recipient, ?results, "Transfer txn succeeded in {} secs", elapsed);

diff --git a/crates/sui-types/src/error.rs b/crates/sui-types/src/error.rs
@@ -333,9 +333,10 @@ pub enum SuiError {
     ConcurrentTransactionError,
     #[error("Transfer should be received by us.")]
     IncorrectRecipientError,
-    #[error("Too many authority errors were detected: {:?}", errors)]
+    #[error("Too many authority errors were detected for {}: {:?}", action, errors)]
     TooManyIncorrectAuthorities {
         errors: Vec<(AuthorityName, SuiError)>,
+        action: &'static str,
     },
     #[error("Inconsistent results observed in the Gateway. This should not happen and typically means there is a bug in the Sui implementation. Details: {error:?}")]
     InconsistentGatewayResult { error: String },