eth/catalyst: make compliant with spec

Author: MariusVanDerWijden

eth/catalyst/api.go

@@ -765,43 +765,58 @@ func (api *ConsensusAPI) GetPayloadBodiesByHashV1(hashes []common.Hash) []*beaco
 	var bodies = make([]*beacon.ExecutionPayloadBodyV1, len(hashes))
 	for i, hash := range hashes {
 		block := api.eth.BlockChain().GetBlockByHash(hash)
-		bodies[i] = getBody(block)
+		bodies[i] = api.getBody(block)
 	}
 	return bodies
 }
 
 // GetPayloadBodiesByRangeV1 implements engine_getPayloadBodiesByRangeV1 which allows for retrieval of a range
 // of block bodies by the engine api.
-func (api *ConsensusAPI) GetPayloadBodiesByRangeV1(start, count uint64) []*beacon.ExecutionPayloadBodyV1 {
-	if api.eth.BlockChain().CurrentBlock().NumberU64() < start {
-		// Return [] if the requested range is past our latest block
-		return []*beacon.ExecutionPayloadBodyV1{}
-	}
-	bodies := make([]*beacon.ExecutionPayloadBodyV1, count)
-	for i := uint64(0); i < count; i++ {
-		block := api.eth.BlockChain().GetBlockByNumber(start + i)
-		bodies[i] = getBody(block)
-	}
-	return bodies
+func (api *ConsensusAPI) GetPayloadBodiesByRangeV1(start, count uint64) ([]*beacon.ExecutionPayloadBodyV1, error) {
+	if start == 0 || count == 0 || count > 1024 {
+		return nil, beacon.InvalidParams.With(fmt.Errorf("invalid start or count, start: %v count: %v", start, count))
+	}
+	current := api.eth.BlockChain().CurrentBlock().NumberU64()
+	// Return [] if the requested range is past our latest block
+	if current < start {
+		return []*beacon.ExecutionPayloadBodyV1{}, nil
+	}
+	// limit count up until current
+	end := start + count
+	if end > current {
+		end = current
+	}
+	var bodies []*beacon.ExecutionPayloadBodyV1
+	for i := uint64(start); i < end; i++ {
+		block := api.eth.BlockChain().GetBlockByNumber(i)
+		bodies = append(bodies, api.getBody(block))
+	}
+	return bodies, nil
 }
 
-func getBody(block *types.Block) *beacon.ExecutionPayloadBodyV1 {
+func (api *ConsensusAPI) getBody(block *types.Block) *beacon.ExecutionPayloadBodyV1 {
 	if block == nil {
 		return nil
 	}
 
 	var (
-		body = block.Body()
-		txs  = make([]hexutil.Bytes, len(body.Transactions))
+		body        = block.Body()
+		txs         = make([]hexutil.Bytes, len(body.Transactions))
+		withdrawals = body.Withdrawals
 	)
 
 	for j, tx := range body.Transactions {
 		data, _ := tx.MarshalBinary()
 		txs[j] = hexutil.Bytes(data)
 	}
 
+	// Post-shanghai withdrawals MUST be set to empty slice instead of nil
+	if withdrawals == nil && api.eth.APIBackend.ChainConfig().IsShanghai(block.Time()) {
+		withdrawals = make([]*types.Withdrawal, 0)
+	}
+
 	return &beacon.ExecutionPayloadBodyV1{
 		TransactionData: txs,
-		Withdrawals:     body.Withdrawals,
+		Withdrawals:     withdrawals,
 	}
 }

eth/catalyst/api_test.go

@@ -1314,8 +1314,8 @@ func TestGetBlockBodiesByRange(t *testing.T) {
 	}{
 		// Genesis
 		{
-			results: []*types.Body{eth.BlockChain().GetBlockByNumber(0).Body()},
-			start:   0,
+			results: []*types.Body{blocks[0].Body()},
+			start:   1,
 			count:   1,
 		},
 		// First post-merge block
@@ -1332,26 +1332,70 @@ func TestGetBlockBodiesByRange(t *testing.T) {
 		},
 		// unavailable block
 		{
-			results: []*types.Body{blocks[19].Body(), nil, nil},
-			start:   20,
+			results: []*types.Body{blocks[18].Body()},
+			start:   19,
 			count:   3,
 		},
 		// after range
 		{
 			results: make([]*types.Body, 0),
-			start:   21,
+			start:   20,
 			count:   2,
 		},
 	}
 
 	for k, test := range tests {
-		result := api.GetPayloadBodiesByRangeV1(test.start, test.count)
-		if len(result) != len(test.results) {
+		result, err := api.GetPayloadBodiesByRangeV1(test.start, test.count)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if len(result) == len(test.results) {
 			for i, r := range result {
 				if !equalBody(test.results[i], r) {
 					t.Fatalf("test %v: invalid response: expected %+v got %+v", k, test.results[i], r)
 				}
 			}
+		} else {
+			t.Fatalf("invalid length want %v got %v", len(test.results), len(result))
+		}
+	}
+}
+
+func TestGetBlockBodiesByRangeInvalidParams(t *testing.T) {
+	node, eth, _ := setupBodies(t)
+	api := NewConsensusAPI(eth)
+	defer node.Close()
+
+	tests := []struct {
+		start uint64
+		count uint64
+	}{
+		// Genesis
+		{
+			start: 0,
+			count: 1,
+		},
+		// No block requested
+		{
+			start: 1,
+			count: 0,
+		},
+		// Genesis & no block
+		{
+			start: 0,
+			count: 0,
+		},
+		// More than 1024 blocks
+		{
+			start: 1,
+			count: 1025,
+		},
+	}
+
+	for _, test := range tests {
+		result, err := api.GetPayloadBodiesByRangeV1(test.start, test.count)
+		if err == nil {
+			t.Fatalf("expected error, got %v", result)
 		}
 	}
 }