use sha3 of NodeID for distance calculations; tests fail. ref #540 #495

p2p/discover/node.go

@@ -15,11 +15,22 @@ import (
 	"time"
 
 	"github.com/ethereum/go-ethereum/crypto/secp256k1"
+	"github.com/ethereum/go-ethereum/crypto/sha3"
 	"github.com/ethereum/go-ethereum/rlp"
 )
 
 const nodeIDBits = 512
 
+// nodeIDHash is the hash of a NodeID
+type nodeIDHash []byte
+
+// hashNodeID generates a nodeIDHash from a NodeID by hashing it using sha3-256
+func hashNodeID(id NodeID) nodeIDHash {
+	h := sha3.NewKeccak256()
+	h.Write(id[:])
+	return nodeIDHash(h.Sum(nil))
+}
+
 // Node represents a host on the network.
 type Node struct {
 	ID NodeID
@@ -29,6 +40,7 @@ type Node struct {
 	TCPPort  int // TCP listening port for RLPx
 
 	active time.Time
+	idHash []byte
 }
 
 func newNode(id NodeID, addr *net.UDPAddr) *Node {
@@ -38,6 +50,7 @@ func newNode(id NodeID, addr *net.UDPAddr) *Node {
 		DiscPort: addr.Port,
 		TCPPort:  addr.Port,
 		active:   time.Now(),
+		idHash:   hashNodeID(id),
 	}
 }
 
@@ -206,7 +219,7 @@ func recoverNodeID(hash, sig []byte) (id NodeID, err error) {
 // distcmp compares the distances a->target and b->target.
 // Returns -1 if a is closer to target, 1 if b is closer to target
 // and 0 if they are equal.
-func distcmp(target, a, b NodeID) int {
+func distcmp(target, a, b nodeIDHash) int {
 	for i := range target {
 		da := a[i] ^ target[i]
 		db := b[i] ^ target[i]
@@ -256,14 +269,14 @@ var lzcount = [256]int{
 }
 
 // logdist returns the logarithmic distance between a and b, log2(a ^ b).
-func logdist(a, b NodeID) int {
+func logdist(a, b nodeIDHash) int {
+
 	lz := 0
 	for i := range a {
 		x := a[i] ^ b[i]
-		if x == 0 {
-			lz += 8
-		} else {
 		lz += lzcount[x]
+
+		if x != 0 {
 			break
 		}
 	}

p2p/discover/node_test.go

@@ -151,13 +151,17 @@ func TestNodeID_distcmp(t *testing.T) {
 func TestNodeID_distcmpEqual(t *testing.T) {
 	base := NodeID{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}
 	x := NodeID{15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0}
-	if distcmp(base, x, x) != 0 {
+
-		t.Errorf("distcmp(base, x, x) != 0")
+	baseHash := hashNodeID(base)
+	xHash := hashNodeID(x)
+
+	if distcmp(baseHash, xHash, xHash) != 0 {
+		t.Errorf("distcmp(baseHash, xHash, xHash) != 0")
 	}
 }
 
 func TestNodeID_logdist(t *testing.T) {
-	logdistBig := func(a, b NodeID) int {
+	logdistBig := func(a, b nodeIDHash) int {
 		abig, bbig := new(big.Int).SetBytes(a[:]), new(big.Int).SetBytes(b[:])
 		return new(big.Int).Xor(abig, bbig).BitLen()
 	}
@@ -169,8 +173,9 @@ func TestNodeID_logdist(t *testing.T) {
 // the random tests is likely to miss the case where they're equal.
 func TestNodeID_logdistEqual(t *testing.T) {
 	x := NodeID{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}
-	if logdist(x, x) != 0 {
+	xHash := hashNodeID(x)
-		t.Errorf("logdist(x, x) != 0")
+	if logdist(xHash, xHash) != 0 {
+		t.Errorf("logdist(xHash, xHash) != 0")
 	}
 }
 
@@ -181,7 +186,11 @@ func TestNodeID_randomID(t *testing.T) {
 		a := gen(NodeID{}, quickrand).(NodeID)
 		dist := quickrand.Intn(len(NodeID{}) * 8)
 		result := randomID(a, dist)
-		actualdist := logdist(result, a)
+
+		resultHash := hashNodeID(result)
+		aHash := hashNodeID(a)
+
+		actualdist := logdist(resultHash, aHash)
 
 		if dist != actualdist {
 			t.Log("a:     ", a)

p2p/discover/table.go

@@ -86,7 +86,9 @@ func (tab *Table) Lookup(target NodeID) []*Node {
 		seen           = make(map[NodeID]bool)
 		reply          = make(chan []*Node, alpha)
 		pendingQueries = 0
+		targetHash     = hashNodeID(target)
 	)
+
 	// don't query further if we hit the target or ourself.
 	// unlikely to happen often in practice.
 	asked[target] = true
@@ -94,7 +96,7 @@ func (tab *Table) Lookup(target NodeID) []*Node {
 
 	tab.mutex.Lock()
 	// update last lookup stamp (for refresh logic)
-	tab.buckets[logdist(tab.self.ID, target)].lastLookup = time.Now()
+	tab.buckets[logdist(tab.self.idHash, targetHash)].lastLookup = time.Now()
 	// generate initial result set
 	result := tab.closest(target, bucketSize)
 	tab.mutex.Unlock()
@@ -160,7 +162,7 @@ func (tab *Table) closest(target NodeID, nresults int) *nodesByDistance {
 	// This is a very wasteful way to find the closest nodes but
 	// obviously correct. I believe that tree-based buckets would make
 	// this easier to implement efficiently.
-	close := &nodesByDistance{target: target}
+	close := &nodesByDistance{target: hashNodeID(target)}
 	for _, b := range tab.buckets {
 		for _, n := range b.entries {
 			close.push(n, nresults)
@@ -180,7 +182,7 @@ func (tab *Table) len() (n int) {
 // attempts to insert the node into a bucket. The returned Node might
 // not be part of the table. The caller must hold tab.mutex.
 func (tab *Table) bumpOrAdd(node NodeID, from *net.UDPAddr) (n *Node) {
-	b := tab.buckets[logdist(tab.self.ID, node)]
+	b := tab.buckets[logdist(tab.self.idHash, hashNodeID(node))]
 	if n = b.bump(node); n == nil {
 		n = newNode(node, from)
 		if len(b.entries) == bucketSize {
@@ -214,7 +216,7 @@ func (tab *Table) pingReplace(n *Node, b *bucket) {
 // bump updates the activity timestamp for the given node.
 // The caller must hold tab.mutex.
 func (tab *Table) bump(node NodeID) {
-	tab.buckets[logdist(tab.self.ID, node)].bump(node)
+	tab.buckets[logdist(tab.self.idHash, hashNodeID(node))].bump(node)
 }
 
 // add puts the entries into the table if their corresponding
@@ -227,7 +229,7 @@ outer:
 			// input lists.
 			continue
 		}
-		bucket := tab.buckets[logdist(tab.self.ID, n.ID)]
+		bucket := tab.buckets[logdist(tab.self.idHash, n.idHash)]
 		for i := range bucket.entries {
 			if bucket.entries[i].ID == n.ID {
 				// already in bucket
@@ -257,13 +259,13 @@ func (b *bucket) bump(id NodeID) *Node {
 // distance to target.
 type nodesByDistance struct {
 	entries []*Node
-	target  NodeID
+	target  nodeIDHash
 }
 
 // push adds the given node to the list, keeping the total size below maxElems.
 func (h *nodesByDistance) push(n *Node, maxElems int) {
 	ix := sort.Search(len(h.entries), func(i int) bool {
-		return distcmp(h.target, h.entries[i].ID, n.ID) > 0
+		return distcmp(h.target, h.entries[i].idHash, n.idHash) > 0
 	})
 	if len(h.entries) < maxElems {
 		h.entries = append(h.entries, n)

p2p/discover/table_test.go

@@ -140,7 +140,7 @@ func TestTable_closest(t *testing.T) {
 			t.Errorf("result contains duplicates")
 			return false
 		}
-		if !sortedByDistanceTo(test.Target, result) {
+		if !sortedByDistanceTo(test.TargetHash, result) {
 			t.Errorf("result is not sorted by distance to target")
 			return false
 		}
@@ -163,8 +163,8 @@ func TestTable_closest(t *testing.T) {
 				if contains(result, n.ID) {
 					continue // don't run the check below for nodes in result
 				}
-				farthestResult := result[len(result)-1].ID
+				farthestResult := result[len(result)-1].idHash
-				if distcmp(test.Target, n.ID, farthestResult) < 0 {
+				if distcmp(test.TargetHash, n.idHash, farthestResult) < 0 {
 					t.Errorf("table contains node that is closer to target but it's not in result")
 					t.Logf("  Target:          %v", test.Target)
 					t.Logf("  Farthest Result: %v", farthestResult)
@@ -183,6 +183,10 @@ func TestTable_closest(t *testing.T) {
 type closeTest struct {
 	Self   NodeID
 	Target NodeID
+
+	SelfHash   nodeIDHash
+	TargetHash nodeIDHash
+
 	All []*Node
 	N   int
 }
@@ -193,8 +197,12 @@ func (*closeTest) Generate(rand *rand.Rand, size int) reflect.Value {
 		Target: gen(NodeID{}, rand).(NodeID),
 		N:      rand.Intn(bucketSize),
 	}
+
+	t.SelfHash = hashNodeID(t.Self)
+	t.TargetHash = hashNodeID(t.Target)
+
 	for _, id := range gen([]NodeID{}, rand).([]NodeID) {
-		t.All = append(t.All, &Node{ID: id})
+		t.All = append(t.All, &Node{ID: id, idHash: hashNodeID(id)})
 	}
 	return reflect.ValueOf(t)
 }
@@ -202,6 +210,7 @@ func (*closeTest) Generate(rand *rand.Rand, size int) reflect.Value {
 func TestTable_Lookup(t *testing.T) {
 	self := gen(NodeID{}, quickrand).(NodeID)
 	target := randomID(self, 200)
+	targetHash := hashNodeID(target)
 	transport := findnodeOracle{t, target}
 	tab := newTable(transport, self, &net.UDPAddr{})
 
@@ -215,7 +224,7 @@ func TestTable_Lookup(t *testing.T) {
 	results := tab.Lookup(target)
 	t.Logf("results:")
 	for _, e := range results {
-		t.Logf("  ld=%d, %v", logdist(target, e.ID), e.ID)
+		t.Logf("  ld=%d, %v", logdist(targetHash, e.idHash), e.ID)
 	}
 	if len(results) != bucketSize {
 		t.Errorf("wrong number of results: got %d, want %d", len(results), bucketSize)
@@ -223,7 +232,7 @@ func TestTable_Lookup(t *testing.T) {
 	if hasDuplicates(results) {
 		t.Errorf("result set contains duplicate entries")
 	}
-	if !sortedByDistanceTo(target, results) {
+	if !sortedByDistanceTo(targetHash, results) {
 		t.Errorf("result set not sorted by distance to target")
 	}
 	if !contains(results, target) {
@@ -272,13 +281,13 @@ func hasDuplicates(slice []*Node) bool {
 	return false
 }
 
-func sortedByDistanceTo(distbase NodeID, slice []*Node) bool {
+func sortedByDistanceTo(distbase nodeIDHash, slice []*Node) bool {
-	var last NodeID
+	var last nodeIDHash
 	for i, e := range slice {
-		if i > 0 && distcmp(distbase, e.ID, last) < 0 {
+		if i > 0 && distcmp(distbase, e.idHash, last) < 0 {
 			return false
 		}
-		last = e.ID
+		last = e.idHash
 	}
 	return true
 }

p2p/discover/udp_test.go

@@ -57,7 +57,7 @@ func TestUDP_findnode(t *testing.T) {
 	// matter much, altough we need to take care not to overflow
 	// any bucket.
 	target := randomID(n1.self.ID, 100)
-	nodes := &nodesByDistance{target: target}
+	nodes := &nodesByDistance{target: hashNodeID(target)}
 	for i := 0; i < bucketSize; i++ {
 		n2.add([]*Node{&Node{
 			IP:       net.IP{1, 2, 3, byte(i)},