From 2fa448c45f5854e13bfec9ceb9da4c29e53b0ac2 Mon Sep 17 00:00:00 2001 From: Andrew Ashikhmin <34320705+yperbasis@users.noreply.github.com> Date: Mon, 6 Feb 2023 18:38:45 +0100 Subject: [PATCH] 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 +++++++++++-- ethdb/privateapi/ethbackend.go | 41 +++++++++++++++++----------- rpc/errors.go | 10 +++---- rpc/handler.go | 6 ++-- 4 files changed, 50 insertions(+), 26 deletions(-) diff --git a/cmd/rpcdaemon/commands/engine_api.go b/cmd/rpcdaemon/commands/engine_api.go index 3ac505e42..7777e2dd7 100644 --- a/cmd/rpcdaemon/commands/engine_api.go +++ b/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 } } diff --git a/ethdb/privateapi/ethbackend.go b/ethdb/privateapi/ethbackend.go index 6ab58d09b..b9f73ab48 100644 --- a/ethdb/privateapi/ethbackend.go +++ b/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 { - return nil, &InvalidParamsErr + if err := s.checkWithdrawalsPresence(header.Time, withdrawals); err != nil { + 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) || - (s.config.IsShanghai(payloadAttributes.Timestamp) && param.Withdrawals == nil) { - return nil, &InvalidParamsErr + if err := s.checkWithdrawalsPresence(payloadAttributes.Timestamp, param.Withdrawals); err != nil { + return nil, err } // 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 = 0; i < request.Count; i++ { - block, err := rawdb.ReadBlockByNumber(tx, request.Start+i) + for i := uint64(0); i < request.Count; i++ { + hash, err := rawdb.ReadCanonicalHash(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 { - // 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 + bodies = append(bodies, body) } return &remote.EngineGetPayloadBodiesV1Response{Bodies: bodies}, nil diff --git a/rpc/errors.go b/rpc/errors.go index 5375a6862..b6f8f2ff6 100644 --- a/rpc/errors.go +++ b/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 -type invalidParamsError struct{ message string } +// unable to decode supplied params, or invalid parameters +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 diff --git a/rpc/handler.go b/rpc/handler.go index a5fe261ef..2cae39e03 100644 --- a/rpc/handler.go +++ b/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:]