fix: type the unsatisfiable witness-provider-range failure

bin/debug-trace-server/src/data_provider.rs

@@ -46,7 +46,8 @@ use op_alloy_rpc_types::Transaction;
 use quick_cache::sync::Cache;
 use revm::state::Bytecode;
 use stateless_common::{
-    CodeFetchError, R2Band, RpcClient, RpcDeadlineExceeded, WitnessSizeBreakdown, r2_band,
+    CodeFetchError, R2Band, RpcClient, RpcDeadlineExceeded, WitnessFetchError,
+    WitnessSizeBreakdown, r2_band,
 };
 use stateless_core::{
     ContractStore, LightWitness, StoreResult, db::StoreError, withdrawals::MptWitness,
@@ -269,6 +270,18 @@ impl From<RpcDeadlineExceeded> for DataProviderError {
     }
 }
 
+impl From<WitnessFetchError> for DataProviderError {
+    fn from(e: WitnessFetchError) -> Self {
+        match e {
+            // Only a blown deadline is a timeout. A range failure is a wiring bug in this
+            // process — routing it to `Timeout { Witness }` would fire the `deadline_witness`
+            // alarm, which must mean "an upstream witness fetch ran out of budget".
+            WitnessFetchError::Deadline(d) => d.into(),
+            WitnessFetchError::NoProviderInRange { .. } => eyre::eyre!("{e}").into(),
+        }
+    }
+}
+
 impl From<CodeFetchError> for DataProviderError {
     fn from(e: CodeFetchError) -> Self {
         match e {
@@ -3154,4 +3167,26 @@ mod tests {
         .into();
         assert!(matches!(block_err, DataProviderError::Timeout { stage: TimeoutStage::Block, .. }));
     }
+
+    /// A witness fetch whose provider range is unsatisfiable is a wiring bug, not a blown
+    /// budget: it must land on `Internal`, never on `Timeout { Witness }`. That bucket feeds
+    /// the `deadline_witness` error reason, whose whole value is meaning "an upstream witness
+    /// fetch ran out of time" — a wiring bug landing there would page for the wrong incident.
+    /// The deadline variant still classifies by method, exactly as before.
+    #[test]
+    fn witness_range_failure_is_internal_not_a_witness_timeout() {
+        let range_err: DataProviderError =
+            WitnessFetchError::NoProviderInRange { skip: 2, configured: 1 }.into();
+        assert!(matches!(range_err, DataProviderError::Internal(_)), "got {range_err:?}");
+
+        let deadline_err: DataProviderError = WitnessFetchError::Deadline(RpcDeadlineExceeded {
+            method: stateless_common::RpcMethod::MegaGetBlockWitness,
+            elapsed: Duration::from_secs(3),
+        })
+        .into();
+        assert!(matches!(
+            deadline_err,
+            DataProviderError::Timeout { stage: TimeoutStage::Witness, .. }
+        ));
+    }
 }

crates/stateless-common/src/lib.rs

@@ -4,7 +4,7 @@ pub use metrics::{RpcMethod, RpcMetrics};
 pub mod rpc_client;
 pub use rpc_client::{
     CodeFetchError, RpcClient, RpcClientConfig, RpcDeadlineExceeded, SetValidatedBlocksResponse,
-    WitnessRequestKeys,
+    WitnessFetchError, WitnessRequestKeys,
 };
 /// Exponential-backoff policy used by [`RpcClient`]'s round-level retry loop: `initial` is the
 /// first sleep duration; each round doubles it up to `max`.

crates/stateless-common/src/rpc_client.rs

@@ -225,6 +225,23 @@ pub struct SetValidatedBlocksResponse {
     pub last_validated_block: (U64, B256),
 }
 
+/// Error returned by the witness fetches that take a caller-computed provider range.
+///
+/// `NoProviderInRange` is a wiring failure, not a transport one: the caller's `skip` selected
+/// past the configured witness endpoints. The constructor rejects an empty endpoint list, so
+/// reaching this means a routing bug in the skip computation rather than a misconfiguration.
+/// It is a typed error rather than an `assert!` so such a bug fails one request instead of the
+/// process.
+#[derive(Debug, thiserror::Error)]
+pub enum WitnessFetchError {
+    #[error(
+        "witness fetch selected providers {skip}.. of {configured} configured — no witness provider in range"
+    )]
+    NoProviderInRange { skip: usize, configured: usize },
+    #[error(transparent)]
+    Deadline(#[from] RpcDeadlineExceeded),
+}
+
 /// Errors returned by [`RpcClient::get_codes`] / [`RpcClient::get_codes_with_deadline`].
 ///
 /// - `VerificationFailure` is deterministic (upstream returned bytecode whose keccak does not match
@@ -291,7 +308,8 @@ impl RpcClient {
     /// # Arguments
     /// * `data_apis` - HTTP URLs of the standard JSON-RPC endpoints for blocks and contract data
     ///   (tried in order, non-empty)
-    /// * `witness_apis` - HTTP URLs of the witness RPC endpoints (tried in order, non-empty)
+    /// * `witness_apis` - HTTP URLs of the witness RPC endpoints (tried in order, non-empty). R2
+    ///   does not replace them: it is tried first and these remain the fallback
     /// * `config` - Configuration controlling verification, retry, and concurrency behavior
     /// * `report_api` - Optional HTTP URL of the endpoint for reporting validated blocks
     pub fn new_with_config(
@@ -713,7 +731,8 @@ impl RpcClient {
                 decode_witness_response,
                 "Witness decoded",
             )
-            .await?;
+            .await
+            .map_err(deadline_only)?;
 
         if let Some(ref metrics) = self.config.metrics {
             metrics.on_witness_fetch(WitnessSizeBreakdown::new(&witness.0, &witness.1));
@@ -744,7 +763,9 @@ impl RpcClient {
         hash: B256,
         deadline: Option<Instant>,
     ) -> std::result::Result<(LightWitness, MptWitness), RpcDeadlineExceeded> {
-        self.get_witness_light_with_deadline_from(0, number, hash, deadline).await
+        self.get_witness_light_with_deadline_from(0, number, hash, deadline)
+            .await
+            .map_err(deadline_only)
     }
 
     /// Like [`Self::get_witness_light_with_deadline`], but skips the first `skip` witness
@@ -753,15 +774,16 @@ impl RpcClient {
     /// position in the full configured witness endpoint list, and the shared witness
     /// concurrency cap still applies.
     ///
-    /// # Panics
-    /// Panics if `skip >= witness_provider_count()` — at least one provider must remain.
+    /// Returns [`WitnessFetchError::NoProviderInRange`] when `skip` selects past the
+    /// configured witness endpoints — a routing bug fails this one request rather than the
+    /// process.
     pub async fn get_witness_light_with_deadline_from(
         &self,
         skip: usize,
         number: u64,
         hash: B256,
         deadline: Option<Instant>,
-    ) -> std::result::Result<(LightWitness, MptWitness), RpcDeadlineExceeded> {
+    ) -> std::result::Result<(LightWitness, MptWitness), WitnessFetchError> {
         self.witness_round_robin(
             skip..self.witness_providers.len(),
             number,
@@ -796,7 +818,7 @@ impl RpcClient {
             "Witness light-decoded",
         )
         .await
-        .expect("None deadline cannot time out")
+        .expect("pinned 0..1 range and a None deadline cannot fail")
     }
 
     /// Shared `mega_getBlockWitness` retry loop: primary-failover rounds (always start from
@@ -808,8 +830,8 @@ impl RpcClient {
     /// the logged endpoint labels stay aligned with the full configured list because each
     /// label bakes in its original index (see [`endpoint_label`]).
     ///
-    /// # Panics
-    /// Panics if `providers` is empty or out of bounds — at least one provider must remain.
+    /// An empty or out-of-bounds `providers` range is a wiring failure, surfaced as
+    /// [`WitnessFetchError::NoProviderInRange`] rather than a panic.
     // A `warn`-level span (not the usual `info`) so it stays enabled at the default `warn` log
     // filter: the generic retry loop's per-attempt failure logs then inherit `block_number`,
     // which they cannot see otherwise, so an endpoint stall/error is traceable to its block.
@@ -822,12 +844,11 @@ impl RpcClient {
         deadline: Option<Instant>,
         decode: fn(&str) -> std::result::Result<T, crate::WitnessDecodingError>,
         trace_msg: &'static str,
-    ) -> std::result::Result<T, RpcDeadlineExceeded> {
-        assert!(
-            !providers.is_empty() && providers.end <= self.witness_providers.len(),
-            "witness provider range ({providers:?}) must select at least one of {} providers",
-            self.witness_providers.len()
-        );
+    ) -> std::result::Result<T, WitnessFetchError> {
+        let configured = self.witness_providers.len();
+        if providers.is_empty() || providers.end > configured {
+            return Err(WitnessFetchError::NoProviderInRange { skip: providers.start, configured });
+        }
         // Deadline-bound witness attempts run under the reserve-half policy: the tightest of
         // the configured ceiling, the general per-attempt timeout, and — recomputed at each
         // attempt, after any permit wait — half of what the call still has, so neither a
@@ -863,6 +884,7 @@ impl RpcClient {
             },
         )
         .await
+        .map_err(WitnessFetchError::Deadline)
     }
 
     /// Reports a range of validated blocks via the dedicated report endpoint.
@@ -1606,6 +1628,19 @@ async fn verify_block_on_blocking_pool(block: Block<Transaction>) -> Result<Bloc
     .context("block verification task panicked")?
 }
 
+/// Unwraps a [`WitnessFetchError`] from a full-range witness fetch, where
+/// [`WitnessFetchError::NoProviderInRange`] cannot occur: `0..len` is empty only when the
+/// client carries no witness providers at all, which both binaries reject at startup.
+fn deadline_only(e: WitnessFetchError) -> RpcDeadlineExceeded {
+    match e {
+        WitnessFetchError::Deadline(d) => d,
+        WitnessFetchError::NoProviderInRange { skip, configured } => unreachable!(
+            "full-range witness fetch on a client with no witness providers \
+             (skip={skip}, configured={configured})"
+        ),
+    }
+}
+
 /// Verifies structural integrity of a block fetched from RPC.
 ///
 /// Checks:
@@ -2113,6 +2148,21 @@ mod tests {
         hb.stop().unwrap();
     }
 
+    /// A `skip` past the configured witness endpoints is a wiring failure, and must fail this
+    /// one request rather than take the process down.
+    #[tokio::test]
+    async fn witness_fetch_out_of_range_returns_a_typed_error() {
+        let client = RpcClient::new(&[LOCALHOST_A], &[LOCALHOST_B]).unwrap();
+        let err = client
+            .get_witness_light_with_deadline_from(1, 7, B256::ZERO, None)
+            .await
+            .expect_err("skip == provider count leaves no provider");
+        assert!(
+            matches!(err, WitnessFetchError::NoProviderInRange { skip: 1, configured: 1 }),
+            "unexpected error: {err:?}"
+        );
+    }
+
     /// `get_witness` pins `rr_start = 0`, so every round visits the primary first and only
     /// falls through to the backup on failure. We can't easily make the primary succeed in
     /// a unit test (a valid witness payload needs real cryptographic proof material), but
@@ -2266,15 +2316,6 @@ mod tests {
         hc.stop().unwrap();
     }
 
-    /// Skipping every configured witness provider is a caller bug and must panic loudly
-    /// instead of silently retrying over an empty provider set.
-    #[tokio::test]
-    #[should_panic(expected = "must select at least one")]
-    async fn test_witness_fetch_skip_of_all_providers_panics() {
-        let client = RpcClient::new(&[LOCALHOST_A], &[LOCALHOST_B]).unwrap();
-        let _ = client.get_witness_light_with_deadline_from(1, 1, BlockHash::ZERO, None).await;
-    }
-
     /// Serves `mega_getBlockWitness` returning a stub that decodes-fails, while recording
     /// the provider's label to a shared `order` log on each hit. Used to verify call routing.
     async fn start_ordered_witness_rpc(