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.

Sign up for GitHub

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

Jump to bottom

Add support for networks field in Init message #2329

Merged
merged 2 commits into from
Jun 5, 2023
Merged

Add support for networks field in Init message #2329

merged 2 commits into from
Jun 5, 2023

Conversation

dunxen
Copy link
Contributor

@dunxen dunxen commented Jun 1, 2023

Resolves #2241

@codecov-commenter
Copy link

codecov-commenter commented Jun 1, 2023 •
edited

Codecov Report

Patch coverage: 97.49% and no project coverage change.

Comparison is base (fb140b5) 90.34% compared to head (b52b936) 90.35%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files 
@@           Coverage Diff            @@
##             main    #2329    +/-   ##
========================================
  Coverage   90.34%   90.35%            
========================================
  Files         104      104            
  Lines       53392    53590   +198     
  Branches    53392    53590   +198     
========================================
+ Hits        48237    48421   +184     
- Misses       5155     5169    +14
Impacted Files Coverage Δ
lightning/src/ln/peer_handler.rs 61.47% <89.70%> (+1.15%) ⬆️
lightning-background-processor/src/lib.rs 82.58% <100.00%> (+0.12%) ⬆️
lightning-net-tokio/src/lib.rs 76.94% <100.00%> (+0.20%) ⬆️
lightning/src/ln/chanmon_update_fail_tests.rs 98.63% <100.00%> (+0.02%) ⬆️
lightning/src/ln/channelmanager.rs 86.29% <100.00%> (+<0.01%) ⬆️
lightning/src/ln/functional_test_utils.rs 88.03% <100.00%> (+0.12%) ⬆️
lightning/src/ln/functional_tests.rs 98.25% <100.00%> (+<0.01%) ⬆️
lightning/src/ln/msgs.rs 84.58% <100.00%> (+0.01%) ⬆️
lightning/src/ln/payment_tests.rs 97.58% <100.00%> (+0.01%) ⬆️
lightning/src/ln/priv_short_conf_tests.rs 97.63% <100.00%> (+0.03%) ⬆️
... and 6 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@dunxen dunxen marked this pull request as ready for review June 1, 2023 15:40
TheBlueMatt
TheBlueMatt reviewed Jun 1, 2023
View reviewed changes
Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks! Basically LGTM.

lightning/src/ln/peer_handler.rs Outdated Show resolved Hide resolved
@dunxen dunxen force-pushed the 2023-05-initgenesischeck branch 2 times, most recently from ecce9e3 to abdcef3 Compare June 2, 2023 11:11
@dunxen
Copy link
Contributor Author

dunxen commented Jun 2, 2023

Going to just do some testing on the sample against some other nodes on the network with this change in.

@dunxen
Copy link
Contributor Author

dunxen commented Jun 2, 2023

Cool, so at least checked against a mainnet and testnet eclair node using LDK sample on mainnet

Connecting to peer supporting mainnet successfully:

2023-06-02 14:55:49.334 TRACE [lightning::ln::peer_handler:1209] Enqueueing message Init { features: [161, 81, 10, 8, 128, 160, 8], networks: Some([6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000]), remote_network_address: Some(IPv4 { addr: [/* scrubbed */], port: 9735 }) } to 038cec4bc800a175d6ec6bc2285a92dd4e70bdc9eb9efa45f917f27f09cd7e66cb
2023-06-02 14:55:49.641 INFO  [lightning::ln::peer_handler:1498] Received peer Init message from 038cec4bc800a175d6ec6bc2285a92dd4e70bdc9eb9efa45f917f27f09cd7e66cb: DataLossProtect: required, InitialRoutingSync: not supported, UpfrontShutdownScript: supported, GossipQueries: supported, VariableLengthOnion: supported, StaticRemoteKey: required, PaymentSecret: required, BasicMPP: supported, Wumbo: supported, AnchorsZeroFeeHtlcTx: supported, ShutdownAnySegwit: supported, OnionMessages: not supported, ChannelType: supported, SCIDPrivacy: not supported, ZeroConf: not supported, unknown flags: supported
2023-06-02 14:55:49.642 DEBUG [lightning::ln::channelmanager:6902] Generating channel_reestablish events for 038cec4bc800a175d6ec6bc2285a92dd4e70bdc9eb9efa45f917f27f09cd7e66cb
2023-06-02 14:55:49.642 TRACE [lightning::ln::peer_handler:1209] Enqueueing message GossipTimestampFilter { chain_hash: 000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f, first_timestamp: 1684508149, timestamp_range: 4294967295 } to 038cec4bc800a175d6ec6bc2285a92dd4e70bdc9eb9efa45f917f27f09cd7e66cb
2023-06-02 14:55:49.642 TRACE [lightning_background_processor:645] Persisting ChannelManager...
2023-06-02 14:55:49.655 TRACE [lightning_background_processor:645] Done persisting ChannelManager.

Connecting to peer only supporting testnet results in immediate disconnection:

2023-06-02 15:02:00.146 TRACE [lightning::ln::peer_handler:1209] Enqueueing message Init { features: [161, 81, 10, 8, 128, 160, 8], networks: Some([6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000]), remote_network_address: Some(IPv4 { addr: [/* scrubbed */], port: 9735 }) } to 03933884aaf1d6b108397e5efe5c86bcf2d8ca8d2f700eda99db9214fc2712b134
2023-06-02 15:02:00.333 DEBUG [lightning::ln::peer_handler:1477] Peer does not support our chain (6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000)
2023-06-02 15:02:00.333 TRACE [lightning::ln::peer_handler:1197] Disconnecting peer due to a protocol error (usually a duplicate connection).
2023-06-02 15:02:00.333 TRACE [lightning::ln::peer_handler:2245] Handling disconnection of peer 03933884aaf1d6b108397e5efe5c86bcf2d8ca8d2f700eda99db9214fc2712b134

wpaulino
wpaulino reviewed Jun 2, 2023
View reviewed changes
lightning/src/ln/msgs.rs Outdated Show resolved Hide resolved
TheBlueMatt
TheBlueMatt reviewed Jun 4, 2023
View reviewed changes
Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM

lightning/src/util/ser.rs Outdated Show resolved Hide resolved
@dunxen
Add networks TLV to Init's TLV stream
e23102f 
This was a fairly old introduction to the spec to allow nodes to indicate
to their peers what chains they are interested in (i.e. will open channels
and gossip for).

We don't do any of the handling of this message in this commit and leave
that to the very next commit, so the behaviour is effectively the same
(ignore networks preference).
@dunxen
Send and handle networks field in Init messages
b52b936 
If the `networks` field is present in a received `Init` message, then
we need to make sure our genesis chain hash matches one of those, otherwise
we should disconnect the peer.

We now also always send our genesis chain hash in `Init` messages to
our peers.
@TheBlueMatt TheBlueMatt merged commit 166f326 into lightningdevkit:main Jun 5, 2023
14 checks passed
@dunxen dunxen deleted the 2023-05-initgenesischeck branch August 28, 2023 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheBlueMatt TheBlueMatt TheBlueMatt approved these changes

@wpaulino wpaulino wpaulino approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for the genesis block hash in init messages
4 participants
@dunxen @codecov-commenter @TheBlueMatt @wpaulino