cmd/devp2p/internal/ethtest: add correct chain files and improve test output (#21782)

This PR replaces the old test genesis.json and chain.rlp files in the testdata
directory for the eth protocol test suite, and also adds documentation for
running the eth test suite locally.

It also improves the test output text and adds more timeouts.

Co-authored-by: Felix Lange <fjl@twurst.com>

cmd/devp2p/README.md

@@ -81,6 +81,25 @@ Now get the ENR of your node and store it in the `NODE` environment variable.
 
 Start the test by running `devp2p discv5 test -listen1 127.0.0.1 -listen2 127.0.0.2 $NODE`.
 
+### Eth Protocol Test Suite
+
+The Eth Protocol test suite is a conformance test suite for the [eth protocol][eth].
+
+To run the eth protocol test suite against your implementation, the node needs to be initialized as such:
+
+1. initialize the geth node with the `genesis.json` file contained in the `testdata` directory
+2. import the `halfchain.rlp` file in the `testdata` directory
+3. run geth with the following flags:
+```
+geth --datadir <datadir> --nodiscover --nat=none --networkid 19763 --verbosity 5
+```
+
+Then, run the following command, replacing `<enode ID>` with the enode of the geth node: 
+ ```
+ devp2p rlpx eth-test <enode ID> cmd/devp2p/internal/ethtest/testdata/fullchain.rlp cmd/devp2p/internal/ethtest/testdata/genesis.json
+```
+ 
+[eth]: https://github.com/ethereum/devp2p/blob/master/caps/eth.md
 [dns-tutorial]: https://geth.ethereum.org/docs/developers/dns-discovery-setup
 [discv4]: https://github.com/ethereum/devp2p/tree/master/discv4.md
 [discv5]: https://github.com/ethereum/devp2p/tree/master/discv5/discv5.md

cmd/devp2p/internal/ethtest/chain_test.go

@@ -73,7 +73,7 @@ func TestEthProtocolNegotiation(t *testing.T) {
 // TestChain_GetHeaders tests whether the test suite can correctly
 // respond to a GetBlockHeaders request from a node.
 func TestChain_GetHeaders(t *testing.T) {
-	chainFile, err := filepath.Abs("./testdata/chain.rlp.gz")
+	chainFile, err := filepath.Abs("./testdata/fullchain.rlp.gz")
 	if err != nil {
 		t.Fatal(err)
 	}

cmd/devp2p/internal/ethtest/suite.go

@@ -19,7 +19,9 @@ package ethtest
 import (
 	"fmt"
 	"net"
+	"time"
 
+	"github.com/davecgh/go-spew/spew"
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/internal/utesting"
 	"github.com/ethereum/go-ethereum/p2p/enode"
@@ -27,6 +29,13 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+var pretty = spew.ConfigState{
+	Indent:                  "  ",
+	DisableCapacities:       true,
+	DisablePointerAddresses: true,
+	SortKeys:                true,
+}
+
 // Suite represents a structure used to test the eth
 // protocol of a node(s).
 type Suite struct {
@@ -73,9 +82,9 @@ func (s *Suite) TestStatus(t *utesting.T) {
 	// get status
 	switch msg := conn.statusExchange(t, s.chain).(type) {
 	case *Status:
-		t.Logf("%+v\n", msg)
+		t.Logf("got status message: %s", pretty.Sdump(msg))
 	default:
-		t.Fatalf("unexpected: %#v", msg)
+		t.Fatalf("unexpected: %s", pretty.Sdump(msg))
 	}
 }
 
@@ -104,16 +113,17 @@ func (s *Suite) TestGetBlockHeaders(t *utesting.T) {
 		t.Fatalf("could not write to connection: %v", err)
 	}
 
-	switch msg := conn.ReadAndServe(s.chain).(type) {
+	timeout := 20 * time.Second
+	switch msg := conn.ReadAndServe(s.chain, timeout).(type) {
 	case *BlockHeaders:
 		headers := msg
 		for _, header := range *headers {
 			num := header.Number.Uint64()
+			t.Logf("received header (%d): %s", num, pretty.Sdump(header))
 			assert.Equal(t, s.chain.blocks[int(num)].Header(), header)
-			t.Logf("\nHEADER FOR BLOCK NUMBER %d: %+v\n", header.Number, header)
 		}
 	default:
-		t.Fatalf("unexpected: %#v", msg)
+		t.Fatalf("unexpected: %s", pretty.Sdump(msg))
 	}
 }
 
@@ -133,14 +143,12 @@ func (s *Suite) TestGetBlockBodies(t *utesting.T) {
 		t.Fatalf("could not write to connection: %v", err)
 	}
 
-	switch msg := conn.ReadAndServe(s.chain).(type) {
+	timeout := 20 * time.Second
+	switch msg := conn.ReadAndServe(s.chain, timeout).(type) {
 	case *BlockBodies:
-		bodies := msg
-		for _, body := range *bodies {
-			t.Logf("\nBODY: %+v\n", body)
-		}
+		t.Logf("received %d block bodies", len(*msg))
 	default:
-		t.Fatalf("unexpected: %#v", msg)
+		t.Fatalf("unexpected: %s", pretty.Sdump(msg))
 	}
 }
 
@@ -173,18 +181,27 @@ func (s *Suite) TestBroadcast(t *utesting.T) {
 		t.Fatalf("could not write to connection: %v", err)
 	}
 
-	switch msg := receiveConn.ReadAndServe(s.chain).(type) {
+	timeout := 20 * time.Second
+	switch msg := receiveConn.ReadAndServe(s.chain, timeout).(type) {
 	case *NewBlock:
-		assert.Equal(t, blockAnnouncement.Block.Header(), msg.Block.Header(),
-			"wrong block header in announcement")
-		assert.Equal(t, blockAnnouncement.TD, msg.TD,
-			"wrong TD in announcement")
+		t.Logf("received NewBlock message: %s", pretty.Sdump(msg.Block))
+		assert.Equal(t,
+			blockAnnouncement.Block.Header(), msg.Block.Header(),
+			"wrong block header in announcement",
+		)
+		assert.Equal(t,
+			blockAnnouncement.TD, msg.TD,
+			"wrong TD in announcement",
+		)
 	case *NewBlockHashes:
 		hashes := *msg
-		assert.Equal(t, blockAnnouncement.Block.Hash(), hashes[0].Hash,
-			"wrong block hash in announcement")
+		t.Logf("received NewBlockHashes message: %s", pretty.Sdump(hashes))
+		assert.Equal(t,
+			blockAnnouncement.Block.Hash(), hashes[0].Hash,
+			"wrong block hash in announcement",
+		)
 	default:
-		t.Fatalf("unexpected: %#v", msg)
+		t.Fatalf("unexpected: %s", pretty.Sdump(msg))
 	}
 	// update test suite chain
 	s.chain.blocks = append(s.chain.blocks, s.fullChain.blocks[1000])

cmd/devp2p/internal/ethtest/types.go

@@ -42,10 +42,14 @@ type Error struct {
 	err error
 }
 
-func (e *Error) Unwrap() error    { return e.err }
-func (e *Error) Error() string    { return e.err.Error() }
-func (e *Error) Code() int        { return -1 }
-func (e *Error) GoString() string { return e.Error() }
+func (e *Error) Unwrap() error  { return e.err }
+func (e *Error) Error() string  { return e.err.Error() }
+func (e *Error) Code() int      { return -1 }
+func (e *Error) String() string { return e.Error() }
+
+func errorf(format string, args ...interface{}) *Error {
+	return &Error{fmt.Errorf(format, args...)}
+}
 
 // Hello is the RLP structure of the protocol handshake.
 type Hello struct {
@@ -174,7 +178,7 @@ type Conn struct {
 func (c *Conn) Read() Message {
 	code, rawData, _, err := c.Conn.Read()
 	if err != nil {
-		return &Error{fmt.Errorf("could not read from connection: %v", err)}
+		return errorf("could not read from connection: %v", err)
 	}
 
 	var msg Message
@@ -202,20 +206,22 @@ func (c *Conn) Read() Message {
 	case (NewBlockHashes{}).Code():
 		msg = new(NewBlockHashes)
 	default:
-		return &Error{fmt.Errorf("invalid message code: %d", code)}
+		return errorf("invalid message code: %d", code)
 	}
 
 	if err := rlp.DecodeBytes(rawData, msg); err != nil {
-		return &Error{fmt.Errorf("could not rlp decode message: %v", err)}
+		return errorf("could not rlp decode message: %v", err)
 	}
-
 	return msg
 }
 
 // ReadAndServe serves GetBlockHeaders requests while waiting
 // on another message from the node.
-func (c *Conn) ReadAndServe(chain *Chain) Message {
-	for {
+func (c *Conn) ReadAndServe(chain *Chain, timeout time.Duration) Message {
+	start := time.Now()
+	for time.Since(start) < timeout {
+		timeout := time.Now().Add(10 * time.Second)
+		c.SetReadDeadline(timeout)
 		switch msg := c.Read().(type) {
 		case *Ping:
 			c.Write(&Pong{})
@@ -223,16 +229,17 @@ func (c *Conn) ReadAndServe(chain *Chain) Message {
 			req := *msg
 			headers, err := chain.GetHeaders(req)
 			if err != nil {
-				return &Error{fmt.Errorf("could not get headers for inbound header request: %v", err)}
+				return errorf("could not get headers for inbound header request: %v", err)
 			}
 
 			if err := c.Write(headers); err != nil {
-				return &Error{fmt.Errorf("could not write to connection: %v", err)}
+				return errorf("could not write to connection: %v", err)
 			}
 		default:
 			return msg
 		}
 	}
+	return errorf("no message received within %v", timeout)
 }
 
 func (c *Conn) Write(msg Message) error {
@@ -308,7 +315,7 @@ loop:
 		switch msg := c.Read().(type) {
 		case *Status:
 			if msg.Head != chain.blocks[chain.Len()-1].Hash() {
-				t.Fatalf("wrong head in status: %v", msg.Head)
+				t.Fatalf("wrong head block in status: %s", msg.Head.String())
 			}
 			if msg.TD.Cmp(chain.TD(chain.Len())) != 0 {
 				t.Fatalf("wrong TD in status: %v", msg.TD)
@@ -324,7 +331,7 @@ loop:
 			c.Write(&Pong{}) // TODO (renaynay): in the future, this should be an error
 			// (PINGs should not be a response upon fresh connection)
 		default:
-			t.Fatalf("bad status message: %#v", msg)
+			t.Fatalf("bad status message: %s", pretty.Sdump(msg))
 		}
 	}
 	// make sure eth protocol version is set for negotiation
@@ -366,7 +373,7 @@ func (c *Conn) waitForBlock(block *types.Block) error {
 			}
 			time.Sleep(100 * time.Millisecond)
 		default:
-			return fmt.Errorf("invalid message: %v", msg)
+			return fmt.Errorf("invalid message: %s", pretty.Sdump(msg))
 		}
 	}
 }