tlnworker: fix type error re pending_payments, and impl malformed htlcs - electrum - Electrum Bitcoin wallet
HTML git clone https://git.parazyd.org/electrum
DIR Log
DIR Files
DIR Refs
DIR Submodules
---
DIR commit 2cc76fbbbda3b49262dda7f00a9d7ea6a1c39761
DIR parent 9a70b79eea77aab42d7d681bfa5f74b9f90dd3ca
HTML Author: SomberNight <somber.night@protonmail.com>
Date: Tue, 17 Mar 2020 19:23:04 +0100
lnworker: fix type error re pending_payments, and impl malformed htlcs
In old code, in lnpeer.htlc_switch(), "error" in lnworker.pending_payments
had incorrect type.
TODO: we need tests for payment failures...
Diffstat:
M electrum/lnchannel.py | 17 +++++++++++------
M electrum/lnpeer.py | 51 +++++++++++++++++++------------
M electrum/lnutil.py | 16 ++++++++++++----
M electrum/lnworker.py | 66 +++++++++++++++++--------------
M electrum/tests/test_lnchannel.py | 2 +-
5 files changed, 92 insertions(+), 60 deletions(-)
---
DIR diff --git a/electrum/lnchannel.py b/electrum/lnchannel.py
t@@ -50,7 +50,7 @@ from .lnutil import (Outpoint, LocalConfig, RemoteConfig, Keypair, OnlyPubkeyKey
HTLC_TIMEOUT_WEIGHT, HTLC_SUCCESS_WEIGHT, extract_ctn_from_tx_and_chan, UpdateAddHtlc,
funding_output_script, SENT, RECEIVED, LOCAL, REMOTE, HTLCOwner, make_commitment_outputs,
ScriptHtlc, PaymentFailure, calc_onchain_fees, RemoteMisbehaving, make_htlc_output_witness_script,
- ShortChannelID, map_htlcs_to_ctx_output_idxs, LNPeerAddr)
+ ShortChannelID, map_htlcs_to_ctx_output_idxs, LNPeerAddr, BarePaymentAttemptLog)
from .lnsweep import create_sweeptxs_for_our_ctx, create_sweeptxs_for_their_ctx
from .lnsweep import create_sweeptx_for_their_revoked_htlc, SweepInfo
from .lnhtlc import HTLCManager
t@@ -158,7 +158,7 @@ class Channel(Logger):
self._chan_ann_without_sigs = None # type: Optional[bytes]
self.revocation_store = RevocationStore(state["revocation_store"])
self._can_send_ctx_updates = True # type: bool
- self._receive_fail_reasons = {}
+ self._receive_fail_reasons = {} # type: Dict[int, BarePaymentAttemptLog]
def get_id_for_log(self) -> str:
scid = self.short_channel_id
t@@ -622,8 +622,8 @@ class Channel(Logger):
self.lnworker.payment_sent(self, htlc.payment_hash)
failed = self.hm.failed_in_ctn(new_ctn)
for htlc in failed:
- reason = self._receive_fail_reasons.get(htlc.htlc_id)
- self.lnworker.payment_failed(self, htlc.payment_hash, reason)
+ payment_attempt = self._receive_fail_reasons.get(htlc.htlc_id)
+ self.lnworker.payment_failed(self, htlc.payment_hash, payment_attempt)
def balance(self, whose: HTLCOwner, *, ctx_owner=HTLCOwner.LOCAL, ctn: int = None) -> int:
"""
t@@ -793,11 +793,16 @@ class Channel(Logger):
with self.db_lock:
self.hm.send_fail(htlc_id)
- def receive_fail_htlc(self, htlc_id: int, reason: bytes):
+ def receive_fail_htlc(self, htlc_id: int, *,
+ error_bytes: Optional[bytes],
+ reason: Optional[OnionRoutingFailureMessage] = None):
self.logger.info("receive_fail_htlc")
with self.db_lock:
self.hm.recv_fail(htlc_id)
- self._receive_fail_reasons[htlc_id] = reason
+ self._receive_fail_reasons[htlc_id] = BarePaymentAttemptLog(success=False,
+ preimage=None,
+ error_bytes=error_bytes,
+ error_reason=reason)
def pending_local_fee(self):
return self.constraints.capacity - sum(x.value for x in self.get_next_commitment(LOCAL).outputs())
DIR diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py
t@@ -1007,7 +1007,7 @@ class Peer(Logger):
htlc_id = int.from_bytes(payload["id"], "big")
reason = payload["reason"]
self.logger.info(f"on_update_fail_htlc. chan {chan.short_channel_id}. htlc_id {htlc_id}")
- chan.receive_fail_htlc(htlc_id, reason) # TODO handle exc and maybe fail channel (e.g. bad htlc_id)
+ chan.receive_fail_htlc(htlc_id, error_bytes=reason) # TODO handle exc and maybe fail channel (e.g. bad htlc_id)
self.maybe_send_commitment(chan)
def maybe_send_commitment(self, chan: Channel):
t@@ -1093,10 +1093,9 @@ class Peer(Logger):
if failure_code & OnionFailureCodeMetaFlag.BADONION == 0:
asyncio.ensure_future(self.lnworker.try_force_closing(chan.channel_id))
raise RemoteMisbehaving(f"received update_fail_malformed_htlc with unexpected failure code: {failure_code}")
- reason = b'' # TODO somehow propagate "failure_code" ?
- chan.receive_fail_htlc(htlc_id, reason) # TODO handle exc and maybe fail channel (e.g. bad htlc_id)
+ reason = OnionRoutingFailureMessage(code=failure_code, data=payload["sha256_of_onion"])
+ chan.receive_fail_htlc(htlc_id, error_bytes=None, reason=reason)
self.maybe_send_commitment(chan)
- # TODO when forwarding, we need to propagate this "update_fail_malformed_htlc" downstream
def on_update_add_htlc(self, chan: Channel, payload):
payment_hash = payload["payment_hash"]
t@@ -1222,20 +1221,22 @@ class Peer(Logger):
id=htlc_id,
payment_preimage=preimage)
- def fail_htlc(self, chan: Channel, htlc_id: int, onion_packet: Optional[OnionPacket],
- reason: OnionRoutingFailureMessage):
+ def fail_htlc(self, *, chan: Channel, htlc_id: int, onion_packet: Optional[OnionPacket],
+ reason: Optional[OnionRoutingFailureMessage], error_bytes: Optional[bytes]):
self.logger.info(f"fail_htlc. chan {chan.short_channel_id}. htlc_id {htlc_id}. reason: {reason}")
assert chan.can_send_ctx_updates(), f"cannot send updates: {chan.short_channel_id}"
chan.fail_htlc(htlc_id)
if onion_packet:
- error_packet = construct_onion_error(reason, onion_packet, our_onion_private_key=self.privkey)
+ error_bytes = construct_onion_error(reason, onion_packet, our_onion_private_key=self.privkey)
+ if error_bytes:
self.send_message("update_fail_htlc",
channel_id=chan.channel_id,
id=htlc_id,
- len=len(error_packet),
- reason=error_packet)
+ len=len(error_bytes),
+ reason=error_bytes)
else:
- assert len(reason.data) == 32, f"unexpected reason when sending 'update_fail_malformed_htlc': {reason!r}"
+ if not (reason.code & OnionFailureCodeMetaFlag.BADONION and len(reason.data) == 32):
+ raise Exception(f"unexpected reason when sending 'update_fail_malformed_htlc': {reason!r}")
self.send_message("update_fail_malformed_htlc",
channel_id=chan.channel_id,
id=htlc_id,
t@@ -1425,7 +1426,8 @@ class Peer(Logger):
chan.logger.info(f'found unfulfilled htlc: {htlc_id}')
htlc = chan.hm.log[REMOTE]['adds'][htlc_id]
payment_hash = htlc.payment_hash
- error = None # type: Optional[OnionRoutingFailureMessage]
+ error_reason = None # type: Optional[OnionRoutingFailureMessage]
+ error_bytes = None # type: Optional[bytes]
preimage = None
onion_packet_bytes = bytes.fromhex(onion_packet_hex)
onion_packet = None
t@@ -1433,36 +1435,45 @@ class Peer(Logger):
onion_packet = OnionPacket.from_bytes(onion_packet_bytes)
processed_onion = process_onion_packet(onion_packet, associated_data=payment_hash, our_onion_private_key=self.privkey)
except UnsupportedOnionPacketVersion:
- error = OnionRoutingFailureMessage(code=OnionFailureCode.INVALID_ONION_VERSION, data=sha256(onion_packet_bytes))
+ error_reason = OnionRoutingFailureMessage(code=OnionFailureCode.INVALID_ONION_VERSION, data=sha256(onion_packet_bytes))
except InvalidOnionPubkey:
- error = OnionRoutingFailureMessage(code=OnionFailureCode.INVALID_ONION_KEY, data=sha256(onion_packet_bytes))
+ error_reason = OnionRoutingFailureMessage(code=OnionFailureCode.INVALID_ONION_KEY, data=sha256(onion_packet_bytes))
except InvalidOnionMac:
- error = OnionRoutingFailureMessage(code=OnionFailureCode.INVALID_ONION_HMAC, data=sha256(onion_packet_bytes))
+ error_reason = OnionRoutingFailureMessage(code=OnionFailureCode.INVALID_ONION_HMAC, data=sha256(onion_packet_bytes))
else:
if processed_onion.are_we_final:
- preimage, error = self.maybe_fulfill_htlc(
+ preimage, error_reason = self.maybe_fulfill_htlc(
chan=chan,
htlc=htlc,
onion_packet=onion_packet,
processed_onion=processed_onion)
elif not forwarded:
- error = self.maybe_forward_htlc(
+ error_reason = self.maybe_forward_htlc(
chan=chan,
htlc=htlc,
onion_packet=onion_packet,
processed_onion=processed_onion)
- if not error:
+ if not error_reason:
unfulfilled[htlc_id] = local_ctn, remote_ctn, onion_packet_hex, True
else:
+ # TODO self.lnworker.pending_payments is not persisted,
+ # so what happens if we restart the process?...
f = self.lnworker.pending_payments[payment_hash]
if f.done():
- success, preimage, error = f.result()
+ payment_attempt = f.result()
+ preimage = payment_attempt.preimage
+ error_bytes = payment_attempt.error_bytes
+ error_reason = payment_attempt.error_reason
if preimage:
await self.lnworker.enable_htlc_settle.wait()
self.fulfill_htlc(chan, htlc.htlc_id, preimage)
done.add(htlc_id)
- if error:
- self.fail_htlc(chan, htlc.htlc_id, onion_packet, error)
+ if error_reason or error_bytes:
+ self.fail_htlc(chan=chan,
+ htlc_id=htlc.htlc_id,
+ onion_packet=onion_packet,
+ reason=error_reason,
+ error_bytes=error_bytes)
done.add(htlc_id)
# cleanup
for htlc_id in done:
DIR diff --git a/electrum/lnutil.py b/electrum/lnutil.py
t@@ -112,7 +112,7 @@ class Outpoint(StoredObject):
class PaymentAttemptFailureDetails(NamedTuple):
- sender_idx: int
+ sender_idx: Optional[int]
failure_msg: 'OnionRoutingFailureMessage'
is_blacklisted: bool
t@@ -128,16 +128,17 @@ class PaymentAttemptLog(NamedTuple):
if not self.exception:
route = self.route
route_str = '%d'%len(route)
+ short_channel_id = None
if not self.success:
sender_idx = self.failure_details.sender_idx
failure_msg = self.failure_details.failure_msg
- short_channel_id = route[sender_idx+1].short_channel_id
- data = failure_msg.data
+ if sender_idx is not None:
+ short_channel_id = route[sender_idx+1].short_channel_id
message = str(failure_msg.code.name)
else:
short_channel_id = route[-1].short_channel_id
message = _('Success')
- chan_str = str(short_channel_id)
+ chan_str = str(short_channel_id) if short_channel_id else _("Unknown")
else:
route_str = 'None'
chan_str = 'N/A'
t@@ -145,6 +146,13 @@ class PaymentAttemptLog(NamedTuple):
return route_str, chan_str, message
+class BarePaymentAttemptLog(NamedTuple):
+ success: bool
+ preimage: Optional[bytes]
+ error_bytes: Optional[bytes]
+ error_reason: Optional['OnionRoutingFailureMessage'] = None
+
+
class LightningError(Exception): pass
class LightningPeerConnectionClosed(LightningError): pass
class UnableToDeriveSecret(LightningError): pass
DIR diff --git a/electrum/lnworker.py b/electrum/lnworker.py
t@@ -53,7 +53,8 @@ from .lnutil import (Outpoint, LNPeerAddr,
UnknownPaymentHash, MIN_FINAL_CLTV_EXPIRY_FOR_INVOICE,
NUM_MAX_EDGES_IN_PAYMENT_PATH, SENT, RECEIVED, HTLCOwner,
UpdateAddHtlc, Direction, LnLocalFeatures,
- ShortChannelID, PaymentAttemptLog, PaymentAttemptFailureDetails)
+ ShortChannelID, PaymentAttemptLog, PaymentAttemptFailureDetails,
+ BarePaymentAttemptLog)
from .lnutil import ln_dummy_address, ln_compare_features
from .transaction import PartialTxOutput, PartialTransaction, PartialTxInput
from .lnonion import OnionFailureCode, process_onion_packet, OnionPacket
t@@ -436,8 +437,7 @@ class LNWallet(LNWorker):
for channel_id, c in channels.items():
self.channels[bfh(channel_id)] = Channel(c, sweep_address=self.sweep_address, lnworker=self)
- # timestamps of opening and closing transactions
- self.pending_payments = defaultdict(asyncio.Future)
+ self.pending_payments = defaultdict(asyncio.Future) # type: Dict[bytes, asyncio.Future[BarePaymentAttemptLog]]
@ignore_exceptions
@log_exceptions
t@@ -954,31 +954,36 @@ class LNWallet(LNWorker):
await peer.initialized
htlc = peer.pay(route, chan, int(lnaddr.amount * COIN * 1000), lnaddr.paymenthash, lnaddr.get_min_final_cltv_expiry())
self.network.trigger_callback('htlc_added', htlc, lnaddr, SENT)
- success, preimage, reason = await self.await_payment(lnaddr.paymenthash)
- if success:
+ payment_attempt = await self.await_payment(lnaddr.paymenthash)
+ if payment_attempt.success:
failure_log = None
else:
- # TODO this blacklisting is fragile, consider (who to ban/penalize?):
- # - we might not be able to decode "reason" (coming from update_fail_htlc).
- # - handle update_fail_malformed_htlc case, where there is (kinda) no "reason"
- failure_msg, sender_idx = chan.decode_onion_error(reason, route, htlc.htlc_id)
- blacklist = self.handle_error_code_from_failed_htlc(failure_msg, sender_idx, route, peer)
- if blacklist:
- # blacklist channel after reporter node
- # TODO this should depend on the error (even more granularity)
- # also, we need finer blacklisting (directed edges; nodes)
- try:
- short_chan_id = route[sender_idx + 1].short_channel_id
- except IndexError:
- self.logger.info("payment destination reported error")
- else:
- self.network.path_finder.add_to_blacklist(short_chan_id)
+ if payment_attempt.error_bytes:
+ # TODO "decode_onion_error" might raise, catch and maybe blacklist/penalise someone?
+ failure_msg, sender_idx = chan.decode_onion_error(payment_attempt.error_bytes, route, htlc.htlc_id)
+ is_blacklisted = self.handle_error_code_from_failed_htlc(failure_msg, sender_idx, route, peer)
+ if is_blacklisted:
+ # blacklist channel after reporter node
+ # TODO this should depend on the error (even more granularity)
+ # also, we need finer blacklisting (directed edges; nodes)
+ try:
+ short_chan_id = route[sender_idx + 1].short_channel_id
+ except IndexError:
+ self.logger.info("payment destination reported error")
+ else:
+ self.network.path_finder.add_to_blacklist(short_chan_id)
+ else:
+ # probably got "update_fail_malformed_htlc". well... who to penalise now?
+ assert payment_attempt.error_reason is not None
+ sender_idx = None
+ failure_msg = payment_attempt.error_reason
+ is_blacklisted = False
failure_log = PaymentAttemptFailureDetails(sender_idx=sender_idx,
failure_msg=failure_msg,
- is_blacklisted=blacklist)
+ is_blacklisted=is_blacklisted)
return PaymentAttemptLog(route=route,
- success=success,
- preimage=preimage,
+ success=payment_attempt.success,
+ preimage=payment_attempt.preimage,
failure_details=failure_log)
def handle_error_code_from_failed_htlc(self, failure_msg, sender_idx, route, peer):
t@@ -1205,10 +1210,10 @@ class LNWallet(LNWorker):
if status in SAVED_PR_STATUS:
self.set_payment_status(bfh(key), status)
- async def await_payment(self, payment_hash):
- success, preimage, reason = await self.pending_payments[payment_hash]
+ async def await_payment(self, payment_hash: bytes) -> BarePaymentAttemptLog:
+ payment_attempt = await self.pending_payments[payment_hash]
self.pending_payments.pop(payment_hash)
- return success, preimage, reason
+ return payment_attempt
def set_payment_status(self, payment_hash: bytes, status):
try:
t@@ -1219,12 +1224,12 @@ class LNWallet(LNWorker):
info = info._replace(status=status)
self.save_payment_info(info)
- def payment_failed(self, chan, payment_hash: bytes, reason: bytes):
+ def payment_failed(self, chan, payment_hash: bytes, payment_attempt: BarePaymentAttemptLog):
self.set_payment_status(payment_hash, PR_UNPAID)
key = payment_hash.hex()
f = self.pending_payments.get(payment_hash)
if f and not f.cancelled():
- f.set_result((False, None, reason))
+ f.set_result(payment_attempt)
else:
chan.logger.info('received unexpected payment_failed, probably from previous session')
self.network.trigger_callback('invoice_status', key)
t@@ -1237,7 +1242,10 @@ class LNWallet(LNWorker):
key = payment_hash.hex()
f = self.pending_payments.get(payment_hash)
if f and not f.cancelled():
- f.set_result((True, preimage, None))
+ payment_attempt = BarePaymentAttemptLog(success=True,
+ preimage=preimage,
+ error_bytes=None)
+ f.set_result(payment_attempt)
else:
chan.logger.info('received unexpected payment_sent, probably from previous session')
self.network.trigger_callback('invoice_status', key)
DIR diff --git a/electrum/tests/test_lnchannel.py b/electrum/tests/test_lnchannel.py
t@@ -619,7 +619,7 @@ class TestAvailableToSpend(ElectrumTestCase):
bob_idx = bob_channel.receive_htlc(htlc_dict).htlc_id
force_state_transition(alice_channel, bob_channel)
bob_channel.fail_htlc(bob_idx)
- alice_channel.receive_fail_htlc(alice_idx, None)
+ alice_channel.receive_fail_htlc(alice_idx, error_bytes=None)
# Alice now has gotten all her original balance (5 BTC) back, however,
# adding a new HTLC at this point SHOULD fail, since if she adds the
# HTLC and signs the next state, Bob cannot assume she received the