Merge pull request #8035 from ethereum-optimism/jg/require_empty_withdrawals

op-node: Check withdrawals hash in P2P validation

op-node/p2p/gossip.go

@@ -326,6 +326,12 @@ func BuildBlocksValidator(log log.Logger, cfg *rollup.Config, runCfg GossipRunti
 			return pubsub.ValidationReject
 		}
 
+		// [REJECT] if a V2 Block has non-empty withdrawals
+		if blockVersion == eth.BlockV2 && len(*payload.Withdrawals) != 0 {
+			log.Warn("payload is on v2 topic, but has non-empty withdrawals", "bad_hash", payload.BlockHash.String(), "withdrawal_count", len(*payload.Withdrawals))
+			return pubsub.ValidationReject
+		}
+
 		seen, ok := blockHeightLRU.Get(uint64(payload.BlockNumber))
 		if !ok {
 			seen = new(seenBlocks)

op-node/p2p/gossip_test.go

 package p2p
 
 import (
+	"bytes"
 	"context"
+	"fmt"
 	"math/big"
 	"testing"
+	"time"
 
 	"github.com/ethereum-optimism/optimism/op-e2e/e2eutils"
 	"github.com/ethereum-optimism/optimism/op-node/rollup"
+	"github.com/golang/snappy"
+
+	// "github.com/ethereum-optimism/optimism/op-service/eth"
+	"github.com/ethereum-optimism/optimism/op-service/eth"
 	"github.com/ethereum-optimism/optimism/op-service/testutils"
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/hexutil"
+	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/crypto"
 
 	"github.com/ethereum/go-ethereum/log"
 	pubsub "github.com/libp2p/go-libp2p-pubsub"
+	pubsub_pb "github.com/libp2p/go-libp2p-pubsub/pb"
 	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/stretchr/testify/require"
 
@@ -89,3 +99,65 @@ func TestVerifyBlockSignature(t *testing.T) {
 		require.Equal(t, pubsub.ValidationIgnore, result)
 	})
 }
+
+func createSignedP2Payload(payload *eth.ExecutionPayload, signer Signer, l2ChainID *big.Int) ([]byte, error) {
+	var buf bytes.Buffer
+	buf.Write(make([]byte, 65))
+	if _, err := payload.MarshalSSZ(&buf); err != nil {
+		return nil, fmt.Errorf("failed to encoded execution payload to publish: %w", err)
+	}
+	data := buf.Bytes()
+	payloadData := data[65:]
+	sig, err := signer.Sign(context.TODO(), SigningDomainBlocksV1, l2ChainID, payloadData)
+	if err != nil {
+		return nil, fmt.Errorf("failed to sign execution payload with signer: %w", err)
+	}
+	copy(data[:65], sig[:])
+
+	// compress the full message
+	// This also copies the data, freeing up the original buffer to go back into the pool
+	return snappy.Encode(nil, data), nil
+}
+
+// TestBlockValidator does some very basic tests of the p2p block validation logic
+func TestBlockValidator(t *testing.T) {
+	// Params Set 1: Create the validation function
+	cfg := &rollup.Config{
+		L2ChainID: big.NewInt(100),
+	}
+	secrets, err := e2eutils.DefaultMnemonicConfig.Secrets()
+	require.NoError(t, err)
+	runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: crypto.PubkeyToAddress(secrets.SequencerP2P.PublicKey)}
+	signer := &PreparedSigner{Signer: NewLocalSigner(secrets.SequencerP2P)}
+
+	// valFnV1 := BuildBlocksValidator(testlog.Logger(t, log.LvlCrit), rollupCfg, runCfg, eth.BlockV1)
+	valFnV2 := BuildBlocksValidator(testlog.Logger(t, log.LvlCrit), cfg, runCfg, eth.BlockV2)
+
+	// Params Set 2: Call the validation function
+	peerID := peer.ID("foo")
+
+	// Valid Case
+	payload := eth.ExecutionPayload{
+		Timestamp:   hexutil.Uint64(time.Now().Unix()),
+		Withdrawals: &types.Withdrawals{},
+	}
+	payload.BlockHash, _ = payload.CheckBlockHash() // hack to generate the block hash easily.
+	data, err := createSignedP2Payload(&payload, signer, cfg.L2ChainID)
+	require.NoError(t, err)
+	message := &pubsub.Message{Message: &pubsub_pb.Message{Data: data}}
+	res := valFnV2(context.TODO(), peerID, message)
+	require.Equal(t, res, pubsub.ValidationAccept)
+
+	// Invalid because non-empty withdrawals when Canyon is active
+	payload = eth.ExecutionPayload{
+		Timestamp:   hexutil.Uint64(time.Now().Unix()),
+		Withdrawals: &types.Withdrawals{&types.Withdrawal{Index: 1, Validator: 1}},
+	}
+	payload.BlockHash, _ = payload.CheckBlockHash()
+	data, err = createSignedP2Payload(&payload, signer, cfg.L2ChainID)
+	require.NoError(t, err)
+	message = &pubsub.Message{Message: &pubsub_pb.Message{Data: data}}
+	res = valFnV2(context.TODO(), peerID, message)
+	require.Equal(t, res, pubsub.ValidationReject)
+
+}

specs/derivation.md

@@ -694,9 +694,12 @@ equivalents. The `v2` methods are backwards compatible with `v1` payloads but su
 [`engine_getPayloadV2`]: exec-engine.md#engine_getpayloadv2
 [`engine_newPayloadV2`]: exec-engine.md#engine_newpayloadv2
 
-The execution payload is an object of type [`ExecutionPayloadV1`][eth-payload].
+The execution payload is an object of type [`ExecutionPayloadV2`][eth-payload].
 
-[eth-payload]: https://github.com/ethereum/execution-apis/blob/main/src/engine/paris.md#executionpayloadv1
+[eth-payload]: https://github.com/ethereum/execution-apis/blob/main/src/engine/shanghai.md#payloadattributesv2
+
+With V2 of the execution payload, before Canyon the withdrawals field is required to be nil. After Canyon the
+withdrawals field is required to be non-nil. The op-node should set the withdrawals field to be an empty list.
 
 #### Forkchoice synchronization
 

specs/rollup-node-p2p.md

@@ -291,6 +291,7 @@ An [extended-validator] checks the incoming messages as follows, in order of ope
 - `[REJECT]` if the `block_hash` in the `payload` is not valid
 - `[REJECT]` if the block is on the V1 topic and has withdrawals
 - `[REJECT]` if the block is on the V2 topic and does not have withdrawals
+- `[REJECT]` if the block is on the V2 topic and has a non-zero amount of withdrawals
 - `[REJECT]` if more than 5 different blocks have been seen with the same block height
 - `[IGNORE]` if the block has already been seen
 - `[REJECT]` if the signature by the sequencer is not valid