Skip to content

Conversation

@vyzo
Copy link
Collaborator

@vyzo vyzo commented Apr 8, 2021

Depends on multiformats/go-multiaddr-dns#26

TBD:

  • tests

@vyzo vyzo requested review from Stebalien, aschmahmann and lidel April 8, 2021 10:15
@vyzo vyzo requested a review from lidel April 8, 2021 15:03
Copy link
Contributor

@aschmahmann aschmahmann left a comment

Choose a reason for hiding this comment

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

I'm not so familiar with the DoH specifics, but it looks like you're just using the libraries and testing against actual DoH servers in the wild and it works.

If @lidel is happy then I'm happy 😄

@vyzo
Copy link
Collaborator Author

vyzo commented Apr 9, 2021

Updated for go-multiaddr-dns@v0.3.0, as the upstream dependency has been merged.

Awaiting @lidel's review.

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

LGTM. Some suggestions for improved tests below, but no blockers.

"testing"
)

func TestLookupIPAddr(t *testing.T) {
Copy link
Member

@lidel lidel Apr 9, 2021

Choose a reason for hiding this comment

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

Would be good to have an explicit check that confirms this returns both ipv4 AND ipv6 (libp2p.io has both, so good candidate)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sure, let me do that/

vyzo and others added 2 commits April 12, 2021 11:45
@vyzo vyzo merged commit 5933266 into main Apr 12, 2021
@vyzo vyzo deleted the implementation branch April 12, 2021 08:49
@lidel lidel mentioned this pull request Apr 12, 2021
9 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.

4 participants