cmd/devp2p/internal/ethtest: tests for BlockRangeUpdate (#31843)

I added a test for BlockRangeUpdate in #29158 but forgot to enable it.
Here I'm adding two more tests for it. Also applied a small refactoring
to combine calls to `dial()` and `peer()` into a single function, since
these two calls are duplicated in each test.

Author: fjl

cmd/devp2p/internal/ethtest/conn.go

@@ -228,6 +228,18 @@ func (c *Conn) ReadSnap() (any, error) {
 	}
 }
 
+// dialAndPeer creates a peer connection and runs the handshake.
+func (s *Suite) dialAndPeer(status *eth.StatusPacket69) (*Conn, error) {
+	c, err := s.dial()
+	if err != nil {
+		return nil, err
+	}
+	if err = c.peer(s.chain, status); err != nil {
+		c.Close()
+	}
+	return c, err
+}
+
 // peer performs both the protocol handshake and the status message
 // exchange with the node in order to peer with it.
 func (c *Conn) peer(chain *Chain, status *eth.StatusPacket69) error {

cmd/devp2p/internal/ethtest/suite.go

@@ -68,6 +68,10 @@ func (s *Suite) EthTests() []utesting.Test {
 	return []utesting.Test{
 		// status
 		{Name: "Status", Fn: s.TestStatus},
+		{Name: "MaliciousHandshake", Fn: s.TestMaliciousHandshake},
+		{Name: "BlockRangeUpdateExpired", Fn: s.TestBlockRangeUpdateHistoryExp},
+		{Name: "BlockRangeUpdateFuture", Fn: s.TestBlockRangeUpdateFuture},
+		{Name: "BlockRangeUpdateInvalid", Fn: s.TestBlockRangeUpdateInvalid},
 		// get block headers
 		{Name: "GetBlockHeaders", Fn: s.TestGetBlockHeaders},
 		{Name: "GetNonexistentBlockHeaders", Fn: s.TestGetNonexistentBlockHeaders},
@@ -77,8 +81,6 @@ func (s *Suite) EthTests() []utesting.Test {
 		// get history
 		{Name: "GetBlockBodies", Fn: s.TestGetBlockBodies},
 		{Name: "GetReceipts", Fn: s.TestGetReceipts},
-		// // malicious handshakes + status
-		{Name: "MaliciousHandshake", Fn: s.TestMaliciousHandshake},
 		// test transactions
 		{Name: "LargeTxRequest", Fn: s.TestLargeTxRequest, Slow: true},
 		{Name: "Transaction", Fn: s.TestTransaction},
@@ -102,15 +104,11 @@ func (s *Suite) SnapTests() []utesting.Test {
 
 func (s *Suite) TestStatus(t *utesting.T) {
 	t.Log(`This test is just a sanity check. It performs an eth protocol handshake.`)
-
-	conn, err := s.dial()
+	conn, err := s.dialAndPeer(nil)
 	if err != nil {
-		t.Fatalf("dial failed: %v", err)
-	}
-	defer conn.Close()
-	if err := conn.peer(s.chain, nil); err != nil {
-		t.Fatalf("peering failed: %v", err)
+		t.Fatal("peering failed:", err)
 	}
+	conn.Close()
 }
 
 // headersMatch returns whether the received headers match the given request
@@ -120,15 +118,12 @@ func headersMatch(expected []*types.Header, headers []*types.Header) bool {
 
 func (s *Suite) TestGetBlockHeaders(t *utesting.T) {
 	t.Log(`This test requests block headers from the node.`)
-
-	conn, err := s.dial()
+	conn, err := s.dialAndPeer(nil)
 	if err != nil {
-		t.Fatalf("dial failed: %v", err)
-	}
-	defer conn.Close()
-	if err = conn.peer(s.chain, nil); err != nil {
 		t.Fatalf("peering failed: %v", err)
 	}
+	defer conn.Close()
+
 	// Send headers request.
 	req := &eth.GetBlockHeadersPacket{
 		RequestId: 33,
@@ -161,18 +156,13 @@ func (s *Suite) TestGetBlockHeaders(t *utesting.T) {
 }
 
 func (s *Suite) TestGetNonexistentBlockHeaders(t *utesting.T) {
-	t.Log(`This test sends GetBlockHeaders requests for nonexistent blocks (using max uint64 value) 
+	t.Log(`This test sends GetBlockHeaders requests for nonexistent blocks (using max uint64 value)
 to check if the node disconnects after receiving multiple invalid requests.`)
-
-	conn, err := s.dial()
+	conn, err := s.dialAndPeer(nil)
 	if err != nil {
-		t.Fatalf("dial failed: %v", err)
-	}
-	defer conn.Close()
-
-	if err := conn.peer(s.chain, nil); err != nil {
 		t.Fatalf("peering failed: %v", err)
 	}
+	defer conn.Close()
 
 	// Create request with max uint64 value for a nonexistent block
 	badReq := &eth.GetBlockHeadersPacket{
@@ -205,15 +195,11 @@ to check if the node disconnects after receiving multiple invalid requests.`)
 func (s *Suite) TestSimultaneousRequests(t *utesting.T) {
 	t.Log(`This test requests blocks headers from the node, performing two requests
 concurrently, with different request IDs.`)
-
-	conn, err := s.dial()
+	conn, err := s.dialAndPeer(nil)
 	if err != nil {
-		t.Fatalf("dial failed: %v", err)
-	}
-	defer conn.Close()
-	if err := conn.peer(s.chain, nil); err != nil {
 		t.Fatalf("peering failed: %v", err)
 	}
+	defer conn.Close()
 
 	// Create two different requests.
 	req1 := &eth.GetBlockHeadersPacket{
@@ -279,15 +265,11 @@ concurrently, with different request IDs.`)
 func (s *Suite) TestSameRequestID(t *utesting.T) {
 	t.Log(`This test requests block headers, performing two concurrent requests with the
 same request ID. The node should handle the request by responding to both requests.`)
-
-	conn, err := s.dial()
+	conn, err := s.dialAndPeer(nil)
 	if err != nil {
-		t.Fatalf("dial failed: %v", err)
-	}
-	defer conn.Close()
-	if err := conn.peer(s.chain, nil); err != nil {
 		t.Fatalf("peering failed: %v", err)
 	}
+	defer conn.Close()
 
 	// Create two different requests with the same ID.
 	reqID := uint64(1234)
@@ -350,15 +332,12 @@ same request ID. The node should handle the request by responding to both reques
 func (s *Suite) TestZeroRequestID(t *utesting.T) {
 	t.Log(`This test sends a GetBlockHeaders message with a request-id of zero,
 and expects a response.`)
-
-	conn, err := s.dial()
+	conn, err := s.dialAndPeer(nil)
 	if err != nil {
-		t.Fatalf("dial failed: %v", err)
-	}
-	defer conn.Close()
-	if err := conn.peer(s.chain, nil); err != nil {
 		t.Fatalf("peering failed: %v", err)
 	}
+	defer conn.Close()
+
 	req := &eth.GetBlockHeadersPacket{
 		GetBlockHeadersRequest: &eth.GetBlockHeadersRequest{
 			Origin: eth.HashOrNumber{Number: 0},
@@ -385,15 +364,12 @@ and expects a response.`)
 
 func (s *Suite) TestGetBlockBodies(t *utesting.T) {
 	t.Log(`This test sends GetBlockBodies requests to the node for known blocks in the test chain.`)
-
-	conn, err := s.dial()
+	conn, err := s.dialAndPeer(nil)
 	if err != nil {
-		t.Fatalf("dial failed: %v", err)
-	}
-	defer conn.Close()
-	if err := conn.peer(s.chain, nil); err != nil {
 		t.Fatalf("peering failed: %v", err)
 	}
+	defer conn.Close()
+
 	// Create block bodies request.
 	req := &eth.GetBlockBodiesPacket{
 		RequestId: 55,
@@ -421,15 +397,11 @@ func (s *Suite) TestGetBlockBodies(t *utesting.T) {
 
 func (s *Suite) TestGetReceipts(t *utesting.T) {
 	t.Log(`This test sends GetReceipts requests to the node for known blocks in the test chain.`)
-
-	conn, err := s.dial()
+	conn, err := s.dialAndPeer(nil)
 	if err != nil {
-		t.Fatalf("dial failed: %v", err)
-	}
-	defer conn.Close()
-	if err := conn.peer(s.chain, nil); err != nil {
 		t.Fatalf("peering failed: %v", err)
 	}
+	defer conn.Close()
 
 	// Find some blocks containing receipts.
 	var hashes = make([]common.Hash, 0, 3)
@@ -546,17 +518,13 @@ func (s *Suite) TestMaliciousHandshake(t *utesting.T) {
 	}
 }
 
-func (s *Suite) TestInvalidBlockRangeUpdate(t *utesting.T) {
+func (s *Suite) TestBlockRangeUpdateInvalid(t *utesting.T) {
 	t.Log(`This test sends an invalid BlockRangeUpdate message to the node and expects to be disconnected.`)
-
-	conn, err := s.dial()
+	conn, err := s.dialAndPeer(nil)
 	if err != nil {
-		t.Fatalf("dial failed: %v", err)
+		t.Fatal(err)
 	}
 	defer conn.Close()
-	if err := conn.peer(s.chain, nil); err != nil {
-		t.Fatalf("peering failed: %v", err)
-	}
 
 	conn.Write(ethProto, eth.BlockRangeUpdateMsg, &eth.BlockRangeUpdatePacket{
 		EarliestBlock:   10,
@@ -571,6 +539,76 @@ func (s *Suite) TestInvalidBlockRangeUpdate(t *utesting.T) {
 	}
 }
 
+func (s *Suite) TestBlockRangeUpdateFuture(t *utesting.T) {
+	t.Log(`This test sends a BlockRangeUpdate that is beyond the chain head.
+The node should accept the update and should not disonnect.`)
+	conn, err := s.dialAndPeer(nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer conn.Close()
+
+	head := s.chain.Head().NumberU64()
+	var hash common.Hash
+	rand.Read(hash[:])
+	conn.Write(ethProto, eth.BlockRangeUpdateMsg, &eth.BlockRangeUpdatePacket{
+		EarliestBlock:   head + 10,
+		LatestBlock:     head + 50,
+		LatestBlockHash: hash,
+	})
+
+	// Ensure the node does not disconnect us.
+	// Just send a few ping messages.
+	for range 10 {
+		time.Sleep(100 * time.Millisecond)
+		if err := conn.Write(baseProto, pingMsg, []any{}); err != nil {
+			t.Fatal("write error:", err)
+		}
+		code, _, err := conn.Read()
+		switch {
+		case err != nil:
+			t.Fatal("read error:", err)
+		case code == discMsg:
+			t.Fatal("got disconnect")
+		case code == pongMsg:
+		}
+	}
+}
+
+func (s *Suite) TestBlockRangeUpdateHistoryExp(t *utesting.T) {
+	t.Log(`This test sends a BlockRangeUpdate announcing incomplete (expired) history.
+The node should accept the update and should not disonnect.`)
+	conn, err := s.dialAndPeer(nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer conn.Close()
+
+	head := s.chain.Head()
+	conn.Write(ethProto, eth.BlockRangeUpdateMsg, &eth.BlockRangeUpdatePacket{
+		EarliestBlock:   head.NumberU64() - 10,
+		LatestBlock:     head.NumberU64(),
+		LatestBlockHash: head.Hash(),
+	})
+
+	// Ensure the node does not disconnect us.
+	// Just send a few ping messages.
+	for range 10 {
+		time.Sleep(100 * time.Millisecond)
+		if err := conn.Write(baseProto, pingMsg, []any{}); err != nil {
+			t.Fatal("write error:", err)
+		}
+		code, _, err := conn.Read()
+		switch {
+		case err != nil:
+			t.Fatal("read error:", err)
+		case code == discMsg:
+			t.Fatal("got disconnect")
+		case code == pongMsg:
+		}
+	}
+}
+
 func (s *Suite) TestTransaction(t *utesting.T) {
 	t.Log(`This test sends a valid transaction to the node and checks if the
 transaction gets propagated.`)