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

Sort addresses in node announcement #1693

Merged
merged 1 commit into from
Feb 17, 2021
Merged

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented Feb 17, 2021

Addresses in node announcement should be sorted.
We accept node announcements that don't do this, but we should do it for our own announcements.

See lightning/bolts#842

Addresses in node announcement should be sorted.
We accept node announcements that don't do this, but we should do it for
our own announcements.

See lightning/bolts#842
@t-bast t-bast requested a review from pm47 February 17, 2021 10:52
Comment on lines +75 to +80
val sortedAddresses = nodeAddresses.map {
case address@(_: IPv4) => (1, address)
case address@(_: IPv6) => (2, address)
case address@(_: Tor2) => (3, address)
case address@(_: Tor3) => (4, address)
}.sortBy(_._1).map(_._2)
Copy link
Member

Choose a reason for hiding this comment

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

We could have defined an Ordering[NodeAddress], but that would probably be over engineering.

Copy link
Member Author

Choose a reason for hiding this comment

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

I toyed with it 😃 but decided that it was indeed a bit over-engineered

@t-bast t-bast merged commit 82e5b59 into master Feb 17, 2021
@t-bast t-bast deleted the sort-node-announcement-addresses branch February 17, 2021 14:12
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