wallet: organise get_tx_fee. store calculated fees. storage version 19.

electrum/address_synchronizer.py

@@ -213,7 +213,8 @@ class AddressSynchronizer(Logger):
                     conflicting_txns -= {tx_hash}
             return conflicting_txns
 
-    def add_transaction(self, tx_hash, tx, allow_unrelated=False):
+    def add_transaction(self, tx_hash, tx, allow_unrelated=False) -> bool:
+        """Returns whether the tx was successfully added to the wallet history."""
         assert tx_hash, tx_hash
         assert tx, tx
         assert tx.is_complete()
@@ -300,6 +301,7 @@ class AddressSynchronizer(Logger):
             self._add_tx_to_local_history(tx_hash)
             # save
             self.db.add_transaction(tx_hash, tx)
+            self.db.add_num_inputs_to_tx(tx_hash, len(tx.inputs()))
             return True
 
     def remove_transaction(self, tx_hash):
@@ -329,6 +331,7 @@ class AddressSynchronizer(Logger):
                 self._get_addr_balance_cache.pop(addr, None)  # invalidate cache
             self.db.remove_txi(tx_hash)
             self.db.remove_txo(tx_hash)
+            self.db.remove_tx_fee(tx_hash)
 
     def get_depending_transactions(self, tx_hash):
         """Returns all (grand-)children of tx_hash in this wallet."""
@@ -344,7 +347,7 @@ class AddressSynchronizer(Logger):
         self.add_unverified_tx(tx_hash, tx_height)
         self.add_transaction(tx_hash, tx, allow_unrelated=True)
 
-    def receive_history_callback(self, addr, hist, tx_fees):
+    def receive_history_callback(self, addr: str, hist, tx_fees: Dict[str, int]):
         with self.lock:
             old_hist = self.get_address_history(addr)
             for tx_hash, height in old_hist:
@@ -366,7 +369,8 @@ class AddressSynchronizer(Logger):
             self.add_transaction(tx_hash, tx, allow_unrelated=True)
 
         # Store fees
-        self.db.update_tx_fees(tx_fees)
+        for tx_hash, fee_sat in tx_fees.items():
+            self.db.add_tx_fee_from_server(tx_hash, fee_sat)
 
     @profiler
     def load_local_history(self):
@@ -447,8 +451,7 @@ class AddressSynchronizer(Logger):
         for tx_hash in tx_deltas:
             delta = tx_deltas[tx_hash]
             tx_mined_status = self.get_tx_height(tx_hash)
-            # FIXME: db should only store fees computed by us...
+            fee, is_calculated_by_us = self.get_tx_fee(tx_hash)
-            fee = self.db.get_tx_fee(tx_hash)
             history.append((tx_hash, tx_mined_status, delta, fee))
         history.sort(key = lambda x: self.get_txpos(x[0]), reverse=True)
         # 3. add balance
@@ -468,7 +471,7 @@ class AddressSynchronizer(Logger):
         h2.reverse()
         # fixme: this may happen if history is incomplete
         if balance not in [None, 0]:
-            self.logger.info("Error: history not synchronized")
+            self.logger.warning("history not synchronized")
             return []
 
         return h2
@@ -686,20 +689,39 @@ class AddressSynchronizer(Logger):
             fee = None
         return is_relevant, is_mine, v, fee
 
-    def get_tx_fee(self, tx: Transaction) -> Optional[int]:
+    def get_tx_fee(self, txid: str) -> Tuple[Optional[int], bool]:
+        """Returns (tx_fee, is_calculated_by_us)."""
+        # check if stored fee is available
+        # return that, if is_calc_by_us
+        fee = None
+        fee_and_bool = self.db.get_tx_fee(txid)
+        if fee_and_bool is not None:
+            fee, is_calc_by_us = fee_and_bool
+            if is_calc_by_us:
+                return fee, is_calc_by_us
+            elif self.get_tx_height(txid).conf > 0:
+                # delete server-sent fee for confirmed txns
+                self.db.add_tx_fee_from_server(txid, None)
+                fee = None
+        # if all inputs are ismine, try to calc fee now;
+        # otherwise, return stored value
+        num_all_inputs = self.db.get_num_all_inputs_of_tx(txid)
+        if num_all_inputs is not None:
+            num_ismine_inputs = self.db.get_num_ismine_inputs_of_tx(txid)
+            assert num_ismine_inputs <= num_all_inputs, (num_ismine_inputs, num_all_inputs)
+            if num_ismine_inputs < num_all_inputs:
+                return fee, False
+        # lookup tx and deserialize it.
+        # note that deserializing is expensive, hence above hacks
+        tx = self.db.get_transaction(txid)
         if not tx:
-            return None
+            return None, False
-        if hasattr(tx, '_cached_fee'):
-            return tx._cached_fee
         with self.lock, self.transaction_lock:
             is_relevant, is_mine, v, fee = self.get_wallet_delta(tx)
-            if fee is None:
+        # save result
-                txid = tx.txid()
+        self.db.add_tx_fee_we_calculated(txid, fee)
-                fee = self.db.get_tx_fee(txid)
+        self.db.add_num_inputs_to_tx(txid, len(tx.inputs()))
-            # only cache non-None, as None can still change while syncing
+        return fee, True
-            if fee is not None:
-                tx._cached_fee = fee
-        return fee
 
     def get_addr_io(self, address):
         with self.lock, self.transaction_lock:

electrum/gui/qt/main_window.py

@@ -3001,9 +3001,11 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger):
         vbox.addLayout(Buttons(CloseButton(d)))
         d.exec_()
 
-    def cpfp(self, parent_tx, new_tx):
+    def cpfp(self, parent_tx: Transaction, new_tx: Transaction) -> None:
         total_size = parent_tx.estimated_size() + new_tx.estimated_size()
-        parent_fee = self.wallet.get_tx_fee(parent_tx)
+        parent_txid = parent_tx.txid()
+        assert parent_txid
+        parent_fee, _calc_by_us = self.wallet.get_tx_fee(parent_txid)
         if parent_fee is None:
             self.show_error(_("Can't CPFP: unknown fee for parent transaction."))
             return
@@ -3079,12 +3081,14 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger):
         new_tx.set_rbf(True)
         self.show_transaction(new_tx)
 
-    def bump_fee_dialog(self, tx):
+    def bump_fee_dialog(self, tx: Transaction):
-        fee = self.wallet.get_tx_fee(tx)
+        txid = tx.txid()
-        if fee is None:
+        assert txid
+        fee, is_calc_by_us = self.wallet.get_tx_fee(txid)
+        if fee is None or not is_calc_by_us:
             self.show_error(_("Can't bump fee: unknown fee for original transaction."))
             return
-        tx_label = self.wallet.get_label(tx.txid())
+        tx_label = self.wallet.get_label(txid)
         tx_size = tx.estimated_size()
         old_fee_rate = fee / tx_size  # sat/vbyte
         d = WindowModalDialog(self, _('Bump Fee'))

electrum/json_db.py

@@ -28,7 +28,7 @@ import json
 import copy
 import threading
 from collections import defaultdict
-from typing import Dict, Optional, List, Tuple, Set, Iterable
+from typing import Dict, Optional, List, Tuple, Set, Iterable, NamedTuple
 
 from . import util, bitcoin
 from .util import profiler, WalletFileException, multisig_type, TxMinedInfo
@@ -40,7 +40,7 @@ from .logging import Logger
 
 OLD_SEED_VERSION = 4        # electrum versions < 2.0
 NEW_SEED_VERSION = 11       # electrum versions >= 2.0
-FINAL_SEED_VERSION = 18     # electrum >= 2.7 will set this to prevent
+FINAL_SEED_VERSION = 19     # electrum >= 2.7 will set this to prevent
                             # old versions from overwriting new format
 
 
@@ -51,6 +51,12 @@ class JsonDBJsonEncoder(util.MyEncoder):
         return super().default(obj)
 
 
+class TxFeesValue(NamedTuple):
+    fee: Optional[int] = None
+    is_calculated_by_us: bool = False
+    num_inputs: Optional[int] = None
+
+
 class JsonDB(Logger):
 
     def __init__(self, raw, *, manual_upgrades):
@@ -210,6 +216,7 @@ class JsonDB(Logger):
         self._convert_version_16()
         self._convert_version_17()
         self._convert_version_18()
+        self._convert_version_19()
         self.put('seed_version', FINAL_SEED_VERSION)  # just to be sure
 
         self._after_upgrade_tasks()
@@ -434,7 +441,14 @@ class JsonDB(Logger):
         self.put('verified_tx3', None)
         self.put('seed_version', 18)
 
-    # def _convert_version_19(self):
+    def _convert_version_19(self):
+        # delete tx_fees as its structure changed
+        if not self._is_upgrade_method_needed(18, 18):
+            return
+        self.put('tx_fees', None)
+        self.put('seed_version', 19)
+
+    # def _convert_version_20(self):
     #     TODO for "next" upgrade:
     #       - move "pw_hash_version" from keystore to storage
     #     pass
@@ -667,12 +681,48 @@ class JsonDB(Logger):
         return txid in self.verified_tx
 
     @modifier
-    def update_tx_fees(self, d):
+    def add_tx_fee_from_server(self, txid: str, fee_sat: Optional[int]) -> None:
-        return self.tx_fees.update(d)
+        # note: when called with (fee_sat is None), rm currently saved value
+        if txid not in self.tx_fees:
+            self.tx_fees[txid] = TxFeesValue()
+        tx_fees_value = self.tx_fees[txid]
+        if tx_fees_value.is_calculated_by_us:
+            return
+        self.tx_fees[txid] = tx_fees_value._replace(fee=fee_sat, is_calculated_by_us=False)
+
+    @modifier
+    def add_tx_fee_we_calculated(self, txid: str, fee_sat: Optional[int]) -> None:
+        if fee_sat is None:
+            return
+        if txid not in self.tx_fees:
+            self.tx_fees[txid] = TxFeesValue()
+        self.tx_fees[txid] = self.tx_fees[txid]._replace(fee=fee_sat, is_calculated_by_us=True)
+
+    @locked
+    def get_tx_fee(self, txid: str) -> Optional[Tuple[Optional[int], bool]]:
+        """Returns (tx_fee, is_calculated_by_us)."""
+        tx_fees_value = self.tx_fees.get(txid)
+        if tx_fees_value is None:
+            return None
+        return tx_fees_value.fee, tx_fees_value.is_calculated_by_us
+
+    @modifier
+    def add_num_inputs_to_tx(self, txid: str, num_inputs: int) -> None:
+        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]:
+        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_tx_fee(self, txid):
+    def get_num_ismine_inputs_of_tx(self, txid: str) -> int:
-        return self.tx_fees.get(txid)
+        txins = self.txi.get(txid, {})
+        return sum([len(tupls) for addr, tupls in txins.items()])
 
     @modifier
     def remove_tx_fee(self, txid):
@@ -764,10 +814,10 @@ class JsonDB(Logger):
         # txid -> address -> set of (output_index, value, is_coinbase)
         self.txo = self.get_data_ref('txo')  # type: Dict[str, Dict[str, Set[Tuple[int, int, bool]]]]
         self.transactions = self.get_data_ref('transactions')   # type: Dict[str, Transaction]
-        self.spent_outpoints = self.get_data_ref('spent_outpoints')
+        self.spent_outpoints = self.get_data_ref('spent_outpoints')  # txid -> output_index -> next_txid
         self.history = self.get_data_ref('addr_history')  # address -> list of (txid, height)
         self.verified_tx = self.get_data_ref('verified_tx3')  # txid -> (height, timestamp, txpos, header_hash)
-        self.tx_fees = self.get_data_ref('tx_fees')
+        self.tx_fees = self.get_data_ref('tx_fees')  # type: Dict[str, TxFeesValue]
         # convert raw hex transactions to Transaction objects
         for tx_hash, raw_tx in self.transactions.items():
             self.transactions[tx_hash] = Transaction(raw_tx)
@@ -788,6 +838,9 @@ class JsonDB(Logger):
                 if spending_txid not in self.transactions:
                     self.logger.info("removing unreferenced spent outpoint")
                     d.pop(prevout_n)
+        # convert tx_fees tuples to NamedTuples
+        for tx_hash, tuple_ in self.tx_fees.items():
+            self.tx_fees[tx_hash] = TxFeesValue(*tuple_)
 
     @modifier
     def clear_history(self):

electrum/wallet.py

@@ -409,7 +409,7 @@ class Abstract_Wallet(AddressSynchronizer):
                 elif tx_mined_status.height in (TX_HEIGHT_UNCONF_PARENT, TX_HEIGHT_UNCONFIRMED):
                     status = _('Unconfirmed')
                     if fee is None:
-                        fee = self.db.get_tx_fee(tx_hash)
+                        fee, _calc_by_us = self.get_tx_fee(tx_hash)
                     if fee and self.network and self.config.has_fee_mempool():
                         size = tx.estimated_size()
                         fee_per_byte = fee / size
@@ -722,9 +722,7 @@ class Abstract_Wallet(AddressSynchronizer):
             is_final = tx and tx.is_final()
             if not is_final:
                 extra.append('rbf')
-            fee = self.get_wallet_delta(tx)[3]
+            fee, _calc_by_us = self.get_tx_fee(tx_hash)
-            if fee is None:
-                fee = self.db.get_tx_fee(tx_hash)
             if fee is not None:
                 size = tx.estimated_size()
                 fee_per_byte = fee / size
@@ -996,7 +994,7 @@ class Abstract_Wallet(AddressSynchronizer):
             max_conf = max(max_conf, tx_age)
         return max_conf >= req_conf
 
-    def bump_fee(self, *, tx, new_fee_rate) -> Transaction:
+    def bump_fee(self, *, tx: Transaction, new_fee_rate) -> Transaction:
         """Increase the miner fee of 'tx'.
         'new_fee_rate' is the target min rate in sat/vbyte
         """
@@ -1004,8 +1002,10 @@ class Abstract_Wallet(AddressSynchronizer):
             raise CannotBumpFee(_('Cannot bump fee') + ': ' + _('transaction is final'))
         new_fee_rate = quantize_feerate(new_fee_rate)  # strip excess precision
         old_tx_size = tx.estimated_size()
-        old_fee = self.get_tx_fee(tx)
+        old_txid = tx.txid()
-        if old_fee is None:
+        assert old_txid
+        old_fee, is_calc_by_us = self.get_tx_fee(old_txid)
+        if old_fee is None or not is_calc_by_us:
             raise CannotBumpFee(_('Cannot bump fee') + ': ' + _('current fee unknown'))
         old_fee_rate = old_fee / old_tx_size  # sat/vbyte
         if new_fee_rate <= old_fee_rate:
@@ -1036,7 +1036,7 @@ class Abstract_Wallet(AddressSynchronizer):
         tx_new.locktime = get_locktime_for_new_transaction(self.network)
         return tx_new
 
-    def _bump_fee_through_coinchooser(self, *, tx, new_fee_rate):
+    def _bump_fee_through_coinchooser(self, *, tx: Transaction, new_fee_rate) -> Transaction:
         tx = Transaction(tx.serialize())
         tx.deserialize(force_full_parse=True)  # need to parse inputs
         tx.remove_signatures()
@@ -1073,7 +1073,7 @@ class Abstract_Wallet(AddressSynchronizer):
         except NotEnoughFunds as e:
             raise CannotBumpFee(e)
 
-    def _bump_fee_through_decreasing_outputs(self, *, tx, new_fee_rate):
+    def _bump_fee_through_decreasing_outputs(self, *, tx: Transaction, new_fee_rate) -> Transaction:
         tx = Transaction(tx.serialize())
         tx.deserialize(force_full_parse=True)  # need to parse inputs
         tx.remove_signatures()
@@ -1115,7 +1115,7 @@ class Abstract_Wallet(AddressSynchronizer):
 
         return Transaction.from_io(inputs, outputs)
 
-    def cpfp(self, tx, fee):
+    def cpfp(self, tx: Transaction, fee: int) -> Optional[Transaction]:
         txid = tx.txid()
         for i, o in enumerate(tx.outputs()):
             address, value = o.address, o.value