From e29d2cf995630bb553aacd4eeb6dc73142d4db43 Mon Sep 17 00:00:00 2001 From: Dennis Trautwein Date: Thu, 20 Oct 2022 12:21:48 +0200 Subject: [PATCH 1/4] feat: search for new multi addresses upon connect failure --- p2p/host/routed/routed.go | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/p2p/host/routed/routed.go b/p2p/host/routed/routed.go index 55c8d3474e..a3e2d48142 100644 --- a/p2p/host/routed/routed.go +++ b/p2p/host/routed/routed.go @@ -107,7 +107,39 @@ 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 - return rh.host.Connect(ctx, pi) + err := rh.host.Connect(ctx, pi) + if err != 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 + // try to connect again. + newAddrs, addrErr := rh.findPeerAddrs(ctx, pi.ID) + if addrErr != nil { + return err // return outer error + } + + // Build lookup map + lookup := make(map[ma.Multiaddr]struct{}, len(addrs)) + for _, addr := range addrs { + lookup[addr] = struct{}{} + } + + // if there's any address that's not in the previous set + // of addresses, try to connect again. If all addresses + // where known previously we return the original error. + for _, newAddr := range newAddrs { + if _, found := lookup[newAddr]; found { + continue + } + + pi.Addrs = newAddrs + return rh.host.Connect(ctx, pi) + } + + return err + } + + return nil } func (rh *RoutedHost) findPeerAddrs(ctx context.Context, id peer.ID) ([]ma.Multiaddr, error) { From 0e1b7dfd5308c0cdf122dce78fabbe44916163de Mon Sep 17 00:00:00 2001 From: Dennis Trautwein Date: Fri, 21 Oct 2022 12:59:06 +0100 Subject: [PATCH 2/4] chore: removed unused provider addr ttl constant --- core/peerstore/peerstore.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/core/peerstore/peerstore.go b/core/peerstore/peerstore.go index 213513bcda..7561f32be2 100644 --- a/core/peerstore/peerstore.go +++ b/core/peerstore/peerstore.go @@ -25,11 +25,6 @@ var ( // TempAddrTTL is the ttl used for a short lived address. TempAddrTTL = time.Minute * 2 - // ProviderAddrTTL is the TTL of an address we've received from a provider. - // This is also a temporary address, but lasts longer. After this expires, - // the records we return will require an extra lookup. - ProviderAddrTTL = time.Minute * 30 - // RecentlyConnectedAddrTTL is used when we recently connected to a peer. // It means that we are reasonably certain of the peer's address. RecentlyConnectedAddrTTL = time.Minute * 30 From eb1e504b101427de85da9abc93b5ef49c9a9b0f7 Mon Sep 17 00:00:00 2001 From: Dennis Trautwein Date: Fri, 21 Oct 2022 16:07:49 +0100 Subject: [PATCH 3/4] test: add routed host obsolete maddr test --- p2p/host/routed/routed_test.go | 73 ++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 p2p/host/routed/routed_test.go diff --git a/p2p/host/routed/routed_test.go b/p2p/host/routed/routed_test.go new file mode 100644 index 0000000000..d4b9daea4b --- /dev/null +++ b/p2p/host/routed/routed_test.go @@ -0,0 +1,73 @@ +package routedhost + +import ( + "context" + "testing" + + "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" +) + +var _ Routing = (*mockRouting)(nil) + +type mockRouting struct { + callCount int + findPeerFn func(ctx context.Context, id peer.ID) (peer.AddrInfo, error) +} + +func (m *mockRouting) FindPeer(ctx context.Context, pid peer.ID) (peer.AddrInfo, error) { + m.callCount += 1 + return m.findPeerFn(ctx, pid) +} + +func TestRoutedHost_Connect_obsoleteAddresses(t *testing.T) { + ctx := context.Background() + + h1, err := basic.NewHost(swarmt.GenSwarm(t), nil) + require.NoError(t, err) + defer h1.Close() + + h2, err := basic.NewHost(swarmt.GenSwarm(t), nil) + 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}, + } + + // 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) { + return peer.AddrInfo{ + ID: h2.ID(), + Addrs: h2.Addrs(), + }, nil + }, + } + + // Build routed host + rh := Wrap(h1, mr) + + // Try to connect + err = rh.Connect(ctx, pi) + + // Connection establishment should have worked without an error + assert.NoError(t, err) + + // The mocked FindPeer function should have been called + assert.Equal(t, 1, mr.callCount) +} From 92a0d7d4a17076c00056fd1986fb29f1a072f2b9 Mon Sep 17 00:00:00 2001 From: Dennis Trautwein Date: Sat, 29 Oct 2022 11:33:30 +0100 Subject: [PATCH 4/4] incorporate pr feedback --- p2p/host/routed/routed.go | 6 +++--- p2p/host/routed/routed_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/p2p/host/routed/routed.go b/p2p/host/routed/routed.go index a3e2d48142..8f00c25b75 100644 --- a/p2p/host/routed/routed.go +++ b/p2p/host/routed/routed.go @@ -113,9 +113,9 @@ func (rh *RoutedHost) Connect(ctx context.Context, pi peer.AddrInfo) error { // up-to-date addresses for the given peer. If there // are addresses we didn't know about previously, we // try to connect again. - newAddrs, addrErr := rh.findPeerAddrs(ctx, pi.ID) - if addrErr != nil { - return err // return outer error + newAddrs, err := rh.findPeerAddrs(ctx, pi.ID) + if err != nil { + return fmt.Errorf("failed to find peers: %w", err) } // Build lookup map diff --git a/p2p/host/routed/routed_test.go b/p2p/host/routed/routed_test.go index d4b9daea4b..ccae2fdb58 100644 --- a/p2p/host/routed/routed_test.go +++ b/p2p/host/routed/routed_test.go @@ -20,11 +20,11 @@ type mockRouting struct { } func (m *mockRouting) FindPeer(ctx context.Context, pid peer.ID) (peer.AddrInfo, error) { - m.callCount += 1 + m.callCount++ return m.findPeerFn(ctx, pid) } -func TestRoutedHost_Connect_obsoleteAddresses(t *testing.T) { +func TestRoutedHostConnectToObsoleteAddresses(t *testing.T) { ctx := context.Background() h1, err := basic.NewHost(swarmt.GenSwarm(t), nil)