op-challenger: Carefully rearrange modules to avoid cycles.

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/fault"
+	"github.com/ethereum-optimism/optimism/op-challenger/flags"
 	"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), fault.TraceTypeAlphabet, alphabetTrace, cannonDatadir, true, 4)
+	defaultCfg := config.NewConfig(l1EthRpc, common.HexToAddress(gameAddressValue), flags.TraceTypeAlphabet, alphabetTrace, cannonDatadir, true, 4)
 	require.Equal(t, defaultCfg, cfg)
 }
 
 func TestDefaultConfigIsValid(t *testing.T) {
-	cfg := config.NewConfig(l1EthRpc, common.HexToAddress(gameAddressValue), fault.TraceTypeAlphabet, alphabetTrace, cannonDatadir, true, 4)
+	cfg := config.NewConfig(l1EthRpc, common.HexToAddress(gameAddressValue), flags.TraceTypeAlphabet, alphabetTrace, cannonDatadir, true, 4)
 	require.NoError(t, cfg.Check())
 }
 
@@ -66,7 +66,7 @@ func TestTraceType(t *testing.T) {
 		verifyArgsInvalid(t, "flag trace-type is required", addRequiredArgsExcept("--trace-type"))
 	})
 
-	for _, traceType := range fault.TraceTypes {
+	for _, traceType := range flags.TraceTypes {
 		traceType := traceType
 		t.Run("Valid_"+traceType.String(), func(t *testing.T) {
 			cfg := configForArgs(t, addRequiredArgsExcept("--trace-type", "--trace-type", traceType.String()))
@@ -172,7 +172,7 @@ func requiredArgs() map[string]string {
 		"--agree-with-proposed-output": agreeWithProposedOutput,
 		"--l1-eth-rpc":                 l1EthRpc,
 		"--game-address":               gameAddressValue,
-		"--trace-type":                 fault.TraceTypeAlphabet.String(),
+		"--trace-type":                 flags.TraceTypeAlphabet.String(),
 		"--alphabet":                   alphabetTrace,
 		"--cannon-datadir":             cannonDatadir,
 	}

op-challenger/config/config.go

@@ -4,7 +4,6 @@ import (
 	"errors"
 	"strings"
 
-	"github.com/ethereum-optimism/optimism/op-challenger/fault"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/urfave/cli/v2"
 
@@ -30,7 +29,7 @@ type Config struct {
 	AgreeWithProposedOutput bool           // Temporary config if we agree or disagree with the posted output
 	GameDepth               int            // Depth of the game tree
 
-	TraceType     fault.TraceType // Type of trace
+	TraceType     flags.TraceType // Type of trace
 	AlphabetTrace string          // String for the AlphabetTraceProvider
 	CannonDatadir string          // Cannon Data Directory for the CannonTraceProvider
 
@@ -40,7 +39,7 @@ type Config struct {
 func NewConfig(
 	l1EthRpc string,
 	gameAddress common.Address,
-	traceType fault.TraceType,
+	traceType flags.TraceType,
 	alphabetTrace string,
 	cannonDatadir string,
 	agreeWithProposedOutput bool,
@@ -71,10 +70,10 @@ func (c Config) Check() error {
 	if c.TraceType == "" {
 		return ErrMissingTraceType
 	}
-	if c.TraceType == fault.TraceTypeCannon && c.CannonDatadir == "" {
+	if c.TraceType == flags.TraceTypeCannon && c.CannonDatadir == "" {
 		return ErrMissingCannonDatadir
 	}
-	if c.TraceType == fault.TraceTypeAlphabet && c.AlphabetTrace == "" {
+	if c.TraceType == flags.TraceTypeAlphabet && c.AlphabetTrace == "" {
 		return ErrMissingAlphabetTrace
 	}
 	if err := c.TxMgrConfig.Check(); err != nil {
@@ -95,7 +94,7 @@ func NewConfigFromCLI(ctx *cli.Context) (*Config, error) {
 
 	txMgrConfig := txmgr.ReadCLIConfig(ctx)
 
-	traceTypeFlag := fault.TraceType(strings.ToLower(ctx.String(flags.TraceTypeFlag.Name)))
+	traceTypeFlag := flags.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/fault"
+	"github.com/ethereum-optimism/optimism/op-challenger/flags"
 	"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 fault.TraceType) Config {
+func validConfig(traceType flags.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(fault.TraceTypeCannon).Check()
+	err := validConfig(flags.TraceTypeCannon).Check()
 	require.NoError(t, err)
 }
 
 func TestTxMgrConfig(t *testing.T) {
 	t.Run("Invalid", func(t *testing.T) {
-		config := validConfig(fault.TraceTypeCannon)
+		config := validConfig(flags.TraceTypeCannon)
 		config.TxMgrConfig = txmgr.CLIConfig{}
 		require.Equal(t, config.Check().Error(), "must provide a L1 RPC url")
 	})
 }
 
 func TestL1EthRpcRequired(t *testing.T) {
-	config := validConfig(fault.TraceTypeCannon)
+	config := validConfig(flags.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(fault.TraceTypeCannon)
+	config := validConfig(flags.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(fault.TraceTypeAlphabet)
+	config := validConfig(flags.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(fault.TraceTypeCannon)
+	config := validConfig(flags.TraceTypeCannon)
 	config.CannonDatadir = ""
 	require.ErrorIs(t, config.Check(), ErrMissingCannonDatadir)
 	config.CannonDatadir = validCannonDatadir

op-challenger/fault/cannon/cannon_provider.go

@@ -7,7 +7,6 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/ethereum-optimism/optimism/op-challenger/fault"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 )
@@ -77,5 +76,3 @@ func (p *CannonTraceProvider) loadProof(i uint64) (*proofData, error) {
 	}
 	return &proof, nil
 }
-
-var _ fault.TraceProvider = (*CannonTraceProvider)(nil)

op-challenger/fault/service.go

@@ -7,6 +7,7 @@ import (
 	"github.com/ethereum-optimism/optimism/op-bindings/bindings"
 	"github.com/ethereum-optimism/optimism/op-challenger/config"
 	"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"
@@ -53,9 +54,9 @@ func NewService(ctx context.Context, logger log.Logger, cfg *config.Config) (*se
 
 	var trace TraceProvider
 	switch cfg.TraceType {
-	case TraceTypeCannon:
+	case flags.TraceTypeCannon:
 		trace = cannon.NewCannonTraceProvider(cfg.CannonDatadir)
-	case TraceTypeAlphabet:
+	case flags.TraceTypeAlphabet:
 		trace = NewAlphabetProvider(cfg.AlphabetTrace, uint64(cfg.GameDepth))
 	default:
 		return nil, fmt.Errorf("unsupported trace type: %v", cfg.TraceType)

op-challenger/fault/types.go

@@ -3,7 +3,6 @@ package fault
 import (
 	"context"
 	"errors"
-	"fmt"
 
 	"github.com/ethereum/go-ethereum/common"
 )
@@ -20,36 +19,6 @@ 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

@@ -4,7 +4,6 @@ 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"
 
@@ -21,6 +20,36 @@ func prefixEnvVars(name string) []string {
 	return opservice.PrefixEnvVar(envVarPrefix, name)
 }
 
+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
+}
+
 var (
 	// Required Flags
 	L1EthRpcFlag = &cli.StringFlag{
@@ -35,10 +64,10 @@ var (
 	}
 	TraceTypeFlag = &cli.GenericFlag{
 		Name:    "trace-type",
-		Usage:   "The trace type. Valid options: " + openum.EnumString(fault.TraceTypes),
+		Usage:   "The trace type. Valid options: " + openum.EnumString(TraceTypes),
 		EnvVars: prefixEnvVars("TRACE_TYPE"),
-		Value: func() *fault.TraceType {
-			out := fault.TraceType("") // No default value
+		Value: func() *TraceType {
+			out := TraceType("") // No default value
 			return &out
 		}(),
 	}
@@ -96,18 +125,18 @@ func CheckRequired(ctx *cli.Context) error {
 			return fmt.Errorf("flag %s is required", f.Names()[0])
 		}
 	}
-	gameType := fault.TraceType(strings.ToLower(ctx.String(TraceTypeFlag.Name)))
+	gameType := TraceType(strings.ToLower(ctx.String(TraceTypeFlag.Name)))
 	switch gameType {
-	case fault.TraceTypeCannon:
+	case TraceTypeCannon:
 		if !ctx.IsSet(CannonDatadirFlag.Name) {
 			return fmt.Errorf("flag %s is required", "cannon-datadir")
 		}
-	case fault.TraceTypeAlphabet:
+	case 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", fault.TraceTypes)
+		return fmt.Errorf("invalid trace type. must be one of %v", TraceTypes)
 	}
 	return nil
 }