From 245cff0a1a6d2f67169fcd59b2997c5b7a98338b Mon Sep 17 00:00:00 2001
From: Marius van der Wijden <m.vanderwijden@live.de>
Date: Fri, 27 Jan 2023 11:42:14 +0100
Subject: [PATCH] eth/catalyst: error on nil withdrawals post-shanghai (#26549)

This adds explicit checks for the presence of withdrawals in the engine API.

Co-authored-by: Felix Lange <fjl@twurst.com>
---
 eth/catalyst/api.go      |  12 +++++
 eth/catalyst/api_test.go | 112 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+)

diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go
index 39dcba04f8..3b931abffe 100644
--- a/eth/catalyst/api.go
+++ b/eth/catalyst/api.go
@@ -164,6 +164,18 @@ func (api *ConsensusAPI) ForkchoiceUpdatedV1(update beacon.ForkchoiceStateV1, pa
 
 // ForkchoiceUpdatedV2 is equivalent to V1 with the addition of withdrawals in the payload attributes.
 func (api *ConsensusAPI) ForkchoiceUpdatedV2(update beacon.ForkchoiceStateV1, payloadAttributes *beacon.PayloadAttributes) (beacon.ForkChoiceResponse, error) {
+	if !api.eth.BlockChain().Config().IsShanghai(payloadAttributes.Timestamp) {
+		// Reject payload attributes with withdrawals before shanghai
+		if payloadAttributes != nil && payloadAttributes.Withdrawals != nil {
+			return beacon.STATUS_INVALID, beacon.InvalidPayloadAttributes.With(errors.New("withdrawals before shanghai"))
+		}
+	} else {
+		// Reject payload attributes with nil withdrawals after shanghai
+		if payloadAttributes != nil && payloadAttributes.Withdrawals == nil {
+			return beacon.STATUS_INVALID, beacon.InvalidPayloadAttributes.With(errors.New("missing withdrawals list"))
+		}
+	}
+
 	return api.forkchoiceUpdated(update, payloadAttributes)
 }
 
diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go
index 8df48bd08e..d9280e99d6 100644
--- a/eth/catalyst/api_test.go
+++ b/eth/catalyst/api_test.go
@@ -1113,3 +1113,115 @@ func TestWithdrawals(t *testing.T) {
 		}
 	}
 }
+
+func TestNilWithdrawals(t *testing.T) {
+	genesis, blocks := generateMergeChain(10, true)
+	// Set shanghai time to last block + 4 seconds (first post-merge block)
+	time := blocks[len(blocks)-1].Time() + 4
+	genesis.Config.ShanghaiTime = &time
+
+	n, ethservice := startEthService(t, genesis, blocks)
+	ethservice.Merger().ReachTTD()
+	defer n.Close()
+
+	api := NewConsensusAPI(ethservice)
+	parent := ethservice.BlockChain().CurrentHeader()
+	aa := common.Address{0xaa}
+
+	type test struct {
+		blockParams beacon.PayloadAttributes
+		wantErr     bool
+	}
+	tests := []test{
+		// Before Shanghai
+		{
+			blockParams: beacon.PayloadAttributes{
+				Timestamp:   parent.Time + 2,
+				Withdrawals: nil,
+			},
+			wantErr: false,
+		},
+		{
+			blockParams: beacon.PayloadAttributes{
+				Timestamp:   parent.Time + 2,
+				Withdrawals: make([]*types.Withdrawal, 0),
+			},
+			wantErr: true,
+		},
+		{
+			blockParams: beacon.PayloadAttributes{
+				Timestamp: parent.Time + 2,
+				Withdrawals: []*types.Withdrawal{
+					{
+						Index:   0,
+						Address: aa,
+						Amount:  32,
+					},
+				},
+			},
+			wantErr: true,
+		},
+		// After Shanghai
+		{
+			blockParams: beacon.PayloadAttributes{
+				Timestamp:   parent.Time + 5,
+				Withdrawals: nil,
+			},
+			wantErr: true,
+		},
+		{
+			blockParams: beacon.PayloadAttributes{
+				Timestamp:   parent.Time + 5,
+				Withdrawals: make([]*types.Withdrawal, 0),
+			},
+			wantErr: false,
+		},
+		{
+			blockParams: beacon.PayloadAttributes{
+				Timestamp: parent.Time + 5,
+				Withdrawals: []*types.Withdrawal{
+					{
+						Index:   0,
+						Address: aa,
+						Amount:  32,
+					},
+				},
+			},
+			wantErr: false,
+		},
+	}
+
+	fcState := beacon.ForkchoiceStateV1{
+		HeadBlockHash: parent.Hash(),
+	}
+
+	for _, test := range tests {
+		_, err := api.ForkchoiceUpdatedV2(fcState, &test.blockParams)
+		if test.wantErr {
+			if err == nil {
+				t.Fatal("wanted error on fcuv2 with invalid withdrawals")
+			}
+			continue
+		}
+		if err != nil {
+			t.Fatalf("error preparing payload, err=%v", err)
+		}
+
+		// 11: verify locally build block.
+		payloadID := (&miner.BuildPayloadArgs{
+			Parent:       fcState.HeadBlockHash,
+			Timestamp:    test.blockParams.Timestamp,
+			FeeRecipient: test.blockParams.SuggestedFeeRecipient,
+			Random:       test.blockParams.Random,
+		}).Id()
+		execData, err := api.GetPayloadV2(payloadID)
+		if err != nil {
+			t.Fatalf("error getting payload, err=%v", err)
+		}
+		if status, err := api.NewPayloadV2(*execData.ExecutionPayload); err != nil {
+			t.Fatalf("error validating payload: %v", err)
+		} else if status.Status != beacon.VALID {
+			t.Fatalf("invalid payload")
+		}
+	}
+}