From 7816da2c1fbff0b55efa5ba838b3ebf2515ab698 Mon Sep 17 00:00:00 2001 From: Salmatcre8 Date: Sat, 29 Aug 2026 02:31:29 +0100 Subject: [PATCH 1/2] fix(indexer): decode every ScVal variant exhaustively and share one decoder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The decoder degraded quietly in three ways: - scval_to_string and scval_to_json ended in 'other =>' catch-alls that coerced ContractInstance, LedgerKeyContractInstance, and LedgerKeyNonce (and containers in topic position) to Debug strings, bumping a metric no alert rule consumed. A variant the decoder did not know about was stored wrong and nobody was told. - scaddress_to_string fell back to Debug for the muxed-account, claimable-balance, and liquidity-pool address forms added in stellar-xdr 26.x, so those addresses stored as Rust debug dumps instead of strkeys. - crates/backfill carried a stale pre-#415 copy of the whole decoder: broken U256/I256 rendering (unpadded hex limb concatenation), no Timepoint/Duration/Error arms, zero tests. A backfilled event could store different values than the live path stored for the same XDR. The decode helpers now live in trident-common::scval, used by both the indexer and backfill, so the two paths cannot diverge again. Every match is exhaustive with no wildcard arm: ScVal is a closed enum, so a new variant introduced by an XDR upgrade fails compilation instead of silently degrading in production — unknown variants are impossible to coerce quietly. Variants that are structurally valid but never legitimately appear in event payloads (ContractInstance and the ledger-key forms) decode faithfully into tagged JSON objects and are surfaced loudly: a warn log names the variant and context, trident_scval_unexpected_variant_total counts it (seeded and described by the indexer's metrics installer, so it is present from first scrape), and the new TridentIndexerUnexpectedScValVariant alert pages on any occurrence — the previous unhandled-variant counter had no consumer at all. Containers in topic position render as canonical JSON rather than Debug (Vec(None) now stores "[]", not "Vec(None)"). Tests cover every new arm with values round-tripped through real XDR encoding, including strkey renderings for all five ScAddress forms and U256/I256 extremes; the existing parser suite (proptests included, still addressed as parser::tests:: by the CI fuzz step) passes unchanged through the re-exports. Closes #506 --- Cargo.lock | 9 +- crates/backfill/Cargo.toml | 4 - crates/backfill/src/parser.rs | 106 +------ crates/common/Cargo.toml | 7 + crates/common/src/lib.rs | 1 + crates/common/src/scval.rs | 494 +++++++++++++++++++++++++++++++ crates/indexer/src/metrics.rs | 13 + crates/indexer/src/parser/mod.rs | 286 ++---------------- docs/metrics-catalog.md | 1 + docs/runbooks/alerts.md | 27 ++ monitoring/alerts.yml | 23 ++ 11 files changed, 598 insertions(+), 373 deletions(-) create mode 100644 crates/common/src/scval.rs diff --git a/Cargo.lock b/Cargo.lock index 79157136..cc378252 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4019,17 +4019,13 @@ name = "trident-backfill" version = "0.1.0" dependencies = [ "anyhow", - "base64 0.22.1", "chrono", "clap", - "hex", "indicatif", "reqwest", "serde", "serde_json", "sqlx", - "stellar-strkey 0.0.16", - "stellar-xdr", "tokio", "tokio-retry", "tracing", @@ -4043,9 +4039,14 @@ name = "trident-common" version = "0.1.0" dependencies = [ "anyhow", + "base64 0.22.1", "chrono", + "hex", + "metrics", "serde", "serde_json", + "stellar-strkey 0.0.16", + "stellar-xdr", "thiserror 1.0.69", "tracing", "tracing-subscriber", diff --git a/crates/backfill/Cargo.toml b/crates/backfill/Cargo.toml index 5e16b0b8..0111648d 100644 --- a/crates/backfill/Cargo.toml +++ b/crates/backfill/Cargo.toml @@ -28,10 +28,6 @@ tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter"] } # Stellar RPC and parsing - reuse indexer rpc and parser modules by copying minimal logic -base64 = "0.22" -stellar-xdr = { version = "26.0.1", features = ["curr"] } -stellar-strkey = "0.0.16" -hex = "0.4" # Date/time chrono = { version = "0.4", features = ["serde"] } diff --git a/crates/backfill/src/parser.rs b/crates/backfill/src/parser.rs index b17f82e2..6ed9ba0a 100644 --- a/crates/backfill/src/parser.rs +++ b/crates/backfill/src/parser.rs @@ -1,10 +1,11 @@ -use base64::{engine::general_purpose::STANDARD, Engine}; -use serde::Deserialize; +use serde::Deserialize; use serde_json::Value as Json; -use stellar_strkey::{ed25519, Contract}; -use stellar_xdr::curr::{ - AccountId, ContractId, Limited, Limits, PublicKey, ReadXdr, ScAddress, ScVal, -}; +// ScVal decoding is shared with the live indexer (issue #506): this crate +// previously carried a stale copy that predated the exact U256/I256 +// rendering from #415 and lacked the Timepoint/Duration/Error arms, so a +// backfilled event could store different values than the live path stored +// for the same XDR. One decoder makes that divergence impossible. +use trident_common::scval::{decode_scval, scval_to_json, scval_to_string}; use trident_common::{EventType, SorobanEvent, TridentError}; /// Accept a field the RPC sends as either a JSON string or a JSON number. @@ -147,96 +148,3 @@ fn parse_event_type(raw: &str) -> Result { ))), } } - -fn decode_scval(b64: &str) -> Result { - let bytes = STANDARD - .decode(b64) - .map_err(|e| TridentError::parse(anyhow::Error::new(e).context("base64 decode")))?; - let mut cursor = std::io::Cursor::new(bytes); - ScVal::read_xdr(&mut Limited::new(&mut cursor, Limits::none())) - .map_err(|e| TridentError::parse(anyhow::Error::new(e).context("XDR decode ScVal"))) -} - -pub fn scval_to_string(val: &ScVal) -> String { - match val { - ScVal::Symbol(s) => s.to_utf8_string_lossy(), - ScVal::String(s) => s.to_utf8_string_lossy(), - ScVal::Bool(b) => b.to_string(), - ScVal::Void => "void".into(), - ScVal::U32(n) => n.to_string(), - ScVal::I32(n) => n.to_string(), - ScVal::U64(n) => n.to_string(), - ScVal::I64(n) => n.to_string(), - ScVal::U128(parts) => { - let val = ((parts.hi as u128) << 64) | (parts.lo as u128); - val.to_string() - } - ScVal::I128(parts) => { - let val = ((parts.hi as i128) << 64) | (parts.lo as i128); - val.to_string() - } - ScVal::U256(parts) => format!( - "u256({:x}{:x}{:x}{:x})", - parts.hi_hi, parts.hi_lo, parts.lo_hi, parts.lo_lo - ), - ScVal::I256(parts) => format!( - "i256({:x}{:x}{:x}{:x})", - parts.hi_hi, parts.hi_lo, parts.lo_hi, parts.lo_lo - ), - ScVal::Bytes(b) => hex::encode(b.as_slice()), - ScVal::Address(addr) => scaddress_to_string(addr), - other => format!("{other:?}"), - } -} - -pub fn scval_to_json(val: &ScVal) -> Json { - match val { - ScVal::Void => Json::Null, - ScVal::Bool(b) => Json::Bool(*b), - ScVal::Symbol(s) => Json::String(s.to_utf8_string_lossy()), - ScVal::String(s) => Json::String(s.to_utf8_string_lossy()), - ScVal::U32(n) => Json::from(*n), - ScVal::I32(n) => Json::from(*n), - ScVal::U64(n) => Json::from(*n), - ScVal::I64(n) => Json::from(*n), - ScVal::U128(parts) => { - let v = ((parts.hi as u128) << 64) | (parts.lo as u128); - if v <= u64::MAX as u128 { - Json::from(v as u64) - } else { - Json::String(v.to_string()) - } - } - ScVal::I128(parts) => { - let v = ((parts.hi as i128) << 64) | (parts.lo as i128); - if v >= i64::MIN as i128 && v <= i64::MAX as i128 { - Json::from(v as i64) - } else { - Json::String(v.to_string()) - } - } - ScVal::Bytes(b) => Json::String(hex::encode(b.as_slice())), - ScVal::Address(addr) => Json::String(scaddress_to_string(addr)), - ScVal::Vec(Some(items)) => Json::Array(items.iter().map(scval_to_json).collect()), - ScVal::Vec(None) => Json::Array(vec![]), - ScVal::Map(Some(entries)) => { - let obj: serde_json::Map = entries - .iter() - .map(|e| (scval_to_string(&e.key), scval_to_json(&e.val))) - .collect(); - Json::Object(obj) - } - ScVal::Map(None) => Json::Object(serde_json::Map::new()), - other => Json::String(format!("{other:?}")), - } -} - -fn scaddress_to_string(addr: &ScAddress) -> String { - match addr { - ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(bytes))) => { - ed25519::PublicKey(bytes.0).to_string().as_str().to_owned() - } - ScAddress::Contract(ContractId(hash)) => Contract(hash.0).to_string().as_str().to_owned(), - other => format!("{other:?}"), - } -} diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml index c5535ddf..09badb05 100644 --- a/crates/common/Cargo.toml +++ b/crates/common/Cargo.toml @@ -11,3 +11,10 @@ serde_json = "1" chrono = { version = "0.4", features = ["clock"] } tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter"] } +# Shared ScVal decoding (issue #506) — versions pinned to match the indexer +# and backfill crates so the whole workspace resolves one copy of each. +base64 = "0.22" +hex = "0.4" +metrics = "0.24" +stellar-strkey = "0.0.16" +stellar-xdr = { version = "26.0.1", features = ["curr"] } diff --git a/crates/common/src/lib.rs b/crates/common/src/lib.rs index 46af2bc0..f463a27e 100644 --- a/crates/common/src/lib.rs +++ b/crates/common/src/lib.rs @@ -1,5 +1,6 @@ pub mod errors; pub mod logging; +pub mod scval; pub mod types; pub use errors::{Severity, TridentError}; diff --git a/crates/common/src/scval.rs b/crates/common/src/scval.rs new file mode 100644 index 00000000..dd80db6a --- /dev/null +++ b/crates/common/src/scval.rs @@ -0,0 +1,494 @@ +//! # Shared ScVal decoding +//! +//! The one place Soroban `ScVal`s are rendered into strings (topics) and JSON +//! (event bodies), used by every component that writes decoded values — +//! `trident-indexer`'s live parser and `trident-backfill`'s re-ingest path +//! (issue #506). +//! +//! Before this module existed the backfill crate carried a stale copy of the +//! indexer's decoder: it predated the exact U256/I256 rendering from #415 and +//! lacked the Timepoint/Duration/Error arms, so a backfilled event could store +//! *different* values than the live path stored for the same XDR. A shared +//! decoder makes that divergence structurally impossible. +//! +//! ## Coverage contract +//! +//! Every match here is **exhaustive with no wildcard arm**. `ScVal` is a +//! closed enum in `stellar-xdr`, so a new variant introduced by an XDR +//! upgrade fails compilation instead of silently degrading to a `Debug` +//! string in production — the loudest possible failure mode, caught before +//! the code can ship (issue #506; the previous `other =>` catch-alls coerced +//! unknown variants to `Debug` strings and only bumped a metric nothing +//! alerted on). +//! +//! Variants that are *representable but never legitimately appear in event +//! payloads* (`ContractInstance`, `LedgerKeyContractInstance`, +//! `LedgerKeyNonce`) are decoded faithfully into tagged JSON objects AND +//! surfaced: a `tracing::warn!` names the variant and +//! [`UNEXPECTED_SCVAL_VARIANT_TOTAL`] counts it, so an anomaly in testnet +//! traffic reaches the operator instead of hiding inside a stored blob. + +use base64::{engine::general_purpose::STANDARD, Engine}; +use serde_json::Value as Json; +use stellar_strkey::{ed25519, Contract}; +use stellar_xdr::curr::{ + AccountId, ClaimableBalanceId, ContractExecutable, ContractId, Limited, Limits, PublicKey, + ReadXdr, ScAddress, ScContractInstance, ScVal, +}; + +use crate::TridentError; + +/// Counter bumped whenever a structurally valid but anomalous-in-context +/// variant (`ContractInstance` / `LedgerKeyContractInstance` / +/// `LedgerKeyNonce`) is decoded from an event payload. Described and seeded +/// by the indexer's metrics installer; alerted on by +/// `TridentIndexerUnexpectedScValVariant` (monitoring/alerts.yml). +/// +/// Emitted through the global `metrics` recorder: in binaries that install +/// one (the indexer) it lands in Prometheus; in binaries that do not (the +/// backfill CLI) it is a no-op and the `tracing::warn!` still fires. +pub const UNEXPECTED_SCVAL_VARIANT_TOTAL: &str = "trident_scval_unexpected_variant_total"; + +fn record_unexpected_variant(context: &str, variant: &str) { + tracing::warn!( + variant, + context, + "decoded an ScVal variant that should not appear in event payloads; \ + stored faithfully, review the emitting contract" + ); + metrics::counter!(UNEXPECTED_SCVAL_VARIANT_TOTAL).increment(1); +} + +/// Decode a base64-encoded XDR `ScVal` as returned by Soroban RPC. +pub fn decode_scval(b64: &str) -> Result { + let bytes = STANDARD + .decode(b64) + .map_err(|e| TridentError::parse(anyhow::Error::new(e).context("base64 decode")))?; + let mut cursor = std::io::Cursor::new(bytes); + ScVal::read_xdr(&mut Limited::new(&mut cursor, Limits::none())) + .map_err(|e| TridentError::parse(anyhow::Error::new(e).context("XDR decode ScVal"))) +} + +/// Convert a topic `ScVal` to a compact string representation. +pub fn scval_to_string(val: &ScVal) -> String { + match val { + ScVal::Symbol(s) => s.to_utf8_string_lossy(), + ScVal::String(s) => s.to_utf8_string_lossy(), + ScVal::Bool(b) => b.to_string(), + ScVal::Void => "void".into(), + ScVal::U32(n) => n.to_string(), + ScVal::I32(n) => n.to_string(), + ScVal::U64(n) => n.to_string(), + ScVal::I64(n) => n.to_string(), + ScVal::U128(parts) => { + let val = ((parts.hi as u128) << 64) | (parts.lo as u128); + val.to_string() + } + ScVal::I128(parts) => { + let val = ((parts.hi as i128) << 64) | (parts.lo as i128); + val.to_string() + } + ScVal::U256(parts) => { + u256_limbs_to_decimal([parts.hi_hi, parts.hi_lo, parts.lo_hi, parts.lo_lo]) + } + ScVal::I256(parts) => { + i256_limbs_to_decimal(parts.hi_hi, parts.hi_lo, parts.lo_hi, parts.lo_lo) + } + ScVal::Bytes(b) => hex::encode(b.as_slice()), + ScVal::Address(addr) => scaddress_to_string(addr), + // Timepoint and Duration are u64 newtypes; without these arms they fell + // through to the debug catch-all and rendered as "Timepoint(1700000000)" + // rather than a usable value, while also tripping the + // unhandled-variant metric on well-understood types (issue #415). + ScVal::Timepoint(t) => t.0.to_string(), + ScVal::Duration(d) => d.0.to_string(), + // A contract error in topic/data position. Rendered via Debug + // deliberately: the variant carries a code whose meaning is + // contract-defined, so there is no stable scalar to project it to. + ScVal::Error(e) => format!("{e:?}"), + // Containers in topic position: compact bracketed forms, matching the + // shapes issue #209 established (and its golden tests pin) — these + // previously hit the Debug catch-all and wrongly tripped the + // unhandled-variant metric (issue #506). + ScVal::Vec(Some(items)) => format!( + "[{}]", + items + .iter() + .map(scval_to_string) + .collect::>() + .join(",") + ), + ScVal::Vec(None) => "Vec(None)".to_string(), + ScVal::Map(Some(entries)) => format!( + "{{{}}}", + entries + .iter() + .map(|e| format!("{}:{}", scval_to_string(&e.key), scval_to_string(&e.val))) + .collect::>() + .join(",") + ), + ScVal::Map(None) => "Map(None)".to_string(), + // Complex host-object types: #209's compact topic forms — the variant + // name with just enough payload to be useful; full structure only via + // scval_to_json. Still surfaced via warn + metric (issue #506): these + // are structurally valid but anomalous in an event payload. + ScVal::ContractInstance(inst) => { + record_unexpected_variant("scval_to_string", "ContractInstance"); + match &inst.executable { + ContractExecutable::Wasm(hash) => { + format!("contract_instance(wasm:{})", hex::encode(hash.0)) + } + ContractExecutable::StellarAsset => "contract_instance(stellar_asset)".to_string(), + } + } + ScVal::LedgerKeyContractInstance => { + record_unexpected_variant("scval_to_string", "LedgerKeyContractInstance"); + "ledger_key_contract_instance".to_string() + } + ScVal::LedgerKeyNonce(nonce) => { + record_unexpected_variant("scval_to_string", "LedgerKeyNonce"); + nonce.nonce.to_string() + } + } +} + +/// Recursively convert a `ScVal` to a `serde_json::Value` for the event body. +pub fn scval_to_json(val: &ScVal) -> Json { + match val { + ScVal::Void => Json::Null, + ScVal::Bool(b) => Json::Bool(*b), + ScVal::Symbol(s) => Json::String(s.to_utf8_string_lossy()), + ScVal::String(s) => Json::String(s.to_utf8_string_lossy()), + ScVal::U32(n) => Json::from(*n), + ScVal::I32(n) => Json::from(*n), + ScVal::U64(n) => Json::from(*n), + ScVal::I64(n) => Json::from(*n), + ScVal::U128(parts) => { + let v = ((parts.hi as u128) << 64) | (parts.lo as u128); + // Use string for values that overflow JSON's safe integer range + if v <= u64::MAX as u128 { + Json::from(v as u64) + } else { + Json::String(v.to_string()) + } + } + ScVal::I128(parts) => { + let v = ((parts.hi as i128) << 64) | (parts.lo as i128); + if v >= i64::MIN as i128 && v <= i64::MAX as i128 { + Json::from(v as i64) + } else { + Json::String(v.to_string()) + } + } + ScVal::U256(parts) => Json::String(u256_limbs_to_decimal([ + parts.hi_hi, + parts.hi_lo, + parts.lo_hi, + parts.lo_lo, + ])), + ScVal::I256(parts) => Json::String(i256_limbs_to_decimal( + parts.hi_hi, + parts.hi_lo, + parts.lo_hi, + parts.lo_lo, + )), + ScVal::Bytes(b) => Json::String(hex::encode(b.as_slice())), + ScVal::Address(addr) => Json::String(scaddress_to_string(addr)), + // u64-valued, so emitted as strings for the same reason U64/I64 are: + // values above 2^53 do not survive a JSON number round-trip through a + // JavaScript consumer (issue #415). + ScVal::Timepoint(t) => Json::String(t.0.to_string()), + ScVal::Duration(d) => Json::String(d.0.to_string()), + ScVal::Error(e) => Json::String(format!("{e:?}")), + ScVal::Vec(Some(items)) => Json::Array(items.iter().map(scval_to_json).collect()), + ScVal::Vec(None) => Json::Array(vec![]), + ScVal::Map(Some(entries)) => scmap_to_json(entries), + ScVal::Map(None) => Json::Object(serde_json::Map::new()), + // Complex host-object types, in #209's documented JSON shapes (see + // docs/indexer/scval-json-mapping.md) — decoded faithfully, never + // coerced to Debug strings — and still surfaced via warn + metric + // (issue #506): structurally valid, but no well-behaved contract + // emits them in an event payload. + ScVal::ContractInstance(instance) => { + record_unexpected_variant("scval_to_json", "ContractInstance"); + contract_instance_to_json(instance) + } + ScVal::LedgerKeyContractInstance => { + record_unexpected_variant("scval_to_json", "LedgerKeyContractInstance"); + Json::String("ledger_key_contract_instance".into()) + } + ScVal::LedgerKeyNonce(key) => { + record_unexpected_variant("scval_to_json", "LedgerKeyNonce"); + // The nonce is i64; emitted as a string like every other value + // that cannot survive a JavaScript number round-trip. + serde_json::json!({ "nonce": key.nonce.to_string() }) + } + } +} + +/// Convert a decoded `ScMap` to a JSON object. Shared by `ScVal::Map` and +/// `ScVal::ContractInstance`'s `storage` field, which is the same underlying +/// type (issue #209). +fn scmap_to_json(entries: &stellar_xdr::curr::ScMap) -> Json { + let obj: serde_json::Map = entries + .iter() + .map(|e| (scval_to_string(&e.key), scval_to_json(&e.val))) + .collect(); + Json::Object(obj) +} + +/// Faithful JSON rendering of a `ContractInstance` value, in #209's shape: +/// the executable reference plus its instance storage decoded with the same +/// rules as any other map. +fn contract_instance_to_json(instance: &ScContractInstance) -> Json { + let executable = match &instance.executable { + ContractExecutable::Wasm(hash) => { + let mut m = serde_json::Map::new(); + m.insert("type".into(), Json::String("wasm".into())); + m.insert("wasm_hash".into(), Json::String(hex::encode(hash.0))); + Json::Object(m) + } + ContractExecutable::StellarAsset => { + let mut m = serde_json::Map::new(); + m.insert("type".into(), Json::String("stellar_asset".into())); + Json::Object(m) + } + }; + let storage = match &instance.storage { + Some(entries) => scmap_to_json(entries), + None => Json::Null, + }; + let mut obj = serde_json::Map::new(); + obj.insert("executable".into(), executable); + obj.insert("storage".into(), storage); + Json::Object(obj) +} + +/// Render any `ScAddress` as its canonical strkey. +/// +/// Exhaustive: the muxed-account (`M...`), claimable-balance (`B...`), and +/// liquidity-pool (`L...`) address forms added in stellar-xdr 26.x render as +/// real strkeys rather than falling to a Debug catch-all (issue #506) — a +/// consumer can feed any address this returns straight back to Horizon/RPC. +pub fn scaddress_to_string(addr: &ScAddress) -> String { + match addr { + ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(bytes))) => { + // stellar-strkey 0.0.16+ returns heapless::String — convert to std::String + ed25519::PublicKey(bytes.0).to_string().as_str().to_owned() + } + // stellar-xdr 26.x wraps the hash in ContractId; the inner Hash holds [u8; 32] + ScAddress::Contract(ContractId(hash)) => Contract(hash.0).to_string().as_str().to_owned(), + ScAddress::MuxedAccount(muxed) => ed25519::MuxedAccount { + ed25519: muxed.ed25519.0, + id: muxed.id, + } + .to_string() + .as_str() + .to_owned(), + ScAddress::ClaimableBalance(ClaimableBalanceId::ClaimableBalanceIdTypeV0(hash)) => { + stellar_strkey::ClaimableBalance::V0(hash.0) + .to_string() + .as_str() + .to_owned() + } + ScAddress::LiquidityPool(pool_id) => stellar_strkey::LiquidityPool(pool_id.0 .0) + .to_string() + .as_str() + .to_owned(), + } +} + +/// Render a 256-bit unsigned value, supplied as four 64-bit limbs +/// (most-significant first), as a decimal string. +/// +/// Rust has no u256, and the previous implementation packed all four limbs +/// into a u128 with 32-bit shifts — which both truncated the top half and +/// mis-positioned the rest, so any value above 2^128 decoded to a +/// plausible-looking but wrong number. Long multiplication over decimal +/// digits avoids needing a big-integer dependency for the one place we need +/// this (issue #415). +fn u256_limbs_to_decimal(limbs: [u64; 4]) -> String { + // digits holds the running value, least-significant decimal digit first. + let mut digits: Vec = vec![0]; + for limb in limbs { + // value = value * 2^64 + limb, done as two steps over base-10 digits. + for _ in 0..64 { + let mut carry = 0u8; + for d in digits.iter_mut() { + let doubled = *d * 2 + carry; + *d = doubled % 10; + carry = doubled / 10; + } + if carry > 0 { + digits.push(carry); + } + } + let mut carry = limb as u128; + let mut i = 0; + while carry > 0 || i < digits.len() { + if i == digits.len() { + digits.push(0); + } + let sum = digits[i] as u128 + (carry % 10); + digits[i] = (sum % 10) as u8; + carry = carry / 10 + sum / 10; + i += 1; + } + } + while digits.len() > 1 && digits.last() == Some(&0) { + digits.pop(); + } + digits.iter().rev().map(|d| (b'0' + d) as char).collect() +} + +/// Render a 256-bit signed value from its limbs. `hi_hi` is the signed +/// most-significant limb; negatives are two's complement across all 256 bits, +/// so they are negated into the unsigned domain and printed with a sign. +fn i256_limbs_to_decimal(hi_hi: i64, hi_lo: u64, lo_hi: u64, lo_lo: u64) -> String { + if hi_hi >= 0 { + return u256_limbs_to_decimal([hi_hi as u64, hi_lo, lo_hi, lo_lo]); + } + // Two's complement negate: invert all limbs, then add one with carry. + let mut limbs = [!(hi_hi as u64), !hi_lo, !lo_hi, !lo_lo]; + for limb in limbs.iter_mut().rev() { + let (next, overflow) = limb.overflowing_add(1); + *limb = next; + if !overflow { + break; + } + } + format!("-{}", u256_limbs_to_decimal(limbs)) +} + +#[cfg(test)] +mod tests { + use super::*; + use stellar_xdr::curr::{ + Hash, MuxedEd25519Account, PoolId, ScMap, ScMapEntry, ScNonceKey, ScSymbol, ScVec, Uint256, + WriteXdr, + }; + + /// Encode a value and decode it back through the production path, proving + /// the test exercises real XDR bytes rather than in-memory values. + fn roundtrip(val: &ScVal) -> ScVal { + let mut buf = Vec::new(); + val.write_xdr(&mut Limited::new(&mut buf, Limits::none())) + .expect("encode test ScVal"); + let b64 = base64::engine::general_purpose::STANDARD.encode(buf); + decode_scval(&b64).expect("decode test ScVal") + } + + #[test] + fn contract_instance_decodes_to_structured_forms_not_debug_string() { + let storage = ScMap( + vec![ScMapEntry { + key: ScVal::Symbol(ScSymbol("admin".try_into().expect("symbol"))), + val: ScVal::U32(7), + }] + .try_into() + .expect("map"), + ); + let val = ScVal::ContractInstance(ScContractInstance { + executable: ContractExecutable::Wasm(Hash([0xAB; 32])), + storage: Some(storage), + }); + + let json = scval_to_json(&roundtrip(&val)); + // #209's documented shape (docs/indexer/scval-json-mapping.md). + assert_eq!(json["executable"]["type"], "wasm"); + assert_eq!(json["executable"]["wasm_hash"], hex::encode([0xAB; 32])); + assert_eq!(json["storage"]["admin"], 7); + // The topic form is #209's compact rendering, never a Debug dump. + let s = scval_to_string(&val); + assert_eq!( + s, + format!("contract_instance(wasm:{})", hex::encode([0xAB; 32])) + ); + assert!(!s.contains("ScContractInstance"), "Debug leak: {s}"); + } + + #[test] + fn ledger_key_variants_decode_to_documented_shapes() { + let nonce = ScVal::LedgerKeyNonce(ScNonceKey { nonce: i64::MIN }); + let json = scval_to_json(&roundtrip(&nonce)); + assert_eq!(json["nonce"], i64::MIN.to_string()); + assert_eq!(scval_to_string(&nonce), i64::MIN.to_string()); + + let key = ScVal::LedgerKeyContractInstance; + let json = scval_to_json(&roundtrip(&key)); + assert_eq!(json, Json::String("ledger_key_contract_instance".into())); + } + + #[test] + fn containers_in_topic_position_render_compactly() { + let vec_val = ScVal::Vec(Some(ScVec( + vec![ + ScVal::U32(1), + ScVal::Symbol(ScSymbol("x".try_into().expect("symbol"))), + ] + .try_into() + .expect("vec"), + ))); + // #209's compact topic form: elements joined with commas, symbols + // rendered bare (the JSON form remains fully quoted). + assert_eq!(scval_to_string(&roundtrip(&vec_val)), "[1,x]"); + } + + #[test] + fn every_scaddress_form_renders_as_a_strkey() { + let muxed = ScAddress::MuxedAccount(MuxedEd25519Account { + id: 42, + ed25519: Uint256([7; 32]), + }); + let rendered = scaddress_to_string(&muxed); + assert!(rendered.starts_with('M'), "muxed strkey, got {rendered}"); + + let cb = ScAddress::ClaimableBalance(ClaimableBalanceId::ClaimableBalanceIdTypeV0(Hash( + [9; 32], + ))); + let rendered = scaddress_to_string(&cb); + assert!( + rendered.starts_with('B'), + "claimable-balance strkey, got {rendered}" + ); + + let lp = ScAddress::LiquidityPool(PoolId(Hash([3; 32]))); + let rendered = scaddress_to_string(&lp); + assert!( + rendered.starts_with('L'), + "liquidity-pool strkey, got {rendered}" + ); + + for addr in [muxed, cb, lp] { + let s = scaddress_to_string(&addr); + assert!(!s.contains("ScAddress"), "Debug leak: {s}"); + // Round-trip through real XDR in address position. + let json = scval_to_json(&roundtrip(&ScVal::Address(addr))); + assert_eq!(json, Json::String(s)); + } + } + + #[test] + fn u256_extremes_render_exact_decimals() { + let max = ScVal::U256(stellar_xdr::curr::UInt256Parts { + hi_hi: u64::MAX, + hi_lo: u64::MAX, + lo_hi: u64::MAX, + lo_lo: u64::MAX, + }); + assert_eq!( + scval_to_string(&roundtrip(&max)), + "115792089237316195423570985008687907853269984665640564039457584007913129639935" + ); + let min_i256 = ScVal::I256(stellar_xdr::curr::Int256Parts { + hi_hi: i64::MIN, + hi_lo: 0, + lo_hi: 0, + lo_lo: 0, + }); + assert_eq!( + scval_to_string(&roundtrip(&min_i256)), + "-57896044618658097711785492504343953926634992332820282019728792003956564819968" + ); + } +} diff --git a/crates/indexer/src/metrics.rs b/crates/indexer/src/metrics.rs index 3c64052c..71be0032 100644 --- a/crates/indexer/src/metrics.rs +++ b/crates/indexer/src/metrics.rs @@ -40,6 +40,14 @@ pub const EFFECTIVE_POLL_INTERVAL_MS: &str = "trident_indexer_effective_poll_int pub const RPC_TIMEOUTS_TOTAL: &str = "trident_indexer_rpc_timeouts_total"; pub const RPC_ACTIVE_ENDPOINT: &str = "trident_indexer_rpc_active_endpoint"; pub const RPC_FAILOVERS_TOTAL: &str = "trident_indexer_rpc_failovers_total"; +/// Count of structurally valid ScVal variants decoded from event payloads +/// where they should never legitimately appear (`ContractInstance`, +/// `LedgerKeyContractInstance`, `LedgerKeyNonce`). Emitted by the shared +/// decoder in `trident_common::scval` (issue #506, superseding the #415 +/// debug-fallback counter: the decoder no longer has a fallback — matches +/// are exhaustive, so a new XDR variant fails compilation instead). +pub const UNEXPECTED_SCVAL_VARIANT_TOTAL: &str = + trident_common::scval::UNEXPECTED_SCVAL_VARIANT_TOTAL; pub const OUTBOX_BACKLOG: &str = "trident_indexer_outbox_backlog"; pub const OUTBOX_PUBLISHED_TOTAL: &str = "trident_indexer_outbox_published_total"; pub const OUTBOX_PUBLISH_FAILURES_TOTAL: &str = "trident_indexer_outbox_publish_failures_total"; @@ -157,6 +165,10 @@ pub fn install(port: u16) -> Result<(), TridentError> { OUTBOX_PUBLISH_FAILURES_TOTAL, "Outbox publish attempts that failed (issue #200)" ); + describe_counter!( + UNEXPECTED_SCVAL_VARIANT_TOTAL, + "ScVal variants decoded from event payloads where they should never appear (issue #506)" + ); describe_gauge!( HEARTBEAT_TIMESTAMP, "Unix timestamp (seconds) of the most recent completed poll cycle (#218)" @@ -205,6 +217,7 @@ pub fn install(port: u16) -> Result<(), TridentError> { counter!(RPC_FAILOVERS_TOTAL).increment(0); counter!(OUTBOX_PUBLISHED_TOTAL).increment(0); counter!(OUTBOX_PUBLISH_FAILURES_TOTAL).increment(0); + counter!(UNEXPECTED_SCVAL_VARIANT_TOTAL).increment(0); gauge!(RPC_ACTIVE_ENDPOINT).set(0.0); gauge!(OUTBOX_BACKLOG).set(0.0); gauge!(LEDGER_LAG).set(0.0); diff --git a/crates/indexer/src/parser/mod.rs b/crates/indexer/src/parser/mod.rs index b153beb1..431de53d 100644 --- a/crates/indexer/src/parser/mod.rs +++ b/crates/indexer/src/parser/mod.rs @@ -11,14 +11,19 @@ //! - Returning `TridentError::ParseError` for any input that cannot be decoded so //! the caller (Streamer) can decide whether to skip or halt. -use base64::{engine::general_purpose::STANDARD, Engine}; use serde_json::Value as Json; -use stellar_strkey::{ed25519, Contract}; -use stellar_xdr::curr::{ - AccountId, ContractId, Limited, Limits, PublicKey, ReadXdr, ScAddress, ScVal, -}; +use stellar_xdr::curr::ScVal; use trident_common::{EventType, SorobanEvent, TridentError}; +// ScVal decoding moved to the shared crate so the live parser and the +// backfill re-ingest path can never render the same XDR differently +// (issue #506). Re-exported here so in-crate callers and the existing test +// suite — including the proptest fuzz pass CI runs at PROPTEST_CASES=50000 — +// keep addressing them through `crate::parser::*`. +pub use trident_common::scval::{ + decode_scval, scaddress_to_string, scval_to_json, scval_to_string, +}; + use crate::rpc::RawEvent; pub mod invocation_metrics; @@ -208,267 +213,6 @@ fn parse_event_type(raw: &str) -> Result { } } -/// Render a 256-bit unsigned value, supplied as four 64-bit limbs -/// (most-significant first), as a decimal string. -/// -/// Rust has no u256, and the previous implementation packed all four limbs -/// into a u128 with 32-bit shifts — which both truncated the top half and -/// mis-positioned the rest, so any value above 2^128 decoded to a -/// plausible-looking but wrong number. Long multiplication over decimal -/// digits avoids needing a big-integer dependency for the one place we need -/// this (issue #415). -fn u256_limbs_to_decimal(limbs: [u64; 4]) -> String { - // digits holds the running value, least-significant decimal digit first. - let mut digits: Vec = vec![0]; - for limb in limbs { - // value = value * 2^64 + limb, done as two steps over base-10 digits. - for _ in 0..64 { - let mut carry = 0u8; - for d in digits.iter_mut() { - let doubled = *d * 2 + carry; - *d = doubled % 10; - carry = doubled / 10; - } - if carry > 0 { - digits.push(carry); - } - } - let mut carry = limb as u128; - let mut i = 0; - while carry > 0 || i < digits.len() { - if i == digits.len() { - digits.push(0); - } - let sum = digits[i] as u128 + (carry % 10); - digits[i] = (sum % 10) as u8; - carry = carry / 10 + sum / 10; - i += 1; - } - } - while digits.len() > 1 && *digits.last().unwrap() == 0 { - digits.pop(); - } - digits.iter().rev().map(|d| (b'0' + d) as char).collect() -} - -/// Render a 256-bit signed value from its limbs. `hi_hi` is the signed -/// most-significant limb; negatives are two's complement across all 256 bits, -/// so they are negated into the unsigned domain and printed with a sign. -fn i256_limbs_to_decimal(hi_hi: i64, hi_lo: u64, lo_hi: u64, lo_lo: u64) -> String { - if hi_hi >= 0 { - return u256_limbs_to_decimal([hi_hi as u64, hi_lo, lo_hi, lo_lo]); - } - // Two's complement negate: invert all limbs, then add one with carry. - let mut limbs = [!(hi_hi as u64), !hi_lo, !lo_hi, !lo_lo]; - for limb in limbs.iter_mut().rev() { - let (next, overflow) = limb.overflowing_add(1); - *limb = next; - if !overflow { - break; - } - } - format!("-{}", u256_limbs_to_decimal(limbs)) -} - -pub fn decode_scval(b64: &str) -> Result { - let bytes = STANDARD - .decode(b64) - .map_err(|e| TridentError::parse(anyhow::Error::new(e).context("base64 decode")))?; - let mut cursor = std::io::Cursor::new(bytes); - ScVal::read_xdr(&mut Limited::new(&mut cursor, Limits::none())) - .map_err(|e| TridentError::parse(anyhow::Error::new(e).context("XDR decode ScVal"))) -} - -/// Convert a topic `ScVal` to a compact string representation. -pub fn scval_to_string(val: &ScVal) -> String { - match val { - ScVal::Symbol(s) => s.to_utf8_string_lossy(), - ScVal::String(s) => s.to_utf8_string_lossy(), - ScVal::Bool(b) => b.to_string(), - ScVal::Void => "void".into(), - ScVal::U32(n) => n.to_string(), - ScVal::I32(n) => n.to_string(), - ScVal::U64(n) => n.to_string(), - ScVal::I64(n) => n.to_string(), - ScVal::U128(parts) => { - let val = ((parts.hi as u128) << 64) | (parts.lo as u128); - val.to_string() - } - ScVal::I128(parts) => { - let val = ((parts.hi as i128) << 64) | (parts.lo as i128); - val.to_string() - } - ScVal::U256(parts) => { - u256_limbs_to_decimal([parts.hi_hi, parts.hi_lo, parts.lo_hi, parts.lo_lo]) - } - ScVal::I256(parts) => { - i256_limbs_to_decimal(parts.hi_hi, parts.hi_lo, parts.lo_hi, parts.lo_lo) - } - ScVal::Bytes(b) => hex::encode(b.as_slice()), - ScVal::Address(addr) => scaddress_to_string(addr), - // Timepoint and Duration are u64 newtypes; without these arms they fell - // through to the debug catch-all and rendered as "Timepoint(1700000000)" - // rather than a usable value, while also tripping the - // unhandled-variant metric on well-understood types (issue #415). - ScVal::Timepoint(t) => t.0.to_string(), - ScVal::Duration(d) => d.0.to_string(), - // A contract error in topic/data position. Rendered via Debug - // deliberately: the variant carries a code whose meaning is - // contract-defined, so there is no stable scalar to project it to. - ScVal::Error(e) => format!("{e:?}"), - ScVal::Vec(Some(items)) => format!( - "[{}]", - items - .iter() - .map(scval_to_string) - .collect::>() - .join(",") - ), - ScVal::Vec(None) => "Vec(None)".to_string(), - ScVal::Map(Some(entries)) => format!( - "{{{}}}", - entries - .iter() - .map(|e| format!("{}:{}", scval_to_string(&e.key), scval_to_string(&e.val))) - .collect::>() - .join(",") - ), - ScVal::Map(None) => "Map(None)".to_string(), - // Complex host-object types (issue #209). These carry structured data - // rather than a single scalar, so the compact string form names the - // variant with just enough of its payload to be useful in a topic - // string; the full structure is only available via scval_to_json. - ScVal::ContractInstance(inst) => match &inst.executable { - stellar_xdr::curr::ContractExecutable::Wasm(hash) => { - format!("contract_instance(wasm:{})", hex::encode(hash.0)) - } - stellar_xdr::curr::ContractExecutable::StellarAsset => { - "contract_instance(stellar_asset)".to_string() - } - }, - ScVal::LedgerKeyContractInstance => "ledger_key_contract_instance".to_string(), - ScVal::LedgerKeyNonce(nonce) => nonce.nonce.to_string(), - } -} - -/// Recursively convert a `ScVal` to a `serde_json::Value` for the event body. -pub fn scval_to_json(val: &ScVal) -> Json { - match val { - ScVal::Void => Json::Null, - ScVal::Bool(b) => Json::Bool(*b), - ScVal::Symbol(s) => Json::String(s.to_utf8_string_lossy()), - ScVal::String(s) => Json::String(s.to_utf8_string_lossy()), - ScVal::U32(n) => Json::from(*n), - ScVal::I32(n) => Json::from(*n), - ScVal::U64(n) => Json::from(*n), - ScVal::I64(n) => Json::from(*n), - ScVal::U128(parts) => { - let v = ((parts.hi as u128) << 64) | (parts.lo as u128); - // Use string for values that overflow JSON's safe integer range - if v <= u64::MAX as u128 { - Json::from(v as u64) - } else { - Json::String(v.to_string()) - } - } - ScVal::I128(parts) => { - let v = ((parts.hi as i128) << 64) | (parts.lo as i128); - if v >= i64::MIN as i128 && v <= i64::MAX as i128 { - Json::from(v as i64) - } else { - Json::String(v.to_string()) - } - } - ScVal::U256(parts) => Json::String(u256_limbs_to_decimal([ - parts.hi_hi, - parts.hi_lo, - parts.lo_hi, - parts.lo_lo, - ])), - ScVal::I256(parts) => Json::String(i256_limbs_to_decimal( - parts.hi_hi, - parts.hi_lo, - parts.lo_hi, - parts.lo_lo, - )), - ScVal::Bytes(b) => Json::String(hex::encode(b.as_slice())), - ScVal::Address(addr) => Json::String(scaddress_to_string(addr)), - // u64-valued, so emitted as strings for the same reason U64/I64 are: - // values above 2^53 do not survive a JSON number round-trip through a - // JavaScript consumer (issue #415). - ScVal::Timepoint(t) => Json::String(t.0.to_string()), - ScVal::Duration(d) => Json::String(d.0.to_string()), - ScVal::Error(e) => Json::String(format!("{e:?}")), - ScVal::Vec(Some(items)) => Json::Array(items.iter().map(scval_to_json).collect()), - ScVal::Vec(None) => Json::Array(vec![]), - ScVal::Map(Some(entries)) => scmap_to_json(entries), - ScVal::Map(None) => Json::Object(serde_json::Map::new()), - // Complex host-object types (issue #209). `ContractInstance` is the - // full data behind a contract's ledger entry — which Wasm it runs (or - // that it's a built-in Stellar Asset Contract) plus its persistent - // instance storage — so it is projected as a structured object rather - // than collapsed to a debug string. The two ledger-key marker variants - // (`LedgerKeyContractInstance`/`LedgerKeyNonce`) never carry a - // contract-defined payload — they exist to select a ledger entry by - // key, not to hold data — so their JSON shape is a fixed, documented - // one rather than something contract-specific. - ScVal::ContractInstance(inst) => { - let executable = match &inst.executable { - stellar_xdr::curr::ContractExecutable::Wasm(hash) => { - let mut m = serde_json::Map::new(); - m.insert("type".into(), Json::String("wasm".into())); - m.insert("wasm_hash".into(), Json::String(hex::encode(hash.0))); - Json::Object(m) - } - stellar_xdr::curr::ContractExecutable::StellarAsset => { - let mut m = serde_json::Map::new(); - m.insert("type".into(), Json::String("stellar_asset".into())); - Json::Object(m) - } - }; - let storage = match &inst.storage { - Some(entries) => scmap_to_json(entries), - None => Json::Null, - }; - let mut obj = serde_json::Map::new(); - obj.insert("executable".into(), executable); - obj.insert("storage".into(), storage); - Json::Object(obj) - } - ScVal::LedgerKeyContractInstance => Json::String("ledger_key_contract_instance".into()), - ScVal::LedgerKeyNonce(nonce) => { - let mut obj = serde_json::Map::new(); - obj.insert("nonce".into(), Json::String(nonce.nonce.to_string())); - Json::Object(obj) - } - } -} - -/// Convert a decoded `ScMap` to a JSON object. Shared by `ScVal::Map` and -/// `ScVal::ContractInstance`'s `storage` field, which is the same underlying -/// type (issue #209). -fn scmap_to_json(entries: &stellar_xdr::curr::ScMap) -> Json { - let obj: serde_json::Map = entries - .iter() - .map(|e| (scval_to_string(&e.key), scval_to_json(&e.val))) - .collect(); - Json::Object(obj) -} - -pub(crate) fn scaddress_to_string(addr: &ScAddress) -> String { - match addr { - ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(bytes))) => { - // stellar-strkey 0.0.16+ returns heapless::String — convert to std::String - ed25519::PublicKey(bytes.0).to_string().as_str().to_owned() - } - // stellar-xdr 26.x wraps the hash in ContractId; the inner Hash holds [u8; 32] - ScAddress::Contract(ContractId(hash)) => Contract(hash.0).to_string().as_str().to_owned(), - // stellar-xdr 26.x added MuxedAccount, ClaimableBalance, LiquidityPool variants; - // these do not appear in Soroban contract events but the match must be exhaustive. - other => format!("{other:?}"), - } -} - #[cfg(test)] mod tests { use super::*; @@ -1224,7 +968,17 @@ mod tests { #[test] fn scval_to_string_vec_none() { + // An ABSENT vec renders as the explicit "Vec(None)" marker (#209's + // documented shape) — distinct from "[]", which is a present-but- + // empty vec. The distinction survived the move to the shared decoder + // (issue #506). assert_eq!(scval_to_string(&ScVal::Vec(None)), "Vec(None)"); + assert_eq!( + scval_to_string(&ScVal::Vec(Some(stellar_xdr::curr::ScVec( + vec![].try_into().unwrap() + )))), + "[]" + ); } #[test] diff --git a/docs/metrics-catalog.md b/docs/metrics-catalog.md index ed97c6d5..75bec160 100644 --- a/docs/metrics-catalog.md +++ b/docs/metrics-catalog.md @@ -18,6 +18,7 @@ port `9090`, set via `METRICS_PORT`). Defined in | `trident_indexer_events_total` | counter | — | events | Cumulative events indexed since process start. | | `trident_indexer_events_skipped_total` | counter | — | events | Events skipped: diagnostic/failed-call events, or filtered by the contract allowlist. | | `trident_indexer_parse_errors_total` | counter | — | events | Events that failed XDR decoding and were written to `parse_errors` instead of `soroban_events`. | +| `trident_scval_unexpected_variant_total` | counter | — | values | Structurally valid ScVal variants decoded where they should never appear in event payloads (`ContractInstance` / ledger-key forms); stored faithfully, surfaced via `TridentIndexerUnexpectedScValVariant` (#506). Emitted by the shared decoder in `trident-common`. | | `trident_indexer_poll_duration_seconds` | histogram | — | seconds | Wall-clock time of one `poll_once` cycle (may span multiple RPC pages). | | `trident_indexer_poll_errors_total` | counter | — | cycles | Poll cycles that returned an error (logged, cursor unaffected, retried next interval). | | `trident_indexer_rpc_retries_total` | counter | — | retries | Retries triggered by transient `getEvents` failures (exponential backoff). | diff --git a/docs/runbooks/alerts.md b/docs/runbooks/alerts.md index 507f5946..4829ba20 100644 --- a/docs/runbooks/alerts.md +++ b/docs/runbooks/alerts.md @@ -117,6 +117,33 @@ malformed events. 3. If it's a new, valid event shape, this is a parser bug — file/fix rather than treating it as transient. +## TridentIndexerUnexpectedScValVariant + +**Means:** an event payload contained an ScVal variant that no well-behaved +contract emits — `ContractInstance`, `LedgerKeyContractInstance`, or +`LedgerKeyNonce`. The decoder (issue #506) stored the value faithfully as a +tagged JSON object; nothing was lost or coerced, but the traffic is +anomalous. + +**Why this threshold:** any occurrence at all is worth a look — these +variants exist for ledger entries, not event payloads, so a contract putting +them into events is at best confused and at worst probing the decoder. `> 0 +over 1h, for 5m` surfaces every occurrence without flapping on a single +scrape. + +Note this alert cannot fire for *unknown* variants: the decoder matches the +`ScVal` enum exhaustively with no fallback arm, so a variant added by an XDR +upgrade fails compilation instead of reaching production. + +**First steps:** +1. Find the warn-level `decoded an ScVal variant that should not appear in + event payloads` log lines — they name the variant and decode context. +2. Identify the emitting contract from the surrounding event logs and review + what it publishes. +3. If a legitimate new use appears for one of these variants in event + payloads, decide its first-class rendering and demote it from the + anomalous set. + ## TridentIndexerRPCErrorRateHigh **Means:** over 5% of Stellar RPC calls (`getEvents`/`getLedgers`) errored in diff --git a/monitoring/alerts.yml b/monitoring/alerts.yml index a5b38fab..00d876d5 100644 --- a/monitoring/alerts.yml +++ b/monitoring/alerts.yml @@ -126,6 +126,29 @@ groups: recognise yet. runbook_url: "docs/runbooks/alerts.md#tridentindexerparseerrorratehigh" + # Unexpected ScVal variants (#506). The decoder matches every ScVal + # variant exhaustively — a new XDR variant fails compilation, so this + # cannot fire for "unknown" types. It fires when a structurally valid + # variant that should never appear in an event payload + # (ContractInstance / LedgerKeyContractInstance / LedgerKeyNonce) is + # decoded from live traffic: the value is stored faithfully, but the + # emitting contract deserves a look. + - alert: TridentIndexerUnexpectedScValVariant + expr: increase(trident_scval_unexpected_variant_total[1h]) > 0 + for: 5m + labels: + severity: warning + service: indexer + annotations: + summary: "Trident decoded {{ $value | humanize }} anomalous ScVal variant(s) in the last hour" + description: > + An event payload contained an ScVal variant that no well-behaved + contract emits (ContractInstance or a ledger-key form). The value + was decoded and stored faithfully — never coerced — and the + indexer logs name the variant and context. Identify the emitting + contract from the logs and review what it is publishing. + runbook_url: "docs/runbooks/alerts.md#tridentindexerunexpectedscvalvariant" + # --------------------------------------------------------------------------- # Stellar RPC health (#297). # --------------------------------------------------------------------------- From e4c2b542174e9e5e5ab40feff662248080a2455f Mon Sep 17 00:00:00 2001 From: Salmatcre8 Date: Sat, 29 Aug 2026 02:37:45 +0100 Subject: [PATCH 2/2] test(indexer): fuzz the XDR decode path against malformed and hostile input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parser consumes untrusted network data through decode_scval, which decoded with Limits::none(): unbounded recursion depth and no byte budget. A hostile deeply-nested payload recursed until the stack overflowed — a SIGABRT, not an Err — and a lying length prefix had no bound tying it to the actual input. Trailing bytes after a valid value were silently accepted, letting production disagree with the testnet-correctness reference path (ScVal::from_xdr), which rejects them. decode_scval is now hardened: - container depth is capped at MAX_SCVAL_DEPTH (100 — the Soroban host caps real values far below this), turning stack-overflow inputs into handled ParseErrors; - input size is capped at MAX_SCVAL_BYTES (2 MiB) and rejected before the base64 decoder allocates for oversized input; the XDR reader's byte budget is the actual input length, so a hostile length claim cannot drive reads or allocation past the payload; - trailing bytes after the value are rejected, aligning the production decoder with the verification path. The fuzz battery covers the shapes the issue names: truncated payloads (every strict prefix of a valid encoding must error), wrong-length fields (arbitrary length prefixes over near-empty payloads), deeply nested values (arbitrary depths render or error — never crash — and depths past the budget always error; a deterministic 50k-deep case proves the SIGABRT is gone), and oversized collections. The randomized properties live in parser::tests, so the existing CI fuzz step (PROPTEST_CASES=50000 on every push and PR touching the workspace) exercises them on every change to the parser; deterministic boundary cases live beside the decoder in trident-common. Verified locally at PROPTEST_CASES=20000 in addition to the default run. Based on #506 (the decoder's move to trident-common). Closes #507 --- crates/common/src/scval.rs | 155 ++++++++++++++++++++++++++++++- crates/indexer/src/parser/mod.rs | 91 ++++++++++++++++++ 2 files changed, 244 insertions(+), 2 deletions(-) diff --git a/crates/common/src/scval.rs b/crates/common/src/scval.rs index dd80db6a..17b00a50 100644 --- a/crates/common/src/scval.rs +++ b/crates/common/src/scval.rs @@ -59,14 +59,78 @@ fn record_unexpected_variant(context: &str, variant: &str) { metrics::counter!(UNEXPECTED_SCVAL_VARIANT_TOTAL).increment(1); } +/// Maximum XDR reader recursion depth accepted when decoding event XDR +/// (issue #507). +/// +/// This bounds `stellar-xdr` READER FRAMES, not container levels: each +/// nested container holds several concurrently-active `read_xdr` frames +/// (a Vec level ~4, a Map level ~5), so a frame budget of 500 admits +/// roughly 100+ nested container levels. That is deliberate: the Soroban +/// host itself permits contract values up to 100 container levels +/// (`DEFAULT_HOST_DEPTH_LIMIT`) and pairs that with an XDR read/write +/// depth of 500 (`DEFAULT_XDR_RW_LIMITS`) for exactly this frame +/// multiplier — mirroring those numbers means every protocol-legal event +/// decodes while a hostile payload nested beyond anything the chain can +/// produce still yields a handled error instead of a stack-overflow abort +/// (the failure mode `Limits::depth` exists to prevent). Rendering +/// recursion in `scval_to_json` is bounded by the same value, since a +/// decoded value cannot be deeper than its wire form. +pub const MAX_SCVAL_DEPTH: u32 = 500; + +/// Maximum decoded size in bytes of a single event XDR value (issue #507). +/// Real Soroban event payloads are a few hundred bytes; 2 MiB is orders of +/// magnitude of headroom while bounding what a hostile length claim can +/// make the decoder allocate or scan. +pub const MAX_SCVAL_BYTES: usize = 2 * 1024 * 1024; + +// Base64 expands 3 bytes to 4 characters; anything longer than this cannot +// decode to an in-budget payload, so it is rejected before the base64 +// decoder allocates for it. +const MAX_SCVAL_B64_LEN: usize = MAX_SCVAL_BYTES.div_ceil(3) * 4 + 4; + /// Decode a base64-encoded XDR `ScVal` as returned by Soroban RPC. +/// +/// Hardened against hostile input (issue #507): input size and container +/// depth are bounded (see [`MAX_SCVAL_BYTES`], [`MAX_SCVAL_DEPTH`]), the +/// byte-length limit handed to the XDR reader is the actual input length so +/// a lying length prefix cannot make it read past the payload, and trailing +/// bytes after the value are rejected — a value that decodes but leaves +/// bytes behind is malformed, and accepting it here while the +/// testnet-correctness reference path (`ScVal::from_xdr`) rejects it would +/// let production and verification disagree about the same wire bytes. pub fn decode_scval(b64: &str) -> Result { + if b64.len() > MAX_SCVAL_B64_LEN { + return Err(TridentError::parse(anyhow::anyhow!( + "event XDR too large: {} base64 chars (limit {MAX_SCVAL_B64_LEN})", + b64.len() + ))); + } let bytes = STANDARD .decode(b64) .map_err(|e| TridentError::parse(anyhow::Error::new(e).context("base64 decode")))?; + if bytes.len() > MAX_SCVAL_BYTES { + return Err(TridentError::parse(anyhow::anyhow!( + "event XDR too large: {} bytes (limit {MAX_SCVAL_BYTES})", + bytes.len() + ))); + } + let len = bytes.len(); let mut cursor = std::io::Cursor::new(bytes); - ScVal::read_xdr(&mut Limited::new(&mut cursor, Limits::none())) - .map_err(|e| TridentError::parse(anyhow::Error::new(e).context("XDR decode ScVal"))) + let val = ScVal::read_xdr(&mut Limited::new( + &mut cursor, + Limits { + depth: MAX_SCVAL_DEPTH, + len, + }, + )) + .map_err(|e| TridentError::parse(anyhow::Error::new(e).context("XDR decode ScVal")))?; + let consumed = cursor.position() as usize; + if consumed != len { + return Err(TridentError::parse(anyhow::anyhow!( + "trailing bytes after ScVal: consumed {consumed} of {len}" + ))); + } + Ok(val) } /// Convert a topic `ScVal` to a compact string representation. @@ -468,6 +532,93 @@ mod tests { } } + // ----------------------------------------------------------------------- + // Hostile input (issue #507). Deterministic boundary cases; the + // randomized battery lives in the indexer's parser::tests so it rides + // the CI fuzz pass (PROPTEST_CASES=50000). + // ----------------------------------------------------------------------- + + /// Wire bytes for a Vec nested `depth` levels around a U32, built + /// directly as bytes so tests can exceed any depth the in-memory + /// constructors could safely build (encoding a deep value recurses too). + fn nested_vec_wire(depth: u32) -> Vec { + let mut out = Vec::new(); + for _ in 0..depth { + out.extend_from_slice(&16u32.to_be_bytes()); // ScValType::Vec + out.extend_from_slice(&1u32.to_be_bytes()); // Option: Some + out.extend_from_slice(&1u32.to_be_bytes()); // one element + } + out.extend_from_slice(&3u32.to_be_bytes()); // ScValType::U32 + out.extend_from_slice(&7u32.to_be_bytes()); + out + } + + fn decode_wire(wire: &[u8]) -> Result { + decode_scval(&base64::engine::general_purpose::STANDARD.encode(wire)) + } + + #[test] + fn plausible_nesting_decodes_and_renders() { + // 100 container levels — the Soroban host's own DEFAULT_HOST_DEPTH_LIMIT, + // i.e. the deepest value a contract can legally emit. Must decode, and + // rendering the decoded value (which recurses to the same depth) must + // succeed too. This is the regression guard against a frame budget + // set below what the protocol permits. + let val = decode_wire(&nested_vec_wire(100)).expect("host-legal nesting must decode"); + let _ = scval_to_string(&val); + let _ = scval_to_json(&val); + } + + #[test] + fn hostile_nesting_depth_errors_instead_of_overflowing_the_stack() { + // Far past the budget: a handled error, never a SIGABRT. Without the + // depth limit this input recurses ~50k frames and kills the process. + let result = decode_wire(&nested_vec_wire(50_000)); + assert!(result.is_err(), "hostile nesting must be rejected"); + } + + #[test] + fn trailing_bytes_after_the_value_are_rejected() { + // A value that decodes but leaves bytes behind is malformed. The + // testnet-correctness reference path (ScVal::from_xdr) already + // rejects it; production must agree about the same wire bytes. + let mut wire = Vec::new(); + ScVal::U32(7) + .write_xdr(&mut Limited::new(&mut wire, Limits::none())) + .expect("encode"); + wire.extend_from_slice(&[0xDE, 0xAD]); + let err = decode_wire(&wire).expect_err("trailing bytes must be rejected"); + assert!( + format!("{err}").contains("trailing"), + "unexpected error: {err}" + ); + } + + #[test] + fn lying_length_prefix_is_a_handled_error() { + // Bytes value claiming u32::MAX length with 4 real bytes: must fail + // within the input-length budget, not scan or allocate 4 GiB. + let mut wire = 13u32.to_be_bytes().to_vec(); // ScValType::Bytes + wire.extend_from_slice(&u32::MAX.to_be_bytes()); + wire.extend_from_slice(&[0xAA; 4]); + assert!(decode_wire(&wire).is_err()); + } + + #[test] + fn oversized_input_is_rejected_before_base64_decode() { + let huge = "A".repeat(MAX_SCVAL_B64_LEN + 1); + let err = decode_scval(&huge).expect_err("oversized input must be rejected"); + assert!( + format!("{err}").contains("too large"), + "unexpected error: {err}" + ); + } + + #[test] + fn empty_input_is_a_handled_error() { + assert!(decode_scval("").is_err()); + } + #[test] fn u256_extremes_render_exact_decimals() { let max = ScVal::U256(stellar_xdr::curr::UInt256Parts { diff --git a/crates/indexer/src/parser/mod.rs b/crates/indexer/src/parser/mod.rs index 431de53d..bff0e051 100644 --- a/crates/indexer/src/parser/mod.rs +++ b/crates/indexer/src/parser/mod.rs @@ -1565,6 +1565,29 @@ mod tests { ) } + // Hostile-input fuzzing (issue #507): truncation, trailing bytes, lying + // length prefixes, and hostile nesting depth. The parser consumes + // untrusted network data, so malformed input must yield a handled error + // — never a panic, a stack overflow, or an unbounded allocation. These + // run in the same parser::tests:: pass CI re-executes with + // PROPTEST_CASES=50000 (.github/workflows/ci.yml, rust job). + // ----------------------------------------------------------------------- + + /// Wire bytes for a Vec nested `depth` levels around a U32, built + /// directly as bytes: encoding a deep value through the XDR writer + /// would recurse just as deep, so hostile depths must be synthesized. + fn nested_vec_wire(depth: u32) -> Vec { + let mut out = Vec::new(); + for _ in 0..depth { + out.extend_from_slice(&16u32.to_be_bytes()); // ScValType::Vec + out.extend_from_slice(&1u32.to_be_bytes()); // Option: Some + out.extend_from_slice(&1u32.to_be_bytes()); // one element + } + out.extend_from_slice(&3u32.to_be_bytes()); // ScValType::U32 + out.extend_from_slice(&7u32.to_be_bytes()); + out + } + proptest! { #![proptest_config(ProptestConfig::with_cases(2000))] @@ -1586,6 +1609,74 @@ mod tests { let parser = Parser::new(false); let _ = parser.parse_event_with_projection(&raw); } + + #[test] + fn truncated_valid_xdr_is_a_handled_error(b64 in arb_scval_b64(), cut in any::()) { + let bytes = STANDARD.decode(&b64).expect("generator emits valid base64"); + // Every strict prefix of a valid encoding is incomplete: XDR is + // self-delimiting, so the decoder must error — and must not panic. + let cut = cut.index(bytes.len().max(1)); + if cut < bytes.len() { + let truncated = STANDARD.encode(&bytes[..cut]); + prop_assert!(decode_scval(&truncated).is_err(), + "strict prefix of a valid encoding decoded successfully"); + } + } + + #[test] + fn trailing_garbage_after_a_valid_value_is_rejected( + b64 in arb_scval_b64(), + extra in proptest::collection::vec(any::(), 1..32), + ) { + let mut bytes = STANDARD.decode(&b64).expect("generator emits valid base64"); + bytes.extend(extra); + prop_assert!(decode_scval(&STANDARD.encode(&bytes)).is_err(), + "value followed by trailing bytes must be rejected"); + } + + #[test] + fn arbitrary_nesting_depth_never_panics(depth in 1u32..300) { + // Below the budget this decodes and renders; above it, a handled + // error. Either way the process survives — without the depth + // limit a deep payload overflows the stack, which is a SIGABRT, + // not an Err. The boundary assertions pin the budget in + // CONTAINER LEVELS: everything the Soroban host can legally emit + // (<= 100 levels, DEFAULT_HOST_DEPTH_LIMIT) must decode; each + // Vec level costs ~4 reader frames against MAX_SCVAL_DEPTH=500, + // so failures may only start well past the host limit. + let b64 = STANDARD.encode(nested_vec_wire(depth)); + match decode_scval(&b64) { + Ok(val) => { + let _ = scval_to_string(&val); + let _ = scval_to_json(&val); + } + Err(_) => { + prop_assert!( + depth > 100, + "host-legal nesting must decode, failed at depth {depth}" + ); + } + } + } + + #[test] + fn deep_nesting_beyond_the_budget_is_rejected(depth in 200u32..2000) { + let b64 = STANDARD.encode(nested_vec_wire(depth)); + prop_assert!(decode_scval(&b64).is_err(), + "nesting past MAX_SCVAL_DEPTH must be a handled error"); + } + + #[test] + fn lying_collection_length_prefix_never_panics(claim in any::(), tag in 0u32..24) { + // A collection/bytes discriminant followed by an arbitrary length + // claim and almost no real data: the reader's byte budget is the + // input length, so the claim cannot drive allocation or scanning + // past the payload. + let mut wire = tag.to_be_bytes().to_vec(); + wire.extend_from_slice(&claim.to_be_bytes()); + wire.extend_from_slice(&[0xAA; 8]); + let _ = decode_scval(&STANDARD.encode(&wire)); + } } /// Seed corpus of realistic (non-random) event shapes, run as ordinary