Merge pull request #4878 from ethereum-optimism/chain-ops/immutable-check

op-chain-ops: more strict checks for config validity

op-chain-ops/genesis/config.go

@@ -22,7 +22,10 @@ import (
 	"github.com/ethereum-optimism/optimism/op-node/rollup"
 )
 
-var ErrInvalidDeployConfig = errors.New("invalid deploy config")
+var (
+	ErrInvalidDeployConfig     = errors.New("invalid deploy config")
+	ErrInvalidImmutablesConfig = errors.New("invalid immutables config")
+)
 
 // DeployConfig represents the deployment configuration for Optimism
 type DeployConfig struct {
@@ -344,12 +347,27 @@ func NewDeployConfigWithNetwork(network, path string) (*DeployConfig, error) {
 }
 
 // NewL2ImmutableConfig will create an ImmutableConfig given an instance of a
-// Hardhat and a DeployConfig.
+// DeployConfig and a block.
 func NewL2ImmutableConfig(config *DeployConfig, block *types.Block) (immutables.ImmutableConfig, error) {
 	immutable := make(immutables.ImmutableConfig)
 
+	if config.L1StandardBridgeProxy == (common.Address{}) {
+		return immutable, fmt.Errorf("L1StandardBridgeProxy cannot be address(0): %w", ErrInvalidImmutablesConfig)
+	}
+	if config.L1CrossDomainMessengerProxy == (common.Address{}) {
+		return immutable, fmt.Errorf("L1CrossDomainMessengerProxy cannot be address(0): %w", ErrInvalidImmutablesConfig)
+	}
 	if config.L1ERC721BridgeProxy == (common.Address{}) {
-		return immutable, errors.New("L1ERC721BridgeProxy cannot be address(0)")
+		return immutable, fmt.Errorf("L1ERC721BridgeProxy cannot be address(0): %w", ErrInvalidImmutablesConfig)
+	}
+	if config.SequencerFeeVaultRecipient == (common.Address{}) {
+		return immutable, fmt.Errorf("SequencerFeeVaultRecipient cannot be address(0): %w", ErrInvalidImmutablesConfig)
+	}
+	if config.BaseFeeVaultRecipient == (common.Address{}) {
+		return immutable, fmt.Errorf("BaseFeeVaultRecipient cannot be address(0): %w", ErrInvalidImmutablesConfig)
+	}
+	if config.L1FeeVaultRecipient == (common.Address{}) {
+		return immutable, fmt.Errorf("L1FeeVaultRecipient cannot be address(0): %w", ErrInvalidImmutablesConfig)
 	}
 
 	immutable["L2StandardBridge"] = immutables.ImmutableValues{

op-chain-ops/genesis/testdata/test-deploy-config-devnet-l1.json

@@ -23,6 +23,8 @@
   "l1FeeVaultRecipient": "0x71bE63f3384f5fb98995898A86B02Fb2426c5788",
   "sequencerFeeVaultRecipient": "0x71bE63f3384f5fb98995898A86B02Fb2426c5788",
   "l1ERC721BridgeProxy": "0xff000000000000000000000000000000000000ff",
+  "l1StandardBridgeProxy": "0xff000000000000000000000000000000000000fd",
+  "l1CrossDomainMessengerProxy": "0xff000000000000000000000000000000000000dd",
 
   "deploymentWaitConfirmations": 1,
   "fundDevAccounts": true

op-chain-ops/immutables/immutables.go

 package immutables
 
 import (
+	"errors"
 	"fmt"
 	"math/big"
 
@@ -24,6 +25,39 @@ type ImmutableValues map[string]any
 // contracts.
 type ImmutableConfig map[string]ImmutableValues
 
+// Check does a sanity check that the specific values that
+// Optimism uses are set inside of the ImmutableConfig.
+func (i ImmutableConfig) Check() error {
+	if _, ok := i["L2CrossDomainMessenger"]["otherMessenger"]; !ok {
+		return errors.New("L2CrossDomainMessenger otherMessenger not set")
+	}
+	if _, ok := i["L2StandardBridge"]["otherBridge"]; !ok {
+		return errors.New("L2StandardBridge otherBridge not set")
+	}
+	if _, ok := i["L2ERC721Bridge"]["messenger"]; !ok {
+		return errors.New("L2ERC721Bridge messenger not set")
+	}
+	if _, ok := i["L2ERC721Bridge"]["otherBridge"]; !ok {
+		return errors.New("L2ERC721Bridge otherBridge not set")
+	}
+	if _, ok := i["OptimismMintableERC721Factory"]["bridge"]; !ok {
+		return errors.New("OptimismMintableERC20Factory bridge not set")
+	}
+	if _, ok := i["OptimismMintableERC721Factory"]["remoteChainId"]; !ok {
+		return errors.New("OptimismMintableERC20Factory remoteChainId not set")
+	}
+	if _, ok := i["SequencerFeeVault"]["recipient"]; !ok {
+		return errors.New("SequencerFeeVault recipient not set")
+	}
+	if _, ok := i["L1FeeVault"]["recipient"]; !ok {
+		return errors.New("L1FeeVault recipient not set")
+	}
+	if _, ok := i["BaseFeeVault"]["recipient"]; !ok {
+		return errors.New("BaseFeeVault recipient not set")
+	}
+	return nil
+}
+
 // DeploymentResults represents the output of deploying each of the
 // contracts so that the immutables can be set properly in the bytecode.
 type DeploymentResults map[string]hexutil.Bytes
@@ -31,6 +65,10 @@ type DeploymentResults map[string]hexutil.Bytes
 // BuildOptimism will deploy the L2 predeploys so that their immutables are set
 // correctly.
 func BuildOptimism(immutable ImmutableConfig) (DeploymentResults, error) {
+	if err := immutable.Check(); err != nil {
+		return DeploymentResults{}, err
+	}
+
 	deployments := []deployer.Constructor{
 		{
 			Name: "GasPriceOracle",

op-chain-ops/immutables/immutables_test.go

@@ -19,9 +19,11 @@ func TestBuildOptimism(t *testing.T) {
 		},
 		"L2ERC721Bridge": {
 			"otherBridge": common.HexToAddress("0x1234567890123456789012345678901234567890"),
+			"messenger":   common.HexToAddress("0x1234567890123456789012345678901234567890"),
 		},
 		"OptimismMintableERC721Factory": {
 			"remoteChainId": big.NewInt(1),
+			"bridge":        common.HexToAddress("0x1234567890123456789012345678901234567890"),
 		},
 		"SequencerFeeVault": {
 			"recipient": common.HexToAddress("0x1234567890123456789012345678901234567890"),

op-e2e/e2eutils/setup.go

@@ -102,6 +102,10 @@ func MakeDeployParams(t require.TestingT, tp *TestParams) *DeployParams {
 		GasPriceOracleScalar:        1000_000,
 		DeploymentWaitConfirmations: 1,
 
+		SequencerFeeVaultRecipient: common.Address{19: 1},
+		BaseFeeVaultRecipient:      common.Address{19: 2},
+		L1FeeVaultRecipient:        common.Address{19: 3},
+
 		EIP1559Elasticity:  10,
 		EIP1559Denominator: 50,
 

op-e2e/setup.go

@@ -92,6 +92,10 @@ func DefaultSystemConfig(t *testing.T) SystemConfig {
 		GasPriceOracleOverhead: 2100,
 		GasPriceOracleScalar:   1_000_000,
 
+		SequencerFeeVaultRecipient: common.Address{19: 1},
+		BaseFeeVaultRecipient:      common.Address{19: 2},
+		L1FeeVaultRecipient:        common.Address{19: 3},
+
 		DeploymentWaitConfirmations: 1,
 
 		EIP1559Elasticity:  2,