From 7d081c77461796ce41d0d5755422f6b95aeb7677 Mon Sep 17 00:00:00 2001 From: rob thijssen Date: Thu, 10 Sep 2026 10:50:14 +0300 Subject: [PATCH] fix: map enumeration asked for more keys than the node allows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `/heisenberg/reversible` reported 0 pending. The chain has 2. `state_getKeysPaged` caps `count` at 1,000 and *rejects* rather than truncates: `PENDING_LIMIT` was 1,600, so every call errored, and `.unwrap_or_default()` turned the error into an empty page. Nothing looked broken — it looked like a chain with nothing in flight, which is precisely what mainnet legitimately looks like. It would have stayed wrong indefinitely. Caught only because `ReversibleTransfers::NextTransactionId` is a plain counter reading 5 on Heisenberg, which contradicted the page. Every path tested during the original work happened to be under the cap — `GENESIS_LIMIT` is 500, and the `System::Account` check used 3 — so the machinery looked verified. Three fixes. The clamp lives in `rpc.rs`, because 1,000 is the node's ceiling rather than a caller's preference. The route no longer swallows the error: a chain whose runtime lacks the pallet already returns 404, so a failure here is a real fault and now says so. And the constant is under the cap so the common case is one request. Verified against Heisenberg's two real pending transfers, which also exercise the unknown-deadline path — both were scheduled before the event index reaches, so they render as pending with no countdown rather than being dropped. CLAUDE.md gains the general shape: `unwrap_or_default()` on anything that talks to a node is how a wrong answer comes to look like a right one. Closes #4 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Jp6a8EDar9ueEhAxzep4V5 --- CLAUDE.md | 9 +++++++++ crates/blackbeard-api/src/routes.rs | 20 +++++++++++++++----- crates/blackbeard-data/src/rpc.rs | 14 ++++++++++++++ 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 4679972..e662152 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -210,6 +210,15 @@ block. The first cut of the roles index treated "contains an account" as a role and labelled half the chain's active addresses `Events`. The test is `value` being a string that is itself in the account set. +**`state_getKeysPaged` rejects a count over 1,000, it does not truncate.** +Asking for 1,600 is an RPC *error*, and a caller that `unwrap_or_default()`s it +has turned "could not ask" into "the answer is nothing" — which for a map like +`PendingTransfers` reads exactly like a chain with nothing in flight. +`MAX_KEYS_PER_PAGE` clamps it in `rpc.rs`, because the ceiling is the node's +rather than a caller's preference. More generally: `unwrap_or_default()` on +anything that talks to a node is how a wrong answer comes to look like a right +one. + **A wrong storage hasher reads as absent, not as an error.** A key is `twox128(pallet prefix) ++ twox128(item) ++ hashed keys`, and each key's hash is declared per entry — `System::Account` is `Blake2_128Concat`, other maps are diff --git a/crates/blackbeard-api/src/routes.rs b/crates/blackbeard-api/src/routes.rs index 700ad65..9b3e21b 100644 --- a/crates/blackbeard-api/src/routes.rs +++ b/crates/blackbeard-api/src/routes.rs @@ -1091,10 +1091,14 @@ async fn genesis_accounts( /// How many pending transfers one request will enumerate. /// -/// `MaxPendingPerAccount` is 16, so this is a hundred accounts' worth. A cap -/// exists because enumeration is one RPC round trip per entry and an unbounded -/// map would be an unbounded page. -const PENDING_LIMIT: u32 = 1_600; +/// `MaxPendingPerAccount` is 16, so this is sixty accounts' worth. A cap exists +/// because enumeration is one RPC round trip per entry and an unbounded map +/// would be an unbounded page. +/// +/// Under `MAX_KEYS_PER_PAGE` deliberately, so this is one request. It was 1,600 +/// once, which the node rejects outright — and the rejection became an empty +/// page that read exactly like a chain with nothing in flight. +const PENDING_LIMIT: u32 = 1_000; /// Money in flight, and still cancellable. /// @@ -1169,7 +1173,13 @@ async fn reversible( .rpc .storage_keys_paged(&prefix, PENDING_LIMIT, None, None) .await - .unwrap_or_default(); + .map_err(|e| { + tracing::warn!(chain = %id, error = %e, "pending transfers could not be enumerated"); + Failure( + StatusCode::SERVICE_UNAVAILABLE, + ApiError::new("state_unavailable", e.to_string()), + ) + })?; let mut pending = Vec::new(); for key in keys { diff --git a/crates/blackbeard-data/src/rpc.rs b/crates/blackbeard-data/src/rpc.rs index b650a3b..b136f3a 100644 --- a/crates/blackbeard-data/src/rpc.rs +++ b/crates/blackbeard-data/src/rpc.rs @@ -111,6 +111,14 @@ pub struct ChainProperties { pub ss58_format: Option, } +/// The most keys `state_getKeysPaged` will return in one call. +/// +/// Substrate's own limit, and it *rejects* rather than truncates: asking for +/// 1,600 is `count exceeds maximum value. value: 1600, max: 1000`. A caller +/// that turns that error into an empty page has produced a wrong answer that +/// looks exactly like a right one. +pub const MAX_KEYS_PER_PAGE: u32 = 1000; + /// An HTTP JSON-RPC client for one node. #[derive(Debug, Clone)] pub struct RpcClient { @@ -212,6 +220,11 @@ impl RpcClient { /// /// `start` is the last key of the previous page, exclusive. A page shorter /// than `count` is the end. + /// + /// `count` is clamped to [`MAX_KEYS_PER_PAGE`]. That ceiling is the node's, + /// not a caller's preference, so it is enforced here rather than in each + /// caller's constant — asking for more is an RPC *error*, and an error that + /// a caller turns into an empty page reads exactly like an empty map. pub async fn storage_keys_paged( &self, prefix: &str, @@ -222,6 +235,7 @@ impl RpcClient { // `at` is what makes genesis readable: the endowed set exists only in // block zero's state, and enumerating it at the tip returns whatever // the map holds now instead. Needs an archive node. + let count = count.min(MAX_KEYS_PER_PAGE); let params = match (start, at) { (Some(s), Some(h)) => json!([prefix, count, s, h]), (Some(s), None) => json!([prefix, count, s]),