Skip to content

Commit

Permalink
fix(p2p.QriConnectePeers): properly tag QriPeers on both ends of the …
Browse files Browse the repository at this point in the history
…ConnManager

turns out we weren't properly tagging Qri peers in the ConnManager, which was preventing
the receiving end of a p2p connection from reporting a peer as supporting Qri when
ConnectedQriPeers is called. This is fixed by moving peerstore and connmanager manipluation
lower in the connection ceremony
  • Loading branch information
b5 committed Oct 10, 2018
1 parent 183f111 commit 14d473f
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 13 deletions.
14 changes: 11 additions & 3 deletions p2p/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,11 @@ func (n *QriNode) SendMessage(msg Message, replies chan Message, pids ...peer.ID
}
defer s.Close()

// now that we have a confirmed working connection
// tag this peer as supporting the qri protocol in the connection manager
n.Host.ConnManager().TagPeer(peerID, qriConnManagerTag, qriConnManagerValue)
n.Host.Peerstore().AddAddr(peerID, s.Conn().RemoteMultiaddr(), pstore.TempAddrTTL)

ws := WrapStream(s)
go n.handleStream(ws, replies)
if err := ws.sendMessage(msg); err != nil {
Expand Down Expand Up @@ -333,6 +338,10 @@ func (n *QriNode) handleStream(ws *WrappedStream, replies chan Message) {
break
}

conn := ws.stream.Conn()
n.Host.ConnManager().TagPeer(conn.RemotePeer(), qriConnManagerTag, qriConnManagerValue)
n.Host.Peerstore().AddAddr(conn.RemotePeer(), conn.RemoteMultiaddr(), pstore.TempAddrTTL)

if replies != nil {
go func() { replies <- msg }()
}
Expand Down Expand Up @@ -385,9 +394,8 @@ func (n *QriNode) HostNetwork() net.Network {
// MakeHandlers generates a map of MsgTypes to their corresponding handler functions
func MakeHandlers(n *QriNode) map[MsgType]HandlerFunc {
return map[MsgType]HandlerFunc{
MtPing: n.handlePing,
MtProfile: n.handleProfile,
// MtProfiles: n.handleProfiles,
MtPing: n.handlePing,
MtProfile: n.handleProfile,
MtDatasetInfo: n.handleDataset,
MtDatasets: n.handleDatasetsList,
MtEvents: n.handleEvents,
Expand Down
4 changes: 2 additions & 2 deletions p2p/peers.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ func (n *QriNode) PeerInfo(pid peer.ID) pstore.PeerInfo {
// AddQriPeer negotiates a connection with a peer to get their profile details
// and peer list.
func (n *QriNode) AddQriPeer(pinfo pstore.PeerInfo) error {
// add this peer to our store
// add this peer to our store so libp2p has the provided addresses of
// the peer in the next call
n.Host.Peerstore().AddAddrs(pinfo.ID, pinfo.Addrs, pstore.TempAddrTTL)
n.Host.ConnManager().TagPeer(pinfo.ID, qriConnManagerTag, qriConnManagerValue)

if _, err := n.RequestProfile(pinfo.ID); err != nil {
log.Debug(err.Error())
Expand Down
1 change: 1 addition & 0 deletions p2p/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package p2p
import (
"encoding/json"
"fmt"

// "time"

"github.com/qri-io/qri/config"
Expand Down
8 changes: 4 additions & 4 deletions p2p/profile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ func TestRequestProfileConnectNodes(t *testing.T) {

addrsP1 := p1.Host.Peerstore().Addrs(p2.ID)
if len(addrsP1) == 0 {
t.Errorf("%s (request node) should have addrs of %s (response node)", p1.ID.Pretty(), p2.ID.Pretty())
t.Errorf("%s (request node) should have addrs of %s (response node)", p1.ID, p2.ID)
}
addrsP2 := p2.Host.Peerstore().Addrs(p1.ID)
if len(addrsP2) == 0 {
t.Errorf("%s (request node) should have addrs of %s (response node)", p2.ID.Pretty(), p1.ID.Pretty())
t.Errorf("%s (request node) should have addrs of %s (response node)", p2.ID, p1.ID)
}

pid := pro.PeerIDs[0]
Expand Down Expand Up @@ -147,11 +147,11 @@ func TestRequestProfileOneWayConnection(t *testing.T) {

peerInfo2 := p1.Host.Peerstore().PeerInfo(p2.ID)
if len(peerInfo2.Addrs) == 0 {
t.Errorf("%s (request node) should have addrs of %s (response node)", p1.ID.Pretty(), p2.ID.Pretty())
t.Errorf("%s (request node) should have addrs of %s (response node)", p1.ID, p2.ID)
}
peerInfo1 := p2.Host.Peerstore().PeerInfo(p1.ID)
if len(peerInfo1.Addrs) == 0 {
t.Errorf("%s (request node) should have addrs of %s (response node)", p2.ID.Pretty(), p1.ID.Pretty())
t.Errorf("%s (response node) should have addrs of %s (request node)", p2.ID, p1.ID)
}

pid := pro.PeerIDs[0]
Expand Down
5 changes: 1 addition & 4 deletions p2p/qri_peers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package p2p

import (
"context"
"fmt"
"testing"
"time"

Expand All @@ -14,7 +13,6 @@ import (
// Test passes when the fifth node connects to the other three nodes by asking
// it's one connection for the other three peer's profiles
func TestSharePeers(t *testing.T) {
fmt.Println("hallo?")
ctx := context.Background()
f := p2ptest.NewTestNodeFactory(NewTestableQriNode)
testPeers, err := p2ptest.NewTestNetwork(ctx, f, 5)
Expand All @@ -33,9 +31,8 @@ func TestSharePeers(t *testing.T) {
done := make(chan bool)
deadline := time.NewTimer(time.Second * 2)
go func() {
for msg := range nasma.ReceiveMessages() {
for range nasma.ReceiveMessages() {
if len(nasma.ConnectedPeers()) == len(group) {
fmt.Println(msg.Type, len(nasma.ConnectedPeers()))
done <- true
}
}
Expand Down

0 comments on commit 14d473f

Please sign in to comment.