fix: map enumeration asked for more keys than the node allows
`/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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jp6a8EDar9ueEhAxzep4V5
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -111,6 +111,14 @@ pub struct ChainProperties {
|
||||
pub ss58_format: Option<u16>,
|
||||
}
|
||||
|
||||
/// 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]),
|
||||
|
||||
Reference in New Issue
Block a user