op-node: Fix gossip validation for Jovian blocks (#17940)

Jovian blocks can have a non-zero blobGasUsed value, as the DA footprint
is stored in it.

Author: sebastianst

op-node/p2p/gossip.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/ethereum-optimism/optimism/op-node/rollup"
 	"github.com/ethereum-optimism/optimism/op-service/eth"
+	"github.com/ethereum-optimism/optimism/op-service/ptr"
 	opsigner "github.com/ethereum-optimism/optimism/op-service/signer"
 )
 
@@ -48,8 +49,10 @@ const (
 // Message domains, the msg id function uncompresses to keep data monomorphic,
 // but invalid compressed data will need a unique different id.
 
-var MessageDomainInvalidSnappy = [4]byte{0, 0, 0, 0}
-var MessageDomainValidSnappy = [4]byte{1, 0, 0, 0}
+var (
+	MessageDomainInvalidSnappy = [4]byte{0, 0, 0, 0}
+	MessageDomainValidSnappy   = [4]byte{1, 0, 0, 0}
+)
 
 type GossipSetupConfigurables interface {
 	PeerScoringParams() *ScoringParams
@@ -262,7 +265,6 @@ func (sb *seenBlocks) markSeen(h common.Hash) {
 }
 
 func BuildBlocksValidator(log log.Logger, cfg *rollup.Config, runCfg GossipRuntimeConfig, blockVersion eth.BlockVersion, gossipConf GossipSetupConfigurables) pubsub.ValidatorEx {
-
 	// Seen block hashes per block height
 	// uint64 -> *seenBlocks
 	blockHeightLRU, err := lru.New[uint64, *seenBlocks](1000)
@@ -380,15 +382,21 @@ func BuildBlocksValidator(log log.Logger, cfg *rollup.Config, runCfg GossipRunti
 		}
 
 		if blockVersion.HasBlobProperties() {
-			// [REJECT] if the block is on a topic >= V3 and has a blob gas used value that is not zero
-			if payload.BlobGasUsed == nil || *payload.BlobGasUsed != 0 {
-				log.Warn("payload is on v3 topic, but has non-zero blob gas used", "bad_hash", payload.BlockHash.String(), "blob_gas_used", payload.BlobGasUsed)
+			// [REJECT] if the block is on a topic >= V3 and has a nil blob gas used
+			if payload.BlobGasUsed == nil {
+				log.Warn("payload is on v3 topic, but has nil blob gas used", "bad_hash", payload.BlockHash.String())
+				return pubsub.ValidationReject
+				// [REJECT] if the block is on a topic >= V3 and has a non-zero blob gas used field pre-Jovian
+			} else if !cfg.IsDAFootprintBlockLimit(uint64(payload.Timestamp)) && *payload.BlobGasUsed != 0 {
+				log.Warn("payload is on v3 topic, but has non-zero blob gas used",
+					"bad_hash", payload.BlockHash.String(), "blob_gas_used", *payload.BlobGasUsed)
 				return pubsub.ValidationReject
 			}
 
 			// [REJECT] if the block is on a topic >= V3 and has an excess blob gas value that is not zero
 			if payload.ExcessBlobGas == nil || *payload.ExcessBlobGas != 0 {
-				log.Warn("payload is on v3 topic, but has non-zero excess blob gas", "bad_hash", payload.BlockHash.String(), "excess_blob_gas", payload.ExcessBlobGas)
+				log.Warn("payload is on v3 topic, but has non-zero excess blob gas",
+					"bad_hash", payload.BlockHash.String(), "excess_blob_gas", ptr.Str(payload.ExcessBlobGas))
 				return pubsub.ValidationReject
 			}
 		}
@@ -504,7 +512,7 @@ type publisher struct {
 var _ GossipOut = (*publisher)(nil)
 
 func combinePeers(allPeers ...[]peer.ID) []peer.ID {
-	var seen = make(map[peer.ID]bool)
+	seen := make(map[peer.ID]bool)
 	var res []peer.ID
 	for _, peers := range allPeers {
 		for _, p := range peers {
@@ -674,7 +682,6 @@ func newBlockTopic(ctx context.Context, topicId string, ps *pubsub.PubSub, log l
 		validator,
 		pubsub.WithValidatorTimeout(3*time.Second),
 		pubsub.WithValidatorConcurrency(4))
-
 	if err != nil {
 		return nil, fmt.Errorf("failed to register gossip topic: %w", err)
 	}
@@ -707,8 +714,10 @@ func newBlockTopic(ctx context.Context, topicId string, ps *pubsub.PubSub, log l
 	}, nil
 }
 
-type TopicSubscriber func(ctx context.Context, sub *pubsub.Subscription)
-type MessageHandler func(ctx context.Context, from peer.ID, msg any) error
+type (
+	TopicSubscriber func(ctx context.Context, sub *pubsub.Subscription)
+	MessageHandler  func(ctx context.Context, from peer.ID, msg any) error
+)
 
 func BlocksHandler(onBlock func(ctx context.Context, from peer.ID, msg *eth.ExecutionPayloadEnvelope) error) MessageHandler {
 	return func(ctx context.Context, from peer.ID, msg any) error {

op-node/p2p/gossip_test.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/ethereum-optimism/optimism/op-node/rollup"
 	"github.com/ethereum-optimism/optimism/op-service/eth"
+	"github.com/ethereum-optimism/optimism/op-service/ptr"
 	opsigner "github.com/ethereum-optimism/optimism/op-service/signer"
 	"github.com/ethereum-optimism/optimism/op-service/testlog"
 	"github.com/ethereum-optimism/optimism/op-service/testutils"
@@ -123,19 +124,19 @@ func createSignedP2Payload(payload MarshalSSZ, signer Signer, l2ChainID *big.Int
 	return snappy.Encode(nil, data), nil
 }
 
-func createExecutionPayload(w types.Withdrawals, withdrawalsRoot *common.Hash, excessGas, gasUsed *uint64) *eth.ExecutionPayload {
+func createExecutionPayload(w types.Withdrawals, withdrawalsRoot *common.Hash, excessBlobGas, blobGasUsed *uint64) *eth.ExecutionPayload {
 	return &eth.ExecutionPayload{
 		Timestamp:       hexutil.Uint64(time.Now().Unix()),
 		Withdrawals:     &w,
 		WithdrawalsRoot: withdrawalsRoot,
-		ExcessBlobGas:   (*eth.Uint64Quantity)(excessGas),
-		BlobGasUsed:     (*eth.Uint64Quantity)(gasUsed),
+		ExcessBlobGas:   (*eth.Uint64Quantity)(excessBlobGas),
+		BlobGasUsed:     (*eth.Uint64Quantity)(blobGasUsed),
 	}
 }
 
-func createEnvelope(h *common.Hash, w types.Withdrawals, withdrawalsRoot *common.Hash, excessGas, gasUsed *uint64) *eth.ExecutionPayloadEnvelope {
+func createEnvelope(h *common.Hash, w types.Withdrawals, withdrawalsRoot *common.Hash, excessBlobGas, blobGasUsed *uint64) *eth.ExecutionPayloadEnvelope {
 	return &eth.ExecutionPayloadEnvelope{
-		ExecutionPayload:      createExecutionPayload(w, withdrawalsRoot, excessGas, gasUsed),
+		ExecutionPayload:      createExecutionPayload(w, withdrawalsRoot, excessBlobGas, blobGasUsed),
 		ParentBeaconBlockRoot: h,
 	}
 }
@@ -157,7 +158,12 @@ func TestBlockValidator(t *testing.T) {
 	mockGossipConf := &mockGossipSetupConfigurablesWithThreshold{threshold: 60 * time.Second}
 	v2Validator := BuildBlocksValidator(testlog.Logger(t, log.LevelCrit), cfg, runCfg, eth.BlockV2, mockGossipConf)
 	v3Validator := BuildBlocksValidator(testlog.Logger(t, log.LevelCrit), cfg, runCfg, eth.BlockV3, mockGossipConf)
-	v4Validator := BuildBlocksValidator(testlog.Logger(t, log.LevelCrit), cfg, runCfg, eth.BlockV4, mockGossipConf)
+	v4Validator := BuildBlocksValidator(testlog.Logger(t, log.LevelDebug), cfg, runCfg, eth.BlockV4, mockGossipConf)
+	jovianCfg := &rollup.Config{
+		L2ChainID:  big.NewInt(100),
+		JovianTime: ptr.New(uint64(0)),
+	}
+	v4JovianValidator := BuildBlocksValidator(testlog.Logger(t, log.LevelCrit), jovianCfg, runCfg, eth.BlockV4, mockGossipConf)
 
 	zero, one := uint64(0), uint64(1)
 	beaconHash, withdrawalsRoot := common.HexToHash("0x1234"), common.HexToHash("0x9876")
@@ -174,8 +180,7 @@ func TestBlockValidator(t *testing.T) {
 		{"V3RejectExecutionPayload", v3Validator, pubsub.ValidationReject, createExecutionPayload(types.Withdrawals{}, nil, &zero, &zero)},
 	}
 
-	for _, tt := range payloadTests {
-		test := tt
+	for _, test := range payloadTests {
 		t.Run(fmt.Sprintf("ExecutionPayload_%s", test.name), func(t *testing.T) {
 			e := &eth.ExecutionPayloadEnvelope{ExecutionPayload: test.payload}
 			test.payload.BlockHash, _ = e.CheckBlockHash() // hack to generate the block hash easily.
@@ -198,10 +203,11 @@ func TestBlockValidator(t *testing.T) {
 		{"V3Valid", v3Validator, pubsub.ValidationAccept, createEnvelope(&beaconHash, types.Withdrawals{}, nil, &zero, &zero)},
 		{"V4Valid", v4Validator, pubsub.ValidationAccept, createEnvelope(&beaconHash, types.Withdrawals{}, &withdrawalsRoot, &zero, &zero)},
 		{"V4RejectNoWithdrawalRoot", v4Validator, pubsub.ValidationReject, createEnvelope(&beaconHash, types.Withdrawals{}, nil, &zero, &zero)},
+		{"V4AcceptNonZeroBlobGasUsedJovian", v4JovianValidator, pubsub.ValidationAccept, createEnvelope(&beaconHash, types.Withdrawals{}, &withdrawalsRoot, &zero, &one)},
+		// Note: v3+ test cases with nil blobGasUsed cannot easily be included because they already fail at the SSZ marshaling stage.
 	}
 
-	for _, tt := range envelopeTests {
-		test := tt
+	for _, test := range envelopeTests {
 		t.Run(fmt.Sprintf("ExecutionPayloadEnvelope_%s", test.name), func(t *testing.T) {
 			test.payload.ExecutionPayload.BlockHash, _ = test.payload.CheckBlockHash() // hack to generate the block hash easily.
 			data, err := createSignedP2Payload(test.payload, signer, cfg.L2ChainID)

op-service/ptr/ptr.go

@@ -0,0 +1,15 @@
+// Package ptr provides helper functions for working with pointers.
+package ptr
+
+import "fmt"
+
+func New[T any](v T) *T {
+	return &v
+}
+
+func Str[T any](v *T) string {
+	if v == nil {
+		return "<nil>"
+	}
+	return fmt.Sprintf("%v", *v)
+}