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

p2p/enode: improve IPv6 support, add ENR text representation #19663

Merged
merged 6 commits into from
Jun 7, 2019

Conversation

fjl
Copy link
Contributor

@fjl fjl commented Jun 4, 2019

This PR adapts p2p/enr and p2p/enode to the final version of EIP-778 (ethereum/EIPs#2097). It also changes all invocations of enode.ParseV4 to enode.Parse, allowing signed ENRs to be set as bootnodes and static peers.

fjl added 2 commits June 5, 2019 13:54
This adds entry types for "ip6", "udp6", "tcp6" keys. The IP type stays
around because removing it would break a lot of code and force everyone
to care about the distinction.

I also added a Signature method for completeness' sake.
@fjl fjl added this to the 1.9.0 milestone Jun 5, 2019
fjl added 4 commits June 5, 2019 16:35
LocalNode predicts the local node's UDP endpoint and updates the record.
This change makes it predict IPv4 and IPv6 endpoints separately since
they can now be in the record at the same time.
This allows passing base64-encoded node records to all the places that
previously accepted enode:// URLs. The URL format is still supported.
...and return the base64 record in NodeInfo.
Copy link
Member

@gballet gballet left a comment

Choose a reason for hiding this comment

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

Tested the Whisper stuff with IPV4, works fine.

@@ -747,9 +747,9 @@ func requestExpiredMessagesLoop() {
}

func extractIDFromEnode(s string) []byte {
n, err := enode.ParseV4(s)
Copy link
Member

Choose a reason for hiding this comment

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

Quick reminder to also change the NodeInfo().Enode on line 302

Copy link
Member

@FrankSzendzielarz FrankSzendzielarz left a comment

Choose a reason for hiding this comment

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

LGTM + tests pass on Windows

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