[portlandhodl]

Stacked on #1104. This branch is #1104's commit (1094e987) plus two commits (3586aae3, a3d8bd10). Review those two; once #1104 merges, this diff shrinks to them. PRs from a fork can't target the fork's branch, so the base here is master.

Problem

batched_pin_engine_fault_retries_one_block_at_a_time fails intermittently in the coverage job (seen on #1104):

confirm load stamp reject 706b10…b59 @ 104: unexpected previous header iflight=0L/0 drain_fk=103 fence_h=Some(102)
stall at Some(103)

Load stamps tip+2 while tip+1 is still being written (drain_fk at tip+1, fence at tip), and gets a BadPrev. That reject can reach IBD before tip+1's Accepted. prev_is_hub_tip then reads the old tip, so IBD takes tip+2 for a competing fork and logs slot evicted, not re-get same hash. The block leaves the path and is never asked for again:

stall: rejects=[(706b10…b59, SoftWire, 1)] asked=[] scan_lo=104 path_lo=104

That is a production stall, not only a test problem. isolated_mutated_head_punishes_its_sender_when_lookup_runs_ahead failed in coverage on #1104 with the same SoftWire, 1 … asked=[] signature. I could not reproduce that one locally, so I'm not claiming this fixes it.

Fix

bad_prev_parent (replaces prev_is_hub_tip) classifies the parent as BadPrevParent::Tip, Committing (this path's own block at height - 1, not the tip yet) or OffPath. Tip is a tip child as before: it is fetched again under the existing three-strike halt. Committing is fetched again without a strike, because the tip has not moved during the race and the strike key (hash, tip) would otherwise halt IBD on the third repeat. OffPath is still a fork.

Tests

• New bad_prev_racing_its_parents_commit_is_fetched_again: tip+1 on the path but not confirmed, then the same BadPrev at tip+2 four times with a frozen tip. The hash must stay on the path, missing, with st.halt unset. On master it fails (left: None, the hash is dropped from height_to_hash). On 3586aae3 it fails with "a committing parent is not a strike".
• batched_pin_engine_fault_retries_one_block_at_a_time discarded every confirm event after the mend, so a dropped tip+2 could never be fetched (docs/concurrency.md: after a load reject, bodies may be "fetched again from peers"). It now runs WireRig::pump, which applies the events and serves the fetch.
• Stress, 120 parallel copies of the test binary per round: before, 2 failures in 720 runs; after, 0 in 600. isolated_mutated_head… passed 600 of 600 after the change.
• Local: cargo fmt --check, cargo clippy --workspace --all-targets -D warnings, cargo test --workspace, ./scripts/ci-os-smoke.sh all pass.

🤖 Generated with Claude Code

[rearden-grok]

Review

Reviewed the top commit 668e9b76 only. Its parent is #1104 (41930dab); the GitHub file list also contains that SOCKS commit because this PR targets master.

prev_continues_own_path matches the race it describes. A BadPrev whose parent is the hub tip, or the hash already in height_to_hash at height - 1, stays on the path and is fetched again. A parent that is only some older path block still takes the evict arm (slot evicted, not re-get same hash). == binds tighter than ||, so the second arm is only the immediate predecessor. maybe_rewind_to_best_work still returns before that decision when a heavier fork wins. prev_is_hub_tip had one caller and it is gone.

bad_prev_racing_its_parents_commit_is_fetched_again fails on the parent commit: the old predicate drops height_to_hash[t + 2], and the new assertions require the slot, is_missing, and densify_scan_lo. WireRig::pump in the engine-fault test still stalls if the hash is never asked for.

Requesting changes because the retry uses a halt that treats this race as fatal.

Severity	Location	Finding
High	crates/rbitcoin-net/src/ibd/events/mod.rs:1140	retry_tip_child calls apply_tip_child_bad_prev. The count key is (hash, hub tip bytes) and the halt is 3 (TIP_CHILD_HALT_AFTER). This race is exactly "hub tip still the grandparent": each BadPrev of the same child increments, and the third sets st.halt. The IBD loop takes that as NetError::Consensus (ibd/mod.rs:1171). note_confirm_stuck only records confirm_stuck_since; it does not delay the re-get. A parent still in write (the drain_fk / fence gap in the PR) can outlast three getdata round-trips of the child, and the new test applies the reject once.
Low	crates/rbitcoin-net/src/ibd/events/mod.rs:1090	The comment and the re-get log still say the parent is the hub tip when the new arm matched only the path slot at height - 1.

Please count strikes only once the parent is the hub tip (the old prev_is_hub_tip case), and keep re-getting while the parent is still the uncommitted path slot. A test that applies the reject three times with a frozen tip should still show the hash missing and st.halt unset.

[rearden-grok]

Review

Reviewed only the top commit 3586aae3 (parent 1094e987, #1104). The predicate change matches the race, and the strike counter is the same one called out on 668e9b76. The previous high is unfixed. The commit message still says the child is fetched again under the three-strike halt.

prev_continues_own_path is true when the parent is the hub tip or the hash in height_to_hash at height - 1. A parent that is only an older path block still takes the evict arm. maybe_rewind_to_best_work still returns before that decision. prev_is_hub_tip is gone. bad_prev_racing_its_parents_commit_is_fetched_again still fails on the parent commit, and it still applies the reject once.

The height-1 arm sets retry_tip_child and calls apply_tip_child_bad_prev (events/mod.rs:1177). The count key is (hash, hub tip bytes) and TIP_CHILD_HALT_AFTER is 3 (:1267). This race is an unchanged hub tip, so the third BadPrev sets st.halt, and the IBD loop returns NetError::Consensus. note_confirm_stuck only records a timestamp. Strikes 1 and 2 call mark_missing immediately.

Count strikes only once the parent is the hub tip. Keep re-getting while the parent is still the uncommitted path slot. A test that applies the reject three times with a frozen tip should leave the hash missing and st.halt unset.

The re-get log at :1191 still says the parent is the hub tip when the match was only the path slot at height - 1.

[portlandhodl]

Thanks. You're right, and the earlier commit message was wrong. Addressed in a3d8bd10:

Finding	Change
High: the height-1 arm takes the tip-child strikes, and a frozen tip halts on the third	bad_prev_parent returns BadPrevParent::{Tip, Committing, OffPath}. Only Tip runs apply_tip_child_bad_prev. Committing is re-fetched (mark_missing / reopen_for_densify) without a strike and leaves cascade_at alone. OffPath keeps the evict arm.
Test	bad_prev_racing_its_parents_commit_is_fetched_again applies the reject four times with the tip frozen and asserts st.halt is unset and the hash is still on the path and missing each time. It fails on 3586aae3 with "a committing parent is not a strike".
Log at :1191	Committing logs (parent is the path block still committing, re-getdata). Tip keeps (parent is tip, re-getdata).

The existing tip-child tests (tip_child_bad_prev_halts_on_the_third_retry and the others) still pass. batched_pin_engine_fault_retries_one_block_at_a_time passed 360 of 360 runs at 120 parallel copies.

[portlandhodl]

Reworded both commit messages; the trees are unchanged (a3d8bd10 and bb9b9f2e have the same tree).

• 2bb7d039 (was 3586aae3) no longer says the race uses the three-strike halt. It now says the committing parent still shares the tip-child counter at that commit, and that the next commit takes it off.
• bb9b9f2e (was a3d8bd10) drops the line about the earlier message.

[rearden-grok]

Review

Reviewed a3d8bd10 on top of 3586aae3. The last change request is fixed.

bad_prev_parent splits the old predicate. A parent that is the hub tip is Tip and still goes through apply_tip_child_bad_prev, which halts at 3 while that tip is unchanged (tip_child_bad_prev_halts_on_the_third_retry). A parent that is only height_to_hash[height - 1] is Committing: the match arm does not touch the strike counter, then the hash is marked missing and densify is reopened. OffPath still drops the slot and takes the evict log. Tip is tested before Committing, so once the parent commit lands, further BadPrevs of the same child start the three-strike count. maybe_rewind_to_best_work still returns before either arm.

bad_prev_racing_its_parents_commit_is_fetched_again applies the reject four times with the hub tip frozen on the grandparent. It requires st.halt to stay unset, the hash to stay at t + 2, and the block to stay missing. That fails on 3586aae3, where the third reject sets the halt. An off-path parent still does not match Committing.

A Committing parent that never becomes the tip and is never removed from height_to_hash is re-fetched without that cap. That is the race window this commit is for: the classification changes when the parent is accepted or the slot is dropped.