Repository: spesmilo/electrum
Issue: invoice-key mismatch for pre-4.0.8 outgoing onchain invoices

On master, in the Qt `Send` tab, right-clicking an old onchain invoice results in an error:
```
Traceback (most recent call last):
  File "/home/user/wspace/electrum/electrum/gui/qt/invoice_list.py", line 155, in create_menu
    if invoice.is_lightning():
AttributeError: 'NoneType' object has no attribute 'is_lightning'
```

This is because around version 4.0.8, the key for outgoing invoices was changed: https://github.com/spesmilo/electrum/commit/cdfaaa260942b807f809c2c0414fb242a03e945a

such example invoices I have saved in a wallet:
```
a1. inv=Invoice(amount_msat=100000000, message='', time=1591209612, exp=0, outputs=[<TxOutput script=00140f4dcc26e0ac0ec1890223c4b8c5b48808cc1d67 address=tb1qpaxucfhq4s8vrzgzy0zt33d53qyvc8t8pnqg8n value=100000>], height=0, bip70=None, lightning_invoice=None)
a2. realkey='654b80e92a5032f63c251cad2ce8d109' expected_key='bcc8ea937f'
a1. inv=Invoice(amount_msat=1000000000, message='', time=1592943967, exp=0, outputs=[<TxOutput script=6a0401020304 address=None value=0>, <TxOutput script=ac21020202020202020202020202020202020202020202020202020202020202020202 address=None value=1000000>], height=0, bip70=None, lightning_invoice=None)
a2. realkey='35c7f061df3f00e195af3b205492468e' expected_key='18e079e36e'
a1. inv=Invoice(amount_msat=5500000000000, message='', time=1593687959, exp=0, outputs=[<TxOutput script=0014ec8f27fdb283bfb5f76937e3640c1e4631e9827c address=tb1qaj8j0ldjswlmtamfxl3kgrq7gcc7nqnun4dwn6 value=5500000000>], height=0, bip70=None, lightning_invoice=None)
a2. realkey='849c7f4067a248ad1dfd9a3e8c62b29c' expected_key='8d9fdab586'
```

Until recently (https://github.com/spesmilo/electrum/commit/e392197ab94715815c00503d8fd5cb881d0a26c3), there was an "id" field stored in invoices, but now we are computing the id on demand using the "new algorithm", which does not work for pre-4.0.8 invoices.

Changed files:
electrum/paymentrequest.py, electrum/gui/qt/main_window.py, electrum/wallet.py, electrum/invoices.py, electrum/wallet_db.py, electrum/gui/kivy/main_window.py

diff --git a/electrum/gui/kivy/main_window.py b/electrum/gui/kivy/main_window.py
index ae9f82f0a..ce8d1ca72 100644
--- a/electrum/gui/kivy/main_window.py
+++ b/electrum/gui/kivy/main_window.py
@@ -19,7 +19,7 @@ from electrum import util
 from electrum.util import (profiler, InvalidPassword, send_exception_to_crash_reporter,
                            format_satoshis, format_satoshis_plain, format_fee_satoshis,
                            maybe_extract_bolt11_invoice, parse_max_spend)
-from electrum.invoices import PR_PAID, PR_FAILED
+from electrum.invoices import PR_PAID, PR_FAILED, Invoice
 from electrum import blockchain
 from electrum.network import Network, TxBroadcastError, BestEffortRequestFailed
 from electrum.interface import PREFERRED_NETWORK_PROTOCOL, ServerAddr
@@ -447,8 +447,7 @@ class ElectrumWindow(App, Logger):
             self.show_error(_('No wallet loaded.'))
             return
         if pr.verify(self.wallet.contacts):
-            key = pr.get_id()
-            invoice = self.wallet.get_invoice(key)  # FIXME wrong key...
+            invoice = Invoice.from_bip70_payreq(pr, height=0)
             if invoice and self.wallet.get_invoice_status(invoice) == PR_PAID:
                 self.show_error("invoice already paid")
                 self.send_screen.do_clear()
diff --git a/electrum/gui/qt/main_window.py b/electrum/gui/qt/main_window.py
index 93de73f80..1c9f72240 100644
--- a/electrum/gui/qt/main_window.py
+++ b/electrum/gui/qt/main_window.py
@@ -2092,9 +2092,8 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger):
         pr = self.payment_request
         if not pr:
             return
-        key = pr.get_id()
-        invoice = self.wallet.get_invoice(key)
-        if invoice and self.wallet.get_invoice_status(invoice) == PR_PAID:
+        invoice = Invoice.from_bip70_payreq(pr, height=0)
+        if self.wallet.get_invoice_status(invoice) == PR_PAID:
             self.show_message("invoice already paid")
             self.do_clear()
             self.payment_request = None
@@ -2333,8 +2332,8 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger):
             grid.addWidget(QLabel(_("Signature") + ':'), 6, 0)
             grid.addWidget(QLabel(pr.get_verify_status()), 6, 1)
             def do_export():
-                key = pr.get_id()
-                name = str(key) + '.bip70'
+                name = pr.get_name_for_export() or "payment_request"
+                name = f"{name}.bip70"
                 fn = getSaveFileName(
                     parent=self,
                     title=_("Save invoice to file"),
diff --git a/electrum/invoices.py b/electrum/invoices.py
index 7bf2eee6a..f9e13538d 100644
--- a/electrum/invoices.py
+++ b/electrum/invoices.py
@@ -12,6 +12,7 @@ from . import constants
 from .bitcoin import COIN, TOTAL_COIN_SUPPLY_LIMIT_IN_BTC
 from .bitcoin import address_to_script
 from .transaction import PartialTxOutput
+from .crypto import sha256d
 
 if TYPE_CHECKING:
     from .paymentrequest import PaymentRequest
@@ -65,7 +66,7 @@ pr_expiration_values = {
 assert PR_DEFAULT_EXPIRATION_WHEN_CREATING in pr_expiration_values
 
 
-def _decode_outputs(outputs) -> List[PartialTxOutput]:
+def _decode_outputs(outputs) -> Optional[List[PartialTxOutput]]:
     if outputs is None:
         return None
     ret = []
@@ -98,13 +99,13 @@ class Invoice(StoredObject):
     # an invoice (outgoing) is constructed from a source: bip21, bip70, lnaddr
 
     # onchain only
-    outputs = attr.ib(kw_only=True, converter=_decode_outputs)  # type: List[PartialTxOutput]
+    outputs = attr.ib(kw_only=True, converter=_decode_outputs)  # type: Optional[List[PartialTxOutput]]
     height = attr.ib(type=int, kw_only=True, validator=attr.validators.instance_of(int)) # only for receiving
     bip70 = attr.ib(type=str, kw_only=True)  # type: Optional[str]
     #bip70_requestor = attr.ib(type=str, kw_only=True)  # type: Optional[str]
 
     # lightning only
-    lightning_invoice = attr.ib(type=str, kw_only=True)
+    lightning_invoice = attr.ib(type=str, kw_only=True)  # type: Optional[str]
 
     __lnaddr = None
 
@@ -227,7 +228,7 @@ class Invoice(StoredObject):
         )
 
     @classmethod
-    def from_bip70_payreq(cls, pr: 'PaymentRequest', height:int) -> 'Invoice':
+    def from_bip70_payreq(cls, pr: 'PaymentRequest', *, height: int = 0) -> 'Invoice':
         return Invoice(
             amount_msat=pr.get_amount()*1000,
             message=pr.get_memo(),
@@ -251,3 +252,14 @@ class Invoice(StoredObject):
             # 'tags': str(lnaddr.tags),
         })
         return d
+
+    def get_id(self) -> str:
+        if self.is_lightning():
+            return self.rhash
+        else:  # on-chain
+            return get_id_from_onchain_outputs(outputs=self.get_outputs(), timestamp=self.time)
+
+
+def get_id_from_onchain_outputs(outputs: List[PartialTxOutput], *, timestamp: int) -> str:
+    outputs_str = "\\n".join(f"{txout.scriptpubkey.hex()}, {txout.value}" for txout in outputs)
+    return sha256d(outputs_str + "%d" % timestamp).hex()[0:10]
diff --git a/electrum/paymentrequest.py b/electrum/paymentrequest.py
index d6ac9ca15..550797a56 100644
--- a/electrum/paymentrequest.py
+++ b/electrum/paymentrequest.py
@@ -41,7 +41,7 @@ except ImportError:
 
 from . import bitcoin, constants, ecc, util, transaction, x509, rsakey
 from .util import bh2u, bfh, make_aiohttp_session
-from .invoices import Invoice
+from .invoices import Invoice, get_id_from_onchain_outputs
 from .crypto import sha256
 from .bitcoin import address_to_script
 from .transaction import PartialTxOutput
@@ -121,7 +121,7 @@ async def get_payment_request(url: str) -> 'PaymentRequest':
 
 class PaymentRequest:
 
-    def __init__(self, data, *, error=None):
+    def __init__(self, data: bytes, *, error=None):
         self.raw = data
         self.error = error  # FIXME overloaded and also used when 'verify' succeeds
         self.parse(data)
@@ -131,11 +131,10 @@ class PaymentRequest:
     def __str__(self):
         return str(self.raw)
 
-    def parse(self, r):
+    def parse(self, r: bytes):
         self.outputs = []  # type: List[PartialTxOutput]
         if self.error:
             return
-        self.id = bh2u(sha256(r)[0:16])
         try:
             self.data = pb2.PaymentRequest()
             self.data.ParseFromString(r)
@@ -275,8 +274,10 @@ class PaymentRequest:
     def get_memo(self):
         return self.memo
 
-    def get_id(self):
-        return self.id if self.requestor else self.get_address()
+    def get_name_for_export(self) -> Optional[str]:
+        if not hasattr(self, 'details'):
+            return None
+        return get_id_from_onchain_outputs(self.outputs, timestamp=self.get_time())
 
     def get_outputs(self):
         return self.outputs[:]
diff --git a/electrum/wallet.py b/electrum/wallet.py
index b0c78c48e..f6867a324 100644
--- a/electrum/wallet.py
+++ b/electrum/wallet.py
@@ -773,9 +773,9 @@ class Abstract_Wallet(AddressSynchronizer, ABC):
             }
 
     def create_invoice(self, *, outputs: List[PartialTxOutput], message, pr, URI) -> Invoice:
-        height=self.get_local_height()
+        height = self.get_local_height()
         if pr:
-            return Invoice.from_bip70_payreq(pr, height)
+            return Invoice.from_bip70_payreq(pr, height=height)
         amount_msat = 0
         for x in outputs:
             if parse_max_spend(x.value):
@@ -2380,11 +2380,7 @@ class Abstract_Wallet(AddressSynchronizer, ABC):
     @classmethod
     def get_key_for_outgoing_invoice(cls, invoice: Invoice) -> str:
         """Return the key to use for this invoice in self.invoices."""
-        if invoice.is_lightning():
-            key = invoice.rhash
-        else:
-            key = bh2u(sha256d(repr(invoice.get_outputs()) + "%d"%invoice.time))[0:10]
-        return key
+        return invoice.get_id()
 
     def get_key_for_receive_request(self, req: Invoice, *, sanity_checks: bool = False) -> str:
         """Return the key to use for this invoice in self.receive_requests."""
diff --git a/electrum/wallet_db.py b/electrum/wallet_db.py
index 6c84c7291..c9c25d04e 100644
--- a/electrum/wallet_db.py
+++ b/electrum/wallet_db.py
@@ -42,7 +42,6 @@ from .lnutil import ImportedChannelBackupStorage, OnchainChannelBackupStorage
 from .lnutil import ChannelConstraints, Outpoint, ShachainElement
 from .json_db import StoredDict, JsonDB, locked, modifier
 from .plugin import run_hook, plugin_loaders
-from .paymentrequest import PaymentRequest
 from .submarine_swaps import SwapData
 
 if TYPE_CHECKING:
@@ -53,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 = 45     # electrum >= 2.7 will set this to prevent
+FINAL_SEED_VERSION = 46     # electrum >= 2.7 will set this to prevent
                             # old versions from overwriting new format
 
 
@@ -194,6 +193,7 @@ class WalletDB(JsonDB):
         self._convert_version_43()
         self._convert_version_44()
         self._convert_version_45()
+        self._convert_version_46()
         self.put('seed_version', FINAL_SEED_VERSION)  # just to be sure
 
         self._after_upgrade_tasks()
@@ -558,6 +558,7 @@ class WalletDB(JsonDB):
         self.data['seed_version'] = 24
 
     def _convert_version_25(self):
+        from .crypto import sha256
         if not self._is_upgrade_method_needed(24, 24):
             return
         # add 'type' field to onchain requests
@@ -574,25 +575,15 @@ class WalletDB(JsonDB):
                     'time': r.get('time'),
                     'type': PR_TYPE_ONCHAIN,
                 }
-        # convert bip70 invoices
+        # delete bip70 invoices
+        # note: this upgrade was changed ~2 years after-the-fact to delete instead of converting
         invoices = self.data.get('invoices', {})
         for k, r in list(invoices.items()):
             data = r.get("hex")
-            if data:
-                pr = PaymentRequest(bytes.fromhex(data))
-                if pr.id != k:
-                    continue
-                invoices[k] = {
-                    'type': PR_TYPE_ONCHAIN,
-                    'amount': pr.get_amount(),
-                    'bip70': data,
-                    'exp': pr.get_expiration_date() - pr.get_time(),
-                    'id': pr.id,
-                    'message': pr.get_memo(),
-                    'outputs': [x.to_legacy_tuple() for x in pr.get_outputs()],
-                    'time': pr.get_time(),
-                    'requestor': pr.get_requestor(),
-                }
+            pr_id = sha256(bytes.fromhex(data))[0:16].hex()
+            if pr_id != k:
+                continue
+            del invoices[k]
         self.data['seed_version'] = 25
 
     def _convert_version_26(self):
@@ -908,6 +899,30 @@ class WalletDB(JsonDB):
                 }
         self.data['seed_version'] = 45
 
+    def _convert_version_46(self):
+        from .crypto import sha256d
+        if not self._is_upgrade_method_needed(45, 45):
+            return
+        # recalc keys of outgoing on-chain invoices
+        def get_id_from_onchain_outputs(raw_outputs, timestamp):
+            outputs = [PartialTxOutput.from_legacy_tuple(*output) for output in raw_outputs]
+            outputs_str = "\\n".join(f"{txout.scriptpubkey.hex()}, {txout.value}" for txout in outputs)
+            return sha256d(outputs_str + "%d" % timestamp).hex()[0:10]
+
+        invoices = self.data.get('invoices', {})
+        for key, item in list(invoices.items()):
+            is_lightning = item['lightning_invoice'] is not None
+            if is_lightning:
+                continue
+            outputs_raw = item['outputs']
+            assert outputs_raw, outputs_raw
+            timestamp = item['time']
+            newkey = get_id_from_onchain_outputs(outputs_raw, timestamp)
+            if newkey != key:
+                invoices[newkey] = item
+                del invoices[key]
+        self.data['seed_version'] = 46
+
     def _convert_imported(self):
         if not self._is_upgrade_method_needed(0, 13):
             return