Bugfix: method getaddressinfo not implemented (#2313)

This PR fixes #2312 which are four things:
* getaddressinfo was not implemented in spectrum which results in "method not found"
* There was an unrelated KeyError which occured in Transactions where one the output was a change-address. It's a bit unknown why that hasn't occured earlier. At least it was diffcult to spot as the fetch_transactions call was way to huge and very confusing. So ...
* the fetch_transactions call was refactored in its own class, TxFetcher. As we did that,  we also moved two other related files in a newly created wallet-package.
* Tiny bug around tx["amount"] which only exists as tx["flow_amount"] for transactions.

requirements.in

@@ -33,5 +33,5 @@ psycopg2-binary==2.9.5
 cryptoadvance-liquidissuer==0.2.4
 specterext-exfund==0.1.7
 specterext-faucet==0.1.2
-cryptoadvance.spectrum==0.6.3
+cryptoadvance.spectrum==0.6.4
 specterext-stacktrack==0.3.0

requirements.txt

@@ -2,7 +2,7 @@
 # This file is autogenerated by pip-compile with Python 3.10
 # by the following command:
 #
-#    pip-compile --generate-hashes requirements.in
+#    pip-compile --generate-hashes --resolver=backtracking requirements.in
 #
 aniso8601==9.0.1 \
     --hash=sha256:1d2b7ef82963909e93c4f24ce48d4de9e66009a21bf1c1e1c85bdd0812fe412f \
@@ -119,9 +119,9 @@ cryptoadvance-liquidissuer==0.2.4 \
     --hash=sha256:5a2c531801854c5a4a46daf184877e22f731cdb42d2cfb840785bda7371ba6fb \
     --hash=sha256:9e468f3e35ecc566b3f74a2263677cf26632548abb194521dba15ad37acd1e9b
     # via -r requirements.in
-cryptoadvance-spectrum==0.6.3 \
-    --hash=sha256:706287a9e1640f1a27a1a3c6e27a147599dde9969e7dbd2df99c890514b2b1ba \
-    --hash=sha256:877006d81c5be0d5da12ca8d4b0dd7dc21b104bc826cd35c7de5019ca56cf64a
+cryptoadvance-spectrum==0.6.4 \
+    --hash=sha256:226f89dfe96dac2fcd82f462706e118f017582df5565ff1924f27e4392bc6a0e \
+    --hash=sha256:fa6c877b3a5ba683ccde991f020fce2f3b06e0b8f5219a6fdc592658cacb1d3d
     # via -r requirements.in
 cryptography==39.0.1 \
     --hash=sha256:0f8da300b5c8af9f98111ffd512910bc792b4c77392a9523624680f7956a99d4 \

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
 
 

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

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):

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
@@ -1075,8 +1075,8 @@ def process_txlist(txlist, idx=0, limit=100, search=None, sortby=None, sortdir="
             )
             or (
                 any(search in str(amount) for amount in tx["amount"])
-                if isinstance(tx["amount"], list)
-                else search in str(tx["amount"])
+                if isinstance(tx["flow_amount"], list)
+                else search in str(tx["flow_amount"])
             )
             or search in str(tx["confirmations"])
             or search in str(tx["time"])

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
 
 

src/cryptoadvance/specter/wallet/__init__.py

@@ -0,0 +1,3 @@
+from .wallet import Wallet, purposes
+from .addresslist import Address
+from .txlist import WalletAwareTxItem

src/cryptoadvance/specter/wallet/abstract_wallet.py

@@ -0,0 +1,25 @@
+from .txlist import TxList
+
+
+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

src/cryptoadvance/specter/addresslist.py → 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__)

src/cryptoadvance/specter/wallet/tx_fetcher.py

@@ -0,0 +1,275 @@
+import logging
+from .abstract_wallet import AbstractWallet
+
+logger = logging.getLogger(__name__)
+
+
+class TxFetcher:
+    """A class to refactor the fetch_transaction_method which no one understands"""
+
+    LISTTRANSACTIONS_BATCH_SIZE = 1000
+
+    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)
+        # or doesn't have an address(?)
+        # or has wallet conflicts
+        """
+        if (
+            # we don't know about tx
+            tx["txid"] not in self.wallet._transactions
+            # we don't know addresses
+            or not self.wallet._transactions[tx["txid"]].get("address", None)
+            # blockhash is different (reorg / unconfirmed)
+            or self.wallet._transactions[tx["txid"]].get("blockhash", None)
+            != tx.get("blockhash", None)
+            # we have conflicts
+            or self.wallet._transactions[tx["txid"]].get("conflicts", [])
+            != tx.get("walletconflicts", [])
+        ):
+            return True
+        return False
+
+    def interesting_txs(self):
+        """returns an array of interesting transactions (see is_interesting_tx() ) where txid is
+        the key and the result is whatever listtransactions is retuirning as values
+        """
+        idx = 0
+        arr = []
+        while True:
+            txlist = self.wallet.rpc.listtransactions(
+                "*",
+                self.LISTTRANSACTIONS_BATCH_SIZE,  # count
+                self.LISTTRANSACTIONS_BATCH_SIZE * idx,  # skip
+                True,
+            )
+            # filter interesting TXs
+            res = [tx for tx in txlist if self.is_interesting_tx(tx)]
+            arr.extend(res)
+            idx += 1
+            # stop if we reached known transactions
+            # not sure if Core <20 returns last batch or empty array at the end
+            if (
+                len(res) < self.LISTTRANSACTIONS_BATCH_SIZE
+                or len(arr) < self.LISTTRANSACTIONS_BATCH_SIZE * idx
+            ):
+                break
+
+        return arr
+
+    def transform_to_dict_with_txid_as_key(self, arr):
+        """gets an array of tx-dicts where and transoforms it
+        to a dict with txid as keys and the corresponding result
+        of gettransaction as value.
+        """
+        # Start with an dict with txids as keys and None as values:
+        txs = dict.fromkeys([a["txid"] for a in arr])
+        txids = list(txs.keys())
+        # get all raw transactions
+        res = self.wallet.rpc.multi([("gettransaction", txid) for txid in txids])
+        for i, r in enumerate(res):
+            txid = txids[i]
+            # check if we already added it
+            if txs.get(txid, None) is not None:
+                continue
+            txs[txid] = r["result"]
+        return txs
+
+    def fill_blockheight_if_necessary(self, txs):
+        """
+        This is a fix for Bitcoin Core versions < v0.20
+        These do not return the blockheight as part of the `gettransaction` command
+        So here we check if this property is lacking and if so
+        query the current block height and manually calculate it.
+
+        Remove after dropping Core v0.19 support
+        """
+        check_blockheight = False
+        for tx in txs.values():
+            if tx and tx.get("confirmations", 0) > 0 and "blockheight" not in tx:
+                check_blockheight = True
+                break
+        if check_blockheight:
+            current_blockheight = self.wallet.rpc.getblockcount()
+            for tx in txs.values():
+                if tx.get("confirmations", 0) > 0:
+                    tx["blockheight"] = current_blockheight - tx["confirmations"] + 1
+
+    @property
+    def unconfirmed_selftransfers_txs(self):
+        if not hasattr(self, "_unconfirmed_selftransfers_txs"):
+            unconfirmed_selftransfers = [
+                txid
+                for txid in self.wallet._transactions
+                if self.wallet._transactions[txid].category == "selftransfer"
+                and not self.wallet._transactions[txid].get("blockhash", None)
+            ]
+            unconfirmed_selftransfers_txs = []
+            if unconfirmed_selftransfers:
+                self._unconfirmed_selftransfers_txs = self.wallet.rpc.multi(
+                    [("gettransaction", txid) for txid in unconfirmed_selftransfers]
+                )
+            else:
+                self._unconfirmed_selftransfers_txs = []
+        return self._unconfirmed_selftransfers_txs
+
+    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
+            )
+        ]
+
+        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 = [
+            r["result"]
+            for r in addresses_info_multi
+            if r["result"].get("ismine", False)
+        ]
+        logger.info(f"Those addresses got used recently: {addresses_info}")
+        return addresses_info
+
+    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)
+
+        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"]))
+        return max_used_receiving, max_used_change
+
+    @classmethod
+    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:
+        1. Adding the new interesting transactions to wallet._transactions
+        2. for wallet.use_descriptors create new addresses and add them to wallet._addresses
+        3. calls wallet.delete_spent_pending_psbts
+        Most of that code could probably encapsulated in the TxList class.
+        """
+        tx_fetcher = TxFetcher(wallet)
+        tx_fetcher._fetch_transactions()