From 15e5e6176bb384514271c24e53c0b9e2f862b2a4 Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Fri, 17 Feb 2023 20:30:38 +0200 Subject: [PATCH] eth/catalyst: request too large error (#26722) The method `GetPayloadBodiesByRangeV1` now returns "-38004: Too large request" error if the requested range is too large, according to spec Co-authored-by: Martin Holst Swende --- beacon/engine/errors.go | 1 + eth/catalyst/api.go | 5 ++++- eth/catalyst/api_test.go | 16 +++++++++++----- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/beacon/engine/errors.go b/beacon/engine/errors.go index fe1137e0041d..769001b9e3d7 100644 --- a/beacon/engine/errors.go +++ b/beacon/engine/errors.go @@ -78,6 +78,7 @@ var ( UnknownPayload = &EngineAPIError{code: -38001, msg: "Unknown payload"} InvalidForkChoiceState = &EngineAPIError{code: -38002, msg: "Invalid forkchoice state"} InvalidPayloadAttributes = &EngineAPIError{code: -38003, msg: "Invalid payload attributes"} + TooLargeRequest = &EngineAPIError{code: -38004, msg: "Too large request"} InvalidParams = &EngineAPIError{code: -32602, msg: "Invalid parameters"} STATUS_INVALID = ForkChoiceResponse{PayloadStatus: PayloadStatusV1{Status: INVALID}, PayloadID: nil} diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index cee9b2c508cc..95eed408f031 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -785,9 +785,12 @@ func (api *ConsensusAPI) GetPayloadBodiesByHashV1(hashes []common.Hash) []*engin // GetPayloadBodiesByRangeV1 implements engine_getPayloadBodiesByRangeV1 which allows for retrieval of a range // of block bodies by the engine api. func (api *ConsensusAPI) GetPayloadBodiesByRangeV1(start, count hexutil.Uint64) ([]*engine.ExecutionPayloadBodyV1, error) { - if start == 0 || count == 0 || count > 1024 { + if start == 0 || count == 0 { return nil, engine.InvalidParams.With(fmt.Errorf("invalid start or count, start: %v count: %v", start, count)) } + if count > 1024 { + return nil, engine.TooLargeRequest.With(fmt.Errorf("requested count too large: %v", count)) + } // limit count up until current current := api.eth.BlockChain().CurrentBlock().NumberU64() last := uint64(start) + uint64(count) - 1 diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index 7155fb54d038..4dc0c0ea7ea9 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -1383,37 +1383,43 @@ func TestGetBlockBodiesByRangeInvalidParams(t *testing.T) { node, eth, _ := setupBodies(t) api := NewConsensusAPI(eth) defer node.Close() - tests := []struct { start hexutil.Uint64 count hexutil.Uint64 + want *engine.EngineAPIError }{ // Genesis { start: 0, count: 1, + want: engine.InvalidParams, }, // No block requested { start: 1, count: 0, + want: engine.InvalidParams, }, // Genesis & no block { start: 0, count: 0, + want: engine.InvalidParams, }, // More than 1024 blocks { start: 1, count: 1025, + want: engine.TooLargeRequest, }, } - - for _, test := range tests { - result, err := api.GetPayloadBodiesByRangeV1(test.start, test.count) + for i, tc := range tests { + result, err := api.GetPayloadBodiesByRangeV1(tc.start, tc.count) if err == nil { - t.Fatalf("expected error, got %v", result) + t.Fatalf("test %d: expected error, got %v", i, result) + } + if have, want := err.Error(), tc.want.Error(); have != want { + t.Fatalf("test %d: have %s, want %s", i, have, want) } } }