From 79d2b19fc0ad91db517a4ebd243b2f5ac880f38b Mon Sep 17 00:00:00 2001 From: SomberNight Date: Mon, 6 May 2024 18:36:29 +0000 Subject: [PATCH 1/4] trampoline: rm hardcoded TRAMPOLINE_FEES. just use exponential search Values for exponential search are based on available fee budget: we try with budget/64, budget/32, ..., budget/1 (spread uniformly among the selected Trampoline Forwarders). Hence, if we make the fee budget configurable, that will usefully affect the trampoline fees as well. related https://github.com/spesmilo/electrum/issues/9033 --- electrum/lnworker.py | 6 +-- electrum/trampoline.py | 105 +++++++++++++++++------------------------ 2 files changed, 45 insertions(+), 66 deletions(-) diff --git a/electrum/lnworker.py b/electrum/lnworker.py index 7fb40590d..5589f4083 100644 --- a/electrum/lnworker.py +++ b/electrum/lnworker.py @@ -86,7 +86,7 @@ from .channel_db import get_mychannel_info, get_mychannel_policy from .submarine_swaps import SwapManager from .channel_db import ChannelInfo, Policy from .mpp_split import suggest_splits, SplitConfigRating -from .trampoline import create_trampoline_route_and_onion, TRAMPOLINE_FEES, is_legacy_relay +from .trampoline import create_trampoline_route_and_onion, is_legacy_relay if TYPE_CHECKING: from .network import Network @@ -2630,8 +2630,8 @@ class LNWallet(LNWorker): def fee_estimate(self, amount_sat): # Here we have to guess a fee, because some callers (submarine swaps) # use this method to initiate a payment, which would otherwise fail. - fee_base_msat = TRAMPOLINE_FEES[3]['fee_base_msat'] - fee_proportional_millionths = TRAMPOLINE_FEES[3]['fee_proportional_millionths'] + fee_base_msat = 5000 # FIXME ehh.. there ought to be a better way... + fee_proportional_millionths = 500 # FIXME # inverse of fee_for_edge_msat amount_msat = amount_sat * 1000 amount_minus_fees = (amount_msat - fee_base_msat) * 1_000_000 // ( 1_000_000 + fee_proportional_millionths) diff --git a/electrum/trampoline.py b/electrum/trampoline.py index 39a3cc905..b8a6208ef 100644 --- a/electrum/trampoline.py +++ b/electrum/trampoline.py @@ -13,45 +13,6 @@ from .logging import get_logger _logger = get_logger(__name__) -# trampoline nodes are supposed to advertise their fee and cltv in node_update message -TRAMPOLINE_FEES = [ - { - 'fee_base_msat': 0, - 'fee_proportional_millionths': 0, - 'cltv_expiry_delta': 576, - }, - { - 'fee_base_msat': 1000, - 'fee_proportional_millionths': 100, - 'cltv_expiry_delta': 576, - }, - { - 'fee_base_msat': 3000, - 'fee_proportional_millionths': 100, - 'cltv_expiry_delta': 576, - }, - { - 'fee_base_msat': 5000, - 'fee_proportional_millionths': 500, - 'cltv_expiry_delta': 576, - }, - { - 'fee_base_msat': 7000, - 'fee_proportional_millionths': 1000, - 'cltv_expiry_delta': 576, - }, - { - 'fee_base_msat': 12000, - 'fee_proportional_millionths': 3000, - 'cltv_expiry_delta': 576, - }, - { - 'fee_base_msat': 100000, - 'fee_proportional_millionths': 3000, - 'cltv_expiry_delta': 576, - }, -] - # hardcoded list # TODO for some pubkeys, there are multiple network addresses we could try TRAMPOLINE_NODES_MAINNET = { @@ -156,27 +117,12 @@ def is_legacy_relay(invoice_features, r_tags) -> Tuple[bool, Set[bytes]]: return True, set() -def trampoline_policy( - trampoline_fee_level: int, -) -> Dict: - """Return the fee policy for all trampoline nodes. - - Raises NoPathFound if the fee level is exhausted.""" - # TODO: ideally we want to use individual fee levels for each trampoline node, - # but because at the moment we can't attribute insufficient fee errors to - # downstream trampolines we need to use a global fee level here - if trampoline_fee_level < len(TRAMPOLINE_FEES): - return TRAMPOLINE_FEES[trampoline_fee_level] - else: - raise NoPathFound() - - +PLACEHOLDER_FEE = None def _extend_trampoline_route( route: List[TrampolineEdge], *, start_node: bytes = None, end_node: bytes, - trampoline_fee_level: int, pay_fees: bool = True, ): """Extends the route and modifies it in place.""" @@ -185,17 +131,47 @@ def _extend_trampoline_route( start_node = route[-1].end_node trampoline_features = LnFeatures.VAR_ONION_OPT # get policy for *start_node* - policy = trampoline_policy(trampoline_fee_level) + # note: trampoline nodes are supposed to advertise their fee and cltv in node_update message. + # However, in the temporary spec, they do not. + # They also don't send their fee policy in the error message if we lowball the fee... route.append( TrampolineEdge( start_node=start_node, end_node=end_node, - fee_base_msat=policy['fee_base_msat'] if pay_fees else 0, - fee_proportional_millionths=policy['fee_proportional_millionths'] if pay_fees else 0, - cltv_delta=policy['cltv_expiry_delta'] if pay_fees else 0, + fee_base_msat=PLACEHOLDER_FEE if pay_fees else 0, + fee_proportional_millionths=PLACEHOLDER_FEE if pay_fees else 0, + cltv_delta=576 if pay_fees else 0, node_features=trampoline_features)) +def _allocate_fee_along_route( + route: List[TrampolineEdge], + *, + budget: PaymentFeeBudget, + trampoline_fee_level: int, +) -> None: + # calculate budget_to_use, based on given max available "budget" + if trampoline_fee_level == 0: + budget_to_use = 0 + else: + assert trampoline_fee_level > 0 + MAX_LEVEL = 6 + if trampoline_fee_level > MAX_LEVEL: + raise NoPathFound() + budget_to_use = budget.fee_msat // (2 ** (MAX_LEVEL - trampoline_fee_level)) + _logger.debug(f"_allocate_fee_along_route(). {trampoline_fee_level=}, {budget.fee_msat=}, {budget_to_use=}") + # replace placeholder fees + for edge in route: + assert edge.fee_base_msat in (0, PLACEHOLDER_FEE), edge.fee_base_msat + assert edge.fee_proportional_millionths in (0, PLACEHOLDER_FEE), edge.fee_proportional_millionths + edges_to_update = [ + edge for edge in route + if edge.fee_base_msat == PLACEHOLDER_FEE] + for edge in edges_to_update: + edge.fee_base_msat = budget_to_use // len(edges_to_update) + edge.fee_proportional_millionths = 0 + + def _choose_second_trampoline( my_trampoline: bytes, trampolines: Iterable[bytes], @@ -237,7 +213,7 @@ def create_trampoline_route( # our first trampoline hop is decided by the channel we use _extend_trampoline_route( route, start_node=my_pubkey, end_node=my_trampoline, - trampoline_fee_level=trampoline_fee_level, pay_fees=False, + pay_fees=False, ) if is_legacy: @@ -245,7 +221,7 @@ def create_trampoline_route( if use_two_trampolines: trampolines = trampolines_by_id() second_trampoline = _choose_second_trampoline(my_trampoline, list(trampolines.keys()), failed_routes) - _extend_trampoline_route(route, end_node=second_trampoline, trampoline_fee_level=trampoline_fee_level) + _extend_trampoline_route(route, end_node=second_trampoline) # the last trampoline onion must contain routing hints for the last trampoline # node to find the recipient invoice_routing_info = encode_routing_info(r_tags) @@ -267,12 +243,15 @@ def create_trampoline_route( add_trampoline = True if add_trampoline: second_trampoline = _choose_second_trampoline(my_trampoline, invoice_trampolines, failed_routes) - _extend_trampoline_route(route, end_node=second_trampoline, trampoline_fee_level=trampoline_fee_level) + _extend_trampoline_route(route, end_node=second_trampoline) # Add final edge. note: eclair requires an encrypted t-onion blob even in legacy case. # Also needed for fees for last TF! if route[-1].end_node != invoice_pubkey: - _extend_trampoline_route(route, end_node=invoice_pubkey, trampoline_fee_level=trampoline_fee_level) + _extend_trampoline_route(route, end_node=invoice_pubkey) + + # replace placeholder fees in route + _allocate_fee_along_route(route, budget=budget, trampoline_fee_level=trampoline_fee_level) # check that we can pay amount and fees if not is_route_within_budget( From 67d373357b751bf521f6784590598176a2d3b498 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Mon, 6 May 2024 20:16:05 +0000 Subject: [PATCH 2/4] lnworker: make PaymentFeeBudget defaults configurable - make PaymentFeeBudget proportional fee and flat cutoff fee configurable - closes https://github.com/spesmilo/electrum/issues/7622 - increase flat cutoff fee default to 10 sat - closes https://github.com/spesmilo/electrum/issues/7669 - rm RouteEdge.is_sane_to_use() (per edge limit) and just rely on budgets (per route limit) --- electrum/lnrouter.py | 22 +++------------------- electrum/lnutil.py | 21 ++++++++++++++++++--- electrum/lnworker.py | 2 +- electrum/simple_config.py | 10 ++++++++++ tests/test_lnpeer.py | 2 +- 5 files changed, 33 insertions(+), 24 deletions(-) diff --git a/electrum/lnrouter.py b/electrum/lnrouter.py index 06ac795b4..afd813ea3 100644 --- a/electrum/lnrouter.py +++ b/electrum/lnrouter.py @@ -105,16 +105,6 @@ class RouteEdge(PathEdge): cltv_delta=channel_policy.cltv_delta, node_features=node_info.features if node_info else 0) - def is_sane_to_use(self, amount_msat: int) -> bool: - # TODO revise ad-hoc heuristics - # cltv cannot be more than 2 weeks - if self.cltv_delta > 14 * 144: - return False - total_fee = self.fee_for_edge(amount_msat) - if total_fee > get_default_fee_budget_msat(invoice_amount_msat=amount_msat): - return False - return True - def has_feature_varonion(self) -> bool: features = LnFeatures(self.node_features) return features.supports(LnFeatures.VAR_ONION_OPT) @@ -153,7 +143,6 @@ def is_route_within_budget( amt = amount_msat_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_cost_of_route += route_edge.cltv_delta fee_cost = amt - amount_msat_for_dest @@ -169,12 +158,6 @@ def is_route_within_budget( return True -def get_default_fee_budget_msat(*, invoice_amount_msat: int) -> int: - # fees <= 1 % of payment are fine - # fees <= 5 sat are fine - return max(5_000, invoice_amount_msat // 100) - - class LiquidityHint: """Encodes the amounts that can and cannot be sent over the direction of a channel. @@ -520,8 +503,9 @@ class LNPathFinder(Logger): start_node=start_node, end_node=end_node, node_info=node_info) - if not route_edge.is_sane_to_use(payment_amt_msat): - return float('inf'), 0 # thanks but no thanks + # Cap cltv of any given edge at 2 weeks (the cost function would not work well for extreme cases) + if route_edge.cltv_delta > 14 * 144: + return float('inf'), 0 # Distance metric notes: # TODO constants are ad-hoc # ( somewhat based on https://github.com/lightningnetwork/lnd/pull/1358 ) # - Edges have a base cost. (more edges -> less likely none will fail) diff --git a/electrum/lnutil.py b/electrum/lnutil.py index 102548c58..8d1c020da 100644 --- a/electrum/lnutil.py +++ b/electrum/lnutil.py @@ -1672,9 +1672,24 @@ class PaymentFeeBudget(NamedTuple): #num_htlc: int @classmethod - def default(cls, *, invoice_amount_msat: int) -> 'PaymentFeeBudget': - from .lnrouter import get_default_fee_budget_msat + def default(cls, *, invoice_amount_msat: int, config: 'SimpleConfig') -> 'PaymentFeeBudget': + millionths_orig = config.LIGHTNING_PAYMENT_FEE_MAX_MILLIONTHS + millionths = min(max(0, millionths_orig), 250_000) # clamp into [0, 25%] + cutoff_orig = config.LIGHTNING_PAYMENT_FEE_CUTOFF_MSAT + cutoff = min(max(0, cutoff_orig), 10_000_000) # clamp into [0, 10k sat] + if millionths != millionths_orig: + _logger.warning( + f"PaymentFeeBudget. found insane fee millionths in config. " + f"clamped: {millionths_orig}->{millionths}") + if cutoff != cutoff_orig: + _logger.warning( + f"PaymentFeeBudget. found insane fee cutoff in config. " + f"clamped: {cutoff_orig}->{cutoff}") + # for small payments, fees <= constant cutoff are fine + # for large payments, the max fee is percentage-based + fee_msat = invoice_amount_msat * millionths // 1_000_000 + fee_msat = max(fee_msat, cutoff) return PaymentFeeBudget( - fee_msat=get_default_fee_budget_msat(invoice_amount_msat=invoice_amount_msat), + fee_msat=fee_msat, cltv=NBLOCK_CLTV_DELTA_TOO_FAR_INTO_FUTURE, ) diff --git a/electrum/lnworker.py b/electrum/lnworker.py index 5589f4083..33bdb6c14 100644 --- a/electrum/lnworker.py +++ b/electrum/lnworker.py @@ -1516,7 +1516,7 @@ class LNWallet(LNWorker): f"num_channels={self.channel_db.num_channels}, " f"num_policies={self.channel_db.num_policies}.") self.set_invoice_status(key, PR_INFLIGHT) - budget = PaymentFeeBudget.default(invoice_amount_msat=amount_to_pay) + budget = PaymentFeeBudget.default(invoice_amount_msat=amount_to_pay, config=self.config) success = False try: await self.pay_to_node( diff --git a/electrum/simple_config.py b/electrum/simple_config.py index 3b85373d5..bbaf6b23b 100644 --- a/electrum/simple_config.py +++ b/electrum/simple_config.py @@ -1040,6 +1040,16 @@ Note you are at risk of losing the funds in the swap, if the funding transaction This will result in longer routes; it might increase your fees and decrease the success rate of your payments."""), ) INITIAL_TRAMPOLINE_FEE_LEVEL = ConfigVar('initial_trampoline_fee_level', default=1, type_=int) + LIGHTNING_PAYMENT_FEE_MAX_MILLIONTHS = ConfigVar( + 'lightning_payment_fee_max_millionths', default=10_000, # 1% + type_=int, + short_desc=lambda: _("Max lightning fees (%) to pay"), + ) + LIGHTNING_PAYMENT_FEE_CUTOFF_MSAT = ConfigVar( + 'lightning_payment_fee_cutoff_msat', default=10_000, # 10 sat + type_=int, + short_desc=lambda: _("Max lightning fees to pay for small payments"), + ) LIGHTNING_NODE_ALIAS = ConfigVar('lightning_node_alias', default='', type_=str) EXPERIMENTAL_LN_FORWARD_PAYMENTS = ConfigVar('lightning_forward_payments', default=False, type_=bool) diff --git a/tests/test_lnpeer.py b/tests/test_lnpeer.py index 1eae2580d..fa0b8c206 100644 --- a/tests/test_lnpeer.py +++ b/tests/test_lnpeer.py @@ -258,7 +258,7 @@ class MockLNWallet(Logger, EventListener, NetworkRetryManager[LNPeerAddr]): amount_msat=amount_msat, paysession=paysession, full_path=full_path, - budget=PaymentFeeBudget.default(invoice_amount_msat=amount_msat), + budget=PaymentFeeBudget.default(invoice_amount_msat=amount_msat, config=self.config), )] get_payments = LNWallet.get_payments From 967ceb77408562884f7297a0415c7d4823fe8459 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Mon, 6 May 2024 22:04:33 +0000 Subject: [PATCH 3/4] lnworker: move around some logging re PaySession, also log budget --- electrum/lnworker.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/electrum/lnworker.py b/electrum/lnworker.py index 33bdb6c14..1d43065bf 100644 --- a/electrum/lnworker.py +++ b/electrum/lnworker.py @@ -1505,16 +1505,6 @@ class LNWallet(LNWorker): info = PaymentInfo(payment_hash, amount_to_pay, SENT, PR_UNPAID) self.save_payment_info(info) self.wallet.set_label(key, lnaddr.get_description()) - self.logger.info( - f"pay_invoice starting session for RHASH={payment_hash.hex()}. " - f"using_trampoline={self.uses_trampoline()}. " - f"invoice_features={invoice_features.get_names()}") - if not self.uses_trampoline(): - self.logger.info( - f"gossip_db status. sync progress: {self.network.lngossip.get_sync_progress_estimate()}. " - f"num_nodes={self.channel_db.num_nodes}, " - f"num_channels={self.channel_db.num_channels}, " - f"num_policies={self.channel_db.num_policies}.") self.set_invoice_status(key, PR_INFLIGHT) budget = PaymentFeeBudget.default(invoice_amount_msat=amount_to_pay, config=self.config) success = False @@ -1587,6 +1577,18 @@ class LNWallet(LNWorker): ) self.logs[payment_hash.hex()] = log = [] # TODO incl payment_secret in key (re trampoline forwarding) + paysession.logger.info( + f"pay_to_node starting session for RHASH={payment_hash.hex()}. " + f"using_trampoline={self.uses_trampoline()}. " + f"invoice_features={paysession.invoice_features.get_names()}. " + f"{amount_to_pay=} msat. {budget=}") + if not self.uses_trampoline(): + self.logger.info( + f"gossip_db status. sync progress: {self.network.lngossip.get_sync_progress_estimate()}. " + f"num_nodes={self.channel_db.num_nodes}, " + f"num_channels={self.channel_db.num_channels}, " + f"num_policies={self.channel_db.num_policies}.") + # when encountering trampoline forwarding difficulties in the legacy case, we # sometimes need to fall back to a single trampoline forwarder, at the expense # of privacy @@ -1665,6 +1667,7 @@ class LNWallet(LNWorker): paysession.is_active = False if paysession.can_be_deleted(): self._paysessions.pop(payment_key) + paysession.logger.info(f"pay_to_node ending session for RHASH={payment_hash.hex()}") async def pay_to_route( self, *, From 5b74aa443e56a41bbca64f2f764f280d131c55ba Mon Sep 17 00:00:00 2001 From: SomberNight Date: Mon, 6 May 2024 22:08:30 +0000 Subject: [PATCH 4/4] qt settings: expose LIGHTNING_PAYMENT_BUDGET_FEE_MAX_MILLIONTHS Expose as a slider; perhaps it is less footgunny this way? --- electrum/gui/qt/settings_dialog.py | 34 +++++++++++++++++++++++++++++- electrum/simple_config.py | 5 ++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/electrum/gui/qt/settings_dialog.py b/electrum/gui/qt/settings_dialog.py index f8f755467..ed67edab9 100644 --- a/electrum/gui/qt/settings_dialog.py +++ b/electrum/gui/qt/settings_dialog.py @@ -30,7 +30,7 @@ from PyQt5.QtCore import Qt from PyQt5.QtWidgets import (QComboBox, QTabWidget, QDialog, QSpinBox, QFileDialog, QCheckBox, QLabel, QVBoxLayout, QGridLayout, QLineEdit, - QPushButton, QWidget, QHBoxLayout) + QPushButton, QWidget, QHBoxLayout, QSlider) from electrum.i18n import _, languages from electrum import util, paymentrequest @@ -155,6 +155,37 @@ class SettingsDialog(QDialog, QtEventListener): self.config.WATCHTOWER_CLIENT_URL = url self.watchtower_url_e.editingFinished.connect(on_wt_url) + lnfee_hlabel = HelpLabel.from_configvar(self.config.cv.LIGHTNING_PAYMENT_FEE_MAX_MILLIONTHS) + lnfee_map = [500, 1_000, 3_000, 5_000, 10_000, 20_000, 30_000, 50_000] + def lnfee_update_vlabel(fee_val: int): + lnfee_vlabel.setText(_("{}% of payment").format(f"{fee_val / 10 ** 4:.2f}")) + def lnfee_slider_moved(): + pos = lnfee_slider.sliderPosition() + fee_val = lnfee_map[pos] + lnfee_update_vlabel(fee_val) + def lnfee_slider_released(): + pos = lnfee_slider.sliderPosition() + fee_val = lnfee_map[pos] + self.config.LIGHTNING_PAYMENT_FEE_MAX_MILLIONTHS = fee_val + lnfee_slider = QSlider(Qt.Horizontal) + lnfee_slider.setRange(0, len(lnfee_map)-1) + lnfee_slider.setTracking(True) + try: + lnfee_spos = lnfee_map.index(self.config.LIGHTNING_PAYMENT_FEE_MAX_MILLIONTHS) + except ValueError: + lnfee_spos = 0 + lnfee_slider.setSliderPosition(lnfee_spos) + lnfee_vlabel = QLabel("") + lnfee_update_vlabel(self.config.LIGHTNING_PAYMENT_FEE_MAX_MILLIONTHS) + lnfee_slider.valueChanged.connect(lnfee_slider_moved) + lnfee_slider.sliderReleased.connect(lnfee_slider_released) + lnfee_hbox = QHBoxLayout() + lnfee_hbox.setContentsMargins(0, 0, 0, 0) + lnfee_hbox.addWidget(lnfee_vlabel) + lnfee_hbox.addWidget(lnfee_slider) + lnfee_hbox_w = QWidget() + lnfee_hbox_w.setLayout(lnfee_hbox) + alias_label = HelpLabel.from_configvar(self.config.cv.OPENALIAS_ID) alias = self.config.OPENALIAS_ID self.alias_e = QLineEdit(alias) @@ -351,6 +382,7 @@ class SettingsDialog(QDialog, QtEventListener): lightning_widgets.append((trampoline_cb, None)) lightning_widgets.append((legacy_add_trampoline_cb, None)) lightning_widgets.append((remote_wt_cb, self.watchtower_url_e)) + lightning_widgets.append((lnfee_hlabel, lnfee_hbox_w)) fiat_widgets = [] fiat_widgets.append((QLabel(_('Fiat currency')), ccy_combo)) fiat_widgets.append((QLabel(_('Source')), ex_combo)) diff --git a/electrum/simple_config.py b/electrum/simple_config.py index bbaf6b23b..c7cd8918e 100644 --- a/electrum/simple_config.py +++ b/electrum/simple_config.py @@ -1043,7 +1043,10 @@ This will result in longer routes; it might increase your fees and decrease the LIGHTNING_PAYMENT_FEE_MAX_MILLIONTHS = ConfigVar( 'lightning_payment_fee_max_millionths', default=10_000, # 1% type_=int, - short_desc=lambda: _("Max lightning fees (%) to pay"), + short_desc=lambda: _("Max lightning fees to pay"), + long_desc=lambda: _("""When sending lightning payments, this value is an upper bound for the fees we allow paying, proportional to the payment amount. The fees are paid in addition to the payment amount, by the sender. + +Warning: setting this to too low will result in lots of payment failures."""), ) LIGHTNING_PAYMENT_FEE_CUTOFF_MSAT = ConfigVar( 'lightning_payment_fee_cutoff_msat', default=10_000, # 10 sat