op-supervisor: Add datadir option (#11000)

Implement checks for required fields in config and add tests for them.

op-service/metrics/cli.go

@@ -17,6 +17,8 @@ const (
 	defaultListenPort  = 7300
 )
 
+var ErrInvalidPort = errors.New("invalid metrics port")
+
 func DefaultCLIConfig() CLIConfig {
 	return CLIConfig{
 		Enabled:    false,
@@ -59,7 +61,7 @@ func (m CLIConfig) Check() error {
 	}
 
 	if m.ListenPort < 0 || m.ListenPort > math.MaxUint16 {
-		return errors.New("invalid metrics port")
+		return ErrInvalidPort
 	}
 
 	return nil

op-service/oppprof/cli.go

@@ -22,6 +22,7 @@ const (
 	defaultListenPort   = 6060
 )
 
+var ErrInvalidPort = errors.New("invalid pprof port")
 var allowedProfileTypes = []profileType{"cpu", "heap", "goroutine", "threadcreate", "block", "mutex", "allocs"}
 
 type profileType string
@@ -122,7 +123,7 @@ func (m CLIConfig) Check() error {
 	}
 
 	if m.ListenPort < 0 || m.ListenPort > math.MaxUint16 {
-		return errors.New("invalid pprof port")
+		return ErrInvalidPort
 	}
 
 	return nil

op-service/rpc/cli.go

@@ -14,6 +14,8 @@ const (
 	EnableAdminFlagName = "rpc.enable-admin"
 )
 
+var ErrInvalidPort = errors.New("invalid RPC port")
+
 func CLIFlags(envPrefix string) []cli.Flag {
 	return []cli.Flag{
 		&cli.StringFlag{
@@ -52,7 +54,7 @@ func DefaultCLIConfig() CLIConfig {
 
 func (c CLIConfig) Check() error {
 	if c.ListenPort < 0 || c.ListenPort > math.MaxUint16 {
-		return errors.New("invalid RPC port")
+		return ErrInvalidPort
 	}
 
 	return nil

op-supervisor/cmd/main.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"os"
 
+	"github.com/ethereum-optimism/optimism/op-supervisor/config"
 	"github.com/urfave/cli/v2"
 
 	"github.com/ethereum/go-ethereum/log"
@@ -41,7 +42,7 @@ func run(ctx context.Context, args []string, fn supervisor.MainFn) error {
 	app.Name = "op-supervisor"
 	app.Usage = "op-supervisor monitors cross-L2 interop messaging"
 	app.Description = "The op-supervisor monitors cross-L2 interop messaging by pre-fetching events and then resolving the cross-L2 dependencies to answer safety queries."
-	app.Action = cliapp.LifecycleCmd(supervisor.Main(Version, fn))
+	app.Action = cliapp.LifecycleCmd(supervisor.Main(app.Version, fn))
 	app.Commands = []*cli.Command{
 		{
 			Name:        "doc",
@@ -51,6 +52,6 @@ func run(ctx context.Context, args []string, fn supervisor.MainFn) error {
 	return app.RunContext(ctx, args)
 }
 
-func fromConfig(ctx context.Context, cfg *supervisor.CLIConfig, logger log.Logger) (cliapp.Lifecycle, error) {
-	return supervisor.SupervisorFromCLIConfig(ctx, cfg, logger)
+func fromConfig(ctx context.Context, cfg *config.Config, logger log.Logger) (cliapp.Lifecycle, error) {
+	return supervisor.SupervisorFromConfig(ctx, cfg, logger)
 }

op-supervisor/cmd/main_test.go

@@ -6,12 +6,17 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/ethereum-optimism/optimism/op-supervisor/config"
 	"github.com/stretchr/testify/require"
 
 	"github.com/ethereum/go-ethereum/log"
 
 	"github.com/ethereum-optimism/optimism/op-service/cliapp"
-	"github.com/ethereum-optimism/optimism/op-supervisor/supervisor"
+)
+
+var (
+	ValidL2RPCs  = []string{"http;//localhost:8545"}
+	ValidDatadir = "./supervisor_test_datadir"
 )
 
 func TestLogLevel(t *testing.T) {
@@ -31,7 +36,7 @@ func TestLogLevel(t *testing.T) {
 
 func TestDefaultCLIOptionsMatchDefaultConfig(t *testing.T) {
 	cfg := configForArgs(t, addRequiredArgs())
-	defaultCfgTempl := supervisor.DefaultCLIConfig()
+	defaultCfgTempl := config.NewConfig(ValidL2RPCs, ValidDatadir)
 	defaultCfg := *defaultCfgTempl
 	defaultCfg.Version = Version
 	require.Equal(t, defaultCfg, *cfg)
@@ -50,6 +55,18 @@ func TestL2RPCs(t *testing.T) {
 	})
 }
 
+func TestDatadir(t *testing.T) {
+	t.Run("Required", func(t *testing.T) {
+		verifyArgsInvalid(t, "flag datadir is required", addRequiredArgsExcept("--datadir"))
+	})
+
+	t.Run("Valid", func(t *testing.T) {
+		dir := "foo"
+		cfg := configForArgs(t, addRequiredArgsExcept("--datadir", "--datadir", dir))
+		require.Equal(t, dir, cfg.Datadir)
+	})
+}
+
 func TestMockRun(t *testing.T) {
 	t.Run("Valid", func(t *testing.T) {
 		cfg := configForArgs(t, addRequiredArgs("--mock-run"))
@@ -62,18 +79,18 @@ func verifyArgsInvalid(t *testing.T, messageContains string, cliArgs []string) {
 	require.ErrorContains(t, err, messageContains)
 }
 
-func configForArgs(t *testing.T, cliArgs []string) *supervisor.CLIConfig {
+func configForArgs(t *testing.T, cliArgs []string) *config.Config {
 	_, cfg, err := dryRunWithArgs(cliArgs)
 	require.NoError(t, err)
 	return cfg
 }
 
-func dryRunWithArgs(cliArgs []string) (log.Logger, *supervisor.CLIConfig, error) {
-	cfg := new(supervisor.CLIConfig)
+func dryRunWithArgs(cliArgs []string) (log.Logger, *config.Config, error) {
+	cfg := new(config.Config)
 	var logger log.Logger
 	fullArgs := append([]string{"op-supervisor"}, cliArgs...)
 	testErr := errors.New("dry-run")
-	err := run(context.Background(), fullArgs, func(ctx context.Context, config *supervisor.CLIConfig, log log.Logger) (cliapp.Lifecycle, error) {
+	err := run(context.Background(), fullArgs, func(ctx context.Context, config *config.Config, log log.Logger) (cliapp.Lifecycle, error) {
 		logger = log
 		cfg = config
 		return nil, testErr
@@ -106,7 +123,8 @@ func toArgList(req map[string]string) []string {
 
 func requiredArgs() map[string]string {
 	args := map[string]string{
-		"--l2-rpcs": "http://localhost:8545",
+		"--l2-rpcs": ValidL2RPCs[0],
+		"--datadir": ValidDatadir,
 	}
 	return args
 }

op-supervisor/supervisor/cli_config.go → op-supervisor/config/config.go

-package supervisor
+package config
 
 import (
 	"errors"
 
-	"github.com/urfave/cli/v2"
-
 	oplog "github.com/ethereum-optimism/optimism/op-service/log"
 	opmetrics "github.com/ethereum-optimism/optimism/op-service/metrics"
 	"github.com/ethereum-optimism/optimism/op-service/oppprof"
 	oprpc "github.com/ethereum-optimism/optimism/op-service/rpc"
-	"github.com/ethereum-optimism/optimism/op-supervisor/flags"
 )
 
-type CLIConfig struct {
+var (
+	ErrMissingL2RPC   = errors.New("must specify at least one L2 RPC")
+	ErrMissingDatadir = errors.New("must specify datadir")
+)
+
+type Config struct {
 	Version string
 
 	LogConfig     oplog.CLIConfig
@@ -23,37 +25,34 @@ type CLIConfig struct {
 	// MockRun runs the service with a mock backend
 	MockRun bool
 
-	L2RPCs []string
-}
-
-func CLIConfigFromCLI(ctx *cli.Context, version string) *CLIConfig {
-	return &CLIConfig{
-		Version:       version,
-		LogConfig:     oplog.ReadCLIConfig(ctx),
-		MetricsConfig: opmetrics.ReadCLIConfig(ctx),
-		PprofConfig:   oppprof.ReadCLIConfig(ctx),
-		RPC:           oprpc.ReadCLIConfig(ctx),
-		MockRun:       ctx.Bool(flags.MockRunFlag.Name),
-		L2RPCs:        ctx.StringSlice(flags.L2RPCsFlag.Name),
-	}
+	L2RPCs  []string
+	Datadir string
 }
 
-func (c *CLIConfig) Check() error {
+func (c *Config) Check() error {
 	var result error
 	result = errors.Join(result, c.MetricsConfig.Check())
 	result = errors.Join(result, c.PprofConfig.Check())
 	result = errors.Join(result, c.RPC.Check())
+	if len(c.L2RPCs) == 0 {
+		result = errors.Join(result, ErrMissingL2RPC)
+	}
+	if c.Datadir == "" {
+		result = errors.Join(result, ErrMissingDatadir)
+	}
 	return result
 }
 
-func DefaultCLIConfig() *CLIConfig {
-	return &CLIConfig{
-		Version:       "",
+// NewConfig creates a new config using default values whenever possible.
+// Required options with no suitable default are passed as parameters.
+func NewConfig(l2RPCs []string, datadir string) *Config {
+	return &Config{
 		LogConfig:     oplog.DefaultCLIConfig(),
 		MetricsConfig: opmetrics.DefaultCLIConfig(),
 		PprofConfig:   oppprof.DefaultCLIConfig(),
 		RPC:           oprpc.DefaultCLIConfig(),
 		MockRun:       false,
-		L2RPCs:        flags.L2RPCsFlag.Value.Value(),
+		L2RPCs:        l2RPCs,
+		Datadir:       datadir,
 	}
 }

op-supervisor/config/config_test.go

+package config
+
+import (
+	"testing"
+
+	"github.com/ethereum-optimism/optimism/op-service/metrics"
+	"github.com/ethereum-optimism/optimism/op-service/oppprof"
+	"github.com/ethereum-optimism/optimism/op-service/rpc"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDefaultConfigIsValid(t *testing.T) {
+	cfg := validConfig()
+	require.NoError(t, cfg.Check())
+}
+
+func TestRequireL2RPC(t *testing.T) {
+	cfg := validConfig()
+	cfg.L2RPCs = []string{}
+	require.ErrorIs(t, cfg.Check(), ErrMissingL2RPC)
+}
+
+func TestRequireDatadir(t *testing.T) {
+	cfg := validConfig()
+	cfg.Datadir = ""
+	require.ErrorIs(t, cfg.Check(), ErrMissingDatadir)
+}
+
+func TestValidateMetricsConfig(t *testing.T) {
+	cfg := validConfig()
+	cfg.MetricsConfig.Enabled = true
+	cfg.MetricsConfig.ListenPort = -1
+	require.ErrorIs(t, cfg.Check(), metrics.ErrInvalidPort)
+}
+
+func TestValidatePprofConfig(t *testing.T) {
+	cfg := validConfig()
+	cfg.PprofConfig.ListenEnabled = true
+	cfg.PprofConfig.ListenPort = -1
+	require.ErrorIs(t, cfg.Check(), oppprof.ErrInvalidPort)
+}
+
+func TestValidateRPCConfig(t *testing.T) {
+	cfg := validConfig()
+	cfg.RPC.ListenPort = -1
+	require.ErrorIs(t, cfg.Check(), rpc.ErrInvalidPort)
+}
+
+func validConfig() *Config {
+	// Should be valid using only the required arguments passed in via the constructor.
+	return NewConfig([]string{"http://localhost:8545"}, "./supervisor_config_testdir")
+}

op-supervisor/flags/flags.go

@@ -3,6 +3,7 @@ package flags
 import (
 	"fmt"
 
+	"github.com/ethereum-optimism/optimism/op-supervisor/config"
 	"github.com/urfave/cli/v2"
 
 	opservice "github.com/ethereum-optimism/optimism/op-service"
@@ -23,7 +24,11 @@ var (
 		Name:    "l2-rpcs",
 		Usage:   "L2 RPC sources.",
 		EnvVars: prefixEnvVars("L2_RPCS"),
-		Value:   cli.NewStringSlice("http://localhost:8545"),
+	}
+	DataDirFlag = &cli.PathFlag{
+		Name:    "datadir",
+		Usage:   "Directory to store data generated as part of responding to games",
+		EnvVars: prefixEnvVars("DATADIR"),
 	}
 	MockRunFlag = &cli.BoolFlag{
 		Name:    "mock-run",
@@ -35,6 +40,7 @@ var (
 
 var requiredFlags = []cli.Flag{
 	L2RPCsFlag,
+	DataDirFlag,
 }
 
 var optionalFlags = []cli.Flag{
@@ -62,3 +68,16 @@ func CheckRequired(ctx *cli.Context) error {
 	}
 	return nil
 }
+
+func ConfigFromCLI(ctx *cli.Context, version string) *config.Config {
+	return &config.Config{
+		Version:       version,
+		LogConfig:     oplog.ReadCLIConfig(ctx),
+		MetricsConfig: opmetrics.ReadCLIConfig(ctx),
+		PprofConfig:   oppprof.ReadCLIConfig(ctx),
+		RPC:           oprpc.ReadCLIConfig(ctx),
+		MockRun:       ctx.Bool(MockRunFlag.Name),
+		L2RPCs:        ctx.StringSlice(L2RPCsFlag.Name),
+		Datadir:       ctx.Path(DataDirFlag.Name),
+	}
+}

op-supervisor/supervisor/cli_config_test.go

-package supervisor
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/require"
-)
-
-func TestDefaultConfigIsValid(t *testing.T) {
-	cfg := DefaultCLIConfig()
-	require.NoError(t, cfg.Check())
-}

op-supervisor/supervisor/entrypoint.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/ethereum-optimism/optimism/op-supervisor/config"
 	"github.com/urfave/cli/v2"
 
 	"github.com/ethereum/go-ethereum/log"
@@ -14,7 +15,7 @@ import (
 	"github.com/ethereum-optimism/optimism/op-supervisor/flags"
 )
 
-type MainFn func(ctx context.Context, cfg *CLIConfig, logger log.Logger) (cliapp.Lifecycle, error)
+type MainFn func(ctx context.Context, cfg *config.Config, logger log.Logger) (cliapp.Lifecycle, error)
 
 // Main is the entrypoint into the Supervisor.
 // This method returns a cliapp.LifecycleAction, to create an op-service CLI-lifecycle-managed supervisor with.
@@ -23,7 +24,7 @@ func Main(version string, fn MainFn) cliapp.LifecycleAction {
 		if err := flags.CheckRequired(cliCtx); err != nil {
 			return nil, err
 		}
-		cfg := CLIConfigFromCLI(cliCtx, version)
+		cfg := flags.ConfigFromCLI(cliCtx, version)
 		if err := cfg.Check(); err != nil {
 			return nil, fmt.Errorf("invalid CLI flags: %w", err)
 		}

op-supervisor/supervisor/service.go

@@ -7,6 +7,7 @@ import (
 	"io"
 	"sync/atomic"
 
+	"github.com/ethereum-optimism/optimism/op-supervisor/config"
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/ethereum/go-ethereum/rpc"
 
@@ -43,7 +44,7 @@ type SupervisorService struct {
 
 var _ cliapp.Lifecycle = (*SupervisorService)(nil)
 
-func SupervisorFromCLIConfig(ctx context.Context, cfg *CLIConfig, logger log.Logger) (*SupervisorService, error) {
+func SupervisorFromConfig(ctx context.Context, cfg *config.Config, logger log.Logger) (*SupervisorService, error) {
 	su := &SupervisorService{log: logger}
 	if err := su.initFromCLIConfig(ctx, cfg); err != nil {
 		return nil, errors.Join(err, su.Stop(ctx)) // try to clean up our failed initialization attempt
@@ -51,7 +52,7 @@ func SupervisorFromCLIConfig(ctx context.Context, cfg *CLIConfig, logger log.Log
 	return su, nil
 }
 
-func (su *SupervisorService) initFromCLIConfig(ctx context.Context, cfg *CLIConfig) error {
+func (su *SupervisorService) initFromCLIConfig(ctx context.Context, cfg *config.Config) error {
 	su.initMetrics(cfg)
 	if err := su.initPProf(cfg); err != nil {
 		return fmt.Errorf("failed to start PProf server: %w", err)
@@ -66,7 +67,7 @@ func (su *SupervisorService) initFromCLIConfig(ctx context.Context, cfg *CLIConf
 	return nil
 }
 
-func (su *SupervisorService) initBackend(cfg *CLIConfig) {
+func (su *SupervisorService) initBackend(cfg *config.Config) {
 	if cfg.MockRun {
 		su.backend = backend.NewMockBackend()
 	} else {
@@ -74,7 +75,7 @@ func (su *SupervisorService) initBackend(cfg *CLIConfig) {
 	}
 }
 
-func (su *SupervisorService) initMetrics(cfg *CLIConfig) {
+func (su *SupervisorService) initMetrics(cfg *config.Config) {
 	if cfg.MetricsConfig.Enabled {
 		procName := "default"
 		su.metrics = metrics.NewMetrics(procName)
@@ -84,7 +85,7 @@ func (su *SupervisorService) initMetrics(cfg *CLIConfig) {
 	}
 }
 
-func (su *SupervisorService) initPProf(cfg *CLIConfig) error {
+func (su *SupervisorService) initPProf(cfg *config.Config) error {
 	su.pprofService = oppprof.New(
 		cfg.PprofConfig.ListenEnabled,
 		cfg.PprofConfig.ListenAddr,
@@ -101,7 +102,7 @@ func (su *SupervisorService) initPProf(cfg *CLIConfig) error {
 	return nil
 }
 
-func (su *SupervisorService) initMetricsServer(cfg *CLIConfig) error {
+func (su *SupervisorService) initMetricsServer(cfg *config.Config) error {
 	if !cfg.MetricsConfig.Enabled {
 		su.log.Info("Metrics disabled")
 		return nil
@@ -120,7 +121,7 @@ func (su *SupervisorService) initMetricsServer(cfg *CLIConfig) error {
 	return nil
 }
 
-func (su *SupervisorService) initRPCServer(cfg *CLIConfig) error {
+func (su *SupervisorService) initRPCServer(cfg *config.Config) error {
 	server := oprpc.NewServer(
 		cfg.RPC.ListenAddr,
 		cfg.RPC.ListenPort,

op-supervisor/supervisor/service_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/ethereum-optimism/optimism/op-supervisor/config"
 	"github.com/holiman/uint256"
 	"github.com/stretchr/testify/require"
 
@@ -22,7 +23,7 @@ import (
 )
 
 func TestSupervisorService(t *testing.T) {
-	cfg := &CLIConfig{
+	cfg := &config.Config{
 		Version: "",
 		LogConfig: oplog.CLIConfig{
 			Level:  log.LevelError,
@@ -50,7 +51,7 @@ func TestSupervisorService(t *testing.T) {
 		MockRun: true,
 	}
 	logger := testlog.Logger(t, log.LevelError)
-	supervisor, err := SupervisorFromCLIConfig(context.Background(), cfg, logger)
+	supervisor, err := SupervisorFromConfig(context.Background(), cfg, logger)
 	require.NoError(t, err)
 	require.NoError(t, supervisor.Start(context.Background()), "start service")
 	// run some RPC tests against the service with the mock backend