WalletDB: add more type-check asserts, adb: change adb_added_verified_tx evt API

electrum/address_synchronizer.py

@@ -651,7 +651,10 @@ def add_verified_tx(self, tx_hash: str, info: TxMinedInfo):
         with self.lock:
             self.unverified_tx.pop(tx_hash, None)
             self.db.add_verified_tx(tx_hash, info)
-        util.trigger_callback('adb_added_verified_tx', self, tx_hash)
+            del info
+            info2 = self.get_tx_height(tx_hash)  # populates the 'conf' field
+            assert isinstance(info2.conf, int) and info2.conf > 0, f"{info2.conf=}, {info2.height=}, {self.get_local_height()=}"
+        util.trigger_callback('adb_added_verified_tx', self, tx_hash, info2)
 
     @with_lock
     def get_unverified_txs(self) -> Dict[str, int]:

electrum/gui/qml/qetransactionlistmodel.py

@@ -47,7 +47,7 @@ def on_destroy(self):
         self.unregister_callbacks()
 
     @qt_event_listener
-    def on_event_verified(self, wallet, txid, info):
+    def on_event_verified(self, wallet, txid: str, info: TxMinedInfo):
         if wallet == self.wallet:
             self._logger.debug('verified event for txid %s' % txid)
             self.on_tx_verified(txid, info)
@@ -228,7 +228,7 @@ def initModel(self, force: bool = False):
 
         self._dirty = False
 
-    def on_tx_verified(self, txid, info):
+    def on_tx_verified(self, txid: str, info: TxMinedInfo):
         for i, tx in enumerate(self.tx_history):
             if 'txid' in tx and tx['txid'] == txid:
                 tx['height'] = info.height

electrum/lnwatcher.py

@@ -72,7 +72,7 @@ async def on_event_adb_added_tx(self, adb, tx_hash, tx):
         await self.trigger_callbacks()
 
     @event_listener
-    async def on_event_adb_added_verified_tx(self, adb, tx_hash):
+    async def on_event_adb_added_verified_tx(self, adb, tx_hash, info: TxMinedInfo):
         if adb != self.adb:
             return
         await self.trigger_callbacks()

electrum/plugins/watchtower/watchtower.py

@@ -38,7 +38,7 @@
 from electrum.wallet_db import WalletDB
 from electrum.lnutil import WITNESS_TEMPLATE_RECEIVED_HTLC, WITNESS_TEMPLATE_OFFERED_HTLC
 from electrum.logging import Logger
-from electrum.util import EventListener, event_listener
+from electrum.util import EventListener, event_listener, TxMinedInfo
 
 from .server import WatchTowerServer
 
@@ -96,7 +96,7 @@ async def on_event_wallet_updated(self, wallet):
         await self.trigger_callbacks()
 
     @event_listener
-    async def on_event_adb_added_verified_tx(self, adb, tx_hash):
+    async def on_event_adb_added_verified_tx(self, adb, tx_hash, info: TxMinedInfo):
         if adb != self.adb:
             return
         await self.trigger_callbacks()

electrum/wallet.py

@@ -633,12 +633,11 @@ def on_event_adb_removed_tx(self, adb, txid: str, tx: Transaction):
         util.trigger_callback('removed_transaction', self, tx)
 
     @event_listener
-    def on_event_adb_added_verified_tx(self, adb, tx_hash):
+    def on_event_adb_added_verified_tx(self, adb, tx_hash, info: TxMinedInfo):
         if adb != self.adb:
             return
         self._update_invoices_and_reqs_touched_by_tx(tx_hash)
-        tx_mined_status = self.adb.get_tx_height(tx_hash)
-        util.trigger_callback('verified', self, tx_hash, tx_mined_status)
+        util.trigger_callback('verified', self, tx_hash, info)
 
     @event_listener
     def on_event_adb_removed_verified_tx(self, adb, tx_hash):

electrum/wallet_db.py

@@ -38,7 +38,7 @@
 import attr
 
 from . import util, bitcoin
-from .util import profiler, WalletFileException, multisig_type, TxMinedInfo, bfh, MyEncoder
+from .util import profiler, WalletFileException, multisig_type, TxMinedInfo, bfh, MyEncoder, is_hash256_str
 from .invoices import Invoice, Request
 from .keystore import bip44_derivation
 from .transaction import Transaction, TxOutpoint, tx_from_any, PartialTransaction, PartialTxOutput, BadHeaderMagic
@@ -1312,34 +1312,34 @@ def get_db_metadata(self) -> Optional[DBMetadata]:
     @locked
     def get_txi_addresses(self, tx_hash: str) -> List[str]:
         """Returns list of is_mine addresses that appear as inputs in tx."""
-        assert isinstance(tx_hash, str)
+        assert is_hash256_str(tx_hash), tx_hash
         return list(self.txi.get(tx_hash, {}).keys())
 
     @locked
     def get_txo_addresses(self, tx_hash: str) -> List[str]:
         """Returns list of is_mine addresses that appear as outputs in tx."""
-        assert isinstance(tx_hash, str)
+        assert is_hash256_str(tx_hash), tx_hash
         return list(self.txo.get(tx_hash, {}).keys())
 
     @locked
     def get_txi_addr(self, tx_hash: str, address: str) -> Iterable[Tuple[str, int]]:
         """Returns an iterable of (prev_outpoint, value)."""
-        assert isinstance(tx_hash, str)
+        assert is_hash256_str(tx_hash), tx_hash
         assert isinstance(address, str)
         d = self.txi.get(tx_hash, {}).get(address, {})
         return list(d.items())
 
     @locked
     def get_txo_addr(self, tx_hash: str, address: str) -> Dict[int, Tuple[int, bool]]:
         """Returns a dict: output_index -> (value, is_coinbase)."""
-        assert isinstance(tx_hash, str)
+        assert is_hash256_str(tx_hash), tx_hash
         assert isinstance(address, str)
         d = self.txo.get(tx_hash, {}).get(address, {})
         return {int(n): (v, cb) for (n, (v, cb)) in d.items()}
 
     @modifier
     def add_txi_addr(self, tx_hash: str, addr: str, ser: str, v: int) -> None:
-        assert isinstance(tx_hash, str)
+        assert is_hash256_str(tx_hash), tx_hash
         assert isinstance(addr, str)
         assert isinstance(ser, str)
         assert isinstance(v, int)
@@ -1353,7 +1353,7 @@ def add_txi_addr(self, tx_hash: str, addr: str, ser: str, v: int) -> None:
     @modifier
     def add_txo_addr(self, tx_hash: str, addr: str, n: Union[int, str], v: int, is_coinbase: bool) -> None:
         n = str(n)
-        assert isinstance(tx_hash, str)
+        assert is_hash256_str(tx_hash), tx_hash
         assert isinstance(addr, str)
         assert isinstance(n, str)
         assert isinstance(v, int)
@@ -1375,12 +1375,12 @@ def list_txo(self) -> Sequence[str]:
 
     @modifier
     def remove_txi(self, tx_hash: str) -> None:
-        assert isinstance(tx_hash, str)
+        assert is_hash256_str(tx_hash), tx_hash
         self.txi.pop(tx_hash, None)
 
     @modifier
     def remove_txo(self, tx_hash: str) -> None:
-        assert isinstance(tx_hash, str)
+        assert is_hash256_str(tx_hash), tx_hash
         self.txo.pop(tx_hash, None)
 
     @locked
@@ -1392,35 +1392,35 @@ def list_spent_outpoints(self) -> Sequence[Tuple[str, str]]:
 
     @locked
     def get_spent_outpoints(self, prevout_hash: str) -> Sequence[str]:
-        assert isinstance(prevout_hash, str)
+        assert is_hash256_str(prevout_hash), prevout_hash
         return list(self.spent_outpoints.get(prevout_hash, {}).keys())
 
     @locked
     def get_spent_outpoint(self, prevout_hash: str, prevout_n: Union[int, str]) -> Optional[str]:
-        assert isinstance(prevout_hash, str)
+        assert is_hash256_str(prevout_hash), prevout_hash
         prevout_n = str(prevout_n)
         return self.spent_outpoints.get(prevout_hash, {}).get(prevout_n)
 
     @modifier
     def remove_spent_outpoint(self, prevout_hash: str, prevout_n: Union[int, str]) -> None:
-        assert isinstance(prevout_hash, str)
+        assert is_hash256_str(prevout_hash), prevout_hash
         prevout_n = str(prevout_n)
         self.spent_outpoints[prevout_hash].pop(prevout_n, None)
         if not self.spent_outpoints[prevout_hash]:
             self.spent_outpoints.pop(prevout_hash)
 
     @modifier
     def set_spent_outpoint(self, prevout_hash: str, prevout_n: Union[int, str], tx_hash: str) -> None:
-        assert isinstance(prevout_hash, str)
-        assert isinstance(tx_hash, str)
+        assert is_hash256_str(prevout_hash), prevout_hash
+        assert is_hash256_str(tx_hash), tx_hash
         prevout_n = str(prevout_n)
         if prevout_hash not in self.spent_outpoints:
             self.spent_outpoints[prevout_hash] = {}
         self.spent_outpoints[prevout_hash][prevout_n] = tx_hash
 
     @modifier
     def add_prevout_by_scripthash(self, scripthash: str, *, prevout: TxOutpoint, value: int) -> None:
-        assert isinstance(scripthash, str)
+        assert is_hash256_str(scripthash)
         assert isinstance(prevout, TxOutpoint)
         assert isinstance(value, int)
         if scripthash not in self._prevouts_by_scripthash:
@@ -1429,7 +1429,7 @@ def add_prevout_by_scripthash(self, scripthash: str, *, prevout: TxOutpoint, val
 
     @modifier
     def remove_prevout_by_scripthash(self, scripthash: str, *, prevout: TxOutpoint, value: int) -> None:
-        assert isinstance(scripthash, str)
+        assert is_hash256_str(scripthash)
         assert isinstance(prevout, TxOutpoint)
         assert isinstance(value, int)
         self._prevouts_by_scripthash[scripthash].pop(prevout.to_str(), None)
@@ -1438,13 +1438,13 @@ def remove_prevout_by_scripthash(self, scripthash: str, *, prevout: TxOutpoint,
 
     @locked
     def get_prevouts_by_scripthash(self, scripthash: str) -> Set[Tuple[TxOutpoint, int]]:
-        assert isinstance(scripthash, str)
+        assert is_hash256_str(scripthash)
         prevouts_and_values = self._prevouts_by_scripthash.get(scripthash, {})
         return {(TxOutpoint.from_str(prevout), value) for prevout, value in prevouts_and_values.items()}
 
     @modifier
     def add_transaction(self, tx_hash: str, tx: Transaction) -> None:
-        assert isinstance(tx_hash, str)
+        assert is_hash256_str(tx_hash), tx_hash
         assert isinstance(tx, Transaction), tx
         # note that tx might be a PartialTransaction
         # serialize and de-serialize tx now. this might e.g. convert a complete PartialTx to a Tx
@@ -1460,14 +1460,14 @@ def add_transaction(self, tx_hash: str, tx: Transaction) -> None:
 
     @modifier
     def remove_transaction(self, tx_hash: str) -> Optional[Transaction]:
-        assert isinstance(tx_hash, str)
+        assert is_hash256_str(tx_hash), tx_hash
         return self.transactions.pop(tx_hash, None)
 
     @locked
     def get_transaction(self, tx_hash: Optional[str]) -> Optional[Transaction]:
         if tx_hash is None:
             return None
-        assert isinstance(tx_hash, str)
+        assert is_hash256_str(tx_hash), tx_hash
         return self.transactions.get(tx_hash)
 
     @locked
@@ -1504,10 +1504,14 @@ def list_verified_tx(self) -> Sequence[str]:
 
     @locked
     def get_verified_tx(self, txid: str) -> Optional[TxMinedInfo]:
-        assert isinstance(txid, str)
+        assert is_hash256_str(txid), txid
         if txid not in self.verified_tx:
             return None
         height, timestamp, txpos, header_hash = self.verified_tx[txid]
+        assert isinstance(height, int) and height > 0, height
+        assert isinstance(timestamp, int), timestamp
+        assert isinstance(txpos, int) and txpos >= 0, txpos
+        assert is_hash256_str(header_hash), header_hash
         return TxMinedInfo(height=height,
                            conf=None,
                            timestamp=timestamp,
@@ -1516,22 +1520,26 @@ def get_verified_tx(self, txid: str) -> Optional[TxMinedInfo]:
 
     @modifier
     def add_verified_tx(self, txid: str, info: TxMinedInfo):
-        assert isinstance(txid, str)
+        assert is_hash256_str(txid), txid
         assert isinstance(info, TxMinedInfo)
+        assert isinstance(info.height, int) and info.height > 0, info.height
+        assert isinstance(info.timestamp, int), info.timestamp
+        assert isinstance(info.txpos, int) and info.txpos >= 0, info.txpos
+        assert is_hash256_str(info.header_hash), info.header_hash
         self.verified_tx[txid] = (info.height, info.timestamp, info.txpos, info.header_hash)
 
     @modifier
     def remove_verified_tx(self, txid: str):
-        assert isinstance(txid, str)
+        assert is_hash256_str(txid), txid
         self.verified_tx.pop(txid, None)
 
     def is_in_verified_tx(self, txid: str) -> bool:
-        assert isinstance(txid, str)
+        assert is_hash256_str(txid), txid
         return txid in self.verified_tx
 
     @modifier
     def add_tx_fee_from_server(self, txid: str, fee_sat: Optional[int]) -> None:
-        assert isinstance(txid, str)
+        assert is_hash256_str(txid), txid
         # note: when called with (fee_sat is None), rm currently saved value
         if txid not in self.tx_fees:
             self.tx_fees[txid] = TxFeesValue()
@@ -1542,7 +1550,7 @@ def add_tx_fee_from_server(self, txid: str, fee_sat: Optional[int]) -> None:
 
     @modifier
     def add_tx_fee_we_calculated(self, txid: str, fee_sat: Optional[int]) -> None:
-        assert isinstance(txid, str)
+        assert is_hash256_str(txid), txid
         if fee_sat is None:
             return
         assert isinstance(fee_sat, int)
@@ -1552,7 +1560,7 @@ def add_tx_fee_we_calculated(self, txid: str, fee_sat: Optional[int]) -> None:
 
     @locked
     def get_tx_fee(self, txid: str, *, trust_server: bool = False) -> Optional[int]:
-        assert isinstance(txid, str)
+        assert is_hash256_str(txid), txid
         """Returns tx_fee."""
         tx_fees_value = self.tx_fees.get(txid)
         if tx_fees_value is None:
@@ -1563,29 +1571,29 @@ def get_tx_fee(self, txid: str, *, trust_server: bool = False) -> Optional[int]:
 
     @modifier
     def add_num_inputs_to_tx(self, txid: str, num_inputs: int) -> None:
-        assert isinstance(txid, str)
+        assert is_hash256_str(txid), txid
         assert isinstance(num_inputs, int)
         if txid not in self.tx_fees:
             self.tx_fees[txid] = TxFeesValue()
         self.tx_fees[txid] = self.tx_fees[txid]._replace(num_inputs=num_inputs)
 
     @locked
     def get_num_all_inputs_of_tx(self, txid: str) -> Optional[int]:
-        assert isinstance(txid, str)
+        assert is_hash256_str(txid), txid
         tx_fees_value = self.tx_fees.get(txid)
         if tx_fees_value is None:
             return None
         return tx_fees_value.num_inputs
 
     @locked
     def get_num_ismine_inputs_of_tx(self, txid: str) -> int:
-        assert isinstance(txid, str)
+        assert is_hash256_str(txid), txid
         txins = self.txi.get(txid, {})
         return sum([len(tupls) for addr, tupls in txins.items()])
 
     @modifier
     def remove_tx_fee(self, txid: str) -> None:
-        assert isinstance(txid, str)
+        assert is_hash256_str(txid), txid
         self.tx_fees.pop(txid, None)
 
     @locked

tests/test_txbatcher.py

@@ -7,6 +7,7 @@
 from electrum import storage, bitcoin, keystore, wallet
 from electrum import SimpleConfig
 from electrum import util
+from electrum.util import TxMinedInfo
 from electrum.address_synchronizer import TX_HEIGHT_UNCONFIRMED, TX_HEIGHT_UNCONF_PARENT, TX_HEIGHT_LOCAL
 from electrum.transaction import Transaction, PartialTxInput, PartialTxOutput, TxOutpoint
 from electrum.logging import console_stderr_handler, Logger
@@ -151,8 +152,10 @@ async def test_batch_payments(self, mock_save_db):
         assert output2 in tx1_prime.outputs()
         # tx1 gets confirmed, tx2 gets removed
         wallet.adb.receive_tx_callback(tx1, tx_height=1)
-        tx_mined_status = wallet.adb.get_tx_height(tx1.txid())
-        wallet.adb.add_verified_tx(tx1.txid(), tx_mined_status._replace(conf=1))
+        wallet.adb.add_verified_tx(
+            tx1.txid(),
+            TxMinedInfo(height=1, timestamp=999999, txpos=0, header_hash="aa"*32),
+        )
         assert wallet.adb.get_transaction(tx1.txid()) is not None
         assert wallet.adb.get_transaction(tx1_prime.txid()) is None
         # txbatcher creates tx2
@@ -194,8 +197,10 @@ async def test_rbf_batching__cannot_batch_as_would_need_to_use_ismine_outputs_of
 
         # tx1 gets confirmed
         wallet.adb.receive_tx_callback(tx1, tx_height=1)
-        tx_mined_status = wallet.adb.get_tx_height(tx1.txid())
-        wallet.adb.add_verified_tx(tx1.txid(), tx_mined_status._replace(conf=1))
+        wallet.adb.add_verified_tx(
+            tx1.txid(),
+            TxMinedInfo(height=1, timestamp=999999, txpos=0, header_hash="aa"*32),
+        )
 
         tx2 = await self.network.next_tx()
         assert len(tx2.outputs()) == 2