op-upgrade: cleanup

op-chain-ops/cmd/op-upgrade/main.go

@@ -105,18 +105,25 @@ func entrypoint(ctx *cli.Context) error {
 			log.Warn("Cannot find deploy config for network", "name", chainConfig.Name, "deploy-config-name", name, "path", deployConfig)
 		}
 
+		if config != nil {
+			log.Info("Checking deploy config validity", "name", chainConfig.Name)
+			if err := config.Check(); err != nil {
+				return fmt.Errorf("error checking deploy config: %w", err)
+			}
+		}
+
 		clients, err := clients.NewClients(ctx.String("l1-rpc-url"), chainConfig.PublicRPC)
 		if err != nil {
-			return err
+			return fmt.Errorf("cannot create RPC clients: %w", err)
 		}
 
 		l1ChainID, err := clients.L1Client.ChainID(ctx.Context)
 		if err != nil {
-			return err
+			return fmt.Errorf("cannot fetch L1 chain ID: %w", err)
 		}
 		l2ChainID, err := clients.L2Client.ChainID(ctx.Context)
 		if err != nil {
-			return err
+			return fmt.Errorf("cannot fetch L2 chain ID: %w", err)
 		}
 		log.Info(chainConfig.Name, "l1-chain-id", l1ChainID, "l2-chain-id", l2ChainID)
 
@@ -159,15 +166,19 @@ func entrypoint(ctx *cli.Context) error {
 		log.Info("OptimismPortal", "version", list.OptimismPortal.Version, "address", list.OptimismPortal.Address)
 		log.Info("SystemConfig", "version", list.SystemConfig.Version, "address", list.SystemConfig.Address)
 
+		// Ensure that the superchain registry information is correct by checking the
+		// actual versions based on what the registry says is true.
 		if err := upgrades.CheckL1(ctx.Context, &list, clients.L1Client); err != nil {
 			return fmt.Errorf("error checking L1: %w", err)
 		}
 
+		// Build the batch
 		if err := upgrades.L1(&batch, list, *addresses, config, chainConfig, clients.L1Client); err != nil {
 			return err
 		}
 	}
 
+	// Write the batch to disk or stdout
 	if outfile := ctx.Path("outfile"); outfile != "" {
 		if err := writeJSON(outfile, batch); err != nil {
 			return err

op-chain-ops/upgrades/l1.go

@@ -348,9 +348,16 @@ func SystemConfig(batch *safe.Batch, implementations superchain.ImplementationLi
 		return fmt.Errorf("no initialize method")
 	}
 
-	var gasPriceOracleOverhead, gasPriceOracleScalar, startBlock *big.Int
+	// If we want to be able to override these based on the values in the config,
+	// the logic below will need to be updated. Right now the logic prefers the
+	// on chain values over the offchain values. This to maintain backwards compatibilty
+	// in the short term.
+	startBlock := big.NewInt(0)
+	batchInboxAddress := common.HexToAddress(chainConfig.BatchInboxAddr.String())
+
+	var gasPriceOracleOverhead, gasPriceOracleScalar *big.Int
 	var batcherHash common.Hash
-	var batchInboxAddress, p2pSequencerAddress, finalSystemOwner common.Address
+	var p2pSequencerAddress, finalSystemOwner common.Address
 	var l2GenesisBlockGasLimit uint64
 
 	if config != nil {
@@ -384,14 +391,14 @@ func SystemConfig(batch *safe.Batch, implementations superchain.ImplementationLi
 			return err
 		}
 		// StartBlock is a new property, we want to explicitly set it to 0 if there is an error fetching it
-		startBlock, err = systemConfig.StartBlock(&bind.CallOpts{})
-		if err != nil {
-			startBlock = big.NewInt(0)
+		systemConfigStartBlock, err := systemConfig.StartBlock(&bind.CallOpts{})
+		if err != nil && systemConfigStartBlock != nil {
+			startBlock = systemConfigStartBlock
 		}
 		// BatchInboxAddress is a new property, we want to set it to the offchain value if there is an error fetching it
-		batchInboxAddress, err = systemConfig.BatchInbox(&bind.CallOpts{})
-		if err != nil {
-			batchInboxAddress = common.HexToAddress(chainConfig.BatchInboxAddr.String())
+		systemConfigBatchInboxAddress, err := systemConfig.BatchInbox(&bind.CallOpts{})
+		if err != nil && systemConfigBatchInboxAddress != (common.Address{}) {
+			batchInboxAddress = systemConfigBatchInboxAddress
 		}
 		p2pSequencerAddress, err = systemConfig.UnsafeBlockSigner(&bind.CallOpts{})
 		if err != nil {