Repository: bitcoin/bitcoin (language: cpp)

Issue:
migratewallet crashes on an freshly created datadir ( wallet/wallet.h:959: int wallet::CWallet::GetLastBlockHeight() const: Assertion `m_last_block_processed_height >= 0' failed.)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

crash

### Expected behaviour

no crash

### Steps to reproduce

```
./src/qt/bitcoin-qt -datadir=/tmp/ -regtest -printtoconsole=1 -nolisten  # Create fresh datadir (empty), stop
mv /tmp/reg_scratch/.bitcoin/regtest/wallet.1491854176.bak /tmp/regtest/wallets/  # Move over a wallet file
(Start qt with the same args as above)
(Call migratewallet)
```



### Relevant log output

```
#5  0x0000555555bd2bda in wallet::CWallet::GetTxDepthInMainChain(wallet::CWalletTx const&) const (this=<optimized out>, wtx=<optimized out>) at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/std_function.h:290
#6  wallet::CWallet::MarkConflicted(uint256 const&, int, uint256 const&)::$_0::operator()(wallet::CWalletTx&) const (this=<optimized out>, wtx=<optimized out>) at wallet/wallet.cpp:1351
#7  std::__invoke_impl<wallet::CWallet::TxUpdate, wallet::CWallet::MarkConflicted(uint256 const&, int, uint256 const&)::$_0&, wallet::CWalletTx&>(std::__invoke_other, wallet::CWallet::MarkConflicted(uint256 const&, int, uint256 const&)::$_0&, wallet::CWalletTx&) (__f=<optimized out>, __args=<optimized out>) at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/invoke.h:61
#8  std::__invoke_r<wallet::CWallet::TxUpdate, wallet::CWallet::MarkConflicted(uint256 const&, int, uint256 const&)::$_0&, wallet::CWalletTx&>(wallet::CWallet::MarkConflicted(uint256 const&, int, uint256 const&)::$_0&, wallet::CWalletTx&) (__fn=<optimized out>, __args=<optimized out>) at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/invoke.h:114
#9  std::_Function_handler<wallet::CWallet::TxUpdate (wallet::CWalletTx&), wallet::CWallet::MarkConflicted(uint256 const&, int, uint256 const&)::$_0>::_M_invoke(std::_Any_data const&, wallet::CWalletTx&)
    (__functor=<optimized out>, __args=<optimized out>) at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/std_function.h:290
#10 0x0000555555bbb73b in std::function<wallet::CWallet::TxUpdate (wallet::CWalletTx&)>::operator()(wallet::CWalletTx&) const (this=0x7fff417f6bc0, __args=...)
    at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/std_function.h:591
#11 wallet::CWallet::RecursiveUpdateTxState(uint256 const&, std::function<wallet::CWallet::TxUpdate (wallet::CWalletTx&)> const&) (this=0x7fff2db509f0, tx_hash=..., try_updating_state=...) at wallet/wallet.cpp:1382
#12 0x0000555555bb9c4b in wallet::CWallet::MarkConflicted(uint256 const&, int, uint256 const&) (this=0x7fff2db509f0, hashBlock=..., conflicting_height=-1, hashTx=...) at wallet/wallet.cpp:1361
#13 0x0000555555bb9b55 in wallet::CWallet::LoadToWallet(uint256 const&, std::function<bool (wallet::CWalletTx&, bool)> const&) (this=0x7fff2db509f0, hash=<optimized out>, fill_wtx=<optimized out>) at wallet/wallet.cpp:1213
#14 0x0000555555bfc8c6 in wallet::LoadTxRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::vector<uint256, std::allocator<uint256> >&, bool&)::$_0::operator()(wallet::CWallet*, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) const (this=0x7fff417f6ed0, pwallet=0x7fff2db509f0, key=<optimized out>, value=..., err="") at wallet/walletdb.cpp:1065
#15 std::__invoke_impl<wallet::DBErrors, wallet::LoadTxRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::vector<uint256, std::allocator<uint256> >&, bool&)::$_0&, wallet::CWallet*, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&>(std::__invoke_other, wallet::LoadTxRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::vector<uint256, std::allocator<uint256> >&, bool&)::$_0&, wallet::CWallet*&&, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (__f=..., __args=<optimized out>, __args=<optimized out>, __args=..., __args="")
    at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/invoke.h:61
#16 std::__invoke_r<wallet::DBErrors, wallet::LoadTxRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::vector<uint256, std::allocator<uint256> >&, bool&)::$_0&, wallet::CWallet*, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&>(wallet::LoadTxRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::vector<uint256, std::allocator<uint256> >&, bool&)::$_0&, wallet::CWallet*&&, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (__fn=..., __args=<optimized out>, __args=<optimized out>, __args=..., __args="")
    at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/invoke.h:114
#17 std::_Function_handler<wallet::DBErrors (wallet::CWallet*, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&), wallet::LoadTxRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::vector<uint256, std::allocator<uint256> >&, bool&)::$_0>::_M_invoke(std::_Any_data const&, wallet::CWallet*&&, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&)
    (__functor=..., __args=<optimized out>, __args=<optimized out>, __args=..., __args="") at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/std_function.h:290
#18 0x0000555555bf87dc in std::function<wallet::DBErrors (wallet::CWallet*, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&)>::operator()(wallet::CWallet*, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) const (this=0x7fff417f6ed0, __args=0x7fff2db509f0, __args=..., __args=..., __args="")
    at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/std_function.h:591
#19 wallet::LoadRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, DataStream&, std::function<wallet::DBErrors (wallet::CWallet*, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&)>) (pwallet=pwallet@entry=0x7fff2db509f0, batch=..., key="tx", prefix=..., load_func=...) at wallet/walletdb.cpp:509
#20 0x0000555555bf8542 in wallet::LoadRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<wallet::DBErrors (wallet::CWallet*, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&)>) (pwallet=pwallet@entry=0x7fff2db509f0, batch=..., key="tx", load_func=...) at wallet/walletdb.cpp:523
#21 0x0000555555bf40a2 in wallet::LoadTxRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::vector<uint256, std::allocator<uint256> >&, bool&)
    (pwallet=0x7fff2db509f0, batch=..., upgraded_txs=std::vector of length 0, capacity 0, any_unordered=@0x7fff417f6f9f: false) at wallet/walletdb.cpp:1021
#22 wallet::WalletBatch::LoadWallet(wallet::CWallet*) (this=0x7fff417f7208, pwallet=0x7fff2db509f0) at wallet/walletdb.cpp:1187
#23 0x0000555555bc29fd in wallet::CWallet::LoadWallet() (this=0x7fff2db509f0) at wallet/wallet.cpp:2304
#24 0x0000555555bad386 in wallet::CWallet::Create(wallet::WalletContext&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::unique_ptr<wallet::WalletDatabase, std::default_delete<wallet::WalletDatabase> >, unsigned long, bilingual_str&, std::vector<bilingual_str, std::allocator<bilingual_str> >&)
    (context=<optimized out>, name="wallet.1491854156.bak", database=std::unique_ptr<wallet::WalletDatabase> = {...}, wallet_creation_flags=0, error=..., warnings=std::vector of length 0, capacity 0) at wallet/wallet.cpp:2868
#25 0x0000555555bcf830 in wallet::MigrateLegacyToDescriptor(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, secure_allocator<char> > const&, wallet::WalletContext&) (wallet_name="wallet.1491854156.bak", passphrase="", context=...) at wallet/wallet.cpp:4169
```

### How did you obtain Bitcoin Core

Compiled from source

### What version of Bitcoin Core are you using?

current master

### Operating system and version

Linux

### Machine specifications

yes


Discussion / hints:
[wallet.1491854156.bak.gpg.not.txt](https://github.com/bitcoin/bitcoin/files/12672554/wallet.1491854156.bak.gpg.not.txt)

@achow101 
The datadir is not corrupt, because it is empty. However, the wallet may be corrupt, as it is just a random file I found on my storage.
For reference, this is my normal regtest wallet, which may or may not be corrupt as well:

[wallet.dat.gpg.not.txt](https://github.com/bitcoin/bitcoin/files/12672613/wallet.dat.gpg.not.txt) (encrypted to DB88DB0BD2EDFBFC)
> However, the wallet may be corrupt, as it is just a random file I found on my storage.

That's not a supported use case.
Yeah, I wasn't sure how to check for wallet corruption. `getwalletinfo` passes on the file.
The issue appears to be due to having txs that are conflicted.

Fix patch:
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 245990841950c..d00f8de85f5bf 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;
 


Test patch:
diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py
index 9d381a2cd2d8b..8b0c8ce4057c0 100755
--- a/test/functional/tool_wallet.py
+++ b/test/functional/tool_wallet.py
@@ -394,6 +394,62 @@ def test_dump_createfromdump(self):
         self.assert_raises_tool_error('Error: Checksum is not the correct size', '-wallet=badload', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump')
         assert not (self.nodes[0].wallets_path / "badload").is_dir()
 
+    def test_chainless_conflicts(self):
+        self.log.info("Test wallet tool when wallet contains conflicting transactions")
+        self.restart_node(0)
+        self.generate(self.nodes[0], 101)
+
+        def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
+
+        self.nodes[0].createwallet("conflicts")
+        wallet = self.nodes[0].get_wallet_rpc("conflicts")
+        def_wallet.sendtoaddress(wallet.getnewaddress(), 10)
+        self.generate(self.nodes[0], 1)
+
+        # parent tx
+        parent_txid = wallet.sendtoaddress(wallet.getnewaddress(), 9)
+        parent_txid_bytes = bytes.fromhex(parent_txid)[::-1]
+        conflict_utxo = wallet.gettransaction(txid=parent_txid, verbose=True)["decoded"]["vin"][0]
+
+        # The specific assertion in MarkConflicted being tested requires that the parent tx is already loaded
+        # by the time the child tx is loaded. Since transactions end up being loaded in txid order due to how both
+        # and sqlite store things, we can just grind the child tx until it has a txid that is greater than the parent's.
+        locktime = 500000000 # Use locktime as nonce, starting at unix timestamp minimum
+        addr = wallet.getnewaddress()
+        while True:
+            child_send_res = wallet.send(outputs=[{addr: 8}], add_to_wallet=False, locktime=locktime)
+            child_txid = child_send_res["txid"]
+            child_txid_bytes = bytes.fromhex(child_txid)[::-1]
+            if (child_txid_bytes > parent_txid_bytes):
+                wallet.sendrawtransaction(child_send_res["hex"])
+                break
+            locktime += 1
+
+        # conflict with parent
+        conflict_unsigned = self.nodes[0].createrawtransaction(inputs=[conflict_utxo], outputs=[{wallet.getnewaddress(): 9.9999}])
+        conflict_signed = wallet.signrawtransactionwithwallet(conflict_unsigned)["hex"]
+        conflict_txid = self.nodes[0].sendrawtransaction(conflict_signed)
+        self.generate(self.nodes[0], 1)
+        assert_equal(wallet.gettransaction(txid=parent_txid)["confirmations"], -1)
+        assert_equal(wallet.gettransaction(txid=child_txid)["confirmations"], -1)
+        assert_equal(wallet.gettransaction(txid=conflict_txid)["confirmations"], 1)
+
+        self.stop_node(0)
+
+        # Wallet tool should successfully give info for this wallet
+        expected_output = textwrap.dedent(f'''\
+            Wallet info
+            ===========
+            Name: conflicts
+            Format: {"sqlite" if self.options.descriptors else "bdb"}
+            Descriptors: {"yes" if self.options.descriptors else "no"}
+            Encrypted: no
+            HD (hd seed available): yes
+            Keypool Size: {"8" if self.options.descriptors else "1"}
+            Transactions: 4
+            Address Book: 4
+        ''')
+        self.assert_tool_output(expected_output, "-wallet=conflicts", "info")
 
     def run_test(self):
         self.wallet_path = os.path.join(self.nodes[0].wallets_path, self.default_wallet_name, self.wallet_data_filename)
@@ -407,6 +463,7 @@ def run_test(self):
             # Salvage is a legacy wallet only thing
             self.test_salvage()
         self.test_dump_createfromdump()
+        self.test_chainless_conflicts()
 
 if __name__ == '__main__':
     ToolWalletTest().main()
diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py
index 395044c8b2e17..bcd71197bf138 100755
--- a/test/functional/wallet_migration.py
+++ b/test/functional/wallet_migration.py
@@ -727,6 +727,49 @@ def send_to_script(script, amount):
         self.nodes[0].loadwallet(info_migration["watchonly_name"])
         assert_equal(wallet_wo.getbalances()['mine']['trusted'], 5)
 
+    def test_conflict_txs(self):
+        self.log.info("Test migration when wallet contains conflicting transactions")
+        def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
+
+        wallet = self.create_legacy_wallet("conflicts")
+        def_wallet.sendtoaddress(wallet.getnewaddress(), 10)
+        self.generate(self.nodes[0], 1)
+
+        # parent tx
+        parent_txid = wallet.sendtoaddress(wallet.getnewaddress(), 9)
+        parent_txid_bytes = bytes.fromhex(parent_txid)[::-1]
+        conflict_utxo = wallet.gettransaction(txid=parent_txid, verbose=True)["decoded"]["vin"][0]
+
+        # The specific assertion in MarkConflicted being tested requires that the parent tx is already loaded
+        # by the time the child tx is loaded. Since transactions end up being loaded in txid order due to how both
+        # and sqlite store things, we can just grind the child tx until it has a txid that is greater than the parent's.
+        locktime = 500000000 # Use locktime as nonce, starting at unix timestamp minimum
+        addr = wallet.getnewaddress()
+        while True:
+            child_send_res = wallet.send(outputs=[{addr: 8}], add_to_wallet=False, locktime=locktime)
+            child_txid = child_send_res["txid"]
+            child_txid_bytes = bytes.fromhex(child_txid)[::-1]
+            if (child_txid_bytes > parent_txid_bytes):
+                wallet.sendrawtransaction(child_send_res["hex"])
+                break
+            locktime += 1
+
+        # conflict with parent
+        conflict_unsigned = self.nodes[0].createrawtransaction(inputs=[conflict_utxo], outputs=[{wallet.getnewaddress(): 9.9999}])
+        conflict_signed = wallet.signrawtransactionwithwallet(conflict_unsigned)["hex"]
+        conflict_txid = self.nodes[0].sendrawtransaction(conflict_signed)
+        self.generate(self.nodes[0], 1)
+        assert_equal(wallet.gettransaction(txid=parent_txid)["confirmations"], -1)
+        assert_equal(wallet.gettransaction(txid=child_txid)["confirmations"], -1)
+        assert_equal(wallet.gettransaction(txid=conflict_txid)["confirmations"], 1)
+
+        wallet.migratewallet()
+        assert_equal(wallet.gettransaction(txid=parent_txid)["confirmations"], -1)
+        assert_equal(wallet.gettransaction(txid=child_txid)["confirmations"], -1)
+        assert_equal(wallet.gettransaction(txid=conflict_txid)["confirmations"], 1)
+
+        wallet.unloadwallet()
+
     def run_test(self):
         self.generate(self.nodes[0], 101)
 
@@ -743,6 +786,7 @@ def run_test(self):
         self.test_direct_file()
         self.test_addressbook()
         self.test_migrate_raw_p2sh()
+        self.test_conflict_txs()
 
 if __name__ == '__main__':
     WalletMigrationTest().main()