Merge pull request #7665 from ethereum-optimism/aj/multi-type-config

op-challenger: Support multiple trace types on the command line/config

op-challenger/cmd/main_test.go

@@ -46,14 +46,14 @@ func TestLogLevel(t *testing.T) {
 
 func TestDefaultCLIOptionsMatchDefaultConfig(t *testing.T) {
 	cfg := configForArgs(t, addRequiredArgs(config.TraceTypeAlphabet))
-	defaultCfg := config.NewConfig(common.HexToAddress(gameFactoryAddressValue), l1EthRpc, config.TraceTypeAlphabet, true, datadir)
+	defaultCfg := config.NewConfig(common.HexToAddress(gameFactoryAddressValue), l1EthRpc, true, datadir, config.TraceTypeAlphabet)
 	// Add in the extra CLI options required when using alphabet trace type
 	defaultCfg.AlphabetTrace = alphabetTrace
 	require.Equal(t, defaultCfg, cfg)
 }
 
 func TestDefaultConfigIsValid(t *testing.T) {
-	cfg := config.NewConfig(common.HexToAddress(gameFactoryAddressValue), l1EthRpc, config.TraceTypeAlphabet, true, datadir)
+	cfg := config.NewConfig(common.HexToAddress(gameFactoryAddressValue), l1EthRpc, true, datadir, config.TraceTypeAlphabet)
 	// Add in options that are required based on the specific trace type
 	// To avoid needing to specify unused options, these aren't included in the params for NewConfig
 	cfg.AlphabetTrace = alphabetTrace
@@ -82,7 +82,7 @@ func TestTraceType(t *testing.T) {
 		traceType := traceType
 		t.Run("Valid_"+traceType.String(), func(t *testing.T) {
 			cfg := configForArgs(t, addRequiredArgs(traceType))
-			require.Equal(t, traceType, cfg.TraceType)
+			require.Equal(t, []config.TraceType{traceType}, cfg.TraceTypes)
 		})
 	}
 
@@ -91,6 +91,41 @@ func TestTraceType(t *testing.T) {
 	})
 }
 
+func TestMultipleTraceTypes(t *testing.T) {
+	t.Run("WithAllOptions", func(t *testing.T) {
+		argsMap := requiredArgs(config.TraceTypeCannon)
+		addRequiredOutputCannonArgs(argsMap)
+		addRequiredAlphabetArgs(argsMap)
+		args := toArgList(argsMap)
+		// Add extra trace types (cannon is already specified)
+		args = append(args,
+			"--trace-type", config.TraceTypeOutputCannon.String(),
+			"--trace-type", config.TraceTypeAlphabet.String())
+		cfg := configForArgs(t, args)
+		require.Equal(t, []config.TraceType{config.TraceTypeCannon, config.TraceTypeOutputCannon, config.TraceTypeAlphabet}, cfg.TraceTypes)
+	})
+	t.Run("WithSomeOptions", func(t *testing.T) {
+		argsMap := requiredArgs(config.TraceTypeCannon)
+		addRequiredAlphabetArgs(argsMap)
+		args := toArgList(argsMap)
+		// Add extra trace types (cannon is already specified)
+		args = append(args,
+			"--trace-type", config.TraceTypeAlphabet.String())
+		cfg := configForArgs(t, args)
+		require.Equal(t, []config.TraceType{config.TraceTypeCannon, config.TraceTypeAlphabet}, cfg.TraceTypes)
+	})
+
+	t.Run("SpecifySameOptionMultipleTimes", func(t *testing.T) {
+		argsMap := requiredArgs(config.TraceTypeCannon)
+		args := toArgList(argsMap)
+		// Add cannon trace type again
+		args = append(args, "--trace-type", config.TraceTypeCannon.String())
+		// We're fine with the same option being listed multiple times, just deduplicate them.
+		cfg := configForArgs(t, args)
+		require.Equal(t, []config.TraceType{config.TraceTypeCannon}, cfg.TraceTypes)
+	})
+}
+
 func TestGameFactoryAddress(t *testing.T) {
 	t.Run("Required", func(t *testing.T) {
 		verifyArgsInvalid(t, "flag game-factory-address is required", addRequiredArgsExcept(config.TraceTypeAlphabet, "--game-factory-address"))
@@ -441,18 +476,30 @@ func requiredArgs(traceType config.TraceType) map[string]string {
 	}
 	switch traceType {
 	case config.TraceTypeAlphabet:
+		addRequiredAlphabetArgs(args)
+	case config.TraceTypeCannon:
+		addRequiredCannonArgs(args)
+	case config.TraceTypeOutputCannon:
+		addRequiredOutputCannonArgs(args)
+	}
+	return args
+}
+
+func addRequiredAlphabetArgs(args map[string]string) {
 	args["--alphabet"] = alphabetTrace
-	case config.TraceTypeCannon, config.TraceTypeOutputCannon:
+}
+
+func addRequiredOutputCannonArgs(args map[string]string) {
+	addRequiredCannonArgs(args)
+	args["--rollup-rpc"] = rollupRpc
+}
+
+func addRequiredCannonArgs(args map[string]string) {
 	args["--cannon-network"] = cannonNetwork
 	args["--cannon-bin"] = cannonBin
 	args["--cannon-server"] = cannonServer
 	args["--cannon-prestate"] = cannonPreState
 	args["--cannon-l2"] = cannonL2
-	}
-	if traceType == config.TraceTypeOutputCannon {
-		args["--rollup-rpc"] = rollupRpc
-	}
-	return args
 }
 
 func toArgList(req map[string]string) []string {

op-challenger/config/config.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"runtime"
+	"slices"
 	"time"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -15,7 +16,7 @@ import (
 )
 
 var (
-	ErrMissingTraceType              = errors.New("missing trace type")
+	ErrMissingTraceType              = errors.New("no supported trace types specified")
 	ErrMissingDatadir                = errors.New("missing datadir")
 	ErrMaxConcurrencyZero            = errors.New("max concurrency must not be 0")
 	ErrMissingCannonL2               = errors.New("missing cannon L2")
@@ -108,7 +109,7 @@ type Config struct {
 	MaxConcurrency          uint             // Maximum number of threads to use when progressing games
 	PollInterval            time.Duration    // Polling interval for latest-block subscription when using an HTTP RPC provider
 
-	TraceType TraceType // Type of trace
+	TraceTypes []TraceType // Type of traces supported
 
 	// Specific to the alphabet trace provider
 	AlphabetTrace string // String for the AlphabetTraceProvider
@@ -135,9 +136,9 @@ type Config struct {
 func NewConfig(
 	gameFactoryAddress common.Address,
 	l1EthRpc string,
-	traceType TraceType,
 	agreeWithProposedOutput bool,
 	datadir string,
+	supportedTraceTypes ...TraceType,
 ) Config {
 	return Config{
 		L1EthRpc:           l1EthRpc,
@@ -147,7 +148,7 @@ func NewConfig(
 
 		AgreeWithProposedOutput: agreeWithProposedOutput,
 
-		TraceType: traceType,
+		TraceTypes: supportedTraceTypes,
 
 		TxMgrConfig:   txmgr.NewCLIConfig(l1EthRpc, txmgr.DefaultChallengerFlagValues),
 		MetricsConfig: opmetrics.DefaultCLIConfig(),
@@ -161,6 +162,10 @@ func NewConfig(
 	}
 }
 
+func (c Config) TraceTypeEnabled(t TraceType) bool {
+	return slices.Contains(c.TraceTypes, t)
+}
+
 func (c Config) Check() error {
 	if c.L1EthRpc == "" {
 		return ErrMissingL1EthRPC
@@ -168,7 +173,7 @@ func (c Config) Check() error {
 	if c.GameFactoryAddress == (common.Address{}) {
 		return ErrMissingGameFactoryAddress
 	}
-	if c.TraceType == "" {
+	if len(c.TraceTypes) == 0 {
 		return ErrMissingTraceType
 	}
 	if c.Datadir == "" {
@@ -177,12 +182,12 @@ func (c Config) Check() error {
 	if c.MaxConcurrency == 0 {
 		return ErrMaxConcurrencyZero
 	}
-	if c.TraceType == TraceTypeOutputCannon {
+	if c.TraceTypeEnabled(TraceTypeOutputCannon) {
 		if c.RollupRpc == "" {
 			return ErrMissingRollupRpc
 		}
 	}
-	if c.TraceType == TraceTypeCannon || c.TraceType == TraceTypeOutputCannon {
+	if c.TraceTypeEnabled(TraceTypeCannon) || c.TraceTypeEnabled(TraceTypeOutputCannon) {
 		if c.CannonBin == "" {
 			return ErrMissingCannonBin
 		}
@@ -220,7 +225,7 @@ func (c Config) Check() error {
 			return ErrMissingCannonInfoFreq
 		}
 	}
-	if c.TraceType == TraceTypeAlphabet && c.AlphabetTrace == "" {
+	if c.TraceTypeEnabled(TraceTypeAlphabet) && c.AlphabetTrace == "" {
 		return ErrMissingAlphabetTrace
 	}
 	if err := c.TxMgrConfig.Check(); err != nil {

op-challenger/config/config_test.go

@@ -25,7 +25,7 @@ var (
 )
 
 func validConfig(traceType TraceType) Config {
-	cfg := NewConfig(validGameFactoryAddress, validL1EthRpc, traceType, agreeWithProposedOutput, validDatadir)
+	cfg := NewConfig(validGameFactoryAddress, validL1EthRpc, agreeWithProposedOutput, validDatadir, traceType)
 	switch traceType {
 	case TraceTypeAlphabet:
 		cfg.AlphabetTrace = validAlphabetTrace
@@ -194,3 +194,26 @@ func TestNetworkMustBeValid(t *testing.T) {
 	cfg.CannonNetwork = "unknown"
 	require.ErrorIs(t, cfg.Check(), ErrCannonNetworkUnknown)
 }
+
+func TestRequireConfigForAllSupportedTraceTypes(t *testing.T) {
+	cfg := validConfig(TraceTypeCannon)
+	cfg.TraceTypes = []TraceType{TraceTypeCannon, TraceTypeOutputCannon, TraceTypeAlphabet}
+	// Set all required options and check its valid
+	cfg.RollupRpc = validRollupRpc
+	cfg.AlphabetTrace = validAlphabetTrace
+	require.NoError(t, cfg.Check())
+
+	// Require output cannon specific args
+	cfg.RollupRpc = ""
+	require.ErrorIs(t, cfg.Check(), ErrMissingRollupRpc)
+	cfg.RollupRpc = validRollupRpc
+
+	// Require cannon specific args
+	cfg.CannonL2 = ""
+	require.ErrorIs(t, cfg.Check(), ErrMissingCannonL2)
+	cfg.CannonL2 = validCannonL2
+
+	// Require alphabet specific args
+	cfg.AlphabetTrace = ""
+	require.ErrorIs(t, cfg.Check(), ErrMissingAlphabetTrace)
+}

op-challenger/flags/flags.go

@@ -3,6 +3,7 @@ package flags
 import (
 	"fmt"
 	"runtime"
+	"slices"
 	"strings"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -44,14 +45,10 @@ var (
 			"If empty, the challenger will play all games.",
 		EnvVars: prefixEnvVars("GAME_ALLOWLIST"),
 	}
-	TraceTypeFlag = &cli.GenericFlag{
+	TraceTypeFlag = &cli.StringSliceFlag{
 		Name:    "trace-type",
-		Usage:   "The trace type. Valid options: " + openum.EnumString(config.TraceTypes),
+		Usage:   "The trace types to support. Valid options: " + openum.EnumString(config.TraceTypes),
 		EnvVars: prefixEnvVars("TRACE_TYPE"),
-		Value: func() *config.TraceType {
-			out := config.TraceType("") // No default value
-			return &out
-		}(),
 	}
 	AgreeWithProposedOutputFlag = &cli.BoolFlag{
 		Name:    "agree-with-proposed-output",
@@ -210,14 +207,14 @@ func CheckCannonFlags(ctx *cli.Context) error {
 	return nil
 }
 
-func CheckRequired(ctx *cli.Context) error {
+func CheckRequired(ctx *cli.Context, traceTypes []config.TraceType) error {
 	for _, f := range requiredFlags {
 		if !ctx.IsSet(f.Names()[0]) {
 			return fmt.Errorf("flag %s is required", f.Names()[0])
 		}
 	}
-	gameType := config.TraceType(strings.ToLower(ctx.String(TraceTypeFlag.Name)))
-	switch gameType {
+	for _, traceType := range traceTypes {
+		switch traceType {
 		case config.TraceTypeCannon:
 			if err := CheckCannonFlags(ctx); err != nil {
 				return err
@@ -236,12 +233,31 @@ func CheckRequired(ctx *cli.Context) error {
 		default:
 			return fmt.Errorf("invalid trace type. must be one of %v", config.TraceTypes)
 		}
+	}
 	return nil
 }
 
+func parseTraceTypes(ctx *cli.Context) ([]config.TraceType, error) {
+	var traceTypes []config.TraceType
+	for _, typeName := range ctx.StringSlice(TraceTypeFlag.Name) {
+		traceType := new(config.TraceType)
+		if err := traceType.Set(typeName); err != nil {
+			return nil, err
+		}
+		if !slices.Contains(traceTypes, *traceType) {
+			traceTypes = append(traceTypes, *traceType)
+		}
+	}
+	return traceTypes, nil
+}
+
 // NewConfigFromCLI parses the Config from the provided flags or environment variables.
 func NewConfigFromCLI(ctx *cli.Context) (*config.Config, error) {
-	if err := CheckRequired(ctx); err != nil {
+	traceTypes, err := parseTraceTypes(ctx)
+	if err != nil {
+		return nil, err
+	}
+	if err := CheckRequired(ctx, traceTypes); err != nil {
 		return nil, err
 	}
 	gameFactoryAddress, err := opservice.ParseAddress(ctx.String(FactoryAddressFlag.Name))
@@ -263,8 +279,6 @@ func NewConfigFromCLI(ctx *cli.Context) (*config.Config, error) {
 	metricsConfig := opmetrics.ReadCLIConfig(ctx)
 	pprofConfig := oppprof.ReadCLIConfig(ctx)
 
-	traceTypeFlag := config.TraceType(strings.ToLower(ctx.String(TraceTypeFlag.Name)))
-
 	maxConcurrency := ctx.Uint(MaxConcurrencyFlag.Name)
 	if maxConcurrency == 0 {
 		return nil, fmt.Errorf("%v must not be 0", MaxConcurrencyFlag.Name)
@@ -272,7 +286,7 @@ func NewConfigFromCLI(ctx *cli.Context) (*config.Config, error) {
 	return &config.Config{
 		// Required Flags
 		L1EthRpc:                ctx.String(L1EthRpcFlag.Name),
-		TraceType:               traceTypeFlag,
+		TraceTypes:              traceTypes,
 		GameFactoryAddress:      gameFactoryAddress,
 		GameAllowlist:           allowedGames,
 		GameWindow:              ctx.Duration(GameWindowFlag.Name),

op-challenger/game/fault/register.go

@@ -35,8 +35,7 @@ func RegisterGameTypes(
 	txMgr txmgr.TxManager,
 	client bind.ContractCaller,
 ) {
-	switch cfg.TraceType {
-	case config.TraceTypeCannon:
+	if cfg.TraceTypeEnabled(config.TraceTypeCannon) {
 		resourceCreator := func(addr common.Address, gameDepth uint64, dir string) (faultTypes.TraceProvider, faultTypes.OracleUpdater, error) {
 			provider, err := cannon.NewTraceProvider(ctx, logger, m, cfg, client, dir, addr, gameDepth)
 			if err != nil {
@@ -52,7 +51,8 @@ func RegisterGameTypes(
 			return NewGamePlayer(ctx, logger, m, cfg, dir, game.Proxy, txMgr, client, resourceCreator)
 		}
 		registry.RegisterGameType(cannonGameType, playerCreator)
-	case config.TraceTypeAlphabet:
+	}
+	if cfg.TraceTypeEnabled(config.TraceTypeAlphabet) {
 		resourceCreator := func(addr common.Address, gameDepth uint64, dir string) (faultTypes.TraceProvider, faultTypes.OracleUpdater, error) {
 			provider := alphabet.NewTraceProvider(cfg.AlphabetTrace, gameDepth)
 			updater := alphabet.NewOracleUpdater(logger)

op-challenger/game/fault/trace/cannon/executor_test.go

@@ -24,7 +24,7 @@ func TestGenerateProof(t *testing.T) {
 	input := "starting.json"
 	tempDir := t.TempDir()
 	dir := filepath.Join(tempDir, "gameDir")
-	cfg := config.NewConfig(common.Address{0xbb}, "http://localhost:8888", config.TraceTypeCannon, true, tempDir)
+	cfg := config.NewConfig(common.Address{0xbb}, "http://localhost:8888", true, tempDir, config.TraceTypeCannon)
 	cfg.CannonAbsolutePreState = "pre.json"
 	cfg.CannonBin = "./bin/cannon"
 	cfg.CannonServer = "./bin/op-program"

op-e2e/e2eutils/challenger/helper.go

@@ -42,9 +42,6 @@ func WithFactoryAddress(addr common.Address) Option {
 
 func WithGameAddress(addr common.Address) Option {
 	return func(c *config.Config) {
-		if c.GameAllowlist == nil {
-			c.GameAllowlist = make([]common.Address, 0)
-		}
 		c.GameAllowlist = append(c.GameAllowlist, addr)
 	}
 }
@@ -63,7 +60,7 @@ func WithAgreeProposedOutput(agree bool) Option {
 
 func WithAlphabet(alphabet string) Option {
 	return func(c *config.Config) {
-		c.TraceType = config.TraceTypeAlphabet
+		c.TraceTypes = append(c.TraceTypes, config.TraceTypeAlphabet)
 		c.AlphabetTrace = alphabet
 	}
 }
@@ -82,7 +79,7 @@ func WithCannon(
 ) Option {
 	return func(c *config.Config) {
 		require := require.New(t)
-		c.TraceType = config.TraceTypeCannon
+		c.TraceTypes = append(c.TraceTypes, config.TraceTypeCannon)
 		c.CannonL2 = l2Endpoint
 		c.CannonBin = "../cannon/bin/cannon"
 		c.CannonServer = "../op-program/bin/op-program"
@@ -126,7 +123,7 @@ func NewChallenger(t *testing.T, ctx context.Context, l1Endpoint string, name st
 
 func NewChallengerConfig(t *testing.T, l1Endpoint string, options ...Option) *config.Config {
 	// Use the NewConfig method to ensure we pick up any defaults that are set.
-	cfg := config.NewConfig(common.Address{}, l1Endpoint, config.TraceTypeAlphabet, true, t.TempDir())
+	cfg := config.NewConfig(common.Address{}, l1Endpoint, true, t.TempDir())
 	cfg.TxMgrConfig.NumConfirmations = 1
 	cfg.TxMgrConfig.ReceiptQueryInterval = 1 * time.Second
 	if cfg.MaxConcurrency > 4 {

op-e2e/e2eutils/disputegame/alphabet_helper.go

@@ -3,11 +3,8 @@ package disputegame
 import (
 	"context"
 
-	"github.com/ethereum-optimism/optimism/op-challenger/config"
 	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/trace/alphabet"
 	"github.com/ethereum-optimism/optimism/op-e2e/e2eutils/challenger"
-
-	"github.com/ethereum/go-ethereum/common"
 )
 
 type AlphabetGameHelper struct {
@@ -17,15 +14,12 @@ type AlphabetGameHelper struct {
 
 func (g *AlphabetGameHelper) StartChallenger(ctx context.Context, l1Endpoint string, name string, options ...challenger.Option) *challenger.Helper {
 	opts := []challenger.Option{
-		func(c *config.Config) {
-			c.GameFactoryAddress = g.factoryAddr
-			c.GameAllowlist = []common.Address{g.addr}
-			c.TraceType = config.TraceTypeAlphabet
+		challenger.WithFactoryAddress(g.factoryAddr),
+		challenger.WithGameAddress(g.addr),
 		// By default the challenger agrees with the root claim (thus disagrees with the proposed output)
 		// This can be overridden by passing in options
-			c.AlphabetTrace = g.claimedAlphabet
-			c.AgreeWithProposedOutput = false
-		},
+		challenger.WithAlphabet(g.claimedAlphabet),
+		challenger.WithAgreeProposedOutput(false),
 	}
 	opts = append(opts, options...)
 	c := challenger.NewChallenger(g.t, ctx, l1Endpoint, name, opts...)

op-e2e/faultproof_test.go

@@ -2,9 +2,12 @@ package op_e2e
 
 import (
 	"context"
+	"math/big"
 	"testing"
 	"time"
 
+	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/trace/alphabet"
+	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/types"
 	"github.com/ethereum-optimism/optimism/op-e2e/e2eutils/challenger"
 	"github.com/ethereum-optimism/optimism/op-e2e/e2eutils/disputegame"
 	l2oo2 "github.com/ethereum-optimism/optimism/op-e2e/e2eutils/l2oo"
@@ -74,6 +77,41 @@ func TestMultipleCannonGames(t *testing.T) {
 	challenger.WaitForGameDataDeletion(ctx, game1, game2)
 }
 
+func TestMultipleGameTypes(t *testing.T) {
+	InitParallel(t)
+
+	ctx := context.Background()
+	sys, l1Client := startFaultDisputeSystem(t)
+	t.Cleanup(sys.Close)
+
+	gameFactory := disputegame.NewFactoryHelper(t, ctx, sys.cfg.L1Deployments, l1Client)
+	// Start a challenger with both cannon and alphabet support
+	gameFactory.StartChallenger(ctx, sys.NodeEndpoint("l1"), "TowerDefense",
+		challenger.WithCannon(t, sys.RollupConfig, sys.L2GenesisCfg, sys.NodeEndpoint("sequencer")),
+		challenger.WithAlphabet(disputegame.CorrectAlphabet),
+		challenger.WithPrivKey(sys.cfg.Secrets.Alice),
+		challenger.WithAgreeProposedOutput(true),
+	)
+
+	game1 := gameFactory.StartCannonGame(ctx, common.Hash{0x01, 0xaa})
+	game2 := gameFactory.StartAlphabetGame(ctx, "xyzabc")
+
+	// Wait for the challenger to respond to both games
+	game1.WaitForClaimCount(ctx, 2)
+	game2.WaitForClaimCount(ctx, 2)
+	game1Response := game1.GetClaimValue(ctx, 1)
+	game2Response := game2.GetClaimValue(ctx, 1)
+	// The alphabet game always posts the same traces, so if they're different they can't both be from the alphabet.
+	require.NotEqual(t, game1Response, game2Response, "should have posted different claims")
+	// Now check they aren't both just from different cannon games by confirming the alphabet value.
+	correctAlphabet := alphabet.NewTraceProvider(disputegame.CorrectAlphabet, uint64(game2.MaxDepth(ctx)))
+	expectedClaim, err := correctAlphabet.Get(ctx, types.NewPositionFromGIndex(big.NewInt(1)).Attack())
+	require.NoError(t, err)
+	require.Equal(t, expectedClaim, game2Response)
+	// We don't confirm the cannon value because generating the correct claim is expensive
+	// Just being different is enough to confirm the challenger isn't just playing two alphabet games incorrectly
+}
+
 func TestChallengerCompleteDisputeGame(t *testing.T) {
 	InitParallel(t)