Skip to content

mining: Optionally make extra pool persistent#313

Open
pdath wants to merge 3 commits into
bitcoinknots:29.x-knotsfrom
pdath:feature-persist-extra-pool
Open

mining: Optionally make extra pool persistent#313
pdath wants to merge 3 commits into
bitcoinknots:29.x-knotsfrom
pdath:feature-persist-extra-pool

Conversation

@pdath

@pdath pdath commented Jun 14, 2026

Copy link
Copy Markdown

When rejecttokens=1, tokens are stored in the extra pool rather than the mempool. When Bitcoin Knots is restarted, the extra pool is lost.

The issue this creates is that compact block reconstructions for tokens become very slow after a restart because the extra pool is empty. The causes Bitcoin Knots to have to fetch thousands of transactions from peers. Examples are (with "debug=cmpctblock"):

2026-06-10T06:22:16.634294Z [cmpctblock] Successfully reconstructed block 00000000000000000000aa2798940f3de06a3b6b8bf845bb872a1322a8ce6688 with 1 txn prefilled, 2181 txn from mempool (incl at least 804 from extra pool) and 2297 txn requested 2026-06-10T06:47:45.132446Z [cmpctblock] Successfully reconstructed block 000000000000000000009b2bb14d59eb073cb3aa0a1a370236b5822b99663340 with 1 txn prefilled, 2847 txn from mempool (incl at least 435 from extra pool) and 959 txn requested 2026-06-10T06:55:41.588544Z [cmpctblock] Successfully reconstructed block 00000000000000000000a51c141257c439152ffb45c6a3bded318ffaef161b78 with 1 txn prefilled, 2523 txn from mempool (incl at least 521 from extra pool) and 1800 txn requested

The issue does not happen when rejecttokens=0, because tokens then get stored in "mempool.dat", and persisted across restarts.

To resolve this issue, I propose treating the extra pool like the mempool and saving/loading the state to "extrapool.dat", controlled by a new configuration setting persistextrapool, that works similarly to persistmempool. persistextrapool will default to 0 (off).

When rejecttokens=1 tokens are stored in the extra pool, rather than the mempool.  When Bitcoin Knots is restarted, the extra pool is lost.

The issue this creates is that compact block reconstructions for tokens becomes very slow after a restart, because the extra pool is empty.  The causes Bitcoin Knots to have to fetch thousands of transactions from peers.  Examples are (with "debug=cmpctblock"):

2026-06-10T06:22:16.634294Z [cmpctblock] Successfully reconstructed block 00000000000000000000aa2798940f3de06a3b6b8bf845bb872a1322a8ce6688 with 1 txn prefilled, 2181 txn from mempool (incl at least 804 from extra pool) and 2297 txn requested
2026-06-10T06:47:45.132446Z [cmpctblock] Successfully reconstructed block 000000000000000000009b2bb14d59eb073cb3aa0a1a370236b5822b99663340 with 1 txn prefilled, 2847 txn from mempool (incl at least 435 from extra pool) and 959 txn requested
2026-06-10T06:55:41.588544Z [cmpctblock] Successfully reconstructed block 00000000000000000000a51c141257c439152ffb45c6a3bded318ffaef161b78 with 1 txn prefilled, 2523 txn from mempool (incl at least 521 from extra pool) and 1800 txn requested

To resolve this issue I propose treating the extra pool like the mempool, and saving/loading the state to extrapool.dat, but only when rejecttokens=1.

The issue does not happen when rejecttokens=0, because tokens then get stored in mempool.dat, and persisted across restarts.
Comment thread src/node/extrapool_persist.cpp Outdated
Comment thread src/node/extrapool_persist.cpp Outdated
Comment thread src/node/extrapool_persist.cpp Outdated
Comment thread src/net_processing.cpp
@pdath pdath changed the title mining: Make extra pool persistent when rejecttokens=1 mining: Optionally make extra pool persistent Jul 5, 2026
@pdath pdath force-pushed the feature-persist-extra-pool branch from ff8b3c8 to 266e83b Compare July 5, 2026 05:41
…pool

Implement reviewer feedback for extra pool persistence:

- Introduce -persistextrapool option (default: 0) to independently control
  whether the extra pool is persisted to disk, decoupling from -rejecttokens
- Add memory DoS protection during LoadExtraPool: enforce per-TX size limit
  (BLOCK_RECONSTRUCTION_EXTRA_TXN_PER_TXN_SIZE_LIMIT) and cumulative memory
  limit (max_mem_bytes) to prevent malicious extrapool.dat from exhausting memory
- Fix ring buffer position inconsistency: derive vExtraTxnForCompactIt from
  pool size on load instead of persisting position to disk, maintaining
  the invariant that TXs are at [0, pool.size()) with next write at
  pool.size() % max_count
- Simplify interface: GetExtraPoolForDump() returns vector only (no position),
  SetExtraPool() takes pool and memusage only (position derived from pool size)
- Add Doxygen comments to all public functions in extrapool_persist
- Update file format: version(8) + count(8) + transactions (24 bytes removed)
- Tighten functional test assertion: require len(requested) == 0 instead of
  len(requested) < num_token_txs to ensure all persisted transactions are
  actually reloaded
- Update copyright headers to 2026 The Bitcoin Knots
@pdath pdath force-pushed the feature-persist-extra-pool branch from 266e83b to 1447153 Compare July 5, 2026 05:51

@Retropex Retropex left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK

…error handling.

- Replace pre-allocation with push_back() to eliminate null entries when
  skipping oversized transactions during LoadExtraPool()
- Guard against division by zero: check max_count > 0 before modulo operation
  in pool_pos calculation
- Add test case for zero-capacity (max_count=0) handling

This ensures the extra pool vector only contains valid transactions, matching
runtime behavior where oversized TXs are rejected before insertion.
@pdath

pdath commented Jul 9, 2026

Copy link
Copy Markdown
Author

I have added an extra commit to resolve a UBSAN failure in the unit test. I have also further improved some of the error handling. Highlights:

  • Replace pre-allocation with push_back() to eliminate null entries when
    skipping oversized transactions during LoadExtraPool()
  • Guard against division by zero: check max_count > 0 before modulo operation
    in pool_pos calculation
  • Add test case for zero-capacity (max_count=0) handling

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.

3 participants