fix robust_value ordering (MystenLabs#3683)

* fix robust_value

* remove sorting option

crates/sui-core/src/authority_active.rs

@@ -38,7 +38,7 @@ use sui_types::{
 };
 use tokio::sync::Mutex;
 use tokio::task::JoinHandle;
-use tracing::info;
+use tracing::{debug, info};
 
 use crate::{
     authority::AuthorityState,
@@ -67,6 +67,7 @@ const DELAY_FOR_1_RETRY_MS: u64 = 2_000;
 const EXPONENTIAL_DELAY_BASIS: u64 = 2;
 pub const MAX_RETRY_DELAY_MS: u64 = 30_000;
 
+#[derive(Debug)]
 pub struct AuthorityHealth {
     // Records the number of retries
     pub retries: u32,
@@ -175,6 +176,19 @@ impl<A> ActiveAuthority<A> {
     /// even if we have a few connections.
     pub async fn minimum_wait_for_majority_honest_available(&self) -> Instant {
         let lock = self.health.lock().await;
+
+        let health_overview: Vec<_> = lock
+            .iter()
+            .map(|(name, h)| {
+                (
+                    *name,
+                    h.retries,
+                    h.no_contact_before - tokio::time::Instant::now(),
+                )
+            })
+            .collect();
+        debug!(health_overview = ?health_overview, "Current validator health metrics");
+
         let (_, instant) = self.net.load().committee.robust_value(
             lock.iter().map(|(name, h)| (*name, h.no_contact_before)),
             // At least one honest node is at or above it.

crates/sui-core/src/authority_aggregator.rs

@@ -20,7 +20,7 @@ use sui_types::{
         CheckpointContents, CheckpointRequest, CheckpointResponse,
     },
 };
-use tracing::{debug, info, instrument, trace, Instrument};
+use tracing::{debug, error, info, instrument, trace, Instrument};
 
 use prometheus::{
     register_histogram_with_registry, register_int_counter_with_registry, Histogram, IntCounter,
@@ -1606,7 +1606,12 @@ where
                     if effects.effects.is_object_mutated_here(obj_ref) {
                         is_ok = true;
                     } else {
-                        // TODO: Report a byzantine fault here
+                        // TODO: Throw a byzantine fault here
+                        error!(
+                            ?object_id,
+                            ?tx_digest,
+                            "get_object_info_execute. Byzantine failure!"
+                        );
                         continue;
                     }
                 }

crates/sui-types/src/committee.rs

@@ -231,7 +231,7 @@ impl Committee {
         let mut total = 0;
         for (v, s, a) in items {
             total += s;
-            if threshold < total {
+            if threshold <= total {
                 return (a, v);
             }
         }
@@ -323,4 +323,34 @@ mod test {
         let res = committee.shuffle_by_stake(None, Some(&BTreeSet::new()));
         assert_eq!(0, res.len());
     }
+
+    #[test]
+    fn test_robust_value() {
+        let (_, sec1): (_, AuthorityKeyPair) = get_key_pair();
+        let (_, sec2): (_, AuthorityKeyPair) = get_key_pair();
+        let (_, sec3): (_, AuthorityKeyPair) = get_key_pair();
+        let (_, sec4): (_, AuthorityKeyPair) = get_key_pair();
+        let a1: AuthorityName = sec1.public().into();
+        let a2: AuthorityName = sec2.public().into();
+        let a3: AuthorityName = sec3.public().into();
+        let a4: AuthorityName = sec4.public().into();
+
+        let mut authorities = BTreeMap::new();
+        authorities.insert(a1, 1);
+        authorities.insert(a2, 1);
+        authorities.insert(a3, 1);
+        authorities.insert(a4, 1);
+        let committee = Committee::new(0, authorities).unwrap();
+        let items = vec![(a1, 666), (a2, 1), (a3, 2), (a4, 0)];
+        assert_eq!(
+            committee.robust_value(items.into_iter(), committee.quorum_threshold()),
+            (a3, 2)
+        );
+
+        let items = vec![(a1, "a"), (a2, "b"), (a3, "c"), (a4, "d")];
+        assert_eq!(
+            committee.robust_value(items.into_iter(), committee.quorum_threshold()),
+            (a3, "c")
+        );
+    }
 }