fix(consensus): exit non-zero on transient remote-signer startup failure

[tomasztrzos]

When the remote signer is unreachable at startup, the node's eager connect() fails fast (unlike the runtime signing path, which retries via sign_message_with_retry). run() caught this in its startup error arm and called wait_for_termination(), parking the process alive-but-idle until SIGTERM. A supervisor's restart policy never fires and a process-only liveness check doesn't notice, so the node stays parked indefinitely even after the signer recovers seconds later.

Return an error for transient remote-signer failures (Transport, ServiceUnavailable, Timeout) so main exits non-zero and the supervisor restarts the node — the same behavior already used for the EL IPC watchdog. Permanent errors (configuration, authentication, malformed responses) still park for manual intervention.

gRPC Status errors are intentionally excluded: the failure mode this guards against is the eager connect() at startup, which surfaces as Transport.

Add unit tests covering the helper through the production eyre wrap_err chain.

[romac]

Thanks a lot for the PR, sounds totally reasonable to me to crash on connectivity issues for improved signaling/recovery 👍

[romac]

After another round of review, I just realized we need to preserve a couple more errors within the error trace, otherwise these two cases will preserve the alive-but-idle failure mode this PR is trying to remove.

Please find attached a tentative diff for fixing that.

Show suggested changes

diff --git a/Cargo.lock b/Cargo.lock
index 47d70219..648750e9 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1820,6 +1820,7 @@ dependencies = [
  "tokio",
  "tokio-tungstenite 0.26.2",
  "tokio-util",
+ "tonic 0.12.3",
  "tower 0.5.3",
  "tracing",
  "url",
diff --git a/crates/malachite-app/Cargo.toml b/crates/malachite-app/Cargo.toml
index a827a4cd..8a3c796f 100644
--- a/crates/malachite-app/Cargo.toml
+++ b/crates/malachite-app/Cargo.toml
@@ -89,6 +89,7 @@ reqwest = { workspace = true, features = ["json"] }
 serial_test = { workspace = true }
 tempfile = { workspace = true }
 tokio = { workspace = true, features = ["test-util"] }
+tonic.workspace = true
 tower = { workspace = true }
 
 [lints]
diff --git a/crates/malachite-app/src/node.rs b/crates/malachite-app/src/node.rs
index c9b3e103..a7769434 100644
--- a/crates/malachite-app/src/node.rs
+++ b/crates/malachite-app/src/node.rs
@@ -894,21 +894,13 @@ fn install_sigterm_handler(_handle: &Handle) {}
 /// authentication, malformed responses) return `false` so they still park for
 /// manual intervention rather than restart-looping.
 ///
-/// Note: gRPC `Status` errors (`RemoteSigningError::Status`) are intentionally out
-/// of scope here — the failure this guards against is the eager `connect()` at
-/// startup, which surfaces as `Transport`. Keeping the set narrow avoids
-/// restart-looping on errors whose transience is ambiguous.
+/// The exact retryability rules live with `RemoteSigningError`, where tonic status
+/// codes and retry-exhaustion sources can be classified without string matching.
 fn is_retryable_startup_error(error: &eyre::Report) -> bool {
-    error.chain().any(|cause| {
-        matches!(
-            cause.downcast_ref::<RemoteSigningError>(),
-            Some(
-                RemoteSigningError::Transport(_)
-                    | RemoteSigningError::ServiceUnavailable(_)
-                    | RemoteSigningError::Timeout(_)
-            )
-        )
-    })
+    error
+        .chain()
+        .filter_map(|cause| cause.downcast_ref::<RemoteSigningError>())
+        .any(RemoteSigningError::is_retryable_startup_error)
 }
 
 /// Wait for a termination signal (SIGTERM on Unix)
@@ -1049,6 +1041,14 @@ mod tests {
             .wrap_err("Node failed to start")
     }
 
+    fn wrapped_validator_proof_signing(err: RemoteSigningError) -> eyre::Report {
+        let signing_error = arc_consensus_types::signing::SigningError::from_source(err);
+        eyre::Report::new(signing_error)
+            .wrap_err("Failed to sign validator proof")
+            .wrap_err("Failed to create network identity with validator proof")
+            .wrap_err("Node failed to start")
+    }
+
     #[test]
     fn transient_remote_signer_errors_are_retryable() {
         assert!(is_retryable_startup_error(&wrapped(
@@ -1057,6 +1057,22 @@ mod tests {
         assert!(is_retryable_startup_error(&wrapped(
             RemoteSigningError::Timeout("slow".into())
         )));
+        assert!(is_retryable_startup_error(&wrapped(
+            RemoteSigningError::Status(Box::new(tonic::Status::unavailable("signer warming up")))
+        )));
+        assert!(is_retryable_startup_error(&wrapped(
+            RemoteSigningError::Status(Box::new(tonic::Status::deadline_exceeded(
+                "signer request timed out"
+            )))
+        )));
+        assert!(is_retryable_startup_error(
+            &wrapped_validator_proof_signing(RemoteSigningError::RetryExhausted {
+                retries: 3,
+                source: Box::new(RemoteSigningError::Status(Box::new(
+                    tonic::Status::unavailable("signer warming up")
+                ))),
+            })
+        ));
     }
 
     #[test]
@@ -1067,6 +1083,17 @@ mod tests {
         assert!(!is_retryable_startup_error(&wrapped(
             RemoteSigningError::Authentication("denied".into())
         )));
+        assert!(!is_retryable_startup_error(&wrapped(
+            RemoteSigningError::Status(Box::new(tonic::Status::unauthenticated("bad credentials")))
+        )));
+        assert!(!is_retryable_startup_error(
+            &wrapped_validator_proof_signing(RemoteSigningError::RetryExhausted {
+                retries: 3,
+                source: Box::new(RemoteSigningError::Status(Box::new(
+                    tonic::Status::unauthenticated("bad credentials")
+                ))),
+            })
+        ));
     }
 
     #[test]
diff --git a/crates/malachite-app/src/validator_proof.rs b/crates/malachite-app/src/validator_proof.rs
index 6fc755ff..11b9317b 100644
--- a/crates/malachite-app/src/validator_proof.rs
+++ b/crates/malachite-app/src/validator_proof.rs
@@ -25,7 +25,7 @@ use arc_consensus_types::codec::{network::NetCodec, Codec};
 use arc_consensus_types::signing::Signer;
 use arc_signer::ArcSigningProvider;
 use bytes::Bytes;
-use eyre::Result;
+use eyre::{Result, WrapErr};
 use tracing::info;
 
 /// Create a signed validator proof binding the consensus public key to the P2P peer ID.
@@ -50,7 +50,8 @@ pub async fn create_validator_proof(
     let proof = signing_provider
         .sign_validator_proof(public_key_bytes, peer_id_bytes)
         .await
-        .map_err(|e| eyre::eyre!("Failed to sign validator proof: {e}"))?;
+        .map_err(eyre::Report::new)
+        .wrap_err("Failed to sign validator proof")?;
 
     let proof_bytes = NetCodec
         .encode(&proof)
diff --git a/crates/remote-signer/src/client.rs b/crates/remote-signer/src/client.rs
index f9dbd0a7..18985d9c 100644
--- a/crates/remote-signer/src/client.rs
+++ b/crates/remote-signer/src/client.rs
@@ -209,6 +209,7 @@ impl RemoteSignerClient {
 
                 Err(RemoteSigningError::RetryExhausted {
                     retries: config.retry_config.max_retries,
+                    source: Box::new(e),
                 })
             }
         }
@@ -389,7 +390,7 @@ mod integration_tests {
                 "Signing should fail with bad endpoint"
             );
 
-            if let Err(RemoteSigningError::RetryExhausted { retries }) = sign_result {
+            if let Err(RemoteSigningError::RetryExhausted { retries, .. }) = sign_result {
                 assert_eq!(retries, 2, "Should have exhausted exactly 2 retries");
             }
         }
diff --git a/crates/remote-signer/src/error.rs b/crates/remote-signer/src/error.rs
index 7d9f7a28..b5faf4d1 100644
--- a/crates/remote-signer/src/error.rs
+++ b/crates/remote-signer/src/error.rs
@@ -37,7 +37,11 @@ pub enum RemoteSigningError {
     Timeout(String),
 
     #[error("Retry exhausted after {retries} attempts")]
-    RetryExhausted { retries: usize },
+    RetryExhausted {
+        retries: usize,
+        #[source]
+        source: Box<RemoteSigningError>,
+    },
 
     #[error("Configuration error: {0}")]
     Configuration(String),
@@ -45,3 +49,19 @@ pub enum RemoteSigningError {
     #[error("Service unavailable: {0}")]
     ServiceUnavailable(String),
 }
+
+impl RemoteSigningError {
+    /// Returns `true` if this error represents a transient startup failure that a
+    /// process restart is likely to recover from.
+    pub fn is_retryable_startup_error(&self) -> bool {
+        match self {
+            Self::Transport(_) | Self::ServiceUnavailable(_) | Self::Timeout(_) => true,
+            Self::Status(status) => matches!(
+                status.code(),
+                tonic::Code::Unavailable | tonic::Code::DeadlineExceeded
+            ),
+            Self::RetryExhausted { source, .. } => source.is_retryable_startup_error(),
+            Self::Authentication(_) | Self::Configuration(_) | Self::InvalidResponse(_) => false,
+        }
+    }
+}

[ZhiyuCircle]

Hi @tomasztrzos,

Thank you for your interest in contributing to Arc Node, and apologies for the delay in getting back to this PR.

We're closing out the pull request backlog that predates our current contribution policy. This PR is being closed because it does not reference a GitHub issue. All PRs must reference an existing issue using the format Closes: #XXX, and the author must be assigned to that issue before the PR is opened.

This is not a judgement on the change itself. If you'd still like to land it:

1. Open an issue describing the problem, or find the existing one
2. Comment on the issue requesting assignment, and wait for maintainer approval
3. Open a fresh PR once you have been assigned

Please see CONTRIBUTING.md for details. Thanks again for taking the time to contribute.