Skip to content

Commit 8a56826

Browse files
committed
fix(p2p/discover): return nil when v4 resolve cannot refresh node
Restore Resolve failure semantics to return nil so dial backoff can detect unresolved nodes correctly.\n\nAdd a regression test covering unresolved ENR refresh paths.
1 parent e98320f commit 8a56826

3 files changed

Lines changed: 44 additions & 4 deletions

File tree

cmd/devp2p/discv4cmd.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,12 @@ func discv4Resolve(ctx *cli.Context) error {
137137
disc := startV4(ctx)
138138
defer disc.Close()
139139

140-
fmt.Println(disc.Resolve(n).String())
140+
resolved := disc.Resolve(n)
141+
if resolved == nil {
142+
fmt.Println("unresolved")
143+
return fmt.Errorf("could not resolve node %s", n.ID())
144+
}
145+
fmt.Println(resolved.String())
141146
return nil
142147
}
143148

p2p/discover/v4_udp.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ func (t *UDPv4) Close() {
305305
}
306306

307307
// Resolve searches for a specific node with the given ID and tries to get the most recent
308-
// version of the node record for it. It returns n if the node could not be resolved.
308+
// version of the node record for it. It returns nil if the node could not be resolved.
309309
func (t *UDPv4) Resolve(n *enode.Node) *enode.Node {
310310
// Try asking directly. This works if the node is still responding on the endpoint we have.
311311
if rn, err := t.RequestENR(n); err == nil {
@@ -321,7 +321,7 @@ func (t *UDPv4) Resolve(n *enode.Node) *enode.Node {
321321
// Otherwise perform a network lookup.
322322
var key enode.Secp256k1
323323
if n.Load(&key) != nil {
324-
return n // no secp256k1 key
324+
return nil // no secp256k1 key
325325
}
326326
result := t.LookupPubkey((*ecdsa.PublicKey)(&key))
327327
for _, rn := range result {
@@ -331,7 +331,7 @@ func (t *UDPv4) Resolve(n *enode.Node) *enode.Node {
331331
}
332332
}
333333
}
334-
return n
334+
return nil
335335
}
336336

337337
func (t *UDPv4) ourEndpoint() rpcEndpoint {

p2p/discover/v4_udp_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,41 @@ func TestUDPv4_pingTimeout(t *testing.T) {
181181
}
182182
}
183183

184+
func TestUDPv4_resolveReturnsNilWhenUnresolved(t *testing.T) {
185+
test := newUDPTest(t)
186+
defer test.close()
187+
188+
var r enr.Record
189+
ip := net.IP{127, 0, 0, 1}
190+
id := enode.ID{9}
191+
r.Set(enr.IP(ip))
192+
r.Set(enr.UDP(30303))
193+
n := enode.SignNull(&r, id)
194+
195+
// Avoid triggering the extra bonding ping path in requestENR.
196+
test.udp.db.UpdateLastPingReceived(id, ip, time.Now())
197+
198+
if resolved := test.udp.Resolve(n); resolved != nil {
199+
t.Fatalf("expected nil for unresolved node, got: %v", resolved)
200+
}
201+
}
202+
203+
func TestUDPv4_resolveReturnsNilWhenLookupMisses(t *testing.T) {
204+
test := newUDPTest(t)
205+
defer test.close()
206+
207+
key := newkey()
208+
ip := net.IP{127, 0, 0, 1}
209+
n := enode.NewV4(&key.PublicKey, ip, 30303, 30303)
210+
211+
// Avoid triggering the extra bonding ping path in requestENR.
212+
test.udp.db.UpdateLastPingReceived(n.ID(), ip, time.Now())
213+
214+
if resolved := test.udp.Resolve(n); resolved != nil {
215+
t.Fatalf("expected nil for unresolved node with secp256k1 key, got: %v", resolved)
216+
}
217+
}
218+
184219
type testPacket byte
185220

186221
func (req testPacket) kind() byte { return byte(req) }

0 commit comments

Comments
 (0)