mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-22 23:07:59 +01:00
Code Issues Projects Releases Wiki Activity

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

Browse source 
782701ce7d test: Test loading wallets with conflicts without a chain (Andrew Chow)
4660fc82a1 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 782701ce7d. Nice catch, and clever test (grinding the txid)
  furszy:
    ACK 782701ce

Tree-SHA512: 1344e0279ec5413a43a2819d101fb571fbf4821de2d13958a0fdffc99f57082ef3243ec454c8343f97dc02ed1fce8c8b0fd89388420ab2e55618af42ad5630a9
This commit is contained in:
fanquake 2023-10-02 13:09:46 +01:00
commit 50f250a67d
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
3 changed files with 105 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
src/wallet/wallet.cpp
View file

@ -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;

57
test/functional/tool_wallet.py
View file

@ -394,6 +394,62 @@ class ToolWalletTest(BitcoinTestFramework):
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 @@ class ToolWalletTest(BitcoinTestFramework):
# Salvage is a legacy wallet only thing
self.test_salvage()
self.test_dump_createfromdump()
self.test_chainless_conflicts()
if __name__ == '__main__':
ToolWalletTest().main()

44
test/functional/wallet_migration.py
View file

@ -727,6 +727,49 @@ class WalletMigrationTest(BitcoinTestFramework):
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 @@ class WalletMigrationTest(BitcoinTestFramework):
self.test_direct_file()
self.test_addressbook()
self.test_migrate_raw_p2sh()
self.test_conflict_txs()
if __name__ == '__main__':
WalletMigrationTest().main()