diff --git a/electrum/commands.py b/electrum/commands.py index f249f55f9..889243d80 100644 --- a/electrum/commands.py +++ b/electrum/commands.py @@ -1389,7 +1389,9 @@ class Commands(Logger): ) -> dict: """ Create a lightning hold invoice for the given payment hash. Hold invoices have to get settled manually later. - HTLCs will get failed automatically if block_height + 144 > htlc.cltv_abs. + HTLCs will get failed automatically if block_height + 144 > htlc.cltv_abs, if the intention is to + settle them as late as possible a safety margin of some blocks should be used to prevent them + from getting failed accidentally. arg:str:payment_hash:Hex encoded payment hash to be used for the invoice arg:decimal:amount:Optional requested amount (in btc) @@ -1399,7 +1401,7 @@ class Commands(Logger): """ assert len(payment_hash) == 64, f"Invalid payment hash length: {len(payment_hash)} != 64" assert payment_hash not in wallet.lnworker.payment_info, "Payment hash already used!" - assert payment_hash not in wallet.lnworker.dont_settle_htlcs, "Payment hash already used!" + assert payment_hash not in wallet.lnworker.dont_expire_htlcs, "Payment hash already used!" assert wallet.lnworker.get_preimage(bfh(payment_hash)) is None, "Already got a preimage for this payment hash!" assert MIN_FINAL_CLTV_DELTA_ACCEPTED < min_final_cltv_expiry_delta < 576, "Use a sane min_final_cltv_expiry_delta value" amount = amount if amount and satoshis(amount) > 0 else None # make amount either >0 or None @@ -1419,7 +1421,9 @@ class Commands(Logger): message=memo, fallback_address=None ) - wallet.lnworker.dont_settle_htlcs[payment_hash] = None + # this prevents incoming htlcs from getting expired while the preimage isn't set. + # If their blocks to expiry fall below MIN_FINAL_CLTV_DELTA_ACCEPTED they will get failed. + wallet.lnworker.dont_expire_htlcs[payment_hash] = MIN_FINAL_CLTV_DELTA_ACCEPTED wallet.set_label(payment_hash, memo) result = { "invoice": invoice @@ -1439,12 +1443,11 @@ class Commands(Logger): assert payment_hash not in wallet.lnworker._preimages, f"Invoice {payment_hash=} already settled" assert payment_hash in wallet.lnworker.payment_info, \ f"Couldn't find lightning invoice for {payment_hash=}" - assert payment_hash in wallet.lnworker.dont_settle_htlcs, f"Invoice {payment_hash=} not a hold invoice?" + assert payment_hash in wallet.lnworker.dont_expire_htlcs, f"Invoice {payment_hash=} not a hold invoice?" assert wallet.lnworker.is_complete_mpp(bfh(payment_hash)), \ f"MPP incomplete, cannot settle hold invoice {payment_hash} yet" info: Optional['PaymentInfo'] = wallet.lnworker.get_payment_info(bfh(payment_hash)) assert (wallet.lnworker.get_payment_mpp_amount_msat(bfh(payment_hash)) or 0) >= (info.amount_msat or 0) - del wallet.lnworker.dont_settle_htlcs[payment_hash] wallet.lnworker.save_preimage(bfh(payment_hash), bfh(preimage)) util.trigger_callback('wallet_updated', wallet) result = { @@ -1462,15 +1465,15 @@ class Commands(Logger): assert payment_hash in wallet.lnworker.payment_info, \ f"Couldn't find lightning invoice for payment hash {payment_hash}" assert payment_hash not in wallet.lnworker._preimages, "Cannot cancel anymore, preimage already given." - assert payment_hash in wallet.lnworker.dont_settle_htlcs, f"{payment_hash=} not a hold invoice?" + assert payment_hash in wallet.lnworker.dont_expire_htlcs, f"{payment_hash=} not a hold invoice?" # set to PR_UNPAID so it can get deleted wallet.lnworker.set_payment_status(bfh(payment_hash), PR_UNPAID) wallet.lnworker.delete_payment_info(payment_hash) wallet.set_label(payment_hash, None) + del wallet.lnworker.dont_expire_htlcs[payment_hash] while wallet.lnworker.is_complete_mpp(bfh(payment_hash)): - # wait until the htlcs got failed so the payment won't get settled accidentally in a race + # block until the htlcs got failed await asyncio.sleep(0.1) - del wallet.lnworker.dont_settle_htlcs[payment_hash] result = { "cancelled": payment_hash } @@ -1503,15 +1506,14 @@ class Commands(Logger): elif not is_complete_mpp and not wallet.lnworker.get_preimage_hex(payment_hash): # is_complete_mpp is False for settled payments result["status"] = "unpaid" - elif is_complete_mpp and payment_hash in wallet.lnworker.dont_settle_htlcs: + elif is_complete_mpp and payment_hash in wallet.lnworker.dont_expire_htlcs: result["status"] = "paid" payment_key: str = wallet.lnworker._get_payment_key(bfh(payment_hash)).hex() htlc_status = wallet.lnworker.received_mpp_htlcs[payment_key] result["closest_htlc_expiry_height"] = min( mpp_htlc.htlc.cltv_abs for mpp_htlc in htlc_status.htlcs ) - elif wallet.lnworker.get_preimage_hex(payment_hash) is not None \ - and payment_hash not in wallet.lnworker.dont_settle_htlcs: + elif wallet.lnworker.get_preimage_hex(payment_hash) is not None: result["status"] = "settled" plist = wallet.lnworker.get_payments(status='settled')[bfh(payment_hash)] _dir, amount_msat, _fee, _ts = wallet.lnworker.get_payment_value(info, plist) diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py index f09ef97a2..e263da1ca 100644 --- a/electrum/lnpeer.py +++ b/electrum/lnpeer.py @@ -2185,6 +2185,7 @@ class Peer(Logger, EventListener): # the htlc set representing the whole payment (payment key derived from trampoline/invoice secret). payment_key = (payment_hash + (outer_onion_payment_secret or payment_secret_from_onion)).hex() + # for safety, still enforce MIN_FINAL_CLTV_DELTA here even if payment_hash is in dont_expire_htlcs if blocks_to_expiry < MIN_FINAL_CLTV_DELTA_ACCEPTED: # this check should be done here for new htlcs and ongoing on pending sets. # Here it is done so that invalid received htlcs will never get added to a set, @@ -2261,6 +2262,8 @@ class Peer(Logger, EventListener): # get payment hash of any htlc in the set (they are all the same) payment_hash = htlc_set.get_payment_hash() assert payment_hash is not None, htlc_set + assert payment_hash not in self.lnworker.dont_settle_htlcs + self.lnworker.dont_expire_htlcs.pop(payment_hash.hex(), None) # htlcs wont get expired anymore for mpp_htlc in list(htlc_set.htlcs): htlc_id = mpp_htlc.htlc.htlc_id chan = self.lnworker.get_channel_by_short_id(mpp_htlc.scid) @@ -2300,6 +2303,10 @@ class Peer(Logger, EventListener): raw_error, error_code, error_data = error_tuple local_height = self.network.blockchain().height() + payment_hash = htlc_set.get_payment_hash() + assert payment_hash is not None, "Empty htlc set?" + self.lnworker.dont_expire_htlcs.pop(payment_hash.hex(), None) + self.lnworker.dont_settle_htlcs.pop(payment_hash.hex(), None) # already failed for mpp_htlc in list(htlc_set.htlcs): chan = self.lnworker.get_channel_by_short_id(mpp_htlc.scid) htlc_id = mpp_htlc.htlc.htlc_id @@ -2317,7 +2324,7 @@ class Peer(Logger, EventListener): onion_packet = self._parse_onion_packet(mpp_htlc.unprocessed_onion) processed_onion_packet = self._process_incoming_onion_packet( onion_packet, - payment_hash=mpp_htlc.htlc.payment_hash, + payment_hash=payment_hash, is_trampoline=False, ) if raw_error: @@ -2331,7 +2338,7 @@ class Peer(Logger, EventListener): if processed_onion_packet.trampoline_onion_packet: processed_trampoline_onion_packet = self._process_incoming_onion_packet( processed_onion_packet.trampoline_onion_packet, - payment_hash=mpp_htlc.htlc.payment_hash, + payment_hash=payment_hash, is_trampoline=True, ) amount_to_forward = processed_trampoline_onion_packet.amt_to_forward @@ -3048,7 +3055,8 @@ class Peer(Logger, EventListener): # check for expiry over time and potentially fail the whole set if any # htlc's cltv becomes too close blocks_to_expiry = max(0, closest_cltv_abs - local_height) - if blocks_to_expiry < MIN_FINAL_CLTV_DELTA_ACCEPTED: + accepted_expiry_delta = self.lnworker.dont_expire_htlcs.get(payment_hash.hex(), MIN_FINAL_CLTV_DELTA_ACCEPTED) + if accepted_expiry_delta is not None and blocks_to_expiry < accepted_expiry_delta: _log_fail_reason(f"htlc.cltv_abs is unreasonably close") return OnionFailureCode.INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS, None, None @@ -3119,11 +3127,13 @@ class Peer(Logger, EventListener): return OnionFailureCode.INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS, None, None return None, None, None - if payment_hash.hex() in self.lnworker.dont_settle_htlcs: - # used by hold invoice cli to prevent the htlcs from getting fulfilled automatically + preimage = self.lnworker.get_preimage(payment_hash) + settling_blocked = preimage is not None and payment_hash.hex() in self.lnworker.dont_settle_htlcs + waiting_for_preimage = preimage is None and payment_hash.hex() in self.lnworker.dont_expire_htlcs + if settling_blocked or waiting_for_preimage: + # used by hold invoice cli and JIT channels to prevent the htlcs from getting fulfilled automatically return None, None, None - preimage = self.lnworker.get_preimage(payment_hash) hold_invoice_callback = self.lnworker.hold_invoice_callbacks.get(payment_hash) if not preimage and not hold_invoice_callback: _log_fail_reason(f"cannot settle, no preimage or callback found for {payment_hash.hex()=}") diff --git a/electrum/lnworker.py b/electrum/lnworker.py index d4c5bc8cb..347bcc084 100644 --- a/electrum/lnworker.py +++ b/electrum/lnworker.py @@ -925,7 +925,22 @@ class LNWallet(LNWorker): self.active_forwardings = self.db.get_dict('active_forwardings') # type: Dict[str, List[str]] # Dict: payment_key -> list of htlc_keys self.forwarding_failures = self.db.get_dict('forwarding_failures') # type: Dict[str, Tuple[str, str]] # Dict: payment_key -> (error_bytes, error_message) self.downstream_to_upstream_htlc = {} # type: Dict[str, str] # Dict: htlc_key -> htlc_key (not persisted) - self.dont_settle_htlcs = self.db.get_dict('dont_settle_htlcs') # type: Dict[str, None] # payment_hashes of htlcs that we should not settle back yet even if we have the preimage + + # k: payment_hashes of htlcs that we should not expire even if we don't know the preimage + # v: If `None` the htlcs won't get expired and potentially get timed out in a force close. + # Note: it might not be safe to release the preimage shortly before expiry as this would allow the + # remote node to ignore our fulfill_htlc, wait until expiry and try to time out the htlc onchain + # in a fee race against us and then use our released preimage to fulfill upstream. + # v: If `int`: Overwrites `MIN_FINAL_CLTV_DELTA_ACCEPTED` in htlc switch and allows to set custom + # expiration delta. The htlcs will get expired if their blocks left to expiry are + # below the specified expiration delta. + # htlcs will get settled as soon as the preimage becomes available + self.dont_expire_htlcs = self.db.get_dict('dont_expire_htlcs') # type: Dict[str, Optional[int]] + + # k: payment_hash of payments for which we don't want to release the preimage, no matter + # how close to expiry. Doesn't prevent htlcs from getting expired or failed if there is no + # preimage available. Might be used in combination with dont_expire_htlcs. + self.dont_settle_htlcs = self.db.get_dict('dont_settle_htlcs') # type: Dict[str, None] # payment_hash -> callback: self.hold_invoice_callbacks = {} # type: Dict[bytes, Callable[[bytes], Awaitable[None]]] diff --git a/tests/test_commands.py b/tests/test_commands.py index 455f7b87e..3016263e1 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -510,7 +510,7 @@ class TestCommandsTestnet(ElectrumTestCase): invoice = lndecode(invoice=result['invoice']) assert invoice.paymenthash.hex() == payment_hash assert payment_hash in wallet.lnworker.payment_info - assert payment_hash in wallet.lnworker.dont_settle_htlcs + assert payment_hash in wallet.lnworker.dont_expire_htlcs assert invoice.get_amount_sat() == 10000 assert invoice.get_description() == "test" assert wallet.get_label_for_rhash(rhash=invoice.paymenthash.hex()) == "test" @@ -521,7 +521,7 @@ class TestCommandsTestnet(ElectrumTestCase): wallet=wallet, ) assert payment_hash not in wallet.lnworker.payment_info - assert payment_hash not in wallet.lnworker.dont_settle_htlcs + assert payment_hash not in wallet.lnworker.dont_expire_htlcs assert wallet.get_label_for_rhash(rhash=invoice.paymenthash.hex()) == "" assert cancel_result['cancelled'] == payment_hash @@ -571,7 +571,6 @@ class TestCommandsTestnet(ElectrumTestCase): ) assert settle_result['settled'] == payment_hash assert wallet.lnworker._preimages[payment_hash] == preimage.hex() - assert payment_hash not in wallet.lnworker.dont_settle_htlcs with (mock.patch.object( wallet.lnworker, 'get_payment_value', diff --git a/tests/test_lnpeer.py b/tests/test_lnpeer.py index 09cdbc24d..e2597b855 100644 --- a/tests/test_lnpeer.py +++ b/tests/test_lnpeer.py @@ -218,6 +218,7 @@ class MockLNWallet(Logger, EventListener, NetworkRetryManager[LNPeerAddr]): self._preimages = {} self.stopping_soon = False self.downstream_to_upstream_htlc = {} + self.dont_expire_htlcs = {} self.dont_settle_htlcs = {} self.hold_invoice_callbacks = {} self._payment_bundles_pkey_to_canon = {} # type: Dict[bytes, bytes]