Improve test coverage

v2/backend_provider.go

@@ -6,7 +6,6 @@ import (
 	"encoding/binary"
 	"fmt"
 	"io"
-	"path"
 	"strings"
 	"sync"
 	"time"
@@ -469,7 +468,8 @@ func newDatastoreKey(namespace string, binStrs ...string) ds.Key {
 	for i, bin := range binStrs {
 		elems[i+1] = base32.RawStdEncoding.EncodeToString([]byte(bin))
 	}
-	return ds.NewKey("/" + path.Join(elems...))
+
+	return ds.NewKey("/" + strings.Join(elems, "/"))
 }
 
 // newRoutingKey uses the given namespace and binary string key and constructs

v2/config.go

@@ -269,6 +269,15 @@ func (c *Config) Validate() error {
 		}
 	}
 
+	for _, bp := range c.BootstrapPeers {
+		if len(bp.Addrs) == 0 {
+			return &ConfigurationError{
+				Component: "Config",
+				Err:       fmt.Errorf("bootstrap peer with no address"),
+			}
+		}
+	}
+
 	if c.ProtocolID == "" {
 		return &ConfigurationError{
 			Component: "Config",

v2/config_test.go

@@ -5,7 +5,9 @@ import (
 	"time"
 
 	"github.com/libp2p/go-libp2p/core/peer"
+	ma "github.com/multiformats/go-multiaddr"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestConfig_Validate(t *testing.T) {
@@ -140,6 +142,25 @@ func TestConfig_Validate(t *testing.T) {
 		cfg.BootstrapPeers = []peer.AddrInfo{}
 		assert.Error(t, cfg.Validate())
 	})
+
+	t.Run("bootstrap peers no addresses", func(t *testing.T) {
+		cfg := DefaultConfig()
+		cfg.BootstrapPeers = []peer.AddrInfo{
+			{ID: newPeerID(t), Addrs: []ma.Multiaddr{}},
+		}
+		assert.Error(t, cfg.Validate())
+	})
+
+	t.Run("bootstrap peers mixed no addresses", func(t *testing.T) {
+		cfg := DefaultConfig()
+		maddr, err := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/1234")
+		require.NoError(t, err)
+		cfg.BootstrapPeers = []peer.AddrInfo{
+			{ID: newPeerID(t), Addrs: []ma.Multiaddr{}},
+			{ID: newPeerID(t), Addrs: []ma.Multiaddr{maddr}},
+		}
+		assert.Error(t, cfg.Validate()) // still an error
+	})
 }
 
 func TestQueryConfig_Validate(t *testing.T) {

v2/routing.go

@@ -40,11 +40,9 @@ func (d *DHT) FindPeer(ctx context.Context, id peer.ID) (peer.AddrInfo, error) {
 			return addrInfo, nil
 		}
 	default:
-		// we're
+		// we're not connected or were recently connected
 	}
 
-	target := kadt.PeerID(id)
-
 	var foundPeer peer.ID
 	fn := func(ctx context.Context, visited kadt.PeerID, msg *pb.Message, stats coordt.QueryStats) error {
 		if peer.ID(visited) == id {
@@ -54,7 +52,7 @@ func (d *DHT) FindPeer(ctx context.Context, id peer.ID) (peer.AddrInfo, error) {
 		return nil
 	}
 
-	_, _, err := d.kad.QueryClosest(ctx, target.Key(), fn, 20)
+	_, _, err := d.kad.QueryClosest(ctx, kadt.PeerID(id).Key(), fn, 20)
 	if err != nil {
 		return peer.AddrInfo{}, fmt.Errorf("failed to run query: %w", err)
 	}
@@ -272,7 +270,7 @@ func (d *DHT) putValueLocal(ctx context.Context, key string, value []byte) error
 	}
 
 	rec := record.MakePutRecord(key, value)
-	rec.TimeReceived = time.Now().UTC().Format(time.RFC3339Nano)
+	rec.TimeReceived = d.cfg.Clock.Now().UTC().Format(time.RFC3339Nano)
 
 	_, err = b.Store(ctx, path, rec)
 	if err != nil {
@@ -395,7 +393,6 @@ func (d *DHT) searchValueRoutine(ctx context.Context, backend Backend, ns string
 		}
 
 		if !bytes.Equal(routingKey, rec.GetKey()) {
-			d.log.Debug("record key mismatch")
 			return nil
 		}
 

v2/routing_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/stretchr/testify/suite"
 
 	"github.com/libp2p/go-libp2p-kad-dht/v2/internal/kadtest"
+	"github.com/libp2p/go-libp2p-kad-dht/v2/kadt"
 )
 
 // newRandomContent reads 1024 bytes from crypto/rand and builds a content struct.
@@ -50,6 +51,87 @@ func makePkKeyValue(t testing.TB) (string, []byte) {
 	return routing.KeyForPublicKey(id), v
 }
 
+func TestDHT_FindPeer_happy_path(t *testing.T) {
+	ctx := kadtest.CtxShort(t)
+
+	top := NewTopology(t)
+	d1 := top.AddServer(nil)
+	d2 := top.AddServer(nil)
+	d3 := top.AddServer(nil)
+	d4 := top.AddServer(nil)
+	top.ConnectChain(ctx, d1, d2, d3, d4)
+
+	addrInfo, err := d1.FindPeer(ctx, d4.host.ID())
+	require.NoError(t, err)
+	assert.Equal(t, d4.host.ID(), addrInfo.ID)
+}
+
+func TestDHT_FindPeer_not_found(t *testing.T) {
+	ctx := kadtest.CtxShort(t)
+
+	top := NewTopology(t)
+	d1 := top.AddServer(nil)
+	d2 := top.AddServer(nil)
+	d3 := top.AddServer(nil)
+	d4 := top.AddServer(nil)
+	top.ConnectChain(ctx, d1, d2, d3)
+
+	_, err := d1.FindPeer(ctx, d4.host.ID())
+	assert.Error(t, err)
+}
+
+func TestDHT_FindPeer_already_connected(t *testing.T) {
+	ctx := kadtest.CtxShort(t)
+
+	top := NewTopology(t)
+	d1 := top.AddServer(nil)
+	d2 := top.AddServer(nil)
+	d3 := top.AddServer(nil)
+	d4 := top.AddServer(nil)
+	top.ConnectChain(ctx, d1, d2, d3)
+
+	err := d1.host.Connect(ctx, peer.AddrInfo{
+		ID:    d4.host.ID(),
+		Addrs: d4.host.Addrs(),
+	})
+	require.NoError(t, err)
+
+	_, err = d1.FindPeer(ctx, d4.host.ID())
+	assert.NoError(t, err)
+}
+
+func TestDHT_PutValue_happy_path(t *testing.T) {
+	// TIMING: this test is based on timeouts - so might become flaky!
+	ctx := kadtest.CtxShort(t)
+
+	top := NewTopology(t)
+	d1 := top.AddServer(nil)
+	d2 := top.AddServer(nil)
+
+	top.ConnectChain(ctx, d1, d2)
+
+	k, v := makePkKeyValue(t)
+
+	err := d1.PutValue(ctx, k, v)
+	require.NoError(t, err)
+
+	deadline, hasDeadline := ctx.Deadline()
+	if !hasDeadline {
+		deadline = time.Now().Add(5 * time.Second)
+	}
+
+	// putting data to a remote peer is an asynchronous operation. Even after
+	// PutValue returns, and although we have closed the stream on our end, an
+	// acknowledgement that the other peer has received the data is not
+	// guaranteed. The data will be flushed at this point, but the remote might
+	// not have handled it yet. Therefore, we use "EventuallyWithT" here.
+	assert.EventuallyWithT(t, func(t *assert.CollectT) {
+		val, err := d2.GetValue(ctx, k, routing.Offline)
+		assert.NoError(t, err)
+		assert.Equal(t, v, val)
+	}, time.Until(deadline), 10*time.Millisecond)
+}
+
 func TestDHT_PutValue_local_only(t *testing.T) {
 	ctx := kadtest.CtxShort(t)
 
@@ -81,6 +163,18 @@ func TestDHT_PutValue_invalid_key(t *testing.T) {
 	})
 }
 
+func TestDHT_PutValue_routing_option_returns_error(t *testing.T) {
+	ctx := kadtest.CtxShort(t)
+	d := newTestDHT(t)
+
+	errOption := func(opts *routing.Options) error {
+		return fmt.Errorf("some error")
+	}
+
+	err := d.PutValue(ctx, "/ipns/some-key", []byte("some value"), errOption)
+	assert.ErrorContains(t, err, "routing options")
+}
+
 func TestGetValueOnePeer(t *testing.T) {
 	ctx := kadtest.CtxShort(t)
 	top := NewTopology(t)
@@ -833,3 +927,40 @@ func TestDHT_SearchValue_offline_not_found_locally(t *testing.T) {
 	assert.ErrorIs(t, err, routing.ErrNotFound)
 	assert.Nil(t, valChan)
 }
+
+func TestDHT_Bootstrap_no_peers_configured(t *testing.T) {
+	// TIMING: this test is based on timeouts - so might become flaky!
+	ctx := kadtest.CtxShort(t)
+
+	top := NewTopology(t)
+	d1 := top.AddServer(nil)
+	d2 := top.AddServer(nil)
+	d3 := top.AddServer(nil)
+
+	d1.cfg.BootstrapPeers = []peer.AddrInfo{
+		{ID: d2.host.ID(), Addrs: d2.host.Addrs()},
+		{ID: d3.host.ID(), Addrs: d3.host.Addrs()},
+	}
+
+	err := d1.Bootstrap(ctx)
+	assert.NoError(t, err)
+
+	deadline, hasDeadline := ctx.Deadline()
+	if !hasDeadline {
+		deadline = time.Now().Add(5 * time.Second)
+	}
+
+	// bootstrapping is an asynchronous process, so we periodically check
+	// if the peers have each other in their routing tables
+	assert.EventuallyWithT(t, func(collect *assert.CollectT) {
+		_, found := d1.rt.GetNode(kadt.PeerID(d2.host.ID()).Key())
+		assert.True(collect, found)
+		_, found = d1.rt.GetNode(kadt.PeerID(d3.host.ID()).Key())
+		assert.True(collect, found)
+
+		_, found = d2.rt.GetNode(kadt.PeerID(d1.host.ID()).Key())
+		assert.True(collect, found)
+		_, found = d3.rt.GetNode(kadt.PeerID(d1.host.ID()).Key())
+		assert.True(collect, found)
+	}, time.Until(deadline), 10*time.Millisecond)
+}

v2/tele/tele.go

@@ -27,7 +27,7 @@ func NoopTracer() trace.Tracer {
 	return trace.NewNoopTracerProvider().Tracer("")
 }
 
-// NoopMeterProvider returns a meter provider that does not record or emit metrics.
+// NoopMeter returns a meter provider that does not record or emit metrics.
 func NoopMeter() metric.Meter {
 	return noop.NewMeterProvider().Meter("")
 }