fix(sync): prevent redundant runtime download during warp sync by replghost · Pull Request #3200 · paritytech/smoldot

replghost · 2026-04-16T21:50:30Z

Summary

set_chain_information() in warp_sync.rs unconditionally resets warped_block_ty to AlreadyVerified and runtime_download to NotStarted, even when warp sync has already completed fragment verification and is ready for the runtime download. When a GrandPa commit advances finality by 1 block mid-warp-sync, this can deadlock the state machine or cause an unnecessary extra round-trip.

Problem

Sequence without fix:

Warp sync verifies final fragment → warped_block_ty = Normal, ready for runtime download
GrandPa commit for block N+1 → set_chain_information(N+1)
Unconditionally resets warped_block_ty = AlreadyVerified and runtime_download = NotStarted

Two consequences:

Deadlock near the tip: desired_requests needs Normal to return a runtime download request, but all sources are at or below the new finalized height — no warp sync fragments are requested either. The state machine is stuck permanently.
Extra round-trip mid-sync: If sources are far ahead, the machine requests another warp sync fragment, verifies it, returns to Normal, then starts the runtime download. One unnecessary P2P round-trip + verification cycle.

Fix

Three changes to set_chain_information:

Short-circuit on same header hash — if the incoming chain information has the same header hash as warped_header_hash, return early. Nothing to update.
Preserve warped_block_ty = Normal — a GrandPa commit advancing finality should not force another fragment verification round when we've already verified past that point.
Only reset runtime_download when the state root changed — cached storage proofs are valid as long as the state root matches. When an adjacent block has the same state root, preserve the runtime download state.

Test plan

grandpa_commit_mid_warp_sync_preserves_runtime_download — reproduces the production scenario: fragment verified at block 100 → GrandPa commit to 101 → assert runtime download still desired. Fails without fix.
set_chain_information_same_state_root_preserves_runtime_download — same-state-root preservation path. Fails without fix.
hint_causes_ancestor_key_in_desired_request / no_hint_requests_code_key — existing hint tests still pass.
cargo check -p smoldot -p smoldot-light clean
cargo fmt clean
cargo test -p smoldot --lib sync::warp_sync::tests — 4 passed, 0 failed

replghost · 2026-04-21T02:38:01Z

How often does this happen?

On every cold start on live networks. The race window is deterministic:

GrandPa commits arrive every ~6 seconds (every finalized block)
The vulnerable window is between "last warp sync fragment verified" and "runtime download completes" — several seconds for 1.5–2.5 MiB :code over P2P
A GrandPa commit landing in that window is near-certain, especially with fresh checkpoints (1–3 fragments), which is the common case

Reproduction

Use runtime-download-count.mjs from smolbench:

cd smolbench
CHAIN_SPEC=chain-specs/paseo.json RUNS=5 node bench/runtime-download-count.mjs

Without fix: storage_proofs=2 (double download) on most runs. The second proof request is the redundant re-download triggered by the GrandPa commit resetting warped_block_ty to AlreadyVerified.

With fix: storage_proofs=1 consistently.

Regression test

The Rust unit test in warp_sync.rs (set_chain_information_preserves_normal_warped_block_ty) reproduces the exact sequence offline:

Verify a warp sync fragment → enters Normal (runtime download phase)
Call set_chain_information (simulating GrandPa commit advancing finality)
Assert desired_requests() still yields StorageGetMerkleProof — no second fragment verification needed

Confirmed: fails without fix, passes with fix.

replghost · 2026-04-21T03:23:28Z

Empirical data: redundant runtime downloads on live networks

Ran runtime-download-count.mjs from smolbench against three networks (3 runs each, smoldot 3.0.0 — before this fix):

Network	Run	Time	Fragments	Storage Proofs	Runtime Builds	Bug?
Polkadot	1	30.4s	206	2	2	✗
Polkadot	2	27.3s	207	3	1	✗✗
Polkadot	3	28.4s	206	1	1	—
Kusama	1	119.2s	571	2	1	✗
Kusama	2	57.1s	571	2	1	✗
Kusama	3	110.3s	570	1	1	—
Westend	1	12.6s	419	1	1	—
Westend	2	12.4s	419	1	1	—
Westend	3	9.2s	419	2	2	✗

6 of 9 cold starts triggered redundant :code downloads. One Polkadot run downloaded the runtime three times (storage_proofs=3). Each redundant proof is ~1.5–2.5 MiB wasted bandwidth plus the fragment verification round-trip that precedes it.

Expected after fix: storage_proofs=1 consistently on all networks.

Copilot

Pull request overview

Adjusts the warp sync state machine so that advancing finalized chain information mid-warp-sync doesn’t unnecessarily force re-verification before runtime download, reducing redundant :code re-downloads and improving cold-start time.

Changes:

Preserve WarpedBlockTy::Normal in set_chain_information() so runtime download can proceed without an extra fragment verification cycle.
Add unit tests covering the regression scenario and code trie hint key selection in runtime download requests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-22T09:38:16Z

+            // Preserve `Normal` if warp sync already completed fragment verification.
+            // A GrandPa commit advancing finality by a few blocks should not force
+            // another round of fragment verification before the runtime download can
+            // proceed. The runtime_download reset below is still necessary (the block
+            // hash changed so any cached proof is for the wrong state root).


The explanatory comment block is duplicated verbatim (same 5 lines repeated twice), which adds noise and makes the intent harder to read. Please remove the duplicate copy and keep a single concise comment above the conditional.

Suggested change

// Preserve `Normal` if warp sync already completed fragment verification.

// A GrandPa commit advancing finality by a few blocks should not force

// another round of fragment verification before the runtime download can

// proceed. The runtime_download reset below is still necessary (the block

// hash changed so any cached proof is for the wrong state root).

Copilot · 2026-04-22T09:38:17Z

+        out.extend_from_slice(target_hash);
+        out.extend_from_slice(&num_bytes[..block_number_bytes]);
+        // 1 precommit, compact-encoded as 0x04


build_justification writes target_number into the justification using &num_bytes[..block_number_bytes], which will panic if block_number_bytes > 8. Consider mirroring the min(8, block_number_bytes) + zero-padding approach used for the signed message (or assert block_number_bytes <= 8).

Copilot · 2026-04-22T09:38:17Z

+        out.extend_from_slice(target_hash);
+        out.extend_from_slice(&num_bytes[..block_number_bytes]);
+        out.extend_from_slice(&<[u8; 64]>::from(signature));


Same issue as above: the precommit also encodes target_number via &num_bytes[..block_number_bytes], which can panic for block_number_bytes > 8. Use padded encoding (or an explicit assertion) to make the helper robust.

lexnv · 2026-04-22T11:08:22Z

+        ws.set_source_finality_state(source_id, 101);
+        let new_chain_info = chain_info_at_block(101, public_key, 1);
+        ws.set_chain_information((&new_chain_info).into());
+


IIUC: the test sets set_source_finality_state(source_id, 101) immediately before set_chain_information(chain_info_at_block(101)). Since they contain the same warped_header_number this makes the guared in the warp sync effectively skipped and therefore we wont download the runtime (with or without the patch)

You're right — the old test was trivially passing regardless of the patch. Replaced with grandpa_commit_mid_warp_sync_preserves_runtime_download which uses setup_warp_sync_at_normal (source at height 100, fragment verified) then advances to block 101 with the source at 101. This matches the production scenario and actually fails without the fix.

lexnv · 2026-04-22T11:09:24Z

 }
+
+#[cfg(test)]
+mod tests {


A production repro isn't demonstrated in any of the tests

Added. grandpa_commit_mid_warp_sync_preserves_runtime_download reproduces the production sequence: fragment verified at block 100 → warped_block_ty=Normal → GrandPa commit advances to 101 → set_chain_information(101) → assert runtime download still desired. Also added set_chain_information_same_state_root_preserves_runtime_download for the same-state-root preservation path.

lexnv · 2026-04-22T11:10:16Z

+            // proceed. The runtime_download reset below is still necessary (the block
+            // hash changed so any cached proof is for the wrong state root).
+            if !matches!(self.warped_block_ty, WarpedBlockTy::Normal) {
+                self.warped_block_ty = WarpedBlockTy::AlreadyVerified;


Is this correct? The fix only suppresses a fragment-verification cycle before the next runtime download is issued, which probably means the runtime download is happening either way soon after becase of the below:

self.runtime_download = RuntimeDownload::NotStarted { hint_doesnt_match: false, };

Correct — the original fix only preserved warped_block_ty but still unconditionally reset runtime_download to NotStarted. Updated: runtime_download is now only reset when warped_header_state_root != old_state_root. When the state root is the same (common for adjacent blocks with no storage changes), the in-progress or not-yet-started runtime download is fully preserved.

lexnv · 2026-04-22T11:13:53Z

+            // another round of fragment verification before the runtime download can
+            // proceed. The runtime_download reset below is still necessary (the block
+            // hash changed so any cached proof is for the wrong state root).
+            if !matches!(self.warped_block_ty, WarpedBlockTy::Normal) {


It looks like this is just patching a symptom. IIUC the problem is that set_chain_information wipes the runtime download / runtime calls and body download even when the incoming ValidChainInformation describes the same warp sync block we wroked with.

Then a fix would be to short-circuit the logic when warped_header_hash is equal with the new header hash. Then, this would preserve all the work we did so far. How was the 1.5/2MiB saving computed?

It looks like the hallucinations are vibe-coded because the storage proof is requested every time the function is called?

Agreed — the original fix was a band-aid. Updated in 590ef4f:

Short-circuit on same header hash — set_chain_information now returns early when the incoming hash matches warped_header_hash. No state is touched.

Preserve runtime download when state root unchanged — runtime_download is only reset to NotStarted when warped_header_state_root actually changed. When the state root is identical, cached proofs remain valid.

Preserve warped_block_ty=Normal — kept from the original fix, so a GrandPa commit advancing finality by 1 block doesn't force another fragment verification round.

The 2 MiB saving comes from avoiding the redundant :code storage proof download when the runtime download was already in progress and the state root didn't change. The benchmark numbers from smolbench are real — measured via runtime-download-count.mjs which counts the actual number of StorageGetMerkleProof requests and their payload sizes.

Root cause: set_chain_information() unconditionally reset warped_block_ty to AlreadyVerified, even when already Normal (runtime download phase). A GrandPa commit advancing finality by 1 block forced another fragment verification before the runtime download could proceed, causing a redundant re-download of :code (~1.5-2.5 MiB). Fix: preserve warped_block_ty = Normal in set_chain_information when warp sync already completed fragment verification. Includes regression test and hint key selection tests — first tests for warp_sync.rs. Regression test fails without fix, passes with fix.

…ownload Address review feedback from lexnv: 1. Short-circuit set_chain_information when the incoming header hash matches the current warped_header_hash — nothing to update. 2. Only reset runtime_download when the state root actually changed. When the state root is identical, cached proofs remain valid. 3. Preserve warped_block_ty=Normal across set_chain_information so a GrandPa commit advancing finality by 1 block doesn't force another fragment verification round before the runtime download can proceed. 4. Remove duplicated comment block. 5. Rewrite tests to demonstrate the actual production scenario: fragment verified → runtime download desired → GrandPa commit for N+1 → runtime download still desired without re-verification. Add separate test for same-state-root preservation path.

lrubasze · 2026-04-23T15:57:57Z

@replghost I cannot reproduce the issue.
Tried smoldot-v3.1.1

Can you check on your side?

replghost · 2026-04-27T00:25:01Z

@lexnv friendly ping — I've addressed your review comments, would you mind taking another look when you get a chance?

replghost · 2026-04-28T17:35:24Z

Closing - the warp_sync_minimum_gap: 32 added in 3.1.0 eliminates this race in practice. GrandPa commits advancing by 1-2 blocks no longer trigger warp sync attempts, so the redundant runtime download doesn't happen. @lrubasze confirmed it's not reproducible on 3.1.1.

The original benchmarks were valid against 3.0.0 but the gap guard is the correct fix at the right layer.

replghost force-pushed the fix/warp-sync-redundant-runtime-download branch 4 times, most recently from c10b5cb to 7c57022 Compare April 21, 2026 02:04

replghost changed the title ~~perf(sync): skip redundant runtime download during warp sync~~ fix(sync): prevent redundant runtime download during warp sync Apr 21, 2026

replghost force-pushed the fix/warp-sync-redundant-runtime-download branch from 7c57022 to aedb628 Compare April 21, 2026 02:34

replghost force-pushed the fix/warp-sync-redundant-runtime-download branch from aedb628 to ff2c395 Compare April 21, 2026 03:17

replghost force-pushed the fix/warp-sync-redundant-runtime-download branch 2 times, most recently from 0aec268 to 074aec6 Compare April 21, 2026 19:39

lexnv requested a review from Copilot April 22, 2026 09:32

Copilot started reviewing on behalf of lexnv April 22, 2026 09:32 View session

Copilot AI reviewed Apr 22, 2026

View reviewed changes

lexnv reviewed Apr 22, 2026

View reviewed changes

replghost force-pushed the fix/warp-sync-redundant-runtime-download branch from 074aec6 to d078d4e Compare April 22, 2026 18:13

replghost added 2 commits April 22, 2026 14:27

replghost force-pushed the fix/warp-sync-redundant-runtime-download branch from 590ef4f to 5b6953e Compare April 22, 2026 18:27

replghost mentioned this pull request Apr 23, 2026

refactor(sync-service): convert start_parachain to reactive state machine #3227

Closed

replghost closed this Apr 28, 2026

Conversation

replghost commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Fix

Test plan

Uh oh!

replghost commented Apr 21, 2026

How often does this happen?

Reproduction

Regression test

Uh oh!

replghost commented Apr 21, 2026

Empirical data: redundant runtime downloads on live networks

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

lexnv Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

replghost Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

lexnv Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

replghost Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

lexnv Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

replghost Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

lexnv Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

replghost Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

lrubasze commented Apr 23, 2026

Uh oh!

replghost commented Apr 27, 2026

Uh oh!

replghost commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

replghost commented Apr 16, 2026 •

edited

Loading

lexnv Apr 22, 2026 •

edited

Loading