viz: tolerate a best-chain switch during tfl_block_sequence walk

[br33zybail]

viz: tolerate a best-chain switch during tfl_block_sequence walk

Fixes the node-killing panic multiple operators hit on 2026-07-20/21:

panicked at zebra-crosslink/src/lib.rs:1939:21:
assertion `left == right` failed: first hash is not the one requested

(line 1939 on s2v9 builds, 1959 on current; stack goes through viz2::service_viz_requests; the process aborts and the node goes down.)

Root cause

tfl_block_sequence resolves a start hash, then walks FindBlockHashes from it. FindBlockHashes follows the CURRENT best chain. If the chain switches between those two steps, the first returned hash is not the requested one; the assert treated that ordinary race as an internal error.

The window gets hit in practice by same-height sibling switches. One reporting operator's log shows the full sequence: BFT block committed at h243840 (01:31:45.28), his own miner submits a stale solution at the same height two seconds later (01:31:47.71, the race #39 fixes), "switched best chain", panic. He crashed twice in ~8h while mining. A non-mining node hits the same window when a sibling arrives over the network, so #39 shrinks the trigger but cannot close this window alone; the walk must tolerate the switch.

Fix

Replace the assert with the surrounding code's own fail-soft idiom (the PARANOID != WRONG guards a few lines up): log and return what we have. The viz2 callers already guard for short or empty sequences and re-request against the new chain on the next loop tick. A reorg mid-walk now costs one retry instead of the node.

One file, no config surface, no behavior change outside the crashing path.

Verification

• cargo check -p zebra-crosslink clean on this base, no new diagnostics.
• Related data from our production miner (39h): stale solutions that trigger this switch occur at ~4/hour on a single-solver node (157 discarded by a submit-path guard, see Miner: reject stale internal solver submissions #39). Our node runs the same viz service thread and has zero of these panics with the trigger suppressed; unguarded mining nodes in the operator channel crash within hours.

[Zk-nd3r]

Traced this through the actual callers. The active path checks out:

• lib.rs launches viz2::service_viz_requests.
• On the mismatch, this patch breaks before accepting chunk_hashes, so tfl_block_sequence returns empty vectors.
• viz2 rejects empty suspect_seq_blocks and continues 'main_loop, which rereads tip and hash state. No sibling sequence is consumed.

One edge remains before calling the shared helper fully covered: viz.rs also calls tfl_block_sequence, but it does not have the same empty-sequence retry guard. That service is not the path launched here, but the helper is shared. Either add the same fail-closed guard there or narrow the PR claim to active viz2.

The missing regression is exact and small: mock BlockHeader(start_hash), then have FindBlockHashes return a same-height sibling as element 0. Assert no panic, empty result, and no state publication before the next loop rereads the tip.

I would take this for v10 with that scope made explicit. #39 removes the dominant local trigger. #50 closes the network-triggerable node-killing consequence. Neither should wait on the separate ack-height latch fix.

[br33zybail]

Both points addressed.

Regression test: added in e1a60b1, shaped exactly as you specified. Mocked BlockHeader(start_hash), FindBlockHashes returning a same-height sibling as element 0; asserts no panic, empty result, and (via read_extra_info=true) that no block data is fetched for the sibling walk, so nothing can be published before the caller rereads the tip. A second test pins the consistent-walk path so the guard cannot over-fire. cargo test -p zebra-crosslink tfl_block_sequence_reorg: 2 passed.

viz.rs scope: narrowing the claim to the active viz2 path, with one observation on why the dormant path is already safe: viz.rs treats the all-empty tuple as its existing failure value on the adjacent lookup-failure branch (break (ZebBlockHeight(0), None, Vec::new(), Vec::new())), and its downstream loops iterate height_hashes/seq_blocks directly, so an empty return from this guard renders one empty frame and rereads on the next tick, identical to its established failure behavior. No sibling data is consumed there either. Happy to add an explicit guard if that service is revived, but I did not want to grow the diff into a path that ships disabled.

On your #39 test request: accepted, setting it up now. Given our production lineage carries wallet-persistence patches that s1_dev does not have yet, we will run your exact head (58723e8) as a dedicated second instance on the same machine (separate state, throwaway wallet seed, its own cores) rather than swapping the bonded node onto it. Real mining against the live network either way; report to follow on #39 with the metrics you listed

[Zk-nd3r]

Rechecked e1a60b1 against current s1_dev (f811d78). The active viz2 path still rejects the empty result before publication, and dormant viz.rs consumes no sibling data. One current-base fix is needed before I can confirm this: 42781e3 removed TFLServiceCalls::force_feed_pow, but state_only_calls still initializes that field. Please update to current s1_dev, remove the obsolete field, and rerun cargo test -p zebra-crosslink tfl_block_sequence_reorg. The production hunk merges cleanly; this is test-harness drift only.

[br33zybail]

@Zk-nd3r Rebased onto current s1_dev.

Head is now 2a41ad4 on 26baf6b, not f811d78. s1_dev moved 13 commits while this sat, so I took the tip. Both commits rebased clean, no conflicts, and the production hunk did not move.

The obsolete field is gone. state_only_calls now builds TFLServiceCalls { state, read_state, mempool, force_feed_pos }, matching the struct after 42781e3. That was the only breakage: force_feed_pos still takes two arguments, so its existing closure needed no change.
cargo test -p zebra-crosslink tfl_block_sequence_reorg on 2a41ad4:

running 2 tests test tfl_block_sequence_reorg_tests::sibling_first_hash_returns_empty_instead_of_panicking ... ok test tfl_block_sequence_reorg_tests::consistent_walk_still_returns_the_sequence ... ok test result: ok. 2 passed; 0 failed

Since the base moved past the commit you checked, I rechecked both callers on the new tip: viz2.rs:140 still rejects the empty sequence before publication, and viz.rs still consumes no sibling data. Diff stays one file, now +139/-4, three lines smaller only because the dead field went away.

[Zk-nd3r]

Confirmed 2a41ad4e against current s1_dev 26baf6b0. The obsolete force_feed_pow initializer is gone, the production hunk is unchanged, and both callers retain the expected empty-result behavior. Your 2/2 focused run covers the requested regression. My review blocker is cleared.