simulators/ethereum/engine: more withdrawals verifications

Author: marioevz

simulators/ethereum/engine/helper/payload.go

@@ -15,21 +15,22 @@ import (
 )
 
 type CustomPayloadData struct {
-	ParentHash    *common.Hash
-	FeeRecipient  *common.Address
-	StateRoot     *common.Hash
-	ReceiptsRoot  *common.Hash
-	LogsBloom     *[]byte
-	PrevRandao    *common.Hash
-	Number        *uint64
-	GasLimit      *uint64
-	GasUsed       *uint64
-	Timestamp     *uint64
-	ExtraData     *[]byte
-	BaseFeePerGas *big.Int
-	BlockHash     *common.Hash
-	Transactions  *[][]byte
-	Withdrawals   types.Withdrawals
+	ParentHash        *common.Hash
+	FeeRecipient      *common.Address
+	StateRoot         *common.Hash
+	ReceiptsRoot      *common.Hash
+	LogsBloom         *[]byte
+	PrevRandao        *common.Hash
+	Number            *uint64
+	GasLimit          *uint64
+	GasUsed           *uint64
+	Timestamp         *uint64
+	ExtraData         *[]byte
+	BaseFeePerGas     *big.Int
+	BlockHash         *common.Hash
+	Transactions      *[][]byte
+	Withdrawals       types.Withdrawals
+	RemoveWithdrawals bool
 }
 
 // Construct a customized payload by taking an existing payload as base and mixing it CustomPayloadData
@@ -100,7 +101,9 @@ func CustomizePayload(basePayload *api.ExecutableData, customData *CustomPayload
 	if customData.BaseFeePerGas != nil {
 		customPayloadHeader.BaseFee = customData.BaseFeePerGas
 	}
-	if customData.Withdrawals != nil {
+	if customData.RemoveWithdrawals {
+		customPayloadHeader.WithdrawalsHash = nil
+	} else if customData.Withdrawals != nil {
 		h := types.DeriveSha(customData.Withdrawals, trie.NewStackTrie(nil))
 		customPayloadHeader.WithdrawalsHash = &h
 	} else if basePayload.Withdrawals != nil {
@@ -125,7 +128,9 @@ func CustomizePayload(basePayload *api.ExecutableData, customData *CustomPayload
 		BlockHash:     customPayloadHeader.Hash(),
 		Transactions:  txs,
 	}
-	if customData.Withdrawals != nil {
+	if customData.RemoveWithdrawals {
+		result.Withdrawals = nil
+	} else if customData.Withdrawals != nil {
 		result.Withdrawals = customData.Withdrawals
 	} else if basePayload.Withdrawals != nil {
 		result.Withdrawals = basePayload.Withdrawals

simulators/ethereum/engine/suites/withdrawals/README.md

@@ -6,8 +6,16 @@ This test suite verifies behavior of the Engine API on each client after the Sha
 ## Shanghai Fork
 - Genesis Shanghai: Tests the withdrawals fork happening since genesis (e.g. on a testnet).
 - Shanghai Fork on Block 1: Tests the withdrawals fork happening directly after genesis.
-- Shanghai Fork on Block 2: Tests the transition to the withdrawals fork after a single block has happened. Block 1 is sent with invalid non-null withdrawals payload and client is expected to respond with the appropriate error.
-- Shanghai Fork on Block 3: Tests the transition to the withdrawals fork after two blocks have happened. Block 2 is sent with invalid non-null withdrawals payload (both in `engine_newPayloadV2` and the attributes of `engine_forkchoiceUpdatedV2`) and client is expected to respond with the appropriate error.
+- Shanghai Fork on Block 2: Tests the transition to the withdrawals fork after a single block has happened. Block 1 is used to send invalid non-null withdrawals payload.
+- Shanghai Fork on Block 3: Tests the transition to the withdrawals fork after two blocks have happened. Block 1 and 2 are used to send invalid non-null withdrawals payload.
+
+All test cases contain following verifications:
+- Verify client responds with error -32602 on the following scenarios:
+  - Send `ExecutionPayloadV2` using a custom valid (correct BlockHash) execution payload that includes an empty array for withdrawals on `timestamp < SHANGHAI_TIMESTAMP` using `engine_newPayloadV2`
+  - Send `PayloadAttributesV2` using an empty array for withdrawals on `timestamp < SHANGHAI_TIMESTAMP` using `engine_forkchoiceUpdatedV2`
+  - Send `ExecutionPayloadV1` using a custom valid (correct BlockHash) execution payload that includes `null` as withdrawals on `timestamp >= SHANGHAI_TIMESTAMP` using `engine_newPayloadV2`
+  - Send `PayloadAttributesV2` using `null` for withdrawals on `timestamp < SHANGHAI_TIMESTAMP` using `engine_forkchoiceUpdatedV2`
+- Use `engine_forkchoiceUpdatedV2` and `engine_newPayloadV2` to send pre-Shanghai payloads/payload attributes, and verify method call succeeds.
 
 ## Withdrawals
 - Withdraw to a single account: Make multiple withdrawals to a single account.
@@ -16,6 +24,11 @@ This test suite verifies behavior of the Engine API on each client after the Sha
 - Withdraw zero amount: Make multiple withdrawals where the amount withdrawn is 0.
 - Empty Withdrawals: Produce withdrawals block with zero withdrawals.
 
+All test cases contain the following verifications:
+- Verify all withdrawal amounts (sent in Gwei) correctly translate to a wei balance increase in the execution client (after the payload has been set as head using `engine_forkchoiceUpdatedV2`).
+- Verify using `eth_getBalance` that the balance of all withdrawn accounts match the expected amount on each block of the chain and `latest`.
+- Payload returned by `engine_getPayloadV2` contains the same list of withdrawals as were passed by `PayloadAttributesV2` in the `engine_forkchoiceUpdatedV2` method call.
+
 ## Sync to Shanghai
 - Sync after 2 blocks - Shanghai on Block 1 - Single Withdrawal Account - No Transactions:
   - Spawn a first client
@@ -151,4 +164,13 @@ This test suite verifies behavior of the Engine API on each client after the Sha
   - Verify that `TxB` returns error on `eth_sendRawTransaction` and also should be absent from the transaction pool of the client
   - Request a new payload from the client and verify that the payload built only includes `TxA`, and `TxB` is not included, nor the contract it could create is present in the `stateRoot`.
   - Create a modified payload where `TxA` is replaced by `TxB` and send using `engine_newPayloadV2`
-  - Verify that `engine_newPayloadV2` returns `INVALID` nad `latestValidHash` points to the latest valid payload in the canonical chain.
+  - Verify that `engine_newPayloadV2` returns `INVALID` nad `latestValidHash` points to the latest valid payload in the canonical chain.
+
+## Block Value Tests
+- Block Value on GetPayloadV2 Post-Shanghai
+  - Create a Shanghai chain where the fork transition happens at block 1
+  - Send transactions, submit forkchoice and get payload built
+  - Verify transactions were included in payload created
+  - Set forkchoice head to the new payload
+  - Calculate the block value by requesting the transaction receipts
+  - Verify that the `blockValue` returned by `engine_getPayloadV2` matches the expected calculated value

simulators/ethereum/engine/suites/withdrawals/tests.go

@@ -162,6 +162,20 @@ var Tests = []test.SpecInterface{
 		WithdrawalsPerBlock:   0,
 	},
 
+	// Block value tests
+	&BlockValueSpec{
+		WithdrawalsBaseSpec: &WithdrawalsBaseSpec{
+			Spec: test.Spec{
+				Name: "GetPayloadV2 Block Value",
+				About: `
+				Verify the block value returned in GetPayloadV2.
+				`,
+			},
+			WithdrawalsForkHeight: 1,
+			WithdrawalsBlockCount: 1,
+		},
+	},
+
 	// Sync Tests
 	&WithdrawalsSyncSpec{
 		WithdrawalsBaseSpec: &WithdrawalsBaseSpec{
@@ -739,14 +753,30 @@ func (ws *WithdrawalsBaseSpec) Execute(t *test.Env) {
 						HeadBlockHash: t.CLMock.LatestHeader.Hash(),
 					},
 					&beacon.PayloadAttributes{
-						Timestamp:             t.CLMock.LatestHeader.Time + 1,
+						Timestamp:             t.CLMock.LatestHeader.Time + ws.GetBlockTimeIncrements(),
 						Random:                common.Hash{},
 						SuggestedFeeRecipient: common.Address{},
 						Withdrawals:           make(types.Withdrawals, 0),
 					},
 				)
-				r.ExpectationDescription = "Sent pre-shanghai fcu using EngineForkchoiceUpdatedV2+Withdrawals, error is expected"
+				r.ExpectationDescription = "Sent pre-shanghai Forkchoice using ForkchoiceUpdatedV2 + Withdrawals, error is expected"
 				r.ExpectErrorCode(InvalidParamsError)
+
+				// Send a valid Pre-Shanghai request using ForkchoiceUpdatedV2
+				// (CLMock uses V1 by default)
+				r = t.TestEngine.TestEngineForkchoiceUpdatedV2(
+					&beacon.ForkchoiceStateV1{
+						HeadBlockHash: t.CLMock.LatestHeader.Hash(),
+					},
+					&beacon.PayloadAttributes{
+						Timestamp:             t.CLMock.LatestHeader.Time + ws.GetBlockTimeIncrements(),
+						Random:                common.Hash{},
+						SuggestedFeeRecipient: common.Address{},
+						Withdrawals:           nil,
+					},
+				)
+				r.ExpectationDescription = "Sent pre-shanghai Forkchoice ForkchoiceUpdatedV2 + null withdrawals, no error is expected"
+				r.ExpectNoError()
 			}
 		},
 		OnGetPayload: func() {
@@ -763,6 +793,11 @@ func (ws *WithdrawalsBaseSpec) Execute(t *test.Env) {
 				r := t.TestEngine.TestEngineNewPayloadV2(payloadPlusWithdrawals)
 				r.ExpectationDescription = "Sent pre-shanghai payload using NewPayloadV2+Withdrawals, error is expected"
 				r.ExpectErrorCode(InvalidParamsError)
+
+				// Send valid ExecutionPayloadV1 using engine_newPayloadV2
+				r = t.TestEngine.TestEngineNewPayloadV2(&t.CLMock.LatestPayloadBuilt)
+				r.ExpectationDescription = "Sent pre-shanghai payload using NewPayloadV2, no error is expected"
+				r.ExpectStatus(test.Valid)
 			}
 		},
 		OnNewPayloadBroadcast: func() {
@@ -790,6 +825,25 @@ func (ws *WithdrawalsBaseSpec) Execute(t *test.Env) {
 
 	t.CLMock.ProduceBlocks(int(ws.WithdrawalsBlockCount), clmock.BlockProcessCallbacks{
 		OnPayloadProducerSelected: func() {
+
+			if !ws.SkipBaseVerifications {
+				// Try to send a PayloadAttributesV1 with null withdrawals after
+				// Shanghai
+				r := t.TestEngine.TestEngineForkchoiceUpdatedV2(
+					&beacon.ForkchoiceStateV1{
+						HeadBlockHash: t.CLMock.LatestHeader.Hash(),
+					},
+					&beacon.PayloadAttributes{
+						Timestamp:             t.CLMock.LatestHeader.Time + ws.GetBlockTimeIncrements(),
+						Random:                common.Hash{},
+						SuggestedFeeRecipient: common.Address{},
+						Withdrawals:           nil,
+					},
+				)
+				r.ExpectationDescription = "Sent shanghai fcu using PayloadAttributesV1, error is expected"
+				r.ExpectErrorCode(InvalidParamsError)
+			}
+
 			// Send some withdrawals
 			t.CLMock.NextWithdrawals, nextIndex = ws.GenerateWithdrawalsForBlock(nextIndex, startAccount)
 			ws.WithdrawalsHistory[t.CLMock.CurrentPayloadNumber] = t.CLMock.NextWithdrawals
@@ -812,7 +866,38 @@ func (ws *WithdrawalsBaseSpec) Execute(t *test.Env) {
 			}
 		},
 		OnGetPayload: func() {
-			// TODO: Send new payload with `withdrawals=null` and expect error
+			if !ws.SkipBaseVerifications {
+				// Send invalid `ExecutionPayloadV1` by replacing withdrawals list
+				// with null, and client must respond with `InvalidParamsError`.
+				// Note that StateRoot is also incorrect but null withdrawals should
+				// be checked first instead of responding `INVALID`
+				nilWithdrawalsPayload, err := helper.CustomizePayload(&t.CLMock.LatestPayloadBuilt, &helper.CustomPayloadData{
+					RemoveWithdrawals: true,
+				})
+				if err != nil {
+					t.Fatalf("Unable to append withdrawals: %v", err)
+				}
+				r := t.TestEngine.TestEngineNewPayloadV2(nilWithdrawalsPayload)
+				r.ExpectationDescription = "Sent shanghai payload using ExecutionPayloadV1, error is expected"
+				r.ExpectErrorCode(InvalidParamsError)
+
+				// Verify the list of withdrawals returned on the payload built
+				// completely matches the list provided in the
+				// engine_forkchoiceUpdatedV2 method call
+				if sentList, ok := ws.WithdrawalsHistory[t.CLMock.CurrentPayloadNumber]; !ok {
+					panic("withdrawals sent list was not saved")
+				} else {
+					if len(sentList) != len(t.CLMock.LatestPayloadBuilt.Withdrawals) {
+						t.Fatalf("FAIL (%s): Incorrect list of withdrawals on built payload: want=%d, got=%d", t.TestName, len(sentList), len(t.CLMock.LatestPayloadBuilt.Withdrawals))
+					}
+					for i := 0; i < len(sentList); i++ {
+						if err := test.CompareWithdrawals(sentList[i], t.CLMock.LatestPayloadBuilt.Withdrawals[i]); err != nil {
+							t.Fatalf("FAIL (%s): Incorrect withdrawal on index %d: %v", t.TestName, i, err)
+						}
+					}
+
+				}
+			}
 		},
 		OnNewPayloadBroadcast: func() {
 			// Check withdrawal addresses and verify withdrawal balances
@@ -1390,3 +1475,37 @@ func (s *MaxInitcodeSizeSpec) Execute(t *test.Env) {
 		},
 	})
 }
+
+type BlockValueSpec struct {
+	*WithdrawalsBaseSpec
+}
+
+func (s *BlockValueSpec) Execute(t *test.Env) {
+	s.WithdrawalsBaseSpec.SkipBaseVerifications = true
+	s.WithdrawalsBaseSpec.Execute(t)
+
+	// Get the latest block and the transactions included
+	b := t.TestEngine.TestBlockByNumber(nil)
+	b.ExpectNoError()
+
+	totalValue := new(big.Int)
+	txs := b.Block.Transactions()
+	if len(txs) == 0 {
+		t.Fatalf("FAIL (%s): No transactions included in latest block", t.TestName)
+	}
+	for _, tx := range txs {
+		r := t.TestEngine.TestTransactionReceipt(tx.Hash())
+		r.ExpectNoError()
+
+		receipt := r.Receipt
+
+		gasUsed := new(big.Int).SetUint64(receipt.GasUsed)
+		txTip, _ := tx.EffectiveGasTip(b.Block.Header().BaseFee)
+		txTip.Mul(txTip, gasUsed)
+		totalValue.Add(totalValue, txTip)
+	}
+
+	if totalValue.Cmp(t.CLMock.LatestBlockValue) != 0 {
+		t.Fatalf("FAIL (%s): Unexpected block value returned on GetPayloadV2: want=%d, got=%d", t.TestName, totalValue, t.CLMock.LatestBlockValue)
+	}
+}

simulators/ethereum/engine/test/expect.go

@@ -1,6 +1,7 @@
 package test
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -390,6 +391,23 @@ func (exp *GetPayloadResponseExpectObject) ExpectTimestamp(expectedTimestamp uin
 	}
 }
 
+func CompareWithdrawals(want *types.Withdrawal, got *types.Withdrawal) error {
+	if want == nil && got != nil || want != nil && got == nil {
+		return fmt.Errorf("want=%v, got=%v", want, got)
+	}
+	if want != nil {
+		if want.Amount != got.Amount ||
+			!bytes.Equal(want.Address[:], got.Address[:]) ||
+			want.Index != got.Index ||
+			want.Validator != got.Validator {
+			wantStr, _ := json.MarshalIndent(want, "", " ")
+			gotStr, _ := json.MarshalIndent(got, "", " ")
+			return fmt.Errorf("want=%v, got=%v", wantStr, gotStr)
+		}
+	}
+	return nil
+}
+
 // BlockNumber
 type BlockNumberResponseExpectObject struct {
 	*ExpectEnv