Merge bitcoin/bitcoin#28542: wallet: Check for uninitialized last processed and conflicting heights in MarkConflicted

782701ce7d31919dba2241ee43b582d8ae5a2541 test: Test loading wallets with conflicts without a chain (Andrew Chow)
4660fc82a1f5cf6eb6404d5268beef5919581661 wallet: Check last block and conflict height are valid in MarkConflicted (Andrew Chow)

Pull request description:

  `MarkConflicted` assumes that `m_last_block_processed_height` is always valid. However it may not be valid when a chain is not attached, as happens in the wallet tool and during migration. In such situations, when the conflicting height is also negative (which occurs on loading when no chain is available), the calculation of the number of conflict confirms results in a non-negative value which passes the existing check for valid values. This will subsequently hit an assertion in `GetTxDepthInMainChain`.

  Furthermore, `MarkConflicted` is also only called on loading a transaction whose parent has a stored state of `TxStateConflicted` and was loaded before the child transaction. This depends on the loading order, which for both sqlite and bdb depends on the txids.

  We can avoid this by explicitly checking that both `m_last_block_processed_height` and `conflicting_height` are non-negative. Both `tool_wallet.py` and `wallet_migration.py` are updated to create wallets with a state that triggers the assertion.

  Fixes #28510

ACKs for top commit:
  ryanofsky:
    Code review ACK 782701ce7d31919dba2241ee43b582d8ae5a2541. Nice catch, and clever test (grinding the txid)
  furszy:
    ACK 782701ce

Tree-SHA512: 1344e0279ec5413a43a2819d101fb571fbf4821de2d13958a0fdffc99f57082ef3243ec454c8343f97dc02ed1fce8c8b0fd89388420ab2e55618af42ad5630a9

1 files changed, 4 insertions, 1 deletions

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 2459908419..d00f8de85f 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1339,11 +1339,14 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c
 {
     LOCK(cs_wallet);
 
-    int conflictconfirms = (m_last_block_processed_height - conflicting_height + 1) * -1;
     // If number of conflict confirms cannot be determined, this means
     // that the block is still unknown or not yet part of the main chain,
     // for example when loading the wallet during a reindex. Do nothing in that
     // case.
+    if (m_last_block_processed_height < 0 || conflicting_height < 0) {
+        return;
+    }
+    int conflictconfirms = (m_last_block_processed_height - conflicting_height + 1) * -1;
     if (conflictconfirms >= 0)
         return;