Validate params of GetPayloadBodiesByHash and ByRange (#6785)

See https://github.com/ethereum/execution-apis/pull/366 and
https://github.com/ethereum/execution-apis/pull/370.

Also fix a couple of issues so that the Hive tests pass.

cmd/rpcdaemon/commands/engine_api.go

@@ -19,6 +19,7 @@ import (
 	"github.com/ledgerwatch/erigon/core/rawdb"
 	"github.com/ledgerwatch/erigon/core/types"
 	"github.com/ledgerwatch/erigon/ethdb/privateapi"
+	"github.com/ledgerwatch/erigon/rpc"
 	"github.com/ledgerwatch/erigon/turbo/rpchelper"
 )
 
@@ -70,7 +71,7 @@ type TransitionConfiguration struct {
 }
 
 type ExecutionPayloadBodyV1 struct {
-	Transactions [][]byte            `json:"transactions" gencodec:"required"`
+	Transactions []hexutil.Bytes     `json:"transactions" gencodec:"required"`
 	Withdrawals  []*types.Withdrawal `json:"withdrawals"  gencodec:"required"`
 }
 
@@ -366,6 +367,10 @@ func (e *EngineImpl) ExchangeTransitionConfigurationV1(ctx context.Context, beac
 }
 
 func (e *EngineImpl) GetPayloadBodiesByHashV1(ctx context.Context, hashes []common.Hash) ([]*ExecutionPayloadBodyV1, error) {
+	if len(hashes) > 1024 {
+		return nil, &privateapi.TooLargeRequestErr
+	}
+
 	h := make([]*types2.H256, len(hashes))
 	for i, hash := range hashes {
 		h[i] = gointerfaces.ConvertHashToH256(hash)
@@ -380,6 +385,13 @@ func (e *EngineImpl) GetPayloadBodiesByHashV1(ctx context.Context, hashes []comm
 }
 
 func (e *EngineImpl) GetPayloadBodiesByRangeV1(ctx context.Context, start uint64, count uint64) ([]*ExecutionPayloadBodyV1, error) {
+	if start == 0 || count == 0 {
+		return nil, &rpc.InvalidParamsError{Message: fmt.Sprintf("invalid start or count, start: %v count: %v", start, count)}
+	}
+	if count > 1024 {
+		return nil, &privateapi.TooLargeRequestErr
+	}
+
 	apiRes, err := e.api.EngineGetPayloadBodiesByRangeV1(ctx, &remote.EngineGetPayloadBodiesByRangeV1Request{Start: start, Count: count})
 	if err != nil {
 		return nil, err
@@ -436,9 +448,12 @@ func convertExecutionPayloadV1(response *remote.EngineGetPayloadBodiesV1Response
 			result[idx] = nil
 		} else {
 			pl := &ExecutionPayloadBodyV1{
-				Transactions: body.Transactions,
+				Transactions: make([]hexutil.Bytes, len(body.Transactions)),
 				Withdrawals:  privateapi.ConvertWithdrawalsFromRpc(body.Withdrawals),
 			}
+			for i := range body.Transactions {
+				pl.Transactions[i] = body.Transactions[i]
+			}
 			result[idx] = pl
 		}
 	}

ethdb/privateapi/ethbackend.go

@@ -48,7 +48,7 @@ const MaxBuilders = 128
 var UnknownPayloadErr = rpc.CustomError{Code: -38001, Message: "Unknown payload"}
 var InvalidForkchoiceStateErr = rpc.CustomError{Code: -38002, Message: "Invalid forkchoice state"}
 var InvalidPayloadAttributesErr = rpc.CustomError{Code: -38003, Message: "Invalid payload attributes"}
-var InvalidParamsErr = rpc.CustomError{Code: -32602, Message: "Invalid params"}
+var TooLargeRequestErr = rpc.CustomError{Code: -38004, Message: "Too large request"}
 
 type EthBackendServer struct {
 	remote.UnimplementedETHBACKENDServer // must be embedded to have forward compatible implementations.
@@ -281,6 +281,16 @@ func (s *EthBackendServer) stageLoopIsBusy() bool {
 	return !s.hd.BeaconRequestList.IsWaiting()
 }
 
+func (s *EthBackendServer) checkWithdrawalsPresence(time uint64, withdrawals []*types.Withdrawal) error {
+	if !s.config.IsShanghai(time) && withdrawals != nil {
+		return &rpc.InvalidParamsError{Message: "withdrawals before shanghai"}
+	}
+	if s.config.IsShanghai(time) && withdrawals == nil {
+		return &rpc.InvalidParamsError{Message: "missing withdrawals list"}
+	}
+	return nil
+}
+
 // EngineNewPayload validates and possibly executes payload
 func (s *EthBackendServer) EngineNewPayload(ctx context.Context, req *types2.ExecutionPayload) (*remote.EnginePayloadStatus, error) {
 	header := types.Header{
@@ -311,8 +321,8 @@ func (s *EthBackendServer) EngineNewPayload(ctx context.Context, req *types2.Exe
 		header.WithdrawalsHash = &wh
 	}
 
-	if !s.config.IsShanghai(header.Time) && withdrawals != nil || s.config.IsShanghai(header.Time) && withdrawals == nil {
+	if err := s.checkWithdrawalsPresence(header.Time, withdrawals); err != nil {
-		return nil, &InvalidParamsErr
+		return nil, err
 	}
 
 	blockHash := gointerfaces.ConvertH256ToHash(req.BlockHash)
@@ -654,9 +664,8 @@ func (s *EthBackendServer) EngineForkChoiceUpdated(ctx context.Context, req *rem
 		param.Withdrawals = ConvertWithdrawalsFromRpc(payloadAttributes.Withdrawals)
 	}
 
-	if (!s.config.IsShanghai(payloadAttributes.Timestamp) && param.Withdrawals != nil) ||
+	if err := s.checkWithdrawalsPresence(payloadAttributes.Timestamp, param.Withdrawals); err != nil {
-		(s.config.IsShanghai(payloadAttributes.Timestamp) && param.Withdrawals == nil) {
+		return nil, err
-		return nil, &InvalidParamsErr
 	}
 
 	// Initiate payload building
@@ -709,24 +718,24 @@ func (s *EthBackendServer) EngineGetPayloadBodiesByRangeV1(ctx context.Context,
 		return nil, err
 	}
 
-	bodies := make([]*types2.ExecutionPayloadBodyV1, request.Count)
+	bodies := make([]*types2.ExecutionPayloadBodyV1, 0, request.Count)
 
-	var i uint64
+	for i := uint64(0); i < request.Count; i++ {
-	for i = 0; i < request.Count; i++ {
+		hash, err := rawdb.ReadCanonicalHash(tx, request.Start+i)
-		block, err := rawdb.ReadBlockByNumber(tx, request.Start+i)
 		if err != nil {
 			return nil, err
 		}
+		if hash == (libcommon.Hash{}) {
+			// break early if beyond the last known canonical header
+			break
+		}
+
+		block := rawdb.ReadBlock(tx, hash, request.Start+i)
 		body, err := extractPayloadBodyFromBlock(block)
 		if err != nil {
 			return nil, err
 		}
-		if body == nil {
+		bodies = append(bodies, body)
-			// break early if the body is nil to trim the response.  A missing body indicates we don't have the
-			// canonical block so can just stop outputting from here
-			break
-		}
-		bodies[i] = body
 	}
 
 	return &remote.EngineGetPayloadBodiesV1Response{Bodies: bodies}, nil

rpc/errors.go

@@ -24,7 +24,7 @@ var (
 	_ Error = new(parseError)
 	_ Error = new(invalidRequestError)
 	_ Error = new(invalidMessageError)
-	_ Error = new(invalidParamsError)
+	_ Error = new(InvalidParamsError)
 	_ Error = new(CustomError)
 )
 
@@ -67,12 +67,12 @@ func (e *invalidMessageError) ErrorCode() int { return -32700 }
 
 func (e *invalidMessageError) Error() string { return e.message }
 
-// unable to decode supplied params, or an invalid number of parameters
+// unable to decode supplied params, or invalid parameters
-type invalidParamsError struct{ message string }
+type InvalidParamsError struct{ Message string }
 
-func (e *invalidParamsError) ErrorCode() int { return -32602 }
+func (e *InvalidParamsError) ErrorCode() int { return -32602 }
 
-func (e *invalidParamsError) Error() string { return e.message }
+func (e *InvalidParamsError) Error() string { return e.Message }
 
 type CustomError struct {
 	Code    int

rpc/handler.go

@@ -438,7 +438,7 @@ func (h *handler) handleCall(cp *callProc, msg *jsonrpcMessage, stream *jsoniter
 	}
 	args, err := parsePositionalArguments(msg.Params, callb.argTypes)
 	if err != nil {
-		return msg.errorResponse(&invalidParamsError{err.Error()})
+		return msg.errorResponse(&InvalidParamsError{err.Error()})
 	}
 	start := time.Now()
 	answer := h.runMethod(cp.ctx, msg, callb, args, stream)
@@ -464,7 +464,7 @@ func (h *handler) handleSubscribe(cp *callProc, msg *jsonrpcMessage, stream *jso
 	// Subscription method name is first argument.
 	name, err := parseSubscriptionName(msg.Params)
 	if err != nil {
-		return msg.errorResponse(&invalidParamsError{err.Error()})
+		return msg.errorResponse(&InvalidParamsError{err.Error()})
 	}
 	namespace := msg.namespace()
 	callb := h.reg.subscription(namespace, name)
@@ -476,7 +476,7 @@ func (h *handler) handleSubscribe(cp *callProc, msg *jsonrpcMessage, stream *jso
 	argTypes := append([]reflect.Type{stringType}, callb.argTypes...)
 	args, err := parsePositionalArguments(msg.Params, argTypes)
 	if err != nil {
-		return msg.errorResponse(&invalidParamsError{err.Error()})
+		return msg.errorResponse(&InvalidParamsError{err.Error()})
 	}
 	args = args[1:]