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

Add private flag to channel updates #2362

Merged
merged 3 commits into from
Nov 4, 2022
Merged

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented Aug 3, 2022

Add a message flag to channel update to specify that an update is private and should not be rebroadcast to other nodes.
We log an error if a private channel update gets into the rebroadcast list.

I also tried strongly typing channel updates, dividing them into PrivateChannelUpdate and PublicChannelUpdate to get compile-time guarantees that private channel updates wouldn't leak where they're not supposed to, but unfortunately I hit some roadblocks...the main issue with this approach is that we would need Router.PublicChannel and Router.PrivateChannel to map to these two types of channel updates, but that's not how we use them internally: when a public channel isn't confirmed yet, we store it as a private channel which messes things up. Ideally we would need to rework this to clearly separate channels that will be public once confirmed from channels that will stay private, but that's quite a lot of work for not much benefits, so I'm leaving it aside for now.

cln has already merged support for this on their master branch, and I have successfully tested interop with them.

See lightning/bolts#999

@t-bast t-bast requested a review from pm47 August 3, 2022 16:24
@t-bast t-bast force-pushed the channel-update-no-forward-flag branch 2 times, most recently from b369ce4 to da27f16 Compare August 4, 2022 08:15
@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2022

Codecov Report

Merging #2362 (3f8a4eb) into master (1b36697) will increase coverage by 0.11%.
The diff coverage is 94.11%.

@@            Coverage Diff             @@
##           master    #2362      +/-   ##
==========================================
+ Coverage   84.82%   84.94%   +0.11%     
==========================================
  Files         198      198              
  Lines       15761    15812      +51     
  Branches      655      711      +56     
==========================================
+ Hits        13369    13431      +62     
+ Misses       2392     2381      -11     
Impacted Files Coverage Δ
...main/scala/fr/acinq/eclair/io/PeerConnection.scala 85.76% <50.00%> (-1.00%) ⬇️
...in/scala/fr/acinq/eclair/channel/fsm/Channel.scala 86.16% <100.00%> (-0.12%) ⬇️
...inq/eclair/channel/fsm/CommonFundingHandlers.scala 91.66% <100.00%> (ø)
...n/scala/fr/acinq/eclair/router/Announcements.scala 100.00% <100.00%> (ø)
...e/src/main/scala/fr/acinq/eclair/router/Sync.scala 98.39% <100.00%> (ø)
...fr/acinq/eclair/wire/protocol/FailureMessage.scala 83.09% <100.00%> (+0.24%) ⬆️
.../eclair/wire/protocol/LightningMessageCodecs.scala 100.00% <100.00%> (ø)
...q/eclair/wire/protocol/LightningMessageTypes.scala 98.36% <100.00%> (+0.08%) ⬆️
...cala/fr/acinq/eclair/payment/relay/NodeRelay.scala 96.55% <0.00%> (-1.01%) ⬇️
... and 33 more

Add a message flag to channel update to specify that an update is private
and should not be rebroadcast to other nodes.

We log an error if a private channel update gets into the rebroadcast list.

See lightning/bolts#999
@t-bast t-bast force-pushed the channel-update-no-forward-flag branch from ddf5b53 to cbd3bbd Compare September 27, 2022 07:40
@t-bast t-bast marked this pull request as ready for review September 27, 2022 07:41
@t-bast t-bast mentioned this pull request Oct 25, 2022
pm47
pm47 previously approved these changes Nov 4, 2022
@t-bast t-bast merged commit a8389a0 into master Nov 4, 2022
@t-bast t-bast deleted the channel-update-no-forward-flag branch November 4, 2022 14:56
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