Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate internal channel config from channel features #1848

Closed
wants to merge 2 commits into from

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented Jun 24, 2021

Our current ChannelVersion field mixes two unrelated concepts: channel features (as defined in Bolt 9) and channel internals (such as custom key derivation). It is more future-proof to separate these two unrelated concepts and will make it easier to implement channel types (see lightning/bolts#880).

NB: we will merge this commit with other pending commits that require a channel codecs migration to minimize the number of migrations applied.

That way, even if the node database or a backup is compromised, the attacker
won't be able to force close channels from the outside.
@t-bast t-bast force-pushed the channel-version-migration branch from b6df708 to a297765 Compare June 29, 2021 16:03
@codecov-commenter
Copy link

Codecov Report

Merging #1848 (a297765) into master (45204e2) will increase coverage by 0.21%.
The diff coverage is 99.41%.

@@            Coverage Diff             @@
##           master    #1848      +/-   ##
==========================================
+ Coverage   88.96%   89.17%   +0.21%     
==========================================
  Files         150      153       +3     
  Lines       11237    11419     +182     
  Branches      429      461      +32     
==========================================
+ Hits         9997    10183     +186     
+ Misses       1240     1236       -4     
Impacted Files Coverage Δ
...n/scala/fr/acinq/eclair/channel/ChannelTypes.scala 100.00% <ø> (+2.85%) ⬆️
...q/eclair/crypto/keymanager/ChannelKeyManager.scala 87.50% <75.00%> (ø)
...wire/internal/channel/version0/ChannelTypes0.scala 98.75% <97.95%> (-1.25%) ⬇️
.../fr/acinq/eclair/blockchain/fee/FeeEstimator.scala 100.00% <100.00%> (ø)
...c/main/scala/fr/acinq/eclair/channel/Channel.scala 86.35% <100.00%> (+0.08%) ⬆️
...fr/acinq/eclair/channel/ChannelConfigOptions.scala 100.00% <100.00%> (ø)
...in/scala/fr/acinq/eclair/channel/ChannelType.scala 100.00% <100.00%> (ø)
...in/scala/fr/acinq/eclair/channel/Commitments.scala 94.66% <100.00%> (+1.09%) ⬆️
...c/main/scala/fr/acinq/eclair/channel/Helpers.scala 95.94% <100.00%> (-0.01%) ⬇️
...clair/channel/publish/ReplaceableTxPublisher.scala 86.98% <100.00%> (+1.18%) ⬆️
... and 15 more

Our current ChannelVersion field mixes two unrelated concepts: channel
features (as defined in Bolt 9) and channel internals (such as custom key
derivation). It is more future-proof to separate these two unrelated concepts
and will make it easier to implement channel types (see
lightning/bolts#880).
@t-bast t-bast force-pushed the channel-version-migration branch from a297765 to a1e31f5 Compare June 29, 2021 17:21
@t-bast t-bast closed this Jun 29, 2021
pm47 added a commit that referenced this pull request Jul 15, 2021
There are three otherwise unrelated changes, that we group together to only have one migration:
- remove local signatures for local commitments (this PR)
- separate internal channel config from channel features (#1848)
- upfront shutdown script (#1846)

We increase database version number in sqlite and postgres to force a full data migration.

The goal of removing local signatures from the channel data is that even if the node database or
a backup is compromised, the attacker won't be able to force close channels from the outside.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants