Skip to content

Clarify entry-count logging and harden manual key-byte parsing in subxt storage iteration #174

@bkontur

Description

@bkontur

Follow-up from a review discussion on #159:
#159 (comment)

The original observation

In provider-node/src/subxt_client.rs, an on-chain storage iteration used a separate entry_count counter that was incremented at the top of the loop, before several continue branches (malformed key length, account mismatch, parse failure):

let mut entry_count = 0u32;
while let Some(result) = entries.next().await {
    let entry = match result { Ok(e) => e, Err(e) => { ...; continue } };
    entry_count += 1;                       // counted before the filters below
    if key_len < 104 { ...; continue }
    if account_bytes != our_bytes { continue }
    let bucket_id = match key_bytes[48..56].try_into() { ...; continue };
    bucket_ids.push(bucket_id);
}
if entry_count > 0 {
    tracing::info!("Scanned {} agreement request entries, {} for us", entry_count, bucket_ids.len());
}

hmm, why not use just bucket_ids.len() here?

entry_count is confusing, it is incremented at the beginning between various continue;, so it is not obvious what entry_count means — success, correct, all, partial?

entry_count was meant as "total entries scanned" (distinct from bucket_ids.len() = "entries matching us"), but the name and mid-loop placement make that intent unclear.

Current status

The specific function this was on (fetch_pending_requests / the AgreementChainClient impl) was removed when agreement negotiation moved off-chain (#105), so the exact snippet no longer exists on the branch.

However, the same pattern persists in fetch_replica_agreements in the same file: it iterates the entire StorageAgreements map and parses keys via hardcoded byte offsets (key_bytes.len() < 32 + 16 + 8 + 16 + 32, bucket_id_start = 32 + 16, etc.). So this is worth carrying forward as cleanup rather than closing as moot.

Suggested work

  1. Logging clarity: where a "total scanned vs. matched" log is useful, give the counter an explicit name (e.g. scanned_entries) and a comment, or drop it and log only bucket_ids.len() / agreements.len(). Avoid incrementing a counter mid-loop where its meaning relative to the continue branches is ambiguous.
  2. Harden / replace manual key-byte parsing: the hardcoded offset arithmetic in fetch_replica_agreements (and the value decoders nearby) silently breaks if the on-chain StorageAgreements key/layout changes. Prefer decoding keys/values through subxt's typed/metadata-aware path (see the related discussion on robust value decoding from Reuse one SubxtChainClient struct that implements all chain client traits in provider-node #159) instead of slicing raw bytes.

Low priority / code-quality. The first item is a good-first-issue-sized change; the second is the more substantial follow-up.

Metadata

Metadata

Labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions