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

correctly handle WebTransport addresses without certhashes #2239

Merged
merged 15 commits into from
Apr 6, 2023

Conversation

MarcoPolo
Copy link
Collaborator

Fixes #2233. Closes #2227. Fixes #2223.

options.go Outdated
@@ -262,6 +262,7 @@ func AddrsFactory(factory config.AddrsFactory) Option {
if cfg.AddrsFactory != nil {
return fmt.Errorf("cannot specify multiple address factories")
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change


// NormalizeMultiaddr returns a multiaddr suitable for equality checks.
// If the multiaddr is a webtransport component, it removes the certhashes.
func (h *BasicHost) NormalizeMultiaddr(addr ma.Multiaddr) ma.Multiaddr {
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this would move into the address pipeline, once we have it, right?

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! Anywhere that isn't adding another method to BasicHost

p2p/host/basic/basic_host.go Outdated Show resolved Hide resolved
p2p/host/basic/basic_host.go Show resolved Hide resolved
@marten-seemann marten-seemann changed the title Handle WebTransport multiaddrs in Observed Addr correctly handle WebTransport addresses without certhashes Apr 6, 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
2 participants