test: add unit tests for wallets (funding sources) (lnbits#2363)

* test: initial commit

* feat: allow external label for `create_invoice` (useful for testing)

* chore: code format

* fix: ignore temp coverage files

* feat: add properties to the Status classes for a better readability

* fix: add extra validation for data

* fix: comment out bad `status.pending` (to be fixed in core)

* fix: 404 tests

* test: first draft of generic rest wallet tests

* test: migrate two more tests

* feat: add response type

* feat: test exceptions

* test: extract first `create_invoice` test

* chore: reminder

* add: error test

* chore: code format

* chore: experiment

* feat: adapt parsing

* refactor: data structure

* fix: some tests

* refactor: extract methods

* fix: make response uniform

* fix: test data

* chore: clean-up

* fix: uniform responses

* fix: user agent

* fix: user agent

* fix: user-agent again

* test: add `with error` test

* feat: customize test name

* fix: better exception handling for `status`

* fix: add `try-catch` for `raise_for_status`

* test: with no mocks

* chore: clean-up generalized tests

* chore: code format

* chore: code format

* chore: remove extracted tests

* test: add `create_invoice`: error test

* add: test for `create_invoice` with http 404

* test: extract `test_pay_invoice_ok`

* test: extract `test_pay_invoice_error_response`

* test: extract `test_pay_invoice_http_404`

* test: add "missing data"

* test: add `bad-json`

* test: add `no mocks` for `create_invoice`

* test: add `no mocks` for `pay_invoice`

* test: add `bad json` tests

* chore: re-order tests

* fix: response type

* test: add `missing data` test for `pay_imvoice`

* chore: re-order tests

* test: add `success` test for `get_invoice_status `

* feat: update test structure

* test: new status

* test: add more test

* fix: error handling

* chore: code clean-up

* test: add success test for `get_payment_status `

* test: add `pending` tests for `check_payment_status`

* chore: remove extracted tests

* test: add more tests

* test: add `no mocks` test

* fix: funding source loading

* refactor: extract `rest_wallet_fixtures_from_json` function

* chore: update comment

* feat: cover `cleanup` call also

* chore: code format

* refactor: start to extract data model

* refactor: extract mock class

* fix: typings

* refactor: improve typings

* chore: add some documentation

* chore: final clean-up

* chore: rename file

* chore: `poetry add --dev pytest_httpserver` (after rebase)

.gitignore

@@ -11,6 +11,7 @@ __pycache__
 *.egg
 *.egg-info
 .coverage
+.coverage.*
 .pytest_cache
 .webassets-cache
 htmlcov

lnbits/wallets/base.py

@@ -18,6 +18,18 @@ class InvoiceResponse(NamedTuple):
     payment_request: Optional[str] = None
     error_message: Optional[str] = None
 
+    @property
+    def success(self) -> bool:
+        return self.ok is True
+
+    @property
+    def pending(self) -> bool:
+        return self.ok is None
+
+    @property
+    def failed(self) -> bool:
+        return self.ok is False
+
 
 class PaymentResponse(NamedTuple):
     # when ok is None it means we don't know if this succeeded
@@ -27,12 +39,28 @@ class PaymentResponse(NamedTuple):
     preimage: Optional[str] = None
     error_message: Optional[str] = None
 
+    @property
+    def success(self) -> bool:
+        return self.ok is True
+
+    @property
+    def pending(self) -> bool:
+        return self.ok is None
+
+    @property
+    def failed(self) -> bool:
+        return self.ok is False
+
 
 class PaymentStatus(NamedTuple):
     paid: Optional[bool] = None
     fee_msat: Optional[int] = None
     preimage: Optional[str] = None
 
+    @property
+    def success(self) -> bool:
+        return self.paid is True
+
     @property
     def pending(self) -> bool:
         return self.paid is not True

lnbits/wallets/corelightningrest.py

@@ -66,23 +66,28 @@ async def cleanup(self):
             logger.warning(f"Error closing wallet connection: {e}")
 
     async def status(self) -> StatusResponse:
-        r = await self.client.get(f"{self.url}/v1/channel/localremotebal", timeout=5)
-        r.raise_for_status()
-        if r.is_error or "error" in r.json():
-            try:
-                data = r.json()
-                error_message = data["error"]
-            except Exception:
-                error_message = r.text
-            return StatusResponse(
-                f"Failed to connect to {self.url}, got: '{error_message}...'", 0
+        try:
+            r = await self.client.get(
+                f"{self.url}/v1/channel/localremotebal", timeout=5
             )
+            r.raise_for_status()
+            data = r.json()
 
-        data = r.json()
-        if len(data) == 0:
-            return StatusResponse("no data", 0)
+            if len(data) == 0:
+                return StatusResponse("no data", 0)
 
-        return StatusResponse(None, int(data.get("localBalance") * 1000))
+            if "error" in data:
+                return StatusResponse(f"""Server error: '{data["error"]}'""", 0)
+
+            if r.is_error or "localBalance" not in data:
+                return StatusResponse(f"Server error: '{r.text}'", 0)
+
+            return StatusResponse(None, int(data.get("localBalance") * 1000))
+        except json.JSONDecodeError:
+            return StatusResponse("Server error: 'invalid json response'", 0)
+        except Exception as exc:
+            logger.warning(exc)
+            return StatusResponse(f"Unable to connect to {self.url}.", 0)
 
     async def create_invoice(
         self,
@@ -92,7 +97,7 @@ async def create_invoice(
         unhashed_description: Optional[bytes] = None,
         **kwargs,
     ) -> InvoiceResponse:
-        label = f"lbl{random.random()}"
+        label = kwargs.get("label", f"lbl{random.random()}")
         data: Dict = {
             "amount": amount * 1000,
             "description": memo,
@@ -113,24 +118,41 @@ async def create_invoice(
         if kwargs.get("preimage"):
             data["preimage"] = kwargs["preimage"]
 
-        r = await self.client.post(
-            f"{self.url}/v1/invoice/genInvoice",
-            data=data,
-        )
+        try:
+            r = await self.client.post(
+                f"{self.url}/v1/invoice/genInvoice",
+                data=data,
+            )
+            r.raise_for_status()
 
-        if r.is_error or "error" in r.json():
-            try:
-                data = r.json()
-                error_message = data["error"]
-            except Exception:
-                error_message = r.text
+            data = r.json()
 
-            return InvoiceResponse(False, None, None, error_message)
+            if len(data) == 0:
+                return InvoiceResponse(False, None, None, "no data")
 
-        data = r.json()
-        assert "payment_hash" in data
-        assert "bolt11" in data
-        return InvoiceResponse(True, data["payment_hash"], data["bolt11"], None)
+            if "error" in data:
+                return InvoiceResponse(
+                    False, None, None, f"""Server error: '{data["error"]}'"""
+                )
+
+            if r.is_error:
+                return InvoiceResponse(False, None, None, f"Server error: '{r.text}'")
+
+            if "payment_hash" not in data or "bolt11" not in data:
+                return InvoiceResponse(
+                    False, None, None, "Server error: 'missing required fields'"
+                )
+
+            return InvoiceResponse(True, data["payment_hash"], data["bolt11"], None)
+        except json.JSONDecodeError:
+            return InvoiceResponse(
+                False, None, None, "Server error: 'invalid json response'"
+            )
+        except Exception as exc:
+            logger.warning(exc)
+            return InvoiceResponse(
+                False, None, None, f"Unable to connect to {self.url}."
+            )
 
     async def pay_invoice(self, bolt11: str, fee_limit_msat: int) -> PaymentResponse:
         try:
@@ -142,34 +164,53 @@ async def pay_invoice(self, bolt11: str, fee_limit_msat: int) -> PaymentResponse
             error_message = "0 amount invoices are not allowed"
             return PaymentResponse(False, None, None, None, error_message)
         fee_limit_percent = fee_limit_msat / invoice.amount_msat * 100
-        r = await self.client.post(
-            f"{self.url}/v1/pay",
-            data={
-                "invoice": bolt11,
-                "maxfeepercent": f"{fee_limit_percent:.11}",
-                "exemptfee": 0,  # so fee_limit_percent is applied even on payments
-                # with fee < 5000 millisatoshi (which is default value of exemptfee)
-            },
-            timeout=None,
-        )
+        try:
+            r = await self.client.post(
+                f"{self.url}/v1/pay",
+                data={
+                    "invoice": bolt11,
+                    "maxfeepercent": f"{fee_limit_percent:.11}",
+                    "exemptfee": 0,  # so fee_limit_percent is applied even on payments
+                    # with fee < 5000 millisatoshi (which is default value of exemptfee)
+                },
+                timeout=None,
+            )
 
-        if r.is_error or "error" in r.json():
-            try:
-                data = r.json()
-                error_message = data["error"]
-            except Exception:
-                error_message = r.text
-            return PaymentResponse(False, None, None, None, error_message)
+            r.raise_for_status()
+            data = r.json()
 
-        data = r.json()
+            if "error" in data:
+                return PaymentResponse(False, None, None, None, data["error"])
+            if r.is_error:
+                return PaymentResponse(False, None, None, None, r.text)
+            if (
+                "payment_hash" not in data
+                or "payment_preimage" not in data
+                or "msatoshi_sent" not in data
+                or "msatoshi" not in data
+                or "status" not in data
+            ):
+                return PaymentResponse(
+                    False, None, None, None, "Server error: 'missing required fields'"
+                )
 
-        checking_id = data["payment_hash"]
-        preimage = data["payment_preimage"]
-        fee_msat = data["msatoshi_sent"] - data["msatoshi"]
+            checking_id = data["payment_hash"]
+            preimage = data["payment_preimage"]
+            fee_msat = data["msatoshi_sent"] - data["msatoshi"]
 
-        return PaymentResponse(
-            self.statuses.get(data["status"]), checking_id, fee_msat, preimage, None
-        )
+            return PaymentResponse(
+                self.statuses.get(data["status"]), checking_id, fee_msat, preimage, None
+            )
+        except json.JSONDecodeError:
+            return PaymentResponse(
+                False, None, None, None, "Server error: 'invalid json response'"
+            )
+        except Exception as exc:
+            logger.info(f"Failed to pay invoice {bolt11}")
+            logger.warning(exc)
+            return PaymentResponse(
+                False, None, None, None, f"Unable to connect to {self.url}."
+            )
 
     async def get_invoice_status(self, checking_id: str) -> PaymentStatus:
         r = await self.client.get(

lnbits/wallets/lndrest.py

@@ -87,15 +87,20 @@ async def status(self) -> StatusResponse:
         try:
             r = await self.client.get("/v1/balance/channels")
             r.raise_for_status()
-        except (httpx.ConnectError, httpx.RequestError) as exc:
-            return StatusResponse(f"Unable to connect to {self.endpoint}. {exc}", 0)
 
-        try:
             data = r.json()
-            if r.is_error:
-                raise Exception
-        except Exception:
-            return StatusResponse(r.text[:200], 0)
+
+            if len(data) == 0:
+                return StatusResponse("no data", 0)
+
+            if r.is_error or "balance" not in data:
+                return StatusResponse(f"Server error: '{r.text}'", 0)
+
+        except json.JSONDecodeError:
+            return StatusResponse("Server error: 'invalid json response'", 0)
+        except Exception as exc:
+            logger.warning(exc)
+            return StatusResponse(f"Unable to connect to {self.endpoint}.", 0)
 
         return StatusResponse(None, int(data["balance"]) * 1000)
 
@@ -123,22 +128,41 @@ async def create_invoice(
                 hashlib.sha256(unhashed_description).digest()
             ).decode("ascii")
 
-        r = await self.client.post(url="/v1/invoices", json=data)
+        try:
+            r = await self.client.post(url="/v1/invoices", json=data)
+            r.raise_for_status()
+            data = r.json()
 
-        if r.is_error:
-            error_message = r.text
-            try:
-                error_message = r.json()["error"]
-            except Exception:
-                pass
-            return InvoiceResponse(False, None, None, error_message)
+            if len(data) == 0:
+                return InvoiceResponse(False, None, None, "no data")
+
+            if "error" in data:
+                return InvoiceResponse(
+                    False, None, None, f"""Server error: '{data["error"]}'"""
+                )
+
+            if r.is_error:
+                return InvoiceResponse(False, None, None, f"Server error: '{r.text}'")
+
+            if "payment_request" not in data or "r_hash" not in data:
+                return InvoiceResponse(
+                    False, None, None, "Server error: 'missing required fields'"
+                )
 
-        data = r.json()
-        payment_request = data["payment_request"]
-        payment_hash = base64.b64decode(data["r_hash"]).hex()
-        checking_id = payment_hash
+            payment_request = data["payment_request"]
+            payment_hash = base64.b64decode(data["r_hash"]).hex()
+            checking_id = payment_hash
 
-        return InvoiceResponse(True, checking_id, payment_request, None)
+            return InvoiceResponse(True, checking_id, payment_request, None)
+        except json.JSONDecodeError:
+            return InvoiceResponse(
+                False, None, None, "Server error: 'invalid json response'"
+            )
+        except Exception as exc:
+            logger.warning(exc)
+            return InvoiceResponse(
+                False, None, None, f"Unable to connect to {self.endpoint}."
+            )
 
     async def pay_invoice(self, bolt11: str, fee_limit_msat: int) -> PaymentResponse:
         # set the fee limit for the payment
@@ -154,29 +178,53 @@ async def pay_invoice(self, bolt11: str, fee_limit_msat: int) -> PaymentResponse
             r.raise_for_status()
         except Exception as exc:
             logger.warning(f"LndRestWallet pay_invoice POST error: {exc}.")
-            return PaymentResponse(None, None, None, None, str(exc))
+            return PaymentResponse(
+                None, None, None, None, f"Unable to connect to {self.endpoint}."
+            )
 
-        data = r.json()
+        try:
+            data = r.json()
 
-        if data.get("payment_error"):
-            error_message = r.json().get("payment_error") or r.text
-            logger.warning(f"LndRestWallet pay_invoice payment_error: {error_message}.")
-            return PaymentResponse(False, None, None, None, error_message)
+            if data.get("payment_error"):
+                error_message = r.json().get("payment_error") or r.text
+                logger.warning(
+                    f"LndRestWallet pay_invoice payment_error: {error_message}."
+                )
+                return PaymentResponse(False, None, None, None, error_message)
+
+            if (
+                "payment_hash" not in data
+                or "payment_route" not in data
+                or "total_fees_msat" not in data["payment_route"]
+                or "payment_preimage" not in data
+            ):
+                return PaymentResponse(
+                    False, None, None, None, "Server error: 'missing required fields'"
+                )
 
-        data = r.json()
-        checking_id = base64.b64decode(data["payment_hash"]).hex()
-        fee_msat = int(data["payment_route"]["total_fees_msat"])
-        preimage = base64.b64decode(data["payment_preimage"]).hex()
-        return PaymentResponse(True, checking_id, fee_msat, preimage, None)
+            checking_id = base64.b64decode(data["payment_hash"]).hex()
+            fee_msat = int(data["payment_route"]["total_fees_msat"])
+            preimage = base64.b64decode(data["payment_preimage"]).hex()
+            return PaymentResponse(True, checking_id, fee_msat, preimage, None)
+        except json.JSONDecodeError:
+            return PaymentResponse(
+                False, None, None, None, "Server error: 'invalid json response'"
+            )
 
     async def get_invoice_status(self, checking_id: str) -> PaymentStatus:
         r = await self.client.get(url=f"/v1/invoice/{checking_id}")
 
-        if r.is_error or not r.json().get("settled"):
-            # this must also work when checking_id is not a hex recognizable by lnd
-            # it will return an error and no "settled" attribute on the object
-            return PaymentPendingStatus()
+        try:
+            r.raise_for_status()
+            data = r.json()
 
+            if r.is_error or not data.get("settled"):
+                # this must also work when checking_id is not a hex recognizable by lnd
+                # it will return an error and no "settled" attribute on the object
+                return PaymentPendingStatus()
+        except Exception as e:
+            logger.error(f"Error getting invoice status: {e}")
+            return PaymentPendingStatus()
         return PaymentSuccessStatus()
 
     async def get_payment_status(self, checking_id: str) -> PaymentStatus: