Remove newID function (#468)

* Removes node.NewID usage from kademlia tests * Updates more tests * remove comments * removes NewID method from overlay tests * adds error checking to NewFullIdentity * removes /x/tools from go.mod * Removes commented out code
2018-10-12 15:10:50 -06:00 · 2018-10-12 15:10:50 -06:00 · 7849d7fa63
commit 7849d7fa63
parent 3e1b16ea99
5 changed files with 33 additions and 54 deletions
--- a/go.sum
+++ b/go.sum
@ -319,6 +319,8 @@ golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2 h1:+DCIGbF/swA92ohVg0//6X2I
 golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/tools v0.0.0-20180828015842-6cd1fcedba52 h1:JG/0uqcGdTNgq7FdU+61l5Pdmb8putNZlXb65bJBROs=
 golang.org/x/tools v0.0.0-20180828015842-6cd1fcedba52/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
+golang.org/x/tools v0.0.0-20181011204843-157aeed469d4 h1:JjbNUNCpxj+qeUAIYZil7iM5eQjlzRbNJ7ganQkuUo4=
+golang.org/x/tools v0.0.0-20181011204843-157aeed469d4/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 google.golang.org/appengine v1.1.0 h1:igQkv0AAhEIvTEpD5LIpAfav2eeVO9HBTjvKHVJPRSs=
 google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
 google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8 h1:Nw54tB0rB7hY/N0NQvRW8DG4Yk3Q6T9cu9RcFQDu1tc=
--- a/pkg/node/id.go
+++ b/pkg/node/id.go
@ -5,9 +5,6 @@ package node

 import (
 	"context"
-	"crypto/rand"
-
-	base58 "github.com/jbenet/go-base58"

 	"storj.io/storj/pkg/provider"
 )
@ -43,23 +40,3 @@ func IDFromString(s string) *ID {
 	n := ID(s)
 	return &n
 }
-
-// NewID is a deprecated function to generate Kademlia ID's for tests. IT IS BEING REMOVED.
-// Use NewFullIdentity instead in the meantime.
-func NewID() (*ID, error) {
-	b, err := newID()
-	if err != nil {
-		return nil, err
-	}
-
-	bb := ID(base58.Encode(b))
-	return &bb, nil
-}
-
-// newID generates a new random ID.
-// This purely to get things working. We shouldn't use this as the ID in the actual network
-func newID() ([]byte, error) {
-	result := make([]byte, 20)
-	_, err := rand.Read(result)
-	return result, err
-}
--- a/pkg/overlay/cache_test.go
+++ b/pkg/overlay/cache_test.go
@ -58,15 +58,10 @@ func kadconfig() kademlia.KadConfig {

 func newTestKademlia(t *testing.T, ip, port string, d dht.DHT, b pb.Node) *kademlia.Kademlia {
 	kc := kadconfig()
-	i, err := node.NewID()
+	fid, err := node.NewFullIdentity(ctx, 12, 4)
 	assert.NoError(t, err)
-	id := *i
 	n := []pb.Node{b}
-	ca, err := provider.NewCA(ctx, 12, 4)
-	assert.NoError(t, err)
-	identity, err := ca.NewIdentity()
-	assert.NoError(t, err)
-	kad, err := kademlia.NewKademlia(&id, n, fmt.Sprintf("%s:%s", ip, port), identity, "db", kc)
+	kad, err := kademlia.NewKademlia(fid.ID, n, fmt.Sprintf("%s:%s", ip, port), fid, "db", kc)
 	assert.NoError(t, err)

 	return kad
@ -74,10 +69,9 @@ func newTestKademlia(t *testing.T, ip, port string, d dht.DHT, b pb.Node) *kadem

 func bootstrapTestNetwork(t *testing.T, ip, port string) ([]dht.DHT, pb.Node) {
 	kc := kadconfig()
-	bid, err := node.NewID()
+	bid, err := node.NewFullIdentity(ctx, 12, 4)
 	assert.NoError(t, err)

-	bnid := *bid
 	dhts := []dht.DHT{}

 	p, err := strconv.Atoi(port)
@ -91,7 +85,7 @@ func bootstrapTestNetwork(t *testing.T, ip, port string) ([]dht.DHT, pb.Node) {
 	identity, err := ca.NewIdentity()
 	assert.NoError(t, err)

-	boot, err := kademlia.NewKademlia(&bnid, []pb.Node{*intro}, fmt.Sprintf("%s:%s", ip, pm), identity, "db", kc)
+	boot, err := kademlia.NewKademlia(bid.ID, []pb.Node{*intro}, fmt.Sprintf("%s:%s", ip, pm), identity, "db", kc)

 	assert.NoError(t, err)
 	rt, err := boot.GetRoutingTable(context.Background())
@ -108,16 +102,10 @@ func bootstrapTestNetwork(t *testing.T, ip, port string) ([]dht.DHT, pb.Node) {
 		gg := strconv.Itoa(p)
 		kc := kadconfig()

-		nid, err := node.NewID()
-		assert.NoError(t, err)
-		id := *nid
-
-		ca, err := provider.NewCA(ctx, 12, 4)
-		assert.NoError(t, err)
-		identity, err := ca.NewIdentity()
+		fid, err := node.NewFullIdentity(ctx, 12, 4)
 		assert.NoError(t, err)

-		dht, err := kademlia.NewKademlia(&id, []pb.Node{bootNode}, fmt.Sprintf("%s:%s", ip, gg), identity, "db", kc)
+		dht, err := kademlia.NewKademlia(fid.ID, []pb.Node{bootNode}, fmt.Sprintf("%s:%s", ip, gg), fid, "db", kc)
 		assert.NoError(t, err)

 		p++
--- a/pkg/overlay/overlay_test.go
+++ b/pkg/overlay/overlay_test.go
@ -21,17 +21,17 @@ func TestFindStorageNodes(t *testing.T) {
 	lis, err := net.Listen("tcp", "127.0.0.1:0")
 	assert.NoError(t, err)

-	id, err := node.NewID()
+	fid, err := node.NewFullIdentity(ctx, 12, 4)
 	assert.NoError(t, err)
-	id2, err := node.NewID()
+	fid2, err := node.NewFullIdentity(ctx, 12, 4)
 	assert.NoError(t, err)

 	srv := NewMockServer([]storage.ListItem{
 		{
-			Key:   storage.Key(id.String()),
+			Key:   storage.Key(fid.ID.String()),
 			Value: test.NewNodeStorageValue(t, "127.0.0.1:9090"),
 		}, {
-			Key:   storage.Key(id2.String()),
+			Key:   storage.Key(fid2.ID.String()),
 			Value: test.NewNodeStorageValue(t, "127.0.0.1:9090"),
 		},
 	})
@ -55,13 +55,13 @@ func TestOverlayLookup(t *testing.T) {
 	lis, err := net.Listen("tcp", "127.0.0.1:0")
 	assert.NoError(t, err)

-	id, err := node.NewID()
+	fid, err := node.NewFullIdentity(ctx, 12, 4)

 	assert.NoError(t, err)

 	srv := NewMockServer([]storage.ListItem{
 		{
-			Key:   storage.Key(id.String()),
+			Key:   storage.Key(fid.ID.String()),
 			Value: test.NewNodeStorageValue(t, "127.0.0.1:9090"),
 		},
 	})
@ -72,7 +72,7 @@ func TestOverlayLookup(t *testing.T) {
 	c, err := NewClient(address, grpc.WithInsecure())
 	assert.NoError(t, err)

-	r, err := c.Lookup(context.Background(), &pb.LookupRequest{NodeID: id.String()})
+	r, err := c.Lookup(context.Background(), &pb.LookupRequest{NodeID: fid.ID.String()})
 	assert.NoError(t, err)
 	assert.NotNil(t, r)
 }
@ -81,14 +81,14 @@ func TestOverlayBulkLookup(t *testing.T) {
 	lis, err := net.Listen("tcp", "127.0.0.1:0")
 	assert.NoError(t, err)

-	id, err := node.NewID()
+	fid, err := node.NewFullIdentity(ctx, 12, 4)
 	assert.NoError(t, err)
-	id2, err := node.NewID()
+	fid2, err := node.NewFullIdentity(ctx, 12, 4)
 	assert.NoError(t, err)

 	srv := NewMockServer([]storage.ListItem{
 		{
-			Key:   storage.Key(id.String()),
+			Key:   storage.Key(fid.ID.String()),
 			Value: test.NewNodeStorageValue(t, "127.0.0.1:9090"),
 		},
 	})
@ -99,8 +99,8 @@ func TestOverlayBulkLookup(t *testing.T) {
 	c, err := NewClient(address, grpc.WithInsecure())
 	assert.NoError(t, err)

-	req1 := &pb.LookupRequest{NodeID: id.String()}
-	req2 := &pb.LookupRequest{NodeID: id2.String()}
+	req1 := &pb.LookupRequest{NodeID: fid.ID.String()}
+	req2 := &pb.LookupRequest{NodeID: fid2.ID.String()}
 	rs := &pb.LookupRequests{Lookuprequest: []*pb.LookupRequest{req1, req2}}
 	r, err := c.BulkLookup(context.Background(), rs)
 	assert.NoError(t, err)
--- a/pkg/piecestore/rpc/client/pieceid_test.go
+++ b/pkg/piecestore/rpc/client/pieceid_test.go
@ -4,11 +4,14 @@
 package client

 import (
+	"context"
 	"testing"

 	"github.com/mr-tron/base58/base58"
 	"github.com/stretchr/testify/assert"
+	"storj.io/storj/pkg/dht"
 	"storj.io/storj/pkg/node"
+	"storj.io/storj/pkg/provider"
 )

 func TestNewPieceID(t *testing.T) {
@ -26,8 +29,9 @@ func TestNewPieceID(t *testing.T) {

 func TestDerivePieceID(t *testing.T) {
 	pid := NewPieceID()
-	nid, err := node.NewID()
+	fid, err := newTestIdentity()
 	assert.NoError(t, err)
+	nid := dht.NodeID(fid.ID)

 	did, err := pid.Derive(nid.Bytes())
 	assert.NoError(t, err)
@ -40,3 +44,11 @@ func TestDerivePieceID(t *testing.T) {
 	_, err = base58.Decode(did.String())
 	assert.NoError(t, err)
 }
+
+// helper function to generate new node identities with
+// correct difficulty and concurrency
+func newTestIdentity() (*provider.FullIdentity, error) {
+	ctx := context.Background()
+	fid, err := node.NewFullIdentity(ctx, 12, 4)
+	return fid, err
+}