Skip to content

Commit 276ed48

Browse files
p2p/discover: add discv5 invalid findnodes result test cases (#32481)
Supersedes #32470. ### What - snap: shorten stall watchdog in `eth/protocols/snap/sync_test.go` from 1m to 10s. - discover/v5: consolidate FINDNODE negative tests into a single table-driven test: - `TestUDPv5_findnodeCall_InvalidNodes` covers: - invalid IP (unspecified `0.0.0.0`) → ignored - low UDP port (`<=1024`) → ignored ### Why - Addresses TODOs: - “Make tests smaller” (reduce long 1m timeout). - “check invalid IPs”; also cover low port per `verifyResponseNode` rules (UDP must be >1024). ### How it’s validated - Test-only changes; no production code touched. - Local runs: - `go test ./p2p/discover -count=1 -timeout=300s` → ok - `go test ./eth/protocols/snap -count=1 -timeout=600s` → ok - Lint: - `go run build/ci.go lint` → 0 issues on modified files. ### Notes - The test harness uses `enode.ValidSchemesForTesting` (which includes the “null” scheme), so records signed with `enode.SignNull` are signature-valid; failures here are due to IP/port validation in `verifyResponseNode` and `netutil.CheckRelayAddr`. - Tests are written as a single table-driven function for clarity; no helpers or environment switching. --------- Co-authored-by: lightclient <lightclient@protonmail.com>
1 parent e965623 commit 276ed48

File tree

1 file changed

+86
-2
lines changed

1 file changed

+86
-2
lines changed

p2p/discover/v5_udp_test.go

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -378,9 +378,93 @@ func TestUDPv5_findnodeCall(t *testing.T) {
378378
if !reflect.DeepEqual(response, nodes) {
379379
t.Fatalf("wrong nodes in response")
380380
}
381+
}
382+
383+
// BadIdentityScheme mocks an identity scheme not supported by the test node.
384+
type BadIdentityScheme struct{}
385+
386+
func (s BadIdentityScheme) Verify(r *enr.Record, sig []byte) error { return nil }
387+
func (s BadIdentityScheme) NodeAddr(r *enr.Record) []byte {
388+
var id enode.ID
389+
r.Load(enr.WithEntry("badaddr", &id))
390+
return id[:]
391+
}
392+
393+
// This test covers invalid NODES responses for the FINDNODE call in a single table-driven test.
394+
func TestUDPv5_findnodeCall_InvalidNodes(t *testing.T) {
395+
t.Parallel()
396+
test := newUDPV5Test(t)
397+
defer test.close()
381398

382-
// TODO: check invalid IPs
383-
// TODO: check invalid/unsigned record
399+
for i, tt := range []struct {
400+
name string
401+
ip enr.Entry
402+
port enr.Entry
403+
sign func(r *enr.Record, id enode.ID) *enode.Node
404+
}{
405+
{
406+
name: "invalid ip (unspecified 0.0.0.0)",
407+
ip: enr.IP(net.IPv4zero),
408+
},
409+
{
410+
name: "invalid udp port (<=1024)",
411+
port: enr.UDP(1024),
412+
},
413+
{
414+
name: "invalid record, no signature",
415+
sign: func(r *enr.Record, id enode.ID) *enode.Node {
416+
r.Set(enr.ID("bad"))
417+
r.Set(enr.WithEntry("badaddr", id))
418+
r.SetSig(BadIdentityScheme{}, []byte{})
419+
n, _ := enode.New(BadIdentityScheme{}, r)
420+
return n
421+
},
422+
},
423+
} {
424+
t.Run(tt.name, func(t *testing.T) {
425+
// Build ENR node for test.
426+
var (
427+
distance = 230
428+
remote = test.getNode(test.remotekey, test.remoteaddr).Node()
429+
id = idAtDistance(remote.ID(), distance)
430+
r enr.Record
431+
)
432+
r.Set(enr.IP(intIP(i)))
433+
if tt.ip != nil {
434+
r.Set(tt.ip)
435+
}
436+
r.Set(enr.UDP(30303))
437+
if tt.port != nil {
438+
r.Set(tt.port)
439+
}
440+
r = *enode.SignNull(&r, id).Record()
441+
if tt.sign != nil {
442+
r = *tt.sign(&r, id).Record()
443+
}
444+
445+
// Launch findnode request.
446+
var (
447+
done = make(chan error, 1)
448+
got []*enode.Node
449+
)
450+
go func() {
451+
var err error
452+
got, err = test.udp.Findnode(remote, []uint{uint(distance)})
453+
done <- err
454+
}()
455+
456+
// Handle request.
457+
test.waitPacketOut(func(p *v5wire.Findnode, _ netip.AddrPort, _ v5wire.Nonce) {
458+
test.packetIn(&v5wire.Nodes{ReqID: p.ReqID, RespCount: 1, Nodes: []*enr.Record{&r}})
459+
})
460+
if err := <-done; err != nil {
461+
t.Fatalf("unexpected error: %v", err)
462+
}
463+
if len(got) != 0 {
464+
t.Fatalf("expected 0 nodes, got %d", len(got))
465+
}
466+
})
467+
}
384468
}
385469

386470
// This test checks that pending calls are re-sent when a handshake happens.

0 commit comments

Comments
 (0)