Increase base consensus timeout and make it a config (#4532)

Sui has a timeout on sending transaction to the nw consensus and automatically adjusts the timeout based on recent latency from nw it has seen in case of success, or linearly increases in case of timeout.

In the stress test network we see quite a few of nw timeouts from the sui transaction submission side, and we indeed [see](http://grafana.shared.internal.sui.io:3000/goto/wqnUWbGVz?orgId=1) that control delay is adjusted accordingly to the timeouts and nw is not stuck.

However, it seems that current timeout adjustment logic drops timeout pretty quickly which causes timeouts again.

While this is not end of the world, it is noise and probably suboptimal.

It also can be noticed that current base timeout value is very close to the actual consensus delay, so the timeout has to be frequently adjusted. Therefore, a short-medium term fix (this PR) is just to bump the delay step for timeout calculations so that it is further away from median nw latency.

In the future we can improve timeout calculation by using weighted avg over a longer window instead of weighting current and next timing only.

crates/sui-config/src/builder.rs

@@ -180,6 +180,7 @@ impl<R: ::rand::RngCore + ::rand::CryptoRng> ConfigBuilder<R> {
                 let consensus_config = ConsensusConfig {
                     consensus_address,
                     consensus_db_path,
+                    delay_step: Some(15_000),
                     narwhal_config: Default::default(),
                 };
 

crates/sui-config/src/node.rs

@@ -152,6 +152,7 @@ impl NodeConfig {
 pub struct ConsensusConfig {
     pub consensus_address: Multiaddr,
     pub consensus_db_path: PathBuf,
+    pub delay_step: Option<u64>,
 
     pub narwhal_config: ConsensusParameters,
 }

crates/sui-config/tests/snapshots/snapshot_tests__network_config_snapshot_matches.snap

@@ -15,6 +15,7 @@ validator_configs:
     consensus-config:
       consensus-address: ""
       consensus-db-path: /tmp/foo/
+      delay-step: 15000
       narwhal-config:
         header_size: 1000
         max_header_delay: 100ms
@@ -55,6 +56,7 @@ validator_configs:
     consensus-config:
       consensus-address: ""
       consensus-db-path: /tmp/foo/
+      delay-step: 15000
       narwhal-config:
         header_size: 1000
         max_header_delay: 100ms
@@ -95,6 +97,7 @@ validator_configs:
     consensus-config:
       consensus-address: ""
       consensus-db-path: /tmp/foo/
+      delay-step: 15000
       narwhal-config:
         header_size: 1000
         max_header_delay: 100ms
@@ -135,6 +138,7 @@ validator_configs:
     consensus-config:
       consensus-address: ""
       consensus-db-path: /tmp/foo/
+      delay-step: 15000
       narwhal-config:
         header_size: 1000
         max_header_delay: 100ms
@@ -175,6 +179,7 @@ validator_configs:
     consensus-config:
       consensus-address: ""
       consensus-db-path: /tmp/foo/
+      delay-step: 15000
       narwhal-config:
         header_size: 1000
         max_header_delay: 100ms
@@ -215,6 +220,7 @@ validator_configs:
     consensus-config:
       consensus-address: ""
       consensus-db-path: /tmp/foo/
+      delay-step: 15000
       narwhal-config:
         header_size: 1000
         max_header_delay: 100ms
@@ -255,6 +261,7 @@ validator_configs:
     consensus-config:
       consensus-address: ""
       consensus-db-path: /tmp/foo/
+      delay-step: 15000
       narwhal-config:
         header_size: 1000
         max_header_delay: 100ms

crates/sui-core/src/authority_server.rs

@@ -213,12 +213,13 @@ impl ValidatorService {
 
         let metrics = ConsensusAdapterMetrics::new(&prometheus_registry);
 
+        let delay_step = consensus_config.delay_step.unwrap_or(15_000);
         // The consensus adapter allows the authority to send user certificates through consensus.
         let consensus_adapter = ConsensusAdapter::new(
             consensus_config.address().to_owned(),
             state.clone_committee(),
             tx_sui_to_consensus.clone(),
-            /* max_delay */ Duration::from_millis(5_000),
+            Duration::from_millis(delay_step),
             metrics.clone(),
         );
 

crates/sui-core/src/consensus_adapter.rs

@@ -207,12 +207,11 @@ pub struct ConsensusAdapter {
     committee: Committee,
     /// A channel to notify the consensus listener to take action for a transactions.
     tx_consensus_listener: Sender<ConsensusListenerMessage>,
-    /// The maximum duration to wait from consensus before aborting the transaction. After
-    /// this delay passed, the client will be notified that its transaction was probably not
-    /// sequence and it should try to resubmit its transaction.
-    max_delay: Duration,
+    /// Additional amount on top of delay_ms to calculate consensus timeout
+    /// Worst case consensus timeout grows linearly by adding delay_step every timeout
+    delay_step: Duration,
 
-    /// Estimation of the conensus delay, to use to dynamically adjust the delay
+    /// Estimation of the consensus delay, to use to dynamically adjust the delay
     /// before we time out, so that we do not spam the consensus adapter with the
     /// same transaction.
     delay_ms: AtomicU64,
@@ -227,7 +226,7 @@ impl ConsensusAdapter {
         consensus_address: Multiaddr,
         committee: Committee,
         tx_consensus_listener: Sender<ConsensusListenerMessage>,
-        max_delay: Duration,
+        delay_step: Duration,
         opt_metrics: OptArcConsensusAdapterMetrics,
     ) -> Self {
         let consensus_client = TransactionsClient::new(
@@ -238,8 +237,8 @@ impl ConsensusAdapter {
             consensus_client,
             committee,
             tx_consensus_listener,
-            max_delay,
-            delay_ms: AtomicU64::new(max_delay.as_millis() as u64),
+            delay_step,
+            delay_ms: AtomicU64::new(delay_step.as_millis() as u64),
             opt_metrics,
         }
     }
@@ -310,7 +309,7 @@ impl ConsensusAdapter {
         // client to retry if we timeout without hearing back from consensus (this module does not
         // handle retries). The best timeout value depends on the consensus protocol.
         let back_off_delay =
-            self.max_delay + Duration::from_millis(self.delay_ms.load(Ordering::Relaxed));
+            self.delay_step + Duration::from_millis(self.delay_ms.load(Ordering::Relaxed));
         let result = match timeout(back_off_delay, waiter.wait_for_result()).await {
             Ok(_) => {
                 // Increment the attempted certificate sequencing success
@@ -345,7 +344,7 @@ impl ConsensusAdapter {
             // but all we really need here is some max so that we do not wait for ever
             // in case consensus if dead.
             let new_delay =
-                new_delay.min((self.max_delay.as_millis() as u64) * MAX_DELAY_MULTIPLIER);
+                new_delay.min((self.delay_step.as_millis() as u64) * MAX_DELAY_MULTIPLIER);
 
             // Store the latest latency
             self.opt_metrics.as_ref().map(|metrics| {