op-challenger: Switch to using GenericFlag for --trace-type

op-challenger/challenger.go

@@ -8,7 +8,6 @@ import (
 	"github.com/ethereum-optimism/optimism/op-challenger/config"
 	"github.com/ethereum-optimism/optimism/op-challenger/fault"
 	"github.com/ethereum-optimism/optimism/op-challenger/fault/cannon"
-	"github.com/ethereum-optimism/optimism/op-challenger/flags"
 	"github.com/ethereum-optimism/optimism/op-service/txmgr"
 	"github.com/ethereum-optimism/optimism/op-service/txmgr/metrics"
 	"github.com/ethereum/go-ethereum/ethclient"
@@ -40,10 +39,13 @@ func Main(ctx context.Context, logger log.Logger, cfg *config.Config) error {
 	}
 
 	var trace fault.TraceProvider
-	if cfg.TraceType == flags.CannonTraceType {
+	switch cfg.TraceType {
+	case fault.TraceTypeCannon:
 		trace = cannon.NewCannonTraceProvider(cfg.CannonDatadir)
-	} else if cfg.TraceType == flags.AlphabetTraceType {
+	case fault.TraceTypeAlphabet:
 		trace = fault.NewAlphabetProvider(cfg.AlphabetTrace, uint64(cfg.GameDepth))
+	default:
+		return fmt.Errorf("unsupported trace type: %v", cfg.TraceType)
 	}
 
 	agent := fault.NewAgent(loader, cfg.GameDepth, trace, responder, cfg.AgreeWithProposedOutput, gameLogger)

op-challenger/cmd/main_test.go

@@ -6,7 +6,7 @@ import (
 	"testing"
 
 	"github.com/ethereum-optimism/optimism/op-challenger/config"
-	"github.com/ethereum-optimism/optimism/op-challenger/flags"
+	"github.com/ethereum-optimism/optimism/op-challenger/fault"
 	"github.com/ethereum-optimism/optimism/op-service/txmgr"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/log"
@@ -39,12 +39,12 @@ func TestLogLevel(t *testing.T) {
 
 func TestDefaultCLIOptionsMatchDefaultConfig(t *testing.T) {
 	cfg := configForArgs(t, addRequiredArgs())
-	defaultCfg := config.NewConfig(l1EthRpc, common.HexToAddress(gameAddressValue), flags.AlphabetTraceType, alphabetTrace, cannonDatadir, true, 4)
+	defaultCfg := config.NewConfig(l1EthRpc, common.HexToAddress(gameAddressValue), fault.TraceTypeAlphabet, alphabetTrace, cannonDatadir, true, 4)
 	require.Equal(t, defaultCfg, cfg)
 }
 
 func TestDefaultConfigIsValid(t *testing.T) {
-	cfg := config.NewConfig(l1EthRpc, common.HexToAddress(gameAddressValue), flags.AlphabetTraceType, alphabetTrace, cannonDatadir, true, 4)
+	cfg := config.NewConfig(l1EthRpc, common.HexToAddress(gameAddressValue), fault.TraceTypeAlphabet, alphabetTrace, cannonDatadir, true, 4)
 	require.NoError(t, cfg.Check())
 }
 
@@ -66,9 +66,16 @@ func TestTraceType(t *testing.T) {
 		verifyArgsInvalid(t, "flag trace-type is required", addRequiredArgsExcept("--trace-type"))
 	})
 
-	t.Run("Valid", func(t *testing.T) {
-		cfg := configForArgs(t, addRequiredArgsExcept("--trace-type", "--trace-type="+flags.AlphabetTraceType))
-		require.Equal(t, flags.AlphabetTraceType, cfg.TraceType)
+	for _, traceType := range fault.TraceTypes {
+		traceType := traceType
+		t.Run("Valid_"+traceType.String(), func(t *testing.T) {
+			cfg := configForArgs(t, addRequiredArgsExcept("--trace-type", "--trace-type", traceType.String()))
+			require.Equal(t, traceType, cfg.TraceType)
+		})
+	}
+
+	t.Run("Invalid", func(t *testing.T) {
+		verifyArgsInvalid(t, "unknown trace type: \"foo\"", addRequiredArgsExcept("--trace-type", "--trace-type=foo"))
 	})
 }
 
@@ -165,7 +172,7 @@ func requiredArgs() map[string]string {
 		"--agree-with-proposed-output": agreeWithProposedOutput,
 		"--l1-eth-rpc":                 l1EthRpc,
 		"--game-address":               gameAddressValue,
-		"--trace-type":                 flags.AlphabetTraceType,
+		"--trace-type":                 fault.TraceTypeAlphabet.String(),
 		"--alphabet":                   alphabetTrace,
 		"--cannon-datadir":             cannonDatadir,
 	}

op-challenger/config/config.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"strings"
 
+	"github.com/ethereum-optimism/optimism/op-challenger/fault"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/urfave/cli/v2"
 
@@ -29,9 +30,9 @@ type Config struct {
 	AgreeWithProposedOutput bool           // Temporary config if we agree or disagree with the posted output
 	GameDepth               int            // Depth of the game tree
 
-	TraceType     string // Type of trace
-	AlphabetTrace string // String for the AlphabetTraceProvider
-	CannonDatadir string // Cannon Data Directory for the CannonTraceProvider
+	TraceType     fault.TraceType // Type of trace
+	AlphabetTrace string          // String for the AlphabetTraceProvider
+	CannonDatadir string          // Cannon Data Directory for the CannonTraceProvider
 
 	TxMgrConfig txmgr.CLIConfig
 }
@@ -39,7 +40,7 @@ type Config struct {
 func NewConfig(
 	l1EthRpc string,
 	gameAddress common.Address,
-	traceType string,
+	traceType fault.TraceType,
 	alphabetTrace string,
 	cannonDatadir string,
 	agreeWithProposedOutput bool,
@@ -70,10 +71,10 @@ func (c Config) Check() error {
 	if c.TraceType == "" {
 		return ErrMissingTraceType
 	}
-	if c.TraceType == flags.CannonTraceType && c.CannonDatadir == "" {
+	if c.TraceType == fault.TraceTypeCannon && c.CannonDatadir == "" {
 		return ErrMissingCannonDatadir
 	}
-	if c.TraceType == flags.AlphabetTraceType && c.AlphabetTrace == "" {
+	if c.TraceType == fault.TraceTypeAlphabet && c.AlphabetTrace == "" {
 		return ErrMissingAlphabetTrace
 	}
 	if err := c.TxMgrConfig.Check(); err != nil {
@@ -94,8 +95,7 @@ func NewConfigFromCLI(ctx *cli.Context) (*Config, error) {
 
 	txMgrConfig := txmgr.ReadCLIConfig(ctx)
 
-	traceTypeFlag := strings.TrimLeft(ctx.String(flags.TraceTypeFlag.Name), "[")
-	traceTypeFlag = strings.TrimRight(traceTypeFlag, "]")
+	traceTypeFlag := fault.TraceType(strings.ToLower(ctx.String(flags.TraceTypeFlag.Name)))
 
 	return &Config{
 		// Required Flags

op-challenger/config/config_test.go

@@ -3,7 +3,7 @@ package config
 import (
 	"testing"
 
-	"github.com/ethereum-optimism/optimism/op-challenger/flags"
+	"github.com/ethereum-optimism/optimism/op-challenger/fault"
 	"github.com/ethereum-optimism/optimism/op-service/txmgr"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/stretchr/testify/require"
@@ -18,27 +18,27 @@ var (
 	gameDepth               = 4
 )
 
-func validConfig(traceType string) Config {
+func validConfig(traceType fault.TraceType) Config {
 	cfg := NewConfig(validL1EthRpc, validGameAddress, traceType, validAlphabetTrace, validCannonDatadir, agreeWithProposedOutput, gameDepth)
 	return cfg
 }
 
 // TestValidConfigIsValid checks that the config provided by validConfig is actually valid
 func TestValidConfigIsValid(t *testing.T) {
-	err := validConfig(flags.CannonTraceType).Check()
+	err := validConfig(fault.TraceTypeCannon).Check()
 	require.NoError(t, err)
 }
 
 func TestTxMgrConfig(t *testing.T) {
 	t.Run("Invalid", func(t *testing.T) {
-		config := validConfig(flags.CannonTraceType)
+		config := validConfig(fault.TraceTypeCannon)
 		config.TxMgrConfig = txmgr.CLIConfig{}
 		require.Equal(t, config.Check().Error(), "must provide a L1 RPC url")
 	})
 }
 
 func TestL1EthRpcRequired(t *testing.T) {
-	config := validConfig(flags.CannonTraceType)
+	config := validConfig(fault.TraceTypeCannon)
 	config.L1EthRpc = ""
 	require.ErrorIs(t, config.Check(), ErrMissingL1EthRPC)
 	config.L1EthRpc = validL1EthRpc
@@ -46,7 +46,7 @@ func TestL1EthRpcRequired(t *testing.T) {
 }
 
 func TestGameAddressRequired(t *testing.T) {
-	config := validConfig(flags.CannonTraceType)
+	config := validConfig(fault.TraceTypeCannon)
 	config.GameAddress = common.Address{}
 	require.ErrorIs(t, config.Check(), ErrMissingGameAddress)
 	config.GameAddress = validGameAddress
@@ -54,7 +54,7 @@ func TestGameAddressRequired(t *testing.T) {
 }
 
 func TestAlphabetTraceRequired(t *testing.T) {
-	config := validConfig(flags.AlphabetTraceType)
+	config := validConfig(fault.TraceTypeAlphabet)
 	config.AlphabetTrace = ""
 	require.ErrorIs(t, config.Check(), ErrMissingAlphabetTrace)
 	config.AlphabetTrace = validAlphabetTrace
@@ -62,7 +62,7 @@ func TestAlphabetTraceRequired(t *testing.T) {
 }
 
 func TestCannonTraceRequired(t *testing.T) {
-	config := validConfig(flags.CannonTraceType)
+	config := validConfig(fault.TraceTypeCannon)
 	config.CannonDatadir = ""
 	require.ErrorIs(t, config.Check(), ErrMissingCannonDatadir)
 	config.CannonDatadir = validCannonDatadir

op-challenger/fault/types.go

@@ -3,6 +3,7 @@ package fault
 import (
 	"context"
 	"errors"
+	"fmt"
 
 	"github.com/ethereum/go-ethereum/common"
 )
@@ -19,6 +20,36 @@ const (
 	GameStatusDefenderWon
 )
 
+type TraceType string
+
+const (
+	TraceTypeAlphabet TraceType = "alphabet"
+	TraceTypeCannon   TraceType = "cannon"
+)
+
+var TraceTypes = []TraceType{TraceTypeAlphabet, TraceTypeCannon}
+
+func (t TraceType) String() string {
+	return string(t)
+}
+
+func (t *TraceType) Set(value string) error {
+	if !ValidTraceType(TraceType(value)) {
+		return fmt.Errorf("unknown trace type: %q", value)
+	}
+	*t = TraceType(value)
+	return nil
+}
+
+func ValidTraceType(value TraceType) bool {
+	for _, t := range TraceTypes {
+		if t == value {
+			return true
+		}
+	}
+	return false
+}
+
 // StepCallData encapsulates the data needed to perform a step.
 type StepCallData struct {
 	ClaimIndex uint64

op-challenger/flags/flags.go

@@ -2,7 +2,10 @@ package flags
 
 import (
 	"fmt"
+	"strings"
 
+	"github.com/ethereum-optimism/optimism/op-challenger/fault"
+	openum "github.com/ethereum-optimism/optimism/op-service/enum"
 	"github.com/urfave/cli/v2"
 
 	opservice "github.com/ethereum-optimism/optimism/op-service"
@@ -11,20 +14,13 @@ import (
 )
 
 const (
-	envVarPrefix      = "OP_CHALLENGER"
-	CannonTraceType   = "cannon"
-	AlphabetTraceType = "alphabet"
+	envVarPrefix = "OP_CHALLENGER"
 )
 
 func prefixEnvVars(name string) []string {
 	return opservice.PrefixEnvVar(envVarPrefix, name)
 }
 
-var (
-	validTraceTypes = []string{CannonTraceType, AlphabetTraceType}
-	traceTypes      = cli.NewStringSlice(validTraceTypes...)
-)
-
 var (
 	// Required Flags
 	L1EthRpcFlag = &cli.StringFlag{
@@ -37,11 +33,14 @@ var (
 		Usage:   "Address of the Fault Game contract.",
 		EnvVars: prefixEnvVars("GAME_ADDRESS"),
 	}
-	TraceTypeFlag = &cli.StringSliceFlag{
-		Value:   traceTypes,
+	TraceTypeFlag = &cli.GenericFlag{
 		Name:    "trace-type",
-		Usage:   "The trace type.",
+		Usage:   "The trace type. Valid options: " + openum.EnumString(fault.TraceTypes),
 		EnvVars: prefixEnvVars("TRACE_TYPE"),
+		Value: func() *fault.TraceType {
+			out := fault.TraceType("") // No default value
+			return &out
+		}(),
 	}
 	AgreeWithProposedOutputFlag = &cli.BoolFlag{
 		Name:    "agree-with-proposed-output",
@@ -97,17 +96,18 @@ func CheckRequired(ctx *cli.Context) error {
 			return fmt.Errorf("flag %s is required", f.Names()[0])
 		}
 	}
-	switch ctx.String(TraceTypeFlag.Name) {
-	case "[" + CannonTraceType + "]":
+	gameType := fault.TraceType(strings.ToLower(ctx.String(TraceTypeFlag.Name)))
+	switch gameType {
+	case fault.TraceTypeCannon:
 		if !ctx.IsSet(CannonDatadirFlag.Name) {
 			return fmt.Errorf("flag %s is required", "cannon-datadir")
 		}
-	case "[" + AlphabetTraceType + "]":
+	case fault.TraceTypeAlphabet:
 		if !ctx.IsSet(AlphabetFlag.Name) {
 			return fmt.Errorf("flag %s is required", "alphabet")
 		}
 	default:
-		return fmt.Errorf("invalid trace type. must be one of %v", validTraceTypes)
+		return fmt.Errorf("invalid trace type. must be one of %v", fault.TraceTypes)
 	}
 	return nil
 }