From 46e4f0b5c1d269e29d26a273016b18afbd13bbc4 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 29 Aug 2025 14:23:45 +0200 Subject: [PATCH 01/17] p2p/discover: add waitForNodes --- p2p/discover/lookup.go | 17 +++-------------- p2p/discover/table.go | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/p2p/discover/lookup.go b/p2p/discover/lookup.go index 09808b71e0..db1bbe32f9 100644 --- a/p2p/discover/lookup.go +++ b/p2p/discover/lookup.go @@ -19,7 +19,6 @@ package discover import ( "context" "errors" - "time" "github.com/ethereum/go-ethereum/p2p/enode" ) @@ -106,11 +105,10 @@ func (it *lookup) startQueries() bool { // The first query returns nodes from the local table. if it.queries == -1 { closest := it.tab.findnodeByID(it.result.target, bucketSize, false) - // Avoid finishing the lookup too quickly if table is empty. It'd be better to wait - // for the table to fill in this case, but there is no good mechanism for that - // yet. + // Avoid finishing the lookup too quickly if table is empty. + // Wait for the table to fill. if len(closest.entries) == 0 { - it.slowdown() + it.tab.waitForNodes(1) } it.queries = 1 it.replyCh <- closest.entries @@ -130,15 +128,6 @@ func (it *lookup) startQueries() bool { return it.queries > 0 } -func (it *lookup) slowdown() { - sleep := time.NewTimer(1 * time.Second) - defer sleep.Stop() - select { - case <-sleep.C: - case <-it.tab.closeReq: - } -} - func (it *lookup) query(n *enode.Node, reply chan<- []*enode.Node) { r, err := it.queryfunc(n) if !errors.Is(err, errClosed) { // avoid recording failures on shutdown. diff --git a/p2p/discover/table.go b/p2p/discover/table.go index b6c35aaaa9..63aa655256 100644 --- a/p2p/discover/table.go +++ b/p2p/discover/table.go @@ -32,6 +32,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/mclock" + "github.com/ethereum/go-ethereum/event" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/metrics" "github.com/ethereum/go-ethereum/p2p/enode" @@ -84,6 +85,7 @@ type Table struct { closeReq chan struct{} closed chan struct{} + nodeFeed event.FeedOf[*enode.Node] nodeAddedHook func(*bucket, *tableNode) nodeRemovedHook func(*bucket, *tableNode) } @@ -567,6 +569,8 @@ func (tab *Table) nodeAdded(b *bucket, n *tableNode) { } n.addedToBucket = time.Now() tab.revalidation.nodeAdded(tab, n) + + tab.nodeFeed.Send(n.Node) if tab.nodeAddedHook != nil { tab.nodeAddedHook(b, n) } @@ -702,3 +706,36 @@ func (tab *Table) deleteNode(n *enode.Node) { b := tab.bucket(n.ID()) tab.deleteInBucket(b, n.ID()) } + +// waitForNodes blocks until the table contains at least n nodes. +func (tab *Table) waitForNodes(n int) bool { + getlength := func() (count int) { + for _, b := range &tab.buckets { + count += len(b.entries) + } + return count + } + + var ch chan *enode.Node + for { + tab.mutex.Lock() + if getlength() >= n { + tab.mutex.Unlock() + return true + } + if ch == nil { + // Init subscription. + ch = make(chan *enode.Node) + sub := tab.nodeFeed.Subscribe(ch) + defer sub.Unsubscribe() + } + tab.mutex.Unlock() + + // Wait for a node add event. + select { + case <-ch: + case <-tab.closeReq: + return false + } + } +} From f8e0e8dc550621711d6702966a06c35fe9c99126 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 29 Aug 2025 15:47:09 +0200 Subject: [PATCH 02/17] p2p/discover: add context in waitForNodes --- p2p/discover/table.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/p2p/discover/table.go b/p2p/discover/table.go index 63aa655256..6a1c7494ee 100644 --- a/p2p/discover/table.go +++ b/p2p/discover/table.go @@ -708,7 +708,7 @@ func (tab *Table) deleteNode(n *enode.Node) { } // waitForNodes blocks until the table contains at least n nodes. -func (tab *Table) waitForNodes(n int) bool { +func (tab *Table) waitForNodes(ctx context.Context, n int) error { getlength := func() (count int) { for _, b := range &tab.buckets { count += len(b.entries) @@ -721,7 +721,7 @@ func (tab *Table) waitForNodes(n int) bool { tab.mutex.Lock() if getlength() >= n { tab.mutex.Unlock() - return true + return nil } if ch == nil { // Init subscription. @@ -734,8 +734,10 @@ func (tab *Table) waitForNodes(n int) bool { // Wait for a node add event. select { case <-ch: + case <-ctx.Done(): + return ctx.Err() case <-tab.closeReq: - return false + return errClosed } } } From f4046b0cfbfb60d1117a74d05a07dcd50d8dc753 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 29 Aug 2025 15:47:28 +0200 Subject: [PATCH 03/17] p2p/discover: move wait condition to lookupIterator --- p2p/discover/lookup.go | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/p2p/discover/lookup.go b/p2p/discover/lookup.go index db1bbe32f9..de4761d737 100644 --- a/p2p/discover/lookup.go +++ b/p2p/discover/lookup.go @@ -19,6 +19,7 @@ package discover import ( "context" "errors" + "time" "github.com/ethereum/go-ethereum/p2p/enode" ) @@ -105,10 +106,8 @@ func (it *lookup) startQueries() bool { // The first query returns nodes from the local table. if it.queries == -1 { closest := it.tab.findnodeByID(it.result.target, bucketSize, false) - // Avoid finishing the lookup too quickly if table is empty. - // Wait for the table to fill. if len(closest.entries) == 0 { - it.tab.waitForNodes(1) + return false } it.queries = 1 it.replyCh <- closest.entries @@ -171,6 +170,7 @@ func (it *lookupIterator) Next() bool { if len(it.buffer) > 0 { it.buffer = it.buffer[1:] } + // Advance the lookup to refill the buffer. for len(it.buffer) == 0 { if it.ctx.Err() != nil { @@ -183,6 +183,7 @@ func (it *lookupIterator) Next() bool { continue } if !it.lookup.advance() { + it.lookupFailed(it.lookup.tab) it.lookup = nil continue } @@ -191,6 +192,16 @@ func (it *lookupIterator) Next() bool { return true } +// lookupFailed handles failed lookup attempts. This can be called when the table has +// exited, or when it runs out of nodes. +func (it *lookupIterator) lookupFailed(tab *Table) { + timeout, cancel := context.WithTimeout(it.ctx, 1*time.Minute) + defer cancel() + tab.waitForNodes(timeout, 1) + + // TODO: here we need to trigger a table refresh somehow +} + // Close ends the iterator. func (it *lookupIterator) Close() { it.cancel() From 4ed8f5ee2b99a42fb3dca9e7083de1c7e05c39c4 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 5 Sep 2025 11:15:09 +0200 Subject: [PATCH 04/17] p2p/discover: improve iterator --- p2p/discover/lookup.go | 48 +++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/p2p/discover/lookup.go b/p2p/discover/lookup.go index de4761d737..91d4d7a919 100644 --- a/p2p/discover/lookup.go +++ b/p2p/discover/lookup.go @@ -49,11 +49,14 @@ func newLookup(ctx context.Context, tab *Table, target enode.ID, q queryFunc) *l result: nodesByDistance{target: target}, replyCh: make(chan []*enode.Node, alpha), cancelCh: ctx.Done(), - queries: -1, } // Don't query further if we hit ourself. // Unlikely to happen often in practice. it.asked[tab.self().ID()] = true + + // Initialize the lookup with nodes from table. + closest := it.tab.findnodeByID(it.result.target, bucketSize, false) + it.addNodes(closest.entries) return it } @@ -64,22 +67,18 @@ func (it *lookup) run() []*enode.Node { return it.result.entries } +func (it *lookup) empty() bool { + return len(it.replyBuffer) == 0 +} + // advance advances the lookup until any new nodes have been found. // It returns false when the lookup has ended. func (it *lookup) advance() bool { for it.startQueries() { select { case nodes := <-it.replyCh: - it.replyBuffer = it.replyBuffer[:0] - for _, n := range nodes { - if n != nil && !it.seen[n.ID()] { - it.seen[n.ID()] = true - it.result.push(n, bucketSize) - it.replyBuffer = append(it.replyBuffer, n) - } - } it.queries-- - if len(it.replyBuffer) > 0 { + if it.addNodes(nodes) { return true } case <-it.cancelCh: @@ -89,6 +88,18 @@ func (it *lookup) advance() bool { return false } +func (it *lookup) addNodes(nodes []*enode.Node) (done bool) { + it.replyBuffer = it.replyBuffer[:0] + for _, n := range nodes { + if n != nil && !it.seen[n.ID()] { + it.seen[n.ID()] = true + it.result.push(n, bucketSize) + it.replyBuffer = append(it.replyBuffer, n) + } + } + return len(it.replyBuffer) == 0 +} + func (it *lookup) shutdown() { for it.queries > 0 { <-it.replyCh @@ -103,17 +114,6 @@ func (it *lookup) startQueries() bool { return false } - // The first query returns nodes from the local table. - if it.queries == -1 { - closest := it.tab.findnodeByID(it.result.target, bucketSize, false) - if len(closest.entries) == 0 { - return false - } - it.queries = 1 - it.replyCh <- closest.entries - return true - } - // Ask the closest nodes that we haven't asked yet. for i := 0; i < len(it.result.entries) && it.queries < alpha; i++ { n := it.result.entries[i] @@ -180,10 +180,14 @@ func (it *lookupIterator) Next() bool { } if it.lookup == nil { it.lookup = it.nextLookup(it.ctx) + if it.lookup.empty() { + // If the lookup is empty right after creation, it means the local table + // is in a degraded state, and we need to wait for it to fill again. + it.lookupFailed(it.lookup.tab) + } continue } if !it.lookup.advance() { - it.lookupFailed(it.lookup.tab) it.lookup = nil continue } From e58e7f79272bdec8f0fd367ea943130461d5e94b Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 5 Sep 2025 14:15:09 +0200 Subject: [PATCH 05/17] p2p/discover: fix bug in lookup --- p2p/discover/lookup.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/p2p/discover/lookup.go b/p2p/discover/lookup.go index 91d4d7a919..c2b39a8c2b 100644 --- a/p2p/discover/lookup.go +++ b/p2p/discover/lookup.go @@ -78,7 +78,8 @@ func (it *lookup) advance() bool { select { case nodes := <-it.replyCh: it.queries-- - if it.addNodes(nodes) { + it.addNodes(nodes) + if !it.empty() { return true } case <-it.cancelCh: @@ -88,7 +89,7 @@ func (it *lookup) advance() bool { return false } -func (it *lookup) addNodes(nodes []*enode.Node) (done bool) { +func (it *lookup) addNodes(nodes []*enode.Node) { it.replyBuffer = it.replyBuffer[:0] for _, n := range nodes { if n != nil && !it.seen[n.ID()] { @@ -97,7 +98,6 @@ func (it *lookup) addNodes(nodes []*enode.Node) (done bool) { it.replyBuffer = append(it.replyBuffer, n) } } - return len(it.replyBuffer) == 0 } func (it *lookup) shutdown() { From 721c8de7389cb713c5013817356a99a67a0a2dce Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 5 Sep 2025 14:16:17 +0200 Subject: [PATCH 06/17] p2p/discover: trigger refresh in lookupIterator --- p2p/discover/lookup.go | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/p2p/discover/lookup.go b/p2p/discover/lookup.go index c2b39a8c2b..d15cdccc48 100644 --- a/p2p/discover/lookup.go +++ b/p2p/discover/lookup.go @@ -142,11 +142,12 @@ func (it *lookup) query(n *enode.Node, reply chan<- []*enode.Node) { // lookupIterator performs lookup operations and iterates over all seen nodes. // When a lookup finishes, a new one is created through nextLookup. type lookupIterator struct { - buffer []*enode.Node - nextLookup lookupFunc - ctx context.Context - cancel func() - lookup *lookup + buffer []*enode.Node + nextLookup lookupFunc + ctx context.Context + cancel func() + lookup *lookup + tabRefreshing <-chan struct{} } type lookupFunc func(ctx context.Context) *lookup @@ -187,11 +188,11 @@ func (it *lookupIterator) Next() bool { } continue } - if !it.lookup.advance() { - it.lookup = nil - continue - } + newNodes := it.lookup.advance() it.buffer = it.lookup.replyBuffer + if !newNodes { + it.lookup = nil + } } return true } @@ -201,9 +202,27 @@ func (it *lookupIterator) Next() bool { func (it *lookupIterator) lookupFailed(tab *Table) { timeout, cancel := context.WithTimeout(it.ctx, 1*time.Minute) defer cancel() - tab.waitForNodes(timeout, 1) - // TODO: here we need to trigger a table refresh somehow + // Wait for Table initialization to complete, in case it is still in progress. + select { + case <-tab.initDone: + case <-timeout.Done(): + return + } + + // Wait for ongoing refresh operation, or trigger one. + if it.tabRefreshing == nil { + it.tabRefreshing = tab.refresh() + } + select { + case <-it.tabRefreshing: + it.tabRefreshing = nil + case <-timeout.Done(): + return + } + + // Wait for the table to fill. + tab.waitForNodes(timeout, 1) } // Close ends the iterator. From cf0503da7ceffb5c24937b1f31e49acfacf9437d Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 5 Sep 2025 14:16:51 +0200 Subject: [PATCH 07/17] p2p/discover: track missing nodes in test --- p2p/discover/v4_udp_test.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/p2p/discover/v4_udp_test.go b/p2p/discover/v4_udp_test.go index 1af31f4f1b..b1363c73b0 100644 --- a/p2p/discover/v4_udp_test.go +++ b/p2p/discover/v4_udp_test.go @@ -509,18 +509,27 @@ func TestUDPv4_smallNetConvergence(t *testing.T) { // they have all found each other. status := make(chan error, len(nodes)) for i := range nodes { - node := nodes[i] + self := nodes[i] go func() { - found := make(map[enode.ID]bool, len(nodes)) - it := node.RandomNodes() + missing := make(map[enode.ID]bool, len(nodes)) + for _, n := range nodes { + if n.Self().ID() == self.Self().ID() { + continue // skip self + } + missing[n.Self().ID()] = true + } + + it := self.RandomNodes() for it.Next() { - found[it.Node().ID()] = true - if len(found) == len(nodes) { + fmt.Println(self.Self().ID(), "found:", it.Node().ID()) + delete(missing, it.Node().ID()) + if len(missing) == 0 { status <- nil return } } - status <- fmt.Errorf("node %s didn't find all nodes", node.Self().ID().TerminalString()) + missingIDs := slices.Collect(maps.Keys(missing)) + status <- fmt.Errorf("node %s didn't find all nodes, missing %v", self.Self().ID().TerminalString(), missingIDs) }() } @@ -537,7 +546,6 @@ func TestUDPv4_smallNetConvergence(t *testing.T) { received++ if err != nil { t.Error("ERROR:", err) - return } } } From 394670893546240b9c01987e075523ca3c33a300 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 9 Sep 2025 12:05:04 +0200 Subject: [PATCH 08/17] p2p/discover: fix two bugs in lookup iterator The lookup would add self into the replyBuffer if returned by another node. Avoid doing that by marking self as seen. With the changed initialization behavior of lookup, the lookupIterator needs to yield the buffer right after creation. This fixes the smallNetConvergence test, where all results are straight out of the local table. --- p2p/discover/lookup.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/p2p/discover/lookup.go b/p2p/discover/lookup.go index d15cdccc48..88c93a57cf 100644 --- a/p2p/discover/lookup.go +++ b/p2p/discover/lookup.go @@ -53,6 +53,7 @@ func newLookup(ctx context.Context, tab *Table, target enode.ID, q queryFunc) *l // Don't query further if we hit ourself. // Unlikely to happen often in practice. it.asked[tab.self().ID()] = true + it.seen[tab.self().ID()] = true // Initialize the lookup with nodes from table. closest := it.tab.findnodeByID(it.result.target, bucketSize, false) @@ -186,8 +187,12 @@ func (it *lookupIterator) Next() bool { // is in a degraded state, and we need to wait for it to fill again. it.lookupFailed(it.lookup.tab) } + // Otherwise, yield the initial nodes from the iterator before advancing + // the lookup. + it.buffer = it.lookup.replyBuffer continue } + newNodes := it.lookup.advance() it.buffer = it.lookup.replyBuffer if !newNodes { From 3133fd369a7bde62a6964ef1907a07fe2eb3c91a Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 9 Sep 2025 12:07:54 +0200 Subject: [PATCH 09/17] p2p/discover: remove print in test --- p2p/discover/v4_udp_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/p2p/discover/v4_udp_test.go b/p2p/discover/v4_udp_test.go index b1363c73b0..21c4daada3 100644 --- a/p2p/discover/v4_udp_test.go +++ b/p2p/discover/v4_udp_test.go @@ -521,7 +521,6 @@ func TestUDPv4_smallNetConvergence(t *testing.T) { it := self.RandomNodes() for it.Next() { - fmt.Println(self.Self().ID(), "found:", it.Node().ID()) delete(missing, it.Node().ID()) if len(missing) == 0 { status <- nil From a9f9e0d5894eea88b4ae5acdb637e0448de19c85 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 9 Sep 2025 12:08:44 +0200 Subject: [PATCH 10/17] p2p/discover: add imports in test --- p2p/discover/v4_udp_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/p2p/discover/v4_udp_test.go b/p2p/discover/v4_udp_test.go index 21c4daada3..44863183fa 100644 --- a/p2p/discover/v4_udp_test.go +++ b/p2p/discover/v4_udp_test.go @@ -24,10 +24,12 @@ import ( "errors" "fmt" "io" + "maps" "math/rand" "net" "net/netip" "reflect" + "slices" "sync" "testing" "time" From 72d3e881b32c214b51863661f0d625bb3e5bf319 Mon Sep 17 00:00:00 2001 From: Csaba Kiraly Date: Fri, 12 Sep 2025 10:52:53 +0200 Subject: [PATCH 11/17] p2p/discover: clarify lookup behavior on empty table We have changed this behavior, better clarify in comment. Signed-off-by: Csaba Kiraly --- p2p/discover/lookup.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/p2p/discover/lookup.go b/p2p/discover/lookup.go index 88c93a57cf..7382bbe41a 100644 --- a/p2p/discover/lookup.go +++ b/p2p/discover/lookup.go @@ -27,6 +27,7 @@ import ( // lookup performs a network search for nodes close to the given target. It approaches the // target by querying nodes that are closer to it on each iteration. The given target does // not need to be an actual node identifier. +// lookup on an empty table will return immediately with no nodes. type lookup struct { tab *Table queryfunc queryFunc @@ -142,6 +143,9 @@ func (it *lookup) query(n *enode.Node, reply chan<- []*enode.Node) { // lookupIterator performs lookup operations and iterates over all seen nodes. // When a lookup finishes, a new one is created through nextLookup. +// LookupIterator waits for table initialization and triggers a table refresh +// when necessary. + type lookupIterator struct { buffer []*enode.Node nextLookup lookupFunc From 3eab4616a69bc313a94f073e24cd5093b0dc922b Mon Sep 17 00:00:00 2001 From: Csaba Kiraly Date: Fri, 12 Sep 2025 10:59:29 +0200 Subject: [PATCH 12/17] p2p/discover: add test for lookup returning immediately Signed-off-by: Csaba Kiraly --- p2p/discover/v4_lookup_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/p2p/discover/v4_lookup_test.go b/p2p/discover/v4_lookup_test.go index 29a9dd6645..88ca82039e 100644 --- a/p2p/discover/v4_lookup_test.go +++ b/p2p/discover/v4_lookup_test.go @@ -23,6 +23,7 @@ import ( "slices" "sync" "testing" + "time" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/p2p/discover/v4wire" @@ -34,11 +35,15 @@ func TestUDPv4_Lookup(t *testing.T) { t.Parallel() test := newUDPTest(t) - // Lookup on empty table returns no nodes. + // Lookup on empty table returns immediately with no nodes. targetKey, _ := v4wire.DecodePubkey(crypto.S256(), lookupTestnet.target) + start := time.Now() if results := test.udp.LookupPubkey(targetKey); len(results) > 0 { t.Fatalf("lookup on empty table returned %d results: %#v", len(results), results) } + if time.Since(start) > 100*time.Millisecond { + t.Fatalf("lookup on empty table took too long: %s", time.Since(start)) + } // Seed table with initial node. fillTable(test.table, []*enode.Node{lookupTestnet.node(256, 0)}, true) From 97afa2815beefec5bb90a06b0fcf06ec25d99eb8 Mon Sep 17 00:00:00 2001 From: Csaba Kiraly Date: Fri, 12 Sep 2025 11:29:43 +0200 Subject: [PATCH 13/17] Revert "p2p/discover: add test for lookup returning immediately" This reverts commit 3eab4616a69bc313a94f073e24cd5093b0dc922b. --- p2p/discover/v4_lookup_test.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/p2p/discover/v4_lookup_test.go b/p2p/discover/v4_lookup_test.go index 88ca82039e..29a9dd6645 100644 --- a/p2p/discover/v4_lookup_test.go +++ b/p2p/discover/v4_lookup_test.go @@ -23,7 +23,6 @@ import ( "slices" "sync" "testing" - "time" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/p2p/discover/v4wire" @@ -35,15 +34,11 @@ func TestUDPv4_Lookup(t *testing.T) { t.Parallel() test := newUDPTest(t) - // Lookup on empty table returns immediately with no nodes. + // Lookup on empty table returns no nodes. targetKey, _ := v4wire.DecodePubkey(crypto.S256(), lookupTestnet.target) - start := time.Now() if results := test.udp.LookupPubkey(targetKey); len(results) > 0 { t.Fatalf("lookup on empty table returned %d results: %#v", len(results), results) } - if time.Since(start) > 100*time.Millisecond { - t.Fatalf("lookup on empty table took too long: %s", time.Since(start)) - } // Seed table with initial node. fillTable(test.table, []*enode.Node{lookupTestnet.node(256, 0)}, true) From 68c18ede06df9e28bbd735ccc5f76dc81863054e Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 12 Sep 2025 11:34:44 +0200 Subject: [PATCH 14/17] Update lookup.go --- p2p/discover/lookup.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/p2p/discover/lookup.go b/p2p/discover/lookup.go index 7382bbe41a..efa8191ae0 100644 --- a/p2p/discover/lookup.go +++ b/p2p/discover/lookup.go @@ -191,8 +191,7 @@ func (it *lookupIterator) Next() bool { // is in a degraded state, and we need to wait for it to fill again. it.lookupFailed(it.lookup.tab) } - // Otherwise, yield the initial nodes from the iterator before advancing - // the lookup. + // Yield the initial nodes from the iterator before advancing the lookup. it.buffer = it.lookup.replyBuffer continue } From 06434279655f034529a16c6fe76d7c5ae785ec12 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 12 Sep 2025 12:50:07 +0200 Subject: [PATCH 15/17] p2p/discover: continue --- p2p/discover/lookup.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/p2p/discover/lookup.go b/p2p/discover/lookup.go index efa8191ae0..ff2dc907cd 100644 --- a/p2p/discover/lookup.go +++ b/p2p/discover/lookup.go @@ -190,6 +190,8 @@ func (it *lookupIterator) Next() bool { // If the lookup is empty right after creation, it means the local table // is in a degraded state, and we need to wait for it to fill again. it.lookupFailed(it.lookup.tab) + it.lookup = nil + continue } // Yield the initial nodes from the iterator before advancing the lookup. it.buffer = it.lookup.replyBuffer From 3589c0d59b6121d019ac87bb25a5cfc0c4244ef0 Mon Sep 17 00:00:00 2001 From: Csaba Kiraly Date: Tue, 16 Sep 2025 14:03:11 +0200 Subject: [PATCH 16/17] p2p/discover: expose timeout in lookupFailed Signed-off-by: Csaba Kiraly # Conflicts: # p2p/discover/lookup.go --- p2p/discover/lookup.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/p2p/discover/lookup.go b/p2p/discover/lookup.go index ff2dc907cd..684067a619 100644 --- a/p2p/discover/lookup.go +++ b/p2p/discover/lookup.go @@ -189,7 +189,7 @@ func (it *lookupIterator) Next() bool { if it.lookup.empty() { // If the lookup is empty right after creation, it means the local table // is in a degraded state, and we need to wait for it to fill again. - it.lookupFailed(it.lookup.tab) + it.lookupFailed(1 * time.Minute) it.lookup = nil continue } @@ -209,30 +209,30 @@ func (it *lookupIterator) Next() bool { // lookupFailed handles failed lookup attempts. This can be called when the table has // exited, or when it runs out of nodes. -func (it *lookupIterator) lookupFailed(tab *Table) { - timeout, cancel := context.WithTimeout(it.ctx, 1*time.Minute) +func (it *lookupIterator) lookupFailed(timeout time.Duration) { + ctx, cancel := context.WithTimeout(it.ctx, timeout) defer cancel() // Wait for Table initialization to complete, in case it is still in progress. select { - case <-tab.initDone: - case <-timeout.Done(): + case <-it.lookup.tab.initDone: + case <-ctx.Done(): return } // Wait for ongoing refresh operation, or trigger one. if it.tabRefreshing == nil { - it.tabRefreshing = tab.refresh() + it.tabRefreshing = it.lookup.tab.refresh() } select { case <-it.tabRefreshing: it.tabRefreshing = nil - case <-timeout.Done(): + case <-ctx.Done(): return } // Wait for the table to fill. - tab.waitForNodes(timeout, 1) + it.lookup.tab.waitForNodes(ctx, 1) } // Close ends the iterator. From de9fb9722bd6448c5657c50e82df821a4dd40ea6 Mon Sep 17 00:00:00 2001 From: Csaba Kiraly Date: Wed, 17 Sep 2025 09:02:38 +0200 Subject: [PATCH 17/17] revert to using table parameter using it.lookup.tab inside is unsafe Signed-off-by: Csaba Kiraly --- p2p/discover/lookup.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/p2p/discover/lookup.go b/p2p/discover/lookup.go index 684067a619..9cca0118ac 100644 --- a/p2p/discover/lookup.go +++ b/p2p/discover/lookup.go @@ -189,7 +189,7 @@ func (it *lookupIterator) Next() bool { if it.lookup.empty() { // If the lookup is empty right after creation, it means the local table // is in a degraded state, and we need to wait for it to fill again. - it.lookupFailed(1 * time.Minute) + it.lookupFailed(it.lookup.tab, 1*time.Minute) it.lookup = nil continue } @@ -209,30 +209,30 @@ func (it *lookupIterator) Next() bool { // lookupFailed handles failed lookup attempts. This can be called when the table has // exited, or when it runs out of nodes. -func (it *lookupIterator) lookupFailed(timeout time.Duration) { - ctx, cancel := context.WithTimeout(it.ctx, timeout) +func (it *lookupIterator) lookupFailed(tab *Table, timeout time.Duration) { + tout, cancel := context.WithTimeout(it.ctx, timeout) defer cancel() // Wait for Table initialization to complete, in case it is still in progress. select { - case <-it.lookup.tab.initDone: - case <-ctx.Done(): + case <-tab.initDone: + case <-tout.Done(): return } // Wait for ongoing refresh operation, or trigger one. if it.tabRefreshing == nil { - it.tabRefreshing = it.lookup.tab.refresh() + it.tabRefreshing = tab.refresh() } select { case <-it.tabRefreshing: it.tabRefreshing = nil - case <-ctx.Done(): + case <-tout.Done(): return } // Wait for the table to fill. - it.lookup.tab.waitForNodes(ctx, 1) + tab.waitForNodes(tout, 1) } // Close ends the iterator.