Bugfix: Addressinfo (#2001)

.pre-commit-config.yaml

@@ -3,4 +3,4 @@ repos:
     rev: 22.3.0
     hooks:
     - id: black
-      language_version: python3.9
+      language_version: python3.10

src/cryptoadvance/specter/server_endpoints/controller.py

@@ -63,8 +63,7 @@ logger = logging.getLogger(__name__)
 @app.errorhandler(RpcError)
 def server_rpc_error(rpce):
     """Specific SpecterErrors get passed on to the User as flash"""
-    logger.error(request.headers["Accept"])
-    if request.headers["Accept"] == "application/json":
+    if request.headers.get("Accept") == "application/json":
         return {"error": str(rpce)}
     if rpce.error_code == -18:  # RPC_WALLET_NOT_FOUND
 
@@ -85,7 +84,7 @@ def server_rpc_error(rpce):
 @app.errorhandler(SpecterError)
 def server_specter_error(se):
     """Specific SpecterErrors get passed on to the User as flash (or in error-field for json)"""
-    if request.headers["Accept"] == "application/json":
+    if request.headers.get("Accept") == "application/json":
         return {"error": str(se)}
     flash(str(se), "error")
     try:
@@ -105,7 +104,7 @@ def server_notFound_error(e):
     # if rpc is not available
     error_msg = "Could not find Resource (404): %s" % request.url
     app.logger.error(error_msg)
-    if request.headers["Accept"] == "application/json":
+    if request.headers.get("Accept") == "application/json":
         return {"error": error_msg}
     return render_template("500.jinja", error=e), 404
 
@@ -117,7 +116,7 @@ def server_error(e):
     app.logger.error(error_msg)
     trace = traceback.format_exc()
     app.logger.error(trace)
-    if request.headers["Accept"] == "application/json":
+    if request.headers.get("Accept") == "application/json":
         return {"error": error_msg}
     return render_template("500.jinja", error=e, traceback=trace), 500
 
@@ -126,7 +125,7 @@ def server_error(e):
 def server_broken_core_connection(e):
     error_msg = "You got disconnected from your node (no RPC connection)"
     logger.exception(e)
-    if request.headers["Accept"] == "application/json":
+    if request.headers.get("Accept") == "application/json":
         return {"error": error_msg}
     flash(error_msg, "error")
     try:
@@ -147,7 +146,7 @@ def server_error_timeout(e):
         # make sure specter knows that rpc is not there
         app.specter.check()
     app.logger.error("ExternalProcessTimeoutException: %s" % e)
-    if request.headers["Accept"] == "application/json":
+    if request.headers.get("Accept") == "application/json":
         return {"error": error_msg}
     flash(error_msg, "warn")
     return redirect(
@@ -167,7 +166,7 @@ def server_error_csrf(e):
     app.logger.error("CSRF Exception: %s" % e)
     trace = traceback.format_exc()
     app.logger.error(trace)
-    if request.headers["Accept"] == "application/json":
+    if request.headers.get("Accept") == "application/json":
         return {"error": error_msg}
     flash(error_msg, "error")
     return redirect(request.url)
@@ -180,7 +179,7 @@ def server_error_405(e):
     app.logger.error(f"{error_msg} : {e}")
     trace = traceback.format_exc()
     app.logger.error(trace)
-    if request.headers["Accept"] == "application/json":
+    if request.headers.get("Accept") == "application/json":
         return {"error": error_msg}
     flash(_("Session expired. Please refresh and try again."), "error")
     return redirect(request.url)

src/cryptoadvance/specter/server_endpoints/wallets/wallets_api.py

@@ -325,7 +325,7 @@ def decoderawtx(wallet_alias):
                 success=True,
                 tx=tx,
                 rawtx=rawtx,
-                walletName=wallet.name,
+                wallet_name=wallet.name,
             )
     except Exception as e:
         app.logger.exception(
@@ -512,38 +512,60 @@ def addresses_list(wallet_alias):
 def addressinfo(wallet_alias):
     wallet = app.specter.wallet_manager.get_by_alias(wallet_alias)
     address = request.form.get("address", "")
-    if address:
-        descriptor = wallet.get_descriptor(
-            address=address, keep_xpubs=False, to_string=True, with_checksum=True
+    if not address:
+        return jsonify(success=False)
+
+    descriptor = wallet.get_descriptor(
+        address=address, keep_xpubs=False, to_string=True, with_checksum=True
+    )
+
+    xpubs_descriptor = wallet.get_descriptor(
+        address=address, keep_xpubs=True, to_string=True, with_checksum=True
+    )
+    if (not descriptor) or (not xpubs_descriptor):
+        logger.debug(
+            f"No descriptor or xpubs_descriptor was found for address {address} in wallet {wallet.name}"
         )
-        xpubs_descriptor = wallet.get_descriptor(
-            address=address, keep_xpubs=True, to_string=True, with_checksum=True
-        )
-        # The last two regex groups are optional since Electrum's derivation path is shorter
-        derivation_path_pattern = (
-            r"(\/[0-9h]+)(\/[0-9h]+)(\/[0-9h]+)(\/[0-9h]+)?(\/[0-9h]+)?"
-        )
-        # Only "descriptor" gives full derivation path, looks usually like this:
-        # wpkh([8c24a510/84h/1h/0h/0/0]0331edcb16cfd ... e02552539d984)#35zjhlhm
-        match = re.search(derivation_path_pattern, descriptor)
-        if not match:
-            logger.debug(
-                f"Derivation path of this descriptor {descriptor} could not be parsed. Sth. wrong with the regex pattern which was {derivation_path_pattern}?"
-            )
+        return jsonify(success=False)
+
+    address_info = wallet.get_address_info(address=address)
+
+    # The last two regex groups are optional since Electrum's derivation path is shorter
+    derivation_path_pattern = (
+        r"(\/[0-9h]+)(\/[0-9h]+)(\/[0-9h]+)(\/[0-9h]+)?(\/[0-9h]+)?"
+    )
+    # Only "descriptor" gives full derivation path, looks usually like this:
+    # wpkh([8c24a510/84h/1h/0h/0/0]0331edcb16cfd ... e02552539d984)#35zjhlhm
+    match = re.search(derivation_path_pattern, descriptor)
+    if match:
         logger.debug(f"This is the derivation path match: {match.group()}")
-        derivation_path = "m" + match.group()
-        address_info = wallet.get_address_info(address=address)
+    else:
+        logger.debug(
+            f"Derivation path of this descriptor {descriptor} could not be parsed. Sth. wrong with the regex pattern which was {derivation_path_pattern}?"
+        )
+        # only partial information can be returned, because no match/derivation_path could be found
         return {
-            "success": True,
+            "success": False,
             "address": address,
             "descriptor": descriptor,
             "xpubs_descriptor": xpubs_descriptor,
-            "derivation_path": derivation_path,
-            "walletName": wallet.name,
-            "isMine": address_info and not address_info.is_external,
+            "wallet_name": wallet.name,
+            "is_mine": address_info and not address_info.is_external,
             **address_info,  # address_info is an instance of Address(dict)
         }
 
+    derivation_path = "m" + match.group()
+    return {
+        "success": True,
+        "address": address,
+        "descriptor": descriptor,
+        "xpubs_descriptor": xpubs_descriptor,
+        "derivation_path": derivation_path,
+        "wallet_name": wallet.name,
+        "is_mine": address_info and not address_info.is_external,
+        **address_info,  # address_info is an instance of Address(dict)
+    }
+
 
 ################## Wallet CSV export data endpoints #######################
 # Export wallet addresses list
@@ -684,14 +706,21 @@ def is_address_mine(wallet_alias, address):
     wallet = app.specter.wallet_manager.get_by_alias(wallet_alias)
 
     # filter out invalid input
-    if (not address) or not isinstance(address, str):
-        return jsonify(False)
+    # and Segwit addresses are always between 14 and 74 characters long.
+    if (not address) or (not isinstance(address, str)) or len(address) < 14:
+        return jsonify(
+            {
+                "is_mine": False,
+                "wallet_name": wallet.name if wallet else None,
+            }
+        )
 
-    # Segwit addresses are always between 14 and 74 characters long.
-    if len(address) < 14:
-        return jsonify(False)
-
-    return jsonify(wallet.is_address_mine(address))
+    return jsonify(
+        {
+            "is_mine": wallet.is_address_mine(address),
+            "wallet_name": wallet.name if wallet else None,
+        }
+    )
 
 
 @wallets_endpoint_api.route("/wallet/<wallet_alias>/send/estimatefee", methods=["POST"])

src/cryptoadvance/specter/static/helpers.js

@@ -196,7 +196,7 @@ async function send_request(url, method_str, csrf_token, formData) {
 			return {"error": `Error while calling ${url} with ${method_str} ${formData}` }
 		}
 		let jsonResponse = await response.json();
-		console.log('The response from the fetch call:')
+		console.log(`${method_str} call response:`)
 		console.log(jsonResponse)
 		if (typeof(jsonResponse) === 'boolean') {
 			return {}

src/cryptoadvance/specter/templates/includes/address-data.html

@@ -94,7 +94,7 @@
                 let derivation_path = jsonResponse.derivation_path;
                 let addressIndex = jsonResponse.index;
                 let isChange = jsonResponse.change;
-                let walletName = jsonResponse.walletName;
+                let walletName = jsonResponse.wallet_name;
                 let address = jsonResponse.address;
                 let walletLink = `{{ url_for('wallets_endpoint.wallet', wallet_alias='WALLET_ALIAS') }}`.replace("WALLET_ALIAS", this.wallet);
                 let addressInfoHTML = `

src/cryptoadvance/specter/templates/includes/recipient-box.html

@@ -470,8 +470,8 @@
 			let url="{{ url_for('wallets_endpoint_api.is_address_mine', wallet_alias=wallet.alias, address='this_address') }}"
 			url = url.replace('this_address', address)			
 					
-			let is_address_mine = send_request(url, 'GET', "{{ csrf_token() }}");
-			return is_address_mine;
+			let response = await send_request(url, 'GET', "{{ csrf_token() }}");
+			return response['is_mine'];
 		}
 
 		markRecipient(is_address_mine){

src/cryptoadvance/specter/templates/includes/tx-data.html

@@ -75,7 +75,7 @@
             if (jsonResponse !== null && jsonResponse.success) {
                 console.log(jsonResponse);
                 let rawtx = jsonResponse.rawtx;
-                let walletName = jsonResponse.walletName;
+                let walletName = jsonResponse.wallet_name;
                 let tx = jsonResponse.tx;
                 let walletLink = `{{ url_for('wallets_endpoint.wallet', wallet_alias='WALLET_ALIAS') }}`.replace("WALLET_ALIAS", this.wallet);
                 let rawtxHTML = "";
@@ -272,28 +272,12 @@
         }
         
         async fetchAddressIsMine(address) {
-            let url = `{{ url_for('wallets_endpoint_api.addressinfo', wallet_alias='WALLET_ALIAS') }}`.replace("WALLET_ALIAS", this.wallet);
-            var formData = new FormData();
-            formData.append('address', address)
-            formData.append('csrf_token', '{{ csrf_token() }}');
+            let url = `{{ url_for('wallets_endpoint_api.is_address_mine', wallet_alias='WALLET_ALIAS', address='ADDRESS') }}`.replace("WALLET_ALIAS", this.wallet).replace("ADDRESS", address);
             try {
-                const response = await fetch(
-                    url,
-                    {
-                        method: 'POST',
-                        body: formData
-                    }
-                );
-                if(response.status != 200){
-                    showError(await response.text());
-                    return false;
-                }
-                const jsonResponse = await response.json();
-                if (jsonResponse.success) {
-                    return jsonResponse.isMine;
-                }
+                const response = await send_request(url, 'GET', '{{ csrf_token() }}'); 
+                return response['is_mine'];
             }  catch(e) {
-                showError(`{{ _("Failed to load address data...") }}`);
+                showError(`{{ _("Failed to check is_address_mine...") }}`);
                 showError(e);
             }
             return false;

src/cryptoadvance/specter/wallet.py

@@ -1366,7 +1366,7 @@ class Wallet:
 
     def is_address_mine(self, address):
         addrinfo = self.get_address_info(address)
-        return addrinfo and not addrinfo.is_external
+        return bool(addrinfo) and not addrinfo.is_external
 
     def get_electrum_file(self):
         """Exports the wallet data as Electrum JSON format"""

tests/fix_devices_and_wallets.py

@@ -166,9 +166,10 @@ def funded_ghost_machine_wallet(
     bitcoin_regtest: NodeController, unfunded_ghost_machine_wallet: Wallet
 ) -> Wallet:
     funded_ghost_machine_wallet = unfunded_ghost_machine_wallet
-    bitcoin_regtest.testcoin_faucet(
-        funded_ghost_machine_wallet.getnewaddress()
-    )  # default value are 20 BTC
+    if funded_ghost_machine_wallet.amount_total == 0:
+        bitcoin_regtest.testcoin_faucet(
+            funded_ghost_machine_wallet.getnewaddress()
+        )  # default value are 20 BTC
     return funded_ghost_machine_wallet
 
 

tests/test_ep_wallet_api.py

@@ -147,6 +147,82 @@ def test_txout_set_info(caplog, app, client):
     assert json.loads(res.data)["total_amount"] > 0
 
 
+@pytest.mark.slow
+def test_addressinfo(caplog, client, funded_ghost_machine_wallet):
+    caplog.set_level(logging.DEBUG)
+    caplog.set_level(logging.DEBUG, logger="cryptoadvance.specter")
+    login(client, "secret")
+
+    # get loaded wallet info
+    response = client.get("/wallets/wallets_loading/", follow_redirects=True)
+    loaded_wallet_name = json.loads(response.data)["loaded_wallets"][0]
+    receive_address = funded_ghost_machine_wallet.get_address(0, change=False)
+    url = f"/wallets/wallet/{loaded_wallet_name}/addressinfo/"
+    headers = {
+        "Content-Type": "application/x-www-form-urlencoded",
+        "ACCEPT": "application/json",
+    }
+
+    # send post request
+    res = client.post(
+        url, data={"address": receive_address}, follow_redirects=True, headers=headers
+    )
+    assert res.status == "200 OK"
+    assert (
+        res.data.decode()
+        == '{"address":"bcrt1qvtdx75y4554ngrq6aff3xdqnvjhmct5wck95qs","change":false,"derivation_path":"m/84h/1h/0h/0/0","descriptor":"wpkh([8c24a510/84h/1h/0h/0/0]0331edcb16cfd0f8598052f7b287b07047a11c60967c1e7eb0257e02552539d984)#35zjhlhm","index":0,"is_mine":true,"label":null,"service_id":null,"success":true,"used":null,"wallet_name":"ghost_machine","xpubs_descriptor":"wpkh([8c24a510/84h/1h/0h]tpubDC4DsqH5rqHqipMNqUbDFtQT3AkKkUrvLsN6miySvortU3s1LGaNVAb7wX2No2VsuxQV82T8s3HJLv3kdx1CPjsJ3onC1Zo5mWCQzRVaWVX/0/0)#8f5u4zq2"}\n'
+    )
+
+    # send post request with bad address
+    invalid_address = "bcrt1qvtdx7"
+    res = client.post(
+        url, data={"address": invalid_address}, follow_redirects=True, headers=headers
+    )
+    assert (
+        res.data.decode()
+        == '{"error":"Request error for method getaddressinfo: Invalid address format"}\n'
+    )
+
+    # send post request with address, not belonging to wallet
+    # this recreates an edge case, see https://github.com/cryptoadvance/specter-desktop/issues/2000
+    valid_address_not_beloging_to_wallet = (
+        "bcrt1q895evdudfrmeut083vs85rc85g2wq6p6ql2hla"
+    )
+    res = client.post(
+        url,
+        data={"address": valid_address_not_beloging_to_wallet},
+        follow_redirects=True,
+        headers=headers,
+    )
+    assert res.status == "200 OK"
+    assert res.data.decode() == '{"success":false}\n'
+    assert (
+        caplog.text.count(
+            f"No descriptor or xpubs_descriptor was found for address {valid_address_not_beloging_to_wallet} in wallet ghost_machine"
+        )
+        == 1
+    )
+
+    # set erronious descriptor
+    with mock.patch.object(
+        funded_ghost_machine_wallet,
+        "get_descriptor",
+        return_value="this is not a descriptor",
+        create=True,
+    ) as m:
+        res = client.post(
+            url,
+            data={"address": receive_address},
+            follow_redirects=True,
+            headers=headers,
+        )
+        assert res.status == "200 OK"
+        assert (
+            res.data.decode()
+            == '{"address":"bcrt1qvtdx75y4554ngrq6aff3xdqnvjhmct5wck95qs","change":false,"descriptor":"this is not a descriptor","index":0,"is_mine":true,"label":null,"service_id":null,"success":false,"used":null,"wallet_name":"ghost_machine","xpubs_descriptor":"this is not a descriptor"}\n'
+        )
+
+
 # Ugly: Code duplication. Cannot import from other test_modules
 def login(client, password):
     """login helper-function"""