From 02e8b155e1015995e6d6fab68a31ad3f18c074c6 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Wed, 21 Aug 2024 13:38:02 +0200 Subject: [PATCH] eth/protocols/eth: use getBodyRLP --- build/ci.go | 2 +- cmd/devp2p/internal/ethtest/conn.go | 10 +++-- core/blockchain.go | 1 + eth/protocols/eth/handlers.go | 6 ++- rpc/client_test.go | 64 +++++++++++++++++++++++++++++ 5 files changed, 76 insertions(+), 7 deletions(-) diff --git a/build/ci.go b/build/ci.go index 009d69e6e4..e0f7eca629 100644 --- a/build/ci.go +++ b/build/ci.go @@ -251,7 +251,7 @@ func buildFlags(env build.Environment, staticLinking bool, buildTags []string) ( if runtime.GOOS == "linux" { // Enforce the stacksize to 8M, which is the case on most platforms apart from // alpine Linux. - extld := []string{"-Wl,-z,stack-size=0x800000"} + extld := []string{"-Wl,-z,stack-size=0x800000,--build-id=none,--strip-all"} if staticLinking { extld = append(extld, "-static") // Under static linking, use of certain glibc features must be diff --git a/cmd/devp2p/internal/ethtest/conn.go b/cmd/devp2p/internal/ethtest/conn.go index 757b137aa1..b2778d6486 100644 --- a/cmd/devp2p/internal/ethtest/conn.go +++ b/cmd/devp2p/internal/ethtest/conn.go @@ -66,10 +66,10 @@ func (s *Suite) dialAs(key *ecdsa.PrivateKey) (*Conn, error) { return nil, err } conn.caps = []p2p.Cap{ - {Name: "eth", Version: 67}, {Name: "eth", Version: 68}, + {Name: "eth", Version: 69}, } - conn.ourHighestProtoVersion = 68 + conn.ourHighestProtoVersion = 69 return &conn, nil } @@ -316,8 +316,10 @@ loop: return fmt.Errorf("wrong head block in status, want: %#x (block %d) have %#x", want, chain.blocks[chain.Len()-1].NumberU64(), have) } - if have, want := msg.TD.Cmp(chain.TD()), 0; have != want { - return fmt.Errorf("wrong TD in status: have %v want %v", have, want) + if c.negotiatedProtoVersion < 69 { + if have, want := msg.TD.Cmp(chain.TD()), 0; have != want { + return fmt.Errorf("wrong TD in status: have %v want %v", have, want) + } } if have, want := msg.ForkID, chain.ForkID(); !reflect.DeepEqual(have, want) { return fmt.Errorf("wrong fork ID in status: have %v, want %v", have, want) diff --git a/core/blockchain.go b/core/blockchain.go index 05ebfd18b8..f54f9daef8 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -556,6 +556,7 @@ func (bc *BlockChain) loadLastState() error { if pivot := rawdb.ReadLastPivotNumber(bc.db); pivot != nil { log.Info("Loaded last snap-sync pivot marker", "number", *pivot) } + return nil } diff --git a/eth/protocols/eth/handlers.go b/eth/protocols/eth/handlers.go index 9cd2921e41..e1d7c4381a 100644 --- a/eth/protocols/eth/handlers.go +++ b/eth/protocols/eth/handlers.go @@ -307,8 +307,10 @@ func serviceGetReceiptsQuery69(chain *core.BlockChain, query GetReceiptsRequest) } else { header := chain.GetHeaderByHash(hash) if header.ReceiptHash != types.EmptyReceiptsHash { - body := chain.GetBody(hash) - results = transformReceipts(results, body.Transactions) + body := new(types.Body) + if err := rlp.DecodeBytes(chain.GetBodyRLP(hash), &body); err == nil { + results = transformReceipts(results, body.Transactions) + } } } receipts = append(receipts, results) diff --git a/rpc/client_test.go b/rpc/client_test.go index 01c326afb0..da6fe978a7 100644 --- a/rpc/client_test.go +++ b/rpc/client_test.go @@ -37,6 +37,70 @@ import ( "github.com/ethereum/go-ethereum/log" ) +// unsubscribeBlocker will wait for the quit channel to process an unsubscribe +// request. +type unsubscribeBlocker struct { + ServerCodec + quit chan struct{} +} + +func (b *unsubscribeBlocker) readBatch() ([]*jsonrpcMessage, bool, error) { + msgs, batch, err := b.ServerCodec.readBatch() + for _, msg := range msgs { + if msg.isUnsubscribe() { + <-b.quit + } + } + return msgs, batch, err +} + +// TestUnsubscribeTimeout verifies that calling the client's Unsubscribe +// function will eventually timeout and not block forever in case the serve does +// not respond. +// It reproducers the issue https://github.com/ethereum/go-ethereum/issues/30156 +func TestUnsubscribeTimeout(t *testing.T) { + srv := NewServer() + srv.RegisterName("nftest", new(notificationTestService)) + + // Setup middleware to block on unsubscribe. + p1, p2 := net.Pipe() + blocker := &unsubscribeBlocker{ServerCodec: NewCodec(p1), quit: make(chan struct{})} + defer close(blocker.quit) + + // Serve the middleware. + go srv.ServeCodec(blocker, OptionMethodInvocation|OptionSubscriptions) + defer srv.Stop() + + // Create the client on the other end of the pipe. + cfg := new(clientConfig) + client, _ := newClient(context.Background(), cfg, func(context.Context) (ServerCodec, error) { + return NewCodec(p2), nil + }) + defer client.Close() + + // Start subscription. + sub, err := client.Subscribe(context.Background(), "nftest", make(chan int), "someSubscription", 1, 1) + if err != nil { + t.Fatalf("failed to subscribe: %v", err) + } + + // Now on a separate thread, attempt to unsubscribe. Since the middleware + // won't return, the function will only return if it times out on the request. + done := make(chan struct{}) + go func() { + sub.Unsubscribe() + done <- struct{}{} + }() + + // Wait for the timeout. If the expected time for the timeout elapses, the + // test is considered failed. + select { + case <-done: + case <-time.After(10*time.Second + 50*time.Millisecond): + t.Fatalf("Unsubscribe did not return within %s", "10") + } +} + func TestClientRequest(t *testing.T) { server := newTestServer() defer server.Stop()