Merge "verify_sigs": adb: check sigs of is_mine txins of unconf txs

### electrum/address_synchronizer.py
@@ -27,12 +27,14 @@
 import threading
 import itertools
 from collections import defaultdict
-from typing import TYPE_CHECKING, Dict, Optional, Set, Tuple, NamedTuple, Sequence, List
+from typing import TYPE_CHECKING, Dict, Optional, Set, Tuple, NamedTuple, Sequence, List, Callable, Iterable
 
 from .crypto import sha256
 from . import bitcoin, util
 from .bitcoin import COINBASE_MATURITY
-from .util import profiler, bfh, TxMinedInfo, UnrelatedTransactionException, with_lock, OldTaskGroup
+from .util import (
+    profiler, bfh, TxMinedInfo, UnrelatedTransactionException, InvalidTxSignature, with_lock, OldTaskGroup,
+)
 from .transaction import Transaction, TxOutput, TxInput, PartialTxInput, TxOutpoint, PartialTransaction, tx_from_any
 from .synchronizer import Synchronizer
 from .verifier import SPV
@@ -82,7 +84,11 @@ class AddressSynchronizer(Logger, EventListener):
     synchronizer: Optional['Synchronizer']
     verifier: Optional['SPV']
 
-    def __init__(self, db: 'WalletDB', config: 'SimpleConfig', *, name: str | None = None):
+    def __init__(
+            self, db: 'WalletDB', config: 'SimpleConfig', *,
+            verify_txins: Optional[Callable[[Transaction, Dict[int, Transaction]], bool]],
+            name: str | None = None,
+    ):
         self.db = db
         self.config = config
         self.name = name
@@ -92,9 +98,13 @@ def __init__(self, db: 'WalletDB', config: 'SimpleConfig', *, name: str | None =
         self.synchronizer = None
         self.verifier = None
         self.lock = threading.RLock()
+        # (tx, {txin_idx: prev_tx}) -> whether these txins are validly signed
+        self.verify_txins = None  # type: Optional[Callable[[Transaction, Dict[int, Transaction]], bool]]
         self.future_tx = {}  # type: Dict[str, int]  # txid -> wanted (abs) height
         # Txs the server claims are mined but still pending verification:
         self.unverified_tx = defaultdict(int)  # type: Dict[str, int]  # txid -> height. Access with self.lock.
+        # Txs received from the server that it claims are mined, held until verified:
+        self._held_txs = {}  # type: Dict[str, Transaction]  # txid -> tx. Access with self.lock.
         # Txs the server claims are in the mempool:
         self.unconfirmed_tx = defaultdict(int)  # type: Dict[str, int]  # txid -> height. Access with self.lock.
         # thread local storage for caching stuff
@@ -104,6 +114,8 @@ def __init__(self, db: 'WalletDB', config: 'SimpleConfig', *, name: str | None =
         self._get_utxos_cache = {}
 
         self.load_and_cleanup()
+        # set after load_and_cleanup, as the wallet is not fully constructed yet
+        self.verify_txins = verify_txins
 
     @with_lock
     def invalidate_cache(self):
@@ -308,6 +320,14 @@ def add_transaction(self, tx: Transaction, *, allow_unrelated=False, is_new=True
                 is_for_me = any([self.is_mine(txo.address) for txo in tx.outputs()])
                 if not is_mine and not is_for_me:
                     raise UnrelatedTransactionException()
+            # check signatures of inputs spending our coins
+            stored_tx = self.db.get_transaction(tx_hash)
+            is_new_witness = stored_tx is None or isinstance(stored_tx, PartialTransaction)
+            checked_tx = tx if is_new_witness else stored_tx  # db keeps a complete tx
+            if not self._are_new_inputs_valid(checked_tx, range(len(tx.inputs())), is_new_witness=is_new_witness):
+                if not is_new_witness:
+                    self.remove_transaction(tx_hash)
+                raise InvalidTxSignature()
             # Find all conflicting transactions.
             # In case of a conflict,
             #     1. confirmed > mempool > local
@@ -368,10 +388,16 @@ def add_value_from_prev_output():
                     # give v to txi that spends me
                     next_tx = self.db.get_spent_outpoint(tx_hash, n)
                     if next_tx is not None:
+                        spender_tx = self.db.get_transaction(next_tx)
+                        txin_idx = spender_tx.get_input_idx_that_spent_prevout(TxOutpoint.from_str(ser)) if spender_tx else None
+                        if txin_idx is not None and not self._are_new_inputs_valid(
+                                spender_tx, [txin_idx], prev_tx=tx, is_new_witness=False):
+                            self.logger.warning(f"removing tx with invalid signature: {next_tx}")
+                            self.remove_transaction(next_tx)
+                            continue
                         is_new_txi = self.db.add_txi_addr(next_tx, addr, ser, v)
                         self._add_tx_to_local_history(next_tx)
                         if is_new_txi:
-                            spender_tx = self.db.get_transaction(next_tx)
                             assert spender_tx
                             util.trigger_callback('adb_updated_tx', self, next_tx, spender_tx)
             # add to local history
@@ -386,6 +412,33 @@ def add_value_from_prev_output():
                 util.trigger_callback('adb_updated_tx', self, tx_hash, tx)
             return True
 
+    def _are_new_inputs_valid(
+            self, tx: Transaction, txin_idxs: Iterable[int], *, prev_tx: Transaction = None, is_new_witness: bool,
+    ) -> bool:
+        """Checks the signatures of txins spending our coins.
+        If is_new_witness (tx will replace the stored one), txins already linked to our coins are checked too."""
+        if self.verify_txins is None or not tx.is_complete():
+            return True
+        if isinstance(tx, PartialTransaction):
+            tx = Transaction(tx.serialize())  # input addresses from our db, not from the psbt
+        if self.db.is_in_verified_tx(tx.txid()):
+            return True  # mined
+        prev_txs = {}
+        for txin_idx in txin_idxs:
+            txin = tx.inputs()[txin_idx]
+            if txin.is_coinbase_input():
+                continue
+            addr = self.get_txin_address(txin)
+            if not self.is_mine(addr):
+                continue
+            txin_prev_tx = prev_tx or self.db.get_transaction(txin.prevout.txid.hex())
+            if txin_prev_tx is None:
+                continue  # checked when prev_tx is added
+            if not is_new_witness and txin.prevout.to_str() in dict(self.db.get_txi_addr(tx.txid(), addr)):
+                continue
+            prev_txs[txin_idx] = txin_prev_tx
+        return not prev_txs or self.verify_txins(tx, prev_txs)
+
     @with_lock
     def remove_transaction(self, tx_hash: str) -> None:
         """Removes a transaction AND all its dependents/children
@@ -453,6 +506,11 @@ def receive_tx_callback(self, tx: Transaction, *, tx_height: Optional[int] = Non
         if tx_height is not None:
             # note: tx_height is only set by the unit tests: to inject a tx into the history
             self.add_unverified_or_unconfirmed_tx(txid, tx_height)
+        elif txid in self.unverified_tx:
+            # the server claims tx is mined: add it once verified, so that its signatures need not be checked
+            self._held_txs[txid] = tx
+            return
+        self._held_txs.pop(txid, None)
         self.add_transaction(tx, allow_unrelated=True)
 
     @with_lock
@@ -476,7 +534,13 @@ def receive_history_callback(self, addr: str, hist, tx_fees: Dict[str, int]):
             tx = self.db.get_transaction(tx_hash)
             if tx is None:
                 continue
-            self.add_transaction(tx, allow_unrelated=True, is_new=False)
+            try:
+                self.add_transaction(tx, allow_unrelated=True, is_new=False)
+            except InvalidTxSignature:
+                # do not disconnect here, because the invalid tx may have been sent by another server.
+                # _request_missing_txs will ask the current server for it
+                self.logger.warning(f"removed tx with invalid signature: {tx_hash}")
+                continue
             # if we already had this tx, see if its height changed (e.g. local->unconfirmed)
             old_height = old_hist.get(tx_hash, None)
             if old_height is not None and old_height != tx_height:
@@ -661,15 +725,31 @@ def remove_unverified_tx(self, tx_hash: str, tx_height: int) -> None:
         if new_height == tx_height:
             self.unverified_tx.pop(tx_hash, None)
             self.invalidate_cache()
+            self._add_held_tx(tx_hash)
 
     def add_verified_tx(self, tx_hash: str, info: TxMinedInfo):
         # Remove from the unverified map and add to the verified map
         with self.lock:
             self.unverified_tx.pop(tx_hash, None)
             self.db.add_verified_tx(tx_hash, info)
             self.invalidate_cache()
+            self._add_held_tx(tx_hash)
         util.trigger_callback('adb_added_verified_tx', self, tx_hash)
 
+    def _add_held_tx(self, tx_hash: str) -> None:
+        tx = self._held_txs.pop(tx_hash, None)
+        if tx is not None:
+            self.add_transaction(tx, allow_unrelated=True)
+
+    @with_lock
+    def is_held_tx(self, tx_hash: str) -> bool:
+        # not if the server no longer claims it is mined
+        return tx_hash in self._held_txs and tx_hash in self.unverified_tx
+
+    @with_lock
+    def clear_held_txs(self) -> None:
+        self._held_txs.clear()
+
     @with_lock
     def get_unverified_txs(self) -> Dict[str, int]:
         '''Returns a map from tx hash to transaction height'''

### electrum/plugins/watchtower/watchtower.py
@@ -68,7 +68,8 @@ def __init__(self, network: 'Network'):
         Logger.__init__(self)
         self.config = network.config
         wallet_db = WalletDB('', storage=None, upgrade=True)
-        self.adb = AddressSynchronizer(wallet_db, self.config, name=self.diagnostic_name())
+        # the watchtower has no keys: it cannot verify signatures
+        self.adb = AddressSynchronizer(wallet_db, self.config, verify_txins=None, name=self.diagnostic_name())
         self.adb.start_network(network)
         self.callbacks = {}  # outpoint -> lambda function
         self.register_callbacks()

### electrum/synchronizer.py
@@ -153,6 +153,8 @@ def _reset(self):
         self.requested_tx = set()  # type: Set[str]
         self.requested_histories = set()
         self._stale_histories = dict()  # type: Dict[str, asyncio.Task]
+        # held txs were received from the previous server
+        self.adb.clear_held_txs()
 
     def diagnostic_name(self):
         return self.adb.diagnostic_name()
@@ -241,6 +243,8 @@ async def _request_missing_txs(self, hist, *, allow_server_not_finding_tx=False)
             tx = self.adb.db.get_transaction(tx_hash)
             if tx and not isinstance(tx, PartialTransaction):
                 continue  # already have complete tx
+            if self.adb.is_held_tx(tx_hash):
+                continue  # added once verified
             transaction_hashes.append(tx_hash)
             # note: tx_height might change by the time we get the raw_tx
             self.requested_tx.add(tx_hash)

### electrum/util.py
@@ -189,6 +189,11 @@ def __str__(self):
         return _("Transaction is unrelated to this wallet.")
 
 
+class InvalidTxSignature(AddTransactionException):
+    def __str__(self):
+        return _("Transaction has an invalid signature.")
+
+
 class FileImportFailed(Exception):
     def __init__(self, message=''):
         self.message = str(message)

### electrum/wallet.py
@@ -60,7 +60,9 @@
     OnchainHistoryItem, read_json_file, write_json_file, UserFacingException, FileImportFailed, EventListener,
     event_listener, is_hex_str,
 )
-from .bitcoin import COIN, is_address, is_minikey, relayfee, dust_threshold, DummyAddress, DummyAddressUsedInTxException
+from .bitcoin import (
+    COIN, is_address, is_minikey, relayfee, dust_threshold, DummyAddress, DummyAddressUsedInTxException, construct_witness,
+)
 from .keystore import (
     load_keystore, Hardware_KeyStore, KeyStore, KeyStoreWithMPK, AddressIndexGeneric, CannotDerivePubkey
 )
@@ -70,7 +72,8 @@
 from .storage import  WalletStorage
 from .wallet_db import WalletDB
 from .transaction import (
-    Transaction, TxInput, TxOutput, PartialTransaction, PartialTxInput, PartialTxOutput, TxOutpoint, Sighash
+    Transaction, TxInput, TxOutput, PartialTransaction, PartialTxInput, PartialTxOutput, TxOutpoint, Sighash,
+    script_GetOp, SighashCache,
 )
 from .plugin import run_hook
 from .address_synchronizer import (
@@ -83,7 +86,7 @@
 from .lnworker import LNWallet
 from .lnutil import MIN_FUNDING_SAT, RECEIVED, SENT
 from .lntransport import extract_nodeid
-from .descriptor import Descriptor
+from .descriptor import Descriptor, MissingSolutionPiece
 from .txbatcher import TxBatcher
 from .submarine_swaps import MIN_SWAP_AMOUNT_SAT
 
@@ -426,7 +429,8 @@ def __init__(self, db: WalletDB, *, config: SimpleConfig):
         Logger.__init__(self)
 
         self.network = None
-        self.adb = AddressSynchronizer(db, config, name=self.diagnostic_name())
+        self.adb = AddressSynchronizer(
+            db, config, verify_txins=self._verify_txin_signatures, name=self.diagnostic_name())
         for addr in self.get_addresses():
             self.adb.add_address(addr)
         self.lock = self.adb.lock
@@ -2605,6 +2609,52 @@ def can_rbf_tx(self, tx: Transaction, *, is_dscancel: bool = False) -> bool:
             return False
         return tx.is_rbf_enabled()
 
+    def _verify_txin_signatures(self, tx: Transaction, prev_txs: Mapping[int, Transaction]) -> bool:
+        if isinstance(tx, PartialTransaction):
+            tx = Transaction(tx.serialize())
+        ptx = PartialTransaction.from_tx(tx)
+        sighash_cache = SighashCache()
+        for txin_idx, prev_tx in prev_txs.items():
+            ptxin = ptx.inputs()[txin_idx]
+            prev_txout = prev_tx.outputs()[ptxin.prevout.out_idx]
+            ptxin.witness_utxo = prev_txout  # value and address, also if prev_tx is unsigned
+            ptxin.script_descriptor = self.get_script_descriptor_for_address(prev_txout.address)
+            if not ptxin.script_descriptor:
+                continue  # cannot verify, e.g. watch-only imported address
+            txin = tx.inputs()[txin_idx]
+            try:
+                items = txin.witness_elements() + [x[1] for x in script_GetOp(txin.script_sig or b'')]
+                sigs = [x for x in items if x and x[0] == 0x30 and 9 <= len(x) <= 73]
+                pubkeys = ptxin.pubkeys
+                if len(pubkeys) > 1:  # multisig: sigs are in script order
+                    script = ptxin.witness_script or ptxin.redeem_script
+                    pubkeys = [x[1] for x in script_GetOp(script) if x[1] in pubkeys]
+            except Exception:
+                return False
+            # like OP_CHECKMULTISIG: at most one verification per pubkey
+            sigs_iter = iter(sigs)
+            sig = next(sigs_iter, None)
+            for pubkey in pubkeys:
+                if sig is None:
+                    break
+                try:
+                    is_valid = ptx.verify_sig_for_txin(
+                        txin_index=txin_idx, pubkey_bytes=pubkey, sig=sig, sighash_cache=sighash_cache)
+                except Exception:
+                    is_valid = False
+                if is_valid:
+                    ptxin.sigs_ecdsa[pubkey] = sig
+                    sig = next(sigs_iter, None)
+            # the scriptSig and witness must be exactly what we would produce
+            try:
+                script_sig = Transaction.input_script(ptxin)
+                witness = Transaction.serialize_witness(ptxin)
+            except MissingSolutionPiece:
+                return False
+            if script_sig != (txin.script_sig or b'') or witness != (txin.witness or construct_witness([])):
+                return False
+        return True
+
     def cpfp(self, tx: Transaction, fee: int) -> Optional[PartialTransaction]:
         assert tx
         txid = tx.txid()

### tests/test_address_synchronizer.py
@@ -0,0 +1,214 @@
+from typing import Sequence, Tuple
+from unittest import mock
+
+from electrum import SimpleConfig
+from electrum.address_synchronizer import AddressSynchronizer, TX_HEIGHT_UNCONFIRMED
+from electrum.bitcoin import construct_witness, hash_to_segwit_addr
+from electrum.transaction import Transaction, PartialTransaction, PartialTxInput, PartialTxOutput, TxOutpoint
+from electrum.util import InvalidTxSignature, TxMinedInfo
+from electrum.synchronizer import Synchronizer
+from electrum.wallet_db import WalletDB
+
+from . import ElectrumTestCase
+
+
+ADDR_MINE = hash_to_segwit_addr(bytes(20), witver=0)
+ADDR_OTHER = hash_to_segwit_addr(bytes([1]) * 20, witver=0)
+
+
+def make_tx(prevouts: Sequence[str], outputs: Sequence[Tuple[str, int]], *, witness: bytes = b'good') -> Transaction:
+    """Returns a complete tx. Its txid does not depend on witness."""
+    txins = []
+    for prevout in prevouts:
+        txin = PartialTxInput(prevout=TxOutpoint.from_str(prevout))
+        txin.script_sig = b''
+        txin.witness = construct_witness([witness])
+        txins.append(txin)
+    txouts = [PartialTxOutput.from_address_and_value(addr, value) for addr, value in outputs]
+    return Transaction(PartialTransaction.from_io(txins, txouts, locktime=0).serialize_to_network())
+
+
+class TestAddressSynchronizerSignatures(ElectrumTestCase):
+
+    async def asyncSetUp(self):
+        await super().asyncSetUp()
+        config = SimpleConfig({'electrum_path': self.electrum_path})
+        self.verify_calls = []
+        self.adb = AddressSynchronizer(
+            WalletDB('', storage=None, upgrade=False), config, verify_txins=self._verify_txins)
+        self.adb.add_address(ADDR_MINE)
+        self.funding_tx = make_tx(['00' * 32 + ':0'], [(ADDR_MINE, 100_000)])
+        self.funding_outpoint = self.funding_tx.txid() + ':0'
+
+    def _verify_txins(self, tx, prev_txs) -> bool:
+        self.verify_calls.append((tx.txid(), sorted(prev_txs)))
+        return all(tx.inputs()[txin_idx].witness_elements() == [b'good'] for txin_idx in prev_txs)
+
+    def _receive(self, tx):
+        self.adb.receive_tx_callback(tx, tx_height=TX_HEIGHT_UNCONFIRMED)
+
+    async def test_invalid_tx_is_rejected(self):
+        self._receive(self.funding_tx)
+        tx = make_tx([self.funding_outpoint], [(ADDR_OTHER, 90_000)])
+        self._receive(tx)
+        conflicting_tx = make_tx([self.funding_outpoint], [(ADDR_OTHER, 80_000)], witness=b'bad')
+        with self.assertRaises(InvalidTxSignature):
+            self._receive(conflicting_tx)
+        self.assertIsNone(self.adb.db.get_transaction(conflicting_tx.txid()))
+        # the conflicting tx was not removed
+        self.assertIsNotNone(self.adb.db.get_transaction(tx.txid()))
+
+    async def test_invalid_tx_before_parent(self):
+        tx = make_tx([self.funding_outpoint], [(ADDR_OTHER, 90_000)], witness=b'bad')
+        self._receive(tx)
+        self.assertIsNotNone(self.adb.db.get_transaction(tx.txid()))
+        self._receive(self.funding_tx)
+        self.assertIsNone(self.adb.db.get_transaction(tx.txid()))
+
+    async def test_valid_tx_before_parent(self):
+        tx = make_tx([self.funding_outpoint], [(ADDR_OTHER, 90_000)])
+        self._receive(tx)
+        self._receive(self.funding_tx)
+        self.assertIsNotNone(self.adb.db.get_transaction(tx.txid()))
+        self.assertEqual([ADDR_MINE], self.adb.db.get_txi_addresses(tx.txid()))
+        self.assertEqual([(tx.txid(), [0])], self.verify_calls)
+
+    async def test_linked_inputs_are_checked_once(self):
+        self._receive(self.funding_tx)
+        tx = make_tx([self.funding_outpoint], [(ADDR_OTHER, 90_000)])
+        self._receive(tx)
+        self.adb.add_transaction(tx, allow_unrelated=True, is_new=False)
+        self.assertEqual([(tx.txid(), [0])], self.verify_calls)
+
+    async def test_add_parent_with_stale_spent_outpoint(self):
+        missing_txid = 'ab' * 32
+        self.adb.db.set_spent_outpoint(self.funding_tx.txid(), 0, missing_txid)
+        self.adb.db.add_txi_addr(missing_txid, ADDR_MINE, self.funding_outpoint, 100_000)
+        self._receive(self.funding_tx)
+
+    async def test_invalid_copy_of_stored_tx(self):
+        self._receive(self.funding_tx)
+        tx = make_tx([self.funding_outpoint], [(ADDR_OTHER, 90_000)])
+        self._receive(tx)
+        bad_copy = make_tx([self.funding_outpoint], [(ADDR_OTHER, 90_000)], witness=b'bad')
+        self.assertEqual(tx.txid(), bad_copy.txid())
+        self._receive(bad_copy)
+        self.assertEqual(tx.wtxid(), self.adb.db.get_transaction(tx.txid()).wtxid())
+        self.assertEqual([(tx.txid(), [0])], self.verify_calls)
+
+    async def test_invalid_copy_of_stored_unlinked_tx(self):
+        addr = hash_to_segwit_addr(bytes([2]) * 20, witver=0)
+        funding_tx = make_tx(['00' * 32 + ':1'], [(addr, 100_000)])
+        tx = make_tx([funding_tx.txid() + ':0'], [(ADDR_OTHER, 90_000)])
+        for t in (funding_tx, tx):
+            self._receive(t)  # not related to our addresses yet
+        self.adb.add_address(addr)
+        bad_copy = make_tx([funding_tx.txid() + ':0'], [(ADDR_OTHER, 90_000)], witness=b'bad')
+        self._receive(bad_copy)
+        self.assertEqual(tx.wtxid(), self.adb.db.get_transaction(tx.txid()).wtxid())
+
+    async def test_signed_copy_of_stored_unsigned_tx(self):
+        self._receive(self.funding_tx)
+        txin = PartialTxInput(prevout=TxOutpoint.from_str(self.funding_outpoint))
+        txin.witness_utxo = self.funding_tx.outputs()[0]
+        unsigned_tx = PartialTransaction.from_io(
+            [txin], [PartialTxOutput.from_address_and_value(ADDR_OTHER, 90_000)], locktime=0)
+        bad_copy = make_tx([self.funding_outpoint], [(ADDR_OTHER, 90_000)], witness=b'bad')
+        signed_tx = make_tx([self.funding_outpoint], [(ADDR_OTHER, 90_000)])
+        self.assertFalse(unsigned_tx.is_complete())
+        self.assertEqual(unsigned_tx.txid(), bad_copy.txid())
+        self.assertEqual(unsigned_tx.txid(), signed_tx.txid())
+        self.adb.add_transaction(unsigned_tx)
+        with self.assertRaises(InvalidTxSignature):
+            self._receive(bad_copy)
+        self.assertFalse(self.adb.db.get_transaction(unsigned_tx.txid()).is_complete())
+        self._receive(signed_tx)
+        self.assertEqual(signed_tx.wtxid(), self.adb.db.get_transaction(unsigned_tx.txid()).wtxid())
+
+    async def test_history_update_with_invalid_tx(self):
+        addr = hash_to_segwit_addr(bytes([2]) * 20, witver=0)
+        funding_tx = make_tx(['00' * 32 + ':1'], [(addr, 100_000)])
+        tx = make_tx([funding_tx.txid() + ':0'], [(ADDR_OTHER, 90_000)], witness=b'bad')
+        other_funding_tx = make_tx(['00' * 32 + ':2'], [(addr, 50_000)])
+        for t in (funding_tx, tx, other_funding_tx):
+            self._receive(t)  # not related to our addresses yet
+        self.adb.add_address(addr)
+        hist = [(t.txid(), TX_HEIGHT_UNCONFIRMED) for t in (tx, funding_tx, other_funding_tx)]
+        self.adb.receive_history_callback(addr, hist, {})
+        self.assertIsNone(self.adb.db.get_transaction(tx.txid()))
+        self.assertEqual([addr], self.adb.db.get_txo_addresses(funding_tx.txid()))
+        self.assertEqual([addr], self.adb.db.get_txo_addresses(other_funding_tx.txid()))
+
+    async def test_verified_tx_is_not_checked(self):
+        self._receive(self.funding_tx)
+        tx = make_tx([self.funding_outpoint], [(ADDR_OTHER, 90_000)], witness=b'bad')
+        self.adb.add_verified_tx(tx.txid(), TxMinedInfo(_height=100, timestamp=0, txpos=1, header_hash='00' * 32))
+        self.adb.receive_tx_callback(tx, tx_height=100)
+        self.assertIsNotNone(self.adb.db.get_transaction(tx.txid()))
+        self.assertEqual([], self.verify_calls)
+
+    async def test_tx_claimed_mined_is_added_once_verified(self):
+        self._receive(self.funding_tx)
+        tx = make_tx([self.funding_outpoint], [(ADDR_OTHER, 90_000)], witness=b'bad')
+        self.adb.add_unverified_or_unconfirmed_tx(tx.txid(), 100)
+        self.adb.receive_tx_callback(tx)
+        self.assertIsNone(self.adb.db.get_transaction(tx.txid()))
+        self.adb.add_verified_tx(tx.txid(), TxMinedInfo(_height=100, timestamp=0, txpos=1, header_hash='00' * 32))
+        self.assertIsNotNone(self.adb.db.get_transaction(tx.txid()))
+        self.assertEqual([], self.verify_calls)
+
+    async def test_tx_claimed_mined_is_checked_if_not_at_height(self):
+        self._receive(self.funding_tx)
+        tx = make_tx([self.funding_outpoint], [(ADDR_OTHER, 90_000)], witness=b'bad')
+        self.adb.add_unverified_or_unconfirmed_tx(tx.txid(), 100)
+        self.adb.receive_tx_callback(tx)
+        with self.assertRaises(InvalidTxSignature):
+            self.adb.remove_unverified_tx(tx.txid(), 100)
+        self.assertIsNone(self.adb.db.get_transaction(tx.txid()))
+        self.assertEqual([(tx.txid(), [0])], self.verify_calls)
+
+    async def test_tx_claimed_mined_then_unconfirmed_is_checked(self):
+        self._receive(self.funding_tx)
+        tx = make_tx([self.funding_outpoint], [(ADDR_OTHER, 90_000)], witness=b'bad')
+        self.adb.add_unverified_or_unconfirmed_tx(tx.txid(), 100)
+        self.adb.receive_tx_callback(tx)
+        self.adb.add_unverified_or_unconfirmed_tx(tx.txid(), TX_HEIGHT_UNCONFIRMED)
+        with self.assertRaises(InvalidTxSignature):
+            self.adb.receive_tx_callback(tx)
+        self.adb.add_verified_tx(tx.txid(), TxMinedInfo(_height=100, timestamp=0, txpos=1, header_hash='00' * 32))
+        self.assertIsNone(self.adb.db.get_transaction(tx.txid()))
+
+    def _make_synchronizer(self) -> Synchronizer:
+        sync = Synchronizer.__new__(Synchronizer)
+        sync.adb = self.adb
+        Synchronizer._reset(sync)
+        return sync
+
+    async def test_held_tx_is_not_requested_again(self):
+        tx = make_tx([self.funding_outpoint], [(ADDR_OTHER, 90_000)])
+        self.adb.add_unverified_or_unconfirmed_tx(tx.txid(), 100)
+        sync = self._make_synchronizer()
+        self.adb.receive_tx_callback(tx)
+        await sync._request_missing_txs([(tx.txid(), 100)])
+        self.assertEqual(set(), sync.requested_tx)
+
+    async def test_held_tx_is_requested_again_if_unconfirmed(self):
+        tx = make_tx([self.funding_outpoint], [(ADDR_OTHER, 90_000)])
+        self.adb.add_unverified_or_unconfirmed_tx(tx.txid(), 100)
+        sync = self._make_synchronizer()
+        sync._get_transaction = mock.AsyncMock()
+        self.adb.receive_tx_callback(tx)
+        self.adb.add_unverified_or_unconfirmed_tx(tx.txid(), TX_HEIGHT_UNCONFIRMED)
+        await sync._request_missing_txs([(tx.txid(), TX_HEIGHT_UNCONFIRMED)])
+        sync._get_transaction.assert_called_once_with(tx.txid(), allow_server_not_finding_tx=False)
+
+    async def test_held_txs_are_dropped_on_server_switch(self):
+        self._receive(self.funding_tx)
+        tx = make_tx([self.funding_outpoint], [(ADDR_OTHER, 90_000)])
+        self.adb.add_unverified_or_unconfirmed_tx(tx.txid(), 100)
+        sync = self._make_synchronizer()
+        self.adb.receive_tx_callback(tx)
+        Synchronizer._reset(sync)
+        self.assertFalse(self.adb.is_held_tx(tx.txid()))
+        self.adb.add_verified_tx(tx.txid(), TxMinedInfo(_height=100, timestamp=0, txpos=1, header_hash='00' * 32))
+        self.assertIsNone(self.adb.db.get_transaction(tx.txid()))

### tests/test_wallet_vertical.py
@@ -17,7 +17,9 @@
                              TransactionPotentiallyDangerousException,
                              TransactionDangerousException,
                              TxSighashRiskLevel, CannotDoubleSpendTx)
-from electrum.util import bfh, NotEnoughFunds, UnrelatedTransactionException, UserFacingException, TxMinedInfo
+from electrum.util import (
+    bfh, NotEnoughFunds, UnrelatedTransactionException, UserFacingException, TxMinedInfo, InvalidTxSignature,
+)
 from electrum.fee_policy import FixedFeePolicy
 from electrum.transaction import Transaction, PartialTxOutput, tx_from_any, Sighash
 from electrum.mnemonic import calc_seed_type
@@ -1075,6 +1077,15 @@ async def test_sending_between_p2wsh_2of3_and_p2wsh_p2sh_2of2(self):
         self.assertEqual(tx.wtxid(), tx_copy.wtxid())
         self.assertEqual(txid, tx_copy.txid())
 
+        # signatures must be in the order of pubkeys
+        items = tx_copy.inputs()[0].witness_elements()
+        swapped_items = [items[0], items[2], items[1], items[3]]
+        swapped_tx = Transaction(str(tx_copy).replace(
+            bitcoin.construct_witness(items).hex(), bitcoin.construct_witness(swapped_items).hex()))
+        self.assertEqual(txid, swapped_tx.txid())
+        with self.assertRaises(InvalidTxSignature):
+            wallet1a.adb.receive_tx_callback(swapped_tx, tx_height=TX_HEIGHT_UNCONFIRMED)
+
         wallet1a.adb.receive_tx_callback(tx, tx_height=TX_HEIGHT_UNCONFIRMED)
         wallet2a.adb.receive_tx_callback(tx, tx_height=TX_HEIGHT_UNCONFIRMED)
 
@@ -1503,7 +1514,7 @@ async def _bump_fee_p2wpkh_csv(self, *, config):
         self.assertEqual('dd0bf0d1563cd588b4c93cc1a9623c051ddb1c4f4581cf8ef43cfd27f031f246', funding_txid)
         wallet.adb.receive_tx_callback(funding_tx, tx_height=TX_HEIGHT_UNCONFIRMED)
 
-        orig_rbf_tx = Transaction("0200000000010146f231f027fd3cf48ecf81454f1cdb1d053c62a9c13cc9b488d53c56d1f00bdd01000000000100000002c8af000000000000160014999a95482213a896c72a251b6cc9f3d137b0a45850c3000000000000160014ea76d391236726af7d7a9c10abe600129154eb5a02473044022076d298537b524a926a8fadad0e9ded5868c8f4cf29246048f76f00eb4afa56310220739ad9e0417e97ce03fad98a454b4977972c2805cef37bfa822c6d6c56737c870121024196fb7b766ac987a08b69a5e108feae8513b7e72bc9e47899e27b36100f2af4d48a1d00")
+        orig_rbf_tx = Transaction("0200000000010146f231f027fd3cf48ecf81454f1cdb1d053c62a9c13cc9b488d53c56d1f00bdd01000000000100000002c8af000000000000160014999a95482213a896c72a251b6cc9f3d137b0a45850c3000000000000160014ea76d391236726af7d7a9c10abe600129154eb5a02473044022076886c7d024af73bb6535f75f0bfd5a99c62453573861bbf1912c1e6b115b92402206a913b883291690a1f6bb46834a9b8d5a5baf2d5acdfd6e28825458c3cef00bb0121024196fb7b766ac987a08b69a5e108feae8513b7e72bc9e47899e27b36100f2af4d48a1d00")
         orig_rbf_txid = orig_rbf_tx.txid()
         self.assertEqual('726f97590389fbef8570609eb2f8640464edb8ef44c30e48f4082b0191fa699c', orig_rbf_txid)
         self.assertEqual(orig_rbf_tx.get_block_based_relative_locktime(), 1)
@@ -2290,6 +2301,7 @@ async def test_rbf_batching__merge_duplicate_outputs(self):
                          str(tx1))
         wallet.adb.receive_tx_callback(tx1, tx_height=TX_HEIGHT_UNCONFIRMED)
         self.assertEqual((0, 298_000, 0), wallet.get_balance())
+        tx1_raw = str(tx1)  # make_unsigned_transaction mutates base_tx
 
         # second payment to dest_addr  (merged)
         outputs2 = [PartialTxOutput.from_address_and_value(dest_addr, 100_000)]
@@ -2306,7 +2318,7 @@ async def test_rbf_batching__merge_duplicate_outputs(self):
         self.assertEqual((0, 197_000, 0), wallet.get_balance())
 
         # remove tx2 from wallet, by replacing it with tx1
-        wallet.adb.receive_tx_callback(tx1, tx_height=TX_HEIGHT_UNCONFIRMED)
+        wallet.adb.receive_tx_callback(tx_from_any(tx1_raw), tx_height=TX_HEIGHT_UNCONFIRMED)
         self.assertEqual((0, 298_000, 0), wallet.get_balance())
 
         # second payment to dest_addr  (not merged, just duplicate outputs)
@@ -2383,6 +2395,73 @@ def fund_mywallet_from_faucet():
         self.assertEqual('02000000000102ab99cf51f3e219735c49491a9ecf354517a215a2b462227df7e7624188a7ff830000000000fdffffff417bbd802768510a0c641fc79af6e691a01fd10f37b84162f253b594e5cb87c50100000000fdffffff032c4c0000000000001600144e1b662f616fe134430054e29295ea6e5c18f17330750000000000001600142266c890fad71396f106319368107d5b2a1146fe307500000000000016001476efaaa243327bf3a2c0f5380cb3914099448cec02473044022033f8ddcb07ad7e06cef417208a5244507157cccdd6817029e968ec60a2395add02200720eb6a7c8cea86b470398ce75d69abcc66624a2253b18ea81cd1566eb5132c0121029b1a61d66896486ab893741b38dbafb9673b91a82237d6e4ca0da3cda7cbeb7c0247304402205e00156d74bcd85ed26ee9d0bdbd72890656881e25c04b2ac94f1c6b91f1176f02205f94d055e6385b48fbd3ac1a2dc2f57a640f4b33740cd9fb9e0fc20eb0cf5dcb012102c1ed648e71f15643950b444b864ab784b9d0e31e6ca6ec7d849d3dda4d98da0501093d00',
                          str(outgoing_tx2))
 
+    def _funded_wallet_setup(self):
+        wallet_faucet = self._create_cause_carbon_wallet()[0]
+        mywallet = self.create_standard_wallet_from_seed(
+            'response era cable net spike again observe dumb wage wonder sail tortoise',
+            config=self.config)
+        funding_tx = wallet_faucet.make_unsigned_transaction(
+            outputs=[PartialTxOutput.from_address_and_value(mywallet.get_receiving_address(), 40_000)],
+            fee_policy=FixedFeePolicy(200), locktime=4_000_000, rbf=True,
+        )
+        wallet_faucet.sign_transaction(funding_tx, password=None)
+        mywallet.adb.receive_tx_callback(funding_tx, tx_height=TX_HEIGHT_UNCONFIRMED)
+
+        def make_tx(addr) -> Transaction:
+            tx = mywallet.make_unsigned_transaction(
+                outputs=[PartialTxOutput.from_address_and_value(addr, 30_000)],
+                fee_policy=FixedFeePolicy(200), locktime=4_000_001, rbf=True,
+            )
+            mywallet.sign_transaction(tx, password=None)
+            return Transaction(tx.serialize())
+
+        external_addr = wallet_faucet.get_receiving_addresses()[0]
+        return mywallet, make_tx, external_addr
+
+    def _unsigned_parent_and_signed_child(self):
+        mywallet, make_tx, external_addr = self._funded_wallet_setup()
+        parent_tx = mywallet.make_unsigned_transaction(
+            outputs=[PartialTxOutput.from_address_and_value(external_addr, 30_000)],
+            fee_policy=FixedFeePolicy(200), locktime=4_000_001, rbf=True,
+        )
+        mywallet.adb.add_transaction(parent_tx)
+        coins = [c for c in mywallet.get_spendable_coins(domain=None, nonlocal_only=False, confirmed_only=False)
+                 if c.prevout.txid.hex() == parent_tx.txid()]
+        self.assertEqual(1, len(coins))
+        child_tx = mywallet.make_unsigned_transaction(
+            coins=coins, outputs=[PartialTxOutput.from_address_and_value(external_addr, '!')],
+            fee_policy=FixedFeePolicy(200), locktime=4_000_001, rbf=True,
+        )
+        mywallet.sign_transaction(child_tx, password=None)
+        self.assertFalse(parent_tx.is_complete())
+        self.assertTrue(child_tx.is_complete())
+        return mywallet, parent_tx, child_tx
+
+    async def test_signed_child_of_unsigned_parent(self):
+        mywallet, parent_tx, child_tx = self._unsigned_parent_and_signed_child()
+        mywallet.adb.add_transaction(child_tx)
+        self.assertIsNotNone(mywallet.db.get_transaction(child_tx.txid()))
+
+    async def test_signed_child_of_unsigned_parent_before_parent(self):
+        mywallet, parent_tx, child_tx = self._unsigned_parent_and_signed_child()
+        mywallet.adb.remove_transaction(parent_tx.txid())
+        mywallet.adb.add_transaction(child_tx)
+        mywallet.adb.add_transaction(parent_tx)
+        self.assertIsNotNone(mywallet.db.get_transaction(child_tx.txid()))
+        self.assertEqual(1, len(mywallet.db.get_txi_addresses(child_tx.txid())))
+
+    async def test_tx_with_huge_witness(self):
+        mywallet, make_tx, external_addr = self._funded_wallet_setup()
+        tx = make_tx(external_addr)
+        sig, pubkey = tx.inputs()[0].witness_elements()
+        tx.inputs()[0].witness = bitcoin.construct_witness([sig] * 1000 + [pubkey])
+        tx = Transaction(tx.serialize_to_network())
+        with mock.patch.object(Transaction, 'verify_sig_for_txin', autospec=True,
+                               side_effect=Transaction.verify_sig_for_txin) as verify_sig:
+            with self.assertRaises(InvalidTxSignature):
+                mywallet.adb.receive_tx_callback(tx, tx_height=TX_HEIGHT_UNCONFIRMED)
+        self.assertLessEqual(verify_sig.call_count, 1)
+
     async def test_join_psbts__merge_duplicate_outputs(self):
         """txos paying to the same address might be merged into a single output with a larger value"""
         rawtx1 = "70736274ff01007102000000019264597cffcce8f0c17b16a02adca7a95ae90f2ea51bd4b4df60c76dfe86686e0000000000fdffffff02400d03000000000016001458aee0f1201d1ae12dbd241209bbe92ed45e39b6108c0400000000001600144e1b662f616fe134430054e29295ea6e5c18f173c2ad26000001011f20a1070000000000160014542266519a44eb9b903761d40c6fe1055d33fa050100de02000000000101013548c9019890e27ce9e58766de05f18ea40ede70751fb6cd7a3a1715ece0a30100000000fdffffff0220a1070000000000160014542266519a44eb9b903761d40c6fe1055d33fa05485a080000000000160014bc69f7d82c403a9f35dfb6d1a4531d6b19cab0e3024730440220346b200f21c3024e1d51fb4ecddbdbd68bd24ae7b9dfd501519f6dcbeb7c052402200617e3ce7b0eb308e30caf23894fb0388b68fb1c15dd0681dd13ae5e735f148101210360d0c9ef15b8b6a16912d341ad218a4e4e4e07e9347f4a2dbc7ca8d974f8bc9ec1ad26002206029b1a61d66896486ab893741b38dbafb9673b91a82237d6e4ca0da3cda7cbeb7c101f1b48320000008000000000000000000000220203db4846ec1841f48484590e67fcd7d1039f124a04410c5794f38ec8625329ea23101f1b483200000080010000000000000000"