From 6506abf58370ff18efa4644f5583c97cf092d3b2 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Fri, 27 Oct 2023 16:01:23 +0000 Subject: [PATCH] lnworker: use PaymentFeeBudget - introduce PaymentFeeBudget, which contains limits for fee budget and cltv budget - when splitting a payment, - the fee budget is linearly distributed between the parts - this resolves a FIXME in lnrouter ("FIXME in case of MPP") - the cltv budget is simply copied - we could also add other kinds of budgets later, e.g. for the num in-flight htlcs - resolves TODO in lnworker ("todo: compare to the fee of the actual route we found") --- electrum/lnpeer.py | 26 ++++++++++-------- electrum/lnrouter.py | 41 +++++++++++++++------------- electrum/lnutil.py | 16 +++++++++++ electrum/lnworker.py | 50 ++++++++++++++++++++--------------- electrum/tests/test_lnpeer.py | 8 +++--- electrum/trampoline.py | 12 ++++++--- 6 files changed, 95 insertions(+), 58 deletions(-) diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py index 720e18bf5..afdd4bf31 100644 --- a/electrum/lnpeer.py +++ b/electrum/lnpeer.py @@ -43,7 +43,7 @@ from .lnutil import (Outpoint, LocalConfig, RECEIVED, UpdateAddHtlc, ChannelConf RemoteMisbehaving, ShortChannelID, IncompatibleLightningFeatures, derive_payment_secret_from_payment_preimage, ChannelType, LNProtocolWarning, validate_features, IncompatibleOrInsaneFeatures) -from .lnutil import FeeUpdate, channel_id_from_funding_tx +from .lnutil import FeeUpdate, channel_id_from_funding_tx, PaymentFeeBudget from .lntransport import LNTransport, LNTransportBase from .lnmsg import encode_msg, decode_msg, UnknownOptionalMsgType, FailedToParseMsg from .interface import GracefulDisconnect @@ -1795,11 +1795,13 @@ class Peer(Logger): # these are the fee/cltv paid by the sender # pay_to_node will raise if they are not sufficient - trampoline_cltv_delta = inc_cltv_abs - out_cltv_abs # cltv budget total_msat = outer_onion.hop_data.payload["payment_data"]["total_msat"] - trampoline_fee = total_msat - amt_to_forward - self.logger.info(f'trampoline forwarding. fee_budget={trampoline_fee}') - self.logger.info(f'trampoline forwarding. cltv_budget={trampoline_cltv_delta}. (inc={inc_cltv_abs}. out={out_cltv_abs})') + budget = PaymentFeeBudget( + fee_msat=total_msat - amt_to_forward, + cltv=inc_cltv_abs - out_cltv_abs, + ) + self.logger.info(f'trampoline forwarding. budget={budget}') + self.logger.info(f'trampoline forwarding. {inc_cltv_abs=}, {out_cltv_abs=}') # To convert abs vs rel cltvs, we need to guess blockheight used by original sender as "current blockheight". # Blocks might have been mined since. # - if we skew towards the past, we decrease our own cltv_budget accordingly (which is ok) @@ -1809,22 +1811,24 @@ class Peer(Logger): local_height_of_onion_creator = self.network.get_local_height() - 1 cltv_budget_for_rest_of_route = out_cltv_abs - local_height_of_onion_creator + if budget.fee_msat < 1000: + raise OnionRoutingFailure(code=OnionFailureCode.TRAMPOLINE_FEE_INSUFFICIENT, data=b'') + if budget.cltv < 576: + raise OnionRoutingFailure(code=OnionFailureCode.TRAMPOLINE_EXPIRY_TOO_SOON, data=b'') + try: await self.lnworker.pay_to_node( node_pubkey=outgoing_node_id, payment_hash=payment_hash, payment_secret=payment_secret, amount_to_pay=amt_to_forward, - # FIXME this API (min_final_cltv_delta) is confusing. The value will be added to local_height - # to form the abs cltv used on the last edge on the path to the *next trampoline* node. - # We should rewrite pay_to_node to operate on a cltv-budget (and fee-budget). min_final_cltv_delta=cltv_budget_for_rest_of_route, r_tags=r_tags, invoice_features=invoice_features, fwd_trampoline_onion=next_trampoline_onion, - fwd_trampoline_fee=trampoline_fee, - fwd_trampoline_cltv_delta=trampoline_cltv_delta, - attempts=1) + budget=budget, + attempts=1, + ) except OnionRoutingFailure as e: raise except PaymentFailure as e: diff --git a/electrum/lnrouter.py b/electrum/lnrouter.py index 8be752b1a..06ac795b4 100644 --- a/electrum/lnrouter.py +++ b/electrum/lnrouter.py @@ -35,7 +35,7 @@ from math import inf from .util import profiler, with_lock from .logging import Logger from .lnutil import (NUM_MAX_EDGES_IN_PAYMENT_PATH, ShortChannelID, LnFeatures, - NBLOCK_CLTV_DELTA_TOO_FAR_INTO_FUTURE) + NBLOCK_CLTV_DELTA_TOO_FAR_INTO_FUTURE, PaymentFeeBudget) from .channel_db import ChannelDB, Policy, NodeInfo if TYPE_CHECKING: @@ -111,7 +111,7 @@ class RouteEdge(PathEdge): if self.cltv_delta > 14 * 144: return False total_fee = self.fee_for_edge(amount_msat) - if not is_fee_sane(total_fee, payment_amount_msat=amount_msat): + if total_fee > get_default_fee_budget_msat(invoice_amount_msat=amount_msat): return False return True @@ -138,38 +138,41 @@ LNPaymentRoute = Sequence[RouteEdge] LNPaymentTRoute = Sequence[TrampolineEdge] -def is_route_sane_to_use(route: LNPaymentRoute, *, amount_msat_for_dest: int, cltv_delta_for_dest: int) -> bool: +def is_route_within_budget( + route: LNPaymentRoute, + *, + budget: PaymentFeeBudget, + amount_msat_for_dest: int, # that final receiver gets + cltv_delta_for_dest: int, # that final receiver gets +) -> bool: """Run some sanity checks on the whole route, before attempting to use it. called when we are paying; so e.g. lower cltv is better """ if len(route) > NUM_MAX_EDGES_IN_PAYMENT_PATH: return False amt = amount_msat_for_dest - cltv_delta = cltv_delta_for_dest + cltv_cost_of_route = 0 # excluding cltv_delta_for_dest for route_edge in reversed(route[1:]): if not route_edge.is_sane_to_use(amt): return False amt += route_edge.fee_for_edge(amt) - cltv_delta += route_edge.cltv_delta - total_fee = amt - amount_msat_for_dest - # TODO revise ad-hoc heuristics - if cltv_delta > NBLOCK_CLTV_DELTA_TOO_FAR_INTO_FUTURE: + cltv_cost_of_route += route_edge.cltv_delta + fee_cost = amt - amount_msat_for_dest + # check against budget + if cltv_cost_of_route > budget.cltv: return False - # FIXME in case of MPP, the fee checks are done independently for each part, - # which is ok for the proportional checks but not for the absolute ones. - # This is not that big of a deal though as we don't split into *too many* parts. - if not is_fee_sane(total_fee, payment_amount_msat=amount_msat_for_dest): + if fee_cost > budget.fee_msat: + return False + # sanity check + total_cltv_delta = cltv_cost_of_route + cltv_delta_for_dest + if total_cltv_delta > NBLOCK_CLTV_DELTA_TOO_FAR_INTO_FUTURE: return False return True -def is_fee_sane(fee_msat: int, *, payment_amount_msat: int) -> bool: - # fees <= 5 sat are fine - if fee_msat <= 5_000: - return True +def get_default_fee_budget_msat(*, invoice_amount_msat: int) -> int: # fees <= 1 % of payment are fine - if 100 * fee_msat <= payment_amount_msat: - return True - return False + # fees <= 5 sat are fine + return max(5_000, invoice_amount_msat // 100) class LiquidityHint: diff --git a/electrum/lnutil.py b/electrum/lnutil.py index eaa33ee62..22d144717 100644 --- a/electrum/lnutil.py +++ b/electrum/lnutil.py @@ -1637,3 +1637,19 @@ class OnionFailureCodeMetaFlag(IntFlag): UPDATE = 0x1000 +class PaymentFeeBudget(NamedTuple): + fee_msat: int + + # The cltv budget covers the cost of route to get to the destination, but excluding the + # cltv-delta the destination wants for itself. (e.g. "min_final_cltv_delta" is excluded) + cltv: int # this is cltv-delta-like, no absolute heights here! + + #num_htlc: int + + @classmethod + def default(cls, *, invoice_amount_msat: int) -> 'PaymentFeeBudget': + from .lnrouter import get_default_fee_budget_msat + return PaymentFeeBudget( + fee_msat=get_default_fee_budget_msat(invoice_amount_msat=invoice_amount_msat), + cltv=NBLOCK_CLTV_DELTA_TOO_FAR_INTO_FUTURE, + ) diff --git a/electrum/lnworker.py b/electrum/lnworker.py index 06844c9cf..95c32f4d4 100644 --- a/electrum/lnworker.py +++ b/electrum/lnworker.py @@ -66,12 +66,12 @@ from .lnutil import (Outpoint, LNPeerAddr, UpdateAddHtlc, Direction, LnFeatures, ShortChannelID, HtlcLog, derive_payment_secret_from_payment_preimage, NoPathFound, InvalidGossipMsg) -from .lnutil import ln_compare_features, IncompatibleLightningFeatures +from .lnutil import ln_compare_features, IncompatibleLightningFeatures, PaymentFeeBudget from .transaction import PartialTxOutput, PartialTransaction, PartialTxInput from .lnonion import decode_onion_error, OnionFailureCode, OnionRoutingFailure, OnionPacket from .lnmsg import decode_msg from .i18n import _ -from .lnrouter import (RouteEdge, LNPaymentRoute, LNPaymentPath, is_route_sane_to_use, +from .lnrouter import (RouteEdge, LNPaymentRoute, LNPaymentPath, is_route_within_budget, NoChannelPolicy, LNPathInconsistent) from .address_synchronizer import TX_HEIGHT_LOCAL, TX_TIMESTAMP_INF from . import lnsweep @@ -662,7 +662,7 @@ class PaySession(Logger): initial_trampoline_fee_level: int, invoice_features: int, r_tags, - min_final_cltv_delta: int, # delta for last edge (typically from invoice) + min_final_cltv_delta: int, # delta for last node (typically from invoice) amount_to_pay: int, # total payment amount final receiver will get invoice_pubkey: bytes, uses_trampoline: bool, # whether sender uses trampoline or gossip @@ -1419,6 +1419,7 @@ class LNWallet(LNWorker): f"using_trampoline={self.uses_trampoline()}. " f"invoice_features={invoice_features.get_names()}") self.set_invoice_status(key, PR_INFLIGHT) + budget = PaymentFeeBudget.default(invoice_amount_msat=amount_to_pay) success = False try: await self.pay_to_node( @@ -1431,7 +1432,9 @@ class LNWallet(LNWorker): invoice_features=invoice_features, attempts=attempts, full_path=full_path, - channels=channels) + channels=channels, + budget=budget, + ) success = True except PaymentFailure as e: self.logger.info(f'payment failure: {e!r}') @@ -1462,17 +1465,13 @@ class LNWallet(LNWorker): attempts: int = None, full_path: LNPaymentPath = None, fwd_trampoline_onion: OnionPacket = None, - fwd_trampoline_fee: int = None, - fwd_trampoline_cltv_delta: int = None, + budget: PaymentFeeBudget, channels: Optional[Sequence[Channel]] = None, ) -> None: - if fwd_trampoline_onion: - # todo: compare to the fee of the actual route we found - if fwd_trampoline_fee < 1000: - raise OnionRoutingFailure(code=OnionFailureCode.TRAMPOLINE_FEE_INSUFFICIENT, data=b'') - if fwd_trampoline_cltv_delta < 576: - raise OnionRoutingFailure(code=OnionFailureCode.TRAMPOLINE_EXPIRY_TOO_SOON, data=b'') + assert budget + assert budget.fee_msat >= 0, budget + assert budget.cltv >= 0, budget payment_key = payment_hash + payment_secret assert payment_key not in self._paysessions @@ -1499,12 +1498,14 @@ class LNWallet(LNWorker): # 1. create a set of routes for remaining amount. # note: path-finding runs in a separate thread so that we don't block the asyncio loop # graph updates might occur during the computation + remaining_fee_budget_msat = (budget.fee_msat * amount_to_send) // amount_to_pay routes = self.create_routes_for_payment( paysession=paysession, amount_msat=amount_to_send, full_path=full_path, fwd_trampoline_onion=fwd_trampoline_onion, channels=channels, + budget=budget._replace(fee_msat=remaining_fee_budget_msat), ) # 2. send htlcs async for sent_htlc_info, cltv_delta, trampoline_onion in routes: @@ -1815,6 +1816,7 @@ class LNWallet(LNWorker): fwd_trampoline_onion: OnionPacket = None, full_path: LNPaymentPath = None, channels: Optional[Sequence[Channel]] = None, + budget: PaymentFeeBudget, ) -> AsyncGenerator[Tuple[SentHtlcInfo, int, Optional[OnionPacket]], None]: """Creates multiple routes for splitting a payment over the available @@ -1853,7 +1855,7 @@ class LNWallet(LNWorker): try: if self.uses_trampoline(): per_trampoline_channel_amounts = defaultdict(list) - # categorize by trampoline nodes for trampolin mpp construction + # categorize by trampoline nodes for trampoline mpp construction for (chan_id, _), part_amounts_msat in sc.config.items(): chan = self.channels[chan_id] for part_amount_msat in part_amounts_msat: @@ -1883,7 +1885,9 @@ class LNWallet(LNWorker): local_height=local_height, trampoline_fee_level=paysession.trampoline_fee_level, use_two_trampolines=paysession.use_two_trampolines, - failed_routes=paysession.failed_trampoline_routes) + failed_routes=paysession.failed_trampoline_routes, + budget=budget._replace(fee_msat=budget.fee_msat // len(per_trampoline_channel_amounts)), + ) # node_features is only used to determine is_tlv per_trampoline_secret = os.urandom(32) per_trampoline_fees = per_trampoline_amount_with_fees - per_trampoline_amount @@ -1930,7 +1934,7 @@ class LNWallet(LNWorker): channel = self.channels[chan_id] route = await run_in_thread( partial( - self.create_route_for_payment, + self.create_route_for_single_htlc, amount_msat=part_amount_msat, invoice_pubkey=paysession.invoice_pubkey, min_final_cltv_delta=paysession.min_final_cltv_delta, @@ -1938,6 +1942,7 @@ class LNWallet(LNWorker): invoice_features=paysession.invoice_features, my_sending_channels=[channel] if is_multichan_mpp else my_active_channels, full_path=full_path, + budget=budget._replace(fee_msat=budget.fee_msat // sc.config.number_parts()), ) ) shi = SentHtlcInfo( @@ -1959,15 +1964,17 @@ class LNWallet(LNWorker): raise NoPathFound() @profiler - def create_route_for_payment( + def create_route_for_single_htlc( self, *, - amount_msat: int, + amount_msat: int, # that final receiver gets invoice_pubkey: bytes, min_final_cltv_delta: int, r_tags, invoice_features: int, my_sending_channels: List[Channel], - full_path: Optional[LNPaymentPath]) -> LNPaymentRoute: + full_path: Optional[LNPaymentPath], + budget: PaymentFeeBudget, + ) -> LNPaymentRoute: my_sending_aliases = set(chan.get_local_scid_alias() for chan in my_sending_channels) my_sending_channels = {chan.short_channel_id: chan for chan in my_sending_channels @@ -2022,9 +2029,10 @@ class LNWallet(LNWorker): raise NoPathFound() from e if not route: raise NoPathFound() - # test sanity - if not is_route_sane_to_use(route, amount_msat_for_dest=amount_msat, cltv_delta_for_dest=min_final_cltv_delta): - self.logger.info(f"rejecting insane route {route}") + if not is_route_within_budget( + route, budget=budget, amount_msat_for_dest=amount_msat, cltv_delta_for_dest=min_final_cltv_delta, + ): + self.logger.info(f"rejecting route (exceeds budget): {route=}. {budget=}") raise NoPathFound() assert len(route) > 0 if route[-1].end_node != invoice_pubkey: diff --git a/electrum/tests/test_lnpeer.py b/electrum/tests/test_lnpeer.py index 9224d4923..2fa1a5c52 100644 --- a/electrum/tests/test_lnpeer.py +++ b/electrum/tests/test_lnpeer.py @@ -27,7 +27,7 @@ from electrum.bitcoin import COIN, sha256 from electrum.util import NetworkRetryManager, bfh, OldTaskGroup, EventListener, InvoiceError from electrum.lnpeer import Peer from electrum.lnutil import LNPeerAddr, Keypair, privkey_to_pubkey -from electrum.lnutil import PaymentFailure, LnFeatures, HTLCOwner +from electrum.lnutil import PaymentFailure, LnFeatures, HTLCOwner, PaymentFeeBudget from electrum.lnchannel import ChannelState, PeerState, Channel from electrum.lnrouter import LNPathFinder, PathEdge, LNPathInconsistent from electrum.channel_db import ChannelDB @@ -250,7 +250,9 @@ class MockLNWallet(Logger, EventListener, NetworkRetryManager[LNPeerAddr]): return [r async for r in self.create_routes_for_payment( amount_msat=amount_msat, paysession=paysession, - full_path=full_path)] + full_path=full_path, + budget=PaymentFeeBudget.default(invoice_amount_msat=amount_msat), + )] get_payments = LNWallet.get_payments get_payment_secret = LNWallet.get_payment_secret @@ -265,7 +267,7 @@ class MockLNWallet(Logger, EventListener, NetworkRetryManager[LNPeerAddr]): htlc_failed = LNWallet.htlc_failed save_preimage = LNWallet.save_preimage get_preimage = LNWallet.get_preimage - create_route_for_payment = LNWallet.create_route_for_payment + create_route_for_single_htlc = LNWallet.create_route_for_single_htlc create_routes_for_payment = LNWallet.create_routes_for_payment _check_invoice = LNWallet._check_invoice pay_to_route = LNWallet.pay_to_route diff --git a/electrum/trampoline.py b/electrum/trampoline.py index 04fa45f55..73226bdbd 100644 --- a/electrum/trampoline.py +++ b/electrum/trampoline.py @@ -4,9 +4,9 @@ import random from typing import Mapping, DefaultDict, Tuple, Optional, Dict, List, Iterable, Sequence, Set -from .lnutil import LnFeatures +from .lnutil import LnFeatures, PaymentFeeBudget from .lnonion import calc_hops_data_for_payment, new_onion_packet, OnionPacket -from .lnrouter import RouteEdge, TrampolineEdge, LNPaymentRoute, is_route_sane_to_use, LNPaymentTRoute +from .lnrouter import RouteEdge, TrampolineEdge, LNPaymentRoute, is_route_within_budget, LNPaymentTRoute from .lnutil import NoPathFound, LNPeerAddr from . import constants from .logging import get_logger @@ -222,6 +222,7 @@ def create_trampoline_route( trampoline_fee_level: int, use_two_trampolines: bool, failed_routes: Iterable[Sequence[str]], + budget: PaymentFeeBudget, ) -> LNPaymentTRoute: # we decide whether to convert to a legacy payment is_legacy, invoice_trampolines = is_legacy_relay(invoice_features, r_tags) @@ -268,12 +269,13 @@ def create_trampoline_route( # Also needed for fees for last TF! _extend_trampoline_route(route, end_node=invoice_pubkey, trampoline_fee_level=trampoline_fee_level) # check that we can pay amount and fees - if not is_route_sane_to_use( + if not is_route_within_budget( route=route, + budget=budget, amount_msat_for_dest=amount_msat, cltv_delta_for_dest=min_final_cltv_delta, ): - raise NoPathFound("We cannot afford to pay the fees.") + raise NoPathFound("route exceeds budget") return route @@ -342,6 +344,7 @@ def create_trampoline_route_and_onion( trampoline_fee_level: int, use_two_trampolines: bool, failed_routes: Iterable[Sequence[str]], + budget: PaymentFeeBudget, ) -> Tuple[LNPaymentTRoute, OnionPacket, int, int]: # create route for the trampoline_onion trampoline_route = create_trampoline_route( @@ -355,6 +358,7 @@ def create_trampoline_route_and_onion( trampoline_fee_level=trampoline_fee_level, use_two_trampolines=use_two_trampolines, failed_routes=failed_routes, + budget=budget, ) # compute onion and fees final_cltv_abs = local_height + min_final_cltv_delta