cmd/devp2p/internal/ethtest: skip eth/66 tests when v66 not supported (#22460)

cmd/devp2p/internal/ethtest/chain_test.go

@@ -35,7 +35,31 @@ func TestEthProtocolNegotiation(t *testing.T) {
 		expected uint32
 	}{
 		{
-			conn: &Conn{},
+			conn: &Conn{
+				ourHighestProtoVersion: 65,
+			},
+			caps: []p2p.Cap{
+				{Name: "eth", Version: 63},
+				{Name: "eth", Version: 64},
+				{Name: "eth", Version: 65},
+			},
+			expected: uint32(65),
+		},
+		{
+			conn: &Conn{
+				ourHighestProtoVersion: 65,
+			},
+			caps: []p2p.Cap{
+				{Name: "eth", Version: 63},
+				{Name: "eth", Version: 64},
+				{Name: "eth", Version: 65},
+			},
+			expected: uint32(65),
+		},
+		{
+			conn: &Conn{
+				ourHighestProtoVersion: 65,
+			},
 			caps: []p2p.Cap{
 				{Name: "eth", Version: 63},
 				{Name: "eth", Version: 64},
@@ -44,7 +68,20 @@ func TestEthProtocolNegotiation(t *testing.T) {
 			expected: uint32(65),
 		},
 		{
-			conn: &Conn{},
+			conn: &Conn{
+				ourHighestProtoVersion: 64,
+			},
+			caps: []p2p.Cap{
+				{Name: "eth", Version: 63},
+				{Name: "eth", Version: 64},
+				{Name: "eth", Version: 65},
+			},
+			expected: 64,
+		},
+		{
+			conn: &Conn{
+				ourHighestProtoVersion: 65,
+			},
 			caps: []p2p.Cap{
 				{Name: "eth", Version: 0},
 				{Name: "eth", Version: 89},
@@ -53,7 +90,20 @@ func TestEthProtocolNegotiation(t *testing.T) {
 			expected: uint32(65),
 		},
 		{
-			conn: &Conn{},
+			conn: &Conn{
+				ourHighestProtoVersion: 64,
+			},
+			caps: []p2p.Cap{
+				{Name: "eth", Version: 63},
+				{Name: "eth", Version: 64},
+				{Name: "wrongProto", Version: 65},
+			},
+			expected: uint32(64),
+		},
+		{
+			conn: &Conn{
+				ourHighestProtoVersion: 65,
+			},
 			caps: []p2p.Cap{
 				{Name: "eth", Version: 63},
 				{Name: "eth", Version: 64},
@@ -66,7 +116,7 @@ func TestEthProtocolNegotiation(t *testing.T) {
 	for i, tt := range tests {
 		t.Run(strconv.Itoa(i), func(t *testing.T) {
 			tt.conn.negotiateEthProtocol(tt.caps)
-			assert.Equal(t, tt.expected, uint32(tt.conn.ethProtocolVersion))
+			assert.Equal(t, tt.expected, uint32(tt.conn.negotiatedProtoVersion))
 		})
 	}
 }

cmd/devp2p/internal/ethtest/eth66_suite.go

@@ -26,6 +26,16 @@ import (
 	"github.com/ethereum/go-ethereum/p2p"
 )
 
+// Is_66 checks if the node supports the eth66 protocol version,
+// and if not, exists the test suite
+func (s *Suite) Is_66(t *utesting.T) {
+	conn := s.dial66(t)
+	conn.handshake(t)
+	if conn.negotiatedProtoVersion < 66 {
+		t.Fail()
+	}
+}
+
 // TestStatus_66 attempts to connect to the given node and exchange
 // a status message with it on the eth66 protocol, and then check to
 // make sure the chain head is correct.

cmd/devp2p/internal/ethtest/eth66_suiteHelpers.go

@@ -46,6 +46,7 @@ func (s *Suite) dial66(t *utesting.T) *Conn {
 		t.Fatalf("could not dial: %v", err)
 	}
 	conn.caps = append(conn.caps, p2p.Cap{Name: "eth", Version: 66})
+	conn.ourHighestProtoVersion = 66
 	return conn
 }
 

cmd/devp2p/internal/ethtest/suite.go

@@ -65,7 +65,7 @@ func NewSuite(dest *enode.Node, chainfile string, genesisfile string) (*Suite, e
 	}, nil
 }
 
-func (s *Suite) EthTests() []utesting.Test {
+func (s *Suite) AllEthTests() []utesting.Test {
 	return []utesting.Test{
 		// status
 		{Name: "Status", Fn: s.TestStatus},
@@ -97,6 +97,38 @@ func (s *Suite) EthTests() []utesting.Test {
 	}
 }
 
+func (s *Suite) EthTests() []utesting.Test {
+	return []utesting.Test{
+		{Name: "Status", Fn: s.TestStatus},
+		{Name: "GetBlockHeaders", Fn: s.TestGetBlockHeaders},
+		{Name: "GetBlockBodies", Fn: s.TestGetBlockBodies},
+		{Name: "Broadcast", Fn: s.TestBroadcast},
+		{Name: "TestLargeAnnounce", Fn: s.TestLargeAnnounce},
+		{Name: "TestMaliciousHandshake", Fn: s.TestMaliciousHandshake},
+		{Name: "TestMaliciousStatus", Fn: s.TestMaliciousStatus},
+		{Name: "TestMaliciousStatus_66", Fn: s.TestMaliciousStatus},
+		{Name: "TestTransactions", Fn: s.TestTransaction},
+		{Name: "TestMaliciousTransactions", Fn: s.TestMaliciousTx},
+	}
+}
+
+func (s *Suite) Eth66Tests() []utesting.Test {
+	return []utesting.Test{
+		// only proceed with eth66 test suite if node supports eth 66 protocol
+		{Name: "Status_66", Fn: s.TestStatus_66},
+		{Name: "GetBlockHeaders_66", Fn: s.TestGetBlockHeaders_66},
+		{Name: "TestSimultaneousRequests_66", Fn: s.TestSimultaneousRequests_66},
+		{Name: "TestSameRequestID_66", Fn: s.TestSameRequestID_66},
+		{Name: "TestZeroRequestID_66", Fn: s.TestZeroRequestID_66},
+		{Name: "GetBlockBodies_66", Fn: s.TestGetBlockBodies_66},
+		{Name: "Broadcast_66", Fn: s.TestBroadcast_66},
+		{Name: "TestLargeAnnounce_66", Fn: s.TestLargeAnnounce_66},
+		{Name: "TestMaliciousHandshake_66", Fn: s.TestMaliciousHandshake_66},
+		{Name: "TestTransactions_66", Fn: s.TestTransaction_66},
+		{Name: "TestMaliciousTransactions_66", Fn: s.TestMaliciousTx_66},
+	}
+}
+
 // TestStatus attempts to connect to the given node and exchange
 // a status message with it, and then check to make sure
 // the chain head is correct.
@@ -125,7 +157,7 @@ func (s *Suite) TestMaliciousStatus(t *utesting.T) {
 	// get protoHandshake
 	conn.handshake(t)
 	status := &Status{
-		ProtocolVersion: uint32(conn.ethProtocolVersion),
+		ProtocolVersion: uint32(conn.negotiatedProtoVersion),
 		NetworkID:       s.chain.chainConfig.ChainID.Uint64(),
 		TD:              largeNumber(2),
 		Head:            s.chain.blocks[s.chain.Len()-1].Hash(),
@@ -421,6 +453,7 @@ func (s *Suite) dial() (*Conn, error) {
 		{Name: "eth", Version: 64},
 		{Name: "eth", Version: 65},
 	}
+	conn.ourHighestProtoVersion = 65
 	return &conn, nil
 }
 

cmd/devp2p/internal/ethtest/types.go

@@ -123,9 +123,10 @@ func (nb NewPooledTransactionHashes) Code() int { return 24 }
 // Conn represents an individual connection with a peer
 type Conn struct {
 	*rlpx.Conn
-	ourKey             *ecdsa.PrivateKey
-	ethProtocolVersion uint
-	caps               []p2p.Cap
+	ourKey                 *ecdsa.PrivateKey
+	negotiatedProtoVersion uint
+	ourHighestProtoVersion uint
+	caps                   []p2p.Cap
 }
 
 func (c *Conn) Read() Message {
@@ -236,7 +237,7 @@ func (c *Conn) handshake(t *utesting.T) Message {
 			c.SetSnappy(true)
 		}
 		c.negotiateEthProtocol(msg.Caps)
-		if c.ethProtocolVersion == 0 {
+		if c.negotiatedProtoVersion == 0 {
 			t.Fatalf("unexpected eth protocol version")
 		}
 		return msg
@@ -254,11 +255,11 @@ func (c *Conn) negotiateEthProtocol(caps []p2p.Cap) {
 		if capability.Name != "eth" {
 			continue
 		}
-		if capability.Version > highestEthVersion && capability.Version <= 65 {
+		if capability.Version > highestEthVersion && capability.Version <= c.ourHighestProtoVersion {
 			highestEthVersion = capability.Version
 		}
 	}
-	c.ethProtocolVersion = highestEthVersion
+	c.negotiatedProtoVersion = highestEthVersion
 }
 
 // statusExchange performs a `Status` message exchange with the given
@@ -295,13 +296,13 @@ loop:
 		}
 	}
 	// make sure eth protocol version is set for negotiation
-	if c.ethProtocolVersion == 0 {
+	if c.negotiatedProtoVersion == 0 {
 		t.Fatalf("eth protocol version must be set in Conn")
 	}
 	if status == nil {
 		// write status message to client
 		status = &Status{
-			ProtocolVersion: uint32(c.ethProtocolVersion),
+			ProtocolVersion: uint32(c.negotiatedProtoVersion),
 			NetworkID:       chain.chainConfig.ChainID.Uint64(),
 			TD:              chain.TD(chain.Len()),
 			Head:            chain.blocks[chain.Len()-1].Hash(),

cmd/devp2p/rlpxcmd.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/ethereum/go-ethereum/cmd/devp2p/internal/ethtest"
 	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/ethereum/go-ethereum/internal/utesting"
 	"github.com/ethereum/go-ethereum/p2p"
 	"github.com/ethereum/go-ethereum/p2p/rlpx"
 	"github.com/ethereum/go-ethereum/rlp"
@@ -98,5 +99,10 @@ func rlpxEthTest(ctx *cli.Context) error {
 	if err != nil {
 		exit(err)
 	}
-	return runTests(ctx, suite.EthTests())
+	// check if given node supports eth66, and if so, run eth66 protocol tests as well
+	is66Failed, _ := utesting.Run(utesting.Test{Name: "Is_66", Fn: suite.Is_66})
+	if is66Failed {
+		return runTests(ctx, suite.EthTests())
+	}
+	return runTests(ctx, suite.AllEthTests())
 }