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

Convert deprecated listpeer usage to listpeerchannels #198

Merged
merged 3 commits into from
Jun 2, 2024

Conversation

ksedgwic
Copy link
Collaborator

Construct a "compatibility struct" to convert listpeerchannels output into a "virtual" legacy listpeers format.

This maintains the semantics of all of the use sites which currently traverse peer, then channel to make decisions and take actions.

Tests written using the legacy listpeers format can use the convert_legacy_listpeers utility to construct a compatibility struct.

The test_peerjudge_datagatherer malformed test needed to be malformed differently to achieve the desired effect.

A couple `listpeers` uses can remain because they don't need channel
information.  Others use ListpeersAnnouncer and are covered by the
next commit.
Construct a "compatibility struct" to convert `listpeerchannels`
output into legacy `listpeers` format.

Tests written using the legacy listpeers format can use the
`convert_legacy_listpeers` utility to construct a compatibility
struct.

The test_peerjudge_datagatherer malformed test needed to be malformed
differently to achieve the desired effect.
@chrisguida
Copy link
Contributor

Testing on mutinynet

Copy link
Contributor

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

ACK 1cfd860

std::ostream& operator<<(std::ostream& os, Boss::Mod::ConstructedListpeers const& o) {
for (auto p : o) {
os << p.first << ':'
<< "connected: " << p.second.connected
Copy link
Contributor

Choose a reason for hiding this comment

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

curious what this second does

Copy link
Contributor

Choose a reason for hiding this comment

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

access the (key, vale) in c++ is (first, second)

I bet that ConstructedListpeers is a map

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah cool. How can I verify this? Struggling to find the definition of ConstructedListpeers

Copy link
Contributor

Choose a reason for hiding this comment

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

What I do usually is git grep -n "ConstructedListpeers" and then search inside the result

In this case, I found the following line Boss/Mod/ConstructedListpeers.hpp:24:typedef std::map<Ln::NodeId, ConstructedListpeer> ConstructedListpeers;

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, thanks!! Still getting familiar with how C++ does things...

@ksedgwic
Copy link
Collaborator Author

ksedgwic commented Jun 2, 2024

rebased and merging

@ksedgwic ksedgwic merged commit 0bd255c into ZmnSCPxj:master Jun 2, 2024
1 check passed
@ksedgwic ksedgwic mentioned this pull request Jun 17, 2024
8 tasks
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.

3 participants