diff --git a/electrum/lnchannel.py b/electrum/lnchannel.py index 9aca03433..432187d72 100644 --- a/electrum/lnchannel.py +++ b/electrum/lnchannel.py @@ -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 @@ -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 @@ -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: """ @@ -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()) diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py index 1e02dea6c..647db5d2f 100644 --- a/electrum/lnpeer.py +++ b/electrum/lnpeer.py @@ -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): @@ -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"] @@ -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, @@ -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 @@ -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: diff --git a/electrum/lnutil.py b/electrum/lnutil.py index 4380a0bad..85e719954 100644 --- a/electrum/lnutil.py +++ b/electrum/lnutil.py @@ -112,7 +112,7 @@ class Outpoint(StoredObject): class PaymentAttemptFailureDetails(NamedTuple): - sender_idx: int + sender_idx: Optional[int] failure_msg: 'OnionRoutingFailureMessage' is_blacklisted: bool @@ -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' @@ -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 diff --git a/electrum/lnworker.py b/electrum/lnworker.py index af974ecca..2d6dbe37f 100644 --- a/electrum/lnworker.py +++ b/electrum/lnworker.py @@ -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 @@ -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 @@ -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): @@ -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: @@ -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) @@ -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) diff --git a/electrum/tests/test_lnchannel.py b/electrum/tests/test_lnchannel.py index 9fc3a3347..ffe7e6ef1 100644 --- a/electrum/tests/test_lnchannel.py +++ b/electrum/tests/test_lnchannel.py @@ -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