graph, runtime: Surface ethereum.decode failures

NEWS.md

@@ -1,5 +1,20 @@
 # 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))
+
+### 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 +134,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))

graph/src/components/subgraph/host.rs

@@ -100,6 +100,7 @@ pub struct HostMetrics {
     handler_execution_time: Box<HistogramVec>,
     host_fn_execution_time: Box<HistogramVec>,
     eth_call_execution_time: Box<HistogramVec>,
+    ethereum_decode_failures: Box<CounterVec>,
     pub gas_metrics: GasMetrics,
     pub stopwatch: StopwatchMetrics,
 }
@@ -139,15 +140,34 @@ 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` distinguishes an unparseable type string from data that does not
+    /// match a valid type; the two mean very different things about a subgraph.
+    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][..])

runtime/wasm/src/host_exports.rs

@@ -42,6 +42,54 @@ 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,
+/// but they say very different things about the subgraph, so they are counted
+/// and reported separately.
+#[derive(Debug)]
+pub(crate) enum DecodeError {
+    /// The type string is not a valid ABI type. The mapping passes a literal
+    /// here, 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,23 +1265,24 @@ impl HostExports {
         Ok(encoded)
     }
 
+    /// The outer `Result` is whether the host function could run at all; gas
+    /// errors must reach the mapping as errors, never as a `null` value. The
+    /// inner one is the decode outcome, which the caller turns into `null`.
     pub(crate) fn ethereum_decode(
         &self,
         types: String,
         data: Vec<u8>,
         gas: &GasCounter,
         state: &mut BlockState,
-    ) -> Result<abi::DynSolValue, anyhow::Error> {
+    ) -> Result<Result<abi::DynSolValue, DecodeError>, DeterministicHostError> {
         Self::track_gas_and_ops(
             gas,
             state,
             gas::DEFAULT_GAS_OP.with_args(complexity::Size, &data),
             "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,17 +1296,15 @@ impl HostExports {
         data: Vec<u8>,
         gas: &GasCounter,
         state: &mut BlockState,
-    ) -> Result<abi::DynSolValue, anyhow::Error> {
+    ) -> Result<Result<abi::DynSolValue, DecodeError>, DeterministicHostError> {
         Self::track_gas_and_ops(
             gas,
             state,
             gas::DEFAULT_GAS_OP.with_args(complexity::Size, &data),
             "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 +1360,38 @@ fn bytes_to_string(logger: &Logger, bytes: Vec<u8>) -> String {
     s.trim_end_matches('\u{0000}').to_string()
 }
 
+fn parse_type(types: &str) -> Result<abi::DynSolType, DecodeError> {
+    types
+        .parse::<abi::DynSolType>()
+        .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<abi::DynSolValue, DecodeError> {
+    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<abi::DynSolValue, DecodeError> {
+    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 +1491,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<u8> {
+        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]
+            );
+        }
+    }
+}

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;
@@ -1171,12 +1172,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,13 +1194,50 @@ 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())
+            }
+        }
+    }
+
+    /// Both decode host functions report failure to the mapping as `null`,
+    /// which a mapping is free to handle by skipping the event. That makes a
+    /// permanently broken decode indistinguishable from a single odd event, and
+    /// lets a subgraph drop entities while staying healthy and synced, so
+    /// record it host-side where an operator can see it.
+    fn report_decode_failure(&self, host_fn: &'static str, err: &DecodeError) {
+        let data = self.as_ref();
+
+        match err {
+            // Type strings are literals in practice, so this recurs on every
+            // matching trigger and will not clear on its own. Hence error
+            // rather than warn: it always needs a republished 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" => err.types(),
+                "kind" => err.kind(),
+                "error" => format!("{:#}", err.source()),
+            ),
+            DecodeError::InvalidData { .. } => warn!(
+                data.ctx.logger,
+                "{} returned null: data does not match the type",
+                host_fn;
+                "types" => err.types(),
+                "kind" => err.kind(),
+                "error" => format!("{:#}", err.source()),
+            ),
         }
+
+        data.host_metrics
+            .inc_ethereum_decode_failure(host_fn, err.kind());
     }
 
     /// function arweave.transactionData(txId: string): Bytes | null