p2p/discover: move bond logic from table to transport (#17048)

* p2p/discover: move bond logic from table to transport

This commit moves node endpoint verification (bonding) from the table to
the UDP transport implementation. Previously, adding a node to the table
entailed pinging the node if needed. With this change, the ping-back
logic is embedded in the packet handler at a lower level.

It is easy to verify that the basic protocol is unchanged: we still
require a valid pong reply from the node before findnode is accepted.

The node database tracked the time of last ping sent to the node and
time of last valid pong received from the node. Node endpoints are
considered verified when a valid pong is received and the time of last
pong was called 'bond time'. The time of last ping sent was unused. In
this commit, the last ping database entry is repurposed to mean last
ping _received_. This entry is now used to track whether the node needs
to be pinged back.

The other big change is how nodes are added to the table. We used to add
nodes in Table.bond, which ran when a remote node pinged us or when we
encountered the node in a neighbors reply. The transport now adds to the
table directly after the endpoint is verified through ping. To ensure
that the Table can't be filled just by pinging the node repeatedly, we
retain the isInitDone check. During init, only nodes from neighbors
replies are added.

* p2p/discover: reduce findnode failure counter on success

* p2p/discover: remove unused parameter of loadSeedNodes

* p2p/discover: improve ping-back check and comments

* p2p/discover: add neighbors reply nodes always, not just during init

1 files changed, 10 insertions, 16 deletions

diff --git a/p2p/discover/table_test.go b/p2p/discover/table_test.go
index f2d3f9a2a..ed55ebd9a 100644
--- a/p2p/discover/table_test.go
+++ b/p2p/discover/table_test.go
@@ -52,27 +52,22 @@ func testPingReplace(t *testing.T, newNodeIsResponding, lastInBucketIsResponding
 	tab, _ := newTable(transport, NodeID{}, &net.UDPAddr{}, "", nil)
 	defer tab.Close()
 
-	// Wait for init so bond is accepted.
 	<-tab.initDone
 
-	// fill up the sender's bucket.
+	// Fill up the sender's bucket.
 	pingSender := NewNode(MustHexID("a502af0f59b2aab7746995408c79e9ca312d2793cc997e44fc55eda62f0150bbb8c59a6f9269ba3a081518b62699ee807c7c19c20125ddfccca872608af9e370"), net.IP{}, 99, 99)
 	last := fillBucket(tab, pingSender)
 
-	// this call to bond should replace the last node
-	// in its bucket if the node is not responding.
+	// Add the sender as if it just pinged us. Revalidate should replace the last node in
+	// its bucket if it is unresponsive. Revalidate again to ensure that
 	transport.dead[last.ID] = !lastInBucketIsResponding
 	transport.dead[pingSender.ID] = !newNodeIsResponding
-	tab.bond(true, pingSender.ID, &net.UDPAddr{}, 0)
+	tab.add(pingSender)
+	tab.doRevalidate(make(chan struct{}, 1))
 	tab.doRevalidate(make(chan struct{}, 1))
 
-	// first ping goes to sender (bonding pingback)
-	if !transport.pinged[pingSender.ID] {
-		t.Error("table did not ping back sender")
-	}
 	if !transport.pinged[last.ID] {
-		// second ping goes to oldest node in bucket
-		// to see whether it is still alive.
+		// Oldest node in bucket is pinged to see whether it is still alive.
 		t.Error("table did not ping last node in bucket")
 	}
 
@@ -83,7 +78,7 @@ func testPingReplace(t *testing.T, newNodeIsResponding, lastInBucketIsResponding
 		wantSize--
 	}
 	if l := len(tab.bucket(pingSender.sha).entries); l != wantSize {
-		t.Errorf("wrong bucket size after bond: got %d, want %d", l, wantSize)
+		t.Errorf("wrong bucket size after add: got %d, want %d", l, wantSize)
 	}
 	if found := contains(tab.bucket(pingSender.sha).entries, last.ID); found != lastInBucketIsResponding {
 		t.Errorf("last entry found: %t, want: %t", found, lastInBucketIsResponding)
@@ -206,10 +201,7 @@ func newPingRecorder() *pingRecorder {
 func (t *pingRecorder) findnode(toid NodeID, toaddr *net.UDPAddr, target NodeID) ([]*Node, error) {
 	return nil, nil
 }
-func (t *pingRecorder) close() {}
-func (t *pingRecorder) waitping(from NodeID) error {
-	return nil // remote always pings
-}
+
 func (t *pingRecorder) ping(toid NodeID, toaddr *net.UDPAddr) error {
 	t.mu.Lock()
 	defer t.mu.Unlock()
@@ -222,6 +214,8 @@ func (t *pingRecorder) ping(toid NodeID, toaddr *net.UDPAddr) error {
 	}
 }
 
+func (t *pingRecorder) close() {}
+
 func TestTable_closest(t *testing.T) {
 	t.Parallel()