Index lightning requests with rhash instead of onchain address.

get_unused_addresses() has been broken since #7730, because
addresses are considered as permanently used if they are in
the list of keys of receive_requests. This is true even if
an address is used as fallback for a lightning payment. This
means that the number of lightning payments we can receive
is constrained by the gap limit.

If a payment succeeds off-chain, we want to be able to reuse
its fallback address in other requests (this does not reduce
privacy, because invoices already share the same public key).

This implies that we should not use the onchain address as key
for lightning-enabled requests in wallet.receive_requests. If
we did, paid invoices would be overwritten when the address is
reused. That is the reason for the wallet_db upgrade.

Related: a3faf85e3c

electrum/lnworker.py

@@ -1919,10 +1919,10 @@ class LNWallet(LNWorker):
         if self.get_payment_status(payment_hash) == status:
             return
         self.set_payment_status(payment_hash, status)
-        req = self.wallet.get_request_by_rhash(payment_hash.hex())
+        key = payment_hash.hex()
+        req = self.wallet.get_request(key)
         if req is None:
             return
-        key = self.wallet.get_key_for_receive_request(req)
         util.trigger_callback('request_status', self.wallet, key, status)
 
     def set_payment_status(self, payment_hash: bytes, status: int) -> None:

electrum/tests/test_lnpeer.py

@@ -105,7 +105,7 @@ class MockWallet:
     receive_requests = {}
     adb = MockADB()
 
-    def get_request_by_rhash(self, rhash):
+    def get_request(self, key):
         pass
 
     def get_key_for_receive_request(self, x):

electrum/wallet.py

@@ -968,7 +968,7 @@ class Abstract_Wallet(ABC, Logger):
 
     def clear_requests(self):
         self.receive_requests.clear()
-        self._requests_rhash_to_key.clear()
+        self._requests_addr_to_rhash.clear()
         self.save_db()
 
     def get_invoices(self):
@@ -1020,10 +1020,10 @@ class Abstract_Wallet(ABC, Logger):
         return invoices
 
     def _init_requests_rhash_index(self):
-        self._requests_rhash_to_key = {}
+        self._requests_addr_to_rhash = {}
         for key, req in self.receive_requests.items():
-            if req.is_lightning():
+            if req.is_lightning() and (addr:=req.get_address()):
-                self._requests_rhash_to_key[req.rhash] = key
+                self._requests_addr_to_rhash[addr] = req.rhash
 
     def _prepare_onchain_invoice_paid_detection(self):
         # scriptpubkey -> list(invoice_keys)
@@ -1335,7 +1335,7 @@ class Abstract_Wallet(ABC, Logger):
         return ''
 
     def _get_default_label_for_rhash(self, rhash: str) -> str:
-        req = self.get_request_by_rhash(rhash)
+        req = self.get_request(rhash)
         return req.message if req else ''
 
     def get_label_for_rhash(self, rhash: str) -> str:
@@ -2219,9 +2219,7 @@ class Abstract_Wallet(ABC, Logger):
 
     def get_unused_addresses(self) -> Sequence[str]:
         domain = self.get_receiving_addresses()
-        # TODO we should index receive_requests by id
+        in_use_by_request = set(req.get_address() for req in self.get_unpaid_requests())
-        # add lightning requests. (use as key)
-        in_use_by_request = set(self.receive_requests.keys())
         return [addr for addr in domain if not self.adb.is_used(addr)
                 and addr not in in_use_by_request]
 
@@ -2342,11 +2340,12 @@ class Abstract_Wallet(ABC, Logger):
         return self.check_expired_status(r, status)
 
     def get_request(self, key):
-        return self.receive_requests.get(key) or self.get_request_by_rhash(key)
+        return self.receive_requests.get(key) or self.get_request_by_address(key)
 
-    def get_request_by_rhash(self, rhash):
+    def get_request_by_address(self, addr):
-        key = self._requests_rhash_to_key.get(rhash)
+        rhash = self._requests_addr_to_rhash.get(addr)
-        return self.receive_requests.get(key) if key else None
+        if rhash:
+            return self.receive_requests.get(rhash)
 
     def get_formatted_request(self, key):
         x = self.get_request(key)
@@ -2484,16 +2483,15 @@ class Abstract_Wallet(ABC, Logger):
                     raise Exception(_('Address not in wallet.'))
             key = addr
         else:
-            addr = req.get_address()
+            key = req.rhash
-            key = req.rhash if addr is None else addr
         return key
 
     def add_payment_request(self, req: Invoice, *, write_to_disk: bool = True):
         key = self.get_key_for_receive_request(req, sanity_checks=True)
         message = req.message
         self.receive_requests[key] = req
-        if req.is_lightning():
+        if req.is_lightning() and (addr:=req.get_address()):
-            self._requests_rhash_to_key[req.rhash] = key
+            self._requests_addr_to_rhash[addr] = req.rhash
         if write_to_disk:
             self.save_db()
         return key
@@ -2503,8 +2501,8 @@ class Abstract_Wallet(ABC, Logger):
         req = self.receive_requests.pop(key, None)
         if req is None:
             return
-        if req.is_lightning():
+        if req.is_lightning() and (addr:=req.get_address()):
-            self._requests_rhash_to_key.pop(req.rhash)
+            self._requests_addr_to_rhash.pop(addr)
         if req.is_lightning() and self.lnworker:
             self.lnworker.delete_payment_info(req.rhash)
         self.save_db()

electrum/wallet_db.py

@@ -52,7 +52,7 @@ if TYPE_CHECKING:
 
 OLD_SEED_VERSION = 4        # electrum versions < 2.0
 NEW_SEED_VERSION = 11       # electrum versions >= 2.0
-FINAL_SEED_VERSION = 46     # electrum >= 2.7 will set this to prevent
+FINAL_SEED_VERSION = 47     # electrum >= 2.7 will set this to prevent
                             # old versions from overwriting new format
 
 
@@ -195,6 +195,7 @@ class WalletDB(JsonDB):
         self._convert_version_44()
         self._convert_version_45()
         self._convert_version_46()
+        self._convert_version_47()
         self.put('seed_version', FINAL_SEED_VERSION)  # just to be sure
 
         self._after_upgrade_tasks()
@@ -925,6 +926,22 @@ class WalletDB(JsonDB):
                 del invoices[key]
         self.data['seed_version'] = 46
 
+    def _convert_version_47(self):
+        from .lnaddr import lndecode
+        if not self._is_upgrade_method_needed(46, 46):
+            return
+        # recalc keys of requests
+        requests = self.data.get('payment_requests', {})
+        for key, item in list(requests.items()):
+            lnaddr = item.get('lightning_invoice')
+            if lnaddr:
+                lnaddr = lndecode(lnaddr)
+                rhash = lnaddr.paymenthash.hex()
+                if key != rhash:
+                    requests[rhash] = item
+                    del requests[key]
+        self.data['seed_version'] = 47
+
     def _convert_imported(self):
         if not self._is_upgrade_method_needed(0, 13):
             return