lightning: fix anchor channel backup - #10852
Open
f321x wants to merge 10 commits into
Open
Conversation
f321x
force-pushed
the
fix_10785
branch
2 times, most recently
from
August 17, 2026 11:32
da57099 to
bf0f96d
Compare
SomberNight
reviewed
Aug 17, 2026
f321x
pushed a commit
that referenced
this pull request
Aug 18, 2026
useful for unit tests where it is easy to slip-up and break this invariant when creating a MockNetwork ref #10852 (comment)
f321x
force-pushed
the
fix_10785
branch
2 times, most recently
from
August 18, 2026 12:47
a8a5b2d to
1b11674
Compare
Member
Author
|
@SomberNight as discussed I also added more unittests in c649d51, some might overlap a bit, e.g. the local force close tests are taking a pretty similar path to claim the |
f321x
force-pushed
the
fix_10785
branch
2 times, most recently
from
August 18, 2026 14:22
44d5506 to
bef5dd2
Compare
f321x
force-pushed
the
fix_10785
branch
2 times, most recently
from
August 25, 2026 15:40
768f3fd to
d18913a
Compare
SomberNight
reviewed
Aug 26, 2026
SomberNight
left a comment
SomberNight
left a comment
Member
There was a problem hiding this comment.
some comments (but still looking)
Also, I merged the first two commits into master in 8c260c9
| privkey = ecc.ECPrivkey(payment_basepoint) | ||
| kwargs['payment_basepoint'] = Keypair(privkey=privkey.get_secret_bytes(), pubkey=privkey.get_public_key_bytes()) | ||
| else: | ||
| kwargs['payment_basepoint'] = OnlyPubkeyKeypair(payment_basepoint) |
Member
There was a problem hiding this comment.
maybe assert here that len(payment_basepoint) == 33 or even ECPubkey.is_pubkey_bytes(payment_basepoint)
Member
Author
There was a problem hiding this comment.
Added an ECPubkey.is_pubkey_bytes(payment_basepoint) assert in https://github.com/spesmilo/electrum/compare/d18913ad56c0e076a46f920a1eb2db6164aef242..b8812c06595d1eb6678091d79dda945834fb7c2e
Comment on lines
+405
to
+413
| local_payment_basepoint = None | ||
| if version >= 3: | ||
| if channel_type == ChannelType.OPTION_STATIC_REMOTEKEY | ChannelType.OPTION_ANCHORS: | ||
| local_payment_basepoint = vds.read_bytes(32) | ||
| else: | ||
| assert channel_type == ChannelType.OPTION_STATIC_REMOTEKEY, channel_type | ||
| local_payment_basepoint = vds.read_bytes(33) | ||
| elif version >= 1: | ||
| local_payment_basepoint = vds.read_bytes(33) |
Member
There was a problem hiding this comment.
What do you think about simplifying this branching a bit by always writing 33 bytes?
Similar to how xprvs are serialized, private keys could be prefixed by a zero byte:
diff --git a/electrum/lnutil.py b/electrum/lnutil.py
index a79b4f048c..25b997c423 100644
--- a/electrum/lnutil.py
+++ b/electrum/lnutil.py
@@ -372,11 +372,13 @@ class ImportedChannelBackupStorage(ChannelBackupStorage):
vds.write_uint16(self.remote_delay)
vds.write_string(self.host)
vds.write_uint16(self.port)
- if self.channel_type == ChannelType.OPTION_STATIC_REMOTEKEY | ChannelType.OPTION_ANCHORS:
- vds.write_bytes(self.local_payment_basepoint, 32) # private key
+ if len(self.local_payment_basepoint) == 32: # private key
+ assert self.channel_type == ChannelType.OPTION_STATIC_REMOTEKEY | ChannelType.OPTION_ANCHORS
+ vds.write_bytes(b"\x00" + self.local_payment_basepoint, 33)
else:
+ assert len(self.local_payment_basepoint) == 33 # pubkey
assert self.channel_type == ChannelType.OPTION_STATIC_REMOTEKEY
- vds.write_bytes(self.local_payment_basepoint, 33) # pubkey
+ vds.write_bytes(self.local_payment_basepoint, 33)
vds.write_bytes(self.multisig_funding_privkey, 32)
return bytes(vds.input)
@@ -401,17 +403,11 @@ class ImportedChannelBackupStorage(ChannelBackupStorage):
remote_delay = vds.read_uint16()
host = vds.read_string()
port = vds.read_uint16()
-
- local_payment_basepoint = None
- if version >= 3:
- if channel_type == ChannelType.OPTION_STATIC_REMOTEKEY | ChannelType.OPTION_ANCHORS:
- local_payment_basepoint = vds.read_bytes(32)
- else:
- assert channel_type == ChannelType.OPTION_STATIC_REMOTEKEY, channel_type
- local_payment_basepoint = vds.read_bytes(33)
- elif version >= 1:
+ local_payment_basepoint = None # type: Optional[bytes]
+ if version >= 1:
local_payment_basepoint = vds.read_bytes(33)
-
+ if local_payment_basepoint[0] == 0: # private key
+ local_payment_basepoint = local_payment_basepoint[1:]
if version >= 2:
multisig_funding_privkey = vds.read_bytes(32)
else:
Member
Author
There was a problem hiding this comment.
Keep ImportedChannelBackupStorage only in-memory and don't persist its json representation in the db, instead use the existing (to|from)_bytes de/serializers to have one uniform format for the backups. This also removes the requirement to do wallet db upgrades when changing the format. Also removes the dependency on attrs for the class as they are not a StoredObject anymore, its now a frozen dataclass so the inheritance still works as before.
Extends the channel backup format by including the payment_basepoint so that anchor channel to_remote outputs can be claimed even without deterministic wallet seeds.
When importing a v0 channel backup `LocalConfig.from_seed()` would derive a `payment_basepoint` from the `channel_seed`. This `payment_basepoint` is not useful for static remote key channels, so the derivation is only confusing. Pre-srk channels, the only channel type that could use this derivation, never exported channel backups.
Construct a ChannelBackup in the lnutil tests, this is cheap and catches issues where the `ChannelBackup.from_seed()` call would raise.
Now we have unittests, so this shouldn't be important anymore
now that we have channel_type available in the channel backup, the `ChannelBackup.has_anchors()` method can utilize it.
Don't allow requesting a remote force close if we cannot sweep the to-remote output by not showing the option to request the force close in the UI.
Show a warning in the UI when trying to import a channel backup that cannot be used to properly request a force close.
Implement a mechanism to show one-time warnings to the user on startup of the wallet to inform them about critical changes. Use this mechanism to inform them about the changed lightning channel backup scheme.
SomberNight
reviewed
Sep 1, 2026
Member
There was a problem hiding this comment.
note: you have not signed these commits. Consider enabling "vigilant mode" in the github acc settings.
Member
There was a problem hiding this comment.
If there will be a setting to force maintainers to sign commits, we should enable that.
Looks like it does not exist atm: https://github.com/orgs/community/discussions/69706
There is a setting that requires every commit to be signed but that's unusable in practice given we accept contributions from anyone.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add missing information to lightning channel backup so users of non-deterministic
lightning wallets trying to recover anchor channels are able to sweep the to_remote
output of a remote ctx.
Also shows a warning to affected users, urging them to export a new backup.
Fixes #10785