twallet: organise get_tx_fee. store calculated fees. storage version 19. - electrum - Electrum Bitcoin wallet HTML git clone https://git.parazyd.org/electrum DIR Log DIR Files DIR Refs DIR Submodules --- DIR commit 482605edbbe43aa27cbea58729d38585b75e4ee7 DIR parent 5c83e8bd1cbe7ebd6ebae560f06ee65b7772a1d1 HTML Author: SomberNight <somber.night@protonmail.com> Date: Thu, 12 Sep 2019 03:44:16 +0200 wallet: organise get_tx_fee. store calculated fees. storage version 19. Diffstat: M electrum/address_synchronizer.py | 56 +++++++++++++++++++++---------- M electrum/gui/qt/main_window.py | 16 ++++++++++------ M electrum/json_db.py | 71 +++++++++++++++++++++++++++---- M electrum/wallet.py | 20 ++++++++++---------- 4 files changed, 121 insertions(+), 42 deletions(-) --- DIR diff --git a/electrum/address_synchronizer.py b/electrum/address_synchronizer.py t@@ -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() t@@ -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): t@@ -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.""" t@@ -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: t@@ -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): t@@ -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 = self.db.get_tx_fee(tx_hash) + fee, is_calculated_by_us = self.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 t@@ -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 t@@ -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 - if hasattr(tx, '_cached_fee'): - return tx._cached_fee + return None, False with self.lock, self.transaction_lock: is_relevant, is_mine, v, fee = self.get_wallet_delta(tx) - if fee is None: - txid = tx.txid() - fee = self.db.get_tx_fee(txid) - # only cache non-None, as None can still change while syncing - if fee is not None: - tx._cached_fee = fee - return fee + # save result + self.db.add_tx_fee_we_calculated(txid, fee) + self.db.add_num_inputs_to_tx(txid, len(tx.inputs())) + return fee, True def get_addr_io(self, address): with self.lock, self.transaction_lock: DIR diff --git a/electrum/gui/qt/main_window.py b/electrum/gui/qt/main_window.py t@@ -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 t@@ -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): - fee = self.wallet.get_tx_fee(tx) - if fee is None: + def bump_fee_dialog(self, tx: Transaction): + txid = tx.txid() + 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')) DIR diff --git a/electrum/json_db.py b/electrum/json_db.py t@@ -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 t@@ -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 t@@ -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): t@@ -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() t@@ -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 t@@ -667,12 +681,48 @@ class JsonDB(Logger): return txid in self.verified_tx @modifier - def update_tx_fees(self, d): - return self.tx_fees.update(d) + def add_tx_fee_from_server(self, txid: str, fee_sat: Optional[int]) -> None: + # 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): - return self.tx_fees.get(txid) + def get_num_ismine_inputs_of_tx(self, txid: str) -> int: + txins = self.txi.get(txid, {}) + return sum([len(tupls) for addr, tupls in txins.items()]) @modifier def remove_tx_fee(self, txid): t@@ -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) t@@ -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): DIR diff --git a/electrum/wallet.py b/electrum/wallet.py t@@ -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 t@@ -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] - if fee is None: - fee = self.db.get_tx_fee(tx_hash) + fee, _calc_by_us = self.get_tx_fee(tx_hash) if fee is not None: size = tx.estimated_size() fee_per_byte = fee / size t@@ -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 """ t@@ -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) - if old_fee is None: + old_txid = tx.txid() + 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: t@@ -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() t@@ -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() t@@ -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