Skip to content

Commit

Permalink
routed host: return Connect error if FindPeer doesn't yield new addre…
Browse files Browse the repository at this point in the history
…sses (#1946)
  • Loading branch information
marten-seemann authored Dec 12, 2022
1 parent 3f024f7 commit 735891d
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 23 deletions.
9 changes: 4 additions & 5 deletions p2p/host/routed/routed.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,7 @@ func (rh *RoutedHost) Connect(ctx context.Context, pi peer.AddrInfo) error {

// if we're here, we got some addrs. let's use our wrapped host to connect.
pi.Addrs = addrs
err := rh.host.Connect(ctx, pi)
if err != nil {
if cerr := rh.host.Connect(ctx, pi); cerr != nil {
// We couldn't connect. Let's check if we have the most
// up-to-date addresses for the given peer. If there
// are addresses we didn't know about previously, we
Expand All @@ -135,10 +134,10 @@ func (rh *RoutedHost) Connect(ctx context.Context, pi peer.AddrInfo) error {
pi.Addrs = newAddrs
return rh.host.Connect(ctx, pi)
}

return err
// No appropriate new address found.
// Return the original dial error.
return cerr
}

return nil
}

Expand Down
53 changes: 35 additions & 18 deletions p2p/host/routed/routed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (
"github.com/libp2p/go-libp2p/core/peer"
basic "github.com/libp2p/go-libp2p/p2p/host/basic"
swarmt "github.com/libp2p/go-libp2p/p2p/net/swarm/testing"

ma "github.com/multiformats/go-multiaddr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand All @@ -25,8 +25,6 @@ func (m *mockRouting) FindPeer(ctx context.Context, pid peer.ID) (peer.AddrInfo,
}

func TestRoutedHostConnectToObsoleteAddresses(t *testing.T) {
ctx := context.Background()

h1, err := basic.NewHost(swarmt.GenSwarm(t), nil)
require.NoError(t, err)
defer h1.Close()
Expand All @@ -35,23 +33,19 @@ func TestRoutedHostConnectToObsoleteAddresses(t *testing.T) {
require.NoError(t, err)
defer h2.Close()

// construct a wrong multi address for host 2, so that
// the initial connection attempt will fail
// (we have obsolete, old multi address information)
maddr, err := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/1234")
require.NoError(t, err)

// assemble the AddrInfo struct to use for the connection attempt
pi := peer.AddrInfo{
ID: h2.ID(),
Addrs: []ma.Multiaddr{maddr},
ID: h2.ID(),
// Use a wrong multi address for host 2, so that the initial connection attempt will fail
// (we have obsolete, old multi address information)
Addrs: []ma.Multiaddr{ma.StringCast("/ip4/127.0.0.1/tcp/1234")},
}

// Build mock routing module and replace the FindPeer function.
// Now, that function will return the correct multi addresses for host 2
// (we have fetched the most up-to-date data from the DHT)
mr := &mockRouting{
findPeerFn: func(ctx context.Context, pi peer.ID) (peer.AddrInfo, error) {
findPeerFn: func(context.Context, peer.ID) (peer.AddrInfo, error) {
return peer.AddrInfo{
ID: h2.ID(),
Addrs: h2.Addrs(),
Expand All @@ -61,13 +55,36 @@ func TestRoutedHostConnectToObsoleteAddresses(t *testing.T) {

// Build routed host
rh := Wrap(h1, mr)
// Connection establishment should have worked without an error
require.NoError(t, rh.Connect(context.Background(), pi))
require.Equal(t, 1, mr.callCount, "the mocked FindPeer function should have been called")
}

// Try to connect
err = rh.Connect(ctx, pi)
func TestRoutedHostConnectFindPeerNoUsefulAddrs(t *testing.T) {
h1, err := basic.NewHost(swarmt.GenSwarm(t), nil)
require.NoError(t, err)
defer h1.Close()

// Connection establishment should have worked without an error
assert.NoError(t, err)
h2, err := basic.NewHost(swarmt.GenSwarm(t), nil)
require.NoError(t, err)
defer h2.Close()

// assemble the AddrInfo struct to use for the connection attempt
pi := peer.AddrInfo{
ID: h2.ID(),
// Use a wrong multi address for host 2, so that the initial connection attempt will fail
// (we have obsolete, old multi address information)
Addrs: []ma.Multiaddr{ma.StringCast("/ip4/127.0.0.1/tcp/1234")},
}

// The mocked FindPeer function should have been called
assert.Equal(t, 1, mr.callCount)
// Build mock routing module and replace the FindPeer function.
// Now, that function will return the correct multi addresses for host 2
// (we have fetched the most up-to-date data from the DHT)
mr := &mockRouting{findPeerFn: func(context.Context, peer.ID) (peer.AddrInfo, error) { return pi, nil }}

// Build routed host
rh := Wrap(h1, mr)
// Connection establishment should fail, since we didn't provide any useful addresses in FindPeer.
require.Error(t, rh.Connect(context.Background(), pi))
require.Equal(t, 1, mr.callCount, "the mocked FindPeer function should have been called")
}

0 comments on commit 735891d

Please sign in to comment.