From 964d4b97215ae60306018c92eb2bf53338e6b256 Mon Sep 17 00:00:00 2001 From: Kim Neunert Date: Fri, 24 Mar 2023 13:34:21 +0100 Subject: [PATCH] Further refactoring including address- and txlist to wallet package --- .../specter/liquid/addresslist.py | 2 +- src/cryptoadvance/specter/liquid/txlist.py | 2 +- src/cryptoadvance/specter/liquid/wallet.py | 2 +- .../server_endpoints/wallets/wallets_api.py | 2 +- src/cryptoadvance/specter/services/service.py | 1 - src/cryptoadvance/specter/wallet/__init__.py | 2 + .../specter/wallet/abstract_wallet.py | 27 ++ .../specter/{ => wallet}/addresslist.py | 2 +- .../specter/wallet/tx_fetcher.py | 258 ++++++++++-------- .../specter/{ => wallet}/txlist.py | 12 +- src/cryptoadvance/specter/wallet/wallet.py | 40 ++- src/cryptoadvance/specterext/swan/service.py | 2 +- tests/test_labels_import.py | 2 +- tests/test_managers_wallet.py | 6 +- tests/test_wallet.py | 2 +- .../{test_txlist.py => test_wallet_txlist.py} | 2 +- 16 files changed, 210 insertions(+), 154 deletions(-) create mode 100644 src/cryptoadvance/specter/wallet/abstract_wallet.py rename src/cryptoadvance/specter/{ => wallet}/addresslist.py (99%) rename src/cryptoadvance/specter/{ => wallet}/txlist.py (98%) rename tests/{test_txlist.py => test_wallet_txlist.py} (99%) diff --git a/src/cryptoadvance/specter/liquid/addresslist.py b/src/cryptoadvance/specter/liquid/addresslist.py index 003a9d7435..d3fa5ce6e9 100644 --- a/src/cryptoadvance/specter/liquid/addresslist.py +++ b/src/cryptoadvance/specter/liquid/addresslist.py @@ -1,4 +1,4 @@ -from ..addresslist import * +from ..wallet.addresslist import * from embit.liquid.addresses import addr_decode, to_unconfidential diff --git a/src/cryptoadvance/specter/liquid/txlist.py b/src/cryptoadvance/specter/liquid/txlist.py index f7efecb615..f0f19ef7dd 100644 --- a/src/cryptoadvance/specter/liquid/txlist.py +++ b/src/cryptoadvance/specter/liquid/txlist.py @@ -1,4 +1,4 @@ -from ..txlist import * +from ..wallet.txlist import * from embit.liquid.transaction import LTransaction, TxOutWitness, unblind from embit.liquid.pset import PSET from embit.liquid import slip77 diff --git a/src/cryptoadvance/specter/liquid/wallet.py b/src/cryptoadvance/specter/liquid/wallet.py index ced71a2206..2c9dae9761 100644 --- a/src/cryptoadvance/specter/liquid/wallet.py +++ b/src/cryptoadvance/specter/liquid/wallet.py @@ -8,12 +8,12 @@ from embit.liquid.pset import PSET from embit.liquid.transaction import LTransaction -from ..addresslist import Address from ..specter_error import SpecterError from ..wallet import * from .addresslist import LAddressList from .txlist import LTxList from .util.pset import SpecterPSET +from .addresslist import Address class LWallet(Wallet): diff --git a/src/cryptoadvance/specter/server_endpoints/wallets/wallets_api.py b/src/cryptoadvance/specter/server_endpoints/wallets/wallets_api.py index 065a50a128..6a0018dd34 100644 --- a/src/cryptoadvance/specter/server_endpoints/wallets/wallets_api.py +++ b/src/cryptoadvance/specter/server_endpoints/wallets/wallets_api.py @@ -21,7 +21,7 @@ from flask_login import current_user, login_required from werkzeug.wrappers import Response -from cryptoadvance.specter.txlist import WalletAwareTxItem +from cryptoadvance.specter.wallet.txlist import WalletAwareTxItem from ...commands.psbt_creator import PsbtCreator from ...helpers import bcur2base64 diff --git a/src/cryptoadvance/specter/services/service.py b/src/cryptoadvance/specter/services/service.py index 2409642d6d..5d73ce24f0 100644 --- a/src/cryptoadvance/specter/services/service.py +++ b/src/cryptoadvance/specter/services/service.py @@ -13,7 +13,6 @@ from .service_encrypted_storage import ServiceEncryptedStorageManager from .service_annotations_storage import ServiceAnnotationsStorage -from cryptoadvance.specter.addresslist import Address from cryptoadvance.specter.services import callbacks diff --git a/src/cryptoadvance/specter/wallet/__init__.py b/src/cryptoadvance/specter/wallet/__init__.py index edc5020eb5..e3f2b4d7f8 100644 --- a/src/cryptoadvance/specter/wallet/__init__.py +++ b/src/cryptoadvance/specter/wallet/__init__.py @@ -1 +1,3 @@ from .wallet import Wallet, purposes +from .addresslist import Address +from .txlist import WalletAwareTxItem diff --git a/src/cryptoadvance/specter/wallet/abstract_wallet.py b/src/cryptoadvance/specter/wallet/abstract_wallet.py new file mode 100644 index 0000000000..b3b1ea5296 --- /dev/null +++ b/src/cryptoadvance/specter/wallet/abstract_wallet.py @@ -0,0 +1,27 @@ +from .txlist import TxList +import os +from abc import ABC, abstractmethod + + +class AbstractWallet: + @property + def transactions(self) -> TxList: + if hasattr(self, "_transactions"): + return self._transactions + else: + return None + + @property + def addresses(self) -> TxList: + if hasattr(self, "_addresses"): + return self._addresses + else: + return None + + @property + @abstractmethod + def rpc(self): + """Cache RPC instance. Reuse if manager's RPC instance hasn't changed. Create new RPC instance otherwise. + This RPC instance is also used by objects created by the wallet, such as TxList or TxItem + """ + pass diff --git a/src/cryptoadvance/specter/addresslist.py b/src/cryptoadvance/specter/wallet/addresslist.py similarity index 99% rename from src/cryptoadvance/specter/addresslist.py rename to src/cryptoadvance/specter/wallet/addresslist.py index 4f0f90f311..b24d682e1e 100644 --- a/src/cryptoadvance/specter/addresslist.py +++ b/src/cryptoadvance/specter/wallet/addresslist.py @@ -2,7 +2,7 @@ Manages the list of addresses for the wallet, including labels and derivation paths """ import os -from .persistence import write_csv, read_csv +from ..persistence import write_csv, read_csv import logging logger = logging.getLogger(__name__) diff --git a/src/cryptoadvance/specter/wallet/tx_fetcher.py b/src/cryptoadvance/specter/wallet/tx_fetcher.py index 3d24d0b2a6..946f53e532 100644 --- a/src/cryptoadvance/specter/wallet/tx_fetcher.py +++ b/src/cryptoadvance/specter/wallet/tx_fetcher.py @@ -1,5 +1,5 @@ import logging -from .wallet import Wallet +from .abstract_wallet import AbstractWallet logger = logging.getLogger(__name__) @@ -9,9 +9,94 @@ class TxFetcher: LISTTRANSACTIONS_BATCH_SIZE = 1000 - def __init__(self, wallet: Wallet): + def __init__(self, wallet: AbstractWallet): self.wallet = wallet + def _fetch_transactions(self): + + # unconfirmed_selftransfers needed since Bitcoin Core does not properly list `selftransfer` txs in `listtransactions` command + # Until v0.21, it listed there consolidations to a receive address, but not change address + # Since v0.21, it does not list there consolidations at all + # Therefore we need to check here if a transaction might got confirmed + # NOTE: This might be a problem in case of re-org... + # More details: https://github.com/cryptoadvance/specter-desktop/issues/996 + + arr = [ + tx["result"] + for tx in self.unconfirmed_selftransfers_txs + if tx.get("result") + ] + arr.extend(self.interesting_txs()) + txs = self.transform_to_dict_with_txid_as_key( + arr + ) # and gettransaction as value + + # fix for core versions < v0.20 (add blockheight if not there) + self.fill_blockheight_if_necessary(txs) + + if self.wallet.use_descriptors: + # Get all used addresses that belong to the wallet + + addresses_info = self.extract_addresses(txs) + + # representing the highest index of the addresses from the wallet and + # the passed addresses + ( + max_used_receiving, + max_used_change, + ) = self.calculate_max_used_from_addresses(addresses_info) + + # If max receiving address bigger than current max receiving index minus the gap limit - self._addresses.max_index(change=False) + if ( + max_used_receiving + self.wallet.GAP_LIMIT + > self.wallet._addresses.max_index(change=False) + ): + addresses = [ + dict( + address=self.wallet.get_address( + idx, change=False, check_keypool=False + ), + index=idx, + change=False, + ) + for idx in range( + self.wallet.addresses.max_index(change=False), + max_used_receiving + self.wallet.GAP_LIMIT, + ) + ] + self.wallet.addresses.add(addresses, check_rpc=False) + + # If max change address bigger than current max change index minus the gap limit - wallet.addresses.max_index(change=True) + if ( + max_used_change + self.wallet.GAP_LIMIT + > self.wallet.addresses.max_index(change=True) + ): + # Add change addresses until the new max address plus the GAP_LIMIT + change_addresses = [ + dict( + address=self.wallet.get_address( + idx, change=True, check_keypool=False + ), + index=idx, + change=True, + ) + for idx in range( + self.wallet.addresses.max_index(change=True), + max_used_change + self.wallet.GAP_LIMIT, + ) + ] + self.wallet.addresses.add(change_addresses, check_rpc=False) + + # only delete with confirmed txs + self.wallet.delete_spent_pending_psbts( + [ + tx["hex"] + for tx in txs.values() + if tx.get("confirmations", 0) > 0 or tx.get("blockheight") + ] + ) + self.wallet.transactions.add(txs) + def is_interesting_tx(self, tx: dict): """transactions that we don't know about, # or that it has a different blockhash (reorg / confirmed) @@ -114,134 +199,67 @@ def unconfirmed_selftransfers_txs(self): self._unconfirmed_selftransfers_txs = [] return self._unconfirmed_selftransfers_txs - def _fetch_transactions(self): - - # unconfirmed_selftransfers needed since Bitcoin Core does not properly list `selftransfer` txs in `listtransactions` command - # Until v0.21, it listed there consolidations to a receive address, but not change address - # Since v0.21, it does not list there consolidations at all - # Therefore we need to check here if a transaction might got confirmed - # NOTE: This might be a problem in case of re-org... - # More details: https://github.com/cryptoadvance/specter-desktop/issues/996 - - arr = [ - tx["result"] - for tx in self.unconfirmed_selftransfers_txs - if tx.get("result") + def extract_addresses(self, txs): + """Takes txs (dict with txid as key and the result of gettransaction as value ) + and extracts all the addresses which + * belongs to the wallet + * are not yet in self.addresses + """ + potential_relevant_txs = [ + tx + for tx in txs.values() + if tx + and tx.get("details") + and ( + tx.get("details")[0].get("category") != "send" + and tx["details"][0].get("address") not in self.wallet.addresses + ) ] - arr.extend(self.interesting_txs()) - txs = self.transform_to_dict_with_txid_as_key( - arr - ) # and gettransaction as value - # fix for core versions < v0.20 (add blockheight if not there) - self.fill_blockheight_if_necessary(txs) - - if self.wallet.use_descriptors: - # Get all used addresses that belong to the wallet - - potential_relevant_txs = [ - tx - for tx in txs.values() - if tx - and tx.get("details") - and ( - tx.get("details")[0].get("category") - != "send" - # and tx["details"][0].get("address") not in self.wallet._addresses - ) - ] - - addresses_info_multi = self.wallet.rpc.multi( - [ - ("getaddressinfo", address) - for address in [ - tx["details"][0].get("address") for tx in potential_relevant_txs - ] - if address + addresses_info_multi = self.wallet.rpc.multi( + [ + ("getaddressinfo", address) + for address in [ + tx["details"][0].get("address") for tx in potential_relevant_txs ] - ) - - addresses_info = [ - r["result"] - for r in addresses_info_multi - if r["result"].get("ismine", False) + if address ] + ) - # Gets max index used receiving and change addresses - max_used_receiving = self.wallet._addresses.max_used_index(change=False) - max_used_change = self.wallet._addresses.max_used_index(change=True) - - for address in addresses_info: - desc = self.wallet.DescriptorCls.from_string(address["desc"]) - indexes = [ - { - "idx": k.origin.derivation[-1], - "change": k.origin.derivation[-2], - } - for k in desc.keys - ] - for idx in indexes: - if int(idx["change"]) == 0: - max_used_receiving = max(max_used_receiving, int(idx["idx"])) - elif int(idx["change"]) == 1: - max_used_change = max(max_used_change, int(idx["idx"])) - - # If max receiving address bigger than current max receiving index minus the gap limit - self._addresses.max_index(change=False) - if ( - max_used_receiving + self.wallet.GAP_LIMIT - > self.wallet._addresses.max_index(change=False) - ): - print( - "------------+++++++++++++++++++++++++++++++++Add receiving addresses until the new max address plus the GAP_LIMIT" - ) - addresses = [ - dict( - address=self.wallet.get_address( - idx, change=False, check_keypool=False - ), - index=idx, - change=False, - ) - for idx in range( - self.wallet._addresses.max_index(change=False), - max_used_receiving + self.wallet.GAP_LIMIT, - ) - ] - self.wallet._addresses.add(addresses, check_rpc=False) + addresses_info = [ + r["result"] + for r in addresses_info_multi + if r["result"].get("ismine", False) + ] + return addresses_info - # If max change address bigger than current max change index minus the gap limit - wallet._addresses.max_index(change=True) - if ( - max_used_change + self.wallet.GAP_LIMIT - > self.wallet._addresses.max_index(change=True) - ): - # Add change addresses until the new max address plus the GAP_LIMIT - change_addresses = [ - dict( - address=self.wallet.get_address( - idx, change=True, check_keypool=False - ), - index=idx, - change=True, - ) - for idx in range( - self.wallet._addresses.max_index(change=True), - max_used_change + self.wallet.GAP_LIMIT, - ) - ] - self.wallet._addresses.add(change_addresses, check_rpc=False) + def calculate_max_used_from_addresses(self, addresses_info): + """Return a tuple of max_used_receiving and max_used_change + representing the highest index of the addresses from the wallet and + the passed addresses + """ + # Gets max index used receiving and change addresses + max_used_receiving = self.wallet.addresses.max_used_index(change=False) + max_used_change = self.wallet.addresses.max_used_index(change=True) - # only delete with confirmed txs - self.wallet.delete_spent_pending_psbts( - [ - tx["hex"] - for tx in txs.values() - if tx.get("confirmations", 0) > 0 or tx.get("blockheight") + for address in addresses_info: + desc = self.wallet.DescriptorCls.from_string(address["desc"]) + indexes = [ + { + "idx": k.origin.derivation[-1], + "change": k.origin.derivation[-2], + } + for k in desc.keys ] - ) - self.wallet._transactions.add(txs) + for idx in indexes: + if int(idx["change"]) == 0: + max_used_receiving = max(max_used_receiving, int(idx["idx"])) + elif int(idx["change"]) == 1: + max_used_change = max(max_used_change, int(idx["idx"])) + return max_used_receiving, max_used_change @classmethod - def fetch_transactions(cls, wallet: Wallet): + def fetch_transactions(cls, wallet: AbstractWallet): """Loads new transactions from Bitcoin Core. A quite confusing method which mainly tries to figure out which transactions are new and need to be added to the local TxList wallet._transactions and adding them. So the method doesn't return anything but has these side_effects: diff --git a/src/cryptoadvance/specter/txlist.py b/src/cryptoadvance/specter/wallet/txlist.py similarity index 98% rename from src/cryptoadvance/specter/txlist.py rename to src/cryptoadvance/specter/wallet/txlist.py index 82197b1a79..fa13e2c5ea 100644 --- a/src/cryptoadvance/specter/txlist.py +++ b/src/cryptoadvance/specter/wallet/txlist.py @@ -11,20 +11,20 @@ from embit.liquid.networks import get_network from embit.transaction import Transaction -from .helpers import get_address_from_dict -from .persistence import delete_file, read_csv, write_csv -from .specter_error import SpecterError, SpecterInternalException +from ..helpers import get_address_from_dict +from ..persistence import delete_file, read_csv, write_csv +from ..specter_error import SpecterError, SpecterInternalException from embit.descriptor import Descriptor from embit.liquid.descriptor import LDescriptor -from .util.common import str2bool -from .util.psbt import ( +from ..util.common import str2bool +from ..util.psbt import ( AbstractTxContext, SpecterInputScope, SpecterOutputScope, SpecterPSBT, SpecterTx, ) -from .util.tx import decoderawtransaction +from ..util.tx import decoderawtransaction from threading import RLock logger = logging.getLogger(__name__) diff --git a/src/cryptoadvance/specter/wallet/wallet.py b/src/cryptoadvance/specter/wallet/wallet.py index 385675acb1..30a23ae599 100644 --- a/src/cryptoadvance/specter/wallet/wallet.py +++ b/src/cryptoadvance/specter/wallet/wallet.py @@ -1,10 +1,16 @@ -import json, logging, os, re, csv -import requests +import csv +import json +import logging +import os +import re import threading import time - from collections import OrderedDict from csv import Error +from io import StringIO +from typing import Dict, List + +import requests from embit import bip32 from embit.descriptor import Descriptor from embit.descriptor.checksum import add_checksum @@ -12,23 +18,23 @@ from embit.liquid.networks import get_network from embit.psbt import DerivationPath from embit.transaction import Transaction -from io import StringIO -from typing import Dict, List from cryptoadvance.specter.commands.utxo_scanner import UtxoScanner from cryptoadvance.specter.rpc import RpcError -from ..addresslist import Address, AddressList from ..device import Device -from ..key import Key -from ..util.merkleblock import is_valid_merkle_proof from ..helpers import get_address_from_dict -from ..persistence import write_json_file, delete_file, delete_folder +from ..key import Key +from ..persistence import delete_file, delete_folder, write_json_file from ..specter_error import SpecterError, handle_exception -from ..txlist import TxItem, TxList, WalletAwareTxItem +from ..util.merkleblock import is_valid_merkle_proof from ..util.psbt import SpecterPSBT from ..util.tx import decoderawtransaction from ..util.xpub import get_xpub_fingerprint +from .tx_fetcher import TxFetcher +from .txlist import TxItem, TxList, WalletAwareTxItem +from .abstract_wallet import AbstractWallet +from .addresslist import AddressList, Address logger = logging.getLogger(__name__) LISTTRANSACTIONS_BATCH_SIZE = 1000 @@ -57,7 +63,7 @@ } -class Wallet: +class Wallet(AbstractWallet): # if the wallet is old we import 300 addresses IMPORT_KEYPOOL = 300 # a gap of 20 addresses is what many wallets do (not used with descriptor wallets) @@ -404,8 +410,6 @@ def fetch_transactions(self): 3. calls self.delete_spent_pending_psbts Most of that code could probably encapsulated in the TxList class. """ - from .tx_fetcher import TxFetcher - TxFetcher.fetch_transactions(self) def import_address_labels(self, address_labels): @@ -1544,11 +1548,17 @@ def full_available_balance(self): return round(self.balance["trusted"] + self.balance["untrusted_pending"], 8) @property - def addresses(self): + def relevant_addresses(self): + """Not used much (only in tests?!) + Returns a list of addresses from index 0 until self.address_index + 1 + """ return [self.get_address(idx) for idx in range(0, self.address_index + 1)] @property - def change_addresses(self): + def relevant_change_addresses(self): + """Not used much (only in tests?!) + Returns a list of change-addresses from index 0 until self.change_index + 1 + """ return [ self.get_address(idx, change=True) for idx in range(0, self.change_index + 1) diff --git a/src/cryptoadvance/specterext/swan/service.py b/src/cryptoadvance/specterext/swan/service.py index 0969ef81b7..92190f424f 100644 --- a/src/cryptoadvance/specterext/swan/service.py +++ b/src/cryptoadvance/specterext/swan/service.py @@ -18,7 +18,7 @@ devstatus_beta, devstatus_prod, ) -from cryptoadvance.specter.addresslist import Address +from cryptoadvance.specter.wallet import Address from cryptoadvance.specter.wallet import Wallet from urllib.parse import urlparse diff --git a/tests/test_labels_import.py b/tests/test_labels_import.py index 1a073b0a68..2e7681d8e6 100644 --- a/tests/test_labels_import.py +++ b/tests/test_labels_import.py @@ -19,7 +19,7 @@ def test_import_address_labels( txid = utxos[0]["txid"] logger.debug(f"this is the txid: {txid}.") test_address = utxos[0]["address"] - logger.debug(f"these are the addresses: {wallet.addresses}.") + logger.debug(f"these are the relevant_addresses: {wallet.relevant_addresses}.") logger.debug(f"these are the _addresses: {wallet._addresses}.") assert wallet._addresses[test_address]["label"] is None number_of_addresses = len(wallet._addresses) diff --git a/tests/test_managers_wallet.py b/tests/test_managers_wallet.py index bbe331912b..3a0f62de2a 100644 --- a/tests/test_managers_wallet.py +++ b/tests/test_managers_wallet.py @@ -329,7 +329,7 @@ def test_wallet_labeling(bitcoin_regtest, devices_filled_data_folder, device_man wallet.setlabel(third_address, "") bitcoin_regtest.testcoin_faucet(third_address, amount=0.4) - assert sorted(wallet.addresses) == sorted( + assert sorted(wallet.relevant_addresses) == sorted( [first_address, second_address, third_address] ) @@ -376,8 +376,8 @@ def test_wallet_change_addresses( address = wallet.address change_address = wallet.change_address - assert wallet.addresses == [address] - assert wallet.change_addresses == [change_address] + assert wallet.relevant_addresses == [address] + assert wallet.relevant_change_addresses == [change_address] bitcoin_regtest.testcoin_faucet(change_address, amount=0.1) wallet.update_balance() assert wallet.amount_total == 0.1 diff --git a/tests/test_wallet.py b/tests/test_wallet.py index 0fb89dc6e3..80864cf76c 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -5,7 +5,7 @@ import pytest, logging from cryptoadvance.specter.util.psbt import SpecterPSBT from cryptoadvance.specter.commands.psbt_creator import PsbtCreator -from cryptoadvance.specter.txlist import WalletAwareTxItem +from cryptoadvance.specter.wallet.txlist import WalletAwareTxItem from cryptoadvance.specter.device import Device from cryptoadvance.specter.specter import Specter diff --git a/tests/test_txlist.py b/tests/test_wallet_txlist.py similarity index 99% rename from tests/test_txlist.py rename to tests/test_wallet_txlist.py index 884b963c32..cbf2b4fac1 100644 --- a/tests/test_txlist.py +++ b/tests/test_wallet_txlist.py @@ -13,7 +13,7 @@ BitcoindPlainController, ) from cryptoadvance.specter.rpc import BitcoinRPC -from cryptoadvance.specter.txlist import TxItem, TxList, WalletAwareTxItem +from cryptoadvance.specter.wallet.txlist import TxItem, TxList, WalletAwareTxItem from cryptoadvance.specter.util.psbt import ( SpecterInputScope, SpecterOutputScope,