From 9fe93524b770051da85023f104fc9186621efdb0 Mon Sep 17 00:00:00 2001 From: ThomasV Date: Wed, 15 Jun 2022 16:24:29 +0200 Subject: [PATCH] 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: a3faf85e3cce83f90d5a3af8e0fd48f06ce0cabd --- electrum/lnworker.py | 4 ++-- electrum/tests/test_lnpeer.py | 2 +- electrum/wallet.py | 34 ++++++++++++++++------------------ electrum/wallet_db.py | 19 ++++++++++++++++++- 4 files changed, 37 insertions(+), 22 deletions(-) diff --git a/electrum/lnworker.py b/electrum/lnworker.py index b2c790aa9..da29f52fd 100644 --- a/electrum/lnworker.py +++ b/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: diff --git a/electrum/tests/test_lnpeer.py b/electrum/tests/test_lnpeer.py index 5e1c83940..2a8a9be54 100644 --- a/electrum/tests/test_lnpeer.py +++ b/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): diff --git a/electrum/wallet.py b/electrum/wallet.py index 04fd13868..8740fd463 100644 --- a/electrum/wallet.py +++ b/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(): - self._requests_rhash_to_key[req.rhash] = key + if req.is_lightning() and (addr:=req.get_address()): + 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 - # add lightning requests. (use as key) - in_use_by_request = set(self.receive_requests.keys()) + in_use_by_request = set(req.get_address() for req in self.get_unpaid_requests()) 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): - key = self._requests_rhash_to_key.get(rhash) - return self.receive_requests.get(key) if key else None + def get_request_by_address(self, addr): + rhash = self._requests_addr_to_rhash.get(addr) + 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 if addr is None else addr + key = req.rhash 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(): - self._requests_rhash_to_key[req.rhash] = key + if req.is_lightning() and (addr:=req.get_address()): + 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(): - self._requests_rhash_to_key.pop(req.rhash) + if req.is_lightning() and (addr:=req.get_address()): + self._requests_addr_to_rhash.pop(addr) if req.is_lightning() and self.lnworker: self.lnworker.delete_payment_info(req.rhash) self.save_db() diff --git a/electrum/wallet_db.py b/electrum/wallet_db.py index 08b3caf7d..01c85534d 100644 --- a/electrum/wallet_db.py +++ b/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