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 list_channels_by_counterparty method #2079

Conversation

tnull
Copy link
Contributor

@tnull tnull commented Mar 7, 2023

While we already provide a list_channels method, it could result in quite a large Vec<ChannelDetails>. Here, we provide the means to query our channels by counterparty_node_id and DRY up the code.

@tnull
Copy link
Contributor Author

tnull commented Mar 7, 2023

Uh, CI is broken again, although seems unrelated:

error[E0793]: reference to packed field is unaligned
    --> C:\Users\runneradmin\.cargo\registry\src\github.com-1ecc6299db9ec823\ntapi-0.3.7\src\ntexapi.rs:2783:52
     |
2783 |         *tick_count.QuadPart_mut() = read_volatile(&(*USER_SHARED_DATA).u.TickCountQuad);
     |                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
     = help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error[E0793]: reference to packed field is unaligned
    --> C:\Users\runneradmin\.cargo\registry\src\github.com-1ecc6299db9ec823\ntapi-0.3.7\src\ntexapi.rs:2807:25
     |
2807 |         ((read_volatile(&(*USER_SHARED_DATA).u.TickCountQuad)
     |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
     = help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

For more information about this error, try `rustc --explain E0793`.
error: could not compile `ntapi` due to 2 previous errors

@codecov-commenter
Copy link

codecov-commenter commented Mar 7, 2023

Codecov Report

Patch coverage: 93.10% and project coverage change: +0.12 🎉

Comparison is base (d355ce1) 91.13% compared to head (8419d13) 91.25%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2079      +/-   ##
==========================================
+ Coverage   91.13%   91.25%   +0.12%     
==========================================
  Files         101      101              
  Lines       48769    49913    +1144     
  Branches    48769    49913    +1144     
==========================================
+ Hits        44445    45550    +1105     
- Misses       4324     4363      +39     
Impacted Files Coverage Δ
lightning/src/ln/channelmanager.rs 88.87% <93.10%> (-0.05%) ⬇️
lightning/src/ln/functional_tests.rs 98.24% <0.00%> (+<0.01%) ⬆️
lightning-background-processor/src/lib.rs 78.51% <0.00%> (+0.09%) ⬆️
lightning/src/ln/peer_channel_encryptor.rs 92.85% <0.00%> (+0.51%) ⬆️
lightning-rapid-gossip-sync/src/lib.rs 88.77% <0.00%> (+0.61%) ⬆️
lightning-invoice/src/utils.rs 98.13% <0.00%> (+1.22%) ⬆️
lightning/src/ln/functional_test_utils.rs 94.34% <0.00%> (+2.01%) ⬆️
lightning/src/ln/peer_handler.rs 65.17% <0.00%> (+2.57%) ⬆️
lightning-invoice/src/de.rs 79.22% <0.00%> (+3.20%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved
@@ -1786,6 +1794,25 @@ where
self.list_channels_with_filter(|&(_, ref channel)| channel.is_live())
}

/// Gets the list of channels we have with a given counterparty, in random order.
pub fn list_channels_with_counterparty(&self, counterparty_node_id: &PublicKey) -> Vec<ChannelDetails> {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this significantly faster than just doing a .filter on list_channels?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should avoid the allocation of a large Vec<ChannelDetails> that we then filter down again. But really not sure what that does in actual performance. It would be nice to just expose list_channels_with_filter, but we of course can't do that since Channel is not public.

/// Gets the list of channels we have with a given counterparty, in random order.
pub fn list_channels_with_counterparty(&self, counterparty_node_id: &PublicKey) -> Vec<ChannelDetails> {
let mut res = Vec::with_capacity(self.short_to_chan_info.read().unwrap().len());
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Scope not needed.

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@jkczyz jkczyz left a comment

Choose a reason for hiding this comment

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

LGTM

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved
lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved
While we already provide a `list_channels` method, it could result in
quite a large `Vec<ChannelDetails>`. Here, we provide the means to query
our channels by `counterparty_node_id` and DRY up the code.
@tnull tnull force-pushed the 2023-03-add-list-channel-by-counterparty branch from f08bdb5 to 8419d13 Compare March 7, 2023 18:50
@tnull
Copy link
Contributor Author

tnull commented Mar 7, 2023

Alright, never in a hast trust vim's formatting capabilities.

Squashed commits with the indentation fixups:

$ git diff-tree -U2 f08bdb57 8419d13d
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 779cb5d2..50cdd2a4 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -1251,5 +1251,5 @@ impl ChannelDetails {
                                outbound_htlc_minimum_msat: if channel.have_received_message() {
                                        Some(channel.get_counterparty_htlc_minimum_msat()) } else { None },
-                                       outbound_htlc_maximum_msat: channel.get_counterparty_htlc_maximum_msat(),
+                               outbound_htlc_maximum_msat: channel.get_counterparty_htlc_maximum_msat(),
                        },
                        funding_txo: channel.get_funding_txo(),
@@ -1807,5 +1807,5 @@ where
                                .iter()
                                .map(|(_, channel)|
-                                        ChannelDetails::from_channel(channel, best_block_height, features.clone()))
+                                       ChannelDetails::from_channel(channel, best_block_height, features.clone()))
                                .collect();
                }

@TheBlueMatt TheBlueMatt merged commit 36834b3 into lightningdevkit:main Mar 8, 2023
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.

None yet

6 participants