Skip to content

feat: return supported protocols in id output #7409

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.

Sign up for GitHub

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

Merged
aschmahmann merged 4 commits into from
Aug 14, 2020
Merged

feat: return supported protocols in id output #7409

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dd1093f
fix: use connect instead of findpeer so we ensure we actually connect
Stebalien May 27, 2020
dd47364
feat: add protocol list to ipfs id
Stebalien May 27, 2020
d6b9436
fix: sort protocols returned by ipfs id
aschmahmann Aug 9, 2020
34c0e5c
fix: sort addresses returned by ipfs id
aschmahmann Aug 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions core/commands/id.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"io"
"sort"
"strings"

version "github.com/ipfs/go-ipfs"
Expand Down Expand Up @@ -36,6 +37,7 @@ type IdOutput struct {
Addresses []string
AgentVersion string
ProtocolVersion string
Protocols []string
}

const (
Expand Down Expand Up @@ -97,15 +99,17 @@ EXAMPLE:
return errors.New(offlineIdErrorMessage)
}

p, err := n.Routing.FindPeer(req.Context, id)
if err == kb.ErrLookupFailure {
// We need to actually connect to run identify.
err = n.PeerHost.Connect(req.Context, peer.AddrInfo{ID: id})
Comment on lines +102 to +103
Copy link
Contributor

@aschmahmann aschmahmann Aug 9, 2020

Choose a reason for hiding this comment

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

This is a fine conceptual change. Just a note that this doesn't actually do anything as the DHT (our only PeerRouting system) will connect to the peer, and will not return any addresses if it fails. https://github.com/libp2p/go-libp2p-kad-dht/blob/fc3558cc35274f5d1727dad8201f821df1ee6317/routing.go#L675-L682

Copy link
Member Author

@Stebalien Stebalien Aug 11, 2020

Choose a reason for hiding this comment

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

Hm. I thought I had some trouble where we weren't actually waiting for the connect event to finish.

But honestly, we should do this anyways. We shouldn't assume that the router will always end up connecting to the target peer.

switch err {
case nil:
case kb.ErrLookupFailure:
return errors.New(offlineIdErrorMessage)
}
if err != nil {
default:
return err
}

output, err := printPeer(n.Peerstore, p.ID)
output, err := printPeer(n.Peerstore, id)
if err != nil {
return err
}
Expand All @@ -121,6 +125,7 @@ EXAMPLE:
output = strings.Replace(output, "<pver>", out.ProtocolVersion, -1)
output = strings.Replace(output, "<pubkey>", out.PublicKey, -1)
output = strings.Replace(output, "<addrs>", strings.Join(out.Addresses, "\n"), -1)
output = strings.Replace(output, "<protocols>", strings.Join(out.Protocols, "\n"), -1)
output = strings.Replace(output, "\\n", "\n", -1)
output = strings.Replace(output, "\\t", "\t", -1)
fmt.Fprint(w, output)
Expand Down Expand Up @@ -163,6 +168,13 @@ func printPeer(ps pstore.Peerstore, p peer.ID) (interface{}, error) {
for _, a := range addrs {
info.Addresses = append(info.Addresses, a.String())
}
sort.Strings(info.Addresses)
Copy link
Contributor

@aschmahmann aschmahmann Aug 9, 2020

Choose a reason for hiding this comment

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

I added address sorting to this function, any objections?

Copy link
Member Author

@Stebalien Stebalien Aug 11, 2020

Choose a reason for hiding this comment

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

LGTM!


protocols, _ := ps.GetProtocols(p) // don't care about errors here.
for _, p := range protocols {
info.Protocols = append(info.Protocols, string(p))
}
sort.Strings(info.Protocols)

if v, err := ps.Get(p, "ProtocolVersion"); err == nil {
if vs, ok := v.(string); ok {
Expand Down Expand Up @@ -198,6 +210,9 @@ func printSelf(node *core.IpfsNode) (interface{}, error) {
for _, a := range addrs {
info.Addresses = append(info.Addresses, a.String())
}
sort.Strings(info.Addresses)
Copy link
Contributor

@aschmahmann aschmahmann Aug 9, 2020

Choose a reason for hiding this comment

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

I added address sorting to this function, any objections?

info.Protocols = node.PeerHost.Mux().Protocols()
sort.Strings(info.Protocols)
}
info.ProtocolVersion = identify.LibP2PVersion
info.AgentVersion = version.UserAgent
Expand Down