From ef0e4e02b780654e6e43827859a03ae04b44a3c0 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Thu, 4 Dec 2025 15:32:46 +0000 Subject: [PATCH] txbatcher: add TODOs re nLocktime block-height vs timestamp confusion seems harmless atm but some rethinking is needed and checks should be added --- electrum/lnsweep.py | 1 + electrum/transaction.py | 18 ++++++++++-------- electrum/txbatcher.py | 11 +++++++++-- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/electrum/lnsweep.py b/electrum/lnsweep.py index 28f74f37d..3e1c15210 100644 --- a/electrum/lnsweep.py +++ b/electrum/lnsweep.py @@ -40,6 +40,7 @@ HTLCTX_INPUT_OUTPUT_INDEX = 0 class SweepInfo(NamedTuple): name: str cltv_abs: Optional[int] # set to None only if the script has no cltv + # TODO add asserts that cltv_abs is block-based (see NLOCKTIME_BLOCKHEIGHT_MAX) txin: PartialTxInput txout: Optional[PartialTxOutput] # only for first-stage htlc tx can_be_batched: bool # todo: this could be more fine-grained diff --git a/electrum/transaction.py b/electrum/transaction.py index cca06ff18..7f621af4d 100644 --- a/electrum/transaction.py +++ b/electrum/transaction.py @@ -349,16 +349,18 @@ class TxInput: def get_time_based_relative_locktime(self) -> Optional[int]: # see bip 68 - if self.nsequence & (1<<31): - return - if self.nsequence & (1<<22): + if self.nsequence & (1<<31): # "disable" flag + return None + if self.nsequence & (1<<22): # in units of 512 sec return self.nsequence & 0xffff + return None def get_block_based_relative_locktime(self) -> Optional[int]: - if self.nsequence & (1<<31): - return - if not self.nsequence & (1<<22): + if self.nsequence & (1<<31): # "disable" flag + return None + if not self.nsequence & (1<<22): # in blocks return self.nsequence & 0xffff + return None @property def short_id(self): @@ -1328,13 +1330,13 @@ class Transaction: def get_time_based_relative_locktime(self) -> Optional[int]: if self.version < 2: - return + return None locktimes = list(filter(None, [txin.get_time_based_relative_locktime() for txin in self.inputs()])) return max(locktimes) if locktimes else None def get_block_based_relative_locktime(self) -> Optional[int]: if self.version < 2: - return + return None locktimes = list(filter(None, [txin.get_block_based_relative_locktime() for txin in self.inputs()])) return max(locktimes) if locktimes else None diff --git a/electrum/txbatcher.py b/electrum/txbatcher.py index 91b533289..ee1727e04 100644 --- a/electrum/txbatcher.py +++ b/electrum/txbatcher.py @@ -45,7 +45,7 @@ # # 1. CPFP: # When a batch is forgotten but not mined (because the server returned an error), we no longer bump its fee. -# However, the current code does not theat the next batch as a CPFP when computing the fee. +# However, the current code does not treat the next batch as a CPFP when computing the fee. # # 2. Reorgs: # This code does not guarantee that a payment or a sweep will happen. @@ -56,6 +56,13 @@ # In the case of sweeps, lnwatcher ensures that SweepInfo is added again after a client restart. # In order to generalize that logic to payments, callers would need to pass a unique ID along with # the payment output, so that we can prevent paying twice. +# +# - nLocktime/CLTV values (bip-65) and nSequence/CSV values (bip-112) are either explicitly +# or implicitly block-height-based everywhere in this file. +# SCRIPT execution fails on height vs timestamp confusion, and +# it is not safe to do naive integer comparison between these values without establishing type. +# TODO review this is correct, and add checks. +# nLocktime/CLTV usage in particular seems dangerously *implicit* for being block-heights import asyncio import threading @@ -517,7 +524,7 @@ class TxBatch(Logger): # sort inputs so that txin-txout pairs are first for sweep_info in sorted(to_sweep, key=lambda x: not bool(x.txout)): if sweep_info.cltv_abs is not None: - if locktime is None or locktime < sweep_info.cltv_abs: + if locktime is None or locktime < sweep_info.cltv_abs: # FIXME height vs timestamp confusion # nLockTime must be greater than or equal to the stack operand. locktime = sweep_info.cltv_abs inputs.append(copy.deepcopy(sweep_info.txin))