From ebdfedf24a77b820fbdacd997fda9a9a0f5cf85b Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Tue, 1 Nov 2022 13:26:43 -0500 Subject: [PATCH 1/3] Allow distance of 256 in lookupDistances --- p2p/discover/v4wire/v4wire.go | 2 +- p2p/discover/v5_udp.go | 2 +- p2p/discover/v5_udp_test.go | 7 +++++++ p2p/discover/v5wire/encoding.go | 4 ++-- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/p2p/discover/v4wire/v4wire.go b/p2p/discover/v4wire/v4wire.go index b07a6e341c31..5b0dc921a666 100644 --- a/p2p/discover/v4wire/v4wire.go +++ b/p2p/discover/v4wire/v4wire.go @@ -59,7 +59,7 @@ type ( // Pong is the reply to ping. Pong struct { // This field should mirror the UDP envelope address - // of the ping packet, which provides a way to discover the + // of the ping packet, which provides a way to discover // the external address (after NAT). To Endpoint ReplyTok []byte // This contains the hash of the ping packet. diff --git a/p2p/discover/v5_udp.go b/p2p/discover/v5_udp.go index 071ed65adc7f..0e51c3792bb1 100644 --- a/p2p/discover/v5_udp.go +++ b/p2p/discover/v5_udp.go @@ -323,7 +323,7 @@ func lookupDistances(target, dest enode.ID) (dists []uint) { td := enode.LogDist(target, dest) dists = append(dists, uint(td)) for i := 1; len(dists) < lookupRequestLimit; i++ { - if td+i < 256 { + if td+i <= 256 { dists = append(dists, uint(td+i)) } if td-i > 0 { diff --git a/p2p/discover/v5_udp_test.go b/p2p/discover/v5_udp_test.go index 30d610a4dd8c..99c5bf395b70 100644 --- a/p2p/discover/v5_udp_test.go +++ b/p2p/discover/v5_udp_test.go @@ -524,6 +524,13 @@ func TestUDPv5_lookup(t *testing.T) { t.Parallel() test := newUDPV5Test(t) + // Ensure lookupDistances includes 256 when necessary. + node256 := nodesAtDistance(test.table.self().ID(), 255, 1)[0] + dists := lookupDistances(test.table.self().ID(), node256.ID()) + if !containsUint(256, dists) { + t.Errorf("lookup distances does not contain 256: %v", dists) + } + // Lookup on empty table returns no nodes. if results := test.udp.Lookup(lookupTestnet.target.id()); len(results) > 0 { t.Fatalf("lookup on empty table returned %d results: %#v", len(results), results) diff --git a/p2p/discover/v5wire/encoding.go b/p2p/discover/v5wire/encoding.go index 45f2f0883bad..85f077bf23de 100644 --- a/p2p/discover/v5wire/encoding.go +++ b/p2p/discover/v5wire/encoding.go @@ -65,7 +65,7 @@ type ( handshakeAuthData struct { h struct { SrcID enode.ID - SigSize byte // ignature data + SigSize byte // signature data PubkeySize byte // offset of } // Trailing variable-size data. @@ -524,7 +524,7 @@ func (c *Codec) decodeHandshake(fromAddr string, head *Header) (n *enode.Node, a if err != nil { return nil, auth, nil, errInvalidAuthKey } - // Derive sesssion keys. + // Derive session keys. session := deriveKeys(sha256.New, c.privkey, ephkey, auth.h.SrcID, c.localnode.ID(), cdata) session = session.keysFlipped() return n, auth, session, nil From 84cdc350894da4dc23924afe47509d7afc6abf0b Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Tue, 1 Nov 2022 16:13:33 -0500 Subject: [PATCH 2/3] Rename variable --- p2p/discover/v5_udp_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/p2p/discover/v5_udp_test.go b/p2p/discover/v5_udp_test.go index 99c5bf395b70..bab1771e1202 100644 --- a/p2p/discover/v5_udp_test.go +++ b/p2p/discover/v5_udp_test.go @@ -525,8 +525,8 @@ func TestUDPv5_lookup(t *testing.T) { test := newUDPV5Test(t) // Ensure lookupDistances includes 256 when necessary. - node256 := nodesAtDistance(test.table.self().ID(), 255, 1)[0] - dists := lookupDistances(test.table.self().ID(), node256.ID()) + node255 := nodesAtDistance(test.table.self().ID(), 255, 1)[0] + dists := lookupDistances(test.table.self().ID(), node255.ID()) if !containsUint(256, dists) { t.Errorf("lookup distances does not contain 256: %v", dists) } From 3f7f59fdf0cbb7e77583eb9af948320472bd3d69 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 2 Nov 2022 08:22:05 -0500 Subject: [PATCH 3/3] Make lookupDistances test func with more tests --- p2p/discover/v5_udp_test.go | 44 +++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/p2p/discover/v5_udp_test.go b/p2p/discover/v5_udp_test.go index bab1771e1202..ca63688afa13 100644 --- a/p2p/discover/v5_udp_test.go +++ b/p2p/discover/v5_udp_test.go @@ -34,6 +34,7 @@ import ( "github.com/ethereum/go-ethereum/p2p/enode" "github.com/ethereum/go-ethereum/p2p/enr" "github.com/ethereum/go-ethereum/rlp" + "github.com/stretchr/testify/require" ) // Real sockets, real crypto: this test checks end-to-end connectivity for UDPv5. @@ -519,18 +520,47 @@ func TestUDPv5_talkRequest(t *testing.T) { } } +// This test checks that lookupDistances works. +func TestUDPv5_lookupDistances(t *testing.T) { + test := newUDPV5Test(t) + lnID := test.table.self().ID() + + t.Run("target distance of 1", func(t *testing.T) { + node := nodeAtDistance(lnID, 1, intIP(0)) + dists := lookupDistances(lnID, node.ID()) + require.Equal(t, []uint{1, 2, 3}, dists) + }) + + t.Run("target distance of 2", func(t *testing.T) { + node := nodeAtDistance(lnID, 2, intIP(0)) + dists := lookupDistances(lnID, node.ID()) + require.Equal(t, []uint{2, 3, 1}, dists) + }) + + t.Run("target distance of 128", func(t *testing.T) { + node := nodeAtDistance(lnID, 128, intIP(0)) + dists := lookupDistances(lnID, node.ID()) + require.Equal(t, []uint{128, 129, 127}, dists) + }) + + t.Run("target distance of 255", func(t *testing.T) { + node := nodeAtDistance(lnID, 255, intIP(0)) + dists := lookupDistances(lnID, node.ID()) + require.Equal(t, []uint{255, 256, 254}, dists) + }) + + t.Run("target distance of 256", func(t *testing.T) { + node := nodeAtDistance(lnID, 256, intIP(0)) + dists := lookupDistances(lnID, node.ID()) + require.Equal(t, []uint{256, 255, 254}, dists) + }) +} + // This test checks that lookup works. func TestUDPv5_lookup(t *testing.T) { t.Parallel() test := newUDPV5Test(t) - // Ensure lookupDistances includes 256 when necessary. - node255 := nodesAtDistance(test.table.self().ID(), 255, 1)[0] - dists := lookupDistances(test.table.self().ID(), node255.ID()) - if !containsUint(256, dists) { - t.Errorf("lookup distances does not contain 256: %v", dists) - } - // Lookup on empty table returns no nodes. if results := test.udp.Lookup(lookupTestnet.target.id()); len(results) > 0 { t.Fatalf("lookup on empty table returned %d results: %#v", len(results), results)