Skip to content

Include non-permanently connected peers in list_peers() #95

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

Merged
merged 2 commits into from
Jun 13, 2023
Merged

Include non-permanently connected peers in list_peers() #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5aa8f6f
Include non-permanently connected peers in `list_peers()`
tnull May 12, 2023
f562395
Misc documentation fixes
tnull May 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions bindings/ldk_node.udl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ interface LDKNode {
[Throws=NodeError]
u64 total_onchain_balance_sats();
[Throws=NodeError]
void connect(PublicKey node_id, NetAddress address, boolean permanently);
void connect(PublicKey node_id, NetAddress address, boolean persist);
[Throws=NodeError]
void disconnect(PublicKey node_id);
[Throws=NodeError]
Expand Down Expand Up @@ -157,7 +157,7 @@ dictionary ChannelDetails {
ChannelId channel_id;
PublicKey counterparty_node_id;
OutPoint? funding_txo;
u64 channel_value_satoshis;
u64 channel_value_sats;
u64? unspendable_punishment_reserve;
UserChannelId user_channel_id;
u32 feerate_sat_per_1000_weight;
Expand All @@ -176,6 +176,7 @@ dictionary ChannelDetails {
dictionary PeerDetails {
PublicKey node_id;
NetAddress address;
boolean is_persisted;
boolean is_connected;
};

Expand Down
54 changes: 40 additions & 14 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1284,9 +1284,9 @@ impl<K: KVStore + Sync + Send + 'static> Node<K> {

/// Connect to a node on the peer-to-peer network.
///
/// If `permanently` is set to `true`, we'll remember the peer and reconnect to it on restart.
/// If `persist` is set to `true`, we'll remember the peer and reconnect to it on restart.
pub fn connect(
&self, node_id: PublicKey, address: NetAddress, permanently: bool,
&self, node_id: PublicKey, address: NetAddress, persist: bool,
) -> Result<(), Error> {
let rt_lock = self.runtime.read().unwrap();
if rt_lock.is_none() {
Expand All @@ -1311,7 +1311,7 @@ impl<K: KVStore + Sync + Send + 'static> Node<K> {

log_info!(self.logger, "Connected to peer {}@{}. ", peer_info.node_id, peer_info.address);

if permanently {
if persist {
self.peer_store.add_peer(peer_info)?;
}

Expand Down Expand Up @@ -1833,25 +1833,51 @@ impl<K: KVStore + Sync + Send + 'static> Node<K> {

/// Retrieves a list of known peers.
pub fn list_peers(&self) -> Vec<PeerDetails> {
let active_connected_peers: Vec<PublicKey> =
self.peer_manager.get_peer_node_ids().iter().map(|p| p.0).collect();
self.peer_store
.list_peers()
.iter()
.map(|p| PeerDetails {
let mut peers = Vec::new();

// First add all connected peers, preferring to list the connected address if available.
let connected_peers = self.peer_manager.get_peer_node_ids();
let connected_peers_len = connected_peers.len();
for (node_id, con_addr_opt) in connected_peers {
let stored_peer = self.peer_store.get_peer(&node_id);
let stored_addr_opt = stored_peer.as_ref().map(|p| p.address.clone());
let address = match (con_addr_opt, stored_addr_opt) {
(Some(con_addr), _) => NetAddress(con_addr),
(None, Some(stored_addr)) => stored_addr,
(None, None) => continue,
Copy link
Contributor

Choose a reason for hiding this comment

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

When would this happen?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As we're using lightning-net-tokio and don't support OnionV3 currently, it shouldn't happen. In particular the conn_addr_opt should always be Some. However, I'd rather skip than unwrap here, as it doesn't hurt and is safer if something ever were to change.

};

let is_persisted = stored_peer.is_some();
let is_connected = true;
let details = PeerDetails { node_id, address, is_persisted, is_connected };
peers.push(details);
}

// Now add all known-but-offline peers, too.
for p in self.peer_store.list_peers() {
if peers.iter().take(connected_peers_len).find(|d| d.node_id == p.node_id).is_some() {
continue;
}

let details = PeerDetails {
node_id: p.node_id,
address: p.address.clone(),
is_connected: active_connected_peers.contains(&p.node_id),
})
.collect()
address: p.address,
is_persisted: true,
is_connected: false,
};

peers.push(details);
}

peers
}

/// Creates a digital ECDSA signature of a message with the node's secret key.
///
/// A receiver knowing the corresponding `PublicKey` (e.g. the node’s id) and the message
/// can be sure that the signature was generated by the caller.
/// Signatures are EC recoverable, meaning that given the message and the
/// signature the PublicKey of the signer can be extracted.
/// signature the `PublicKey` of the signer can be extracted.
pub fn sign_message(&self, msg: &[u8]) -> Result<String, Error> {
self.keys_manager.sign_message(msg)
}
Expand Down
52 changes: 27 additions & 25 deletions src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,26 +130,26 @@ impl Readable for UserChannelId {

/// Details of a channel as returned by [`Node::list_channels`].
///
/// [`Node::list_channels`]: [`crate::Node::list_channels`]
/// [`Node::list_channels`]: crate::Node::list_channels
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ChannelDetails {
/// The channel's ID (prior to funding transaction generation, this is a random 32 bytes,
/// thereafter this is the transaction ID of the funding transaction XOR the funding transaction
/// output).
/// The channel ID (prior to funding transaction generation, this is a random 32-byte
/// identifier, afterwards this is the transaction ID of the funding transaction XOR the
/// funding transaction output).
///
/// Note that this means this value is *not* persistent - it can change once during the
/// lifetime of the channel.
pub channel_id: ChannelId,
/// The `node_id` of our channel's counterparty.
/// The node ID of our the channel's counterparty.
pub counterparty_node_id: PublicKey,
/// The channel's funding transaction output, if we've negotiated the funding transaction with
/// our counterparty already.
pub funding_txo: Option<OutPoint>,
/// The value, in satoshis, of this channel as appears in the funding output.
pub channel_value_satoshis: u64,
/// The value, in satoshis, that must always be held in the channel for us. This value ensures
/// that if we broadcast a revoked state, our counterparty can punish us by claiming at least
/// this value on chain.
/// The value, in satoshis, of this channel as it appears in the funding output.
pub channel_value_sats: u64,
Copy link
Contributor

Choose a reason for hiding this comment

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

We use the plural sats but the singular msat below, it seems.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mh, true. Possibly we should decide one way and clean up all at once. This is however doesn't feel like the right place to do it?

/// The value, in satoshis, that must always be held as a reserve in the channel for us. This
/// value ensures that if we broadcast a revoked state, our counterparty can punish us by
/// claiming at least this value on chain.
///
/// This value is not included in [`outbound_capacity_msat`] as it can never be spent.
///
Expand All @@ -162,19 +162,19 @@ pub struct ChannelDetails {
/// The currently negotiated fee rate denominated in satoshi per 1000 weight units,
/// which is applied to commitment and HTLC transactions.
pub feerate_sat_per_1000_weight: u32,
/// Total balance of the channel. This is the amount that will be returned to the user if the
/// channel is closed.
/// The total balance of the channel. This is the amount that will be returned to
/// the user if the channel is closed.
///
/// The value is not exact, due to potential in-flight and fee-rate changes. Therefore, exactly
/// this amount is likely irrecoverable on close.
pub balance_msat: u64,
/// Available outbound capacity for sending HTLCs to the remote peer.
/// The available outbound capacity for sending HTLCs to the remote peer.
///
/// The amount does not include any pending HTLCs which are not yet resolved (and, thus, whose
/// balance is not available for inclusion in new outbound HTLCs). This further does not include
/// any pending outgoing HTLCs which are awaiting some other resolution to be sent.
pub outbound_capacity_msat: u64,
/// Available outbound capacity for sending HTLCs to the remote peer.
/// The available outbound capacity for sending HTLCs to the remote peer.
///
/// The amount does not include any pending HTLCs which are not yet resolved
/// (and, thus, whose balance is not available for inclusion in new inbound HTLCs). This further
Expand All @@ -190,13 +190,13 @@ pub struct ChannelDetails {
pub confirmations: Option<u32>,
/// Returns `true` if the channel was initiated (and therefore funded) by us.
pub is_outbound: bool,
/// Returns `true` if the channel is confirmed, both parties have exchanged `channel_ready`
/// messages, and the channel is not currently being shut down. Both parties exchange
/// `channel_ready` messages upon independently verifying that the required confirmations count
/// provided by `confirmations_required` has been reached.
/// Returns `true` if both parties have exchanged `channel_ready` messages, and the channel is
/// not currently being shut down. Both parties exchange `channel_ready` messages upon
/// independently verifying that the required confirmations count provided by
/// `confirmations_required` has been reached.
pub is_channel_ready: bool,
/// Returns `true` if the channel is (a) confirmed and `channel_ready` has been exchanged,
/// (b) the peer is connected, and (c) the channel is not currently negotiating shutdown.
/// Returns `true` if the channel (a) `channel_ready` messages have been exchanged, (b) the
/// peer is connected, and (c) the channel is not currently negotiating shutdown.
///
/// This is a strict superset of `is_channel_ready`.
pub is_usable: bool,
Expand All @@ -213,7 +213,7 @@ impl From<LdkChannelDetails> for ChannelDetails {
channel_id: ChannelId(value.channel_id),
counterparty_node_id: value.counterparty.node_id,
funding_txo: value.funding_txo.and_then(|o| Some(o.into_bitcoin_outpoint())),
channel_value_satoshis: value.channel_value_satoshis,
channel_value_sats: value.channel_value_satoshis,
unspendable_punishment_reserve: value.unspendable_punishment_reserve,
user_channel_id: UserChannelId(value.user_channel_id),
balance_msat: value.balance_msat,
Expand All @@ -233,14 +233,16 @@ impl From<LdkChannelDetails> for ChannelDetails {

/// Details of a known Lightning peer as returned by [`Node::list_peers`].
///
/// [`Node::list_peers`]: [`crate::Node::list_peers`]
/// [`Node::list_peers`]: crate::Node::list_peers
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct PeerDetails {
/// Our peer's node ID.
/// The node ID of the peer.
pub node_id: PublicKey,
/// The IP address and TCP port of the peer.
/// The network address of the peer.
pub address: NetAddress,
/// Indicates whether or not the user is currently has an active connection with the peer.
/// Indicates whether we'll try to reconnect to this peer after restarts.
pub is_persisted: bool,
/// Indicates whether we currently have an active connection with the peer.
pub is_connected: bool,
}

Expand Down