Skip to content

test: exercise the reorg's fatal path, through a persister it can refuse - #580

Merged
fpelliccioni merged 1 commit into
test/reorg-11-restartfrom
test/reorg-12-fatal-path
Aug 4, 2026
Merged

test: exercise the reorg's fatal path, through a persister it can refuse#580
fpelliccioni merged 1 commit into
test/reorg-11-restartfrom
test/reorg-12-fatal-path

Conversation

@fpelliccioni

Copy link
Copy Markdown
Contributor

Stacked on #579 — it touches the same test file. Merge #579 first; this rebases onto master after.

The write that makes a switch survivable is the one that re-describes the replaced heights. If it fails, the chain in memory and the chain on disk name different branches, nothing repairs that while the node runs, and a restart would come back on the abandoned branch with the UTXO set rewound below it. #578 added that path — reorg_outcome::fatal, on_fatal, full_node::notify_fatal, the executor's stop — and nothing exercised it.

The seam

Reaching the path needs the write to fail on demand. Corrupting a database to get there would test the corruption, not the handling; filling LMDB's map would depend on page sizes and break on changes that have nothing to do with this.

So execute_reorg takes the persister as a parameter:

using header_persister = std::function<code(domain::chain::header::list const&, size_t)>;

Not a test flag. The write is what decides whether the switch can be lived with, and a caller that runs a reorg has to say where it goes — the same shape as the abort callback already there. The coordinator passes block_chain::replace_headers_from; the test passes one that refuses.

What the test pins

  • the persister was reached at all — the case is not being skipped somewhere earlier;
  • the switch itself succeeded (UTXO rewound, chain moved) and it is describing that on disk which failed;
  • the by-height table is untouched: height 13 still answers with A's block, so the transaction left nothing half-written;
  • the reorg is reported fatal, and no blocks are connected for the new branch;
  • after a restart the node is on A, whole, with the validated tip and the UTXO-built height both back at the fork — so it re-downloads rather than trust a UTXO state that no longer matches the chain it came back on;
  • the heavier branch can be announced again and becomes a candidate again.

The hinge above it

test/fatal_shutdown.cpp: notify_fatal hands the reason to the owner once and verbatim, and a node with no handler installed is still safe to report to.

What is not pinned, and why — stated in the file so a passing run is not read as more than it is: that notify_fatal stops the node cannot be shown on a node that never started, because stopped() is already true before the call; and the executor's own stop path (state to stopping, heartbeat ending, join()) needs a running node with a network, which no test stands up.

Local: kth_node_test 773 assertions / 114 cases, kth_blockchain_test 2009 / 178.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4813de4f-b189-4cc6-94fb-859c46738221

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@fpelliccioni
fpelliccioni force-pushed the test/reorg-11-restart branch from 24c9374 to 7bccb50 Compare August 4, 2026 20:05
@fpelliccioni
fpelliccioni force-pushed the test/reorg-12-fatal-path branch from 2d2d9e7 to c8aeb6e Compare August 4, 2026 20:07
The write that makes a switch survivable is the one that re-describes the
replaced heights. If it fails, the chain in memory and the chain on disk name
different branches, nothing repairs that while the node runs, and a restart
would come back on the abandoned branch with the UTXO set rewound below it.
#578 added that path — reorg_outcome::fatal, on_fatal, full_node::notify_fatal,
the executor's stop — and nothing exercised it.

Reaching it needs the write to fail on demand. Corrupting a database to get
there would test the corruption, not the handling, and filling LMDB's map would
depend on page sizes and break on changes that have nothing to do with this.

So execute_reorg takes the persister as a parameter. Not a test flag: the write
is what decides whether the switch can be lived with, and a caller that runs a
reorg has to say where it goes. The coordinator passes
block_chain::replace_headers_from; the test passes one that refuses.

The test then pins what happens on refusal:
- the persister was reached at all (the case is not skipped elsewhere);
- the switch itself succeeded — the UTXO set was rewound, the chain moved — and
  it is describing that on disk which failed;
- the by-height table is untouched: height 13 still answers with A's block, so
  nothing changed it on the way out (not that the write is atomic — the
  persister here never reaches the database, so there is no transaction to have
  been left half-applied);
- the reorg is reported as fatal, and no blocks are connected for the new branch;
- after a restart the node is on A, whole, with the validated tip and the
  UTXO-built height both back at the fork — so it re-downloads rather than trust
  a UTXO state that no longer matches the chain it came back on;
- the heavier branch can be announced again and becomes a candidate again.

Adds test/fatal_shutdown.cpp for the hinge above it: notify_fatal hands the
reason to the owner once and verbatim, and a node with no handler is still safe
to report to. What is NOT pinned is stated there too — that notify_fatal stops
the node cannot be shown on a node that never started, since stopped() is
already true before the call, and the executor's own stop path needs a running
node with a network that no test stands up.
@fpelliccioni
fpelliccioni force-pushed the test/reorg-12-fatal-path branch from c8aeb6e to 6b82106 Compare August 4, 2026 20:50
@fpelliccioni
fpelliccioni merged commit d2beaaa into test/reorg-11-restart Aug 4, 2026
1 check passed
@fpelliccioni
fpelliccioni deleted the test/reorg-12-fatal-path branch August 4, 2026 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant