From 2c2c7f7a070a48d936935fbd24b2d879d586c3d8 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Mon, 22 Apr 2024 16:51:12 +0200 Subject: [PATCH] p2p/discover: rename add node methods This is to better reflect their purpose. The previous naming of 'seen' and 'verified' was kind of arbitrary, especially since 'verified' was the stricter one. --- p2p/discover/lookup.go | 2 +- p2p/discover/table.go | 30 ++++++++++++++++-------------- p2p/discover/table_test.go | 20 ++++++++++---------- p2p/discover/table_util_test.go | 2 +- p2p/discover/v4_udp.go | 4 ++-- p2p/discover/v5_udp.go | 2 +- p2p/discover/v5_udp_test.go | 2 +- 7 files changed, 32 insertions(+), 30 deletions(-) diff --git a/p2p/discover/lookup.go b/p2p/discover/lookup.go index df6cddc833..95b19943d6 100644 --- a/p2p/discover/lookup.go +++ b/p2p/discover/lookup.go @@ -165,7 +165,7 @@ func (it *lookup) query(n *node, reply chan<- []*node) { // 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) + it.tab.addFoundNode(n) } reply <- r } diff --git a/p2p/discover/table.go b/p2p/discover/table.go index 24b3c7b686..421bcae984 100644 --- a/p2p/discover/table.go +++ b/p2p/discover/table.go @@ -108,8 +108,8 @@ type bucket struct { } type addNodeRequest struct { - node *node - isLive bool + node *node + isInbound bool } func newTable(t transport, db *enode.DB, cfg Config) (*Table, error) { @@ -300,13 +300,13 @@ func (tab *Table) len() (n int) { return n } -// addSeenNode adds a node which may not be live. If the bucket has space available, +// addFoundNode adds a node which may not be live. If the bucket has space available, // adding the node succeeds immediately. Otherwise, the node is added to the replacements // list. // // The caller must not hold tab.mutex. -func (tab *Table) addSeenNode(n *node) { - req := addNodeRequest{node: n, isLive: false} +func (tab *Table) addFoundNode(n *node) { + req := addNodeRequest{node: n, isInbound: false} select { case tab.addNodeCh <- req: <-tab.addNodeHandled @@ -314,16 +314,16 @@ func (tab *Table) addSeenNode(n *node) { } } -// addVerifiedNode adds a node whose existence has been verified recently. If the bucket -// has no space, the node is added to the replacements list. +// addInboundNode adds a node from an inbound contact. If the bucket has no space, the +// node is added to the replacements list. // -// There is an additional safety measure: if the table is still initializing the node -// is not added. This prevents an attack where the table could be filled by just sending -// ping repeatedly. +// There is an additional safety measure: if the table is still initializing the node is +// not added. This prevents an attack where the table could be filled by just sending ping +// repeatedly. // // The caller must not hold tab.mutex. -func (tab *Table) addVerifiedNode(n *node) { - req := addNodeRequest{node: n, isLive: true} +func (tab *Table) addInboundNode(n *node) { + req := addNodeRequest{node: n, isInbound: true} select { case tab.addNodeCh <- req: <-tab.addNodeHandled @@ -435,7 +435,7 @@ func (tab *Table) loadSeedNodes() { age := time.Since(tab.db.LastPongReceived(seed.ID(), seed.IP())) tab.log.Trace("Found seed node in database", "id", seed.ID(), "addr", seed.addr(), "age", age) } - tab.handleAddNode(addNodeRequest{node: seed, isLive: true}) + tab.handleAddNode(addNodeRequest{node: seed, isInbound: true}) } } @@ -488,7 +488,9 @@ func (tab *Table) handleAddNode(req addNodeRequest) { if req.node.ID() == tab.self().ID() { return } - if !req.isLive && !tab.isInitDone() { + // For nodes from inbound contact, there is an additional safety measure: if the table + // is still initializing the node is not added. + if req.isInbound && !tab.isInitDone() { return } diff --git a/p2p/discover/table_test.go b/p2p/discover/table_test.go index 9796d576e5..ac159ebd5b 100644 --- a/p2p/discover/table_test.go +++ b/p2p/discover/table_test.go @@ -71,7 +71,7 @@ func testPingReplace(t *testing.T, newNodeIsResponding, lastInBucketIsResponding // this node in the bucket if it is unresponsive. transport.dead[last.ID()] = !lastInBucketIsResponding transport.dead[replacementNode.ID()] = !newNodeIsResponding - tab.addSeenNode(replacementNode) + tab.addFoundNode(replacementNode) // Wait until the last node was pinged. waitForRevalidationPing(t, transport, tab, last.ID()) @@ -127,7 +127,7 @@ func TestTable_IPLimit(t *testing.T) { for i := 0; i < tableIPLimit+1; i++ { n := nodeAtDistance(tab.self().ID(), i, net.IP{172, 0, 1, byte(i)}) - tab.addSeenNode(n) + tab.addFoundNode(n) } if tab.len() > tableIPLimit { t.Errorf("too many nodes in table") @@ -145,7 +145,7 @@ func TestTable_BucketIPLimit(t *testing.T) { d := 3 for i := 0; i < bucketIPLimit+1; i++ { n := nodeAtDistance(tab.self().ID(), d, net.IP{172, 0, 1, byte(i)}) - tab.addSeenNode(n) + tab.addFoundNode(n) } if tab.len() > bucketIPLimit { t.Errorf("too many nodes in table") @@ -258,8 +258,8 @@ func TestTable_addVerifiedNode(t *testing.T) { // Insert two nodes. n1 := nodeAtDistance(tab.self().ID(), 256, net.IP{88, 77, 66, 1}) n2 := nodeAtDistance(tab.self().ID(), 256, net.IP{88, 77, 66, 2}) - tab.addSeenNode(n1) - tab.addSeenNode(n2) + tab.addFoundNode(n1) + tab.addFoundNode(n2) bucket := tab.bucket(n1.ID()) // Verify bucket content: @@ -272,7 +272,7 @@ func TestTable_addVerifiedNode(t *testing.T) { newrec := n2.Record() newrec.Set(enr.IP{99, 99, 99, 99}) newn2 := wrapNode(enode.SignNull(newrec, n2.ID())) - tab.addVerifiedNode(newn2) + tab.addInboundNode(newn2) // Check that bucket is updated correctly. newBcontent := []*node{n1, newn2} @@ -291,8 +291,8 @@ func TestTable_addSeenNode(t *testing.T) { // Insert two nodes. n1 := nodeAtDistance(tab.self().ID(), 256, net.IP{88, 77, 66, 1}) n2 := nodeAtDistance(tab.self().ID(), 256, net.IP{88, 77, 66, 2}) - tab.addSeenNode(n1) - tab.addSeenNode(n2) + tab.addFoundNode(n1) + tab.addFoundNode(n2) // Verify bucket content: bcontent := []*node{n1, n2} @@ -304,7 +304,7 @@ func TestTable_addSeenNode(t *testing.T) { newrec := n2.Record() newrec.Set(enr.IP{99, 99, 99, 99}) newn2 := wrapNode(enode.SignNull(newrec, n2.ID())) - tab.addSeenNode(newn2) + tab.addFoundNode(newn2) // Check that bucket content is unchanged. if !reflect.DeepEqual(tab.bucket(n1.ID()).entries, bcontent) { @@ -330,7 +330,7 @@ func TestTable_revalidateSyncRecord(t *testing.T) { r.Set(enr.IP(net.IP{127, 0, 0, 1})) id := enode.ID{1} n1 := wrapNode(enode.SignNull(&r, id)) - tab.addSeenNode(n1) + tab.addFoundNode(n1) // Update the node record. r.Set(enr.WithEntry("foo", "bar")) diff --git a/p2p/discover/table_util_test.go b/p2p/discover/table_util_test.go index 6ed4bcda90..6d278f5c11 100644 --- a/p2p/discover/table_util_test.go +++ b/p2p/discover/table_util_test.go @@ -118,7 +118,7 @@ func fillTable(tab *Table, nodes []*node, setLive bool) { n.livenessChecks = 1 n.isValidatedLive = true } - tab.addSeenNode(n) + tab.addFoundNode(n) } } diff --git a/p2p/discover/v4_udp.go b/p2p/discover/v4_udp.go index a6121d1923..d4e0641674 100644 --- a/p2p/discover/v4_udp.go +++ b/p2p/discover/v4_udp.go @@ -673,10 +673,10 @@ func (t *UDPv4) handlePing(h *packetHandlerV4, from *net.UDPAddr, fromID enode.I n := wrapNode(enode.NewV4(h.senderKey, from.IP, int(req.From.TCP), from.Port)) if time.Since(t.db.LastPongReceived(n.ID(), from.IP)) > bondExpiration { t.sendPing(fromID, from, func() { - t.tab.addVerifiedNode(n) + t.tab.addInboundNode(n) }) } else { - t.tab.addVerifiedNode(n) + t.tab.addInboundNode(n) } // Update node database and endpoint predictor. diff --git a/p2p/discover/v5_udp.go b/p2p/discover/v5_udp.go index c6e65bfca2..8cdc9dfbce 100644 --- a/p2p/discover/v5_udp.go +++ b/p2p/discover/v5_udp.go @@ -699,7 +699,7 @@ func (t *UDPv5) handlePacket(rawpacket []byte, fromAddr *net.UDPAddr) error { } if fromNode != nil { // Handshake succeeded, add to table. - t.tab.addSeenNode(wrapNode(fromNode)) + t.tab.addInboundNode(wrapNode(fromNode)) } if packet.Kind() != v5wire.WhoareyouPacket { // WHOAREYOU logged separately to report errors. diff --git a/p2p/discover/v5_udp_test.go b/p2p/discover/v5_udp_test.go index 4373ea8184..0015f7cc70 100644 --- a/p2p/discover/v5_udp_test.go +++ b/p2p/discover/v5_udp_test.go @@ -141,7 +141,7 @@ func TestUDPv5_unknownPacket(t *testing.T) { // Make node known. n := test.getNode(test.remotekey, test.remoteaddr).Node() - test.table.addSeenNode(wrapNode(n)) + test.table.addFoundNode(wrapNode(n)) test.packetIn(&v5wire.Unknown{Nonce: nonce}) test.waitPacketOut(func(p *v5wire.Whoareyou, addr *net.UDPAddr, _ v5wire.Nonce) {