[niteshbalusu11]

Fixes #1080. Follows the plan from the issue thread, with a few changes from review, listed at the end.

Summary

reconsiderblock used to re-apply each restored invalidateblock path on its own. It now picks the most-work branch the way invalidateblock does, and keeps losing restored leaves as forks.

 reconsider_block_inner
   clear marks, take related invalidated paths
-  for path in paths
-    accept_branch_inner(path)        # alone; a dependent path fails "parent not on chain"
-  accept_branch_inner(assemble(hash))
+  apply_after_reconsider
+    loop
+      best = most_work_branch(side_branches(leaves, restored, skip = tried))
+      accept_branch_prefix(best)     # on a block failure: remember it, try the next tip
+    park_restored_leaves             # a losing leaf becomes a fork tip if it was connected before

• Candidates: restored path ends, the reconsidered hash, fork tips, held bodies, and precious.
• Kept as a fork tip: a losing restored leaf whose every block came off the active chain, or is at or under a fork tip. Each recorded path now carries a connected flag.
• Not kept: a leaf under a fork tip or another kept leaf. A held child that never connected does not hide a leaf.
• During the reconsider, restored blocks are walked past the 10,000-block cap.
• A store fault while ranking skips that branch. Reconsider still parks the leaves, then returns the fault. invalidateblock skips the branch, as before.
• try_apply_after_invalidate uses the same branch builder and ranking. Tip-follow best_held_branch is unchanged.

Evidence

The same RPC steps, run on bitcoind 31.1, master, and this branch:

Steps	Core	master	this PR
Repro 1 from the issue	a10	b9	a10
Repro 2 from the issue	a5	a4	a5
Two restored paths vs side b2	a3	b2	a3
Losing a3..a4, then invalidate the winner	a4	a2	a4
Fork tip f2 under restored f3..f4, then invalidate the winner	f4	f2	f4
Same, but f3 is invalidated again first	f2	f2	f2
...then reconsider f3	f4	f3	f4
Held invalid child over restored h3..h4	h4	h2	h4
Held child with a bad header over a2	a2	a2	a2
Sibling sharing a restored prefix	e8	e8	e8

getchaintips also matches Core for the restored leaves: valid-fork for the kept leaf, and blocks under it are not listed.

Tests:

• chain_ops_reconsider_most_work (beats added to the mature-chain RPC journey) covers the rows above. Before: fails at the first reconsider. After: passes.
• reconsider_skips_a_branch_that_fails covers:
  • an invalid held branch that ranks first;
  • a held child whose header fails;
  • a failed block's archived body, which must not become a valid-fork;
  • a store fault while ranking, after which the restored tip is still a fork tip.
• One assertion added to chain_edges_pin_work_headers_and_reopen: reconsider does not turn a held block into a valid-fork.
• Hand mutations, each failing a test:
  • no retry;
  • connected flag ignored;
  • store fault swallowed;
  • no fork-tip rule;
  • fork tips under a kept leaf removed;
  • no under-check;
  • held branches hiding leaves;
  • no parking.

Merge Danger

Door: two-way

No schema or on-disk change. Revert restores the old per-path reconsider.

Blast Radius: operator RPCs

invalidateblock / reconsiderblock only. Tip-follow held-branch selection is unchanged.

Known limits, unchanged or not covered:

• Equal work stays on the current tip. That is the COMPAT.md "Equal-work fork choice" difference.
• fork_tips is still capped at 64 (FIFO). A kept leaf takes a slot like any reorg loser, so a full cache can evict an older fallback.
• Interior restored blocks are not kept on their own. Invalidating a leaf that was kept can then fall back lower than Core. Master does the same.
• In one case getchaintips lists an older fork tip under a kept leaf too. Core lists only the leaf; master lists only the older tip.
• Each candidate branch is rebuilt from archive per try. That is quadratic in the number of nested invalidateblock calls.
• A parked leaf deeper than 10,000 blocks is past the walk that a later invalidateblock does, like any other fork tip.
• No test for a restore longer than 10,000 blocks; it would be too slow for the suite.

Changes from the plan in the thread, from review:

• A leaf under a fork tip counts as connected before.
• Fork tips are never removed, so no fallback is lost.
• Only connected-before tips hide a leaf.
• Each tip is tried once, rather than stopping at the first failure.

🤖 Generated with Claude Code

[rearden-grok]

The happy path matches the cases in #1080. I do not think this is ready to merge: a failed reconsiderblock throws away the path records the new fallback depends on, and the 10,000-block exemption does not survive into the later invalidateblock that is supposed to use the parked leaf.

What holds up:

• Related paths are joined by the parent link, then one branch is ranked. That is the fix for repro 1 ([a8,a9] plus [a10] against b9).
• connected is set only for a path taken off the active chain. Parking stops at an existing fork tip, so a held child that never connected does not become a valid-fork and does not hide the leaf under it. Equal-work precious ordering matches held_branch_beats / accept_branch_weaker.
• A connect failure with a kept heavier prefix (Accepted + Some) tries the next untried tip. A full accept or IgnoredWeaker stops, which is right once candidates are ordered by total chain work.
• qc, test, windows, macos, and coverage are green on db2c43b.

Two change requests are on the lines below.

Not blocking, and not a reason to expand this PR:

• branch parent not on chain breaks the retry loop instead of moving on to the next tip. try_apply_held does the same, and assemble_side_branch only returns a branch whose parent is_connected, so this is hard to hit. If it does fire, every lighter candidate is skipped and the RPC still returns success.
• A side invalidateblock still marks only that hash. A descendant parked by this PR stays in fork_tips, so getchaintips can keep saying valid-fork while the ancestor is invalid. The branch is filtered out of activation, so the tip is right and the label is not. The same gap exists for any fork tip; parking makes more of them.
• reconsider_skips_a_branch_that_fails holds the tip body before corrupting the store, and only checks is_local_fault. That hold is what production invalidateblock removes, so the test does not show the lost sibling. assert_ne!(bad5_status, Some("valid-fork")) also passes for any other status.
• The limits you already list (equal work stays on the current tip, fork-tip cap 64, interior blocks not kept, one extra ancestor tip, quadratic rebuild) match the code.

[niteshbalusu11]

@rearden-grok please re-review the latest commit (59b8b04).

[rearden-grok]

Approved. 59b8b040 fixes both blocking notes from the last review, and I do not see a new one.

Store faults while ranking no longer discard a winner that was already scored, and they no longer return before the restored leaves are parked. invalidateblock still skips a branch whose work cannot be summed, as it did before this PR. A branch parent not on chain failure tries the next untried tip instead of stopping the loop. The 10,000-block exemption is now described as reconsider-only: the changelog and the known limits both say a later invalidateblock keeps the usual cap.

Re-checked the activation rules against the #1080 cases. Related paths are still one ranked branch. A losing leaf is parked only when the blocks above an existing fork tip were on the active chain, so a held child that never connected is not a valid-fork and does not hide the leaf. A kept heavier prefix still tries the next tip; a full accept or IgnoredWeaker still stops. qc, test, windows, macos, and coverage are green on this commit.

What I am accepting, not asking to change:

• A store fault on a losing candidate still fails the RPC after the winner is activated and the leaves are parked. That is the fault being returned, not the winner being dropped.
• A local fault from accept_branch_prefix still stops the retry. The leaves are parked first, so a sick store does not forget them.
• A parked leaf deeper than 10,000 blocks is invisible to the next invalidateblock, same as any other fork tip. That is now what the changelog says.
• A side invalidateblock still marks only that hash, so getchaintips can keep a parked descendant as valid-fork while an ancestor is invalid. Activation filters that branch.
• The store-fault test still holds the body before truncating header.body. The new fork_tips assertion does prove the leaf is parked after the error.

[niteshbalusu11]

@rearden-grok please re-review the latest commit (b33787a). It changes only the test: the h/i beat used to submit h5 with submitblock while its parent h4 was invalidated, and relied on rbitcoin holding it. Core refuses that block as bad-prevblk, and an upcoming PR makes submitblock match. The beat now accepts h5 through the hub, the way a peer's block arrives, so the held child still sits over the restored h4. Removing the held-child rule in park_restored_leaves still fails this beat.

[niteshbalusu11]

@rearden-grok correction: the test-only commit is now 8c1d3c5. I only reworded the message: there is no separate submitblock PR. The beat still stops relying on submitblock holding a child of an invalidated block, which Core refuses as bad-prevblk. Please review this commit.

[rearden-grok]

Approved. b33787a2 is test-only, and it still plants the held child the journey is about.

submitblock of h5 while h4 is invalidated is the case a follow-up will reject as bad-prevblk. accept_received_block is the peer path: h4 is off the active chain, so the body is held and UnknownParent does not run connect. While h2 and h3 are still invalid, best_held_branch skips that walk, so the coinbase failure is not marked yet. Reconsider then ranks h5 above h4, fails it, and still parks h4. The later invalidateblock of h5 is what keeps this from passing if the body never landed. qc, test, windows, and macos are green on this commit; coverage was still running.

No production path changed. The limits from the previous approval are unchanged.