From f2342e9769a0742eaa4dc317bdbefc8199346083 Mon Sep 17 00:00:00 2001 From: Haiyue Chen Date: Wed, 5 Aug 2026 17:31:32 +0200 Subject: [PATCH 01/11] feat(probe): probe Aptos for its ledger chain id The chain id lives in the ledger info at the REST root, so `AptosRpcClient` gains a call for it. The status mapping `extract` already had is shared, except for a 404, which on the root means the URL serves no Aptos API rather than a missing transaction. --- .../foreign-chain-health-check/src/probe.rs | 85 +++++++++- .../src/aptos/inspector.rs | 158 ++++++++++++++---- .../tests/aptos_inspector.rs | 31 +++- .../tests/aptos_rpc_manual.rs | 21 +++ .../foreign-chain-rpc-interfaces/src/aptos.rs | 76 +++++++++ docs/foreign-chain-transactions.md | 7 +- 6 files changed, 341 insertions(+), 37 deletions(-) diff --git a/crates/foreign-chain-health-check/src/probe.rs b/crates/foreign-chain-health-check/src/probe.rs index 6987c00a38..05d22dc78b 100644 --- a/crates/foreign-chain-health-check/src/probe.rs +++ b/crates/foreign-chain-health-check/src/probe.rs @@ -5,6 +5,7 @@ use std::collections::BTreeMap; use std::time::Duration; use foreign_chain_inspector::abstract_chain::inspector::Abstract; +use foreign_chain_inspector::aptos::inspector::AptosInspector; use foreign_chain_inspector::arbitrum::inspector::Arbitrum; use foreign_chain_inspector::base::inspector::Base; use foreign_chain_inspector::bitcoin::inspector::BitcoinInspector; @@ -16,11 +17,12 @@ use foreign_chain_inspector::starknet::inspector::StarknetInspector; use foreign_chain_inspector::{ FanOut, ForeignChainInspectionError, NetworkFingerprint, ProviderFailure, }; +use foreign_chain_rpc_interfaces::aptos::ReqwestAptosClient; 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; +use crate::{prepare_aptos, prepare_jsonrpc}; /// One provider's verdict. Anything other than [`ProviderStatus::Healthy`] is unhealthy. #[derive(Debug, Clone, PartialEq, Eq)] @@ -114,8 +116,20 @@ pub async fn probe_all_providers(config: &ForeignChainsConfig) -> ProbeReport { }) .await } - // TODO(#4003): probe Aptos and Sui. Ethereum, Solana and Ton have no inspector, so - // there is nothing to probe them with. + ForeignChain::Aptos => { + let timeout = Duration::from_secs(chain_config.timeout_sec.get()); + probe_chain(chain, chain_config, move |provider| { + let (url, auth_header) = prepare_aptos(provider)?; + Ok(AptosInspector::new(ReqwestAptosClient::new( + url, + auth_header, + timeout, + ))) + }) + .await + } + // TODO(#4003): probe Sui. Ethereum, Solana and Ton have no inspector, so there is + // nothing to probe them with. _ => rows_of(chain, chain_config, ProviderStatus::ProbeNotImplemented), } }); @@ -244,6 +258,9 @@ mod tests { const CLOSED_PORT_URL: &str = "http://127.0.0.1:9"; /// For a chain with no probe: the value is never read, only whether it is set at all. const ANY_FINGERPRINT: &str = "any-fingerprint"; + /// Aptos publishes its ledger chain id in decimal. + const APTOS_MAINNET: u64 = 1; + const APTOS_TESTNET: u64 = 2; /// Bitcoin's genesis block hash, which is what tells its networks apart. const BITCOIN_MAINNET: &str = "000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f"; @@ -885,6 +902,68 @@ mod tests { ); } + async fn mock_ledger_info<'a>( + server: &'a httpmock::MockServer, + chain_id: u64, + ) -> httpmock::Mock<'a> { + let body = serde_json::json!({"chain_id": chain_id, "ledger_version": "1"}); + server + .mock_async(|when, then| { + when.method(httpmock::Method::GET); + then.status(200).json_body(body); + }) + .await + } + + #[tokio::test] + async fn probe_all_providers__should_report_aptos_on_its_expected_chain_id_as_healthy() { + // Given + let server = httpmock::MockServer::start_async().await; + mock_ledger_info(&server, APTOS_MAINNET).await; + let config = ForeignChainsConfig { + aptos: Some(chain_config( + Some("1"), + one_provider("publicnode", &server.base_url()), + )), + ..Default::default() + }; + + // When + let report = probe_all_providers(&config).await; + + // Then + assert_eq!( + must_status_of(&report, ForeignChain::Aptos, "publicnode"), + ProviderStatus::Healthy + ); + } + + #[tokio::test] + async fn probe_all_providers__should_report_aptos_on_another_network_as_wrong_network() { + // Given + let server = httpmock::MockServer::start_async().await; + mock_ledger_info(&server, APTOS_TESTNET).await; + let config = ForeignChainsConfig { + aptos: Some(chain_config( + Some("1"), + one_provider("publicnode", &server.base_url()), + )), + ..Default::default() + }; + + // When + let report = probe_all_providers(&config).await; + + // Then + assert_eq!( + must_status_of(&report, ForeignChain::Aptos, "publicnode"), + ProviderStatus::WrongNetwork { + expected: NetworkFingerprint::new("1"), + observed: NetworkFingerprint::new("2"), + } + ); + } + #[tokio::test] async fn probe_all_providers__should_retry_a_provider_that_refused_with_a_rate_limit_code() { // Given diff --git a/crates/foreign-chain-inspector/src/aptos/inspector.rs b/crates/foreign-chain-inspector/src/aptos/inspector.rs index d265a53051..9312a1f3fd 100644 --- a/crates/foreign-chain-inspector/src/aptos/inspector.rs +++ b/crates/foreign-chain-inspector/src/aptos/inspector.rs @@ -1,7 +1,11 @@ use crate::aptos::{AptosExtractedValue, AptosTransactionHash}; -use crate::{ForeignChainInspectionError, ForeignChainInspector, HexBytes}; +use crate::{ + ForeignChainInspectionError, ForeignChainInspector, HexBytes, NetworkFingerprint, + NetworkFingerprintInspector, +}; use foreign_chain_rpc_interfaces::aptos::{ - AptosRpcClient, AptosRpcError, TransactionResponse, normalize_event_data, + AptosRpcClient, AptosRpcError, TransactionResponse, canonical_chain_id_text, + normalize_event_data, }; use near_mpc_contract_interface::types::{AptosAddress, AptosEvent}; use std::borrow::Cow; @@ -28,6 +32,28 @@ pub enum AptosExtractor { Event { event_index: usize }, } +impl NetworkFingerprintInspector for AptosInspector +where + Client: AptosRpcClient + Send + Sync, +{ + async fn network_fingerprint(&self) -> Result { + let ledger_info = self + .client + .get_ledger_info() + .await + // Unlike a transaction lookup, a 404 here means the URL does not serve the Aptos + // API at all, which no retry can change. + .map_err(classify_rest_error)?; + Ok(Self::canonical_fingerprint( + &ledger_info.chain_id.to_string(), + )) + } + + fn canonical_fingerprint(fingerprint: &str) -> NetworkFingerprint { + NetworkFingerprint::new(canonical_chain_id_text(fingerprint)) + } +} + impl ForeignChainInspector for AptosInspector where Client: AptosRpcClient + Send + Sync, @@ -49,29 +75,12 @@ where .client .get_transaction_by_hash(&tx_hash_hex) .await - .map_err(|e| { - let msg = e.to_string(); - match e { - // 404 = definitively absent → a non-transient verdict. - AptosRpcError::ApiError { status: 404, .. } => { - ForeignChainInspectionError::TransactionNotFound - } - // Rate limits and server errors are provider hiccups → transient, so the - // affected provider is dropped from the quorum instead of blocking it. - AptosRpcError::ApiError { - status: 408 | 429, .. - } => ForeignChainInspectionError::RpcRequestFailed(msg), - AptosRpcError::ApiError { status, .. } if status >= 500 => { - ForeignChainInspectionError::RpcRequestFailed(msg) - } - // Remaining 4xx (400/401/403/410, …) are deterministic rejections — - // retrying cannot change them, so they count as substantive verdicts. - AptosRpcError::ApiError { .. } => { - ForeignChainInspectionError::RpcRequestRejected(msg) - } - // Transport failures, including timeouts. - AptosRpcError::Http(_) => ForeignChainInspectionError::RpcRequestFailed(msg), + // 404 = definitively absent → a non-transient verdict. + .map_err(|e| match e { + AptosRpcError::ApiError { status: 404, .. } => { + ForeignChainInspectionError::TransactionNotFound } + other => classify_rest_error(other), })?; ensure_hash_matches(&tx_id, &tx.hash)?; @@ -103,6 +112,26 @@ where } } +/// A refusal is a substantive verdict, so only what a retry could change is transient. +fn classify_rest_error(error: AptosRpcError) -> ForeignChainInspectionError { + let message = error.to_string(); + match error { + // Rate limits and server errors are provider hiccups → transient, so the affected + // provider is dropped from the quorum instead of blocking it. + AptosRpcError::ApiError { + status: 408 | 429, .. + } => ForeignChainInspectionError::RpcRequestFailed(message), + AptosRpcError::ApiError { status, .. } if status >= 500 => { + ForeignChainInspectionError::RpcRequestFailed(message) + } + // Remaining 4xx (400/401/403/410, …) are deterministic rejections — retrying cannot + // change them, so they count as substantive verdicts. + AptosRpcError::ApiError { .. } => ForeignChainInspectionError::RpcRequestRejected(message), + // Transport failures, including timeouts. + AptosRpcError::Http(_) => ForeignChainInspectionError::RpcRequestFailed(message), + } +} + /// Rejects a backend that returned a different transaction than queried. A non-hex `returned` /// hash is a malformed response; a well-formed but different hash is a hard inconsistency. fn ensure_hash_matches( @@ -236,25 +265,46 @@ mod tests { use super::*; use assert_matches::assert_matches; use foreign_chain_rpc_interfaces::aptos::{ - AptosEventResponse, AptosRpcError, EventGuid, TransactionResponse, + AptosEventResponse, AptosRpcError, EventGuid, LedgerInfoResponse, TransactionResponse, }; use rstest::rstest; + /// Mainnet, as the docs and the config templates ship it. + const MAINNET_CHAIN_ID: u64 = 1; + struct MockAptosClient { response: Result, + ledger_info: Result, } impl MockAptosClient { fn success(tx: TransactionResponse) -> Self { - Self { response: Ok(tx) } + Self { + response: Ok(tx), + ledger_info: Ok(LedgerInfoResponse { + chain_id: MAINNET_CHAIN_ID, + }), + } } fn api_error(status: u16) -> Self { Self { - response: Err(AptosRpcError::ApiError { - status, - body: format!("http {status}"), - }), + response: Err(Self::error(status)), + ledger_info: Err(Self::error(status)), + } + } + + fn on_chain(chain_id: u64) -> Self { + Self { + ledger_info: Ok(LedgerInfoResponse { chain_id }), + ..Self::api_error(404) + } + } + + fn error(status: u16) -> AptosRpcError { + AptosRpcError::ApiError { + status, + body: format!("http {status}"), } } } @@ -277,6 +327,23 @@ mod tests { }; std::future::ready(r) } + + fn get_ledger_info( + &self, + ) -> impl Future> + Send { + let r = match &self.ledger_info { + Ok(info) => Ok(info.clone()), + Err(AptosRpcError::ApiError { status, body }) => Err(AptosRpcError::ApiError { + status: *status, + body: body.clone(), + }), + Err(other) => Err(AptosRpcError::ApiError { + status: 500, + body: other.to_string(), + }), + }; + std::future::ready(r) + } } fn sample_tx(hash: &str, success: bool) -> TransactionResponse { @@ -732,4 +799,35 @@ mod tests { Err(ForeignChainInspectionError::MalformedRpcResponse(_)) ); } + + #[tokio::test] + async fn network_fingerprint__should_return_the_ledger_chain_id() { + // Given + const TESTNET_CHAIN_ID: u64 = 2; + let inspector = AptosInspector::new(MockAptosClient::on_chain(TESTNET_CHAIN_ID)); + + // When + let fingerprint = inspector + .network_fingerprint() + .await + .expect("network_fingerprint should succeed"); + + // Then + assert_eq!(fingerprint.to_string(), "2"); + } + + #[tokio::test] + async fn network_fingerprint__should_report_a_root_that_refuses_as_rejected() { + // Given + let inspector = AptosInspector::new(MockAptosClient::api_error(404)); + + // When + let fingerprint = inspector.network_fingerprint().await; + + // Then + assert_matches!( + fingerprint, + Err(ForeignChainInspectionError::RpcRequestRejected(_)) + ); + } } diff --git a/crates/foreign-chain-inspector/tests/aptos_inspector.rs b/crates/foreign-chain-inspector/tests/aptos_inspector.rs index c002ab3915..b305717202 100644 --- a/crates/foreign-chain-inspector/tests/aptos_inspector.rs +++ b/crates/foreign-chain-inspector/tests/aptos_inspector.rs @@ -4,7 +4,7 @@ use std::time::Duration; use assert_matches::assert_matches; use foreign_chain_inspector::{ - ForeignChainInspectionError, ForeignChainInspector, + ForeignChainInspectionError, ForeignChainInspector, NetworkFingerprintInspector, aptos::{ AptosExtractedValue, AptosTransactionHash, inspector::{AptosExtractor, AptosFinality, AptosInspector}, @@ -237,3 +237,32 @@ async fn extract__should_reject_response_with_mismatched_hash() { Err(ForeignChainInspectionError::InconsistentRpcResponse { .. }) ); } + +/// Aptos mainnet, as the docs table and the config templates ship it. +const MAINNET_CHAIN_ID: u64 = 1; + +#[tokio::test] +async fn network_fingerprint__should_ask_the_rest_root_for_the_ledger_chain_id() { + // Given + let server = MockServer::start_async().await; + let mock = server + .mock_async(|when, then| { + when.method(GET).path("/v1"); + then.status(200).json_body(serde_json::json!({ + "chain_id": MAINNET_CHAIN_ID, + "ledger_version": "2915317", + })); + }) + .await; + let inspector = inspector_for(&server); + + // When + let fingerprint = inspector + .network_fingerprint() + .await + .expect("network_fingerprint should succeed"); + + // Then + mock.assert_async().await; + assert_eq!(fingerprint.to_string(), "1"); +} diff --git a/crates/foreign-chain-inspector/tests/aptos_rpc_manual.rs b/crates/foreign-chain-inspector/tests/aptos_rpc_manual.rs index dda6f54e44..16ecbb4ac0 100644 --- a/crates/foreign-chain-inspector/tests/aptos_rpc_manual.rs +++ b/crates/foreign-chain-inspector/tests/aptos_rpc_manual.rs @@ -60,3 +60,24 @@ fn parse_tx_hash(hash: &str) -> AptosTransactionHash { .expect("transaction hash should be 32 bytes"); AptosTransactionHash::from(array) } + +/// Aptos mainnet's ledger chain id, as shipped in `expected_network_fingerprint`. +const EXPECTED_NETWORK_FINGERPRINT: &str = "1"; + +#[tokio::test] +#[ignore = "manual test to sanity check against live Aptos RPC provider"] +async fn network_fingerprint_matches_the_shipped_config_value_against_live_rpc_provider() { + // given + let client = + ReqwestAptosClient::new(PUBLIC_NODE_URL.to_string(), None, Duration::from_secs(10)); + let inspector = AptosInspector::new(client); + + // when + let fingerprint = + foreign_chain_inspector::NetworkFingerprintInspector::network_fingerprint(&inspector) + .await + .expect("network_fingerprint should succeed"); + + // then + assert_eq!(fingerprint.to_string(), EXPECTED_NETWORK_FINGERPRINT); +} diff --git a/crates/foreign-chain-rpc-interfaces/src/aptos.rs b/crates/foreign-chain-rpc-interfaces/src/aptos.rs index 1f29f379ba..49c1cb4039 100644 --- a/crates/foreign-chain-rpc-interfaces/src/aptos.rs +++ b/crates/foreign-chain-rpc-interfaces/src/aptos.rs @@ -49,12 +49,33 @@ pub enum AptosRpcError { ApiError { status: u16, body: String }, } +/// Partial response of the API root: the ledger info every Aptos node reports. +/// +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] +pub struct LedgerInfoResponse { + /// One byte in the spec, one digit in practice: 1 is mainnet, 2 is testnet. + pub chain_id: u64, +} + +/// Decimal without leading zeros, the form Aptos publishes chain ids in. Text that is not a +/// number is returned unchanged, so a nonsense answer can be reported as it was given. +pub fn canonical_chain_id_text(chain_id: &str) -> String { + match chain_id.parse::() { + Ok(parsed) => parsed.to_string(), + Err(_) => chain_id.to_owned(), + } +} + /// Client interface for the Aptos REST API v1. pub trait AptosRpcClient: Send + Sync { fn get_transaction_by_hash( &self, tx_hash_hex: &str, ) -> impl Future> + Send; + + fn get_ledger_info( + &self, + ) -> impl Future> + Send; } #[derive(Clone)] @@ -119,6 +140,27 @@ impl AptosRpcClient for ReqwestAptosClient { Ok(parsed) } } + + /// The ledger info lives at the API root, so the base URL is requested as configured. + fn get_ledger_info( + &self, + ) -> impl Future> + Send { + let url = self.base.clone(); + let client = self.client.clone(); + async move { + let response = client.get(url).send().await?; + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + return Err(AptosRpcError::ApiError { + status: status.as_u16(), + body, + }); + } + let parsed = response.json::().await?; + Ok(parsed) + } + } } /// Canonical string form of the event `data`: object keys sorted recursively so all nodes hash @@ -154,6 +196,7 @@ fn sort_keys(v: serde_json::Value) -> serde_json::Value { #[expect(non_snake_case)] mod tests { use super::*; + use rstest::rstest; #[test] fn build_request_url__appends_resource_path_to_versioned_base() { @@ -309,4 +352,37 @@ mod tests { r#"{"outer_a":0,"outer_b":{"inner_a":1,"inner_z":9}}"# ); } + + #[rstest] + #[case::mainnet("1", "1")] + #[case::padded("0002", "2")] + // Reported as answered. + #[case::not_a_number("mainnet", "mainnet")] + fn canonical_chain_id_text__should_canonicalize_what_is_configured( + #[case] configured: &str, + #[case] expected: &str, + ) { + // When + let canonical = canonical_chain_id_text(configured); + + // Then + assert_eq!(canonical, expected); + } + + #[test] + fn deserialize_ledger_info__should_ignore_the_fields_the_probe_does_not_read() { + // Given + let json = serde_json::json!({ + "chain_id": 1, + "epoch": "13", + "ledger_version": "1234", + "node_role": "full_node", + }); + + // When + let parsed: LedgerInfoResponse = serde_json::from_value(json).unwrap(); + + // Then + assert_eq!(parsed.chain_id, 1); + } } diff --git a/docs/foreign-chain-transactions.md b/docs/foreign-chain-transactions.md index 9e4224d8a0..cccfec0e9b 100644 --- a/docs/foreign-chain-transactions.md +++ b/docs/foreign-chain-transactions.md @@ -555,8 +555,9 @@ Not every chain has a fingerprint probe. The table lists the ones that do, with | starknet | `starknet_chainId` | | base, bnb, arbitrum, polygon, hyper_evm, abstract | `eth_chainId` | | bitcoin | `getblockhash` at height 0 | +| aptos | the ledger info at the REST root | -The reported and the configured value are normalized before they are compared, because the same fingerprint has several legal spellings. Starknet's is the chain id felt in lowercase `0x` hex without leading zeros, which providers and operators alike are free to pad and upper-case. The EVM chain id is compared in decimal, the form it is published and configured in, while `eth_chainId` answers a `0x` hex quantity. Bitcoin's genesis hash is compared in lowercase hex, with the leading zeros kept, since they are digits of the hash. +The reported and the configured value are normalized before they are compared, because the same fingerprint has several legal spellings. Starknet's is the chain id felt in lowercase `0x` hex without leading zeros, which providers and operators alike are free to pad and upper-case. The EVM chain id is compared in decimal, the form it is published and configured in, while `eth_chainId` answers a `0x` hex quantity. Bitcoin's genesis hash is compared in lowercase hex, with the leading zeros kept, since they are digits of the hash. Aptos answers its chain id as a number, so only the configured value needs normalizing. An answer that is no fingerprint at all is reported as the wrong network, carrying the text the provider sent, so the report says what was actually claimed. An answer longer than any real fingerprint is cut short and ends in `_TRUNCATED`, because it is repeated into logs and metric labels. @@ -715,8 +716,8 @@ The fingerprint is set per chain rather than once per deployment, so a config ca 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, bitcoin and the EVM -chains today, the rest as their probes are written. For those chains, leaving it unset is not a silent skip: every +Only the chains with a fingerprint probe read the field at all — starknet, bitcoin, aptos and the +EVM chains 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. From 1349d6f01e0cf0e7ebc81d4781ac3a7edcf0fc43 Mon Sep 17 00:00:00 2001 From: Haiyue Chen Date: Wed, 5 Aug 2026 18:06:49 +0200 Subject: [PATCH 02/11] fix(probe): report a slow Aptos provider as timed out --- crates/foreign-chain-inspector/src/aptos/inspector.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/foreign-chain-inspector/src/aptos/inspector.rs b/crates/foreign-chain-inspector/src/aptos/inspector.rs index 9312a1f3fd..1e8bd8c3b0 100644 --- a/crates/foreign-chain-inspector/src/aptos/inspector.rs +++ b/crates/foreign-chain-inspector/src/aptos/inspector.rs @@ -127,7 +127,8 @@ fn classify_rest_error(error: AptosRpcError) -> ForeignChainInspectionError { // Remaining 4xx (400/401/403/410, …) are deterministic rejections — retrying cannot // change them, so they count as substantive verdicts. AptosRpcError::ApiError { .. } => ForeignChainInspectionError::RpcRequestRejected(message), - // Transport failures, including timeouts. + // Named so a probe can report a slow provider as timed out rather than unreachable. + AptosRpcError::Http(error) if error.is_timeout() => ForeignChainInspectionError::Timeout, AptosRpcError::Http(_) => ForeignChainInspectionError::RpcRequestFailed(message), } } From 62581d4608f29c0516e996f2ee47e0332d9fb761 Mon Sep 17 00:00:00 2001 From: Haiyue Chen Date: Wed, 5 Aug 2026 18:16:49 +0200 Subject: [PATCH 03/11] refactor(probe): derive the attempt deadline once `probe_chain` hands it to the inspector factory, so a chain whose client carries its own deadline cannot drift from the one the probe enforces. --- crates/foreign-chain-health-check/src/probe.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/crates/foreign-chain-health-check/src/probe.rs b/crates/foreign-chain-health-check/src/probe.rs index 05d22dc78b..2c21134e09 100644 --- a/crates/foreign-chain-health-check/src/probe.rs +++ b/crates/foreign-chain-health-check/src/probe.rs @@ -99,7 +99,7 @@ pub async fn probe_all_providers(config: &ForeignChainsConfig) -> ProbeReport { .map(|(chain, chain_config)| async move { match chain { ForeignChain::Starknet => { - probe_chain(chain, chain_config, |provider| { + probe_chain(chain, chain_config, |provider, _| { Ok(StarknetInspector::new(prepare_jsonrpc(provider)?)) }) .await @@ -111,14 +111,13 @@ pub async fn probe_all_providers(config: &ForeignChainsConfig) -> ProbeReport { ForeignChain::HyperEvm => probe_evm::(chain, chain_config).await, ForeignChain::Polygon => probe_evm::(chain, chain_config).await, ForeignChain::Bitcoin => { - probe_chain(chain, chain_config, |provider| { + probe_chain(chain, chain_config, |provider, _| { Ok(BitcoinInspector::new(prepare_jsonrpc(provider)?)) }) .await } ForeignChain::Aptos => { - let timeout = Duration::from_secs(chain_config.timeout_sec.get()); - probe_chain(chain, chain_config, move |provider| { + probe_chain(chain, chain_config, |provider, timeout| { let (url, auth_header) = prepare_aptos(provider)?; Ok(AptosInspector::new(ReqwestAptosClient::new( url, @@ -142,16 +141,18 @@ async fn probe_evm(chain: ForeignChain, config: &ForeignChainConfig) -> V where Chain: EvmChain + Clone + Send + Sync + 'static, { - probe_chain(chain, config, |provider| { + probe_chain(chain, config, |provider, _| { Ok(EvmInspector::<_, Chain>::new(prepare_jsonrpc(provider)?)) }) .await } +/// `new_inspector` is handed the same deadline the probe gives each attempt, for the clients that +/// carry their own. async fn probe_chain( chain: ForeignChain, config: &ForeignChainConfig, - new_inspector: impl Fn(&ForeignChainProviderConfig) -> anyhow::Result, + new_inspector: impl Fn(&ForeignChainProviderConfig, Duration) -> anyhow::Result, ) -> Vec where I: foreign_chain_inspector::NetworkFingerprintInspector + Clone + Send + Sync + 'static, @@ -161,11 +162,12 @@ where }; let expected = I::canonical_fingerprint(expected); + let timeout = Duration::from_secs(config.timeout_sec.get()); 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) { + match new_inspector(provider, timeout) { Ok(inspector) => inspectors.push((provider_id, inspector)), Err(error) => rows.push(ProviderHealth { chain, @@ -179,7 +181,6 @@ where return rows; }; - let timeout = Duration::from_secs(config.timeout_sec.get()); let fingerprints = FanOut::new(inspectors) .network_fingerprints(timeout, config.max_retries) .await; From 81bac79620e2299f2f148998068e18e355f338dd Mon Sep 17 00:00:00 2001 From: Haiyue Chen Date: Wed, 5 Aug 2026 18:30:13 +0200 Subject: [PATCH 04/11] docs(probe): state the 404 contract once, on the classifier --- crates/foreign-chain-inspector/src/aptos/inspector.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/foreign-chain-inspector/src/aptos/inspector.rs b/crates/foreign-chain-inspector/src/aptos/inspector.rs index 1e8bd8c3b0..0e205f7d75 100644 --- a/crates/foreign-chain-inspector/src/aptos/inspector.rs +++ b/crates/foreign-chain-inspector/src/aptos/inspector.rs @@ -41,8 +41,6 @@ where .client .get_ledger_info() .await - // Unlike a transaction lookup, a 404 here means the URL does not serve the Aptos - // API at all, which no retry can change. .map_err(classify_rest_error)?; Ok(Self::canonical_fingerprint( &ledger_info.chain_id.to_string(), @@ -112,7 +110,9 @@ where } } -/// A refusal is a substantive verdict, so only what a retry could change is transient. +/// A refusal is a substantive verdict, so only what a retry could change is transient. A 404 is a +/// refusal like any other; a caller that asked for something that can legitimately be absent, such +/// as a transaction, maps it before delegating. fn classify_rest_error(error: AptosRpcError) -> ForeignChainInspectionError { let message = error.to_string(); match error { From ca106c7edaa582b1f8eb2002f0053c612a718295 Mon Sep 17 00:00:00 2001 From: Haiyue Chen Date: Wed, 5 Aug 2026 18:35:56 +0200 Subject: [PATCH 05/11] Revert "refactor(probe): derive the attempt deadline once" Threading the deadline through the factory keeps client construction inside the probe, which is the thing to move. Leaves a TODO(#4043) where it belongs. --- .../foreign-chain-health-check/src/probe.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/foreign-chain-health-check/src/probe.rs b/crates/foreign-chain-health-check/src/probe.rs index 2c21134e09..fcd9ade42d 100644 --- a/crates/foreign-chain-health-check/src/probe.rs +++ b/crates/foreign-chain-health-check/src/probe.rs @@ -99,7 +99,7 @@ pub async fn probe_all_providers(config: &ForeignChainsConfig) -> ProbeReport { .map(|(chain, chain_config)| async move { match chain { ForeignChain::Starknet => { - probe_chain(chain, chain_config, |provider, _| { + probe_chain(chain, chain_config, |provider| { Ok(StarknetInspector::new(prepare_jsonrpc(provider)?)) }) .await @@ -111,13 +111,14 @@ pub async fn probe_all_providers(config: &ForeignChainsConfig) -> ProbeReport { ForeignChain::HyperEvm => probe_evm::(chain, chain_config).await, ForeignChain::Polygon => probe_evm::(chain, chain_config).await, ForeignChain::Bitcoin => { - probe_chain(chain, chain_config, |provider, _| { + probe_chain(chain, chain_config, |provider| { Ok(BitcoinInspector::new(prepare_jsonrpc(provider)?)) }) .await } ForeignChain::Aptos => { - probe_chain(chain, chain_config, |provider, timeout| { + let timeout = Duration::from_secs(chain_config.timeout_sec.get()); + probe_chain(chain, chain_config, move |provider| { let (url, auth_header) = prepare_aptos(provider)?; Ok(AptosInspector::new(ReqwestAptosClient::new( url, @@ -141,18 +142,18 @@ async fn probe_evm(chain: ForeignChain, config: &ForeignChainConfig) -> V where Chain: EvmChain + Clone + Send + Sync + 'static, { - probe_chain(chain, config, |provider, _| { + probe_chain(chain, config, |provider| { Ok(EvmInspector::<_, Chain>::new(prepare_jsonrpc(provider)?)) }) .await } -/// `new_inspector` is handed the same deadline the probe gives each attempt, for the clients that -/// carry their own. +// TODO(#4043): take the inspectors as a dependency instead, so that building a client from +// config, and the deadline it is built with, live outside the probe. async fn probe_chain( chain: ForeignChain, config: &ForeignChainConfig, - new_inspector: impl Fn(&ForeignChainProviderConfig, Duration) -> anyhow::Result, + new_inspector: impl Fn(&ForeignChainProviderConfig) -> anyhow::Result, ) -> Vec where I: foreign_chain_inspector::NetworkFingerprintInspector + Clone + Send + Sync + 'static, @@ -162,12 +163,11 @@ where }; let expected = I::canonical_fingerprint(expected); - let timeout = Duration::from_secs(config.timeout_sec.get()); 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, timeout) { + match new_inspector(provider) { Ok(inspector) => inspectors.push((provider_id, inspector)), Err(error) => rows.push(ProviderHealth { chain, @@ -181,6 +181,7 @@ where return rows; }; + let timeout = Duration::from_secs(config.timeout_sec.get()); let fingerprints = FanOut::new(inspectors) .network_fingerprints(timeout, config.max_retries) .await; From 103041fe30868c99dce18bf1aa8b068eb4c07508 Mon Sep 17 00:00:00 2001 From: Haiyue Chen Date: Wed, 5 Aug 2026 18:39:10 +0200 Subject: [PATCH 06/11] docs(probe): move the DI todo to where the clients are chosen --- crates/foreign-chain-health-check/src/probe.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/foreign-chain-health-check/src/probe.rs b/crates/foreign-chain-health-check/src/probe.rs index fcd9ade42d..9e3be7467e 100644 --- a/crates/foreign-chain-health-check/src/probe.rs +++ b/crates/foreign-chain-health-check/src/probe.rs @@ -93,6 +93,8 @@ impl ProbeReport { /// 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. +// TODO(#4043): take the inspectors as a dependency instead, so that choosing a client per chain, +// and the deadline it is built with, happens outside the probe. pub async fn probe_all_providers(config: &ForeignChainsConfig) -> ProbeReport { let probe_attempts = config .iter_chains() @@ -148,8 +150,6 @@ where .await } -// TODO(#4043): take the inspectors as a dependency instead, so that building a client from -// config, and the deadline it is built with, live outside the probe. async fn probe_chain( chain: ForeignChain, config: &ForeignChainConfig, From 14ad249efef64e873bc5363c56e2fb002d2c4300 Mon Sep 17 00:00:00 2001 From: Haiyue Chen Date: Thu, 6 Aug 2026 12:58:33 +0200 Subject: [PATCH 07/11] docs(probe): shorten the DI todo and fold it into the doc comment --- crates/foreign-chain-health-check/src/probe.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/foreign-chain-health-check/src/probe.rs b/crates/foreign-chain-health-check/src/probe.rs index 9e3be7467e..08219d59bc 100644 --- a/crates/foreign-chain-health-check/src/probe.rs +++ b/crates/foreign-chain-health-check/src/probe.rs @@ -93,8 +93,8 @@ impl ProbeReport { /// 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. -// TODO(#4043): take the inspectors as a dependency instead, so that choosing a client per chain, -// and the deadline it is built with, happens outside the probe. +/// +/// TODO(#4043): take the inspectors as a dependency instead pub async fn probe_all_providers(config: &ForeignChainsConfig) -> ProbeReport { let probe_attempts = config .iter_chains() From ce2b9516aa8af96d49d7adf7b6fed48245de661d Mon Sep 17 00:00:00 2001 From: Haiyue Chen Date: Fri, 7 Aug 2026 17:43:55 +0200 Subject: [PATCH 08/11] refactor(probe): let the resource say what a not found answer means A 404 reads differently per endpoint, so the response type carries the verdict as an associated const and the call site passes nothing. --- .../src/aptos/inspector.rs | 104 ++++++++++++++---- crates/foreign-chain-inspector/src/lib.rs | 33 ++++++ .../foreign-chain-rpc-interfaces/src/aptos.rs | 2 +- 3 files changed, 118 insertions(+), 21 deletions(-) diff --git a/crates/foreign-chain-inspector/src/aptos/inspector.rs b/crates/foreign-chain-inspector/src/aptos/inspector.rs index 0e205f7d75..b05398f1c7 100644 --- a/crates/foreign-chain-inspector/src/aptos/inspector.rs +++ b/crates/foreign-chain-inspector/src/aptos/inspector.rs @@ -1,11 +1,11 @@ use crate::aptos::{AptosExtractedValue, AptosTransactionHash}; use crate::{ - ForeignChainInspectionError, ForeignChainInspector, HexBytes, NetworkFingerprint, - NetworkFingerprintInspector, + AbsenceMeaning, ClassifyRpcOutcome, ForeignChainInspectionError, ForeignChainInspector, + HasAbsenceMeaning, HexBytes, NetworkFingerprint, NetworkFingerprintInspector, }; use foreign_chain_rpc_interfaces::aptos::{ - AptosRpcClient, AptosRpcError, TransactionResponse, canonical_chain_id_text, - normalize_event_data, + AptosRpcClient, AptosRpcError, LedgerInfoResponse, TransactionResponse, + canonical_chain_id_text, normalize_event_data, }; use near_mpc_contract_interface::types::{AptosAddress, AptosEvent}; use std::borrow::Cow; @@ -37,11 +37,7 @@ where Client: AptosRpcClient + Send + Sync, { async fn network_fingerprint(&self) -> Result { - let ledger_info = self - .client - .get_ledger_info() - .await - .map_err(classify_rest_error)?; + let ledger_info = self.client.get_ledger_info().await.classified()?; Ok(Self::canonical_fingerprint( &ledger_info.chain_id.to_string(), )) @@ -73,13 +69,7 @@ where .client .get_transaction_by_hash(&tx_hash_hex) .await - // 404 = definitively absent → a non-transient verdict. - .map_err(|e| match e { - AptosRpcError::ApiError { status: 404, .. } => { - ForeignChainInspectionError::TransactionNotFound - } - other => classify_rest_error(other), - })?; + .classified()?; ensure_hash_matches(&tx_id, &tx.hash)?; @@ -110,12 +100,38 @@ where } } -/// A refusal is a substantive verdict, so only what a retry could change is transient. A 404 is a -/// refusal like any other; a caller that asked for something that can legitimately be absent, such -/// as a transaction, maps it before delegating. -fn classify_rest_error(error: AptosRpcError) -> ForeignChainInspectionError { +/// A transaction the chain does not have is a verdict about that transaction. +impl HasAbsenceMeaning for TransactionResponse { + const ABSENCE: AbsenceMeaning = AbsenceMeaning::TransactionIsAbsent; +} + +/// The ledger info is the REST API root, which every Aptos node serves. +impl HasAbsenceMeaning for LedgerInfoResponse { + const ABSENCE: AbsenceMeaning = AbsenceMeaning::ApiIsNotServed; +} + +impl ClassifyRpcOutcome for Result { + type Response = T; + + fn classified(self) -> Result { + self.map_err(|error| classify_rest_error(error, T::ABSENCE)) + } +} + +fn classify_rest_error( + error: AptosRpcError, + absence: AbsenceMeaning, +) -> ForeignChainInspectionError { let message = error.to_string(); match error { + // Aptos answers 404 both for a transaction it does not have and for a path it does not + // route; only the resource that was read tells the two apart. + AptosRpcError::ApiError { status: 404, .. } => match absence { + AbsenceMeaning::TransactionIsAbsent => ForeignChainInspectionError::TransactionNotFound, + AbsenceMeaning::ApiIsNotServed => { + ForeignChainInspectionError::RpcRequestRejected(message) + } + }, // Rate limits and server errors are provider hiccups → transient, so the affected // provider is dropped from the quorum instead of blocking it. AptosRpcError::ApiError { @@ -831,4 +847,52 @@ mod tests { Err(ForeignChainInspectionError::RpcRequestRejected(_)) ); } + + #[test] + fn classified__should_read_an_absent_transaction_as_the_chains_verdict() { + // Given + let answered: Result = Err(MockAptosClient::error(404)); + + // When + let error = answered.classified().unwrap_err(); + + // Then + assert_matches!(error, ForeignChainInspectionError::TransactionNotFound); + } + + #[test] + fn classified__should_read_an_absent_ledger_root_as_a_refusal() { + // Given + let answered: Result = Err(MockAptosClient::error(404)); + + // When + let error = answered.classified().unwrap_err(); + + // Then + assert_matches!(error, ForeignChainInspectionError::RpcRequestRejected(_)); + } + + #[rstest] + #[case::request_timeout(408)] + #[case::rate_limited(429)] + #[case::internal_error(500)] + #[case::bad_request(400)] + #[case::unauthorized(401)] + fn classify_rest_error__should_read_every_status_but_404_alike(#[case] status: u16) { + // When + let as_transaction = classify_rest_error( + MockAptosClient::error(status), + AbsenceMeaning::TransactionIsAbsent, + ); + let as_api_root = classify_rest_error( + MockAptosClient::error(status), + AbsenceMeaning::ApiIsNotServed, + ); + + // Then + assert_eq!( + std::mem::discriminant(&as_transaction), + std::mem::discriminant(&as_api_root) + ); + } } diff --git a/crates/foreign-chain-inspector/src/lib.rs b/crates/foreign-chain-inspector/src/lib.rs index 56cb041219..90868da277 100644 --- a/crates/foreign-chain-inspector/src/lib.rs +++ b/crates/foreign-chain-inspector/src/lib.rs @@ -470,6 +470,39 @@ fn is_retryable_status(status_code: u16) -> bool { matches!(status_code, REQUEST_TIMEOUT | TOO_MANY_REQUESTS) || status_code >= SERVER_ERROR } +/// What a provider's "not found" answer means for the resource that was read. +/// +/// HTTP 404 and gRPC `NOT_FOUND` are the one wire condition whose verdict depends on what was +/// asked for rather than on the status itself, so the resource answers and the chain's status +/// table reads the answer. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum AbsenceMeaning { + /// The chain may legitimately not hold it, so absence is the chain's own verdict. + TransactionIsAbsent, + /// Every node serving this chain's API holds it, so absence says the endpoint serves a + /// different API. + ApiIsNotServed, +} + +/// The [`AbsenceMeaning`] of the resource a response carries, stated once beside that response +/// rather than at each call that reads it. +/// +/// [`ClassifyRpcOutcome::classified`] requires it, so a call reading a response that has no +/// answer to the question does not compile. +pub(crate) trait HasAbsenceMeaning { + const ABSENCE: AbsenceMeaning; +} + +/// Reads a chain client's outcome as an inspection outcome: the transport says how the call +/// failed, the response type says what an absent resource means, and the call site says nothing. +/// +/// One implementation per transport, each holding that chain's whole status table. +pub(crate) trait ClassifyRpcOutcome { + type Response; + + fn classified(self) -> Result; +} + /// 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)] diff --git a/crates/foreign-chain-rpc-interfaces/src/aptos.rs b/crates/foreign-chain-rpc-interfaces/src/aptos.rs index 49c1cb4039..3f787bdef3 100644 --- a/crates/foreign-chain-rpc-interfaces/src/aptos.rs +++ b/crates/foreign-chain-rpc-interfaces/src/aptos.rs @@ -356,7 +356,7 @@ mod tests { #[rstest] #[case::mainnet("1", "1")] #[case::padded("0002", "2")] - // Reported as answered. + // Should be reported as answered by the RPC provider. #[case::not_a_number("mainnet", "mainnet")] fn canonical_chain_id_text__should_canonicalize_what_is_configured( #[case] configured: &str, From 13480cc018bca987d3e810c0898a07207d744807 Mon Sep 17 00:00:00 2001 From: Haiyue Chen Date: Fri, 7 Aug 2026 19:40:10 +0200 Subject: [PATCH 09/11] fix(probe): tell a body that never arrived from one that is not the resource reqwest reports both as a decode error, so the transport step and the decode step now fail with their own types: a truncated or timed out body stays transient, while a body that is not the resource is a verdict about the endpoint. Also folds the status table into `classified`, so the absence meaning is only ever read from the response type. --- .../src/aptos/inspector.rs | 97 +++++++++---------- crates/foreign-chain-inspector/src/lib.rs | 26 ++--- .../tests/aptos_inspector.rs | 23 +++++ .../foreign-chain-rpc-interfaces/src/aptos.rs | 60 ++++++------ 4 files changed, 107 insertions(+), 99 deletions(-) diff --git a/crates/foreign-chain-inspector/src/aptos/inspector.rs b/crates/foreign-chain-inspector/src/aptos/inspector.rs index b05398f1c7..12897f615e 100644 --- a/crates/foreign-chain-inspector/src/aptos/inspector.rs +++ b/crates/foreign-chain-inspector/src/aptos/inspector.rs @@ -100,7 +100,6 @@ where } } -/// A transaction the chain does not have is a verdict about that transaction. impl HasAbsenceMeaning for TransactionResponse { const ABSENCE: AbsenceMeaning = AbsenceMeaning::TransactionIsAbsent; } @@ -114,38 +113,45 @@ impl ClassifyRpcOutcome for Result { type Response = T; fn classified(self) -> Result { - self.map_err(|error| classify_rest_error(error, T::ABSENCE)) - } -} + let error = match self { + Ok(response) => return Ok(response), + Err(error) => error, + }; -fn classify_rest_error( - error: AptosRpcError, - absence: AbsenceMeaning, -) -> ForeignChainInspectionError { - let message = error.to_string(); - match error { - // Aptos answers 404 both for a transaction it does not have and for a path it does not - // route; only the resource that was read tells the two apart. - AptosRpcError::ApiError { status: 404, .. } => match absence { - AbsenceMeaning::TransactionIsAbsent => ForeignChainInspectionError::TransactionNotFound, - AbsenceMeaning::ApiIsNotServed => { + let message = error.to_string(); + Err(match error { + // Aptos answers 404 both for a transaction it lacks and for a path it does not route. + AptosRpcError::ApiError { status: 404, .. } => match T::ABSENCE { + AbsenceMeaning::TransactionIsAbsent => { + ForeignChainInspectionError::TransactionNotFound + } + AbsenceMeaning::ApiIsNotServed => { + ForeignChainInspectionError::RpcRequestRejected(message) + } + }, + // Rate limits and server errors are provider hiccups → transient, so the + // affected provider is dropped from the quorum instead of blocking it. + AptosRpcError::ApiError { + status: 408 | 429, .. + } => ForeignChainInspectionError::RpcRequestFailed(message), + AptosRpcError::ApiError { status, .. } if status >= 500 => { + ForeignChainInspectionError::RpcRequestFailed(message) + } + // Remaining 4xx (400/401/403/410, …) are deterministic rejections — + // retrying cannot change them, so they count as substantive verdicts. + AptosRpcError::ApiError { .. } => { ForeignChainInspectionError::RpcRequestRejected(message) } - }, - // Rate limits and server errors are provider hiccups → transient, so the affected - // provider is dropped from the quorum instead of blocking it. - AptosRpcError::ApiError { - status: 408 | 429, .. - } => ForeignChainInspectionError::RpcRequestFailed(message), - AptosRpcError::ApiError { status, .. } if status >= 500 => { - ForeignChainInspectionError::RpcRequestFailed(message) - } - // Remaining 4xx (400/401/403/410, …) are deterministic rejections — retrying cannot - // change them, so they count as substantive verdicts. - AptosRpcError::ApiError { .. } => ForeignChainInspectionError::RpcRequestRejected(message), - // Named so a probe can report a slow provider as timed out rather than unreachable. - AptosRpcError::Http(error) if error.is_timeout() => ForeignChainInspectionError::Timeout, - AptosRpcError::Http(_) => ForeignChainInspectionError::RpcRequestFailed(message), + // Split timeout from rest of http errors for reporting. + AptosRpcError::Http(error) if error.is_timeout() => { + ForeignChainInspectionError::Timeout + } + AptosRpcError::Http(_) => ForeignChainInspectionError::RpcRequestFailed(message), + // A body that will not decode is not transient. + AptosRpcError::MalformedBody(_) => { + ForeignChainInspectionError::MalformedRpcResponse(message) + } + }) } } @@ -337,10 +343,7 @@ mod tests { status: *status, body: body.clone(), }), - Err(other) => Err(AptosRpcError::ApiError { - status: 500, - body: other.to_string(), - }), + Err(other) => unreachable!("MockAptosClient models only ApiError, got {other}"), }; std::future::ready(r) } @@ -354,10 +357,7 @@ mod tests { status: *status, body: body.clone(), }), - Err(other) => Err(AptosRpcError::ApiError { - status: 500, - body: other.to_string(), - }), + Err(other) => unreachable!("MockAptosClient models only ApiError, got {other}"), }; std::future::ready(r) } @@ -878,21 +878,20 @@ mod tests { #[case::internal_error(500)] #[case::bad_request(400)] #[case::unauthorized(401)] - fn classify_rest_error__should_read_every_status_but_404_alike(#[case] status: u16) { + fn classified__should_read_every_status_but_404_alike(#[case] status: u16) { + // Given + let read_as_transaction: Result = + Err(MockAptosClient::error(status)); + let read_as_api_root: Result = Err(MockAptosClient::error(status)); + // When - let as_transaction = classify_rest_error( - MockAptosClient::error(status), - AbsenceMeaning::TransactionIsAbsent, - ); - let as_api_root = classify_rest_error( - MockAptosClient::error(status), - AbsenceMeaning::ApiIsNotServed, - ); + let from_transaction = read_as_transaction.classified().unwrap_err(); + let from_api_root = read_as_api_root.classified().unwrap_err(); // Then assert_eq!( - std::mem::discriminant(&as_transaction), - std::mem::discriminant(&as_api_root) + std::mem::discriminant(&from_transaction), + std::mem::discriminant(&from_api_root) ); } } diff --git a/crates/foreign-chain-inspector/src/lib.rs b/crates/foreign-chain-inspector/src/lib.rs index 90868da277..684bde0720 100644 --- a/crates/foreign-chain-inspector/src/lib.rs +++ b/crates/foreign-chain-inspector/src/lib.rs @@ -470,33 +470,25 @@ fn is_retryable_status(status_code: u16) -> bool { matches!(status_code, REQUEST_TIMEOUT | TOO_MANY_REQUESTS) || status_code >= SERVER_ERROR } -/// What a provider's "not found" answer means for the resource that was read. -/// -/// HTTP 404 and gRPC `NOT_FOUND` are the one wire condition whose verdict depends on what was -/// asked for rather than on the status itself, so the resource answers and the chain's status -/// table reads the answer. +/// What a provider's "not found" answer means for the resource that was read: the one wire +/// condition whose verdict depends on what was asked for rather than on the status itself. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum AbsenceMeaning { - /// The chain may legitimately not hold it, so absence is the chain's own verdict. + /// The chain may legitimately not hold it. TransactionIsAbsent, - /// Every node serving this chain's API holds it, so absence says the endpoint serves a - /// different API. + /// Every node serving this chain's API holds it. ApiIsNotServed, } -/// The [`AbsenceMeaning`] of the resource a response carries, stated once beside that response -/// rather than at each call that reads it. -/// -/// [`ClassifyRpcOutcome::classified`] requires it, so a call reading a response that has no -/// answer to the question does not compile. +/// The [`AbsenceMeaning`] of the resource a response carries. Required by +/// [`ClassifyRpcOutcome::classified`], so a response that never answered cannot be classified. pub(crate) trait HasAbsenceMeaning { const ABSENCE: AbsenceMeaning; } -/// Reads a chain client's outcome as an inspection outcome: the transport says how the call -/// failed, the response type says what an absent resource means, and the call site says nothing. -/// -/// One implementation per transport, each holding that chain's whole status table. +/// Reads a chain client's outcome as an inspection outcome: the response type supplies what +/// absence means, so the call site supplies nothing. One implementation per transport, each +/// holding that chain's status table. pub(crate) trait ClassifyRpcOutcome { type Response; diff --git a/crates/foreign-chain-inspector/tests/aptos_inspector.rs b/crates/foreign-chain-inspector/tests/aptos_inspector.rs index b305717202..3e0611fa79 100644 --- a/crates/foreign-chain-inspector/tests/aptos_inspector.rs +++ b/crates/foreign-chain-inspector/tests/aptos_inspector.rs @@ -209,6 +209,29 @@ async fn extract__should_classify_http_errors_by_status( assert_eq!(error.is_transient(), expected_transient); } +#[tokio::test] +async fn extract__should_reject_a_response_that_does_not_carry_the_resource() { + // Given — a URL that answers but serves something other than the Aptos REST API. + let server = MockServer::start(); + server.mock(|when, then| { + when.method(GET).path(tx_path()); + then.status(200) + .header("content-type", "text/html") + .body("Sign in to continue"); + }); + let inspector = inspector_for(&server); + + // When + let response = inspector + .extract(tx_id(), AptosFinality::Committed, vec![]) + .await; + + // Then — the endpoint is wrong, not slow, so retrying it cannot help. + let error = response.expect_err("extract should fail"); + assert_matches!(error, ForeignChainInspectionError::MalformedRpcResponse(_)); + assert!(!error.is_transient()); +} + #[tokio::test] async fn extract__should_reject_response_with_mismatched_hash() { // Given — the backend echoes a different transaction than queried. diff --git a/crates/foreign-chain-rpc-interfaces/src/aptos.rs b/crates/foreign-chain-rpc-interfaces/src/aptos.rs index 3f787bdef3..bb528ec768 100644 --- a/crates/foreign-chain-rpc-interfaces/src/aptos.rs +++ b/crates/foreign-chain-rpc-interfaces/src/aptos.rs @@ -1,6 +1,6 @@ use reqwest::Url; use reqwest::header::{HeaderMap, HeaderName, HeaderValue}; -use serde::Deserialize; +use serde::{Deserialize, de::DeserializeOwned}; use std::future::Future; use std::time::Duration; @@ -44,9 +44,11 @@ pub struct EventGuid { #[derive(Debug, thiserror::Error)] pub enum AptosRpcError { #[error("HTTP request failed: {0}")] - Http(#[from] reqwest::Error), + Http(reqwest::Error), #[error("Aptos API returned HTTP {status}: {body}")] ApiError { status: u16, body: String }, + #[error("failed to decode the Aptos API response: {0}")] + MalformedBody(serde_json::Error), } /// Partial response of the API root: the ledger info every Aptos node reports. @@ -106,6 +108,26 @@ impl ReqwestAptosClient { .expect("Aptos rpc_url is validated as a URL by node-config before reaching here"); Self { base, client } } + + async fn get_json(&self, url: Url) -> Result { + let response = self + .client + .get(url) + .send() + .await + .map_err(AptosRpcError::Http)?; + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + return Err(AptosRpcError::ApiError { + status: status.as_u16(), + body, + }); + } + + let body = response.bytes().await.map_err(AptosRpcError::Http)?; + serde_json::from_slice(&body).map_err(AptosRpcError::MalformedBody) + } } /// Appends `transactions/by_hash/{hash}` to `base`, preserving its path and query string (so a @@ -124,42 +146,14 @@ impl AptosRpcClient for ReqwestAptosClient { &self, tx_hash_hex: &str, ) -> impl Future> + Send { - let url = build_request_url(&self.base, tx_hash_hex); - let client = self.client.clone(); - async move { - let response = client.get(url).send().await?; - let status = response.status(); - if !status.is_success() { - let body = response.text().await.unwrap_or_default(); - return Err(AptosRpcError::ApiError { - status: status.as_u16(), - body, - }); - } - let parsed = response.json::().await?; - Ok(parsed) - } + self.get_json(build_request_url(&self.base, tx_hash_hex)) } - /// The ledger info lives at the API root, so the base URL is requested as configured. fn get_ledger_info( &self, ) -> impl Future> + Send { - let url = self.base.clone(); - let client = self.client.clone(); - async move { - let response = client.get(url).send().await?; - let status = response.status(); - if !status.is_success() { - let body = response.text().await.unwrap_or_default(); - return Err(AptosRpcError::ApiError { - status: status.as_u16(), - body, - }); - } - let parsed = response.json::().await?; - Ok(parsed) - } + // The ledger info lives at the API root. + self.get_json(self.base.clone()) } } From 02ad7ff04b22e993d1fb55289a8a69f54a50061f Mon Sep 17 00:00:00 2001 From: Haiyue Chen Date: Fri, 7 Aug 2026 21:40:33 +0200 Subject: [PATCH 10/11] refactor(probe): enforce the absence meaning on the trait `ClassifyRpcOutcome::Response` now requires `HasAbsenceMeaning`, so a transport cannot classify a response type that never declared what a "not found" answer means for it. Also restores the `#[from]` conversions on `AptosRpcError` and sweeps the comments this stack added. --- .../src/aptos/inspector.rs | 8 +++----- crates/foreign-chain-inspector/src/lib.rs | 14 +++++--------- .../tests/aptos_inspector.rs | 4 ++-- crates/foreign-chain-rpc-interfaces/src/aptos.rs | 15 +++++---------- 4 files changed, 15 insertions(+), 26 deletions(-) diff --git a/crates/foreign-chain-inspector/src/aptos/inspector.rs b/crates/foreign-chain-inspector/src/aptos/inspector.rs index 12897f615e..308477cd8e 100644 --- a/crates/foreign-chain-inspector/src/aptos/inspector.rs +++ b/crates/foreign-chain-inspector/src/aptos/inspector.rs @@ -104,7 +104,7 @@ impl HasAbsenceMeaning for TransactionResponse { const ABSENCE: AbsenceMeaning = AbsenceMeaning::TransactionIsAbsent; } -/// The ledger info is the REST API root, which every Aptos node serves. +/// Every Aptos node serves the ledger info at its REST API base. impl HasAbsenceMeaning for LedgerInfoResponse { const ABSENCE: AbsenceMeaning = AbsenceMeaning::ApiIsNotServed; } @@ -137,8 +137,7 @@ impl ClassifyRpcOutcome for Result { AptosRpcError::ApiError { status, .. } if status >= 500 => { ForeignChainInspectionError::RpcRequestFailed(message) } - // Remaining 4xx (400/401/403/410, …) are deterministic rejections — - // retrying cannot change them, so they count as substantive verdicts. + // Retrying cannot change a deterministic 4xx, so it counts as a substantive verdict. AptosRpcError::ApiError { .. } => { ForeignChainInspectionError::RpcRequestRejected(message) } @@ -292,7 +291,6 @@ mod tests { }; use rstest::rstest; - /// Mainnet, as the docs and the config templates ship it. const MAINNET_CHAIN_ID: u64 = 1; struct MockAptosClient { @@ -878,7 +876,7 @@ mod tests { #[case::internal_error(500)] #[case::bad_request(400)] #[case::unauthorized(401)] - fn classified__should_read_every_status_but_404_alike(#[case] status: u16) { + fn classified__should_treat_404_as_the_only_resource_dependent_status(#[case] status: u16) { // Given let read_as_transaction: Result = Err(MockAptosClient::error(status)); diff --git a/crates/foreign-chain-inspector/src/lib.rs b/crates/foreign-chain-inspector/src/lib.rs index 684bde0720..95bb498439 100644 --- a/crates/foreign-chain-inspector/src/lib.rs +++ b/crates/foreign-chain-inspector/src/lib.rs @@ -470,27 +470,23 @@ fn is_retryable_status(status_code: u16) -> bool { matches!(status_code, REQUEST_TIMEOUT | TOO_MANY_REQUESTS) || status_code >= SERVER_ERROR } -/// What a provider's "not found" answer means for the resource that was read: the one wire -/// condition whose verdict depends on what was asked for rather than on the status itself. +/// The meaning of a provider's "not found" answer #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum AbsenceMeaning { - /// The chain may legitimately not hold it. TransactionIsAbsent, - /// Every node serving this chain's API holds it. ApiIsNotServed, } /// The [`AbsenceMeaning`] of the resource a response carries. Required by -/// [`ClassifyRpcOutcome::classified`], so a response that never answered cannot be classified. +/// [`ClassifyRpcOutcome::classified`], so an undeclared response type cannot be classified. pub(crate) trait HasAbsenceMeaning { const ABSENCE: AbsenceMeaning; } -/// Reads a chain client's outcome as an inspection outcome: the response type supplies what -/// absence means, so the call site supplies nothing. One implementation per transport, each -/// holding that chain's status table. +/// Reads a chain client's outcome as an inspection outcome. The absence meaning comes from the +/// response type. pub(crate) trait ClassifyRpcOutcome { - type Response; + type Response: HasAbsenceMeaning; fn classified(self) -> Result; } diff --git a/crates/foreign-chain-inspector/tests/aptos_inspector.rs b/crates/foreign-chain-inspector/tests/aptos_inspector.rs index 3e0611fa79..bec273998b 100644 --- a/crates/foreign-chain-inspector/tests/aptos_inspector.rs +++ b/crates/foreign-chain-inspector/tests/aptos_inspector.rs @@ -211,7 +211,7 @@ async fn extract__should_classify_http_errors_by_status( #[tokio::test] async fn extract__should_reject_a_response_that_does_not_carry_the_resource() { - // Given — a URL that answers but serves something other than the Aptos REST API. + // Given let server = MockServer::start(); server.mock(|when, then| { when.method(GET).path(tx_path()); @@ -226,7 +226,7 @@ async fn extract__should_reject_a_response_that_does_not_carry_the_resource() { .extract(tx_id(), AptosFinality::Committed, vec![]) .await; - // Then — the endpoint is wrong, not slow, so retrying it cannot help. + // Then let error = response.expect_err("extract should fail"); assert_matches!(error, ForeignChainInspectionError::MalformedRpcResponse(_)); assert!(!error.is_transient()); diff --git a/crates/foreign-chain-rpc-interfaces/src/aptos.rs b/crates/foreign-chain-rpc-interfaces/src/aptos.rs index bb528ec768..a3a4dae0b4 100644 --- a/crates/foreign-chain-rpc-interfaces/src/aptos.rs +++ b/crates/foreign-chain-rpc-interfaces/src/aptos.rs @@ -44,11 +44,11 @@ pub struct EventGuid { #[derive(Debug, thiserror::Error)] pub enum AptosRpcError { #[error("HTTP request failed: {0}")] - Http(reqwest::Error), + Http(#[from] reqwest::Error), #[error("Aptos API returned HTTP {status}: {body}")] ApiError { status: u16, body: String }, #[error("failed to decode the Aptos API response: {0}")] - MalformedBody(serde_json::Error), + MalformedBody(#[from] serde_json::Error), } /// Partial response of the API root: the ledger info every Aptos node reports. @@ -110,12 +110,7 @@ impl ReqwestAptosClient { } async fn get_json(&self, url: Url) -> Result { - let response = self - .client - .get(url) - .send() - .await - .map_err(AptosRpcError::Http)?; + let response = self.client.get(url).send().await?; let status = response.status(); if !status.is_success() { let body = response.text().await.unwrap_or_default(); @@ -125,8 +120,8 @@ impl ReqwestAptosClient { }); } - let body = response.bytes().await.map_err(AptosRpcError::Http)?; - serde_json::from_slice(&body).map_err(AptosRpcError::MalformedBody) + let body = response.bytes().await?; + Ok(serde_json::from_slice(&body)?) } } From 30611349572c5c843cdf7443237be3a30f3746fa Mon Sep 17 00:00:00 2001 From: Haiyue Chen Date: Fri, 14 Aug 2026 09:13:27 +0200 Subject: [PATCH 11/11] docs(probe): cut the paraphrasing Aptos comments Name the config field the manual test's fingerprint mirrors, and drop the comments that restate the code they sit on. --- crates/foreign-chain-inspector/src/aptos/inspector.rs | 1 - crates/foreign-chain-inspector/src/lib.rs | 2 -- crates/foreign-chain-inspector/tests/aptos_inspector.rs | 1 - crates/foreign-chain-inspector/tests/aptos_rpc_manual.rs | 3 ++- crates/foreign-chain-rpc-interfaces/src/aptos.rs | 1 - 5 files changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/foreign-chain-inspector/src/aptos/inspector.rs b/crates/foreign-chain-inspector/src/aptos/inspector.rs index 308477cd8e..2c1393b557 100644 --- a/crates/foreign-chain-inspector/src/aptos/inspector.rs +++ b/crates/foreign-chain-inspector/src/aptos/inspector.rs @@ -141,7 +141,6 @@ impl ClassifyRpcOutcome for Result { AptosRpcError::ApiError { .. } => { ForeignChainInspectionError::RpcRequestRejected(message) } - // Split timeout from rest of http errors for reporting. AptosRpcError::Http(error) if error.is_timeout() => { ForeignChainInspectionError::Timeout } diff --git a/crates/foreign-chain-inspector/src/lib.rs b/crates/foreign-chain-inspector/src/lib.rs index 95bb498439..53eab0d5ee 100644 --- a/crates/foreign-chain-inspector/src/lib.rs +++ b/crates/foreign-chain-inspector/src/lib.rs @@ -477,8 +477,6 @@ pub(crate) enum AbsenceMeaning { ApiIsNotServed, } -/// The [`AbsenceMeaning`] of the resource a response carries. Required by -/// [`ClassifyRpcOutcome::classified`], so an undeclared response type cannot be classified. pub(crate) trait HasAbsenceMeaning { const ABSENCE: AbsenceMeaning; } diff --git a/crates/foreign-chain-inspector/tests/aptos_inspector.rs b/crates/foreign-chain-inspector/tests/aptos_inspector.rs index bec273998b..16eca2d46c 100644 --- a/crates/foreign-chain-inspector/tests/aptos_inspector.rs +++ b/crates/foreign-chain-inspector/tests/aptos_inspector.rs @@ -261,7 +261,6 @@ async fn extract__should_reject_response_with_mismatched_hash() { ); } -/// Aptos mainnet, as the docs table and the config templates ship it. const MAINNET_CHAIN_ID: u64 = 1; #[tokio::test] diff --git a/crates/foreign-chain-inspector/tests/aptos_rpc_manual.rs b/crates/foreign-chain-inspector/tests/aptos_rpc_manual.rs index 16ecbb4ac0..b828ad28f0 100644 --- a/crates/foreign-chain-inspector/tests/aptos_rpc_manual.rs +++ b/crates/foreign-chain-inspector/tests/aptos_rpc_manual.rs @@ -61,7 +61,8 @@ fn parse_tx_hash(hash: &str) -> AptosTransactionHash { AptosTransactionHash::from(array) } -/// Aptos mainnet's ledger chain id, as shipped in `expected_network_fingerprint`. +/// Aptos mainnet's ledger chain id, as shipped in the node config file +/// `foreign_chains.aptos.expected_network_fingerprint`. const EXPECTED_NETWORK_FINGERPRINT: &str = "1"; #[tokio::test] diff --git a/crates/foreign-chain-rpc-interfaces/src/aptos.rs b/crates/foreign-chain-rpc-interfaces/src/aptos.rs index a3a4dae0b4..3c2673a7da 100644 --- a/crates/foreign-chain-rpc-interfaces/src/aptos.rs +++ b/crates/foreign-chain-rpc-interfaces/src/aptos.rs @@ -147,7 +147,6 @@ impl AptosRpcClient for ReqwestAptosClient { fn get_ledger_info( &self, ) -> impl Future> + Send { - // The ledger info lives at the API root. self.get_json(self.base.clone()) } }