From 5e254502f66782ac4b54178d52959e5a2f254ed7 Mon Sep 17 00:00:00 2001
From: Joshua Gutow <jgutow@oplabs.co>
Date: Tue, 5 Dec 2023 16:30:44 -0800
Subject: [PATCH] op-batcher: Move config test from op-e2e to unit tests

---
 op-batcher/batcher/config.go      | 21 ++++++-
 op-batcher/batcher/config_test.go | 93 +++++++++++++++++++++++++++++++
 op-e2e/system_test.go             | 11 ----
 op-service/rpc/cli.go             |  8 +++
 4 files changed, 121 insertions(+), 12 deletions(-)
 create mode 100644 op-batcher/batcher/config_test.go

diff --git a/op-batcher/batcher/config.go b/op-batcher/batcher/config.go
index d4f34461f02d..927446f4137b 100644
--- a/op-batcher/batcher/config.go
+++ b/op-batcher/batcher/config.go
@@ -1,6 +1,8 @@
 package batcher
 
 import (
+	"errors"
+	"fmt"
 	"time"
 
 	"github.com/urfave/cli/v2"
@@ -63,7 +65,24 @@ type CLIConfig struct {
 }
 
 func (c *CLIConfig) Check() error {
-	// TODO(7512): check the sanity of flags loaded directly https://github.com/ethereum-optimism/optimism/issues/7512
+	if c.L1EthRpc == "" {
+		return errors.New("empty L1 RPC URL")
+	}
+	if c.L2EthRpc == "" {
+		return errors.New("empty L2 RPC URL")
+	}
+	if c.RollupRpc == "" {
+		return errors.New("empty rollup RPC URL")
+	}
+	if c.PollInterval == 0 {
+		return errors.New("must set PollInterval")
+	}
+	if c.MaxL1TxSize <= 1 {
+		return errors.New("MaxL1TxSize must be greater than 0")
+	}
+	if c.BatchType > 1 {
+		return fmt.Errorf("unknown batch type: %v", c.BatchType)
+	}
 
 	if err := c.MetricsConfig.Check(); err != nil {
 		return err
diff --git a/op-batcher/batcher/config_test.go b/op-batcher/batcher/config_test.go
new file mode 100644
index 000000000000..78566f90de69
--- /dev/null
+++ b/op-batcher/batcher/config_test.go
@@ -0,0 +1,93 @@
+package batcher_test
+
+import (
+	"testing"
+	"time"
+
+	"github.com/ethereum-optimism/optimism/op-batcher/batcher"
+	"github.com/ethereum-optimism/optimism/op-service/log"
+	"github.com/ethereum-optimism/optimism/op-service/metrics"
+	"github.com/ethereum-optimism/optimism/op-service/pprof"
+	"github.com/ethereum-optimism/optimism/op-service/rpc"
+	"github.com/ethereum-optimism/optimism/op-service/txmgr"
+	"github.com/stretchr/testify/require"
+)
+
+func validBatcherConfig() batcher.CLIConfig {
+	return batcher.CLIConfig{
+		L1EthRpc:               "fake",
+		L2EthRpc:               "fake",
+		RollupRpc:              "fake",
+		MaxChannelDuration:     0,
+		SubSafetyMargin:        0,
+		PollInterval:           time.Second,
+		MaxPendingTransactions: 0,
+		MaxL1TxSize:            10,
+		Stopped:                false,
+		BatchType:              0,
+		TxMgrConfig:            txmgr.NewCLIConfig("fake", txmgr.DefaultBatcherFlagValues),
+		LogConfig:              log.DefaultCLIConfig(),
+		MetricsConfig:          metrics.DefaultCLIConfig(),
+		PprofConfig:            pprof.DefaultCLIConfig(),
+		// The compressor config is not checked in config.Check()
+		RPC: rpc.DefaultCLIConfig(),
+	}
+}
+
+func TestValidBatcherConfig(t *testing.T) {
+	cfg := validBatcherConfig()
+	require.NoError(t, cfg.Check(), "valid config should pass the check function")
+}
+
+func TestBatcherConfig(t *testing.T) {
+	tests := []struct {
+		name      string
+		override  func(*batcher.CLIConfig)
+		errString string
+	}{
+		{
+			name:      "empty L1",
+			override:  func(c *batcher.CLIConfig) { c.L1EthRpc = "" },
+			errString: "empty L1 RPC URL",
+		},
+		{
+			name:      "empty L2",
+			override:  func(c *batcher.CLIConfig) { c.L2EthRpc = "" },
+			errString: "empty L2 RPC URL",
+		},
+		{
+			name:      "empty rollup",
+			override:  func(c *batcher.CLIConfig) { c.RollupRpc = "" },
+			errString: "empty rollup RPC URL",
+		},
+		{
+			name:      "empty poll interval",
+			override:  func(c *batcher.CLIConfig) { c.PollInterval = 0 },
+			errString: "must set PollInterval",
+		},
+		{
+			name:      "max L1 tx size too small",
+			override:  func(c *batcher.CLIConfig) { c.MaxL1TxSize = 0 },
+			errString: "MaxL1TxSize must be greater than 0",
+		},
+		{
+			name:      "invalid batch type close",
+			override:  func(c *batcher.CLIConfig) { c.BatchType = 2 },
+			errString: "unknown batch type: 2",
+		},
+		{
+			name:      "invalid batch type far",
+			override:  func(c *batcher.CLIConfig) { c.BatchType = 100 },
+			errString: "unknown batch type: 100",
+		},
+	}
+
+	for _, test := range tests {
+		tc := test
+		t.Run(tc.name, func(t *testing.T) {
+			cfg := validBatcherConfig()
+			tc.override(&cfg)
+			require.ErrorContains(t, cfg.Check(), tc.errString)
+		})
+	}
+}
diff --git a/op-e2e/system_test.go b/op-e2e/system_test.go
index 6ded7cbd9a67..d901697f42fe 100644
--- a/op-e2e/system_test.go
+++ b/op-e2e/system_test.go
@@ -1628,14 +1628,3 @@ func TestRequiredProtocolVersionChangeAndHalt(t *testing.T) {
 	require.NoError(t, err)
 	t.Log("verified that op-geth closed!")
 }
-
-func TestIncorrectBatcherConfiguration(t *testing.T) {
-	InitParallel(t)
-
-	cfg := DefaultSystemConfig(t)
-	// make the batcher configuration invalid
-	cfg.BatcherMaxL1TxSizeBytes = 1
-
-	_, err := cfg.Start(t)
-	require.Error(t, err, "Expected error on invalid batcher configuration")
-}
diff --git a/op-service/rpc/cli.go b/op-service/rpc/cli.go
index a36684fa73cb..866dfd0336d7 100644
--- a/op-service/rpc/cli.go
+++ b/op-service/rpc/cli.go
@@ -42,6 +42,14 @@ type CLIConfig struct {
 	EnableAdmin bool
 }
 
+func DefaultCLIConfig() CLIConfig {
+	return CLIConfig{
+		ListenAddr:  "0.0.0.0",
+		ListenPort:  8545,
+		EnableAdmin: false,
+	}
+}
+
 func (c CLIConfig) Check() error {
 	if c.ListenPort < 0 || c.ListenPort > math.MaxUint16 {
 		return errors.New("invalid RPC port")