paritytech / paritytech/web3-storage
Clarify entry-count logging and harden manual key-byte parsing in subxt storage iteration
@franciscoaguirre is already working on this.
Since Jun 15, 2026.
- Dominant language
- Rust
- Stars
- 12
- Forks
- 3
- Avg merge
- 2d 2h
- Merged PRs (30d)
- 33
Description
Follow-up from a review discussion on #159:
https://github.com/paritytech/web3-storage/pull/159#discussion_r3394267967
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_countis confusing, it is incremented at the beginning between variouscontinue;, so it is not obvious whatentry_countmeans — 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
- 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 onlybucket_ids.len()/agreements.len(). Avoid incrementing a counter mid-loop where its meaning relative to thecontinuebranches is ambiguous. - Harden / replace manual key-byte parsing: the hardcoded offset arithmetic in
fetch_replica_agreements(and the value decoders nearby) silently breaks if the on-chainStorageAgreementskey/layout changes. Prefer decoding keys/values through subxt's typed/metadata-aware path (see the related discussion on robust value decoding from #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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.