op-program: Fix l2 engine API to avoid fetching L2 blocks after the chain head...

op-program: Fix l2 engine API to avoid fetching L2 blocks after the chain head as part of importing.

op-e2e/actions/l2_engine_test.go

@@ -191,7 +191,7 @@ func TestL2EngineAPIFail(gt *testing.T) {
 }
 
 func TestEngineAPITests(t *testing.T) {
-	test.RunEngineAPITests(t, func() engineapi.EngineBackend {
+	test.RunEngineAPITests(t, func(t *testing.T) engineapi.EngineBackend {
 		jwtPath := e2eutils.WriteDefaultJWT(t)
 		dp := e2eutils.MakeDeployParams(t, defaultRollupTestParams)
 		sd := e2eutils.Setup(t, dp, defaultAlloc)

op-program/client/l2/engine_backend.go

@@ -22,6 +22,7 @@ type OracleBackedL2Chain struct {
 	oracle     Oracle
 	chainCfg   *params.ChainConfig
 	engine     consensus.Engine
+	oracleHead *types.Header
 	head       *types.Header
 	safe       *types.Header
 	finalized  *types.Header
@@ -47,6 +48,7 @@ func NewOracleBackedL2Chain(logger log.Logger, oracle Oracle, chainCfg *params.C
 		head:       head.Header(),
 		safe:       head.Header(),
 		finalized:  head.Header(),
+		oracleHead: head.Header(),
 		blocks:     make(map[common.Hash]*types.Block),
 		db:         NewOracleBackedDB(oracle),
 	}, nil
@@ -82,11 +84,7 @@ func (o *OracleBackedL2Chain) CurrentFinalBlock() *types.Header {
 }
 
 func (o *OracleBackedL2Chain) GetHeaderByHash(hash common.Hash) *types.Header {
-	block := o.GetBlockByHash(hash)
-	if block == nil {
-		return nil
-	}
-	return block.Header()
+	return o.GetBlockByHash(hash).Header()
 }
 
 func (o *OracleBackedL2Chain) GetBlockByHash(hash common.Hash) *types.Block {
@@ -96,15 +94,18 @@ func (o *OracleBackedL2Chain) GetBlockByHash(hash common.Hash) *types.Block {
 		return block
 	}
 	// Retrieve from the oracle
-	block = o.oracle.BlockByHash(hash)
-	if block == nil {
-		return nil
-	}
-	return block
+	return o.oracle.BlockByHash(hash)
 }
 
 func (o *OracleBackedL2Chain) GetBlock(hash common.Hash, number uint64) *types.Block {
-	block := o.GetBlockByHash(hash)
+	var block *types.Block
+	if o.oracleHead.Number.Uint64() < number {
+		// For blocks above the chain head, only consider newly built blocks
+		// Avoids requesting an unknown block from the oracle which would panic.
+		block = o.blocks[hash]
+	} else {
+		block = o.GetBlockByHash(hash)
+	}
 	if block == nil {
 		return nil
 	}
@@ -116,9 +117,6 @@ func (o *OracleBackedL2Chain) GetBlock(hash common.Hash, number uint64) *types.B
 
 func (o *OracleBackedL2Chain) GetHeader(hash common.Hash, u uint64) *types.Header {
 	block := o.GetBlock(hash, u)
-	if block == nil {
-		return nil
-	}
 	return block.Header()
 }
 

op-program/client/l2/engine_backend_test.go

@@ -42,17 +42,6 @@ func TestGetBlocks(t *testing.T) {
 	}
 }
 
-func TestUnknownBlock(t *testing.T) {
-	_, chain := setupOracleBackedChain(t, 1)
-	hash := common.HexToHash("0x556677881122")
-	blockNumber := uint64(1)
-	require.Nil(t, chain.GetBlockByHash(hash))
-	require.Nil(t, chain.GetHeaderByHash(hash))
-	require.Nil(t, chain.GetBlock(hash, blockNumber))
-	require.Nil(t, chain.GetHeader(hash, blockNumber))
-	require.False(t, chain.HasBlockAndState(hash, blockNumber))
-}
-
 func TestCanonicalHashNotFoundPastChainHead(t *testing.T) {
 	blocks, chain := setupOracleBackedChainWithLowerHead(t, 5, 3)
 
@@ -69,7 +58,7 @@ func TestCanonicalHashNotFoundPastChainHead(t *testing.T) {
 func TestAppendToChain(t *testing.T) {
 	blocks, chain := setupOracleBackedChainWithLowerHead(t, 4, 3)
 	newBlock := blocks[4]
-	require.Nil(t, chain.GetBlockByHash(newBlock.Hash()), "block unknown before being added")
+	require.Nil(t, chain.GetBlock(newBlock.Hash(), newBlock.NumberU64()), "block unknown before being added")
 
 	require.NoError(t, chain.InsertBlockWithoutSetHead(newBlock))
 	require.Equal(t, blocks[3].Header(), chain.CurrentHeader(), "should not update chain head yet")
@@ -113,9 +102,7 @@ func TestUpdateStateDatabaseWhenImportingBlock(t *testing.T) {
 
 	require.NotEqual(t, blocks[1].Root(), newBlock.Root(), "block should have modified world state")
 
-	require.Panics(t, func() {
-		_, _ = chain.StateAt(newBlock.Root())
-	}, "state from non-imported block should not be available")
+	require.False(t, chain.HasBlockAndState(newBlock.Root(), newBlock.NumberU64()), "state from non-imported block should not be available")
 
 	err = chain.InsertBlockWithoutSetHead(newBlock)
 	require.NoError(t, err)
@@ -181,7 +168,7 @@ func setupOracle(t *testing.T, blockCount int, headBlockNumber int) (*params.Cha
 	genesisBlock := l2Genesis.MustCommit(db)
 	blocks, _ := core.GenerateChain(chainCfg, genesisBlock, consensus, db, blockCount, func(i int, gen *core.BlockGen) {})
 	blocks = append([]*types.Block{genesisBlock}, blocks...)
-	oracle := newStubBlockOracle(blocks[:headBlockNumber+1], db)
+	oracle := newStubBlockOracle(t, blocks[:headBlockNumber+1], db)
 	return chainCfg, blocks, oracle
 }
 
@@ -213,23 +200,27 @@ type stubBlockOracle struct {
 	kvStateOracle
 }
 
-func newStubBlockOracle(chain []*types.Block, db ethdb.Database) *stubBlockOracle {
+func newStubBlockOracle(t *testing.T, chain []*types.Block, db ethdb.Database) *stubBlockOracle {
 	blocks := make(map[common.Hash]*types.Block, len(chain))
 	for _, block := range chain {
 		blocks[block.Hash()] = block
 	}
 	return &stubBlockOracle{
 		blocks:        blocks,
-		kvStateOracle: kvStateOracle{source: db},
+		kvStateOracle: kvStateOracle{t: t, source: db},
 	}
 }
 
 func (o stubBlockOracle) BlockByHash(blockHash common.Hash) *types.Block {
-	return o.blocks[blockHash]
+	block, ok := o.blocks[blockHash]
+	if !ok {
+		o.t.Fatalf("requested unknown block %s", blockHash)
+	}
+	return block
 }
 
 func TestEngineAPITests(t *testing.T) {
-	test.RunEngineAPITests(t, func() engineapi.EngineBackend {
+	test.RunEngineAPITests(t, func(t *testing.T) engineapi.EngineBackend {
 		_, chain := setupOracleBackedChain(t, 0)
 		return chain
 	})

op-program/client/l2/engineapi/l2_engine_api.go

@@ -301,7 +301,7 @@ func (ea *L2EngineAPI) NewPayloadV1(ctx context.Context, payload *eth.ExecutionP
 	}
 	// If we already have the block locally, ignore the entire execution and just
 	// return a fake success.
-	if block := ea.backend.GetBlockByHash(payload.BlockHash); block != nil {
+	if block := ea.backend.GetBlock(payload.BlockHash, uint64(payload.BlockNumber)); block != nil {
 		ea.log.Warn("Ignoring already known beacon payload", "number", payload.BlockNumber, "hash", payload.BlockHash, "age", common.PrettyAge(time.Unix(int64(block.Time()), 0)))
 		hash := block.Hash()
 		return &eth.PayloadStatusV1{Status: eth.ExecutionValid, LatestValidHash: &hash}, nil

op-program/client/l2/engineapi/test/l2_engine_api_tests.go

@@ -18,7 +18,7 @@ import (
 var gasLimit = eth.Uint64Quantity(30_000_000)
 var feeRecipient = common.Address{}
 
-func RunEngineAPITests(t *testing.T, createBackend func() engineapi.EngineBackend) {
+func RunEngineAPITests(t *testing.T, createBackend func(t *testing.T) engineapi.EngineBackend) {
 	t.Run("CreateBlock", func(t *testing.T) {
 		api := newTestHelper(t, createBackend)
 
@@ -292,10 +292,10 @@ type testHelper struct {
 	assert  *require.Assertions
 }
 
-func newTestHelper(t *testing.T, createBackend func() engineapi.EngineBackend) *testHelper {
+func newTestHelper(t *testing.T, createBackend func(t *testing.T) engineapi.EngineBackend) *testHelper {
 	logger := testlog.Logger(t, log.LvlDebug)
 	ctx := context.Background()
-	backend := createBackend()
+	backend := createBackend(t)
 	api := engineapi.NewL2EngineAPI(logger, backend)
 	test := &testHelper{
 		t:       t,