lnhtlc: remove unneeded non-initiator fee_update in log
What the developer wrote
Authored by ThomasV
lnhtlc: remove unneeded non-initiator fee_update in log
What changed, and why it matters
This commit cleans up how Electrum stores initial Bitcoin transaction fee information for Lightning payment channels. Previously, both sides of a channel got a starting fee entry in the internal log, even though only the channel initiator is allowed to change fees later. The patch now records the initial fee only for the initiator and includes a database upgrade routine to remove the duplicate entry from existing wallets. The change is described by the developer as a cleanup, not a security fix, but it touches code that decides which fee rate is used in commitment transactions, which matters for channel safety.
Treat as a correctness/maintenance patch. Reviewers should verify that the new initiator-only bootstrap and the `> 0` threshold in `get_feerate` cannot cause a channel to use a stale or attacker-controlled feerate, especially during recovery from an old wallet database. No immediate user action is indicated absent further security analysis.
Security signals we found
Change affects fee-rate selection logic in Lightning commitment transactions
Database migration removes previously stored fee-update records, implying prior state could be inconsistent
Assertion in get_feerate changes threshold, altering which party's fee_updates are considered authoritative
No CVE, advisory, or vendor security framing is present in the commit or supplied references
Evidence from the diff
HTLCManager’s constructor now takes an initiator argument (LOCAL or REMOTE) and, when bootstrapping fee_updates, creates the initial FeeUpdate only for that party. The get_feerate assertion and selection logic are relaxed from > 1 to > 0 so that a single fee update on the non-initiator side is no longer treated as the active fee log. A new wallet database upgrade (version 66→67) deletes the spurious fee_updates[0] entry from the non-initiator’s log in existing channels. The commit message frames this as removing an ‘unneeded’ entry.
Changed components
electrum/lnhtlc.pyelectrum/lnchannel.pyelectrum/wallet_db.pyInspect captured patch +20 / −8
diff --git a/electrum/lnchannel.py b/electrum/lnchannel.py
index fddae90..ea47dec 100644
--- a/electrum/lnchannel.py
+++ b/electrum/lnchannel.py
@@ -793,7 +793,7 @@ class Channel(AbstractChannel):
self.node_id = bfh(state["node_id"])
self.onion_keys = state['onion_keys'] # type: Dict[int, bytes]
self.data_loss_protect_remote_pcp = state['data_loss_protect_remote_pcp']
- self.hm = HTLCManager(log=state['log'], initial_feerate=initial_feerate)
+ self.hm = HTLCManager(log=state['log'], initiator = LOCAL if self.constraints.is_initiator else REMOTE, initial_feerate=initial_feerate)
self.unfulfilled_htlcs = state["unfulfilled_htlcs"] # type: Dict[int, Optional[str]]
# ^ htlc_id -> onion_packet_hex
self._state = ChannelState[state['state']]
diff --git a/electrum/lnhtlc.py b/electrum/lnhtlc.py
index 9a59507..4ccaa48 100644
--- a/electrum/lnhtlc.py
+++ b/electrum/lnhtlc.py
@@ -10,7 +10,7 @@ if TYPE_CHECKING:
class HTLCManager:
- def __init__(self, log: 'StoredDict', *, initial_feerate=None):
+ def __init__(self, log: 'StoredDict', *, initiator=None, initial_feerate=None):
if len(log) == 0:
initial = {
@@ -32,9 +32,8 @@ class HTLCManager:
# maybe bootstrap fee_updates if initial_feerate was provided
if initial_feerate is not None:
assert type(initial_feerate) is int
- for sub in (LOCAL, REMOTE):
- if not log[sub]['fee_updates']:
- log[sub]['fee_updates'][0] = FeeUpdate(rate=initial_feerate, ctn_local=0, ctn_remote=0)
+ assert initiator in [LOCAL, REMOTE]
+ log[initiator]['fee_updates'][0] = FeeUpdate(rate=initial_feerate, ctn_local=0, ctn_remote=0)
self.log = log
# We need a lock as many methods of HTLCManager are accessed by both the asyncio thread and the GUI.
@@ -595,9 +594,9 @@ class HTLCManager:
"""Return feerate (sat/kw) used in subject's commitment txn at ctn."""
ctn = max(0, ctn) # FIXME rm this
# only one party can update fees; use length of logs to figure out which:
- assert not (len(self.log[LOCAL]['fee_updates']) > 1 and len(self.log[REMOTE]['fee_updates']) > 1)
+ assert not (len(self.log[LOCAL]['fee_updates']) > 0 and len(self.log[REMOTE]['fee_updates']) > 0)
fee_log = self.log[LOCAL]['fee_updates'] # type: Sequence[FeeUpdate]
- if len(self.log[REMOTE]['fee_updates']) > 1:
+ if len(self.log[REMOTE]['fee_updates']) > 0:
fee_log = self.log[REMOTE]['fee_updates']
# binary search
left = 0
diff --git a/electrum/wallet_db.py b/electrum/wallet_db.py
index dfb4bbb..5e6a29d 100644
--- a/electrum/wallet_db.py
+++ b/electrum/wallet_db.py
@@ -69,7 +69,7 @@ class WalletUnfinished(WalletFileException):
# seed_version is now used for the version of the wallet file
OLD_SEED_VERSION = 4 # electrum versions < 2.0
NEW_SEED_VERSION = 11 # electrum versions >= 2.0
-FINAL_SEED_VERSION = 66 # electrum >= 2.7 will set this to prevent
+FINAL_SEED_VERSION = 67 # electrum >= 2.7 will set this to prevent
# old versions from overwriting new format
@@ -238,6 +238,7 @@ class WalletDBUpgrader(Logger):
self._convert_version_64()
self._convert_version_65()
self._convert_version_66()
+ self._convert_version_67()
self.put('seed_version', FINAL_SEED_VERSION) # just to be sure
def _convert_wallet_type(self):
@@ -1332,6 +1333,18 @@ class WalletDBUpgrader(Logger):
self.data['lightning_payments'] = new_payment_infos
self.data['seed_version'] = 66
+ def _convert_version_67(self):
+ if not self._is_upgrade_method_needed(66, 66):
+ return
+ channels = self.data.get('channels', {})
+ for _key, chan in channels.items():
+ is_initiator = chan['constraints']['is_initiator']
+ key = '-1' if is_initiator else '1'
+ assert len(chan['log'][key]['fee_updates']) == 1, chan['log'][key]['fee_updates']
+ chan['log'][key]['fee_updates'] = {}
+ self.data['channels'] = channels
+ self.data['seed_version'] = 67
+
def _convert_imported(self):
if not self._is_upgrade_method_needed(0, 13):
return
Why this scored 43/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.