From c9538bd79d75c54da3f6a363fb2052161128f111 Mon Sep 17 00:00:00 2001 From: incrypto32 Date: Thu, 30 Jul 2026 16:11:40 +0400 Subject: [PATCH] graph, runtime: Surface ethereum.decode failures ethereum.decode and ethereum.decodeParams return null on any failure and discarded the error entirely, so a mapping that handles null by returning early can drop entities on every matching trigger while the deployment stays healthy and synced. Split the outcome into an unparseable type string, which is a mapping bug that recurs for as long as the subgraph runs, and data that does not match a valid type, which can legitimately vary per trigger. Log the first at error and the second at warning, and count both in a new deployment_ethereum_decode_failures{deployment,host_fn,kind} metric. Gas errors now propagate instead of being reported to the mapping as null. Both host functions still return null on every failure they returned null on before, so POIs are unchanged. See https://github.com/graphprotocol/graph-node/issues/6683 --- NEWS.md | 28 ++-- graph/src/components/subgraph/host.rs | 19 +++ runtime/wasm/src/host_exports.rs | 181 ++++++++++++++++++++++++-- runtime/wasm/src/module/context.rs | 72 ++++++++-- 4 files changed, 268 insertions(+), 32 deletions(-) diff --git a/NEWS.md b/NEWS.md index 8068ecbc9fa..0ae16c38e6a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,21 @@ # NEWS +## Unreleased + +### What's New + +- **`ethereum.decode()` failures are now logged and counted.** `ethereum.decode` and `ethereum.decodeParams` return `null` on failure, which a mapping is free to swallow silently. Both now log the type string and the underlying error, and increment `deployment_ethereum_decode_failures{deployment, host_fn, kind}`. See the note below. ([#6702](https://github.com/graphprotocol/graph-node/pull/6702)) +- **Gas exhaustion inside `ethereum.decode`, `ethereum.decodeParams` and `ethereum.encode` now aborts the handler** instead of surfacing to the mapping as `null`. A handler that ran out of gas at one of these calls and made no further gas-consuming call previously completed, so this can change POI for that case against earlier versions. ([#6702](https://github.com/graphprotocol/graph-node/pull/6702)) + +### Note on `ethereum.decode()` type string handling + +The `ethabi` → `alloy` migration in v0.42.0 ([#6063](https://github.com/graphprotocol/graph-node/pull/6063)) made ABI type string parsing strict: type strings `ethabi` accepted but that were never valid ABI are now rejected. `kind` distinguishes: + +- **`invalid_type`** — the type string cannot be parsed (e.g. `bytes128`). It is a literal in the mapping, so every call returns `null` on every block; a mapping that logs and returns then writes no entities while the deployment stays healthy and synced at chain head, diverging in POI from indexers still on pre-v0.42.0 graph-node. Logged at **error**; the subgraph must be republished. Alert on this. ([#6683](https://github.com/graphprotocol/graph-node/issues/6683)) +- **`invalid_data`** — data does not match an otherwise valid type. Can legitimately vary per event; logged at **warning**. + +Separately, `ethabi` decoded a leading space (the `" address"` in `"(uint256, address)"`) as `Uint(8)`; `alloy` parses it correctly, so mappings calling `.toBigInt()` on an `Address` abort on v0.42.0+. Recompile with the correct accessor. ([#6461](https://github.com/graphprotocol/graph-node/issues/6461)) + ## v0.45.0 ``` @@ -119,18 +135,6 @@ Thanks to all contributors for this release: @erayack, @fordN, @incrypto32, @lut - Fixed `graphman config pools` not working due to hardcoded pool size override. ([#6444](https://github.com/graphprotocol/graph-node/pull/6444)) - Fixed unfail retry mechanism stopping after the first attempt when the deployment head was still behind the error block. ([#6529](https://github.com/graphprotocol/graph-node/pull/6529)) -### Note on `ethereum.decode()` whitespace handling - -The migration from `ethabi` to `alloy` in v0.42.0 ([#6063](https://github.com/graphprotocol/graph-node/pull/6063)) incidentally fixed a long-standing parsing bug in `ethabi` where type strings containing whitespace before a type name (e.g. `" address"` with a leading space) were silently decoded as `Uint(8)` instead of the intended type. `alloy` parses these correctly. - -Subgraphs that relied on the incorrect `Uint(8)` decoding to subsequently call `.toBigInt()` on what is actually an `Address` value will abort on v0.42.0+ with: - -``` -Mapping aborted ... Ethereum value is not an int or uint. -``` - -This is not a graph-node regression. Recompile the subgraph with the correct accessor (`.toAddress()` for addresses) to fix. See [#6461](https://github.com/graphprotocol/graph-node/issues/6461) for details. - ### gnd (Graph Node Dev) - `gnd indexer` command that delegates to `graph-indexer`, allowing indexer management (allocations, rules, cost models, status) directly through gnd. ([#6492](https://github.com/graphprotocol/graph-node/pull/6492)) diff --git a/graph/src/components/subgraph/host.rs b/graph/src/components/subgraph/host.rs index 40bdc10f8eb..ace9f8542c2 100644 --- a/graph/src/components/subgraph/host.rs +++ b/graph/src/components/subgraph/host.rs @@ -100,6 +100,7 @@ pub struct HostMetrics { handler_execution_time: Box, host_fn_execution_time: Box, eth_call_execution_time: Box, + ethereum_decode_failures: Box, pub gas_metrics: GasMetrics, pub stopwatch: StopwatchMetrics, } @@ -139,15 +140,33 @@ impl HostMetrics { vec![0.025, 0.05, 0.2, 2.0, 8.0, 20.0], ) .expect("failed to create `deployment_host_fn_execution_time` histogram"); + + let ethereum_decode_failures = registry + .new_deployment_counter_vec( + "deployment_ethereum_decode_failures", + "Counts ethereum.decode and ethereum.decodeParams calls that returned null", + subgraph, + vec![String::from("host_fn"), String::from("kind")], + ) + .expect("failed to create `deployment_ethereum_decode_failures` counter"); + Self { handler_execution_time, host_fn_execution_time, stopwatch, gas_metrics, eth_call_execution_time, + ethereum_decode_failures, } } + /// `kind` is `invalid_type` or `invalid_data`. + pub fn inc_ethereum_decode_failure(&self, host_fn: &str, kind: &str) { + self.ethereum_decode_failures + .with_label_values(&[host_fn, kind][..]) + .inc(); + } + pub fn observe_handler_execution_time(&self, duration: f64, handler: &str) { self.handler_execution_time .with_label_values(&[handler][..]) diff --git a/runtime/wasm/src/host_exports.rs b/runtime/wasm/src/host_exports.rs index b6ab94a066f..35ae9cd4835 100644 --- a/runtime/wasm/src/host_exports.rs +++ b/runtime/wasm/src/host_exports.rs @@ -42,6 +42,52 @@ impl IntoTrap for HostExportError { } } +/// Why an `ethereum.decode` / `ethereum.decodeParams` call could not produce a +/// value. Both variants make the host function return `null` to the mapping. +#[derive(Debug)] +pub(crate) enum DecodeError { + /// The type string is not a valid ABI type. It is typically a literal in + /// the mapping, so this fails identically on every block: the subgraph can + /// never decode this value. + InvalidType { + types: String, + source: anyhow::Error, + }, + + /// The type is valid but `data` does not match it. This can legitimately + /// vary from one event to the next. + InvalidData { + types: String, + source: anyhow::Error, + }, +} + +impl DecodeError { + /// Metric label. Kept short and stable; operators alert on this. + pub(crate) fn kind(&self) -> &'static str { + match self { + DecodeError::InvalidType { .. } => "invalid_type", + DecodeError::InvalidData { .. } => "invalid_data", + } + } + + pub(crate) fn types(&self) -> &str { + match self { + DecodeError::InvalidType { types, .. } | DecodeError::InvalidData { types, .. } => { + types + } + } + } + + pub(crate) fn source(&self) -> &anyhow::Error { + match self { + DecodeError::InvalidType { source, .. } | DecodeError::InvalidData { source, .. } => { + source + } + } + } +} + pub struct HostExports { pub(crate) subgraph_id: DeploymentHash, subgraph_network: String, @@ -1217,13 +1263,16 @@ impl HostExports { Ok(encoded) } + /// The outer `Result` is whether the host function could run at all; gas + /// errors must abort the mapping rather than surface as `null`. The inner + /// one is the decode outcome, which the caller turns into `null`. pub(crate) fn ethereum_decode( &self, types: String, data: Vec, gas: &GasCounter, state: &mut BlockState, - ) -> Result { + ) -> Result, DeterministicHostError> { Self::track_gas_and_ops( gas, state, @@ -1231,9 +1280,7 @@ impl HostExports { "ethereum_decode", )?; - let ty: abi::DynSolType = types.parse().context("Failed to read types")?; - - ty.abi_decode(&data).context("Failed to decode") + Ok(decode_abi(&types, &data)) } /// Like [`Self::ethereum_decode`], but decodes `data` as ABI function @@ -1247,7 +1294,7 @@ impl HostExports { data: Vec, gas: &GasCounter, state: &mut BlockState, - ) -> Result { + ) -> Result, DeterministicHostError> { Self::track_gas_and_ops( gas, state, @@ -1255,9 +1302,7 @@ impl HostExports { "ethereum_decode_params", )?; - let ty: abi::DynSolType = types.parse().context("Failed to read types")?; - - ty.abi_decode_params(&data).context("Failed to decode") + Ok(decode_abi_params(&types, &data)) } pub(crate) fn yaml_from_bytes( @@ -1313,6 +1358,38 @@ fn bytes_to_string(logger: &Logger, bytes: Vec) -> String { s.trim_end_matches('\u{0000}').to_string() } +fn parse_type(types: &str) -> Result { + types + .parse::() + .map_err(|e| DecodeError::InvalidType { + types: types.to_string(), + source: anyhow::Error::new(e), + }) +} + +/// Decode `data` as a single ABI value of type `types`. +fn decode_abi(types: &str, data: &[u8]) -> Result { + let ty = parse_type(types)?; + + ty.abi_decode(data).map_err(|e| DecodeError::InvalidData { + types: types.to_string(), + source: anyhow::Error::new(e), + }) +} + +/// Like [`decode_abi`], but decodes `data` as ABI function parameters (the +/// layout used by transaction calldata and event data) rather than as a single +/// ABI value. +fn decode_abi_params(types: &str, data: &[u8]) -> Result { + let ty = parse_type(types)?; + + ty.abi_decode_params(data) + .map_err(|e| DecodeError::InvalidData { + types: types.to_string(), + source: anyhow::Error::new(e), + }) +} + /// Expose some host functions for testing only #[cfg(debug_assertions)] pub mod test_support { @@ -1412,3 +1489,91 @@ fn bytes_to_string_is_lossy() { ) ) } + +#[cfg(test)] +mod decode_tests { + use super::*; + + /// `(uint32, bytes32)` holding `7` and 32 bytes of `0xaa`. Both fields are + /// static, so `abi_decode` and `abi_decode_params` accept the same layout. + fn encoded_uint32_bytes32() -> Vec { + let mut data = vec![0u8; 32]; + data[31] = 7; + data.extend_from_slice(&[0xaa; 32]); + data + } + + /// `bytes128` is not an ABI type at all — fixed size bytes stop at + /// `bytes32` — but ethabi read it as `FixedBytes(128)`, so subgraphs using + /// it kept working until v0.42.0. See #6683. + #[test] + fn unparseable_type_strings_are_invalid_type() { + let types = [ + "bytes128", + "(uint32,uint32,uint32,uint64,bytes32,bytes32,bytes32,bytes128)", + "(uint32,", + "uint7", + // Leading whitespace is only tolerated inside a tuple, so + // `"(uint256, address)"` parses but a bare `" address"` does not. + " address", + ]; + + for ty in types { + for err in [ + decode_abi(ty, &encoded_uint32_bytes32()).unwrap_err(), + decode_abi_params(ty, &encoded_uint32_bytes32()).unwrap_err(), + ] { + assert!( + matches!(err, DecodeError::InvalidType { .. }), + "expected `{ty}` to be rejected as an invalid type, got {err:?}" + ); + assert_eq!(err.kind(), "invalid_type"); + assert_eq!(err.types(), ty); + } + } + } + + /// Data that cannot be read against an otherwise valid type. Unlike an + /// unparseable type string this can legitimately differ per event, which is + /// why the two are kept apart. + #[test] + fn data_not_matching_a_valid_type_is_invalid_data() { + for data in [vec![], vec![0u8; 8], vec![0u8; 63]] { + for err in [ + decode_abi("(uint32,bytes32)", &data).unwrap_err(), + decode_abi_params("(uint32,bytes32)", &data).unwrap_err(), + ] { + assert!( + matches!(err, DecodeError::InvalidData { .. }), + "expected {} bytes to fail as invalid data, got {err:?}", + data.len() + ); + assert_eq!(err.kind(), "invalid_data"); + assert_eq!(err.types(), "(uint32,bytes32)"); + } + } + } + + #[test] + fn valid_type_and_data_decodes() { + for decoded in [ + decode_abi("(uint32,bytes32)", &encoded_uint32_bytes32()).unwrap(), + decode_abi_params("(uint32,bytes32)", &encoded_uint32_bytes32()).unwrap(), + ] { + let abi::DynSolValue::Tuple(fields) = decoded else { + panic!("expected a tuple, got {decoded:?}"); + }; + + assert!( + matches!(fields[0], abi::DynSolValue::Uint(v, 32) if v == abi::AlloyU256::from(7)), + "unexpected first field: {:?}", + fields[0] + ); + assert!( + matches!(&fields[1], abi::DynSolValue::FixedBytes(b, 32) if b[..32] == [0xaa; 32]), + "unexpected second field: {:?}", + fields[1] + ); + } + } +} diff --git a/runtime/wasm/src/module/context.rs b/runtime/wasm/src/module/context.rs index 70d65489304..647e9a8e723 100644 --- a/runtime/wasm/src/module/context.rs +++ b/runtime/wasm/src/module/context.rs @@ -15,6 +15,7 @@ use never::Never; use crate::HostExports; use crate::asc_abi::class::*; +use crate::host_exports::DecodeError; use graph::data::store; use crate::ExperimentalFeatures; @@ -1152,12 +1153,10 @@ impl WasmInstanceContext<'_> { let token = asc_get(self, token_ptr, gas)?; let host_exports = self.as_ref().ctx.host_exports.cheap_clone(); let ctx = &mut self.as_mut().ctx; - let data = host_exports.ethereum_encode(token, gas, &mut ctx.state); - // return `null` if it fails - match data { - Ok(bytes) => asc_new(self, &*bytes, gas).await, - Err(_) => Ok(AscPtr::null()), - } + // `abi_encode` is infallible, so the only error here is gas, which must + // abort the mapping rather than surface as `null`. + let bytes = host_exports.ethereum_encode(token, gas, &mut ctx.state)?; + asc_new(self, &*bytes, gas).await } /// function decode(types: String, data: Bytes): ethereum.Value | null @@ -1171,12 +1170,14 @@ impl WasmInstanceContext<'_> { let data = asc_get(self, data_ptr, gas)?; let host_exports = self.as_ref().ctx.host_exports.cheap_clone(); let ctx = &mut self.as_mut().ctx; - let result = host_exports.ethereum_decode(types, data, gas, &mut ctx.state); + let result = host_exports.ethereum_decode(types, data, gas, &mut ctx.state)?; - // return `null` if it fails match result { Ok(token) => asc_new(self, &token, gas).await, - Err(_) => Ok(AscPtr::null()), + Err(e) => { + self.report_decode_failure("ethereum.decode", &e); + Ok(AscPtr::null()) + } } } @@ -1191,15 +1192,53 @@ impl WasmInstanceContext<'_> { let data = asc_get(self, data_ptr, gas)?; let host_exports = self.as_ref().ctx.host_exports.cheap_clone(); let ctx = &mut self.as_mut().ctx; - let result = host_exports.ethereum_decode_params(types, data, gas, &mut ctx.state); + let result = host_exports.ethereum_decode_params(types, data, gas, &mut ctx.state)?; - // return `null` if it fails match result { Ok(token) => asc_new(self, &token, gas).await, - Err(_) => Ok(AscPtr::null()), + Err(e) => { + self.report_decode_failure("ethereum.decodeParams", &e); + Ok(AscPtr::null()) + } } } + /// Log a decode failure and count it in `deployment_ethereum_decode_failures`. + fn report_decode_failure(&self, host_fn: &'static str, err: &DecodeError) { + let data = self.as_ref(); + + // The type string comes from the mapping and has no length limit, and + // the parse error quotes it back, so cap both for logging. The value + // passed to the decoder is never truncated. + let types = truncate_for_logging(err.types()); + let source = truncate_for_logging(&format!("{:#}", err.source())); + + match err { + // Recurs on every matching trigger and never clears without + // republishing the subgraph. + DecodeError::InvalidType { .. } => error!( + data.ctx.logger, + "{} returned null: invalid ABI type string, so every such call fails \ + and the mapping may be dropping data", + host_fn; + "types" => &types, + "kind" => err.kind(), + "error" => &source, + ), + DecodeError::InvalidData { .. } => warn!( + data.ctx.logger, + "{} returned null: data does not match the type", + host_fn; + "types" => &types, + "kind" => err.kind(), + "error" => &source, + ), + } + + data.host_metrics + .inc_ethereum_decode_failure(host_fn, err.kind()); + } + /// function arweave.transactionData(txId: string): Bytes | null pub async fn arweave_transaction_data( &self, @@ -1272,6 +1311,15 @@ impl WasmInstanceContext<'_> { } } +/// Mapping-supplied strings have no length limit, so only log the first 1024 +/// characters. Splits on a character boundary since the string is arbitrary. +fn truncate_for_logging(s: &str) -> String { + match s.char_indices().nth(1024) { + Some((end, _)) => format!("(truncated) {}", &s[..end]), + None => s.to_string(), + } +} + /// For debugging, it might be useful to know exactly which bytes could not be parsed as YAML, but /// since we can parse large YAML documents, even one bad mapping could produce terabytes of logs. /// To avoid this, we only log the first 1024 bytes of the failed YAML source.