diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py index f29a64773..da0aa356c 100644 --- a/electrum/lnpeer.py +++ b/electrum/lnpeer.py @@ -1744,9 +1744,9 @@ class Peer(Logger): Return (preimage, forwarding_callback) with at most a single element not None """ def log_fail_reason(reason: str): - self.logger.info(f"maybe_fulfill_htlc. will FAIL HTLC: chan {chan.short_channel_id}. " - f"{reason}. htlc={str(htlc)}. onion_payload={processed_onion.hop_data.payload}") - + self.logger.info( + f"maybe_fulfill_htlc. will FAIL HTLC: chan {chan.short_channel_id}. " + f"{reason}. htlc={str(htlc)}. onion_payload={processed_onion.hop_data.payload}") try: amt_to_forward = processed_onion.hop_data.payload["amt_to_forward"]["amt_to_forward"] except Exception: @@ -1809,7 +1809,7 @@ class Peer(Logger): payment_hash = htlc.payment_hash preimage = self.lnworker.get_preimage(payment_hash) hold_invoice_callback = self.lnworker.hold_invoice_callbacks.get(payment_hash) - if not preimage and hold_invoice_callback: + if hold_invoice_callback: if preimage: return preimage, None else: @@ -1818,12 +1818,11 @@ class Peer(Logger): if int(time.time()) < timeout: return None, lambda: cb(payment_hash) else: - raise OnionRoutingFailure(code=OnionFailureCode.MPP_TIMEOUT, data=b'') + raise exc_incorrect_or_unknown_pd # if there is a trampoline_onion, maybe_fulfill_htlc will be called again if processed_onion.trampoline_onion_packet: # TODO: we should check that all trampoline_onions are the same - trampoline_onion = self.process_onion_packet( processed_onion.trampoline_onion_packet, payment_hash=payment_hash, @@ -1849,15 +1848,18 @@ class Peer(Logger): # TODO don't accept payments twice for same invoice # TODO check invoice expiry - info = self.lnworker.get_payment_info(htlc.payment_hash) + info = self.lnworker.get_payment_info(payment_hash) if info is None: log_fail_reason(f"no payment_info found for RHASH {htlc.payment_hash.hex()}") raise exc_incorrect_or_unknown_pd - preimage = self.lnworker.get_preimage(htlc.payment_hash) + + preimage = self.lnworker.get_preimage(payment_hash) + if not preimage: + self.logger.info(f"missing callback {payment_hash.hex()}") + return None, None + expected_payment_secrets = [self.lnworker.get_payment_secret(htlc.payment_hash)] - if preimage: - # legacy secret for old invoices - expected_payment_secrets.append(derive_payment_secret_from_payment_preimage(preimage)) + expected_payment_secrets.append(derive_payment_secret_from_payment_preimage(preimage)) # legacy secret for old invoices if payment_secret_from_onion not in expected_payment_secrets: log_fail_reason(f'incorrect payment secret {payment_secret_from_onion.hex()} != {expected_payment_secrets[0].hex()}') raise exc_incorrect_or_unknown_pd @@ -1865,9 +1867,7 @@ class Peer(Logger): if not (invoice_msat is None or invoice_msat <= total_msat <= 2 * invoice_msat): log_fail_reason(f"total_msat={total_msat} too different from invoice_msat={invoice_msat}") raise exc_incorrect_or_unknown_pd - if preimage: - self.logger.info(f"maybe_fulfill_htlc. will FULFILL HTLC: chan {chan.short_channel_id}. htlc={str(htlc)}") - self.lnworker.set_request_status(htlc.payment_hash, PR_PAID) + self.logger.info(f"maybe_fulfill_htlc. will FULFILL HTLC: chan {chan.short_channel_id}. htlc={str(htlc)}") return preimage, None def fulfill_htlc(self, chan: Channel, htlc_id: int, preimage: bytes): @@ -2301,6 +2301,7 @@ class Peer(Logger): self.lnworker.downstream_htlc_to_upstream_peer_map[fw_info] = self.pubkey elif preimage or error_reason or error_bytes: if preimage: + self.lnworker.set_request_status(htlc.payment_hash, PR_PAID) if not self.lnworker.enable_htlc_settle: continue self.fulfill_htlc(chan, htlc.htlc_id, preimage) @@ -2363,16 +2364,15 @@ class Peer(Logger): onion_packet_bytes=onion_packet_bytes) if processed_onion.are_we_final: # either we are final recipient; or if trampoline, see cases below - preimage, forwarding_callback = self.maybe_fulfill_htlc( - chan=chan, - htlc=htlc, - processed_onion=processed_onion, - onion_packet_bytes=onion_packet_bytes) - - payment_secret = processed_onion.hop_data.payload["payment_data"]["payment_secret"] - payment_key = payment_hash + payment_secret - if forwarding_callback: - if not forwarding_info: + if not forwarding_info: + preimage, forwarding_callback = self.maybe_fulfill_htlc( + chan=chan, + htlc=htlc, + processed_onion=processed_onion, + onion_packet_bytes=onion_packet_bytes) + if forwarding_callback: + payment_secret = processed_onion.hop_data.payload["payment_data"]["payment_secret"] + payment_key = payment_hash + payment_secret # trampoline- HTLC we are supposed to forward, but haven't forwarded yet if not self.lnworker.enable_htlc_forwarding: pass @@ -2394,15 +2394,16 @@ class Peer(Logger): # remove from list of payments, so that another attempt can be initiated self.lnworker.trampoline_forwardings.remove(payment_key) asyncio.ensure_future(wrapped_callback()) - return None, True, None - else: - # trampoline- HTLC we are supposed to forward, and have already forwarded - preimage = self.lnworker.get_preimage(payment_hash) - # get (and not pop) failure because the incoming payment might be multi-part - error_reason = self.lnworker.trampoline_forwarding_failures.get(payment_key) - if error_reason: - self.logger.info(f'trampoline forwarding failure: {error_reason.code_name()}') - raise error_reason + return None, payment_key, None + else: + payment_key = forwarding_info + # trampoline- HTLC we are supposed to forward, and have already forwarded + preimage = self.lnworker.get_preimage(payment_hash) + # get (and not pop) failure because the incoming payment might be multi-part + error_reason = self.lnworker.trampoline_forwarding_failures.get(payment_key) + if error_reason: + self.logger.info(f'trampoline forwarding failure: {error_reason.code_name()}') + raise error_reason elif not forwarding_info: # HTLC we are supposed to forward, but haven't forwarded yet diff --git a/electrum/tests/test_lnpeer.py b/electrum/tests/test_lnpeer.py index 7b8a805d7..d2eafa6f2 100644 --- a/electrum/tests/test_lnpeer.py +++ b/electrum/tests/test_lnpeer.py @@ -745,7 +745,6 @@ class TestPeer(ElectrumTestCase): self, test_trampoline: bool, test_hold_invoice=False, - test_hold_timeout=False, test_bundle=False, test_bundle_timeout=False ): @@ -765,10 +764,8 @@ class TestPeer(ElectrumTestCase): payment_hash = lnaddr.paymenthash preimage = bytes.fromhex(w2.preimages.pop(payment_hash.hex())) async def cb(payment_hash): - if not test_hold_timeout: - w2.save_preimage(payment_hash, preimage) - timeout = 1 if test_hold_timeout else 60 - w2.register_callback_for_hold_invoice(payment_hash, cb, timeout) + w2.save_preimage(payment_hash, preimage) + w2.register_callback_for_hold_invoice(payment_hash, cb, 60) if test_bundle: lnaddr2, pay_req2 = self.prepare_invoice(w2) @@ -817,11 +814,6 @@ class TestPeer(ElectrumTestCase): with self.assertRaises(PaymentDone): await self._test_simple_payment(test_trampoline=test_trampoline, test_hold_invoice=True) - async def test_simple_payment_with_hold_invoice_timing_out(self): - for test_trampoline in [False, True]: - with self.assertRaises(PaymentFailure): - await self._test_simple_payment(test_trampoline=test_trampoline, test_hold_invoice=True, test_hold_timeout=True) - @needs_test_with_all_chacha20_implementations async def test_payment_race(self): """Alice and Bob pay each other simultaneously.