From db759765d675a5b55b098b5c02a6d36ea55793af Mon Sep 17 00:00:00 2001 From: SomberNight Date: Tue, 27 May 2025 18:19:01 +0000 Subject: [PATCH] adb.get_tx_height: allow future txs to be partially signed If the full tx is missing, we should force mempool/confirmed txs to be LOCAL height, however future txs should not be forced to LOCAL, they should remain FUTURE. follow-up https://github.com/spesmilo/electrum/commit/197933debf1a31734da86edde2678c2b76987ed3 --- electrum/address_synchronizer.py | 38 +++++++++-------- tests/test_wallet_vertical.py | 70 +++++++++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 19 deletions(-) diff --git a/electrum/address_synchronizer.py b/electrum/address_synchronizer.py index de83c82b1..002c12340 100644 --- a/electrum/address_synchronizer.py +++ b/electrum/address_synchronizer.py @@ -617,6 +617,7 @@ class AddressSynchronizer(Logger, EventListener): await self._address_history_changed_events[addr].wait() def add_unverified_or_unconfirmed_tx(self, tx_hash: str, tx_height: int) -> None: + assert tx_height >= TX_HEIGHT_UNCONF_PARENT, f"got {tx_height=} for {tx_hash=}" # forbid local/future txs here with self.lock: if self.db.is_in_verified_tx(tx_hash): if tx_height <= 0: @@ -704,6 +705,24 @@ class AddressSynchronizer(Logger, EventListener): if tx_hash is None: # ugly backwards compat... return TxMinedInfo(height=TX_HEIGHT_LOCAL, conf=0) with self.lock: + if verified_tx_mined_info := self.db.get_verified_tx(tx_hash): # mined and spv-ed + conf = max(self.get_local_height() - verified_tx_mined_info.height + 1, 0) + tx_mined_info = verified_tx_mined_info._replace(conf=conf) + elif tx_hash in self.unverified_tx: # mined, no spv + height = self.unverified_tx[tx_hash] + tx_mined_info = TxMinedInfo(height=height, conf=0) + elif tx_hash in self.unconfirmed_tx: # mempool + height = self.unconfirmed_tx[tx_hash] + tx_mined_info = TxMinedInfo(height=height, conf=0) + elif wanted_height := self.future_tx.get(tx_hash): # future + if wanted_height > self.get_local_height(): + tx_mined_info = TxMinedInfo(height=TX_HEIGHT_FUTURE, conf=0, wanted_height=wanted_height) + else: + tx_mined_info = TxMinedInfo(height=TX_HEIGHT_LOCAL, conf=0) + else: # local + tx_mined_info = TxMinedInfo(height=TX_HEIGHT_LOCAL, conf=0) + if tx_mined_info.height in (TX_HEIGHT_LOCAL, TX_HEIGHT_FUTURE): + return tx_mined_info if force_local_if_missing_tx: # It can happen for a txid in any state (unconf/unverified/verified) that we # don't have the raw tx yet, simply due to network timing. @@ -713,24 +732,7 @@ class AddressSynchronizer(Logger, EventListener): tx = self.db.get_transaction(tx_hash) if tx is None or isinstance(tx, PartialTransaction): return TxMinedInfo(height=TX_HEIGHT_LOCAL, conf=0) - verified_tx_mined_info = self.db.get_verified_tx(tx_hash) - if verified_tx_mined_info: - conf = max(self.get_local_height() - verified_tx_mined_info.height + 1, 0) - return verified_tx_mined_info._replace(conf=conf) - elif tx_hash in self.unverified_tx: - height = self.unverified_tx[tx_hash] - return TxMinedInfo(height=height, conf=0) - elif tx_hash in self.unconfirmed_tx: - height = self.unconfirmed_tx[tx_hash] - return TxMinedInfo(height=height, conf=0) - elif wanted_height := self.future_tx.get(tx_hash): - if wanted_height > self.get_local_height(): - return TxMinedInfo(height=TX_HEIGHT_FUTURE, conf=0, wanted_height=wanted_height) - else: - return TxMinedInfo(height=TX_HEIGHT_LOCAL, conf=0) - else: - # local transaction - return TxMinedInfo(height=TX_HEIGHT_LOCAL, conf=0) + return tx_mined_info def up_to_date_changed(self) -> None: # fire triggers diff --git a/tests/test_wallet_vertical.py b/tests/test_wallet_vertical.py index d47fcdf9f..15e7c04c0 100644 --- a/tests/test_wallet_vertical.py +++ b/tests/test_wallet_vertical.py @@ -9,7 +9,7 @@ import copy from electrum import storage, bitcoin, keystore, bip32, slip39, wallet from electrum import SimpleConfig from electrum import util -from electrum.address_synchronizer import TX_HEIGHT_UNCONFIRMED, TX_HEIGHT_UNCONF_PARENT, TX_HEIGHT_LOCAL +from electrum.address_synchronizer import TX_HEIGHT_UNCONFIRMED, TX_HEIGHT_UNCONF_PARENT, TX_HEIGHT_LOCAL, TX_HEIGHT_FUTURE from electrum.wallet import (sweep, Multisig_Wallet, Standard_Wallet, Imported_Wallet, restore_wallet_from_text, Abstract_Wallet, CannotBumpFee, BumpFeeStrategy, TransactionPotentiallyDangerousException, TransactionDangerousException, @@ -2839,6 +2839,74 @@ class TestWalletSending(ElectrumTestCase): self.assertEqual(1, len(tx.inputs())) self.assertEqual(2, len(tx.outputs())) + @mock.patch.object(wallet.Abstract_Wallet, 'save_db') + async def test_wallet_adb_gettxheight_treats_mempool_txid_as_local_if_missing_fulltx(self, mock_save_db): + wallet = self.create_standard_wallet_from_seed('dismiss smile transfer input market ten damage city duck dolphin entire because', + config=self.config, gap_limit=2) + self.assertEqual(0, len(wallet.get_spendable_coins())) + + # fund wallet with two utxos + funding_tx1 = Transaction('02000000000101bf03f2d37ae084d729e5685d64988c92e8a98cb73062802646dfbb10d77e88410000000000fdffffff02a03007000000000016001443a24a730a7ddd2ce4da777a949a9e87c6ad870920a107000000000016001447597395323a834378d7577d848187684d0d70fe0247304402200e6f1898a0681c4ff1f5995b357c3388ca53fcf56760e0d14d4ea72c48d1134b0220683b8e5045743c087d488dfc5f8c5b7369ff92f611595eaba0dbb0c0009c816e0121021bd313412fad3802801f6c45321a10c7bf35603bf8571aa263ece764d1ab7ef1a2434300') + wallet.adb.receive_tx_callback(funding_tx1, tx_height=TX_HEIGHT_UNCONFIRMED) + self.assertEqual(1, len(wallet.get_spendable_coins(nonlocal_only=True))) + self.assertEqual(1, len(wallet.get_spendable_coins(nonlocal_only=False))) + + funding_tx2 = Transaction('02000000000101d8a9691c534e90655623cd1a642c3b3f31db09548a5922e0218289a34daf27fc0000000000fdffffff021061070000000000160014910f3a772d33c615abe4f1c346476cae1414f6d7c027090000000000160014071955c9141dfaa8df1abbfe04527ff061b652450247304402203e45c9d4191239273af9fa97eb986f66afe66345a2f2b6284e214ab91fce072802205a50e8b74f191202442876d6a0cd7e95262e6c125a21eebaebe0bd93aa15107f0121022e8590152fad3aa6a8730648dfcb84ebe432c9190987d498b81707588e40626da2434300') + wallet.adb.receive_tx_callback(funding_tx2, tx_height=TX_HEIGHT_UNCONFIRMED) + self.assertEqual(2, len(wallet.get_spendable_coins(nonlocal_only=True))) + self.assertEqual(2, len(wallet.get_spendable_coins(nonlocal_only=False))) + + # create payment_tx that spends utxo1 and creates a change txo + outputs = [PartialTxOutput.from_address_and_value('tb1qrxrp08s5d4cgudlmyfasyme9rgxc7n6z29g2m9', 200_000)] + coins = wallet.get_spendable_coins() + payment_tx = wallet.make_unsigned_transaction(coins=[coins[0]], outputs=outputs, fee_policy=FixedFeePolicy(0)) + payment_txid = payment_tx.txid() + assert payment_txid + # save payment_tx as LOCAL and UNSIGNED + wallet.adb.add_transaction(payment_tx) + self.assertEqual(TX_HEIGHT_LOCAL, wallet.adb.get_tx_height(payment_txid).height) + self.assertEqual(1, len(wallet.get_spendable_coins(nonlocal_only=True))) + self.assertEqual(2, len(wallet.get_spendable_coins(nonlocal_only=False))) + # transition payment_tx to mempool (but it is still unsigned!) + # This can happen organically in a workflow if + # 1. we save as local an unsigned tx, + # 2. sign+broadcast it, but we don't save the signed tx as local, + # 3. then some RTTs later the server will tell us that the txid is now in the mempool (or mined), + # 4. then yet more RTTs later we request and receive the full tx from the server + # between (3) and (4), the wallet could consider txid to be mempool/mined, + # but the wallet db does not yet have the corresponding full tx. + # In such cases, we instead want the txid to be considered LOCAL. + wallet.adb.receive_tx_callback(payment_tx, tx_height=TX_HEIGHT_UNCONFIRMED) + self.assertEqual(TX_HEIGHT_LOCAL, wallet.adb.get_tx_height(payment_txid).height) + self.assertEqual(1, len(wallet.get_spendable_coins(nonlocal_only=True))) + self.assertEqual(2, len(wallet.get_spendable_coins(nonlocal_only=False))) + # wallet gets signed tx (e.g. from network). payment_tx is now considered to be in mempool + wallet.sign_transaction(payment_tx, password=None) + wallet.adb.receive_tx_callback(payment_tx, tx_height=TX_HEIGHT_UNCONFIRMED) + self.assertEqual(TX_HEIGHT_UNCONFIRMED, wallet.adb.get_tx_height(payment_txid).height) + self.assertEqual(2, len(wallet.get_spendable_coins(nonlocal_only=True))) + self.assertEqual(2, len(wallet.get_spendable_coins(nonlocal_only=False))) + + @mock.patch.object(wallet.Abstract_Wallet, 'save_db') + async def test_wallet_adb_gettxheight_treats_future_txid_as_future_even_if_missing_fulltx(self, mock_save_db): + wallet = self.create_standard_wallet_from_seed('dismiss smile transfer input market ten damage city duck dolphin entire because', + config=self.config, gap_limit=2) + # fund wallet + funding_tx1 = Transaction('02000000000101bf03f2d37ae084d729e5685d64988c92e8a98cb73062802646dfbb10d77e88410000000000fdffffff02a03007000000000016001443a24a730a7ddd2ce4da777a949a9e87c6ad870920a107000000000016001447597395323a834378d7577d848187684d0d70fe0247304402200e6f1898a0681c4ff1f5995b357c3388ca53fcf56760e0d14d4ea72c48d1134b0220683b8e5045743c087d488dfc5f8c5b7369ff92f611595eaba0dbb0c0009c816e0121021bd313412fad3802801f6c45321a10c7bf35603bf8571aa263ece764d1ab7ef1a2434300') + wallet.adb.receive_tx_callback(funding_tx1, tx_height=TX_HEIGHT_UNCONFIRMED) + # create payment_tx that spends utxo1 and creates a change txo + outputs = [PartialTxOutput.from_address_and_value('tb1qrxrp08s5d4cgudlmyfasyme9rgxc7n6z29g2m9', 200_000)] + coins = wallet.get_spendable_coins() + payment_tx = wallet.make_unsigned_transaction(coins=coins, outputs=outputs, fee_policy=FixedFeePolicy(0)) + payment_txid = payment_tx.txid() + assert payment_txid + # save payment_tx as LOCAL and UNSIGNED + wallet.adb.add_transaction(payment_tx) + self.assertEqual(TX_HEIGHT_LOCAL, wallet.adb.get_tx_height(payment_txid).height) + # mark payment_tx as future + wallet.adb.set_future_tx(payment_txid, wanted_height=300) + self.assertEqual(TX_HEIGHT_FUTURE, wallet.adb.get_tx_height(payment_txid).height) + @mock.patch.object(wallet.Abstract_Wallet, 'save_db') async def test_imported_wallet_usechange_off(self, mock_save_db): wallet = restore_wallet_from_text(