Merge pull request #8584 from SomberNight/202308_fix8582

invoices: also run amount-validator on setter

contrib/requirements/requirements.txt

@@ -6,7 +6,7 @@ aiohttp>=3.3.0,<4.0.0
 aiohttp_socks>=0.3
 certifi
 bitstring
-attrs>=19.2.0
+attrs>=20.1.0
 
 # Note that we also need the dnspython[DNSSEC] extra which pulls in cryptography,
 # but as that is not pure-python it cannot be listed in this file!

electrum/gui/kivy/uix/screens.py

@@ -351,7 +351,7 @@ class SendScreen(CScreen, Logger):
                 assert type(amount_sat) is int
                 invoice = Invoice.from_bech32(address)
                 if invoice.amount_msat is None:
-                    invoice.amount_msat = int(amount_sat * 1000)
+                    invoice.set_amount_msat(int(amount_sat * 1000))
                 return invoice
             else:
                 # on-chain

electrum/gui/qml/qeinvoice.py

@@ -379,8 +379,7 @@ class QEInvoice(QObject, QtEventListener):
         if self.amount.isEmpty:
             if self.amountOverride.isEmpty:
                 raise Exception('can not pay 0 amount')
-            # TODO: is update amount_msat for overrideAmount sufficient?
+            self._effectiveInvoice.set_amount_msat(self.amountOverride.satsInt * 1000)
-            self._effectiveInvoice.amount_msat = self.amountOverride.satsInt * 1000
 
         self._wallet.pay_lightning_invoice(self._effectiveInvoice)
 
@@ -627,9 +626,9 @@ class QEInvoiceParser(QEInvoice):
 
         if not self._effectiveInvoice.amount_msat and not self.amountOverride.isEmpty:
             if self.invoiceType == QEInvoice.Type.OnchainInvoice and self.amountOverride.isMax:
-                self._effectiveInvoice.amount_msat = '!'
+                self._effectiveInvoice.set_amount_msat('!')
             else:
-                self._effectiveInvoice.amount_msat = self.amountOverride.satsInt * 1000
+                self._effectiveInvoice.set_amount_msat(self.amountOverride.satsInt * 1000)
 
         self.canSave = False
 

electrum/gui/text.py

@@ -606,7 +606,7 @@ class ElectrumGui(BaseElectrumGui, EventListener):
             if invoice.amount_msat is None:
                 amount_sat = self.parse_amount(self.str_amount)
                 if amount_sat:
-                    invoice.amount_msat = int(amount_sat * 1000)
+                    invoice.set_amount_msat(int(amount_sat * 1000))
                 else:
                     self.show_error(_('No amount'))
                     return

electrum/invoices.py

@@ -94,13 +94,18 @@ class BaseInvoice(StoredObject):
     """
     Base class for Invoice and Request
     In the code, we use 'invoice' for outgoing payments, and 'request' for incoming payments.
+
+    TODO this class is getting too complicated for "attrs"... maybe we should rewrite it without.
     """
 
     # mandatory fields
-    amount_msat = attr.ib(kw_only=True)  # type: Optional[Union[int, str]]  # can be '!' or None
+    amount_msat = attr.ib(  # can be '!' or None
+        kw_only=True, on_setattr=attr.setters.validate)  # type: Optional[Union[int, str]]
     message = attr.ib(type=str, kw_only=True)
-    time = attr.ib(type=int, kw_only=True, validator=attr.validators.instance_of(int))  # timestamp of the invoice
+    time = attr.ib(  # timestamp of the invoice
-    exp = attr.ib(type=int, kw_only=True, validator=attr.validators.instance_of(int))   # expiration delay (relative). 0 means never
+        type=int, kw_only=True, validator=attr.validators.instance_of(int), on_setattr=attr.setters.validate)
+    exp = attr.ib(  # expiration delay (relative). 0 means never
+        type=int, kw_only=True, validator=attr.validators.instance_of(int), on_setattr=attr.setters.validate)
 
     # optional fields.
     # an request (incoming) can be satisfied onchain, using lightning or using a swap
@@ -108,7 +113,8 @@ class BaseInvoice(StoredObject):
 
     # onchain only
     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
+    height = attr.ib(  # only for receiving
+        type=int, kw_only=True, validator=attr.validators.instance_of(int), on_setattr=attr.setters.validate)
     bip70 = attr.ib(type=str, kw_only=True)  # type: Optional[str]
     #bip70_requestor = attr.ib(type=str, kw_only=True)  # type: Optional[str]
 
@@ -172,6 +178,19 @@ class BaseInvoice(StoredObject):
             return amount_msat
         return int(amount_msat // 1000)
 
+    def set_amount_msat(self, amount_msat: Union[int, str]) -> None:
+        """The GUI uses this to fill the amount for a zero-amount invoice."""
+        if amount_msat == "!":
+            amount_sat = amount_msat
+        else:
+            assert isinstance(amount_msat, int), f"{amount_msat=!r}"
+            assert amount_msat >= 0, amount_msat
+            amount_sat = (amount_msat // 1000) + int(amount_msat % 1000 > 0)  # round up
+        if outputs := self.outputs:
+            assert len(self.outputs) == 1, len(self.outputs)
+            self.outputs = [PartialTxOutput(scriptpubkey=outputs[0].scriptpubkey, value=amount_sat)]
+        self.amount_msat = amount_msat
+
     @amount_msat.validator
     def _validate_amount(self, attribute, value):
         if value is None:

electrum/payment_identifier.py

@@ -674,7 +674,7 @@ def invoice_from_payment_identifier(
         if not invoice:
             return
         if invoice.amount_msat is None:
-            invoice.amount_msat = int(amount_sat * 1000)
+            invoice.set_amount_msat(int(amount_sat * 1000))
         return invoice
     else:
         outputs = pi.get_onchain_outputs(amount_sat)

electrum/tests/test_invoices.py

@@ -5,10 +5,10 @@ from . import ElectrumTestCase
 
 from electrum.simple_config import SimpleConfig
 from electrum.wallet import restore_wallet_from_text, Standard_Wallet, Abstract_Wallet
-from electrum.invoices import PR_UNPAID, PR_PAID, PR_UNCONFIRMED, BaseInvoice
+from electrum.invoices import PR_UNPAID, PR_PAID, PR_UNCONFIRMED, BaseInvoice, Invoice, LN_EXPIRY_NEVER
 from electrum.address_synchronizer import TX_HEIGHT_UNCONFIRMED
 from electrum.transaction import Transaction, PartialTxOutput
-from electrum.util import TxMinedInfo
+from electrum.util import TxMinedInfo, InvoiceError
 
 
 class TestWalletPaymentRequests(ElectrumTestCase):
@@ -266,3 +266,41 @@ class TestWalletPaymentRequests(ElectrumTestCase):
         BaseInvoice._get_cur_time = lambda *args: time.time() + 200_000
         self.assertEqual(PR_UNCONFIRMED, wallet1.get_invoice_status(pr2))
         self.assertEqual(pr2, wallet1.get_request_by_addr(addr1))
+
+
+class TestBaseInvoice(ElectrumTestCase):
+    TESTNET = True
+
+    async def test_arg_validation(self):
+        amount_sat = 10_000
+        outputs = [PartialTxOutput.from_address_and_value("tb1qmjzmg8nd4z56ar4fpngzsr6euktrhnjg9td385", amount_sat)]
+        invoice = Invoice(
+            amount_msat=amount_sat * 1000,
+            message="mymsg",
+            time=1692716965,
+            exp=LN_EXPIRY_NEVER,
+            outputs=outputs,
+            bip70=None,
+            height=0,
+            lightning_invoice=None,
+        )
+        with self.assertRaises(InvoiceError):
+            invoice.amount_msat = 10**20
+        with self.assertRaises(InvoiceError):
+            invoice.set_amount_msat(10**20)
+        with self.assertRaises(InvoiceError):
+            invoice2 = Invoice(
+                amount_msat=10**20,
+                message="mymsg",
+                time=1692716965,
+                exp=LN_EXPIRY_NEVER,
+                outputs=outputs,
+                bip70=None,
+                height=0,
+                lightning_invoice=None,
+            )
+        with self.assertRaises(TypeError):
+            invoice.time = "asd"
+        with self.assertRaises(TypeError):
+            invoice.exp = "asd"
+

electrum/wallet_db.py

@@ -54,7 +54,7 @@ from .version import ELECTRUM_VERSION
 
 OLD_SEED_VERSION = 4        # electrum versions < 2.0
 NEW_SEED_VERSION = 11       # electrum versions >= 2.0
-FINAL_SEED_VERSION = 53     # electrum >= 2.7 will set this to prevent
+FINAL_SEED_VERSION = 54     # electrum >= 2.7 will set this to prevent
                             # old versions from overwriting new format
 
 
@@ -239,6 +239,7 @@ class WalletDB(JsonDB):
         self._convert_version_51()
         self._convert_version_52()
         self._convert_version_53()
+        self._convert_version_54()
         self.put('seed_version', FINAL_SEED_VERSION)  # just to be sure
 
         self._after_upgrade_tasks()
@@ -1076,6 +1077,23 @@ class WalletDB(JsonDB):
                 cb['local_payment_pubkey'] = None
         self.data['seed_version'] = 53
 
+    def _convert_version_54(self):
+        # note: similar to convert_version_38
+        if not self._is_upgrade_method_needed(53, 53):
+            return
+        from .bitcoin import TOTAL_COIN_SUPPLY_LIMIT_IN_BTC, COIN
+        max_sats = TOTAL_COIN_SUPPLY_LIMIT_IN_BTC * COIN
+        requests = self.data.get('payment_requests', {})
+        invoices = self.data.get('invoices', {})
+        for d in [invoices, requests]:
+            for key, item in list(d.items()):
+                amount_msat = item['amount_msat']
+                if amount_msat == '!':
+                    continue
+                if not (isinstance(amount_msat, int) and 0 <= amount_msat <= max_sats * 1000):
+                    del d[key]
+        self.data['seed_version'] = 54
+
     def _convert_imported(self):
         if not self._is_upgrade_method_needed(0, 13):
             return