diff --git a/Cargo.lock b/Cargo.lock index 44078cabe1..f6c207f6b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3802,11 +3802,13 @@ dependencies = [ "foreign-chain-inspector", "foreign-chain-rpc-auth", "foreign-chain-rpc-interfaces", + "futures", "hex", "http", "httpmock", "mpc-node-config", "near-mpc-bounded-collections", + "near-mpc-contract-interface", "serde_json", "tokio", ] @@ -3859,6 +3861,7 @@ dependencies = [ "jsonrpsee", "mpc-primitives", "reqwest 0.13.4", + "rstest", "serde", "serde_json", "sui-rpc", diff --git a/crates/foreign-chain-health-check/Cargo.toml b/crates/foreign-chain-health-check/Cargo.toml index 1098570f6e..a1827e606c 100644 --- a/crates/foreign-chain-health-check/Cargo.toml +++ b/crates/foreign-chain-health-check/Cargo.toml @@ -14,15 +14,17 @@ clap = { workspace = true, optional = true } foreign-chain-inspector = { workspace = true } foreign-chain-rpc-auth = { workspace = true } foreign-chain-rpc-interfaces = { workspace = true } +futures = { workspace = true } hex = { workspace = true } http = { workspace = true } mpc-node-config = { workspace = true } +near-mpc-bounded-collections = { workspace = true } +near-mpc-contract-interface = { workspace = true } tokio = { workspace = true } [dev-dependencies] assert_matches = { workspace = true } httpmock = { workspace = true } -near-mpc-bounded-collections = { workspace = true } serde_json = { workspace = true } [lints] diff --git a/crates/foreign-chain-health-check/src/lib.rs b/crates/foreign-chain-health-check/src/lib.rs index dcb521b9c4..526568ae3e 100644 --- a/crates/foreign-chain-health-check/src/lib.rs +++ b/crates/foreign-chain-health-check/src/lib.rs @@ -1,10 +1,13 @@ -//! Foreign-chain RPC provider health checks: probe every configured provider -//! with a fixed golden request and report a per-provider result. Sui is the -//! exception — see `run_sui`. +//! Foreign-chain RPC provider health checks, in two independent routes: +//! +//! * [`check_all_providers`] replays a fixed golden transaction per chain. +//! * [`probe::probe_all_providers`] asks each provider for the network it serves and compares that +//! against the operator's configured expectation. mod checks; mod golden; mod network; +pub mod probe; mod results; use std::future::Future; @@ -35,6 +38,8 @@ use crate::golden::{AptosVector, BlockHashVector, SuiVector}; /// Chains with no reference for `network`, or configured but unsupported, are /// [`Status::Skipped`]; a chain absent from the config still yields a single /// placeholder `Skipped` result so its absence stays visible. +/// +/// TODO(#3969): retire this route in favour of [`probe::probe_all_providers`]. pub async fn check_all_providers( fc: &ForeignChainsConfig, network: Network, diff --git a/crates/foreign-chain-health-check/src/probe.rs b/crates/foreign-chain-health-check/src/probe.rs new file mode 100644 index 0000000000..348b05e5df --- /dev/null +++ b/crates/foreign-chain-health-check/src/probe.rs @@ -0,0 +1,780 @@ +//! Asks every configured RPC provider which network it serves and compares the answer against the +//! operator's `expected_network_fingerprint`. + +use std::collections::BTreeMap; +use std::time::Duration; + +use foreign_chain_inspector::starknet::inspector::StarknetInspector; +use foreign_chain_inspector::{ + FanOut, ForeignChainInspectionError, NetworkFingerprint, ProviderFailure, +}; +use mpc_node_config::{ForeignChainConfig, ForeignChainProviderConfig, ForeignChainsConfig}; +use near_mpc_bounded_collections::NonEmptyVec; +use near_mpc_contract_interface::types::{ForeignChain, ProviderId}; + +use crate::prepare_jsonrpc; + +/// One provider's verdict. Anything other than [`ProviderStatus::Healthy`] is unhealthy. +/// +/// Carries no rendered RPC error: `Path`/`Query` auth splices the operator's API key into the URL, +/// and upstream errors interpolate that URL into their text. Dropping the text here keeps the key +/// out of anything built from a report. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ProviderStatus { + Healthy, + WrongNetwork { + expected: NetworkFingerprint, + observed: NetworkFingerprint, + }, + /// DNS, TLS, connection refused, 5xx, or rate limiting. + Unreachable, + /// The provider answered and refused: credentials invalid, or not enabled for this chain. + RequestRejected, + MalformedResponse, + TimedOut, + /// The provider's auth token did not resolve, e.g. an environment variable that is not set. + AuthTokenUnresolved, + /// The RPC client could not be built from the provider's URL and auth. + ClientSetupFailed, + /// The chain is configured without an `expected_network_fingerprint`, so its providers cannot + /// be checked. Reported rather than skipped: silence would read as healthy. + MissingExpectedFingerprint, + /// The chain has no fingerprint probe yet, either because none is written for it or because the + /// node cannot inspect it at all. + ProbeNotImplemented, +} + +impl ProviderStatus { + pub fn is_healthy(&self) -> bool { + matches!(self, Self::Healthy) + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ProviderHealth { + pub chain: ForeignChain, + pub provider: ProviderId, + pub status: ProviderStatus, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub struct ProviderCounts { + pub configured: usize, + pub healthy: usize, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ProbeReport { + rows: Vec, +} + +impl ProbeReport { + pub fn rows(&self) -> &[ProviderHealth] { + &self.rows + } + + /// Only configured chains appear, never reports on a chain the operator did not configure. + pub fn counts_per_chain(&self) -> BTreeMap { + let mut counts: BTreeMap = BTreeMap::new(); + for row in &self.rows { + let entry = counts.entry(row.chain).or_default(); + entry.configured += 1; + if row.status.is_healthy() { + entry.healthy += 1; + } + } + counts + } +} + +/// Probe every configured provider concurrently. +/// +/// Each provider is tried up to `max_retries` times, `timeout_sec` per try, and only for as long as +/// the failures stay transient. This returns within the largest configured `timeout_sec * +/// max_retries`, plus the [`foreign_chain_inspector::RETRY_BACKOFF`] between tries. +pub async fn probe_all_providers(config: &ForeignChainsConfig) -> ProbeReport { + let probe_attempts = config + .iter_chains() + .map(|(chain, chain_config)| async move { + match chain { + ForeignChain::Starknet => { + probe_chain(chain, chain_config, |provider| { + Ok(StarknetInspector::new(prepare_jsonrpc(provider)?)) + }) + .await + } + // TODO(#4003): probe the remaining chains. + _ => rows_of(chain, chain_config, ProviderStatus::ProbeNotImplemented), + } + }); + + let report_rows = futures::future::join_all(probe_attempts).await.concat(); + ProbeReport { rows: report_rows } +} + +async fn probe_chain( + chain: ForeignChain, + config: &ForeignChainConfig, + new_inspector: impl Fn(&ForeignChainProviderConfig) -> anyhow::Result, +) -> Vec +where + I: foreign_chain_inspector::NetworkFingerprintInspector + Clone + Send + Sync + 'static, +{ + let Some(expected) = &config.expected_network_fingerprint else { + return rows_of(chain, config, ProviderStatus::MissingExpectedFingerprint); + }; + let expected = I::canonical_fingerprint(expected); + + let mut inspectors = Vec::new(); + let mut rows = Vec::new(); + for (name, provider) in config.providers.iter() { + let provider_id = ProviderId(name.as_str().to_owned()); + match new_inspector(provider) { + Ok(inspector) => inspectors.push((provider_id, inspector)), + Err(error) => rows.push(ProviderHealth { + chain, + provider: provider_id, + status: setup_failure(&error), + }), + } + } + + let Ok(inspectors) = NonEmptyVec::try_from(inspectors) else { + return rows; + }; + + let timeout = Duration::from_secs(config.timeout_sec.get()); + let fingerprints = FanOut::new(inspectors) + .network_fingerprints(timeout, config.max_retries) + .await; + for (provider, reported) in fingerprints { + rows.push(ProviderHealth { + chain, + provider, + status: classify(&expected, reported), + }); + } + rows +} + +/// [`ProviderStatus`] carries no error text, so the one actionable cause gets its own variant. Only +/// a token read from the environment can fail to resolve; a [`std::env::VarError`] identifies it. +fn setup_failure(error: &anyhow::Error) -> ProviderStatus { + if error.chain().any(|cause| cause.is::()) { + ProviderStatus::AuthTokenUnresolved + } else { + ProviderStatus::ClientSetupFailed + } +} + +fn rows_of( + chain: ForeignChain, + config: &ForeignChainConfig, + status: ProviderStatus, +) -> Vec { + config + .providers + .keys() + .map(|name| ProviderHealth { + chain, + provider: ProviderId(name.as_str().to_owned()), + status: status.clone(), + }) + .collect() +} + +/// A provider answers what it likes and the report reaches logs and metric labels, so the length is +/// capped well clear of the longest real fingerprint: Bitcoin's genesis hash, at 66 characters. +fn bounded(observed: NetworkFingerprint) -> NetworkFingerprint { + const MAX_CHARS: usize = 96; + + let observed = observed.to_string(); + match observed.char_indices().nth(MAX_CHARS) { + None => NetworkFingerprint::from(observed), + Some((cutoff, _)) => NetworkFingerprint::from(format!("{}…", &observed[..cutoff])), + } +} + +fn classify( + expected: &NetworkFingerprint, + reported: Result, +) -> ProviderStatus { + match reported { + Ok(observed) if &observed == expected => ProviderStatus::Healthy, + Ok(observed) => ProviderStatus::WrongNetwork { + expected: expected.clone(), + observed: bounded(observed), + }, + Err(error) => match error.provider_failure() { + Some(ProviderFailure::Unreachable) => ProviderStatus::Unreachable, + Some(ProviderFailure::Rejected) => ProviderStatus::RequestRejected, + Some(ProviderFailure::TimedOut) => ProviderStatus::TimedOut, + Some(ProviderFailure::Malformed) => ProviderStatus::MalformedResponse, + // Probing does not inspect transactions, so a transaction-level error means + // an impl answered outside its contract. + None => ProviderStatus::MalformedResponse, + }, + } +} + +#[cfg(test)] +#[expect(non_snake_case)] +mod tests { + use super::*; + use mpc_node_config::{AuthConfig, TokenConfig}; + use near_mpc_bounded_collections::NonEmptyBTreeMap; + use std::num::NonZeroU64; + + /// Starknet mainnet's chain id, `SN_MAIN` in ASCII. + const MAINNET: &str = "0x534e5f4d41494e"; + const SEPOLIA: &str = "0x534e5f5345504f4c4941"; + /// Reserved as "discard", so nothing listens there. + const CLOSED_PORT_URL: &str = "http://127.0.0.1:9"; + + fn provider(rpc_url: &str) -> ForeignChainProviderConfig { + ForeignChainProviderConfig { + rpc_url: rpc_url.to_string(), + auth: AuthConfig::None, + } + } + + fn chain_config( + expected: Option<&str>, + providers: NonEmptyBTreeMap< + mpc_node_config::foreign_chains::RpcProviderName, + ForeignChainProviderConfig, + >, + ) -> ForeignChainConfig { + ForeignChainConfig { + timeout_sec: NonZeroU64::new(1).unwrap(), + max_retries: NonZeroU64::new(1).unwrap(), + expected_network_fingerprint: expected.map(str::to_string), + providers, + } + } + + fn with_retries(config: ForeignChainConfig, max_retries: u64) -> ForeignChainConfig { + ForeignChainConfig { + max_retries: NonZeroU64::new(max_retries).unwrap(), + ..config + } + } + + fn one_provider( + name: &str, + rpc_url: &str, + ) -> NonEmptyBTreeMap< + mpc_node_config::foreign_chains::RpcProviderName, + ForeignChainProviderConfig, + > { + NonEmptyBTreeMap::new(name.to_string().into(), provider(rpc_url)) + } + + fn starknet_only(config: ForeignChainConfig) -> ForeignChainsConfig { + ForeignChainsConfig { + starknet: Some(config), + ..Default::default() + } + } + + async fn mock_chain_id<'a>( + server: &'a httpmock::MockServer, + chain_id: &str, + ) -> httpmock::Mock<'a> { + let body = serde_json::json!({"jsonrpc": "2.0", "result": chain_id, "id": 0}); + server + .mock_async(|when, then| { + when.method(httpmock::Method::POST); + then.status(200).json_body(body); + }) + .await + } + + /// Keyed by chain too: provider names repeat across chains in real configs. + fn must_status_of(report: &ProbeReport, chain: ForeignChain, provider: &str) -> ProviderStatus { + report + .rows() + .iter() + .find(|row| row.chain == chain && row.provider.0 == provider) + .unwrap_or_else(|| panic!("missing row for `{chain:?}` provider `{provider}`")) + .status + .clone() + } + + #[tokio::test] + async fn probe_all_providers__should_report_a_provider_on_the_expected_network_as_healthy() { + // Given + let server = httpmock::MockServer::start_async().await; + let mock = mock_chain_id(&server, MAINNET).await; + let config = starknet_only(chain_config( + Some(MAINNET), + one_provider("publicnode", &server.base_url()), + )); + + // When + let report = probe_all_providers(&config).await; + + // Then + mock.assert_async().await; + assert_eq!( + must_status_of(&report, ForeignChain::Starknet, "publicnode"), + ProviderStatus::Healthy + ); + } + + #[tokio::test] + async fn probe_all_providers__should_report_a_provider_on_another_network_as_wrong_network() { + // Given a provider serving Sepolia while the operator configured mainnet + let server = httpmock::MockServer::start_async().await; + mock_chain_id(&server, SEPOLIA).await; + let config = starknet_only(chain_config( + Some(MAINNET), + one_provider("publicnode", &server.base_url()), + )); + + // When + let report = probe_all_providers(&config).await; + + // Then + assert_eq!( + must_status_of(&report, ForeignChain::Starknet, "publicnode"), + ProviderStatus::WrongNetwork { + expected: NetworkFingerprint::from(MAINNET.to_string()), + observed: NetworkFingerprint::from(SEPOLIA.to_string()), + } + ); + } + + #[tokio::test] + async fn probe_all_providers__should_normalize_the_reported_fingerprint_before_comparing() { + // Given a provider padding and uppercasing the chain id, as the spec permits + let server = httpmock::MockServer::start_async().await; + mock_chain_id(&server, "0x00534E5F4D41494E").await; + let config = starknet_only(chain_config( + Some(MAINNET), + one_provider("publicnode", &server.base_url()), + )); + + // When + let report = probe_all_providers(&config).await; + + // Then + assert_eq!( + must_status_of(&report, ForeignChain::Starknet, "publicnode"), + ProviderStatus::Healthy + ); + } + + #[tokio::test] + async fn probe_all_providers__should_report_a_chain_without_an_expected_fingerprint_without_probing() + { + // Given + let server = httpmock::MockServer::start_async().await; + let mock = mock_chain_id(&server, MAINNET).await; + let config = starknet_only(chain_config( + None, + one_provider("publicnode", &server.base_url()), + )); + + // When + let report = probe_all_providers(&config).await; + + // Then the provider is reported rather than skipped, and no request was sent + assert_eq!( + must_status_of(&report, ForeignChain::Starknet, "publicnode"), + ProviderStatus::MissingExpectedFingerprint + ); + mock.assert_calls_async(0).await; + } + + #[tokio::test] + async fn probe_all_providers__should_report_an_unreachable_provider() { + // Given + let config = starknet_only(chain_config( + Some(MAINNET), + one_provider("publicnode", CLOSED_PORT_URL), + )); + + // When + let report = probe_all_providers(&config).await; + + // Then + assert_eq!( + must_status_of(&report, ForeignChain::Starknet, "publicnode"), + ProviderStatus::Unreachable + ); + } + + #[tokio::test] + async fn probe_all_providers__should_report_a_provider_refusing_the_request_without_retrying() { + // Given a provider answering the way an authenticated provider answers a bad API key + let server = httpmock::MockServer::start_async().await; + let mock = server + .mock_async(|when, then| { + when.method(httpmock::Method::POST); + then.status(401).json_body(serde_json::json!({ + "jsonrpc": "2.0", + "id": 0, + "error": {"code": -32600, "message": "Must be authenticated!"}, + })); + }) + .await; + let config = starknet_only(with_retries( + chain_config(Some(MAINNET), one_provider("keyed", &server.base_url())), + 3, + )); + + // When + let report = probe_all_providers(&config).await; + + // Then the refusal is named as such, and retrying it is pointless + assert_eq!( + must_status_of(&report, ForeignChain::Starknet, "keyed"), + ProviderStatus::RequestRejected + ); + mock.assert_calls_async(1).await; + } + + #[tokio::test] + async fn probe_all_providers__should_report_a_provider_answering_with_a_jsonrpc_error() { + // Given a provider that does not serve this chain's methods + let server = httpmock::MockServer::start_async().await; + server + .mock_async(|when, then| { + when.method(httpmock::Method::POST); + then.status(200).json_body(serde_json::json!({ + "jsonrpc": "2.0", + "id": 0, + "error": {"code": -32601, "message": "Method not found"}, + })); + }) + .await; + let config = starknet_only(chain_config( + Some(MAINNET), + one_provider("publicnode", &server.base_url()), + )); + + // When + let report = probe_all_providers(&config).await; + + // Then + assert_eq!( + must_status_of(&report, ForeignChain::Starknet, "publicnode"), + ProviderStatus::RequestRejected + ); + } + + #[tokio::test] + async fn probe_all_providers__should_report_a_provider_answering_with_an_unusable_body() { + // Given a provider whose answer is not JSON-RPC at all + let server = httpmock::MockServer::start_async().await; + server + .mock_async(|when, then| { + when.method(httpmock::Method::POST); + then.status(200).body("gateway"); + }) + .await; + let config = starknet_only(chain_config( + Some(MAINNET), + one_provider("publicnode", &server.base_url()), + )); + + // When + let report = probe_all_providers(&config).await; + + // Then + assert_eq!( + must_status_of(&report, ForeignChain::Starknet, "publicnode"), + ProviderStatus::MalformedResponse + ); + } + + #[tokio::test] + async fn probe_all_providers__should_report_a_provider_that_does_not_answer_in_time() { + // Given a provider slower than the configured timeout + let server = httpmock::MockServer::start_async().await; + let body = serde_json::json!({"jsonrpc": "2.0", "result": MAINNET, "id": 0}); + server + .mock_async(|when, then| { + when.method(httpmock::Method::POST); + then.status(200) + .json_body(body) + .delay(Duration::from_secs(30)); + }) + .await; + let config = starknet_only(chain_config( + Some(MAINNET), + one_provider("slow", &server.base_url()), + )); + + // When + let report = probe_all_providers(&config).await; + + // Then + assert_eq!( + must_status_of(&report, ForeignChain::Starknet, "slow"), + ProviderStatus::TimedOut + ); + } + + #[tokio::test] + async fn probe_all_providers__should_normalize_the_configured_fingerprint_before_comparing() { + // Given an operator writing the chain id padded and uppercased, as the spec permits + let server = httpmock::MockServer::start_async().await; + mock_chain_id(&server, MAINNET).await; + let config = starknet_only(chain_config( + Some("0x00534E5F4D41494E"), + one_provider("publicnode", &server.base_url()), + )); + + // When + let report = probe_all_providers(&config).await; + + // Then + assert_eq!( + must_status_of(&report, ForeignChain::Starknet, "publicnode"), + ProviderStatus::Healthy + ); + } + + #[tokio::test] + async fn probe_all_providers__should_report_a_provider_whose_auth_token_does_not_resolve() { + // Given a provider whose auth token comes from an environment variable that is not set + let config = starknet_only(chain_config( + Some(MAINNET), + NonEmptyBTreeMap::new( + "keyed".to_string().into(), + ForeignChainProviderConfig { + rpc_url: CLOSED_PORT_URL.to_string(), + auth: AuthConfig::Header { + name: http::HeaderName::from_static("authorization"), + scheme: Some("Bearer".to_string()), + token: TokenConfig::Env { + env: "PROBE_TEST_TOKEN_THAT_IS_NOT_SET".to_string(), + }, + }, + }, + ), + )); + + // When + let report = probe_all_providers(&config).await; + + // Then the operator learns which of the two setup failures it was + assert_eq!( + must_status_of(&report, ForeignChain::Starknet, "keyed"), + ProviderStatus::AuthTokenUnresolved + ); + } + + #[tokio::test] + async fn probe_all_providers__should_report_a_provider_whose_client_cannot_be_built() { + // Given a provider whose URL is not one a client can be built for + let config = starknet_only(chain_config( + Some(MAINNET), + one_provider("wrong-scheme", "ws://127.0.0.1:9"), + )); + + // When + let report = probe_all_providers(&config).await; + + // Then + assert_eq!( + must_status_of(&report, ForeignChain::Starknet, "wrong-scheme"), + ProviderStatus::ClientSetupFailed + ); + } + + #[tokio::test] + async fn probe_all_providers__should_report_each_provider_of_a_chain_separately() { + // Given one healthy provider and one that is unreachable + let server = httpmock::MockServer::start_async().await; + mock_chain_id(&server, MAINNET).await; + let mut providers = one_provider("healthy", &server.base_url()); + providers.insert("broken".to_string().into(), provider(CLOSED_PORT_URL)); + let config = starknet_only(chain_config(Some(MAINNET), providers)); + + // When + let report = probe_all_providers(&config).await; + + // Then + assert_eq!( + must_status_of(&report, ForeignChain::Starknet, "healthy"), + ProviderStatus::Healthy + ); + assert_eq!( + must_status_of(&report, ForeignChain::Starknet, "broken"), + ProviderStatus::Unreachable + ); + assert_eq!( + report.counts_per_chain()[&ForeignChain::Starknet], + ProviderCounts { + configured: 2, + healthy: 1, + } + ); + } + + #[tokio::test] + async fn probe_all_providers__should_report_a_chain_with_no_fingerprint_probe_as_not_implemented() + { + // Given a chain the node can inspect but has no fingerprint probe for + let config = ForeignChainsConfig { + base: Some(chain_config( + Some("8453"), + one_provider("publicnode", CLOSED_PORT_URL), + )), + ..Default::default() + }; + + // When + let report = probe_all_providers(&config).await; + + // Then it is visible in the report rather than silently absent + assert_eq!( + must_status_of(&report, ForeignChain::Base, "publicnode"), + ProviderStatus::ProbeNotImplemented + ); + } + + #[tokio::test] + async fn probe_all_providers__should_report_every_configured_chain_under_its_own_chain() { + // Given the same provider name configured for two chains + let server = httpmock::MockServer::start_async().await; + mock_chain_id(&server, MAINNET).await; + let config = ForeignChainsConfig { + starknet: Some(chain_config( + Some(MAINNET), + one_provider("publicnode", &server.base_url()), + )), + base: Some(chain_config( + Some("8453"), + one_provider("publicnode", CLOSED_PORT_URL), + )), + ..Default::default() + }; + + // When + let report = probe_all_providers(&config).await; + + // Then each chain gets its own row rather than one shadowing the other + assert_eq!( + must_status_of(&report, ForeignChain::Starknet, "publicnode"), + ProviderStatus::Healthy + ); + assert_eq!( + must_status_of(&report, ForeignChain::Base, "publicnode"), + ProviderStatus::ProbeNotImplemented + ); + assert_eq!(report.counts_per_chain().len(), 2); + } + + #[tokio::test] + async fn probe_all_providers__should_retry_a_provider_that_refused_with_a_rate_limit_code() { + // Given a provider signalling throttling as a JSON-RPC error object over HTTP 200 + let server = httpmock::MockServer::start_async().await; + let mock = server + .mock_async(|when, then| { + when.method(httpmock::Method::POST); + then.status(200).json_body(serde_json::json!({ + "jsonrpc": "2.0", + "id": 0, + "error": {"code": -32005, "message": "limit exceeded"}, + })); + }) + .await; + let config = starknet_only(with_retries( + chain_config(Some(MAINNET), one_provider("keyed", &server.base_url())), + 2, + )); + + // When + let report = probe_all_providers(&config).await; + + // Then throttling is the one refusal worth retrying + assert_eq!( + must_status_of(&report, ForeignChain::Starknet, "keyed"), + ProviderStatus::Unreachable + ); + mock.assert_calls_async(2).await; + } + + #[tokio::test] + async fn probe_all_providers__should_bound_the_fingerprint_a_provider_reports() { + // Given a provider answering with far more than a fingerprint + let server = httpmock::MockServer::start_async().await; + let flood = "n".repeat(5_000); + mock_chain_id(&server, &flood).await; + let config = starknet_only(chain_config( + Some(MAINNET), + one_provider("publicnode", &server.base_url()), + )); + + // When + let report = probe_all_providers(&config).await; + + // Then what reaches a log line or a metric label is bounded + let ProviderStatus::WrongNetwork { observed, .. } = + must_status_of(&report, ForeignChain::Starknet, "publicnode") + else { + panic!("expected the flood to read as the wrong network"); + }; + assert!(observed.to_string().chars().count() < 100); + } + + #[test] + fn classify__should_report_a_transaction_level_error_as_malformed() { + // Given an error about a transaction, which the probe never asks about + let expected = NetworkFingerprint::from(MAINNET.to_string()); + + // When + let status = classify( + &expected, + Err(ForeignChainInspectionError::TransactionNotFound), + ); + + // Then the inspector answered outside the contract its probe has + assert_eq!(status, ProviderStatus::MalformedResponse); + } + + #[tokio::test] + async fn probe_all_providers__should_return_an_empty_report_when_no_chains_are_configured() { + // Given + let config = ForeignChainsConfig::default(); + + // When + let report = probe_all_providers(&config).await; + + // Then + assert!(report.rows().is_empty()); + assert!(report.counts_per_chain().is_empty()); + } + + #[tokio::test] + async fn probe_all_providers__should_keep_auth_material_out_of_the_report() { + // Given a provider whose API key is spliced into the URL path + let config = starknet_only(chain_config( + Some(MAINNET), + NonEmptyBTreeMap::new( + "keyed".to_string().into(), + ForeignChainProviderConfig { + rpc_url: format!("{CLOSED_PORT_URL}/v2/API_KEY"), + auth: AuthConfig::Path { + placeholder: "API_KEY".to_string(), + token: TokenConfig::Val { + val: "super-secret".to_string(), + }, + }, + }, + ), + )); + + // When + let report = probe_all_providers(&config).await; + + // Then neither the token nor the URL it was spliced into reaches the report + let rendered = format!("{report:?}"); + assert!(!rendered.contains("super-secret"), "{rendered}"); + assert!(!rendered.contains("127.0.0.1"), "{rendered}"); + } +} diff --git a/crates/foreign-chain-inspector/src/lib.rs b/crates/foreign-chain-inspector/src/lib.rs index 26c60fef74..690eec3e58 100644 --- a/crates/foreign-chain-inspector/src/lib.rs +++ b/crates/foreign-chain-inspector/src/lib.rs @@ -1,10 +1,16 @@ use std::hash::Hash; +use std::num::NonZeroU64; +use std::time::Duration; use derive_more::{Deref, Display, From}; use ethereum_types::H256; use http::{HeaderMap, HeaderName, HeaderValue}; +use jsonrpsee::core::client::error::Error as RpcClientError; +use jsonrpsee::core::http_helpers::HttpError; +use jsonrpsee::http_client::transport::Error as HttpTransportError; use jsonrpsee::http_client::{HttpClient, HttpClientBuilder}; use near_mpc_bounded_collections::NonEmptyVec; +use near_mpc_contract_interface::types::ProviderId; use thiserror::Error; pub use jsonrpsee::http_client; @@ -35,6 +41,25 @@ pub trait ForeignChainInspector { ) -> impl Future, ForeignChainInspectionError>> + Send; } +/// The network a provider serves, as the chain itself reports it: a chain id or a genesis hash, in +/// one canonical text form per chain. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Display, From)] +pub struct NetworkFingerprint(String); + +/// Reports the [`NetworkFingerprint`] of the provider an inspector talks to. +/// +/// Fingerprints are compared verbatim, so both the reported and the expected one go through the +/// impl's canonical form. Fetches a chain-wide constant providers never prune. +pub trait NetworkFingerprintInspector { + fn network_fingerprint( + &self, + ) -> impl Future> + Send; + + /// Puts an operator-supplied fingerprint into the form [`Self::network_fingerprint`] returns, + /// so that a spec-legal spelling of the right network does not read as the wrong network. + fn canonical_fingerprint(expected: &str) -> NetworkFingerprint; +} + /// Combines multiple inspectors that target the same chain into a single inspector. /// /// All inner inspectors are queried concurrently. The fan-out treats every @@ -58,7 +83,7 @@ pub trait ForeignChainInspector { /// inner fields differ. #[derive(Clone, derive_more::Constructor)] pub struct FanOut { - inspectors: NonEmptyVec, + inspectors: NonEmptyVec<(ProviderId, Inspector)>, } impl ForeignChainInspector for FanOut @@ -81,25 +106,30 @@ where extractors: Vec, ) -> Result, ForeignChainInspectionError> { let mut join_set = tokio::task::JoinSet::new(); - for (idx, inspector) in self.inspectors.iter().enumerate() { + for (provider, inspector) in self.inspectors.iter() { let tx_id = tx_id.clone(); let finality = finality.clone(); let extractors = extractors.clone(); let inspector = inspector.clone(); - join_set - .spawn(async move { (idx, inspector.extract(tx_id, finality, extractors).await) }); + let provider = provider.clone(); + join_set.spawn(async move { + ( + provider, + inspector.extract(tx_id, finality, extractors).await, + ) + }); } - let mut successes: Vec<(usize, Vec)> = Vec::new(); - let mut non_transient_errors: Vec<(usize, ForeignChainInspectionError)> = Vec::new(); + let mut successes: Vec<(ProviderId, Vec)> = Vec::new(); + let mut non_transient_errors: Vec<(ProviderId, ForeignChainInspectionError)> = Vec::new(); let mut first_transient_error: Option = None; - for (idx, result) in join_set.join_all().await { + for (provider, result) in join_set.join_all().await { match result { - Ok(values) => successes.push((idx, values)), + Ok(values) => successes.push((provider, values)), Err(err) if err.is_transient() => { tracing::warn!( - inspector_index = idx, + %provider, error = %err, "fan-out inspector failed (transient)", ); @@ -107,11 +137,11 @@ where } Err(err) => { tracing::error!( - inspector_index = idx, + %provider, error = %err, "fan-out inspector failed (non-transient)", ); - non_transient_errors.push((idx, err)); + non_transient_errors.push((provider, err)); } } } @@ -168,6 +198,50 @@ where } } +/// Pause between two tries at the same provider. +pub const RETRY_BACKOFF: Duration = Duration::from_millis(200); + +impl FanOut +where + Inspector: NetworkFingerprintInspector + Clone + Send + Sync + 'static, +{ + /// Ask every provider for the network it serves concurrently, one result each. + /// Unlike [`FanOut::extract`], disagreement is not an error: a diagnostic caller needs the + /// individual answers. Each provider gets up to `attempts` tries, `timeout` per try plus + /// [`RETRY_BACKOFF`] between them, and only a transient failure is retried. + pub async fn network_fingerprints( + &self, + timeout: Duration, + attempts: NonZeroU64, + ) -> Vec<( + ProviderId, + Result, + )> { + let mut join_set = tokio::task::JoinSet::new(); + for (provider, inspector) in self.inspectors.iter() { + let inspector = inspector.clone(); + let provider = provider.clone(); + join_set.spawn(async move { + let ask = || async { + tokio::time::timeout(timeout, inspector.network_fingerprint()) + .await + .unwrap_or(Err(ForeignChainInspectionError::Timeout)) + }; + let mut outcome = ask().await; + for _ in 1..attempts.get() { + if !matches!(&outcome, Err(err) if err.is_transient()) { + break; + } + tokio::time::sleep(RETRY_BACKOFF).await; + outcome = ask().await; + } + (provider, outcome) + }); + } + join_set.join_all().await + } +} + #[derive(Debug, Clone)] pub enum RpcAuthentication { /// The key is in the URL (e.g., Alchemy, QuickNode). @@ -218,6 +292,8 @@ pub enum ForeignChainInspectionError { /// Transient provider failure (transport error, timeout, rate limit, 5xx). #[error("RPC request failed: {0}")] RpcRequestFailed(String), + #[error("RPC request did not complete within the configured timeout")] + Timeout, /// The provider rejected the request with a deterministic client error (4xx other than /// 408/429); retrying cannot change the outcome. #[error("RPC rejected the request: {0}")] @@ -281,10 +357,104 @@ impl ForeignChainInspectionError { self, Self::ClientError(_) | Self::RpcRequestFailed(_) + | Self::Timeout | Self::NotFinalized | Self::NotEnoughBlockConfirmations { .. } ) } + + /// Splits by what the provider did, where the [`From`] impl collapses everything into the one + /// [`Self::ClientError`] that [`Self::is_transient`] retries wholesale. A caller reporting why a + /// provider is unusable needs a 401 told apart from a 429. + /// + /// Messages name the HTTP status or JSON-RPC code, never the URL. + pub fn classify_rpc_client_error(error: RpcClientError) -> Self { + match error { + RpcClientError::Call(object) => { + let code = object.code(); + let message = format!("JSON-RPC error code {code}"); + if is_rate_limit_error_code(code) { + Self::RpcRequestFailed(message) + } else { + Self::RpcRequestRejected(message) + } + } + RpcClientError::ParseError(error) => Self::MalformedRpcResponse(error.to_string()), + RpcClientError::RequestTimeout => Self::Timeout, + RpcClientError::Transport(error) => { + match error.downcast_ref::() { + Some(HttpTransportError::Rejected { status_code }) => { + let status = format!("HTTP status {status_code}"); + if is_retryable_status(*status_code) { + Self::RpcRequestFailed(status) + } else { + Self::RpcRequestRejected(status) + } + } + // Not a response the caller can use, as opposed to no response at all. + Some(HttpTransportError::Http(HttpError::Malformed | HttpError::TooLarge)) => { + Self::MalformedRpcResponse("response was not valid JSON-RPC".to_string()) + } + Some(HttpTransportError::Url(_)) => { + Self::RpcRequestRejected("invalid RPC URL".to_string()) + } + _ => Self::RpcRequestFailed("transport failure".to_string()), + } + } + other => Self::RpcRequestFailed(other.to_string()), + } + } + + /// How the provider failed, or [`None`] when the provider did not: the remaining variants + /// report the transaction's own state, which is an answer rather than a fault. + pub fn provider_failure(&self) -> Option { + match self { + Self::ClientError(_) | Self::RpcRequestFailed(_) => Some(ProviderFailure::Unreachable), + Self::Timeout => Some(ProviderFailure::TimedOut), + Self::RpcRequestRejected(_) => Some(ProviderFailure::Rejected), + Self::MalformedRpcResponse(_) + | Self::InconsistentRpcResponse { .. } + | Self::LogNotBoundToReceipt { .. } + | Self::EventLogFailedBorshSerialization(_) + | Self::InspectorResponseMismatch => Some(ProviderFailure::Malformed), + Self::NotFinalized + | Self::NotEnoughBlockConfirmations { .. } + | Self::NonCanonicalBlock { .. } + | Self::TransactionFailed + | Self::TransactionNotFound + | Self::LogIndexOutOfBounds => None, + } + } +} + +/// Some providers report throttling as a JSON-RPC error object over HTTP 200 rather than a 429, and +/// it is the one refusal worth retrying. Alchemy and Infura send `-32005`, others `-32029`. +fn is_rate_limit_error_code(code: i32) -> bool { + const LIMIT_EXCEEDED: i32 = -32005; + const TOO_MANY_REQUESTS: i32 = -32029; + + matches!(code, LIMIT_EXCEEDED | TOO_MANY_REQUESTS) +} + +fn is_retryable_status(status_code: u16) -> bool { + const REQUEST_TIMEOUT: u16 = 408; + const TOO_MANY_REQUESTS: u16 = 429; + const SERVER_ERROR: u16 = 500; + + matches!(status_code, REQUEST_TIMEOUT | TOO_MANY_REQUESTS) || status_code >= SERVER_ERROR +} + +/// Groups the ways a provider itself can fail, for callers that report an outcome rather than act +/// on it. Says nothing about retryability: see [`ForeignChainInspectionError::is_transient`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum ProviderFailure { + /// No answer arrived: transport failure, 5xx, or rate limiting. + Unreachable, + /// The provider answered and refused. Retrying cannot change it. + Rejected, + /// The provider answered with something the caller could not use. + Malformed, + TimedOut, } /// Builds an HTTP client with the specified authentication method. diff --git a/crates/foreign-chain-inspector/src/starknet/inspector.rs b/crates/foreign-chain-inspector/src/starknet/inspector.rs index 4f33844852..58c7bf3e83 100644 --- a/crates/foreign-chain-inspector/src/starknet/inspector.rs +++ b/crates/foreign-chain-inspector/src/starknet/inspector.rs @@ -1,14 +1,21 @@ use crate::starknet::{StarknetExtractedValue, StarknetTransactionHash}; -use crate::{ForeignChainInspectionError, ForeignChainInspector}; +use crate::{ + ForeignChainInspectionError, ForeignChainInspector, NetworkFingerprint, + NetworkFingerprintInspector, +}; use foreign_chain_rpc_interfaces::starknet::{ - BlockId, GetBlockWithTxHashesArgs, GetBlockWithTxHashesResponse, GetTransactionReceiptArgs, - GetTransactionReceiptResponse, H256, StarknetExecutionStatus, StarknetFinalityStatus, + BlockId, ChainIdResponse, GetBlockWithTxHashesArgs, GetBlockWithTxHashesResponse, + GetTransactionReceiptArgs, GetTransactionReceiptResponse, H256, StarknetExecutionStatus, + StarknetFinalityStatus, }; use jsonrpsee::core::client::ClientT; use near_mpc_contract_interface::types::{StarknetFelt, StarknetLog}; const GET_TRANSACTION_RECEIPT_METHOD: &str = "starknet_getTransactionReceipt"; const GET_BLOCK_WITH_TX_HASHES_METHOD: &str = "starknet_getBlockWithTxHashes"; +const CHAIN_ID_METHOD: &str = "starknet_chainId"; +/// `starknet_chainId` takes no arguments. Sent as an explicit empty array. +const NO_PARAMS: [(); 0] = []; #[derive(Clone)] pub struct StarknetInspector { @@ -21,6 +28,24 @@ pub enum StarknetFinality { AcceptedOnL1, } +impl NetworkFingerprintInspector for StarknetInspector +where + Client: ClientT + Send + Sync, +{ + async fn network_fingerprint(&self) -> Result { + let chain_id: ChainIdResponse = self + .client + .request(CHAIN_ID_METHOD, NO_PARAMS) + .await + .map_err(ForeignChainInspectionError::classify_rpc_client_error)?; + Ok(chain_id.canonical_text().into()) + } + + fn canonical_fingerprint(expected: &str) -> NetworkFingerprint { + ChainIdResponse(expected.to_owned()).canonical_text().into() + } +} + impl ForeignChainInspector for StarknetInspector where Client: ClientT + Send + Sync, diff --git a/crates/foreign-chain-inspector/tests/common.rs b/crates/foreign-chain-inspector/tests/common.rs index b43d94f836..a66d870bdc 100644 --- a/crates/foreign-chain-inspector/tests/common.rs +++ b/crates/foreign-chain-inspector/tests/common.rs @@ -11,6 +11,7 @@ use serde::{Deserialize, Serialize}; /// Useful for tests. /// Note: We have to hold a closure and not just the response /// because `RpcClientError` does not implement `Clone`. +#[derive(Clone)] pub struct FixedResponseRpcClient { response_fn: RespFn, } diff --git a/crates/foreign-chain-inspector/tests/fanout.rs b/crates/foreign-chain-inspector/tests/fanout.rs index 0a237d721f..24526393a8 100644 --- a/crates/foreign-chain-inspector/tests/fanout.rs +++ b/crates/foreign-chain-inspector/tests/fanout.rs @@ -15,6 +15,7 @@ use foreign_chain_inspector::{ BlockConfirmations, FanOut, ForeignChainInspectionError, ForeignChainInspector, }; use near_mpc_bounded_collections::NonEmptyVec; +use near_mpc_contract_interface::types::ProviderId; mockall::mock! { Inspector {} @@ -93,7 +94,12 @@ fn err(make: impl Fn() -> ForeignChainInspectionError + Send + Sync + 'static) - } fn fan_out_of(inspectors: Vec) -> FanOut { - let inspectors: NonEmptyVec = inspectors + let named: Vec<(ProviderId, MockInspector)> = inspectors + .into_iter() + .enumerate() + .map(|(index, inspector)| (ProviderId(format!("provider-{index}")), inspector)) + .collect(); + let inspectors: NonEmptyVec<(ProviderId, MockInspector)> = named .try_into() .expect("test must provide at least one inspector"); FanOut::new(inspectors) diff --git a/crates/foreign-chain-inspector/tests/rpc_error_classification.rs b/crates/foreign-chain-inspector/tests/rpc_error_classification.rs new file mode 100644 index 0000000000..c9b348a707 --- /dev/null +++ b/crates/foreign-chain-inspector/tests/rpc_error_classification.rs @@ -0,0 +1,207 @@ +#![allow(non_snake_case)] + +//! Integration tests for [`ForeignChainInspectionError::classify_rpc_client_error`], which decides +//! whether a provider failed to answer, answered and refused, or answered unusably. + +use assert_matches::assert_matches; +use foreign_chain_inspector::{BlockConfirmations, ForeignChainInspectionError, ProviderFailure}; +use jsonrpsee::core::client::error::Error as RpcClientError; +use jsonrpsee::core::http_helpers::HttpError; +use jsonrpsee::http_client::transport::Error as TransportError; +use rstest::rstest; + +fn transport(error: TransportError) -> RpcClientError { + RpcClientError::Transport(Box::new(error)) +} + +#[rstest] +#[case(400)] +#[case(401)] +#[case(403)] +#[case(404)] +fn classify_rpc_client_error__should_report_a_deterministic_status_as_a_refusal( + #[case] status_code: u16, +) { + // Given + let error = transport(TransportError::Rejected { status_code }); + + // When + let classified = ForeignChainInspectionError::classify_rpc_client_error(error); + + // Then + assert_matches!( + &classified, + ForeignChainInspectionError::RpcRequestRejected(_) + ); + assert!(!classified.is_transient()); +} + +#[rstest] +#[case(408)] +#[case(429)] +#[case(500)] +#[case(503)] +fn classify_rpc_client_error__should_report_a_retryable_status_as_a_transient_failure( + #[case] status_code: u16, +) { + // Given + let error = transport(TransportError::Rejected { status_code }); + + // When + let classified = ForeignChainInspectionError::classify_rpc_client_error(error); + + // Then + assert_matches!( + &classified, + ForeignChainInspectionError::RpcRequestFailed(_) + ); + assert!(classified.is_transient()); +} + +#[test] +fn classify_rpc_client_error__should_report_a_jsonrpc_error_object_as_a_refusal() { + // Given the answer an authenticated provider gives to a request it will not serve + let error = RpcClientError::Call(jsonrpsee::types::ErrorObject::owned( + -32600, + "Must be authenticated!", + None::<()>, + )); + + // When + let classified = ForeignChainInspectionError::classify_rpc_client_error(error); + + // Then + assert_matches!( + &classified, + ForeignChainInspectionError::RpcRequestRejected(_) + ); + assert!(!classified.is_transient()); +} + +#[test] +fn classify_rpc_client_error__should_report_an_unparseable_result_as_malformed() { + // Given + let parse_error = serde_json::from_str::("7").expect_err("a number is not a string"); + let error = RpcClientError::ParseError(parse_error); + + // When + let classified = ForeignChainInspectionError::classify_rpc_client_error(error); + + // Then + assert_matches!( + classified, + ForeignChainInspectionError::MalformedRpcResponse(_) + ); +} + +#[test] +fn classify_rpc_client_error__should_report_a_body_that_is_not_jsonrpc_as_malformed() { + // Given + let error = transport(TransportError::Http(HttpError::Malformed)); + + // When + let classified = ForeignChainInspectionError::classify_rpc_client_error(error); + + // Then + assert_matches!( + classified, + ForeignChainInspectionError::MalformedRpcResponse(_) + ); +} + +#[test] +fn classify_rpc_client_error__should_report_a_connection_failure_as_transient() { + // Given + let error = transport(TransportError::Http(HttpError::Stream(Box::new( + std::io::Error::new(std::io::ErrorKind::ConnectionRefused, "connection refused"), + )))); + + // When + let classified = ForeignChainInspectionError::classify_rpc_client_error(error); + + // Then + assert_matches!( + &classified, + ForeignChainInspectionError::RpcRequestFailed(_) + ); + assert!(classified.is_transient()); +} + +#[test] +fn classify_rpc_client_error__should_keep_the_rpc_url_out_of_the_message() { + // Given a URL a client cannot be built for, as jsonrpsee reports it: with the URL in the text + let error = transport(TransportError::Url( + "http://provider.example/v2/super-secret".to_string(), + )); + + // When + let classified = ForeignChainInspectionError::classify_rpc_client_error(error); + + // Then the API key spliced into the URL by `Path`/`Query` auth cannot travel with the error + let rendered = classified.to_string(); + assert!(!rendered.contains("super-secret"), "{rendered}"); +} + +#[test] +fn classify_rpc_client_error__should_report_a_client_side_timeout_as_a_timeout() { + // Given + let error = RpcClientError::RequestTimeout; + + // When + let classified = ForeignChainInspectionError::classify_rpc_client_error(error); + + // Then + assert_matches!(classified, ForeignChainInspectionError::Timeout); +} + +#[rstest] +#[case(-32005)] +#[case(-32029)] +fn classify_rpc_client_error__should_report_a_rate_limit_code_as_a_transient_failure( + #[case] code: i32, +) { + // Given a provider signalling throttling in the error object rather than with a 429 + let error = RpcClientError::Call(jsonrpsee::types::ErrorObject::owned( + code, + "limit exceeded", + None::<()>, + )); + + // When + let classified = ForeignChainInspectionError::classify_rpc_client_error(error); + + // Then + assert_matches!( + &classified, + ForeignChainInspectionError::RpcRequestFailed(_) + ); + assert!(classified.is_transient()); +} + +#[rstest] +#[case(ForeignChainInspectionError::RpcRequestFailed("_".to_string()), Some(ProviderFailure::Unreachable))] +#[case(ForeignChainInspectionError::RpcRequestRejected("_".to_string()), Some(ProviderFailure::Rejected))] +#[case(ForeignChainInspectionError::Timeout, Some(ProviderFailure::TimedOut))] +#[case(ForeignChainInspectionError::MalformedRpcResponse("_".to_string()), Some(ProviderFailure::Malformed))] +#[case( + ForeignChainInspectionError::InspectorResponseMismatch, + Some(ProviderFailure::Malformed) +)] +// The transaction's own state is an answer, not a fault of the provider that reported it. +#[case(ForeignChainInspectionError::TransactionNotFound, None)] +#[case(ForeignChainInspectionError::TransactionFailed, None)] +#[case(ForeignChainInspectionError::NotFinalized, None)] +#[case(ForeignChainInspectionError::NotEnoughBlockConfirmations { + expected: BlockConfirmations::from(6), + got: BlockConfirmations::from(1), +}, None)] +fn provider_failure__should_name_only_the_failures_the_provider_owns( + #[case] error: ForeignChainInspectionError, + #[case] expected: Option, +) { + // When + let failure = error.provider_failure(); + + // Then + assert_eq!(failure, expected); +} diff --git a/crates/foreign-chain-inspector/tests/starknet_inspector.rs b/crates/foreign-chain-inspector/tests/starknet_inspector.rs index 80caef9552..acf18579af 100644 --- a/crates/foreign-chain-inspector/tests/starknet_inspector.rs +++ b/crates/foreign-chain-inspector/tests/starknet_inspector.rs @@ -7,7 +7,8 @@ use crate::common::{ }; use foreign_chain_inspector::{ - ForeignChainInspectionError, ForeignChainInspector, RpcAuthentication, build_http_client, + FanOut, ForeignChainInspectionError, ForeignChainInspector, NetworkFingerprintInspector, + RpcAuthentication, build_http_client, starknet::{ StarknetBlockHash, StarknetExtractedValue, StarknetTransactionHash, inspector::{StarknetExtractor, StarknetFinality, StarknetInspector}, @@ -22,8 +23,14 @@ use foreign_chain_rpc_interfaces::starknet::{ use httpmock::prelude::*; use httpmock::{HttpMockRequest, HttpMockResponse}; use jsonrpsee::core::client::error::Error as RpcClientError; +use near_mpc_bounded_collections::NonEmptyVec; +use near_mpc_contract_interface::types::ProviderId; use near_mpc_contract_interface::types::{StarknetFelt, StarknetLog}; use rstest::rstest; +use std::num::NonZeroU64; +use std::sync::Arc; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::time::Duration; fn mock_receipt( finality_status: StarknetFinalityStatus, @@ -507,3 +514,145 @@ async fn extract__should_return_event_log_for_specific_index_via_http_rpc_client extracted_values, ); } + +/// Starknet mainnet's chain id, `SN_MAIN` in ASCII. +const MAINNET_CHAIN_ID: &str = "0x534e5f4d41494e"; + +#[tokio::test] +async fn network_fingerprint__should_return_the_canonical_chain_id() { + // Given: the chain id padded and uppercased, as a provider is free to send it. + let inspector = StarknetInspector::new(mock_client_from_fixed_response("0x00534E5F4D41494E")); + + // When + let fingerprint = inspector + .network_fingerprint() + .await + .expect("network_fingerprint should succeed"); + + // Then + assert_eq!(fingerprint.to_string(), MAINNET_CHAIN_ID); +} + +/// Builds a fan-out of one Starknet provider whose client runs `respond` on each call, and reports +/// the number of calls it received. +#[expect( + clippy::type_complexity, + reason = "the client holds an unnameable closure type, so the tuple has to spell it out" +)] +fn single_provider_fan_out( + respond: impl Fn(usize) -> Result + Send + Sync + 'static, +) -> ( + FanOut< + StarknetInspector< + FixedResponseRpcClient< + impl Fn() -> Result + Clone + Sync, + >, + >, + >, + Arc, +) { + let calls = Arc::new(AtomicUsize::new(0)); + let seen = Arc::clone(&calls); + let respond: Arc Result + Send + Sync> = + Arc::new(respond); + let client = FixedResponseRpcClient::new(move || respond(seen.fetch_add(1, Ordering::SeqCst))); + let inspectors: NonEmptyVec<_> = vec![( + ProviderId("only".to_string()), + StarknetInspector::new(client), + )] + .try_into() + .expect("one inspector"); + (FanOut::new(inspectors), calls) +} + +fn transport_error() -> RpcClientError { + RpcClientError::Transport(Box::new(std::io::Error::new( + std::io::ErrorKind::ConnectionRefused, + "connection refused", + ))) +} + +#[tokio::test] +async fn network_fingerprints__should_retry_a_transient_failure_and_report_the_later_success() { + // Given a provider that refuses the first call and answers the second + let (fan_out, calls) = single_provider_fan_out(|call| match call { + 0 => Err(transport_error()), + _ => Ok(serde_json::json!(MAINNET_CHAIN_ID)), + }); + + // When + let results = fan_out + .network_fingerprints(Duration::from_secs(1), NonZeroU64::new(2).unwrap()) + .await; + + // Then + assert_eq!(calls.load(Ordering::SeqCst), 2); + let fingerprint = results[0] + .1 + .as_ref() + .expect("second attempt should succeed"); + assert_eq!(fingerprint.to_string(), MAINNET_CHAIN_ID); +} + +#[tokio::test] +async fn network_fingerprints__should_stop_after_the_configured_number_of_attempts() { + // Given a provider that never answers + let (fan_out, calls) = single_provider_fan_out(|_| Err(transport_error())); + + // When + let results = fan_out + .network_fingerprints(Duration::from_secs(1), NonZeroU64::new(3).unwrap()) + .await; + + // Then + assert_eq!(calls.load(Ordering::SeqCst), 3); + assert_matches!( + results[0].1, + Err(ForeignChainInspectionError::RpcRequestFailed(_)) + ); +} + +#[tokio::test] +async fn network_fingerprints__should_not_retry_a_provider_that_refused_the_request() { + // Given a provider refusing with a JSON-RPC error object, as one does for a bad API key + let (fan_out, calls) = single_provider_fan_out(|_| { + Err(RpcClientError::Call(jsonrpsee::types::ErrorObject::owned( + -32600, + "Must be authenticated!", + None::<()>, + ))) + }); + + // When + let results = fan_out + .network_fingerprints(Duration::from_secs(1), NonZeroU64::new(3).unwrap()) + .await; + + // Then the refusal is reported as one, and the remaining attempts are not spent on it + assert_eq!(calls.load(Ordering::SeqCst), 1); + assert_matches!( + results[0].1, + Err(ForeignChainInspectionError::RpcRequestRejected(_)) + ); +} + +#[tokio::test] +async fn network_fingerprint__should_propagate_rpc_client_errors() { + // Given + let client = FixedResponseRpcClient::new(|| { + Err(RpcClientError::Transport(Box::new(std::io::Error::new( + std::io::ErrorKind::ConnectionRefused, + "connection refused", + )))) + }); + let inspector = StarknetInspector::new(client); + + // When + let response = inspector.network_fingerprint().await; + + // Then + assert_matches!( + response, + Err(ForeignChainInspectionError::RpcRequestFailed(_)) + ); +} diff --git a/crates/foreign-chain-inspector/tests/starknet_rpc_manual.rs b/crates/foreign-chain-inspector/tests/starknet_rpc_manual.rs index ab85b67109..3d03cc0bb1 100644 --- a/crates/foreign-chain-inspector/tests/starknet_rpc_manual.rs +++ b/crates/foreign-chain-inspector/tests/starknet_rpc_manual.rs @@ -108,3 +108,27 @@ fn parse_starknet_felt_hash +/// +/// The spec types this as `CHAIN_ID`, `^0x[a-fA-F0-9]+$`, not as a `FELT`. It carries no length +/// bound and permits leading zeros, so it is kept as Hex text rather than parsed into an [`H256`]. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Deserialize)] +#[serde(transparent)] +pub struct ChainIdResponse(pub String); + +impl ChainIdResponse { + /// Lowercase, no leading zeros. + /// + /// The prefix is matched case-insensitively although the spec's pattern is not: the same + /// normalization is applied to operator-written fingerprints, which the pattern does not bind. + pub fn canonical_text(&self) -> String { + let digits = self + .0 + .strip_prefix("0x") + .or_else(|| self.0.strip_prefix("0X")); + let Some(digits) = digits else { + return self.0.clone(); + }; + let significant = digits.trim_start_matches('0'); + if significant.is_empty() { + "0x0".to_string() + } else { + format!("0x{}", significant.to_ascii_lowercase()) + } + } +} #[cfg(test)] #[expect(non_snake_case)] mod tests { use super::{ - BlockId, GetBlockWithTxHashesArgs, GetBlockWithTxHashesResponse, + BlockId, ChainIdResponse, GetBlockWithTxHashesArgs, GetBlockWithTxHashesResponse, GetTransactionReceiptResponse, StarknetExecutionStatus, StarknetFinalityStatus, parse_felt, }; + use rstest::rstest; const TEST_BLOCK_NUMBER: u64 = 842_750; const TEST_RECEIPT_BLOCK_NUMBER: u64 = 6_195_041; const SHORT_HEX_BLOCK_HASH: &str = "0x5"; + /// Starknet mainnet's chain id, `SN_MAIN` in ASCII. + const MAINNET_CHAIN_ID: &str = "0x534e5f4d41494e"; + + /// 66 hex digits. `CHAIN_ID` carries no length bound, though a `FELT` caps at 63. + const LONGER_THAN_A_FELT: &str = + "0x1234567890123456789012345678901234567890123456789012345678901234ab"; + + #[rstest] + #[case::canonical(MAINNET_CHAIN_ID, MAINNET_CHAIN_ID)] + // Padded and upper-cased, as a provider may send it. + #[case::padded_and_upper_cased("0x00534E5F4D41494E", MAINNET_CHAIN_ID)] + // A spelling only an operator can write, since the spec's pattern binds providers. + #[case::upper_cased_prefix("0X534E5F4D41494E", MAINNET_CHAIN_ID)] + #[case::zero("0x0000", "0x0")] + #[case::longer_than_a_felt(LONGER_THAN_A_FELT, LONGER_THAN_A_FELT)] + // The decoded name rather than the hex: reported as answered by the provider. + #[case::not_hex("NOT_CHAIN_ID", "NOT_CHAIN_ID")] + fn chain_id_response__should_canonicalize_what_a_provider_answers( + #[case] answered: &str, + #[case] expected: &str, + ) { + // Given + let json = serde_json::json!(answered); + + // When + let response: ChainIdResponse = serde_json::from_value(json).unwrap(); + + // Then + assert_eq!(response.canonical_text(), expected); + } #[test] fn deserialize_receipt__should_accept_short_hex_block_hash() { diff --git a/crates/near-mpc-contract-interface/src/types/foreign_chain.rs b/crates/near-mpc-contract-interface/src/types/foreign_chain.rs index 704d0755a9..dab50a23ff 100644 --- a/crates/near-mpc-contract-interface/src/types/foreign_chain.rs +++ b/crates/near-mpc-contract-interface/src/types/foreign_chain.rs @@ -1688,7 +1688,18 @@ impl ForeignTxSignPayload { /// Stable label for an RPC provider entry (e.g. `"alchemy"`, `"ankr"`, `"drpc"`). /// Unique within a chain in the on-chain foreign-chain RPC whitelist. -#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, BorshSerialize, BorshDeserialize)] +#[derive( + Debug, + Clone, + Eq, + PartialEq, + Ord, + PartialOrd, + Hash, + BorshSerialize, + BorshDeserialize, + derive_more::Display, +)] #[cfg_attr(not(target_arch = "wasm32"), derive(Serialize, Deserialize))] #[cfg_attr( all(feature = "abi", not(target_arch = "wasm32")), diff --git a/crates/node-config/src/foreign_chains.rs b/crates/node-config/src/foreign_chains.rs index 41d7f37c29..ca11f72246 100644 --- a/crates/node-config/src/foreign_chains.rs +++ b/crates/node-config/src/foreign_chains.rs @@ -45,6 +45,8 @@ pub struct ForeignChainsConfig { #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] pub struct ForeignChainConfig { pub timeout_sec: NonZeroU64, + /// Total attempts per provider, not additional ones: `1` means a single try. Read by the + /// network fingerprint probe; transaction verification does not retry a provider at all currently. pub max_retries: NonZeroU64, /// The network fingerprint the operator expects every provider of this chain to report, in the /// chain's canonical text form. A chain id for chains that have one, a genesis hash or digest diff --git a/crates/node/src/providers/verify_foreign_tx.rs b/crates/node/src/providers/verify_foreign_tx.rs index 55f35f8c90..9615254efb 100644 --- a/crates/node/src/providers/verify_foreign_tx.rs +++ b/crates/node/src/providers/verify_foreign_tx.rs @@ -22,13 +22,15 @@ use foreign_chain_rpc_auth::auth_config_to_rpc_auth; use foreign_chain_rpc_interfaces::aptos::ReqwestAptosClient; use foreign_chain_rpc_interfaces::sui::GrpcSuiClient; use mpc_node_config::{ConfigFile, ForeignChainConfig, ForeignChainsConfig}; +use near_mpc_contract_interface::types::ProviderId; use std::sync::Arc; use std::time::Duration; -/// Pre-built HTTP clients for each foreign chain, keyed in provider config order. +/// Pre-built HTTP clients for each foreign chain, one per configured provider and named by its +/// [`ProviderId`]. /// -/// Built once at startup so that request handling only needs to select an index -/// instead of re-parsing config and constructing clients on every call. +/// Built once at startup so that request handling fans out over ready clients instead of re-parsing +/// config and constructing them on every call. pub(crate) struct ForeignChainInspectors { pub bitcoin: Option>>, pub abstract_chain: Option>>, @@ -52,12 +54,13 @@ impl ForeignChainInspectors { return Ok(None); }; let timeout = Duration::from_secs(c.timeout_sec.get()); - let inspectors = c.providers.try_map_to_vec(|_, p| { + let inspectors = c.providers.try_map_to_vec(|name, p| { // `Path`/`Query` auth is substituted into `url`; `Header` auth is returned // as `RpcAuthentication::CustomHeader` for the client to install. let mut url = p.rpc_url.clone(); let rpc_auth = auth_config_to_rpc_auth(p.auth.clone(), &mut url)?; - new_inspector(url, rpc_auth, timeout) + let inspector = new_inspector(url, rpc_auth, timeout)?; + anyhow::Ok((ProviderId(name.as_str().to_owned()), inspector)) })?; Ok(Some(FanOut::new(inspectors))) } diff --git a/docs/design/allowing-per-node-foreign-chain-rpc-configuration.md b/docs/design/allowing-per-node-foreign-chain-rpc-configuration.md index 85fbc40e40..894b202149 100644 --- a/docs/design/allowing-per-node-foreign-chain-rpc-configuration.md +++ b/docs/design/allowing-per-node-foreign-chain-rpc-configuration.md @@ -132,6 +132,6 @@ Landing in stacked PRs under [#3208](https://github.com/near/mpc/issues/3208): - **PR 1** ([#3216](https://github.com/near/mpc/pull/3216)): on-chain data shape and storage field (`ForeignChainRpcWhitelist`, `ProviderEntry`, `AuthScheme`, `ChainRouting`, `ProviderId`), `MpcContract` field + storage key, and the `AllowedProviders` data-structure helpers (add/remove/get). No vote endpoints, no view function, no node-side wiring. - **PR 2** ([#3249](https://github.com/near/mpc/pull/3249)): contract-side voting on the whitelist. Adds `vote_update_foreign_chain_providers(votes: Vec)`, the `ProviderVotes` pending-vote storage, canonicalization (`providers` sorted by `provider_id`; duplicate chain or `provider_id` in a batch rejected with `InvalidParameters::MalformedPayload`), and the `clean_tee_status` extension that drops votes from non-participants. Voting is **full-snapshot**: each `ChainVote` proposes the chain's complete state (provider list + RPC response quorum), and the chain's stored `ChainEntry` is replaced once the protocol's signing threshold of participants holds the same canonical `(providers, quorum)` pair (same gate as `vote_add_os_measurement`). Drops the original Add/Remove-ops design for two reasons: (1) snapshot semantics canonicalize trivially (sort the proposed list), avoiding the order-of-apply ambiguity Add/Remove batches introduced, and (2) bundling the RPC response quorum into `ChainVote.quorum` collapses what was originally going to be two separate vote endpoints (whitelist + quorum) into one. -- **PR 3**: node-side wiring — operator-yaml schema change (`provider_id` + `token` only), indexer task streaming the whitelist into a `watch::Receiver`, coordinator startup pipeline (resolve → sample-tx probe → register), per-inspector chain-identity probe. +- **PR 3**: node-side wiring — operator-yaml schema change (`provider_id` + `token` only), indexer task streaming the whitelist into a `watch::Receiver`, coordinator startup pipeline (resolve → sample-tx probe → register), per-inspector network fingerprint probe. -The chain-identity probe is not a gate in that pipeline: it reports each provider's health, its expected values come from each chain's `expected_network_fingerprint` in operator config rather than from constants in the binary, and registration does not wait on it. The sample-tx probe still gates registration. +The network fingerprint probe is not a gate in that pipeline: it reports each provider's health, its expected values come from each chain's `expected_network_fingerprint` in operator config rather than from constants in the binary, and registration does not wait on it. The sample-tx probe still gates registration. diff --git a/docs/foreign-chain-transactions.md b/docs/foreign-chain-transactions.md index 8eede23088..aa756c890e 100644 --- a/docs/foreign-chain-transactions.md +++ b/docs/foreign-chain-transactions.md @@ -354,7 +354,7 @@ Relevant contract methods: ## On-chain RPC Provider Whitelist > Tracked under issue [#3208](https://github.com/near/mpc/issues/3208). Landing in stacked PRs: -> PR 1 (contract storage types) → PR 2 (vote endpoints) → PR 3 (node-side wiring + chain-identity probe). +> PR 1 (contract storage types) → PR 2 (vote endpoints) → PR 3 (node-side wiring + network fingerprint probe). > The text below describes the end-state design; sections call out per-PR scope where relevant. The per-participant registration model above leaves the network with no shared notion of *which RPC providers it trusts* — a TEE-attested node binary still pulls URLs from its own config file. To close that gap the contract carries a per-chain whitelist of providers, voted in by participants. Operators reference providers from the whitelist by `provider_id` in their local `foreign_chains.yaml`; the node assembles the final URL from `base_url` + `chain_routing` + the operator-supplied token (placed per `auth_scheme`). @@ -369,7 +369,7 @@ The per-participant registration model above leaves the network with no shared n | What the operator picks | Full URL, auth scheme, token reference | `provider_id` (label) + token reference | | Adding a new provider | Every operator updates their yaml; the network effectively supports a chain once enough do | Threshold of participants vote in `(chain, ProviderEntry)`; operators reference it by `provider_id` only | | Removing a compromised provider | Every operator manually edits their yaml; coordination problem | Threshold of participants vote remove; nodes pick up the change via the indexer and drop the provider on next reconfigure | -| Testnet vs mainnet separation | Implicit — operator decides what URL goes under which chain | Per-`ForeignChain` map slot, plus a startup *chain-identity probe* that calls the chain's self-identifying RPC and compares the response against the chain's `expected_network_fingerprint` from operator config — catches both lookup-level (wrong bucket) and content-level (wrong URL voted into the right bucket) confusion. | +| Testnet vs mainnet separation | Implicit — operator decides what URL goes under which chain | Per-`ForeignChain` map slot, plus a startup *network fingerprint probe* that calls the chain's self-identifying RPC and compares the response against the chain's `expected_network_fingerprint` from operator config — catches both lookup-level (wrong bucket) and content-level (wrong URL voted into the right bucket) confusion. | ### Whitelist storage shape @@ -521,14 +521,22 @@ Two reasons together drove the snapshot model over an Add/Remove diff-ops endpoi Voting uses the protocol's existing signing threshold (`self.threshold()?.value()`), the same gate as `verify_tee` and `vote_add_os_measurement`. An earlier design proposed a separate per-chain *voting* threshold so mainnet and testnet could be voted in under different agreement requirements; that was dropped because (a) there's no setter that could safely populate it without itself being voted in, leaving a hardcoded default that's strictly weaker than the protocol threshold, and (b) the per-chain numeric on `ChainVote.quorum` already covers the *runtime* security knob — how many of N whitelisted providers must agree for a node to accept a response — which is what operators actually need to tune per chain. -#### Why the chain-identity probe in addition to per-chain keying (PR 3) +#### Why the network fingerprint probe in addition to per-chain keying (PR 3) -The per-chain map key prevents *lookup* confusion: when the node resolves the operator's `ethereum:` section, only `entries[Ethereum]` is consulted, never `entries[Sepolia]`. What it doesn't prevent is a `ChainVote { chain: Ethereum, providers: [ProviderEntry { provider_id: "ankr", chain_routing: PathSegment { segment: "eth_sepolia" }, … }, …], threshold: _ }` getting voted in — the contract just stores what threshold consensus produces; it can't tell whether `"eth_sepolia"` actually corresponds to Ethereum mainnet. Threshold voter review is the first line of defense; the fan-out across a chain's providers is the structural one. The chain-identity probe is a per-node diagnostic on top of both. +The per-chain map key prevents *lookup* confusion: when the node resolves the operator's `ethereum:` section, only `entries[Ethereum]` is consulted, never `entries[Sepolia]`. What it doesn't prevent is a `ChainVote { chain: Ethereum, providers: [ProviderEntry { provider_id: "ankr", chain_routing: PathSegment { segment: "eth_sepolia" }, … }, …], threshold: _ }` getting voted in — the contract just stores what threshold consensus produces; it can't tell whether `"eth_sepolia"` actually corresponds to Ethereum mainnet. Threshold voter review is the first line of defense; the fan-out across a chain's providers is the structural one. The network fingerprint probe is a per-node diagnostic on top of both. At startup, each resolved provider gets its self-identifying RPC called and the response is compared against that chain's `expected_network_fingerprint` from the operator's config. The probe is report-only: a provider serving the wrong network is logged, but is not dropped, because a boot-time network blip should not take a chain out of signing. Taking the expected value from operator config rather than a constant in the attested binary is a deliberate trade. It makes mixed-network and local deployments checkable at all, since a config may pair one chain's mainnet with another's testnet and no binary can ship a value for a devnet. The cost is that the check no longer binds an operator: they can set the wrong value, or omit the field and get no check at all, and either way they fool only their own node's diagnostics. The network-level defenses against a wrong URL are unchanged: threshold voter review of the whitelist, and the provider fan-out, which fails the individual request when a provider disagrees with its siblings. +Not every chain has a fingerprint probe. The table lists the ones that do, with the RPC each probes. A chain absent from it ignores `expected_network_fingerprint`. + +| chain | probe | fingerprint (mainnet) | fingerprint (testnet) | +|---|---|---|---| +| starknet | `starknet_chainId` | `0x534e5f4d41494e` (`SN_MAIN`) | `0x534e5f5345504f4c4941` (`SN_SEPOLIA`) | + +Starknet's fingerprint is the chain id felt in lowercase `0x` hex without leading zeros. Both providers and operators are free to pad and upper-case it, so the reported and the configured value are normalized before they are compared. + #### Why drop-and-log on local-config mismatch, not hard-crash If an operator's `foreign_chains.yaml` references a `provider_id` not on the whitelist for that chain (e.g. just removed by a vote), the node logs a warning and excludes that provider from registration; the chain is still served by surviving providers. A chain falls off the registration set only when zero providers survive. Hard-crashing would let a single hostile vote-removal participant take a node offline by removing a provider that node depends on. @@ -677,12 +685,17 @@ separates the test networks from each other where a network *name* would not: te separates mainnet from testnet, but two devnets can collide. `solana` and `ethereum` are configurable but absent from the table: neither has an inspector, so -setting `expected_network_fingerprint` for them has no effect. +there is nothing about them to verify in the first place. The fingerprint is set per chain rather than once per deployment, so a config can mix networks, and each value must match the network of the `rpc_url` beside it. The value is always a quoted string, including the fingerprints that look numeric. +Only the chains with a fingerprint probe read the field at all — starknet today, the rest as their +probes are written. For those chains, leaving it unset is not a silent skip: every provider of the +chain is reported as `MissingExpectedFingerprint`, because silence reads as healthy on a dashboard. A +chain with no probe yet reports `ProbeNotImplemented` whether the field is set or not. + ## Risks * **RPC trust and correctness**: Verification relies on centralized RPC providers. A malicious