op-node: fix Ecotone beacon-block-root contract deployment (#9216)

* op-node: fix Ecotone beacon-block-root contract deployment

* op-e2e: update time chekc in op-e2e/actions/ecotone_fork_test.go
Co-authored-by: Sebastian Stammler <seb@oplabs.co>

* op-e2e: deduplicate code-hash checking

* op-e2e: ecotone-test update: require instead of assert

* op-e2e: ecotone test L1 fee check comments clarification

---------
Co-authored-by: Sebastian Stammler <seb@oplabs.co>

op-bindings/predeploys/eip4788.go

@@ -11,5 +11,6 @@ import "github.com/ethereum/go-ethereum/common"
 // And https://eips.ethereum.org/EIPS/eip-4788
 var (
 	EIP4788ContractAddr     = common.HexToAddress("0x000F3df6D732807Ef1319fB7B8bB8522d0Beac02")
-	EIP4788ContractCode = common.Hex2Bytes("0x3373fffffffffffffffffffffffffffffffffffffffe14604d57602036146024575f5ffd5b5f35801560495762001fff810690815414603c575f5ffd5b62001fff01545f5260205ff35b5f5ffd5b62001fff42064281555f359062001fff015500")
+	EIP4788ContractCode     = common.FromHex("0x3373fffffffffffffffffffffffffffffffffffffffe14604d57602036146024575f5ffd5b5f35801560495762001fff810690815414603c575f5ffd5b62001fff01545f5260205ff35b5f5ffd5b62001fff42064281555f359062001fff015500")
+	EIP4788ContractCodeHash = common.HexToHash("0xf57acd40259872606d76197ef052f3d35588dadf919ee1f0e3cb9b62d3f4b02c")
 )

op-e2e/actions/ecotone_fork_test.go

@@ -5,7 +5,6 @@ import (
 	"math/big"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -24,15 +23,20 @@ import (
 )
 
 var (
-	l1BlockCodeHash        = common.FromHex("0xc88a313aa75dc4fbf0b6850d9f9ae41e04243b7008cf3eadb29256d4a71c1dfd")
-	gasPriceOracleCodeHash = common.FromHex("0x8b71360ea773b4cfaf1ae6d2bd15464a4e1e2e360f786e475f63aeaed8da0ae5")
+	l1BlockCodeHash        = common.HexToHash("0xc88a313aa75dc4fbf0b6850d9f9ae41e04243b7008cf3eadb29256d4a71c1dfd")
+	gasPriceOracleCodeHash = common.HexToHash("0x8b71360ea773b4cfaf1ae6d2bd15464a4e1e2e360f786e475f63aeaed8da0ae5")
 )
 
-func verifyCodeHashMatches(t *testing.T, client *ethclient.Client, address common.Address, expectedCodeHash []byte) {
+// verifyCodeHashMatches checks that the has of the code at the given address matches the expected code-hash.
+// It also sanity-checks that the code is not empty: we should never deploy empty contract codes.
+// Returns the contract code
+func verifyCodeHashMatches(t Testing, client *ethclient.Client, address common.Address, expectedCodeHash common.Hash) []byte {
 	code, err := client.CodeAt(context.Background(), address, nil)
 	require.NoError(t, err)
+	require.NotEmpty(t, code)
 	codeHash := crypto.Keccak256Hash(code)
-	require.Equal(t, expectedCodeHash, codeHash.Bytes())
+	require.Equal(t, expectedCodeHash, codeHash)
+	return code
 }
 
 func TestEcotoneNetworkUpgradeTransactions(gt *testing.T) {
@@ -53,13 +57,14 @@ func TestEcotoneNetworkUpgradeTransactions(gt *testing.T) {
 	sd := e2eutils.Setup(t, dp, defaultAlloc)
 	log := testlog.Logger(t, log.LvlDebug)
 	_, _, miner, sequencer, engine, verifier, _, _ := setupReorgTestActors(t, dp, sd, log)
+	ethCl := engine.EthClient()
 
 	// start op-nodes
 	sequencer.ActL2PipelineFull(t)
 	verifier.ActL2PipelineFull(t)
 
 	// Get gas price from oracle
-	gasPriceOracle, err := bindings.NewGasPriceOracleCaller(predeploys.GasPriceOracleAddr, engine.EthClient())
+	gasPriceOracle, err := bindings.NewGasPriceOracleCaller(predeploys.GasPriceOracleAddr, ethCl)
 	require.NoError(t, err)
 
 	scalar, err := gasPriceOracle.Scalar(nil)
@@ -68,16 +73,16 @@ func TestEcotoneNetworkUpgradeTransactions(gt *testing.T) {
 	require.True(t, scalar.Cmp(new(big.Int).SetUint64(dp.DeployConfig.GasPriceOracleScalar)) == 0, "must match deploy config")
 
 	// Get current implementations addresses (by slot) for L1Block + GasPriceOracle
-	initialGasPriceOracleAddress, err := engine.EthClient().StorageAt(context.Background(), predeploys.GasPriceOracleAddr, genesis.ImplementationSlot, nil)
+	initialGasPriceOracleAddress, err := ethCl.StorageAt(context.Background(), predeploys.GasPriceOracleAddr, genesis.ImplementationSlot, nil)
 	require.NoError(t, err)
-	initialL1BlockAddress, err := engine.EthClient().StorageAt(context.Background(), predeploys.L1BlockAddr, genesis.ImplementationSlot, nil)
+	initialL1BlockAddress, err := ethCl.StorageAt(context.Background(), predeploys.L1BlockAddr, genesis.ImplementationSlot, nil)
 	require.NoError(t, err)
 
 	// Build to the ecotone block
 	sequencer.ActBuildL2ToEcotone(t)
 
 	// get latest block
-	latestBlock, err := engine.EthClient().BlockByNumber(context.Background(), nil)
+	latestBlock, err := ethCl.BlockByNumber(context.Background(), nil)
 	require.NoError(t, err)
 	require.Equal(t, sequencer.L2Unsafe().Number, latestBlock.Number().Uint64())
 
@@ -94,35 +99,37 @@ func TestEcotoneNetworkUpgradeTransactions(gt *testing.T) {
 	// All transactions are successful
 	for i := 1; i < 7; i++ {
 		txn := transactions[i]
-		receipt, err := engine.EthClient().TransactionReceipt(context.Background(), txn.Hash())
+		receipt, err := ethCl.TransactionReceipt(context.Background(), txn.Hash())
 		require.NoError(t, err)
 		require.Equal(t, types.ReceiptStatusSuccessful, receipt.Status)
+		require.NotEmpty(t, txn.Data(), "upgrade tx must provide input data")
 	}
 
 	expectedL1BlockAddress := crypto.CreateAddress(derive.L1BlockDeployerAddress, 0)
 	expectedGasPriceOracleAddress := crypto.CreateAddress(derive.GasPriceOracleDeployerAddress, 0)
 
 	// Gas Price Oracle Proxy is updated
-	updatedGasPriceOracleAddress, err := engine.EthClient().StorageAt(context.Background(), predeploys.GasPriceOracleAddr, genesis.ImplementationSlot, latestBlock.Number())
+	updatedGasPriceOracleAddress, err := ethCl.StorageAt(context.Background(), predeploys.GasPriceOracleAddr, genesis.ImplementationSlot, latestBlock.Number())
 	require.NoError(t, err)
-	assert.Equal(t, expectedGasPriceOracleAddress, common.BytesToAddress(updatedGasPriceOracleAddress))
-	assert.NotEqualf(t, initialGasPriceOracleAddress, updatedGasPriceOracleAddress, "Gas Price Oracle Proxy address should have changed")
-	verifyCodeHashMatches(gt, engine.EthClient(), expectedGasPriceOracleAddress, gasPriceOracleCodeHash)
+	require.Equal(t, expectedGasPriceOracleAddress, common.BytesToAddress(updatedGasPriceOracleAddress))
+	require.NotEqualf(t, initialGasPriceOracleAddress, updatedGasPriceOracleAddress, "Gas Price Oracle Proxy address should have changed")
+	verifyCodeHashMatches(t, ethCl, expectedGasPriceOracleAddress, gasPriceOracleCodeHash)
 
 	// L1Block Proxy is updated
-	updatedL1BlockAddress, err := engine.EthClient().StorageAt(context.Background(), predeploys.L1BlockAddr, genesis.ImplementationSlot, latestBlock.Number())
+	updatedL1BlockAddress, err := ethCl.StorageAt(context.Background(), predeploys.L1BlockAddr, genesis.ImplementationSlot, latestBlock.Number())
 	require.NoError(t, err)
-	assert.Equal(t, expectedL1BlockAddress, common.BytesToAddress(updatedL1BlockAddress))
-	assert.NotEqualf(t, initialL1BlockAddress, updatedL1BlockAddress, "L1Block Proxy address should have changed")
-	verifyCodeHashMatches(gt, engine.EthClient(), expectedL1BlockAddress, l1BlockCodeHash)
+	require.Equal(t, expectedL1BlockAddress, common.BytesToAddress(updatedL1BlockAddress))
+	require.NotEqualf(t, initialL1BlockAddress, updatedL1BlockAddress, "L1Block Proxy address should have changed")
+	verifyCodeHashMatches(t, ethCl, expectedL1BlockAddress, l1BlockCodeHash)
 
 	_, err = gasPriceOracle.Scalar(nil)
 	require.ErrorContains(t, err, "scalar() is deprecated")
 
 	cost, err := gasPriceOracle.GetL1Fee(nil, []byte{0, 1, 2, 3, 4})
 	require.NoError(t, err)
-	// Pre-ecotone the GPO getL1Fee contract erroneously returned the pre-regolith L1 fee.
-	// Thus we do not assert the exact value here.
+	// The L1 info tx does not get included until after the Ecotone upgrade.
+	// The scalars are thus empty during activation, and only deposits are included, so the L1 fee is unused.
+	require.True(t, cost.IsUint64())
 	require.Equal(t, cost.Uint64(), uint64(0), "expecting zero scalars within activation block")
 
 	// Check that Ecotone was activated
@@ -132,15 +139,47 @@ func TestEcotoneNetworkUpgradeTransactions(gt *testing.T) {
 
 	// 4788 contract is deployed
 	expected4788Address := crypto.CreateAddress(derive.EIP4788From, 0)
-	code, err := engine.EthClient().CodeAt(context.Background(), expected4788Address, latestBlock.Number())
+	require.Equal(t, predeploys.EIP4788ContractAddr, expected4788Address)
+	code := verifyCodeHashMatches(t, ethCl, predeploys.EIP4788ContractAddr, predeploys.EIP4788ContractCodeHash)
+	require.Equal(t, predeploys.EIP4788ContractCode, code)
+	// Test that the beacon-block-root has been set
+	checkBeaconBlockRoot := func(timestamp uint64, expectedHash common.Hash, expectedTime uint64, msg string) {
+		historyBufferLength := uint64(8191)
+		rootIdx := common.BigToHash(new(big.Int).SetUint64((timestamp % historyBufferLength) + historyBufferLength))
+		timeIdx := common.BigToHash(new(big.Int).SetUint64(timestamp % historyBufferLength))
+
+		rootValue, err := ethCl.StorageAt(context.Background(), predeploys.EIP4788ContractAddr, rootIdx, nil)
 		require.NoError(t, err)
-	require.Equal(t, code, predeploys.EIP4788ContractCode)
+		require.Equal(t, expectedHash, common.BytesToHash(rootValue), msg)
+
+		timeValue, err := ethCl.StorageAt(context.Background(), predeploys.EIP4788ContractAddr, timeIdx, nil)
+		require.NoError(t, err)
+		timeBig := new(big.Int).SetBytes(timeValue)
+		require.True(t, timeBig.IsUint64())
+		require.Equal(t, expectedTime, timeBig.Uint64(), msg)
+	}
+	// The header will always have the beacon-block-root, at the very start.
+	require.NotNil(t, latestBlock.BeaconRoot())
+	require.Equal(t, *latestBlock.BeaconRoot(), common.Hash{},
+		"L1 genesis block has zeroed parent-beacon-block-root, since it has no parent block, and that propagates into L2")
+	// The first block is an exception in upgrade-networks,
+	// since the beacon-block root contract isn't there at Ecotone activation,
+	// and the beacon-block-root insertion is processed at the start of the block before deposit txs.
+	// If the contract was permissionlessly deployed before, the contract storage will be updated however.
+	checkBeaconBlockRoot(latestBlock.Time(), common.Hash{}, 0, "ecotone activation block has no data yet (since contract wasn't there)")
 
 	// Build empty L2 block, to pass ecotone activation
 	sequencer.ActL2StartBlock(t)
 	sequencer.ActL2EndBlock(t)
 
-	// assert again, now that we are past activation
+	// Test the L2 block after activation: it should have data in the contract storage now
+	latestBlock, err = ethCl.BlockByNumber(context.Background(), nil)
+	require.NoError(t, err)
+	require.NotNil(t, latestBlock.BeaconRoot())
+	firstBeaconBlockRoot := *latestBlock.BeaconRoot()
+	checkBeaconBlockRoot(latestBlock.Time(), *latestBlock.BeaconRoot(), latestBlock.Time(), "post-activation")
+
+	// require.again, now that we are past activation
 	_, err = gasPriceOracle.Scalar(nil)
 	require.ErrorContains(t, err, "scalar() is deprecated")
 
@@ -151,14 +190,16 @@ func TestEcotoneNetworkUpgradeTransactions(gt *testing.T) {
 
 	cost, err = gasPriceOracle.GetL1Fee(nil, []byte{0, 1, 2, 3, 4})
 	require.NoError(t, err)
+	// The GPO getL1Fee contract returns the L1 fee with approximate signature overhead pre-included,
+	// like the pre-regolith L1 fee. We do the full fee check below. Just sanity check it is not zero anymore first.
 	require.Greater(t, cost.Uint64(), uint64(0), "expecting non-zero scalars after activation block")
 
 	// Get L1Block info
-	l1Block, err := bindings.NewL1BlockCaller(predeploys.L1BlockAddr, engine.EthClient())
+	l1Block, err := bindings.NewL1BlockCaller(predeploys.L1BlockAddr, ethCl)
 	require.NoError(t, err)
 	l1BlockInfo, err := l1Block.Timestamp(nil)
 	require.NoError(t, err)
-	assert.Greater(t, l1BlockInfo, uint64(0))
+	require.Greater(t, l1BlockInfo, uint64(0))
 
 	l1OriginBlock, err := miner.EthClient().BlockByHash(context.Background(), sequencer.L2Unsafe().L1Origin.Hash)
 	require.NoError(t, err)
@@ -174,6 +215,18 @@ func TestEcotoneNetworkUpgradeTransactions(gt *testing.T) {
 	expectedL1Fee = expectedL1Fee.Mul(expectedL1Fee, new(big.Int).SetUint64(uint64(basefeeScalar)))
 	expectedL1Fee = expectedL1Fee.Div(expectedL1Fee, big.NewInt(16e6))
 	require.Equal(t, expectedL1Fee, cost, "expecting cost based on regular base fee scalar alone")
+
+	// build forward, incorporate new L1 data
+	miner.ActEmptyBlock(t)
+	sequencer.ActL1HeadSignal(t)
+	sequencer.ActBuildToL1Head(t)
+
+	// Contract storage should be updated now, different than before
+	latestBlock, err = ethCl.BlockByNumber(context.Background(), nil)
+	require.NoError(t, err)
+	require.NotNil(t, latestBlock.BeaconRoot())
+	require.NotEqual(t, firstBeaconBlockRoot, *latestBlock.BeaconRoot())
+	checkBeaconBlockRoot(latestBlock.Time(), *latestBlock.BeaconRoot(), latestBlock.Time(), "updates on new L1 data")
 }
 
 // TestEcotoneBeforeL1 tests that the L2 Ecotone fork can activate before L1 Dencun does

op-e2e/actions/l1_miner.go

@@ -11,6 +11,7 @@ import (
 	"github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/go-ethereum/core/state"
 	"github.com/ethereum/go-ethereum/core/types"
+	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/ethereum/go-ethereum/trie"
 
@@ -94,7 +95,8 @@ func (s *L1Miner) ActL1StartBlock(timeDelta uint64) Action {
 		if s.l1Cfg.Config.IsCancun(header.Number, header.Time) {
 			header.BlobGasUsed = new(uint64)
 			header.ExcessBlobGas = new(uint64)
-			header.ParentBeaconRoot = new(common.Hash)
+			root := crypto.Keccak256Hash([]byte("fake-beacon-block-root"), header.Number.Bytes())
+			header.ParentBeaconRoot = &root
 		}
 
 		s.l1Building = true

op-node/rollup/derive/ecotone_upgrade_transactions.go

@@ -34,7 +34,7 @@ var (
 	enableEcotoneInput = crypto.Keccak256([]byte("setEcotone()"))[:4]
 
 	EIP4788From         = common.HexToAddress("0x0B799C86a49DEeb90402691F1041aa3AF2d3C875")
-	eip4788CreationData = common.Hex2Bytes("0x60618060095f395ff33373fffffffffffffffffffffffffffffffffffffffe14604d57602036146024575f5ffd5b5f35801560495762001fff810690815414603c575f5ffd5b62001fff01545f5260205ff35b5f5ffd5b62001fff42064281555f359062001fff015500")
+	eip4788CreationData = common.FromHex("0x60618060095f395ff33373fffffffffffffffffffffffffffffffffffffffe14604d57602036146024575f5ffd5b5f35801560495762001fff810690815414603c575f5ffd5b62001fff01545f5260205ff35b5f5ffd5b62001fff42064281555f359062001fff015500")
 	UpgradeToFuncBytes4 = crypto.Keccak256([]byte(UpgradeToFuncSignature))[:4]
 
 	l1BlockDeploymentBytecode        = common.FromHex("0x608060405234801561001057600080fd5b5061053e806100206000396000f3fe608060405234801561001057600080fd5b50600436106100f55760003560e01c80638381f58a11610097578063c598591811610066578063c598591814610229578063e591b28214610249578063e81b2c6d14610289578063f82061401461029257600080fd5b80638381f58a146101e35780638b239f73146101f75780639e8c496614610200578063b80777ea1461020957600080fd5b806354fd4d50116100d357806354fd4d50146101335780635cf249691461017c57806364ca23ef1461018557806368d5dca6146101b257600080fd5b8063015d8eb9146100fa57806309bd5a601461010f578063440a5e201461012b575b600080fd5b61010d61010836600461044c565b61029b565b005b61011860025481565b6040519081526020015b60405180910390f35b61010d6103da565b61016f6040518060400160405280600581526020017f312e322e3000000000000000000000000000000000000000000000000000000081525081565b60405161012291906104be565b61011860015481565b6003546101999067ffffffffffffffff1681565b60405167ffffffffffffffff9091168152602001610122565b6003546101ce9068010000000000000000900463ffffffff1681565b60405163ffffffff9091168152602001610122565b6000546101999067ffffffffffffffff1681565b61011860055481565b61011860065481565b6000546101999068010000000000000000900467ffffffffffffffff1681565b6003546101ce906c01000000000000000000000000900463ffffffff1681565b61026473deaddeaddeaddeaddeaddeaddeaddeaddead000181565b60405173ffffffffffffffffffffffffffffffffffffffff9091168152602001610122565b61011860045481565b61011860075481565b3373deaddeaddeaddeaddeaddeaddeaddeaddead000114610342576040517f08c379a000000000000000000000000000000000000000000000000000000000815260206004820152603b60248201527f4c31426c6f636b3a206f6e6c7920746865206465706f7369746f72206163636f60448201527f756e742063616e20736574204c3120626c6f636b2076616c7565730000000000606482015260840160405180910390fd5b6000805467ffffffffffffffff98891668010000000000000000027fffffffffffffffffffffffffffffffff00000000000000000000000000000000909116998916999099179890981790975560019490945560029290925560038054919094167fffffffffffffffffffffffffffffffffffffffffffffffff00000000000000009190911617909255600491909155600555600655565b3373deaddeaddeaddeaddeaddeaddeaddeaddead00011461040357633cc50b456000526004601cfd5b60043560801c60035560143560801c600055602435600155604435600755606435600255608435600455565b803567ffffffffffffffff8116811461044757600080fd5b919050565b600080600080600080600080610100898b03121561046957600080fd5b6104728961042f565b975061048060208a0161042f565b9650604089013595506060890135945061049c60808a0161042f565b979a969950949793969560a0850135955060c08501359460e001359350915050565b600060208083528351808285015260005b818110156104eb578581018301518582016040015282016104cf565b818111156104fd576000604083870101525b50601f017fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe01692909201604001939250505056fea164736f6c634300080f000a")

op-node/rollup/derive/ecotone_upgrade_transactions_test.go

@@ -110,9 +110,11 @@ func TestEcotoneNetworkTransactions(t *testing.T) {
 	require.Nil(t, beaconRoots.To())
 	require.Equal(t, uint64(250_000), beaconRoots.Gas())
 	require.Equal(t, eip4788CreationData, beaconRoots.Data())
+	require.NotEmpty(t, beaconRoots.Data())
 }
 
 func TestEip4788Params(t *testing.T) {
 	require.Equal(t, EIP4788From, common.HexToAddress("0x0B799C86a49DEeb90402691F1041aa3AF2d3C875"))
-	require.Equal(t, eip4788CreationData, common.Hex2Bytes("0x60618060095f395ff33373fffffffffffffffffffffffffffffffffffffffe14604d57602036146024575f5ffd5b5f35801560495762001fff810690815414603c575f5ffd5b62001fff01545f5260205ff35b5f5ffd5b62001fff42064281555f359062001fff015500"))
+	require.Equal(t, eip4788CreationData, common.FromHex("0x60618060095f395ff33373fffffffffffffffffffffffffffffffffffffffe14604d57602036146024575f5ffd5b5f35801560495762001fff810690815414603c575f5ffd5b62001fff01545f5260205ff35b5f5ffd5b62001fff42064281555f359062001fff015500"))
+	require.NotEmpty(t, eip4788CreationData)
 }

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

@@ -80,6 +80,13 @@ func NewBlockProcessorFromHeader(provider BlockDataProvider, h *types.Header) (*
 	header.BaseFee = eip1559.CalcBaseFee(provider.Config(), parentHeader, header.Time)
 	header.GasUsed = 0
 	gasPool := new(core.GasPool).AddGas(header.GasLimit)
+	if h.ParentBeaconRoot != nil {
+		// Unfortunately this is not part of any Geth environment setup,
+		// we just have to apply it, like how the Geth block-builder worker does.
+		context := core.NewEVMBlockContext(header, provider, nil, provider.Config(), statedb)
+		vmenv := vm.NewEVM(context, vm.TxContext{}, statedb, provider.Config(), vm.Config{})
+		core.ProcessBeaconBlockRoot(*header.ParentBeaconRoot, vmenv, statedb)
+	}
 	return &BlockProcessor{
 		header:       header,
 		state:        statedb,