improve mempool reorg logic when the peak is a non-transaction block

[arvidn]

Purpose:

This PR addresses the issue raised by #17186.

Background

In order for the mempool to evict transactions as they become invalid (either because they made it into a block or the coins they spend became spent by some other transaction), it needs to know about new blocks added to the chain.

The "ground truth" about whether a transaction is valid or not is the CoinStore, which the mempool has access to via the get_coin_record function, passed into it. This function reflects the peak of the chain, the current coin set.

transaction blocks

The mempool needs to keep a reference to the most recent transaction block, because those are what matters for timelock conditions. e.g. height-based timelock conditions compare the argument against the previous transaction block's height, and the time-based timelock conditions compare the argument against the previous transaction block's timestamp. Only transaction blocks have timestamps. (see mempool_check_time_locks())

Since the mempool validates transactions before including them, it needs to know about the most recent transaction block, not necessarily the actual peak.

Reorgs

When a re-org happens, the fork we switch over to is not replayed one block at a time to the mempool, it will just learn about the most recent peak. If this peak is not a transaction block, there's not much the mempool can do about it (currently). It can't update its set of transactions, because it doesn't know what the most recent transcaction block is.

As pointed out in #17186, this delays updating the mempool until we receive a transaction block on the new chain. This in turn risks us failing to create a valid block if we happen to find a valid proof-of-space for that next transaction block.

Solution

Since the mempool itself can't traverse the blockchain, even if it would detect the reorg, it can't step back to find the most recent transaction block on the new chain. A simpler solution (I believe) is to ensure that the full node only ever passes the most recent transaction block to the mempool.

This patch adds a sibling function to Blockchain.get_peak() called Blockchain.get_tx_peak(), which returns the most recent transaction block (which may be the peak, if that is a transaction block).

Current Behavior:

the mempool may be delayed one block in updating after a reorg

New Behavior:

The mempool updates immediately with a reorg

Testing Notes:

There are two new tests for the Blockchain class, one where blocks are just added linearly and the get_tx_block() is tested to always return the most recent tx block. The other test exrcises a reorg where the switch-over happens on a non-transaction block, and ensures it still returns the most recent one.

[coveralls-official]

Pull Request Test Coverage Report for Build 7595896331

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.009%) to 90.578%

---

Totals
Change from base Build 7591684419:	0.009%
Covered Lines:	94817
Relevant Lines:	104638

---

💛 - Coveralls

[AmineKhaldi]

Looks good! Just one suggestion.