From ea1cf7bf5ee07562284f9d050a6def9704d258e7 Mon Sep 17 00:00:00 2001 From: Bosul Mun Date: Wed, 6 May 2026 15:36:54 +0200 Subject: [PATCH] eth/protocols/eth: stop serving on unavailable responses (#34787) This is an alternative PR for https://github.com/ethereum/go-ethereum/pull/34746. This PR implements the second approach among the two possible solutions mentioned in the above PR. Requests for unavailable items are possible when the peer is following a different fork from us. However this is not expected to happen frequently. Considering the amount of complexity added to the codebase, the simpler approach (this PR) can be preferred. --- eth/protocols/eth/handler_test.go | 16 ++++++++++------ eth/protocols/eth/handlers.go | 21 ++++++++++++--------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/eth/protocols/eth/handler_test.go b/eth/protocols/eth/handler_test.go index a45abc90eb..d056d121d9 100644 --- a/eth/protocols/eth/handler_test.go +++ b/eth/protocols/eth/handler_test.go @@ -424,16 +424,20 @@ func testGetBlockBodies(t *testing.T, protocol uint) { {0, []common.Hash{backend.chain.CurrentBlock().Hash()}, []bool{true}, 1}, // The chains head block should be retrievable {0, []common.Hash{{}}, []bool{false}, 0}, // A non existent block should not be returned - // Existing and non-existing blocks interleaved should not cause problems + // Existing blocks followed by a non-existing one should stop at the gap + {0, []common.Hash{ + backend.chain.GetBlockByNumber(1).Hash(), + backend.chain.GetBlockByNumber(10).Hash(), + backend.chain.GetBlockByNumber(100).Hash(), + {}, + }, []bool{true, true, true, false}, 3}, + + // A non-existing block at the start should return nothing {0, []common.Hash{ {}, backend.chain.GetBlockByNumber(1).Hash(), - {}, backend.chain.GetBlockByNumber(10).Hash(), - {}, - backend.chain.GetBlockByNumber(100).Hash(), - {}, - }, []bool{false, true, false, true, false, true, false}, 3}, + }, []bool{false, true, true}, 0}, } // Run each of the tests and verify the results against the chain for i, tt := range tests { diff --git a/eth/protocols/eth/handlers.go b/eth/protocols/eth/handlers.go index 7556df9af2..3254a0abc2 100644 --- a/eth/protocols/eth/handlers.go +++ b/eth/protocols/eth/handlers.go @@ -238,10 +238,12 @@ func ServiceGetBlockBodiesQuery(chain *core.BlockChain, query GetBlockBodiesRequ lookups >= 2*maxBodiesServe { break } - if data := chain.GetBodyRLP(hash); len(data) != 0 { - bodies = append(bodies, data) - bytes += len(data) + data := chain.GetBodyRLP(hash) + if len(data) == 0 { + break // If we don't have this block's body, stop serving. } + bodies = append(bodies, data) + bytes += len(data) } return bodies } @@ -281,16 +283,16 @@ func ServiceGetReceiptsQuery69(chain *core.BlockChain, query GetReceiptsRequest) // Retrieve the requested block's receipts results := chain.GetReceiptsRLP(hash) if results == nil { - continue // Can't retrieve the receipts, so we just skip this block. + break // Don't have this block's receipts, stop serving. } body := chain.GetBodyRLP(hash) if body == nil { - continue // The block body is missing, we also have to skip. + break // The block body is missing, stop serving. } results, _, err := blockReceiptsToNetwork(results, body, receiptQueryParams{}) if err != nil { log.Error("Error in block receipts conversion", "hash", hash, "err", err) - continue + break } receipts.AppendRaw(results) bytes += len(results) @@ -312,12 +314,13 @@ func serviceGetReceiptsQuery70(chain *core.BlockChain, query GetReceiptsRequest, break } results := chain.GetReceiptsRLP(hash) + // If we don't have this block's receipts or body, stop serving. if results == nil { - continue // Can't retrieve the receipts, so we just skip this block. + break } body := chain.GetBodyRLP(hash) if body == nil { - continue // The block body is missing, we also have to skip. + break } q := receiptQueryParams{sizeLimit: uint64(maxPacketSize - bytes)} if i == 0 { @@ -326,7 +329,7 @@ func serviceGetReceiptsQuery70(chain *core.BlockChain, query GetReceiptsRequest, results, incomplete, err := blockReceiptsToNetwork(results, body, q) if err != nil { log.Error("Error in block receipts conversion", "hash", hash, "err", err) - continue + break } if results == nil { // This case triggers when the first receipt of the block receipts list doesn't