diff --git a/eth/protocols/eth/handler_test.go b/eth/protocols/eth/handler_test.go index 36f5e90c9a..bbbd888701 100644 --- a/eth/protocols/eth/handler_test.go +++ b/eth/protocols/eth/handler_test.go @@ -297,6 +297,34 @@ func testGetBlockHeaders(t *testing.T, protocol uint) { backend.chain.GetBlockByNumber(0).Hash(), }, }, + // Check a corner case where skipping causes overflow with reverse=false + { + &GetBlockHeadersRequest{Origin: HashOrNumber{Number: 1}, Amount: 2, Reverse: false, Skip: math.MaxUint64 - 1}, + []common.Hash{ + backend.chain.GetBlockByNumber(1).Hash(), + }, + }, + // Check a corner case where skipping causes overflow with reverse=true + { + &GetBlockHeadersRequest{Origin: HashOrNumber{Number: 1}, Amount: 2, Reverse: true, Skip: math.MaxUint64 - 1}, + []common.Hash{ + backend.chain.GetBlockByNumber(1).Hash(), + }, + }, + // Check another corner case where skipping causes overflow with reverse=false + { + &GetBlockHeadersRequest{Origin: HashOrNumber{Number: 1}, Amount: 2, Reverse: false, Skip: math.MaxUint64}, + []common.Hash{ + backend.chain.GetBlockByNumber(1).Hash(), + }, + }, + // Check another corner case where skipping causes overflow with reverse=true + { + &GetBlockHeadersRequest{Origin: HashOrNumber{Number: 1}, Amount: 2, Reverse: true, Skip: math.MaxUint64}, + []common.Hash{ + backend.chain.GetBlockByNumber(1).Hash(), + }, + }, // Check a corner case where skipping overflow loops back into the chain start { &GetBlockHeadersRequest{Origin: HashOrNumber{Hash: backend.chain.GetBlockByNumber(3).Hash()}, Amount: 2, Reverse: false, Skip: math.MaxUint64 - 1}, diff --git a/eth/protocols/eth/handlers.go b/eth/protocols/eth/handlers.go index ab56be7ffc..fda650da1c 100644 --- a/eth/protocols/eth/handlers.go +++ b/eth/protocols/eth/handlers.go @@ -128,15 +128,22 @@ func serviceNonContiguousBlockHeaderQuery(chain *core.BlockChain, query *GetBloc } case query.Reverse: // Number based traversal towards the genesis block - if query.Origin.Number >= query.Skip+1 { - query.Origin.Number -= query.Skip + 1 - } else { + current := query.Origin.Number + ancestor := current - (query.Skip + 1) + if ancestor >= current { // check for underflow unknown = true + } else { + query.Origin.Number = ancestor } case !query.Reverse: - // Number based traversal towards the leaf block - query.Origin.Number += query.Skip + 1 + current := query.Origin.Number + next := current + query.Skip + 1 + if next <= current { // check for overflow + unknown = true + } else { + query.Origin.Number = next + } } } return headers