Skip to content

Add PeerManager::disconnect_node_id() #783

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

Merged

Conversation

ariard
Copy link

@ariard ariard commented Jan 28, 2021

This public method allows a client to easily disconnect peers while only
owning its node id. It will clean up peer state and disconnect properly
its descriptor.

Motivation: http://gnusha.org/rust-bitcoin/2021-01-28.log

Required to implement disconnect cmd on the sample-side, we might rework our PeerManager API and lightning-net-tokio once we agree on a good interface.

@codecov
Copy link

codecov bot commented Jan 28, 2021

Codecov Report

Merging #783 (97d3bdc) into main (74cd96f) will increase coverage by 0.00%.
The diff coverage is 94.90%.

Impacted file tree graph

@@           Coverage Diff            @@
##             main     #783    +/-   ##
========================================
  Coverage   91.23%   91.23%            
========================================
  Files          37       37            
  Lines       22623    22921   +298     
========================================
+ Hits        20639    20911   +272     
- Misses       1984     2010    +26     
Impacted Files Coverage Δ
lightning/src/lib.rs 100.00% <ø> (ø)
lightning/src/ln/peer_handler.rs 51.02% <ø> (ø)
lightning/src/util/ser.rs 90.35% <ø> (+2.89%) ⬆️
lightning/src/util/config.rs 50.00% <50.00%> (-19.05%) ⬇️
lightning/src/ln/channelmanager.rs 84.99% <66.66%> (+0.05%) ⬆️
lightning/src/util/test_utils.rs 84.78% <87.17%> (-0.05%) ⬇️
lightning/src/chain/keysinterface.rs 93.47% <88.00%> (-1.38%) ⬇️
lightning/src/util/enforcing_trait_impls.rs 90.19% <93.10%> (-9.81%) ⬇️
lightning/src/ln/channel.rs 87.49% <93.12%> (+0.03%) ⬆️
lightning/src/ln/chan_utils.rs 97.14% <95.17%> (+1.48%) ⬆️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 74cd96f...5b7d7ed. Read the comment docs.

@ariard ariard force-pushed the 2021-01-disconnect-node-id branch from c6c6aef to 070ec49 Compare February 1, 2021 10:53
@ariard
Copy link
Author

ariard commented Feb 1, 2021

Updated at 070ec49

/// Set no_connection_possible to true to prevent any further connection with this peer,
/// force-closing any channels we have with it.
///
/// Beware with reentrancy issues about disconnect_socket!
Copy link
Collaborator

@TheBlueMatt TheBlueMatt Feb 1, 2021

Choose a reason for hiding this comment

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

Can we state that "if a peer is connected, this will call disconnect_socket on the descriptor for the peer, so be careful about reentrancy issues" - there's no reason to not provide more detail on exactly what will happen.

Copy link
Author

Choose a reason for hiding this comment

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

Updated with comment.

This public method allows a client to easily disconnect peers while only
owning its node id. It will clean up peer state and disconnect properly
its descriptor.
@ariard ariard force-pushed the 2021-01-disconnect-node-id branch from 070ec49 to 5b7d7ed Compare February 1, 2021 19:14
@TheBlueMatt
Copy link
Collaborator

This is trivial. Going to merge.

@TheBlueMatt TheBlueMatt merged commit 151d4ac into lightningdevkit:main Feb 1, 2021
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.

2 participants