Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

p2p/discover: improved node revalidation #29572

Merged
merged 41 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
bc62df2
internal/testlog: fix level matching
fjl Apr 17, 2024
8f4c72c
p2p/discover: add debug API
fjl Mar 26, 2024
37149d0
cmd/devp2p: add discv4 RPC server
fjl Mar 26, 2024
1a6a64b
cmd/devp2p: add listen command
fjl Mar 26, 2024
f8c7448
cmd/devp2p: fix import
fjl Mar 26, 2024
df8e793
p2p/discover: new revalidation
fjl Apr 17, 2024
70b8918
p2p/discover: fix build
fjl Apr 17, 2024
0a4b314
p2p/discover: update
fjl Apr 18, 2024
fad3fe7
p2p/discover: fix some tests
fjl Apr 18, 2024
f2ac692
p2p/discover: fix shutdown hang
fjl Apr 18, 2024
00e71f5
p2p/discover: fix test
fjl Apr 18, 2024
1d896e8
p2p/discover: fix spin condition
fjl Apr 18, 2024
dc3f78b
p2p/discover: add live flag in debug node
fjl Apr 18, 2024
c1afb3c
p2p/discover: return nextTime from run()
fjl Apr 20, 2024
7667ebf
p2p/discover: simplify rand timer
fjl Apr 20, 2024
b44673f
p2p/discover: move back to fast list when endpoint changed or dead
fjl Apr 20, 2024
822b059
p2p/discover: fix crash in move
fjl Apr 20, 2024
c2ad0a7
p2p/discover: improve list moving
fjl Apr 20, 2024
0b44fc4
p2p/discover: faster
fjl Apr 20, 2024
30eb2bf
p2p/discover: remove logging checks in for dead nodes (it's always zero)
fjl Apr 20, 2024
81e553f
p2p/discover: faster decay
fjl Apr 20, 2024
0048a0f
Update p2p/discover/table_reval.go
fjl Apr 24, 2024
041ce1b
p2p/discover: fix addedAt time
fjl Apr 22, 2024
2c2c7f7
p2p/discover: rename add node methods
fjl Apr 22, 2024
ea9f0a4
p2p/discover: put back findnode request tracking
fjl Apr 25, 2024
edb3a12
p2p/discover: fix lookup failure log
fjl Apr 26, 2024
1765712
p2p/discover: fix double lock
fjl Apr 26, 2024
009658c
p2p/discover: seed nodes are not inbound
fjl Apr 26, 2024
051ca13
p2p/discover: use lock in reseedingRandom
fjl May 3, 2024
9faae76
p2p/discover: improve test reliability
fjl May 3, 2024
008ff21
p2p/discover: fix race in test
fjl May 14, 2024
f96f596
p2p/discover: rename
fjl May 14, 2024
f0504f6
p2p/discover: simplify nextTime
fjl May 14, 2024
cda6f56
p2p/discover: rename fields
fjl May 21, 2024
6e37d52
p2p/discover: change to / 3
fjl May 21, 2024
9ec2553
p2p/discover: explain
fjl May 22, 2024
de7e1c7
p2p/discover: shuffle in appendLiveNodes
fjl May 22, 2024
4ace554
p2p/discover: fix flaky test
fjl May 23, 2024
16b4386
p2p/discover: add documentation
fjl May 23, 2024
4386e8a
p2p: add accessors for discovery instances
fjl May 23, 2024
6101ec3
node: add p2p debug API
fjl May 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions cmd/devp2p/discv4cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"errors"
"fmt"
"net"
"net/http"
"strconv"
"strings"
"time"
Expand All @@ -28,9 +29,11 @@ import (
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/internal/flags"
"github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/p2p/discover"
"github.com/ethereum/go-ethereum/p2p/enode"
"github.com/ethereum/go-ethereum/params"
"github.com/ethereum/go-ethereum/rpc"
"github.com/urfave/cli/v2"
)

Expand All @@ -45,6 +48,7 @@ var (
discv4ResolveJSONCommand,
discv4CrawlCommand,
discv4TestCommand,
discv4ListenCommand,
},
}
discv4PingCommand = &cli.Command{
Expand Down Expand Up @@ -75,6 +79,14 @@ var (
Flags: discoveryNodeFlags,
ArgsUsage: "<nodes.json file>",
}
discv4ListenCommand = &cli.Command{
Name: "listen",
Usage: "Runs a discovery node",
Action: discv4Listen,
Flags: flags.Merge(discoveryNodeFlags, []cli.Flag{
httpAddrFlag,
}),
}
discv4CrawlCommand = &cli.Command{
Name: "crawl",
Usage: "Updates a nodes.json file with random nodes found in the DHT",
Expand Down Expand Up @@ -131,6 +143,10 @@ var (
Usage: "Enode of the remote node under test",
EnvVars: []string{"REMOTE_ENODE"},
}
httpAddrFlag = &cli.StringFlag{
Name: "rpc",
Usage: "HTTP server listening address",
}
)

var discoveryNodeFlags = []cli.Flag{
Expand All @@ -154,6 +170,27 @@ func discv4Ping(ctx *cli.Context) error {
return nil
}

func discv4Listen(ctx *cli.Context) error {
disc, _ := startV4(ctx)
defer disc.Close()

fmt.Println(disc.Self())

httpAddr := ctx.String(httpAddrFlag.Name)
if httpAddr == "" {
// Non-HTTP mode.
select {}
}

api := &discv4API{disc}
log.Info("Starting RPC API server", "addr", httpAddr)
srv := rpc.NewServer()
srv.RegisterName("discv4", api)
http.DefaultServeMux.Handle("/", srv)
httpsrv := http.Server{Addr: httpAddr, Handler: http.DefaultServeMux}
return httpsrv.ListenAndServe()
}

func discv4RequestRecord(ctx *cli.Context) error {
n := getNodeArg(ctx)
disc, _ := startV4(ctx)
Expand Down Expand Up @@ -362,3 +399,23 @@ func parseBootnodes(ctx *cli.Context) ([]*enode.Node, error) {
}
return nodes, nil
}

type discv4API struct {
host *discover.UDPv4
}

func (api *discv4API) LookupRandom(n int) (ns []*enode.Node) {
it := api.host.RandomNodes()
for len(ns) < n && it.Next() {
ns = append(ns, it.Node())
}
return ns
}

func (api *discv4API) Buckets() [][]discover.BucketNode {
return api.host.TableBuckets()
}

func (api *discv4API) Self() *enode.Node {
return api.host.Self()
}
2 changes: 1 addition & 1 deletion internal/testlog/testlog.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (h *bufHandler) Handle(_ context.Context, r slog.Record) error {
}

func (h *bufHandler) Enabled(_ context.Context, lvl slog.Level) bool {
return lvl <= h.level
return lvl >= h.level
}

func (h *bufHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
Expand Down
40 changes: 39 additions & 1 deletion p2p/discover/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ package discover

import (
"crypto/ecdsa"
crand "crypto/rand"
"encoding/binary"
"math/rand"
"net"
"sync"
"time"

"github.com/ethereum/go-ethereum/common/mclock"
Expand Down Expand Up @@ -62,7 +66,7 @@ type Config struct {
func (cfg Config) withDefaults() Config {
// Node table configuration:
if cfg.PingInterval == 0 {
cfg.PingInterval = 10 * time.Second
cfg.PingInterval = 3 * time.Second
}
if cfg.RefreshInterval == 0 {
cfg.RefreshInterval = 30 * time.Minute
Expand Down Expand Up @@ -92,3 +96,37 @@ type ReadPacket struct {
Data []byte
Addr *net.UDPAddr
}

type randomSource interface {
Intn(int) int
Int63n(int64) int64
}

// reseedingRandom is a random number generator that tracks when it was last re-seeded.
type reseedingRandom struct {
mu sync.Mutex
cur *rand.Rand
}

func (r *reseedingRandom) seed() {
var b [8]byte
crand.Read(b[:])
seed := binary.BigEndian.Uint64(b[:])
new := rand.New(rand.NewSource(int64(seed)))

r.mu.Lock()
r.cur = new
r.mu.Unlock()
}

func (r *reseedingRandom) Intn(n int) int {
r.mu.Lock()
defer r.mu.Unlock()
return r.cur.Intn(n)
}

func (r *reseedingRandom) Int63n(n int64) int64 {
r.mu.Lock()
defer r.mu.Unlock()
return r.cur.Int63n(n)
}
29 changes: 5 additions & 24 deletions p2p/discover/lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,32 +140,13 @@ func (it *lookup) slowdown() {
}

func (it *lookup) query(n *node, reply chan<- []*node) {
fails := it.tab.db.FindFails(n.ID(), n.IP())
r, err := it.queryfunc(n)
if errors.Is(err, errClosed) {
// Avoid recording failures on shutdown.
reply <- nil
return
} else if len(r) == 0 {
fails++
it.tab.db.UpdateFindFails(n.ID(), n.IP(), fails)
// Remove the node from the local table if it fails to return anything useful too
// many times, but only if there are enough other nodes in the bucket.
dropped := false
if fails >= maxFindnodeFailures && it.tab.bucketLen(n.ID()) >= bucketSize/2 {
dropped = true
it.tab.delete(n)
if !errors.Is(err, errClosed) { // avoid recording failures on shutdown.
success := len(r) > 0
it.tab.trackRequest(n, success, r)
if err != nil {
it.tab.log.Trace("FINDNODE failed", "id", n.ID(), "err", err)
}
it.tab.log.Trace("FINDNODE failed", "id", n.ID(), "failcount", fails, "dropped", dropped, "err", err)
} else if fails > 0 {
// Reset failure counter because it counts _consecutive_ failures.
it.tab.db.UpdateFindFails(n.ID(), n.IP(), 0)
}

// Grab as many nodes as possible. Some of them might not be alive anymore, but we'll
// just remove those again during revalidation.
for _, n := range r {
it.tab.addSeenNode(n)
}
reply <- r
}
Expand Down
20 changes: 15 additions & 5 deletions p2p/discover/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,22 @@ import (
"github.com/ethereum/go-ethereum/p2p/enode"
)

type BucketNode struct {
Node *enode.Node `json:"node"`
AddedTable time.Time `json:"addedToTable"`
AddedBucket time.Time `json:"addedToBucket"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The node has addedToTable field, and the json tag also contains a to, so it looks a bit odd that the field name itself is missing the to, as in AddedToTable/AddedToBucket

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's an inconsistency. I will fix.

Checks int `json:"checks"`
Live bool `json:"live"`
}

// node represents a host on the network.
// The fields of Node may not be modified.
type node struct {
enode.Node
addedAt time.Time // time when the node was added to the table
livenessChecks uint // how often liveness was checked
*enode.Node
addedToTable time.Time // first time node was added to bucket or replacement list
addedToBucket time.Time // time it was added in the actual bucket
livenessChecks uint // how often liveness was checked
isValidatedLive bool // true if existence of node is considered validated right now
}

type encPubkey [64]byte
Expand Down Expand Up @@ -65,7 +75,7 @@ func (e encPubkey) id() enode.ID {
}

func wrapNode(n *enode.Node) *node {
return &node{Node: *n}
return &node{Node: n}
}

func wrapNodes(ns []*enode.Node) []*node {
Expand All @@ -77,7 +87,7 @@ func wrapNodes(ns []*enode.Node) []*node {
}

func unwrapNode(n *node) *enode.Node {
return &n.Node
return n.Node
}

func unwrapNodes(ns []*node) []*enode.Node {
Expand Down
Loading
Loading