op-acceptance-tests: Edge cases with engine APIs

op-acceptance-tests/tests/sync/elsync/gap_clp2p/sync_test.go

@@ -1,6 +1,7 @@
 package gap_clp2p
 
 import (
+	"bytes"
 	"testing"
 
 	"github.com/ethereum-optimism/optimism/op-devstack/devtest"
@@ -68,3 +69,85 @@ func TestReachUnsafeTipByAppendingUnsafePayload(gt *testing.T) {
 	logger.Info("Second trial for appending payload until tip")
 	sys.L2CLB.AppendUnsafePayloadUntilTip(sys.L2ELB, sys.L2EL, 400)
 }
+
+// TestCLUnsafeNotRewoundOnInvalidDuringELSync verifies that the CL's unsafe head
+// is not rewound when the EL returns INVALID for a payload during EL sync.
+//
+// When the EL is still syncing and cannot append new blocks, ForkchoiceUpdate
+// returns SYNCING. In this state, the CL may continue to advance its unsafe head
+// as it processes new targets, creating temporary divergence from the EL.
+//
+// The test then crafts a payload that the EL can still validate—even though it is
+// not appendable to the EL's current head—by introducing a detectable fault in the
+// payload itself (e.g., malformed ExtraData). The CL relays this payload through
+// engine_newPayload, and the EL immediately responds INVALID based on intrinsic
+// payload validation. The EL does not advance or trigger sync for this payload,
+// and the CL's unsafe head remains unchanged, without rewinding.
+//
+// This confirms that an INVALID response during EL sync halts advancement but does
+// not cause the CL's unsafe head to regress, preserving the last known valid head
+// while maintaining correct Engine API semantics.
+func TestCLUnsafeNotRewoundOnInvalidDuringELSync(gt *testing.T) {
+	t := devtest.SerialT(gt)
+	sys := presets.NewSingleChainMultiNodeWithoutCheck(t)
+	logger := t.Logger()
+	require := t.Require()
+
+	// Advance few blocks to make sure reference node advanced
+	sys.L2CL.Advanced(types.LocalUnsafe, 7, 30)
+
+	// Restart L2CLB to always trigger an EL Sync
+	sys.L2CLB.Stop()
+	// Wipe out L2ELB state to start from genesis
+	sys.L2ELB.Stop()
+	sys.L2ELB.Start()
+	sys.L2CLB.Start()
+
+	// At this point, L2ELB has no ELP2P and no safe advancement because batcher is stopped
+	startNum := sys.L2ELB.BlockRefByLabel(eth.Unsafe).Number
+	sys.L2CLB.UnsafeHead().NumEqualTo(startNum)
+
+	attempts := 3
+	// Check CL and EL divergence when there is a unsafe gap
+	for _, gap := range []uint64{3, 5} {
+		targetNum := startNum + gap
+		sys.L2CLB.SignalTarget(sys.L2EL, targetNum)
+		sys.L2ELB.NotAdvanced(eth.Unsafe)
+		sys.L2ELB.UnsafeHead().NumEqualTo(startNum)
+		// Check FCU returns SYNCING
+		sys.L2ELB.ForkchoiceUpdate(sys.L2EL, targetNum, startNum, startNum, nil).Retry(attempts).ResultAllSyncing()
+		// Even though EL did not advance, CL advanced
+		sys.L2CLB.UnsafeHead().NumEqualTo(targetNum)
+		logger.Info("CL and EL diverged", "CL", targetNum, "EL", startNum)
+	}
+
+	// Inject invalid payload that can be only checked by the EL
+	// Must choose payload number after than CL unsafe to make the payload sent to EL
+	targetNum := sys.L2CLB.UnsafeHead().BlockRef.Number + 1
+	payload := sys.L2EL.PayloadByNumber(targetNum)
+	// inject fault to the payload
+	// Altering extradata makes EL return INVALID even if the EL does not have state to validate
+	// EL will not trigger EL Sync because EL already knows that the payload is INVALID
+	payload.ExecutionPayload.ExtraData = bytes.Repeat([]byte{0xFF}, 32)
+	newHash, ok := payload.CheckBlockHash()
+	require.False(ok)
+	logger.Info("Injected fault to payload", "newHash", newHash, "prevHash", payload.ExecutionPayload.BlockHash)
+	payload.ExecutionPayload.BlockHash = newHash
+	_, ok = payload.CheckBlockHash()
+	require.True(ok)
+	sys.L2CLB.PostUnsafePayload(payload)
+	sys.L2CLB.NotAdvanced(types.LocalUnsafe, attempts)
+	sys.L2ELB.NotAdvanced(eth.Unsafe)
+	// EL did not advance
+	sys.L2ELB.UnsafeHead().NumEqualTo(startNum)
+	// CL did not advance
+	sys.L2CLB.UnsafeHead().NumEqualTo(startNum + 5)
+	// Check newPayload returns INVALID
+	// ex) op-geth error msg: "ignoring bad block: holocene extraData should be 9 bytes, got 32"
+	sys.L2ELB.NewPayloadRaw(payload).IsInvalid()
+
+	t.Cleanup(func() {
+		sys.L2ELB.Start()
+		sys.L2CLB.Start()
+	})
+}

op-acceptance-tests/tests/sync/elsync/gap_elp2p/sync_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/ethereum-optimism/optimism/op-service/eth"
 	"github.com/ethereum-optimism/optimism/op-supervisor/supervisor/types"
 	"github.com/ethereum/go-ethereum"
+	"github.com/ethereum/go-ethereum/common"
 )
 
 // TestL2ELP2PCanonicalChainAdvancedByFCU verifies the interaction between NewPayload,
@@ -230,3 +231,242 @@ func TestL2ELP2PCanonicalChainAdvancedByFCU(gt *testing.T) {
 		sys.L2ELB.DisconnectPeerWith(sys.L2EL)
 	})
 }
+
+// TestELP2PFCUInvalidHash verifies that when an Execution Layer (EL) client
+// receives a Forkchoice Update (FCU) with an unknown head hash (invalid or
+// non-existent) during EL syncing, it remains in the "SYNCING" state and does
+// not advance its canonical chain.
+//
+// In this scenario, the node is EL syncing, and the target forkchoice head hash
+// does not exist in any connected EL peers. When the EL processes an FCU with
+// such an unknown hash, it attempts to fetch the corresponding block from peers
+// once. If the block cannot be retrieved, the EL reports SYNCING. The EL will not
+// retry automatically, but each subsequent FCU with the same unknown hash will
+// trigger another one-time fetch attempt, again resulting in SYNCING.
+//
+// This behavior ensures that the EL client safely handles invalid or unknown
+// forkchoice targets by consistently reporting SYNCING for each FCU attempt
+// and by avoiding advancement of the chain on invalid data.
+func TestELP2PFCUInvalidHash(gt *testing.T) {
+	t := devtest.SerialT(gt)
+	sys := presets.NewSingleChainMultiNodeWithoutCheck(t)
+	logger := t.Logger()
+	genesis := sys.L2ELB.BlockRefByNumber(0)
+
+	// Advance few blocks to make sure reference node advanced
+	sys.L2CL.Advanced(types.LocalUnsafe, 10, 30)
+
+	sys.L2CLB.Stop()
+
+	// At this point, L2ELB has no ELP2P, and L2CL connection
+	startNum := sys.L2ELB.BlockRefByLabel(eth.Unsafe).Number
+
+	// Peer to confirm EL Syncing is working
+	sys.L2ELB.PeerWith(sys.L2EL)
+
+	// Trigger EL Sync to valid hash
+	targetNum := startNum + 3
+	attempts := 5
+	sys.L2ELB.ForkchoiceUpdate(sys.L2EL, targetNum, 0, 0, nil).WaitUntilValid(attempts)
+	// head advanced
+	sys.L2ELB.UnsafeHead().NumEqualTo(targetNum)
+	logger.Info("Canonical chain advanced", "number", targetNum)
+
+	unsafeHashInvalid := common.MaxHash // must be non-existent invalid hash
+	// We retry FCU using the invalid hash
+	// The ELP2P enabled L2EL will ask other peers but fail to fetch the block with the invalid hash
+	// Example logs from L2EL(geth)
+	//  "Fetching the unknown forkchoice head from network"  hash=0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
+	//  "Fetching batch of headers"
+	//  "Could not retrieve unknown head from peers"
+	sys.L2ELB.ForkchoiceUpdateRaw(unsafeHashInvalid, genesis.Hash, genesis.Hash, nil).Retry(attempts).ResultAllSyncing()
+
+	sys.L2ELB.UnsafeHead().NumEqualTo(targetNum)
+	logger.Info("Canonical chain not advanced", "number", targetNum)
+
+	t.Cleanup(func() {
+		sys.L2CLB.Start()
+		sys.L2ELB.DisconnectPeerWith(sys.L2EL)
+	})
+}
+
+// TestSafeDoesNotAdvanceWhenUnsafeIsSyncing_NoELP2P verifies Engine API semantics
+// where ForkchoiceUpdate (FCU) validates the unsafe target first and, if the unsafe
+// head is not directly appendable (e.g., there is a gap), FCU returns SYNCING and
+// exits early without updating the safe head—even when the provided safe hash is
+// independently appendable.
+//
+// The presence or absence of EL P2P is not the core factor here. Disabling EL P2P
+// in this test simply makes the gap persist so the condition is observable. The
+// key behavior is that FCU's unsafe-first check causes an early return, so the safe
+// head is not bumped when the unsafe target cannot be immediately synced.
+//
+// This validates that safe head updates are contingent on the unsafe target passing
+// appendability/sync checks first, per Engine API behavior.
+func TestSafeDoesNotAdvanceWhenUnsafeIsSyncing_NoELP2P(gt *testing.T) {
+	t := devtest.SerialT(gt)
+	sys := presets.NewSingleChainMultiNodeWithoutCheck(t)
+	logger := t.Logger()
+
+	// Advance few blocks to make sure reference node advanced
+	sys.L2CL.Advanced(types.LocalUnsafe, 10, 30)
+
+	sys.L2CLB.Stop()
+
+	// At this point, L2ELB has no ELP2P, and L2CL connection
+	startNum := sys.L2ELB.BlockRefByLabel(eth.Unsafe).Number
+
+	// Try to advance non canonical chain
+	targetNum := startNum + 1
+	logger.Info("NewPayload", "target", targetNum)
+	sys.L2ELB.NewPayload(sys.L2EL, targetNum).IsValid()
+
+	// FCU to advance unsafe and safe normally, promoting non canonical chain to canonical
+	logger.Info("ForkchoiceUpdate", "target", targetNum)
+	sys.L2ELB.ForkchoiceUpdate(sys.L2EL, targetNum, targetNum, 0, nil).IsValid()
+	sys.L2ELB.UnsafeHead().NumEqualTo(targetNum)
+	sys.L2ELB.SafeHead().NumEqualTo(targetNum)
+	logger.Info("Canonical chain advanced for unsafe and safe", "number", targetNum)
+
+	// Try to advance non canonical chain
+	safeTargetNum := startNum + 2
+	logger.Info("NewPayload", "target", safeTargetNum)
+	sys.L2ELB.NewPayload(sys.L2EL, safeTargetNum).IsValid()
+
+	// FCU safe normally, but target unsafe which cannot be synced because of the gap
+	unsafeTargetNum := safeTargetNum + 5
+	attempts := 5
+	logger.Info("ForkchoiceUpdate", "safeTarget", safeTargetNum, "unsafeTarget", unsafeTargetNum)
+	sys.L2ELB.ForkchoiceUpdate(sys.L2EL, unsafeTargetNum, safeTargetNum, 0, nil).Retry(attempts).ResultAllSyncing()
+	sys.L2ELB.UnsafeHead().NumEqualTo(targetNum)
+	sys.L2ELB.SafeHead().NumEqualTo(targetNum)
+	logger.Info("Canonical chain not advanced for unsafe and safe", "number", targetNum)
+
+	// Try to advance non canonical chain
+	safeTargetNum = startNum + 3
+	logger.Info("NewPayload", "target", safeTargetNum)
+	sys.L2ELB.NewPayload(sys.L2EL, safeTargetNum).IsValid()
+
+	// FCU safe normally, but target unsafe which cannot be synced because of the gap
+	unsafeTargetNum = safeTargetNum + 6
+	logger.Info("ForkchoiceUpdate", "safeTarget", safeTargetNum, "unsafeTarget", unsafeTargetNum)
+	sys.L2ELB.ForkchoiceUpdate(sys.L2EL, unsafeTargetNum, safeTargetNum, 0, nil).Retry(attempts).ResultAllSyncing()
+	sys.L2ELB.UnsafeHead().NumEqualTo(targetNum)
+	sys.L2ELB.SafeHead().NumEqualTo(targetNum)
+	logger.Info("Canonical chain not advanced for unsafe and safe", "number", targetNum)
+
+	// Enable EL P2P to update both unsafe and safe at once using EL Sync
+	sys.L2ELB.PeerWith(sys.L2EL)
+	logger.Info("ForkchoiceUpdate", "safeTarget", safeTargetNum, "unsafeTarget", unsafeTargetNum)
+	sys.L2ELB.ForkchoiceUpdate(sys.L2EL, unsafeTargetNum, safeTargetNum, 0, nil).WaitUntilValid(attempts)
+	sys.L2ELB.UnsafeHead().NumEqualTo(unsafeTargetNum)
+	sys.L2ELB.SafeHead().NumEqualTo(safeTargetNum)
+	logger.Info("Canonical chain advanced for unsafe and safe", "safeTarget", safeTargetNum, "unsafeTarget", unsafeTargetNum)
+
+	t.Cleanup(func() {
+		sys.L2CLB.Start()
+		sys.L2ELB.DisconnectPeerWith(sys.L2EL)
+	})
+}
+
+// TestInvalidPayloadThroughCLP2P verifies that invalid L2 payloads propagated via
+// CL P2P (simulated with admin_postUnsafePayload) do not advance either the CL or EL.
+//
+// The test first confirms normal progress on a valid target, then exercises three
+// invalid cases and asserts no advancement on both sides (unsafe head remains at
+// startNum+1):
+//
+//  1. CL-detectable invalidity (bad block hash):
+//     The payload is mutated (e.g., StateRoot) without updating BlockHash.
+//     The CL rejects it immediately (hash mismatch) and does not relay it to the EL.
+//
+//  2. EL-only invalidity (bad state root):
+//     The payload's BlockHash is recomputed so the CL relays it via engine_newPayload,
+//     but the EL rejects it during execution due to an invalid state root.
+//
+//  3. EL-only invalidity via invalid parent:
+//     A new payload builds on a previously rejected block (an invalid parent),
+//     causing the EL to reject it as referencing an invalid ancestor.
+//
+// In all scenarios, both CL and EL remain at the same head height, confirming that
+// invalid payloads—whether rejected at the CL or EL—do not advance the chain.
+func TestInvalidPayloadThroughCLP2P(gt *testing.T) {
+	t := devtest.SerialT(gt)
+	sys := presets.NewSingleChainMultiNodeWithoutCheck(t)
+	logger := t.Logger()
+	require := t.Require()
+	ctx := t.Ctx()
+
+	// Advance few blocks to make sure reference node advanced
+	sys.L2CL.Advanced(types.LocalUnsafe, 4, 30)
+
+	// At this point, L2ELB has no ELP2P, and L2CL connection
+	startNum := sys.L2ELB.BlockRefByLabel(eth.Unsafe).Number
+
+	// We check L2ELB can be advanced using the valid payload first
+	attempts := 3
+	targetNum := startNum + 1
+	sys.L2CLB.SignalTarget(sys.L2EL, targetNum)
+	sys.L2ELB.Reached(eth.Unsafe, targetNum, attempts)
+	logger.Info("Canonical chain advanced", "number", targetNum)
+
+	// Assume sequencer crafted invalid payload and broadcasted via P2P
+	// Simulate the situation using the admin_postUnsafePayload API
+
+	// Scenario 1: Invalid Payload can be checked at the CL side
+	targetNum = startNum + 2
+	payload := sys.L2EL.PayloadByNumber(targetNum)
+	// inject fault to the payload
+	payload.ExecutionPayload.StateRoot = eth.Bytes32{}
+	logger.Info("Injected fault to payload but not updated hash")
+	// Post invalid payload with the fault that can be checked at the CL side
+	require.Error(sys.L2CLB.Escape().RollupAPI().PostUnsafePayload(ctx, payload))
+	// ex) op-node error msg: "payload has bad block hash"
+	// CL will not send the payload but drop immediately due to hash mismatch
+	// EL did not advance
+	sys.L2ELB.UnsafeHead().NumEqualTo(startNum + 1)
+	// CL did not advance
+	sys.L2CLB.UnsafeHead().NumEqualTo(startNum + 1)
+
+	// Scenario 2: Invalid Payload can be only checked at the EL side
+	// Make sure to update the block hash included at payload to CL relay the payload to the EL
+	newHash, ok := payload.CheckBlockHash()
+	require.False(ok)
+	logger.Info("Injected fault to payload", "newHash", newHash, "prevHash", payload.ExecutionPayload.BlockHash)
+	payload.ExecutionPayload.BlockHash = newHash
+	_, ok = payload.CheckBlockHash()
+	require.True(ok)
+	// L2CLB will relay the payload to the L2ELB using the engine_newPayload
+	// L2ELB will return INVALID because payload is invalid, due to wrong stateRoot
+	// ex) op-geth will call InsertBlockWithoutSetHead() to execute the payload while engine_newPayload
+	// Post invalid payload with the fault that can be only checked at the EL side
+	sys.L2CLB.PostUnsafePayload(payload)
+	// ex) op-geth error msg: "ignoring bad block: invalid merkle root"
+	sys.L2CLB.NotAdvanced(types.LocalUnsafe, attempts)
+	sys.L2ELB.NotAdvanced(eth.Unsafe)
+	// EL did not advance
+	sys.L2ELB.UnsafeHead().NumEqualTo(startNum + 1)
+	// CL did not advance
+	sys.L2CLB.UnsafeHead().NumEqualTo(startNum + 1)
+
+	// Scenario 3: Invalid Payload can be only checked at the EL side, invalid because invalid parent
+	// Try to build on top of previously rejected block with block number startNum + 2
+	targetNum = startNum + 3
+	payload2 := sys.L2EL.PayloadByNumber(targetNum)
+	payload2.ExecutionPayload.ParentHash = payload.ExecutionPayload.BlockHash
+	newHash, ok = payload2.CheckBlockHash()
+	require.False(ok)
+	logger.Info("Updated payload parent to invalid payload", "newHash", newHash)
+	payload2.ExecutionPayload.BlockHash = newHash
+	_, ok = payload.CheckBlockHash()
+	require.True(ok)
+	// Post invalid payload with the fault that can be only checked at the EL side
+	sys.L2CLB.PostUnsafePayload(payload)
+	// ex) op-geth error msg: "ignoring bad block: links to previously rejected block"
+	sys.L2CLB.NotAdvanced(types.LocalUnsafe, attempts)
+	sys.L2ELB.NotAdvanced(eth.Unsafe)
+	// EL did not advance
+	sys.L2ELB.UnsafeHead().NumEqualTo(startNum + 1)
+	// CL did not advance
+	sys.L2CLB.UnsafeHead().NumEqualTo(startNum + 1)
+}