Skip to content

Commit

Permalink
p2p: speed-up TestUDPv4_LookupIterator
Browse files Browse the repository at this point in the history
The test was slow, because it was trying to find
predefined nodeIDs (lookupTestnet) by generating random keys
and trying to find their neighbours
until it hits all nodes of the lookupTestnet.
In addition each FindNode response was waited for 0.5 sec (respTimeout).
This could take up to 30 sec and fail the test suite.

A fake random key generator is now used during the test.
It issues the expected keys, and the lookup converges quickly.
The reply timeout is reduced for the test.
Now it normally takes less than.1 sec.
  • Loading branch information
battlmonstr committed Apr 27, 2022
1 parent c9b26c2 commit f2c5d04
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 9 deletions.
11 changes: 11 additions & 0 deletions p2p/discover/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ package discover
import (
"context"
"crypto/ecdsa"
"github.com/ledgerwatch/erigon/crypto"
"net"
"time"

"github.com/ledgerwatch/erigon/common/mclock"
"github.com/ledgerwatch/erigon/p2p/enode"
Expand Down Expand Up @@ -48,6 +50,9 @@ type Config struct {
Log log.Logger // if set, log messages go here
ValidSchemes enr.IdentityScheme // allowed identity schemes
Clock mclock.Clock
ReplyTimeout time.Duration

PrivateKeyGenerator func() (*ecdsa.PrivateKey, error)
}

func (cfg Config) withDefaults() Config {
Expand All @@ -60,6 +65,12 @@ func (cfg Config) withDefaults() Config {
if cfg.Clock == nil {
cfg.Clock = mclock.System{}
}
if cfg.ReplyTimeout == 0 {
cfg.ReplyTimeout = respTimeout
}
if cfg.PrivateKeyGenerator == nil {
cfg.PrivateKeyGenerator = crypto.GenerateKey
}
return cfg
}

Expand Down
24 changes: 23 additions & 1 deletion p2p/discover/v4_lookup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package discover

import (
"context"
"crypto/ecdsa"
"fmt"
"net"
Expand Down Expand Up @@ -74,7 +75,18 @@ func TestUDPv4_LookupIterator(t *testing.T) {
t.Skip("fix me on win please")
}
t.Parallel()
test := newUDPTest(t)

// Set up RandomNodes() to use expected keys instead of generating random ones.
testNetPrivateKeys := lookupTestnet.privateKeys()
testNetPrivateKeyIndex := -1
privateKeyGenerator := func() (*ecdsa.PrivateKey, error) {
testNetPrivateKeyIndex = (testNetPrivateKeyIndex + 1) % len(testNetPrivateKeys)
return testNetPrivateKeys[testNetPrivateKeyIndex], nil
}
ctx := context.Background()
ctx = contextWithPrivateKeyGenerator(ctx, privateKeyGenerator)

test := newUDPTestContext(ctx, t)
defer test.close()

// Seed table with initial nodes.
Expand Down Expand Up @@ -316,6 +328,16 @@ func (tn *preminedTestnet) closest(n int) (nodes []*enode.Node) {
return nodes[:n]
}

func (tn *preminedTestnet) privateKeys() []*ecdsa.PrivateKey {
var keys []*ecdsa.PrivateKey
for d := range tn.dists {
for _, key := range tn.dists[d] {
keys = append(keys, key)
}
}
return keys
}

var _ = (*preminedTestnet).mine // avoid linter warning about mine being dead code.

// mine generates a testnet struct literal with nodes at
Expand Down
14 changes: 10 additions & 4 deletions p2p/discover/v4_udp.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,11 @@ type UDPv4 struct {

addReplyMatcher chan *replyMatcher
gotreply chan reply
replyTimeout time.Duration
closeCtx context.Context
cancelCloseCtx context.CancelFunc

privateKeyGenerator func() (*ecdsa.PrivateKey, error)
}

// replyMatcher represents a pending reply.
Expand Down Expand Up @@ -137,9 +140,12 @@ func ListenV4(ctx context.Context, c UDPConn, ln *enode.LocalNode, cfg Config) (
db: ln.Database(),
gotreply: make(chan reply),
addReplyMatcher: make(chan *replyMatcher),
replyTimeout: cfg.ReplyTimeout,
closeCtx: closeCtx,
cancelCloseCtx: cancel,
log: cfg.Log,

privateKeyGenerator: cfg.PrivateKeyGenerator,
}

tab, err := newTable(t, ln.Database(), cfg.Bootnodes, t.log)
Expand Down Expand Up @@ -282,7 +288,7 @@ func (t *UDPv4) lookupSelf() []*enode.Node {
}

func (t *UDPv4) newRandomLookup(ctx context.Context) *lookup {
key, err := crypto.GenerateKey()
key, err := t.privateKeyGenerator()
if err != nil {
t.log.Warn("Failed to generate a random node key for newRandomLookup", "err", err)
key = t.priv
Expand Down Expand Up @@ -447,7 +453,7 @@ func (t *UDPv4) loop() {
now := time.Now()
for el := plist.Front(); el != nil; el = el.Next() {
nextTimeout = el.Value.(*replyMatcher)
if dist := nextTimeout.deadline.Sub(now); dist < 2*respTimeout {
if dist := nextTimeout.deadline.Sub(now); dist < 2*t.replyTimeout {
timeout.Reset(dist)
return
}
Expand All @@ -472,7 +478,7 @@ func (t *UDPv4) loop() {
return

case p := <-t.addReplyMatcher:
p.deadline = time.Now().Add(respTimeout)
p.deadline = time.Now().Add(t.replyTimeout)
plist.PushBack(p)

case r := <-t.gotreply:
Expand Down Expand Up @@ -595,7 +601,7 @@ func (t *UDPv4) ensureBond(toid enode.ID, toaddr *net.UDPAddr) {
rm := t.sendPing(toid, toaddr, nil)
<-rm.errc
// Wait for them to ping back and process our pong.
time.Sleep(respTimeout)
time.Sleep(t.replyTimeout)
}
}

Expand Down
42 changes: 38 additions & 4 deletions p2p/discover/v4_udp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,17 @@ type udpTest struct {
}

func newUDPTest(t *testing.T) *udpTest {
return newUDPTestContext(context.Background(), t)
}

func newUDPTestContext(ctx context.Context, t *testing.T) *udpTest {
ctx = disableLookupSlowdown(ctx)

replyTimeout := contextGetReplyTimeout(ctx)
if replyTimeout == 0 {
replyTimeout = 50 * time.Millisecond
}

test := &udpTest{
t: t,
pipe: newpipe(),
Expand All @@ -75,11 +86,13 @@ func newUDPTest(t *testing.T) *udpTest {
panic(err)
}
ln := enode.NewLocalNode(test.db, test.localkey)
ctx := context.Background()
ctx = disableLookupSlowdown(ctx)
test.udp, err = ListenV4(ctx, test.pipe, ln, Config{
PrivateKey: test.localkey,
Log: testlog.Logger(t, log.LvlError),

ReplyTimeout: replyTimeout,

PrivateKeyGenerator: contextGetPrivateKeyGenerator(ctx),
})
if err != nil {
panic(err)
Expand Down Expand Up @@ -175,9 +188,12 @@ func TestUDPv4_responseTimeouts(t *testing.T) {
if runtime.GOOS == `darwin` {
t.Skip("unstable test on darwin")
}

t.Parallel()
test := newUDPTest(t)

ctx := context.Background()
ctx = contextWithReplyTimeout(ctx, respTimeout)

test := newUDPTestContext(ctx, t)
defer test.close()

rand.Seed(time.Now().UnixNano())
Expand Down Expand Up @@ -604,6 +620,24 @@ func startLocalhostV4(t *testing.T, cfg Config) *UDPv4 {
return udp
}

func contextWithReplyTimeout(ctx context.Context, value time.Duration) context.Context {
return context.WithValue(ctx, "p2p.discover.Config.ReplyTimeout", value)
}

func contextGetReplyTimeout(ctx context.Context) time.Duration {
value, _ := ctx.Value("p2p.discover.Config.ReplyTimeout").(time.Duration)
return value
}

func contextWithPrivateKeyGenerator(ctx context.Context, value func() (*ecdsa.PrivateKey, error)) context.Context {
return context.WithValue(ctx, "p2p.discover.Config.PrivateKeyGenerator", value)
}

func contextGetPrivateKeyGenerator(ctx context.Context) func() (*ecdsa.PrivateKey, error) {
value, _ := ctx.Value("p2p.discover.Config.PrivateKeyGenerator").(func() (*ecdsa.PrivateKey, error))
return value
}

// dgramPipe is a fake UDP socket. It queues all sent datagrams.
type dgramPipe struct {
queue chan dgram
Expand Down

0 comments on commit f2c5d04

Please sign in to comment.