From f337b4782d03a7478cf532d1da42f98cf8fbef0e Mon Sep 17 00:00:00 2001 From: SomberNight Date: Fri, 27 Jun 2025 14:00:27 +0000 Subject: [PATCH] lnwatcher: keep watching sweep TXOs that are dust due to high fees - if fee estimates are high atm, some outputs are not worth to sweep - however, fee estimates might be only-temporarily very high - previously in such a case lnwatcher would just discard outputs as dust, and mark the channel REDEEMED (and hence never watch it or try again) - now, instead, if the outputs would not be dust if fee estimates were lower, lnwatcher will keep watching the channel - and if estimates go down, lnwatcher will sweep them then - relatedly, previously txbatcher.is_dust() used allow_fallback_to_static_rates=True, and it erroneously almost always fell back to the static rates (150 s/b) during startup (race: lnwatcher was faster than the network managed to get estimates) - now, instead, txbatcher.is_dust() does not fallback to static rates, and the callers are supposed to handle NoDynamicFeeEstimates. - I think this makes much more sense. The previous meaning of "is_dust" with the fallback was weird. Now it means: "is dust at current feerates". fixes https://github.com/spesmilo/electrum/issues/9980 --- electrum/address_synchronizer.py | 2 +- electrum/lnwatcher.py | 36 ++++++++++++++++++++++---------- electrum/submarine_swaps.py | 5 ++++- electrum/txbatcher.py | 10 +++++++-- 4 files changed, 38 insertions(+), 15 deletions(-) diff --git a/electrum/address_synchronizer.py b/electrum/address_synchronizer.py index 3d6cc367a..6183f96ee 100644 --- a/electrum/address_synchronizer.py +++ b/electrum/address_synchronizer.py @@ -1064,7 +1064,7 @@ class AddressSynchronizer(Logger, EventListener): return TxMinedDepth.FREE tx_mined_depth = self.get_tx_height(txid) height, conf = tx_mined_depth.height, tx_mined_depth.conf - if conf > 20: + if conf > 20: # FIXME unify with lnutil.REDEEM_AFTER_DOUBLE_SPENT_DELAY ? return TxMinedDepth.DEEP elif conf > 0: return TxMinedDepth.SHALLOW diff --git a/electrum/lnwatcher.py b/electrum/lnwatcher.py index fea04836e..0dea133ed 100644 --- a/electrum/lnwatcher.py +++ b/electrum/lnwatcher.py @@ -5,11 +5,12 @@ from typing import TYPE_CHECKING, Optional from . import util -from .util import TxMinedInfo, BelowDustLimit +from .util import TxMinedInfo, BelowDustLimit, NoDynamicFeeEstimates from .util import EventListener, event_listener, log_exceptions, ignore_exceptions from .transaction import Transaction, TxOutpoint from .logging import Logger from .address_synchronizer import TX_HEIGHT_LOCAL +from .lnutil import REDEEM_AFTER_DOUBLE_SPENT_DELAY if TYPE_CHECKING: @@ -172,32 +173,31 @@ class LNWatcher(Logger, EventListener): # do not keep watching if prevout does not exist self.logger.info(f'prevout does not exist for {name}: {prevout}') continue - was_added = self.maybe_redeem(sweep_info) + watch_sweep_info = self.maybe_redeem(sweep_info) spender_txid = self.adb.get_spender(prevout) # note: LOCAL spenders don't count spender_tx = self.adb.get_transaction(spender_txid) if spender_txid else None if spender_tx: # the spender might be the remote, revoked or not htlc_sweepinfo = chan.maybe_sweep_htlcs(closing_tx, spender_tx) for prevout2, htlc_sweep_info in htlc_sweepinfo.items(): - htlc_was_added = self.maybe_redeem(htlc_sweep_info) + watch_htlc_sweep_info = self.maybe_redeem(htlc_sweep_info) htlc_tx_spender = self.adb.get_spender(prevout2) self.lnworker.wallet.set_default_label(prevout2, htlc_sweep_info.name) if htlc_tx_spender: keep_watching |= not self.adb.is_deeply_mined(htlc_tx_spender) self.maybe_add_accounting_address(htlc_tx_spender, htlc_sweep_info) else: - keep_watching |= htlc_was_added + keep_watching |= watch_htlc_sweep_info keep_watching |= not self.adb.is_deeply_mined(spender_txid) self.maybe_extract_preimage(chan, spender_tx, prevout) self.maybe_add_accounting_address(spender_txid, sweep_info) else: - keep_watching |= was_added + keep_watching |= watch_sweep_info self.maybe_add_pending_forceclose( chan=chan, spender_txid=spender_txid, is_local_ctx=is_local_ctx, sweep_info=sweep_info, - sweep_info_txo_is_nondust=was_added, ) return keep_watching @@ -205,10 +205,16 @@ class LNWatcher(Logger, EventListener): return self._pending_force_closes def maybe_redeem(self, sweep_info: 'SweepInfo') -> bool: - """ returns False if it was dust """ + """ returns 'keep_watching' """ try: self.lnworker.wallet.txbatcher.add_sweep_input('lnwatcher', sweep_info) except BelowDustLimit: + # utxo is considered dust at *current* fee estimates. + # but maybe the fees atm are very high? We will retry later. + pass + except NoDynamicFeeEstimates: + pass # will retry later + if sweep_info.is_anchor(): return False return True @@ -259,10 +265,18 @@ class LNWatcher(Logger, EventListener): spender_txid: Optional[str], is_local_ctx: bool, sweep_info: 'SweepInfo', - sweep_info_txo_is_nondust: bool, # i.e. we want to sweep it - ): - """ we are waiting for ctx to be confirmed and there are received htlcs """ - if is_local_ctx and sweep_info.name == 'received-htlc' and sweep_info_txo_is_nondust: + ) -> None: + """Adds chan into set of ongoing force-closures if the user should keep the wallet open, waiting for it. + (we are waiting for ctx to be confirmed and there are received htlcs) + """ + if is_local_ctx and sweep_info.name == 'received-htlc': + cltv = sweep_info.cltv_abs + assert cltv is not None, f"missing cltv for {sweep_info}" + if self.adb.get_local_height() > cltv + REDEEM_AFTER_DOUBLE_SPENT_DELAY: + # We had plenty of time to sweep. The remote also had time to time out the htlc. + # Maybe its value has been ~dust at current and past fee levels (every time we checked). + # We should not keep warning the user forever. + return tx_mined_status = self.adb.get_tx_height(spender_txid) if tx_mined_status.height == TX_HEIGHT_LOCAL: self._pending_force_closes.add(chan) diff --git a/electrum/submarine_swaps.py b/electrum/submarine_swaps.py index d8edc1423..274c88063 100644 --- a/electrum/submarine_swaps.py +++ b/electrum/submarine_swaps.py @@ -32,7 +32,7 @@ from .transaction import ( from .util import ( log_exceptions, ignore_exceptions, BelowDustLimit, OldTaskGroup, ca_path, gen_nostr_ann_pow, get_nostr_ann_pow_amount, make_aiohttp_proxy_connector, get_running_loop, get_asyncio_loop, wait_for2, - run_sync_function_on_asyncio_thread, trigger_callback + run_sync_function_on_asyncio_thread, trigger_callback, NoDynamicFeeEstimates ) from . import lnutil from .lnutil import hex_to_bytes, REDEEM_AFTER_DOUBLE_SPENT_DELAY, Keypair @@ -485,6 +485,9 @@ class SwapManager(Logger): except BelowDustLimit: self.logger.info('utxo value below dust threshold') return + except NoDynamicFeeEstimates: + self.logger.info('got NoDynamicFeeEstimates') + return def get_fee_for_txbatcher(self): return self._get_tx_fee(self.config.FEE_POLICY_SWAPS) diff --git a/electrum/txbatcher.py b/electrum/txbatcher.py index 2cb3b0e86..419621c62 100644 --- a/electrum/txbatcher.py +++ b/electrum/txbatcher.py @@ -99,6 +99,7 @@ class TxBatcher(Logger): @locked def add_sweep_input(self, key: str, sweep_info: 'SweepInfo') -> None: + """Can raise BelowDustLimit or NoDynamicFeeEstimates.""" if sweep_info.txin and sweep_info.txout: # detect legacy htlc using name and csv delay if sweep_info.name in ['received-htlc', 'offered-htlc'] and sweep_info.csv_delay == 0: @@ -263,20 +264,25 @@ class TxBatch(Logger): self.batch_payments.append(output) def is_dust(self, sweep_info: SweepInfo) -> bool: + """Can raise NoDynamicFeeEstimates.""" if sweep_info.is_anchor(): return False if sweep_info.txout is not None: return False - value = sweep_info.txin._trusted_value_sats + value = sweep_info.txin.value_sats() witness_size = len(sweep_info.txin.make_witness(71*b'\x00')) tx_size_vbytes = 84 + witness_size//4 # assumes no batching, sweep to p2wpkh self.logger.info(f'{sweep_info.name} size = {tx_size_vbytes}') - fee = self.fee_policy.estimate_fee(tx_size_vbytes, network=self.wallet.network, allow_fallback_to_static_rates=True) + fee = self.fee_policy.estimate_fee(tx_size_vbytes, network=self.wallet.network) return value - fee <= dust_threshold() @locked def add_sweep_input(self, sweep_info: 'SweepInfo') -> None: + """Can raise BelowDustLimit or NoDynamicFeeEstimates.""" if self.is_dust(sweep_info): + # note: this uses the current fee estimates. Just because something is dust + # at the current fee levels, if fees go down, it might still become + # worthwhile to sweep. So callers might want to retry later. raise BelowDustLimit txin = sweep_info.txin if txin.prevout in self._unconfirmed_sweeps: