From 2bef2ddaea0b2e7aac34c561f9fc7c2e04f43d97 Mon Sep 17 00:00:00 2001 From: Louis Pahlavi Date: Tue, 11 Nov 2025 14:32:52 +0100 Subject: [PATCH 1/3] DEFI-2461: Separate metrics for consensus errors --- Cargo.toml | 1 + src/http.rs | 18 ++++++++++++++++++ src/metrics.rs | 5 +++++ src/types/mod.rs | 2 ++ tests/tests.rs | 45 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 71 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index d4c73580..97e8a2ce 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,7 @@ evm_rpc_types = { path = "evm_rpc_types" } getrandom = { workspace = true } http = { workspace = true } ic-ethereum-types = { workspace = true } +ic-error-types = { workspace = true } ic-http-types = { workspace = true } ic-metrics-encoder = { workspace = true } ic-stable-structures = { workspace = true } diff --git a/src/http.rs b/src/http.rs index 1449bee8..8d6ad7d7 100644 --- a/src/http.rs +++ b/src/http.rs @@ -29,6 +29,7 @@ use evm_rpc_types::{ HttpOutcallError, LegacyRejectionCode, ProviderError, RpcError, RpcResult, ValidationError, }; use http::{header::CONTENT_TYPE, HeaderValue}; +use ic_error_types::RejectCode; use ic_management_canister_types::{ HttpRequestArgs as IcHttpRequest, HttpRequestResult as IcHttpResponse, TransformArgs, TransformContext, TransformFunc, @@ -149,6 +150,12 @@ where (req_data.method, req_data.host), 1 ); + } else if is_consensus_error(error) { + add_metric_entry!( + err_no_consensus, + (req_data.method, req_data.host), + 1 + ); } else { log!( Priority::TraceHttp, @@ -407,3 +414,14 @@ pub fn transform_http_request(args: TransformArgs) -> IcHttpResponse { headers: vec![], } } + +fn is_consensus_error(error: &IcError) -> bool { + match error { + IcError::CallRejected { code, message } => { + code == &RejectCode::SysTransient + && message + .contains("No consensus could be reached. Replicas had different responses.") + } + _ => false, + } +} diff --git a/src/metrics.rs b/src/metrics.rs index 6ad66c64..bb1a179d 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -109,6 +109,11 @@ pub fn encode_metrics(w: &mut ic_metrics_encoder::MetricsEncoder>) -> st &m.err_max_response_size_exceeded, "Number of HTTP outcalls with max response size exceeded", ); + w.counter_entries( + "evmrpc_err_no_consensus", + &m.err_no_consensus, + "Number of HTTP outcalls with consensus errors", + ); Ok(()) }) diff --git a/src/types/mod.rs b/src/types/mod.rs index 65ab4b2a..df5b27ec 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -176,6 +176,8 @@ pub struct Metrics { pub err_http_outcall: HashMap<(MetricRpcMethod, MetricRpcHost, LegacyRejectionCode), u64>, #[serde(rename = "errMaxResponseSizeExceeded")] pub err_max_response_size_exceeded: HashMap<(MetricRpcMethod, MetricRpcHost), u64>, + #[serde(rename = "errNoConsensus")] + pub err_no_consensus: HashMap<(MetricRpcMethod, MetricRpcHost), u64>, } #[derive(Clone, Debug, PartialEq, Eq)] diff --git a/tests/tests.rs b/tests/tests.rs index e395e166..a1be56c1 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -1461,6 +1461,51 @@ async fn should_have_metrics_for_request_endpoint() { ); } +#[tokio::test] +async fn should_have_metrics_for_consensus_errors() { + let mocks = MockHttpOutcallsBuilder::new() + .given(get_transaction_count_request()) + .respond_with(CanisterHttpReject::with_reject_code(RejectCode::SysTransient) + .with_message("No consensus could be reached. Replicas had different responses. Details: request_id: 21114231, timeout: 1761906996398580080, hashes: [2f66337c4e46bad3b26f3271d7def54b1b9632dee3146a993bf968ac9fb5bbd5: 15], [6ca1037eb29b619e387de330bc8e248a619b66b04cba26eab59723eddba12d1c: 14], [8ebeb0f2e2390b2e8c63f1ae24d416e6f90e4ddddc47c3df23c40ac03c7d3835: 2], [4fce8e9722ab59f92be2f4a65c5ae7d1f3b69f2b2993287c0795bbfe17d9ed51: 1]") + ); + + let setup = EvmRpcSetup::new().await.mock_api_keys().await; + let result = setup + .client(mocks) + .with_rpc_sources(RpcServices::EthMainnet(Some(vec![ + EthMainnetService::Cloudflare, + ]))) + .build() + .get_transaction_count(( + address!("0xdac17f958d2ee523a2206206994597c13d831ec7"), + BlockNumberOrTag::Latest, + )) + .send() + .await + .expect_consistent(); + assert_matches!( + result, + Err(RpcError::HttpOutcallError(HttpOutcallError::IcError { + code: LegacyRejectionCode::SysTransient, + .. + })) + ); + + setup + .check_metrics() + .await + .assert_contains_metric_matching( + r#"evmrpc_requests\{method="eth_getTransactionCount",host="cloudflare-eth.com"\} 1 \d+"#, + ) + .assert_contains_metric_matching( + r#"evmrpc_err_no_consensus\{method="eth_getTransactionCount",host="cloudflare-eth.com"\} 1 \d+"#, + ).assert_does_not_contain_metric_matching( + r#"evmrpc_responses.*"#, + ).assert_does_not_contain_metric_matching( + r#"evmrpc_err_http_outcall.*"#, + ); +} + #[tokio::test] async fn should_have_metrics_for_multi_request_endpoint() { let mocks = MockHttpOutcallsBuilder::new() From efbdb532e387daf736250b27da2b56a0c20db0ff Mon Sep 17 00:00:00 2001 From: Louis Pahlavi Date: Tue, 11 Nov 2025 14:56:15 +0100 Subject: [PATCH 2/3] Fix test --- tests/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tests.rs b/tests/tests.rs index a1be56c1..e82c48ee 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -1419,8 +1419,8 @@ async fn candid_rpc_should_return_inconsistent_results_with_consensus_error() { setup .check_metrics() .await - .assert_contains_metric_matching(r#"evmrpc_err_http_outcall\{method="eth_getTransactionCount",host="rpc.ankr.com",code="SYS_TRANSIENT"\} 1 \d+"#) - .assert_contains_metric_matching(r#"evmrpc_err_http_outcall\{method="eth_getTransactionCount",host="ethereum-rpc.publicnode.com",code="SYS_TRANSIENT"\} 1 \d+"#); + .assert_contains_metric_matching(r#"evmrpc_err_no_consensus\{method="eth_getTransactionCount",host="rpc.ankr.com"\} 1 \d+"#) + .assert_contains_metric_matching(r#"evmrpc_err_no_consensus\{method="eth_getTransactionCount",host="ethereum-rpc.publicnode.com"\} 1 \d+"#, ); } #[tokio::test] From 22bcd7811bcb8094b6a7facaaba0d01aeb6cad05 Mon Sep 17 00:00:00 2001 From: Louis Pahlavi Date: Thu, 13 Nov 2025 07:39:26 +0100 Subject: [PATCH 3/3] Increase leniency --- src/http.rs | 4 +--- tests/tests.rs | 26 +++++++++----------------- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/src/http.rs b/src/http.rs index 8d6ad7d7..4a0c60d8 100644 --- a/src/http.rs +++ b/src/http.rs @@ -418,9 +418,7 @@ pub fn transform_http_request(args: TransformArgs) -> IcHttpResponse { fn is_consensus_error(error: &IcError) -> bool { match error { IcError::CallRejected { code, message } => { - code == &RejectCode::SysTransient - && message - .contains("No consensus could be reached. Replicas had different responses.") + code == &RejectCode::SysTransient && message.to_lowercase().contains("no consensus") } _ => false, } diff --git a/tests/tests.rs b/tests/tests.rs index e82c48ee..6f0d395d 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -1420,7 +1420,8 @@ async fn candid_rpc_should_return_inconsistent_results_with_consensus_error() { .check_metrics() .await .assert_contains_metric_matching(r#"evmrpc_err_no_consensus\{method="eth_getTransactionCount",host="rpc.ankr.com"\} 1 \d+"#) - .assert_contains_metric_matching(r#"evmrpc_err_no_consensus\{method="eth_getTransactionCount",host="ethereum-rpc.publicnode.com"\} 1 \d+"#, ); + .assert_contains_metric_matching(r#"evmrpc_err_no_consensus\{method="eth_getTransactionCount",host="ethereum-rpc.publicnode.com"\} 1 \d+"#, ) + .assert_does_not_contain_metric_matching(r#"evmrpc_err_http_outcall.*"#); } #[tokio::test] @@ -1453,12 +1454,9 @@ async fn should_have_metrics_for_request_endpoint() { setup .check_metrics() .await - .assert_contains_metric_matching( - r#"evmrpc_requests\{method="request",is_manual_request="true",host="cloudflare-eth.com"\} 1 \d+"#, - ) - .assert_contains_metric_matching( - r#"evmrpc_responses\{method="request",is_manual_request="true",host="cloudflare-eth.com",status="200"\} 1 \d+"#, - ); + .assert_contains_metric_matching(r#"evmrpc_requests\{method="request",is_manual_request="true",host="cloudflare-eth.com"\} 1 \d+"#, ) + .assert_contains_metric_matching(r#"evmrpc_responses\{method="request",is_manual_request="true",host="cloudflare-eth.com",status="200"\} 1 \d+"#, ) + .assert_does_not_contain_metric_matching(r#"evmrpc_err_http_outcall.*"#); } #[tokio::test] @@ -1494,16 +1492,10 @@ async fn should_have_metrics_for_consensus_errors() { setup .check_metrics() .await - .assert_contains_metric_matching( - r#"evmrpc_requests\{method="eth_getTransactionCount",host="cloudflare-eth.com"\} 1 \d+"#, - ) - .assert_contains_metric_matching( - r#"evmrpc_err_no_consensus\{method="eth_getTransactionCount",host="cloudflare-eth.com"\} 1 \d+"#, - ).assert_does_not_contain_metric_matching( - r#"evmrpc_responses.*"#, - ).assert_does_not_contain_metric_matching( - r#"evmrpc_err_http_outcall.*"#, - ); + .assert_contains_metric_matching(r#"evmrpc_requests\{method="eth_getTransactionCount",host="cloudflare-eth.com"\} 1 \d+"#, ) + .assert_contains_metric_matching(r#"evmrpc_err_no_consensus\{method="eth_getTransactionCount",host="cloudflare-eth.com"\} 1 \d+"#, ) + .assert_does_not_contain_metric_matching(r#"evmrpc_responses.*"#, ) + .assert_does_not_contain_metric_matching(r#"evmrpc_err_http_outcall.*"#, ); } #[tokio::test]