AaronFiore/electrum
1
0
Fork 0
Code Activity

lnpeer: do not run maybe_fulfill_htlc more than once, if it

Browse Source 
triggered a payment forwarding.

Final onions may trigger a payment forwarding, through the callback
returned by maybe_fulfill_htlc. In that case, we should not fail the
HTLC later; doing so might result in fund loss.

Remove test_simple_payment_with_hold_invoice_timing_out: once we
have accepted to forward a payment HTLC with a hold invoice, we
do not want to time it out, for the same reason.
This commit is contained in:
ThomasV
2023-07-23 09:40:39 +02:00
parent 1296d3361d
commit 141cd524bc
2 changed files with 36 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

67
electrum/lnpeer.py
View File

@@ -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

12
electrum/tests/test_lnpeer.py
View File

@@ -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.