Skip to content

mempool: save and reload parents before their children - #960

Merged
bkeroack merged 1 commit into
release/0.6from
fix/mempool-reload-parent-first
Oct 14, 2026
Merged

bkeroack merged 1 commit into
release/0.6from
fix/mempool-reload-parent-first

Conversation

@bkeroack

Copy link
Copy Markdown
Contributor

Fixes #949

Problem

dump_mempool wrote mempool.dat in the pool's hash-map order (get_all_entries()), and load_mempool re-admitted the records sorted by entry time only, which has one-second resolution, keeping file order on a tie. A child that entered the pool in the same second as its parent was often tried first, refused with missing-inputs, and every descendant behind it failed too. The error was discarded, so the only trace was the skipped= count. On a synced mainnet node, two clean restarts on 0.6.0 re-admitted 15,825 of 46,711 and 13,799 of 36,121 saved transactions (details in #949).

Bitcoin Core writes the file in an order where parents precede children and loads it in file order:

  • DumpMempool takes pool.infoAll() (src/node/mempool_persist.cpp:169) and writes it as is (:196).
  • infoAll() (src/txmempool.cpp:600) returns GetSortedScoreWithTopology() (src/txmempool.cpp:572), sorted by the txgraph's main order, which is topological (through v28, infoAll used GetSortedDepthAndScore, ancestor count first: v28.0 src/txmempool.cpp:830).
  • LoadMempool admits each record as it reads it (src/node/mempool_persist.cpp:79 onward), with no reordering.

Fix

  • New parent_first_order in node/src/mempool/persist.rs: a stable topological order of a list of transactions. One iterative depth-first pass over the spend edges whose parent is also in the list, so it is linear in the number of inputs and does not recurse on the call stack. Unrelated transactions keep their input order. A duplicate txid links its children to the first copy; nothing is dropped.
  • Dump: entries are sorted by (time, txid) and then put parent-first before they are written, so the file is in Core's shape: parents before children, oldest first otherwise.
  • Load: after the existing time sort, the same pass puts every record after any parent that is in the same file. This makes a file written by 0.6.0, in arbitrary order, reload in full, and also covers a parent stamped later than its child.
  • Everything mempool RPCs: match Bitcoin Core's getmempoolentry, getmempoolinfo, gettxout and sendrawtransaction #946 added is unchanged: entry times are kept, prioritisetransaction deltas are applied before admission, expired records are skipped, and loading stays linear (one hash map and one pass; no retry loop).
  • The mempool.dat format is unchanged.

Tests

New file node/src/mempool/persist_reload_order_tests.rs (a module of persist.rs; the existing test helpers there became pub(super)):

test what it checks fails without the fix
dump_writes_same_second_parents_before_children_and_all_reload 8 generations (a two-output root, two children, a grandchild spending both, a chain of four) admitted in one second; the dumped file is parent-first and a fresh pool re-admits all 8 with their times yes, record N comes before its parent at record M (file-order assertion)
load_admits_a_file_written_children_first the same family written children first in one second (the 0.6.0 shape): all 8 re-admitted yes, (accepted, skipped, expired) assertion
load_admits_a_parent_stamped_later_than_its_child an in-file dependency outranks the times; both keep their own times yes, (accepted, skipped) assertion
load_still_skips_a_child_whose_parent_is_missing paired refusal + must-succeed: a child of an absent parent is still skipped, an unrelated record beside it is admitted n/a (passes both ways; guards against over-admission)
parent_first_order_keeps_unrelated_records_in_place no links: input order is kept (the oldest-first sort survives) n/a
parent_first_order_reorders_a_reversed_family output is a permutation and parent-first yes, with the parent edge removed
parent_first_order_handles_a_long_reversed_chain_and_duplicates a 20,001-deep reversed chain plus a duplicate record: no stack overflow, nothing dropped, parent-first yes, with the parent edge removed

Perturbation, each run on the node lib mempool::persist tests:

  • both call sites reverted (the unfixed code): the three reload tests fail.
  • only the dump ordering reverted: dump_writes_same_second_parents_before_children_and_all_reload fails (file-order assertion; the load pass alone would still reload it, which is why the test checks the file).
  • only the load ordering reverted: load_admits_a_file_written_children_first and load_admits_a_parent_stamped_later_than_its_child fail.
  • the parent edge in parent_first_order removed: five tests fail, including both parent_first_order_* ordering tests.

Local gate: cargo clippy --all-targets --all-features -D warnings, cargo clippy -p node --no-default-features --all-targets -D warnings, cargo test -p node --lib, cargo test -p satd --test regtest -- test_savemempool test_node_restart_persistence, and the --features e2e test build.

Notes

Release notes

A clean restart could drop most of the saved mempool: mempool.dat was re-admitted in order of one-second entry time and written in no particular order, so a child saved in the same second as its parent was often tried first and refused, along with its descendants. satd now writes the file parents first, as Bitcoin Core does, and on load puts every transaction after any parent in the same file, so a mempool.dat saved by 0.6.0 also reloads in full. The file format is unchanged.

🤖 Generated with Claude Code

dump_mempool wrote mempool.dat in the pool's map order, and load_mempool
re-admitted records in order of their one-second entry time, keeping file
order on a tie. A child that entered in the same second as its parent was
often tried first, refused for missing inputs, and its descendants after
it, so a restart could drop most of the saved mempool.

Bitcoin Core writes the file from infoAll(), which is in topological order
(src/txmempool.cpp, GetSortedScoreWithTopology), and LoadMempool admits in
file order (src/node/mempool_persist.cpp). satd now writes the file
oldest-first with every parent ahead of its children, and on load runs
the same stable dependency pass over the records after the time sort, so
a file written by 0.6.0 in arbitrary order also reloads in full. The pass
is one depth-first walk over the in-file spend edges, linear in inputs.
The file format is unchanged.

Fixes #949.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@bkeroack
bkeroack merged commit 1899cf2 into release/0.6 Oct 14, 2026
35 checks passed
@bkeroack
bkeroack deleted the fix/mempool-reload-parent-first branch October 14, 2026 21:44
bkeroack added a commit that referenced this pull request Oct 15, 2026
dump_mempool wrote mempool.dat in the pool's map order, and load_mempool
re-admitted records in order of their one-second entry time, keeping file
order on a tie. A child that entered in the same second as its parent was
often tried first, refused for missing inputs, and its descendants after
it, so a restart could drop most of the saved mempool.

Bitcoin Core writes the file from infoAll(), which is in topological order
(src/txmempool.cpp, GetSortedScoreWithTopology), and LoadMempool admits in
file order (src/node/mempool_persist.cpp). satd now writes the file
oldest-first with every parent ahead of its children, and on load runs
the same stable dependency pass over the records after the time sort, so
a file written by 0.6.0 in arbitrary order also reloads in full. The pass
is one depth-first walk over the in-file spend edges, linear in inputs.
The file format is unchanged.

Fixes #949.

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
(cherry picked from commit 1899cf2)
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