Skip to content

Commit

Permalink
cmd/devp2p/internal/ethtest: some fixes for the eth test suite (ether…
Browse files Browse the repository at this point in the history 
…eum#28996)

Improving two things here:

On hive, where we look at these tests, the Go code comment above the test
is not visible. When there is a failure, it's not obvious what the test is actually
expecting. I have converted the comments in to printed log messages to
explain the test more.

Second, I noticed that besu is failing some tests because it happens to request
a header when we want it to send transactions. Trying the minimal fix here to
serve the headers.

Co-authored-by: lightclient <[email protected]>
  • Loading branch information
@fjl @lightclient
fjl and lightclient authored Feb 15, 2024
1 parent 0c412dc commit 1bdf8b9
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 43 deletions.
92 changes: 51 additions & 41 deletions cmd/devp2p/internal/ethtest/suite.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,23 +64,23 @@ func NewSuite(dest *enode.Node, chainDir, engineURL, jwt string) (*Suite, error)
func (s *Suite) EthTests() []utesting.Test {
return []utesting.Test{
// status
{Name: "TestStatus", Fn: s.TestStatus},
{Name: "Status", Fn: s.TestStatus},
// get block headers
{Name: "TestGetBlockHeaders", Fn: s.TestGetBlockHeaders},
{Name: "TestSimultaneousRequests", Fn: s.TestSimultaneousRequests},
{Name: "TestSameRequestID", Fn: s.TestSameRequestID},
{Name: "TestZeroRequestID", Fn: s.TestZeroRequestID},
{Name: "GetBlockHeaders", Fn: s.TestGetBlockHeaders},
{Name: "SimultaneousRequests", Fn: s.TestSimultaneousRequests},
{Name: "SameRequestID", Fn: s.TestSameRequestID},
{Name: "ZeroRequestID", Fn: s.TestZeroRequestID},
// get block bodies
{Name: "TestGetBlockBodies", Fn: s.TestGetBlockBodies},
{Name: "GetBlockBodies", Fn: s.TestGetBlockBodies},
// // malicious handshakes + status
{Name: "TestMaliciousHandshake", Fn: s.TestMaliciousHandshake},
{Name: "TestMaliciousStatus", Fn: s.TestMaliciousStatus},
{Name: "MaliciousHandshake", Fn: s.TestMaliciousHandshake},
{Name: "MaliciousStatus", Fn: s.TestMaliciousStatus},
// test transactions
{Name: "TestLargeTxRequest", Fn: s.TestLargeTxRequest, Slow: true},
{Name: "TestTransaction", Fn: s.TestTransaction},
{Name: "TestInvalidTxs", Fn: s.TestInvalidTxs},
{Name: "TestNewPooledTxs", Fn: s.TestNewPooledTxs},
{Name: "TestBlobViolations", Fn: s.TestBlobViolations},
{Name: "LargeTxRequest", Fn: s.TestLargeTxRequest, Slow: true},
{Name: "Transaction", Fn: s.TestTransaction},
{Name: "InvalidTxs", Fn: s.TestInvalidTxs},
{Name: "NewPooledTxs", Fn: s.TestNewPooledTxs},
{Name: "BlobViolations", Fn: s.TestBlobViolations},
}
}

Expand All @@ -94,9 +94,9 @@ func (s *Suite) SnapTests() []utesting.Test {
}
}

// TestStatus attempts to connect to the given node and exchange a status
// message with it on the eth protocol.
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()
if err != nil {
t.Fatalf("dial failed: %v", err)
Expand All @@ -112,9 +112,9 @@ func headersMatch(expected []*types.Header, headers []*types.Header) bool {
return reflect.DeepEqual(expected, headers)
}

// TestGetBlockHeaders tests whether the given node can respond to an eth
// `GetBlockHeaders` request and that the response is accurate.
func (s *Suite) TestGetBlockHeaders(t *utesting.T) {
t.Log(`This test requests block headers from the node.`)

conn, err := s.dial()
if err != nil {
t.Fatalf("dial failed: %v", err)
Expand Down Expand Up @@ -154,10 +154,10 @@ func (s *Suite) TestGetBlockHeaders(t *utesting.T) {
}
}

// TestSimultaneousRequests sends two simultaneous `GetBlockHeader` requests
// from the same connection with different request IDs and checks to make sure
// the node responds with the correct headers per request.
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()
if err != nil {
t.Fatalf("dial failed: %v", err)
Expand Down Expand Up @@ -228,9 +228,10 @@ func (s *Suite) TestSimultaneousRequests(t *utesting.T) {
}
}

// TestSameRequestID sends two requests with the same request ID to a single
// node.
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()
if err != nil {
t.Fatalf("dial failed: %v", err)
Expand Down Expand Up @@ -298,9 +299,10 @@ func (s *Suite) TestSameRequestID(t *utesting.T) {
}
}

// TestZeroRequestID checks that a message with a request ID of zero is still handled
// by the node.
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()
if err != nil {
t.Fatalf("dial failed: %v", err)
Expand Down Expand Up @@ -333,9 +335,9 @@ func (s *Suite) TestZeroRequestID(t *utesting.T) {
}
}

// TestGetBlockBodies tests whether the given node can respond to a
// `GetBlockBodies` request and that the response is accurate.
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()
if err != nil {
t.Fatalf("dial failed: %v", err)
Expand Down Expand Up @@ -376,12 +378,12 @@ func randBuf(size int) []byte {
return buf
}

// TestMaliciousHandshake tries to send malicious data during the handshake.
func (s *Suite) TestMaliciousHandshake(t *utesting.T) {
key, _ := crypto.GenerateKey()
t.Log(`This test tries to send malicious data during the devp2p handshake, in various ways.`)

// Write hello to client.
var (
key, _ = crypto.GenerateKey()
pub0 = crypto.FromECDSAPub(&key.PublicKey)[1:]
version = eth.ProtocolVersions[0]
)
Expand Down Expand Up @@ -451,8 +453,9 @@ func (s *Suite) TestMaliciousHandshake(t *utesting.T) {
}
}

// TestMaliciousStatus sends a status package with a large total difficulty.
func (s *Suite) TestMaliciousStatus(t *utesting.T) {
t.Log(`This test sends a malicious eth Status message to the node and expects a disconnect.`)

conn, err := s.dial()
if err != nil {
t.Fatalf("dial failed: %v", err)
Expand Down Expand Up @@ -486,9 +489,10 @@ func (s *Suite) TestMaliciousStatus(t *utesting.T) {
}
}

// TestTransaction sends a valid transaction to the node and checks if the
// transaction gets propagated.
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.`)

// Nudge client out of syncing mode to accept pending txs.
if err := s.engine.sendForkchoiceUpdated(); err != nil {
t.Fatalf("failed to send next block: %v", err)
Expand All @@ -507,15 +511,16 @@ func (s *Suite) TestTransaction(t *utesting.T) {
if err != nil {
t.Fatalf("failed to sign tx: %v", err)
}
if err := s.sendTxs([]*types.Transaction{tx}); err != nil {
if err := s.sendTxs(t, []*types.Transaction{tx}); err != nil {
t.Fatal(err)
}
s.chain.IncNonce(from, 1)
}

// TestInvalidTxs sends several invalid transactions and tests whether
// the node will propagate them.
func (s *Suite) TestInvalidTxs(t *utesting.T) {
t.Log(`This test sends several kinds of invalid transactions and checks that the node
does not propagate them.`)

// Nudge client out of syncing mode to accept pending txs.
if err := s.engine.sendForkchoiceUpdated(); err != nil {
t.Fatalf("failed to send next block: %v", err)
Expand All @@ -534,7 +539,7 @@ func (s *Suite) TestInvalidTxs(t *utesting.T) {
if err != nil {
t.Fatalf("failed to sign tx: %v", err)
}
if err := s.sendTxs([]*types.Transaction{tx}); err != nil {
if err := s.sendTxs(t, []*types.Transaction{tx}); err != nil {
t.Fatalf("failed to send txs: %v", err)
}
s.chain.IncNonce(from, 1)
Expand Down Expand Up @@ -590,14 +595,15 @@ func (s *Suite) TestInvalidTxs(t *utesting.T) {
}
txs = append(txs, tx)
}
if err := s.sendInvalidTxs(txs); err != nil {
if err := s.sendInvalidTxs(t, txs); err != nil {
t.Fatalf("failed to send invalid txs: %v", err)
}
}

// TestLargeTxRequest tests whether a node can fulfill a large GetPooledTransactions
// request.
func (s *Suite) TestLargeTxRequest(t *utesting.T) {
t.Log(`This test first send ~2000 transactions to the node, then requests them
on another peer connection using GetPooledTransactions.`)

// Nudge client out of syncing mode to accept pending txs.
if err := s.engine.sendForkchoiceUpdated(); err != nil {
t.Fatalf("failed to send next block: %v", err)
Expand Down Expand Up @@ -630,7 +636,7 @@ func (s *Suite) TestLargeTxRequest(t *utesting.T) {
s.chain.IncNonce(from, uint64(count))

// Send txs.
if err := s.sendTxs(txs); err != nil {
if err := s.sendTxs(t, txs); err != nil {
t.Fatalf("failed to send txs: %v", err)
}

Expand Down Expand Up @@ -667,13 +673,15 @@ func (s *Suite) TestLargeTxRequest(t *utesting.T) {
}
}

// TestNewPooledTxs tests whether a node will do a GetPooledTransactions request
// upon receiving a NewPooledTransactionHashes announcement.
func (s *Suite) TestNewPooledTxs(t *utesting.T) {
t.Log(`This test announces transaction hashes to the node and expects it to fetch
the transactions using a GetPooledTransactions request.`)

// Nudge client out of syncing mode to accept pending txs.
if err := s.engine.sendForkchoiceUpdated(); err != nil {
t.Fatalf("failed to send next block: %v", err)
}

var (
count = 50
from, nonce = s.chain.GetSender(1)
Expand Down Expand Up @@ -787,6 +795,8 @@ func (s *Suite) makeBlobTxs(count, blobs int, discriminator byte) (txs types.Tra
}

func (s *Suite) TestBlobViolations(t *utesting.T) {
t.Log(`This test sends some invalid blob tx announcements and expects the node to disconnect.`)

if err := s.engine.sendForkchoiceUpdated(); err != nil {
t.Fatalf("send fcu failed: %v", err)
}
Expand Down
23 changes: 21 additions & 2 deletions cmd/devp2p/internal/ethtest/transaction.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ import (
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/eth/protocols/eth"
"github.com/ethereum/go-ethereum/internal/utesting"
)

// sendTxs sends the given transactions to the node and
// expects the node to accept and propagate them.
func (s *Suite) sendTxs(txs []*types.Transaction) error {
func (s *Suite) sendTxs(t *utesting.T, txs []*types.Transaction) error {
// Open sending conn.
sendConn, err := s.dial()
if err != nil {
Expand Down Expand Up @@ -74,6 +75,15 @@ func (s *Suite) sendTxs(txs []*types.Transaction) error {
for _, hash := range msg.Hashes {
got[hash] = true
}
case *eth.GetBlockHeadersPacket:
headers, err := s.chain.GetHeaders(msg)
if err != nil {
t.Logf("invalid GetBlockHeaders request: %v", err)
}
recvConn.Write(ethProto, eth.BlockHeadersMsg, &eth.BlockHeadersPacket{
RequestId: msg.RequestId,
BlockHeadersRequest: headers,
})
default:
return fmt.Errorf("unexpected eth wire msg: %s", pretty.Sdump(msg))
}
Expand All @@ -95,7 +105,7 @@ func (s *Suite) sendTxs(txs []*types.Transaction) error {
return fmt.Errorf("timed out waiting for txs")
}

func (s *Suite) sendInvalidTxs(txs []*types.Transaction) error {
func (s *Suite) sendInvalidTxs(t *utesting.T, txs []*types.Transaction) error {
// Open sending conn.
sendConn, err := s.dial()
if err != nil {
Expand Down Expand Up @@ -152,6 +162,15 @@ func (s *Suite) sendInvalidTxs(txs []*types.Transaction) error {
return fmt.Errorf("received bad tx: %s", hash)
}
}
case *eth.GetBlockHeadersPacket:
headers, err := s.chain.GetHeaders(msg)
if err != nil {
t.Logf("invalid GetBlockHeaders request: %v", err)
}
recvConn.Write(ethProto, eth.BlockHeadersMsg, &eth.BlockHeadersPacket{
RequestId: msg.RequestId,
BlockHeadersRequest: headers,
})
default:
return fmt.Errorf("unexpected eth message: %v", pretty.Sdump(msg))
}
Expand Down

0 comments on commit 1bdf8b9

Please sign in to comment.