Add deleted flag wallet (#1826)

* add deleted flag on wallets

set deleted on delete wallet

2 twelves

format

fail on create invoice

make deleted check on SQL query

nazi flake8

add_test

boom... it works and passes!!

* add app fixture

vlad's recommendations

add deleted

* Add deleted flag to Wallet

* restore crud

* do not check for wallet in services.py

* add deleted flag on wallets

set deleted on delete wallet

2 twelves

format

fail on create invoice

make deleted check on SQL query

nazi flake8

add_test

boom... it works and passes!!

* add app fixture

vlad's recommendations

* add deleted

* error checks

---------

Co-authored-by: callebtc <93376500+callebtc@users.noreply.github.com>

lnbits/core/crud.py

@@ -268,11 +268,8 @@ async def delete_wallet(
 ) -> None:
     await (conn or db).execute(
         """
-        UPDATE wallets AS w
-        SET
-            "user" = 'del:' || w."user",
-            adminkey = 'del:' || w.adminkey,
-            inkey = 'del:' || w.inkey
+        UPDATE wallets
+        SET deleted = true
         WHERE id = ? AND "user" = ?
         """,
         (wallet_id, user_id),

lnbits/core/migrations.py

@@ -327,3 +327,54 @@ async def m012_add_currency_to_wallet(db):
         ALTER TABLE wallets ADD COLUMN currency TEXT
         """
     )
+
+
+async def m013_add_deleted_to_wallets(db):
+    """
+    Adds deleted column to wallets.
+    """
+    try:
+        await db.execute(
+            "ALTER TABLE wallets ADD COLUMN deleted BOOLEAN NOT NULL DEFAULT false"
+        )
+    except OperationalError:
+        pass
+
+
+async def m014_set_deleted_wallets(db):
+    """
+    Sets deleted column to wallets.
+    """
+    try:
+        rows = await (
+            await db.execute(
+                """
+                SELECT *
+                FROM wallets
+                WHERE user LIKE 'del:%'
+                AND adminkey LIKE 'del:%'
+                AND inkey LIKE 'del:%'
+                """
+            )
+        ).fetchall()
+
+        for row in rows:
+            try:
+                user = row[2].split(":")[1]
+                adminkey = row[3].split(":")[1]
+                inkey = row[4].split(":")[1]
+                await db.execute(
+                    """
+                    UPDATE wallets SET user = ?, adminkey = ?, inkey = ?, deleted = true
+                    WHERE id = ?
+                    """,
+                    (user, adminkey, inkey, row[0]),
+                )
+            except Exception:
+                continue
+    except OperationalError:
+        # this is necessary now because it may be the case that this migration will
+        # run twice in some environments.
+        # catching errors like this won't be necessary in anymore now that we
+        # keep track of db versions so no migration ever runs twice.
+        pass

lnbits/core/models.py

@@ -29,6 +29,7 @@ class Wallet(BaseModel):
     inkey: str
     currency: Optional[str]
     balance_msat: int
+    deleted: bool
 
     @property
     def balance(self) -> int:

lnbits/core/services.py

@@ -110,6 +110,9 @@ async def create_invoice(
     if not amount > 0:
         raise InvoiceFailure("Amountless invoices not supported.")
 
+    if await get_wallet(wallet_id, conn=conn) is None:
+        raise InvoiceFailure("Wallet does not exist.")
+
     invoice_memo = None if description_hash else memo
 
     # use the fake wallet if the invoice is for internal use only

lnbits/core/views/api.py

@@ -466,7 +466,7 @@ async def api_payment(payment_hash, X_Api_Key: Optional[str] = Header(None)):
     # We use X_Api_Key here because we want this call to work with and without keys
     # If a valid key is given, we also return the field "details", otherwise not
     wallet = await get_wallet_for_key(X_Api_Key) if isinstance(X_Api_Key, str) else None
-
+    wallet = wallet if wallet and not wallet.deleted else None
     # we have to specify the wallet id here, because postgres and sqlite return
     # internal payments in different order and get_standalone_payment otherwise
     # just fetches the first one, causing unpredictable results

lnbits/decorators.py

@@ -53,12 +53,12 @@ class KeyChecker(SecurityBase):
             #        avoided here. Also, we should not return the wallet here - thats
             #        silly. Possibly store it in a Redis DB
             wallet = await get_wallet_for_key(key_value, self._key_type)
-            self.wallet = wallet  # type: ignore
-            if not wallet:
+            if not wallet or wallet.deleted:
                 raise HTTPException(
                     status_code=HTTPStatus.UNAUTHORIZED,
-                    detail="Invalid key or expired key.",
+                    detail="Invalid key or wallet.",
                 )
+            self.wallet = wallet  # type: ignore
         except KeyError:
             raise HTTPException(
                 status_code=HTTPStatus.BAD_REQUEST, detail="`X-API-KEY` header missing."

pyproject.toml

@@ -95,7 +95,7 @@ module = [
 ignore_missing_imports = "True"
 
 [tool.pytest.ini_options]
-log_cli = true
+log_cli = false
 addopts = "--durations=1 -s --cov=lnbits --cov-report=xml"
 testpaths = [
   "tests"

tests/core/test_db.py

@@ -2,6 +2,12 @@ from datetime import date
 
 import pytest
 
+from lnbits.core.crud import (
+    create_wallet,
+    delete_wallet,
+    get_wallet,
+    get_wallet_for_key,
+)
 from lnbits.db import POSTGRES
 
 
@@ -10,3 +16,23 @@ async def test_date_conversion(db):
     if db.type == POSTGRES:
         row = await db.fetchone("SELECT now()::date")
         assert row and isinstance(row[0], date)
+
+
+# make test to create wallet and delete wallet
+@pytest.mark.asyncio
+async def test_create_wallet_and_delete_wallet(app, to_user):
+    # create wallet
+    wallet = await create_wallet(user_id=to_user.id, wallet_name="test_wallet_delete")
+    assert wallet
+
+    # delete wallet
+    await delete_wallet(user_id=to_user.id, wallet_id=wallet.id)
+
+    # check if wallet is deleted
+    del_wallet = await get_wallet(wallet.id)
+    assert del_wallet is not None
+    assert del_wallet.deleted is True
+
+    del_wallet = await get_wallet_for_key(wallet.inkey)
+    assert del_wallet is not None
+    assert del_wallet.deleted is True