Skip to content

Expose list peers and channels #56

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

Conversation

jurvis
Copy link
Contributor

@jurvis jurvis commented Mar 31, 2023

Based on #25.

Transferred from tnull#3

@tnull tnull mentioned this pull request Apr 4, 2023
47 tasks
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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mh, I wonder if it would be nice to have a is_persisted: bool field on PeerDetails so that this also lists connected-but-not-stored peers. I think it should be possible to get all necessary information from get_peer_node_ids. Feel free to add this here, otherwise I'll add a tracking issue, since it might be easier after our eventual transition to NetAddress.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah... I just took a stab at it and it looks like it may be easier if we both had PeerInfo and the tuple returns by get_peer_node_ids return a consistent type for address.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alright, we'll see what lands first, feel free to leave as is currently.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Now tracking here: #62

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jurvis I think this will be possible now as soon as #85 lands.

src/types.rs Outdated
/// Information about the channel's funding transaction output. `None `unless a funding
/// transaction has been successfully negotiated with the channel's counterparty.
pub funding_txo: Option<OutPoint>,
/// Position of the funding transaction on-chain. `None` unless the funding transaction has been
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mh, yeah, good question after all. Actually I'd like to avoid exposing the complexity of SCID aliases to the user, but I'm not sure we can now that LSP spec might depend on them. Although, maybe we should leave them out for now after all and really just add them when integrating the LSP client.

@jurvis jurvis force-pushed the jurvis/2022-09-start-uniffi/expose-list-peers-and-channels branch 2 times, most recently from 5506f23 to 49f7f59 Compare April 22, 2023 00:43
@jurvis
Copy link
Contributor Author

jurvis commented Apr 22, 2023

resolved conflicts and squashed into 5506f23

@tnull
Copy link
Collaborator

tnull commented Apr 24, 2023

resolved conflicts and squashed into 5506f23

Not exactly sure what happened, but seems your changes are gone, i.e., you just pushed the branch you meant to rebase on?

@jurvis
Copy link
Contributor Author

jurvis commented Apr 24, 2023

Oops, I think I forgot to push something/had a cached version of the page. Squashed into 025b6d2

@jurvis jurvis force-pushed the jurvis/2022-09-start-uniffi/expose-list-peers-and-channels branch from d58cf3c to 53db279 Compare April 24, 2023 17:04
@jurvis
Copy link
Contributor Author

jurvis commented Apr 24, 2023

looks like there may be some conflicts in #25

@tnull
Copy link
Collaborator

tnull commented Apr 25, 2023

looks like there may be some conflicts in #25

Yup, just rebased #25, so you'll need to do the same here.

@tnull tnull added this to the 0.1 milestone Apr 25, 2023
@jurvis jurvis force-pushed the jurvis/2022-09-start-uniffi/expose-list-peers-and-channels branch from 53db279 to 25c947d Compare April 26, 2023 17:48
@jurvis
Copy link
Contributor Author

jurvis commented Apr 26, 2023

hm, looks like there are still conflicts?

@tnull
Copy link
Collaborator

tnull commented Apr 26, 2023

hm, looks like there are still conflicts?

The princess is in another castle.

@jurvis jurvis force-pushed the jurvis/2022-09-start-uniffi/expose-list-peers-and-channels branch from 25c947d to 50e7368 Compare April 26, 2023 17:54
@jurvis
Copy link
Contributor Author

jurvis commented Apr 26, 2023

ok looking good -- rebased to 50e7368

@jurvis jurvis force-pushed the jurvis/2022-09-start-uniffi/expose-list-peers-and-channels branch from 50e7368 to c726ef9 Compare April 26, 2023 19:01
@jurvis
Copy link
Contributor Author

jurvis commented Apr 26, 2023

Fixed lint errors and squashed into c726ef9

@jurvis jurvis force-pushed the jurvis/2022-09-start-uniffi/expose-list-peers-and-channels branch from c726ef9 to 5a1fbe6 Compare May 10, 2023 03:59
@jurvis
Copy link
Contributor Author

jurvis commented May 10, 2023

rebased to 5a1fbe6

src/types.rs Outdated
/// Information about the channel's funding transaction output. `None `unless a funding
/// transaction has been successfully negotiated with the channel's counterparty.
pub funding_txo: Option<OutPoint>,
/// Position of the funding transaction on-chain. `None` unless the funding transaction has been
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jurvis Lets leave the fields and the mentions of scid aliases out for now and re-add them if needed later. Now tracking here: #90

# Conflicts:
#	bindings/ldk_node.udl



# Conflicts:
#	src/types.rs
@jurvis jurvis force-pushed the jurvis/2022-09-start-uniffi/expose-list-peers-and-channels branch from 5a1fbe6 to ce70bfe Compare May 11, 2023 18:50
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

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

LGTM, we can address any outstanding issues in a follow-up.

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