node: switch the active chain onto a heavier branch (reorg execution, part 2) - #573
Conversation
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe PR propagates heavier-branch metadata, adds branch switching and reorganization barriers, and coordinates storage and UTXO tasks. The orchestrator pauses participants, switches chains, updates synchronization state, and retries downloads when required. ChangesChain Reorganization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant HeaderValidation
participant Orchestrator
participant BlockChain
participant UTXOBuilder
HeaderValidation->>Orchestrator: Send heavier-branch metadata
Orchestrator->>BlockChain: Request reorganization pause
UTXOBuilder->>BlockChain: Enter reorganization barrier
Orchestrator->>BlockChain: Switch active branch
BlockChain-->>Orchestrator: Return validated tip
Orchestrator->>BlockChain: Clear reorganization pause
Orchestrator->>UTXOBuilder: Resume and reload chain state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cd24fe4 to
406daac
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
src/blockchain/include/kth/blockchain/interface/block_chain.hpp (1)
262-277: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake the handshake contract explicit at the flag declarations.
The comment states the protocol, but it does not state the required order on the build side: the build task must publish
set_utxo_build_idle(false)before it readsreorg_pause_requested(). With the reverse order there is a window where the switching side observesutxo_build_idle() == truewhile the build task starts a batch. The current build-side implementation uses the reverse order (seesrc/node/src/sync/block_tasks.cppLines 1787-1794).Add that requirement to this comment, and consider a defensive check inside
switch_to_branchthat returns false whenutxo_build_idle()is false.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/blockchain/include/kth/blockchain/interface/block_chain.hpp` around lines 262 - 277, Update the “Reorg quiescing” contract comment above request_reorg_pause/reorg_pause_requested/set_utxo_build_idle/utxo_build_idle to explicitly require the build task to publish set_utxo_build_idle(false) before reading reorg_pause_requested(). Also add a defensive check in switch_to_branch that returns false when utxo_build_idle() is false.src/node/src/sync/orchestrator.cpp (1)
886-888: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftMove the synchronous switch off the coordinator executor.
switch_to_branchreads every abandoned block from disk, parses it, reads its undo record, and applies a UTXO delta per block. It is a plain synchronous call here, so it blocks the coordinator coroutine and every other coroutine sharing this executor for the whole duration.block_chainalready uses theposttopriority_pool_plusco_awaitpattern forstore_chunkandvalidate_chunk.Wrap the call the same way, or expose an awaitable
switch_to_branchthat posts topriority_pool_.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/node/src/sync/orchestrator.cpp` around lines 886 - 888, Update the reorganization path around switched and chain.switch_to_branch so the synchronous branch switch runs on priority_pool_ via the existing post-and-co_await pattern, rather than on the coordinator executor; preserve the current network.stopped() false result and returned switch outcome.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/blockchain/src/interface/block_chain.cpp`:
- Around line 864-874: Update the reorganization loop around disconnect_block so
a failure reports the height reached after successfully disconnecting prior
blocks instead of returning only false. Adjust the enclosing function’s result
type and its callers to propagate that reached height or an equivalent result,
allowing the sync coordinator to resynchronize counters from the database
markers while preserving the existing immediate-abort behavior.
- Around line 845-851: Update switch_to_branch to validate branch_head before
any disconnect_block or active_set_tip operation: reject
database::header_index::null_index and reject heads whose
header_index_.get_height(branch_head) is less than or equal to fork_height,
returning false without truncating the active chain. Preserve the existing
valid-branch switching flow.
In `@src/node/src/sync/block_tasks.cpp`:
- Around line 1787-1794: In the block-build loop around reorg_pause_requested(),
call chain.set_utxo_build_idle(false) before reading the pause flag so a claimed
batch cannot appear idle. When the pause branch is taken, set
chain.set_utxo_build_idle(true) before waiting and continuing, preserving the
idle publication while paused.
- Around line 1796-1802: Update the built-height change handling in the sync
loop to rebuild timestamp_window whenever utxo_built_height changes, using the
same preload logic currently used before the loop. Extract that preload logic
into a local lambda, invoke it during initial setup and after adopting the
externally reported height, and ensure stale entries from the abandoned branch
are removed before calculate_mtp is used.
In `@src/node/src/sync/orchestrator.cpp`:
- Around line 893-903: During the reorg handling in the switched branch, reset
the shared contiguous_height atomic to fork_height + 1 alongside
blocks_synced_to. Update block_storage_task so its local contiguous_height and
max_stored_height can be re-derived from the active chain after a switch,
preventing stale pre-reorg values from being republished.
- Around line 878-890: Before calling switch_to_branch in the reorganization
flow, stop or drain block_download_supervisor, fast_validation_task, and
block_storage_task so no fast-pipeline writes or validation updates remain in
flight; retain the existing UTXO pause and resume behavior around the switch.
---
Nitpick comments:
In `@src/blockchain/include/kth/blockchain/interface/block_chain.hpp`:
- Around line 262-277: Update the “Reorg quiescing” contract comment above
request_reorg_pause/reorg_pause_requested/set_utxo_build_idle/utxo_build_idle to
explicitly require the build task to publish set_utxo_build_idle(false) before
reading reorg_pause_requested(). Also add a defensive check in switch_to_branch
that returns false when utxo_build_idle() is false.
In `@src/node/src/sync/orchestrator.cpp`:
- Around line 886-888: Update the reorganization path around switched and
chain.switch_to_branch so the synchronous branch switch runs on priority_pool_
via the existing post-and-co_await pattern, rather than on the coordinator
executor; preserve the current network.stopped() false result and returned
switch outcome.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d702ff70-838b-41d5-85fd-ad29d2aa9e89
📒 Files selected for processing (8)
src/blockchain/include/kth/blockchain/interface/block_chain.hppsrc/blockchain/include/kth/blockchain/pools/header_organizer.hppsrc/blockchain/src/interface/block_chain.cppsrc/blockchain/src/pools/header_organizer.cppsrc/node/include/kth/node/sync/messages.hppsrc/node/src/sync/block_tasks.cppsrc/node/src/sync/header_tasks.cppsrc/node/src/sync/orchestrator.cpp
|
Moving to draft. CodeRabbit's findings are correct and collectively say the quiescing design is unsound, not just buggy:
Reworking rather than patching: the barrier has to cover the whole block pipeline, and every shared counter has to be rewound as part of the switch. |
406daac to
18160e2
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/blockchain/include/kth/blockchain/interface/block_chain.hpp`:
- Around line 293-303: Replace the hardcoded reorg_participants accounting in
block_chain with explicit participant registration and retirement APIs, ensuring
the expected count reflects tasks that implement enter_reorg_barrier and
leave_reorg_barrier; register block_storage_task as the second participant. In
src/blockchain/include/kth/blockchain/interface/block_chain.hpp lines 293-303,
update the barrier state and methods accordingly. In
src/node/src/sync/block_tasks.cpp lines 2007-2010, deregister the UTXO build
task during shutdown instead of calling enter_reorg_barrier without a matching
leave.
- Around line 305-307: Update build_utxo_set() so each UTXO batch application
validates its captured generation ticket against chain_generation() after the
existing reorg barrier/pause checks and before applying deltas, discarding stale
in-flight work. Preserve the current application path for matching generations;
alternatively, revise chain_generation()’s documentation and contract to remove
the stale-generation behavior if that check is intentionally unsupported.
In `@src/blockchain/src/interface/block_chain.cpp`:
- Around line 867-871: Update the reorg handling branch for fork_height >
heights->block in the surrounding blockchain reorganization function to return
the actual validated tip, heights->block, instead of fork_height. Keep the
existing no-disconnect behavior and logging unchanged so coordinator
synchronization never advances beyond stored validated blocks.
- Around line 848-852: Update the switch_to_branch failure result and its
consumers so an unreadable validated tip is represented as unknown rather than
height 0. Change switch_result::validated_tip to an optional or equivalent
status-bearing value, have the failure path leave it unset, and update the
coordinator assignments around blocks_synced_to and contiguous_height to
preserve existing counters when no validated tip is available.
In `@src/node/src/sync/block_tasks.cpp`:
- Around line 2007-2010: Update the shutdown path around
chain.enter_reorg_barrier() so the exiting task is marked as retired without
permanently inflating the reorganization barrier participant count. Use the
existing participant-accounting mechanism shared with block_chain.hpp,
preserving the shutdown guarantee that a racing switch cannot wait on an exited
task.
- Around line 1757-1771: Adjust reload_timestamp_window so it preloads exactly
11 timestamps, matching the incremental window maintained by the later update
path and calculate_mtp’s expected input. Change the preload range around
reload_timestamp_window to exclude the extra oldest height while preserving the
existing rebuild behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a1a97ee-b8b3-4a66-9c87-130bf4e3db08
📒 Files selected for processing (8)
src/blockchain/include/kth/blockchain/interface/block_chain.hppsrc/blockchain/include/kth/blockchain/pools/header_organizer.hppsrc/blockchain/src/interface/block_chain.cppsrc/blockchain/src/pools/header_organizer.cppsrc/node/include/kth/node/sync/messages.hppsrc/node/src/sync/block_tasks.cppsrc/node/src/sync/header_tasks.cppsrc/node/src/sync/orchestrator.cpp
🚧 Files skipped from review as they are similar to previous changes (5)
- src/blockchain/src/pools/header_organizer.cpp
- src/blockchain/include/kth/blockchain/pools/header_organizer.hpp
- src/node/src/sync/header_tasks.cpp
- src/node/src/sync/orchestrator.cpp
- src/node/include/kth/node/sync/messages.hpp
18160e2 to
ae14873
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/node/src/sync/block_tasks.cpp (1)
1487-1521: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winThe storage task cannot park while its input channel is idle, so the barrier can never be reached.
The loop suspends on
input.async_receiveat Line 1488 and evaluatesreorg_pause_requested()only after a chunk arrives. During a switch the coordinator stops feeding the pipeline, so no chunk arrives.block_storage_taskstays registered and unparked,reorg_barrier_reached()stays false, and the switching side waits for a task that will never park.Check the pause flag before the receive, and use a bounded wait so an idle channel still reaches the park point.
🔒 Proposed restructure
while (true) { + // Park before waiting for input: a paused pipeline delivers no chunk, so a + // check placed after the receive is never reached. + if (chain.reorg_pause_requested()) { + chain.enter_reorg_barrier(); + while (chain.reorg_pause_requested() && ! chain.stopped()) { + ::asio::steady_timer pause_timer(co_await ::asio::this_coro::executor); + pause_timer.expires_after(std::chrono::milliseconds(50)); + co_await pause_timer.async_wait(::asio::as_tuple(::asio::use_awaitable)); + } + chain.leave_reorg_barrier(); + if (contiguous_out) { + contiguous_height = contiguous_out->load(std::memory_order_acquire); + max_stored_height = contiguous_height > 0 ? contiguous_height - 1 : 0; + } + continue; + } + auto [ec, msg] = co_await input.async_receive( ::asio::as_tuple(::asio::use_awaitable));The pause check placed after the receive (Lines 1504-1521) then becomes redundant and can be removed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/node/src/sync/block_tasks.cpp` around lines 1487 - 1521, Move the reorg pause handling in the block storage task loop before input.async_receive so an idle channel can enter the barrier; while reorg_pause_requested() remains true, use the existing bounded timer wait and then resume with the re-derived contiguous height. Remove the now-redundant pause block after receiving a chunk, preserving normal stop and message processing.
🧹 Nitpick comments (1)
src/blockchain/src/interface/block_chain.cpp (1)
854-865: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winMake
fork_heightpart of the switch precondition.
switch_to_branchusesfork_heightto disconnect the present tip, but only checks thatbranch_headis above it. Iffork_heightis too low,disconnect_block()rewinds fewer blocks than the stored chain needs, UTXO effects from the abandoned blocks can remain active, andactive_set_tip()moves the active tip onto a branch whosefork_heightdoes not match the switch. The reorg path derivesbranch_headfromreorg_branch_head, so enforce the fork-point match locally before disconnecting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/blockchain/src/interface/block_chain.cpp` around lines 854 - 865, Update switch_to_branch to require fork_height matches the branch’s actual fork point before any disconnection occurs. Validate the fork-point relationship using the branch_head/reorg_branch_head data available in this method, reject mismatches alongside the existing null and height checks, and preserve the current early-return behavior so disconnect_block and active_set_tip are only reached for a consistent fork.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/blockchain/src/interface/block_chain.cpp`:
- Around line 884-896: Update the reorganization loop around disconnect_block to
handle database::disconnect_result::unclean separately from
database::disconnect_result::failed: preserve the current {false, h} return for
failed, but for unclean report the marker/UTXO divergence and stop the pipeline
without returning h as a resumable validated tip. Keep the existing
successful-disconnect iteration unchanged.
In `@src/node/src/sync/block_tasks.cpp`:
- Around line 1786-1787: Pair reorg participant registration with an RAII scope
guard in both tasks: at src/node/src/sync/block_tasks.cpp lines 1786-1787 and
1460-1462, have the guard unregister on destruction; remove the explicit
unregister calls at lines 2041-2043 and 1606-1607. Ensure the guard covers all
co_return and exception paths in utxo_build_task and the other task.
- Around line 1506-1511: Update the pause loop in the coroutine containing
utxo_build_task to also stop when chain.stopped() is true, while retaining the
existing reorg_pause_requested() check. Ensure chain.leave_reorg_barrier()
executes after the loop even when shutdown ends the wait.
---
Outside diff comments:
In `@src/node/src/sync/block_tasks.cpp`:
- Around line 1487-1521: Move the reorg pause handling in the block storage task
loop before input.async_receive so an idle channel can enter the barrier; while
reorg_pause_requested() remains true, use the existing bounded timer wait and
then resume with the re-derived contiguous height. Remove the now-redundant
pause block after receiving a chunk, preserving normal stop and message
processing.
---
Nitpick comments:
In `@src/blockchain/src/interface/block_chain.cpp`:
- Around line 854-865: Update switch_to_branch to require fork_height matches
the branch’s actual fork point before any disconnection occurs. Validate the
fork-point relationship using the branch_head/reorg_branch_head data available
in this method, reject mismatches alongside the existing null and height checks,
and preserve the current early-return behavior so disconnect_block and
active_set_tip are only reached for a consistent fork.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 16b9a5dc-568b-4e2e-aa9c-d03f67f2c55c
📒 Files selected for processing (8)
src/blockchain/include/kth/blockchain/interface/block_chain.hppsrc/blockchain/include/kth/blockchain/pools/header_organizer.hppsrc/blockchain/src/interface/block_chain.cppsrc/blockchain/src/pools/header_organizer.cppsrc/node/include/kth/node/sync/messages.hppsrc/node/src/sync/block_tasks.cppsrc/node/src/sync/header_tasks.cppsrc/node/src/sync/orchestrator.cpp
🚧 Files skipped from review as they are similar to previous changes (5)
- src/node/src/sync/header_tasks.cpp
- src/blockchain/src/pools/header_organizer.cpp
- src/node/include/kth/node/sync/messages.hpp
- src/node/src/sync/orchestrator.cpp
- src/blockchain/include/kth/blockchain/pools/header_organizer.hpp
ae14873 to
9a5a6b7
Compare
|
On the idle-channel finding (storage task could not park while its input channel was quiet): fixed by moving the pause check before Worth stating plainly: that is the third distinct way this hand-rolled barrier failed to close (Dekker race → hardcoded count with one participant instrumented → unreachable park point). The individual fixes are real, but the pattern says the approach is fighting the architecture, since the participating tasks block on different primitives (channel receive vs timer poll). If another barrier hole turns up, the answer is to stop the sync pipeline with the existing teardown machinery and restart it after the switch — reusing proven code instead of new concurrency primitives — not a fifth patch. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/node/src/sync/block_tasks.cpp`:
- Around line 1506-1516: Make block_storage_task’s pause handling interruptible
through a control path that can wake both input.async_receive() and the pause
wait, rather than relying on the non-atomic reorg_pause_requested() check before
receiving data. Ensure the control path also releases a task already parked when
shutdown begins, so the coordinator cannot wait for an unreachable barrier
participant; update the surrounding pipeline-switch logic as needed while
preserving normal data processing.
- Around line 1518-1526: Update the reorg handling around block-storage
processing and store_chunk() so queued downloaded_chunk messages from the
abandoned fork are discarded or generation-checked before storage. Associate
chunks with the active-chain generation or validate their headers against the
current branch, and only store chunks belonging to the new active chain while
preserving the existing cursor reset after the reorg.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a9e67ab4-d3c2-46b9-8047-4d6db7ffc21b
📒 Files selected for processing (8)
src/blockchain/include/kth/blockchain/interface/block_chain.hppsrc/blockchain/include/kth/blockchain/pools/header_organizer.hppsrc/blockchain/src/interface/block_chain.cppsrc/blockchain/src/pools/header_organizer.cppsrc/node/include/kth/node/sync/messages.hppsrc/node/src/sync/block_tasks.cppsrc/node/src/sync/header_tasks.cppsrc/node/src/sync/orchestrator.cpp
🚧 Files skipped from review as they are similar to previous changes (7)
- src/node/src/sync/header_tasks.cpp
- src/node/include/kth/node/sync/messages.hpp
- src/blockchain/src/pools/header_organizer.cpp
- src/blockchain/include/kth/blockchain/pools/header_organizer.hpp
- src/blockchain/src/interface/block_chain.cpp
- src/node/src/sync/orchestrator.cpp
- src/blockchain/include/kth/blockchain/interface/block_chain.hpp
… part 2) Closes the loop: the node now reorganizes. Detection reported heavier branches and the storage layer could undo blocks; nothing joined the two. - header_organizer reports the branch head alongside the fork height, so the candidate is actionable rather than just observable. - block_chain::switch_to_branch(branch_head, fork_height) disconnects the abandoned blocks newest-first (restoring their spent outputs from undo data) and then re-points the active chain at the new branch. It aborts on the first failed disconnect rather than pressing on: the UTXO set stays consistent with whatever was disconnected, and the height markers say so. - The sync coordinator drives it: on a reorg candidate it quiesces the UTXO build, performs the switch, rewinds blocks_synced_to to the fork and headers_synced_to to the new tip, then re-drives block download for the new branch (headers-only from the fork up). Quiescing is a two-flag handshake on block_chain: the coordinator raises reorg_pause and waits for utxo_build_idle; utxo_build_task parks between batches (never mid-batch, so the UTXO set is always at a block boundary, which is what disconnect expects) and re-reads the built height afterwards, since the switch rewinds it. Not yet wired: the reorg notification to the mempool. block_organizer's broadcaster has had no publisher since the LMDB storage was removed, so full_node::handle_reorganized and mempool::update_for_reorg still never fire — tracked by #498.
9a5a6b7 to
6f62f2d
Compare
1 participant
The node now reorganizes. Detection (#570) reported heavier branches and the storage layer (#571/#572) could undo blocks and address them by height — nothing joined the two. This does.
The switch
header_organizernow reports the branch head alongside the fork height, so a candidate is actionable rather than just observable.block_chain::switch_to_branch(branch_head, fork_height)disconnects the abandoned blocks newest-first (restoring their spent outputs from undo data), then re-points the active chain at the new branch. It aborts on the first failed disconnect instead of pressing on: the UTXO set stays consistent with whatever was disconnected, and the height markers say so.blocks_synced_toto the fork andheaders_synced_toto the new tip → re-drive block download (the new branch is headers-only from the fork up).Quiescing
A switch rewrites the UTXO set, which
utxo_build_taskalso writes. Two flags onblock_chainform the handshake: the coordinator raisesreorg_pauseand waits forutxo_build_idle; the build task parks between batches — never mid-batch, so the UTXO set is always at a block boundary, which is exactly what disconnect expects — and re-reads the built height afterwards, since the switch rewinds it.Not in this PR
The reorg notification to the mempool.
block_organizer's broadcaster has had no publisher since the LMDB storage was removed, sofull_node::handle_reorganizedandmempool::update_for_reorgstill never fire. Re-admitting transactions from disconnected blocks is #498.Testing caveat — please read
Unit suites are green (blockchain 1544, node 326) and everything builds, but no real reorg has been exercised end-to-end. There is no regtest harness that produces competing chains, so the disconnect→switch→re-download path has never actually run. I'd treat that harness as the gating item before trusting this on a node at the tip, and I'd rather say so than imply more confidence than the evidence supports.
Summary by CodeRabbit
New Features
Bug Fixes