op-challenger: Make l2-eth-rpc and rollup-rpc required

op-challenger/cmd/main_test.go

@@ -52,17 +52,12 @@ func TestLogLevel(t *testing.T) {
 
 func TestDefaultCLIOptionsMatchDefaultConfig(t *testing.T) {
 	cfg := configForArgs(t, addRequiredArgs(config.TraceTypeAlphabet))
-	defaultCfg := config.NewConfig(common.HexToAddress(gameFactoryAddressValue), l1EthRpc, l1Beacon, datadir, config.TraceTypeAlphabet)
-	// Add in the extra CLI options required when using alphabet trace type
-	defaultCfg.RollupRpc = rollupRpc
+	defaultCfg := config.NewConfig(common.HexToAddress(gameFactoryAddressValue), l1EthRpc, l1Beacon, rollupRpc, l2EthRpc, datadir, config.TraceTypeAlphabet)
 	require.Equal(t, defaultCfg, cfg)
 }
 
 func TestDefaultConfigIsValid(t *testing.T) {
-	cfg := config.NewConfig(common.HexToAddress(gameFactoryAddressValue), l1EthRpc, l1Beacon, 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.RollupRpc = rollupRpc
+	cfg := config.NewConfig(common.HexToAddress(gameFactoryAddressValue), l1EthRpc, l1Beacon, rollupRpc, l2EthRpc, datadir, config.TraceTypeAlphabet)
 	require.NoError(t, cfg.Check())
 }
 
@@ -114,7 +109,6 @@ func TestTraceType(t *testing.T) {
 func TestMultipleTraceTypes(t *testing.T) {
 	t.Run("WithAllOptions", func(t *testing.T) {
 		argsMap := requiredArgs(config.TraceTypeCannon)
-		addRequiredOutputArgs(argsMap)
 		// Add Asterisc required flags
 		addRequiredAsteriscArgs(argsMap)
 		args := toArgList(argsMap)
@@ -130,7 +124,6 @@ func TestMultipleTraceTypes(t *testing.T) {
 	})
 	t.Run("WithSomeOptions", func(t *testing.T) {
 		argsMap := requiredArgs(config.TraceTypeCannon)
-		addRequiredOutputArgs(argsMap)
 		args := toArgList(argsMap)
 		// Add extra trace types (cannon is already specified)
 		args = append(args,
@@ -315,14 +308,6 @@ func TestAsteriscRequiredArgs(t *testing.T) {
 		})
 
 		t.Run(fmt.Sprintf("TestL2Rpc-%v", traceType), func(t *testing.T) {
-			t.Run("NotRequiredForAlphabetTraceLegacy", func(t *testing.T) {
-				configForArgs(t, addRequiredArgsExcept(config.TraceTypeAlphabet, "--cannon-l2"))
-			})
-
-			t.Run("NotRequiredForAlphabetTrace", func(t *testing.T) {
-				configForArgs(t, addRequiredArgsExcept(config.TraceTypeAlphabet, "--l2-eth-rpc"))
-			})
-
 			t.Run("RequiredForAsteriscTrace", func(t *testing.T) {
 				verifyArgsInvalid(t, "flag l2-eth-rpc is required", addRequiredArgsExcept(traceType, "--l2-eth-rpc"))
 			})
@@ -438,6 +423,25 @@ func TestAsteriscRequiredArgs(t *testing.T) {
 		})
 	}
 }
+
+func TestAlphabetRequiredArgs(t *testing.T) {
+	t.Run(fmt.Sprintf("TestL2Rpc-%v", config.TraceTypeAlphabet), func(t *testing.T) {
+		t.Run("RequiredForAlphabetTrace", func(t *testing.T) {
+			verifyArgsInvalid(t, "flag l2-eth-rpc is required", addRequiredArgsExcept(config.TraceTypeAlphabet, "--l2-eth-rpc"))
+		})
+
+		t.Run("ValidLegacy", func(t *testing.T) {
+			cfg := configForArgs(t, addRequiredArgsExcept(config.TraceTypeAlphabet, "--l2-eth-rpc", fmt.Sprintf("--cannon-l2=%s", l2EthRpc)))
+			require.Equal(t, l2EthRpc, cfg.L2Rpc)
+		})
+
+		t.Run("Valid", func(t *testing.T) {
+			cfg := configForArgs(t, addRequiredArgs(config.TraceTypeAlphabet))
+			require.Equal(t, l2EthRpc, cfg.L2Rpc)
+		})
+	})
+}
+
 func TestCannonRequiredArgs(t *testing.T) {
 	for _, traceType := range []config.TraceType{config.TraceTypeCannon, config.TraceTypePermissioned} {
 		traceType := traceType
@@ -502,14 +506,6 @@ func TestCannonRequiredArgs(t *testing.T) {
 		})
 
 		t.Run(fmt.Sprintf("TestL2Rpc-%v", traceType), func(t *testing.T) {
-			t.Run("NotRequiredForAlphabetTraceLegacy", func(t *testing.T) {
-				configForArgs(t, addRequiredArgsExcept(config.TraceTypeAlphabet, "--cannon-l2"))
-			})
-
-			t.Run("NotRequiredForAlphabetTrace", func(t *testing.T) {
-				configForArgs(t, addRequiredArgsExcept(config.TraceTypeAlphabet, "--l2-eth-rpc"))
-			})
-
 			t.Run("RequiredForCannonTrace", func(t *testing.T) {
 				verifyArgsInvalid(t, "flag l2-eth-rpc is required", addRequiredArgsExcept(traceType, "--l2-eth-rpc"))
 			})
@@ -772,6 +768,8 @@ func requiredArgs(traceType config.TraceType) map[string]string {
 	args := map[string]string{
 		"--l1-eth-rpc":           l1EthRpc,
 		"--l1-beacon":            l1Beacon,
+		"--rollup-rpc":           rollupRpc,
+		"--l2-eth-rpc":           l2EthRpc,
 		"--game-factory-address": gameFactoryAddressValue,
 		"--trace-type":           traceType.String(),
 		"--datadir":              datadir,
@@ -781,8 +779,6 @@ func requiredArgs(traceType config.TraceType) map[string]string {
 		addRequiredCannonArgs(args)
 	case config.TraceTypeAsterisc:
 		addRequiredAsteriscArgs(args)
-	case config.TraceTypeAlphabet:
-		addRequiredOutputArgs(args)
 	}
 	return args
 }
@@ -793,7 +789,6 @@ func addRequiredCannonArgs(args map[string]string) {
 	args["--cannon-server"] = cannonServer
 	args["--cannon-prestate"] = cannonPreState
 	args["--l2-eth-rpc"] = l2EthRpc
-	addRequiredOutputArgs(args)
 }
 
 func addRequiredAsteriscArgs(args map[string]string) {
@@ -802,11 +797,6 @@ func addRequiredAsteriscArgs(args map[string]string) {
 	args["--asterisc-server"] = asteriscServer
 	args["--asterisc-prestate"] = asteriscPreState
 	args["--l2-eth-rpc"] = l2EthRpc
-	addRequiredOutputArgs(args)
-}
-
-func addRequiredOutputArgs(args map[string]string) {
-	args["--rollup-rpc"] = rollupRpc
 }
 
 func toArgList(req map[string]string) []string {

op-challenger/config/config.go

@@ -159,12 +159,16 @@ func NewConfig(
 	gameFactoryAddress common.Address,
 	l1EthRpc string,
 	l1BeaconApi string,
+	l2RollupRpc string,
+	l2EthRpc string,
 	datadir string,
 	supportedTraceTypes ...TraceType,
 ) Config {
 	return Config{
 		L1EthRpc:           l1EthRpc,
 		L1Beacon:           l1BeaconApi,
+		RollupRpc:          l2RollupRpc,
+		L2Rpc:              l2EthRpc,
 		GameFactoryAddress: gameFactoryAddress,
 		MaxConcurrency:     uint(runtime.NumCPU()),
 		PollInterval:       DefaultPollInterval,
@@ -201,6 +205,9 @@ func (c Config) Check() error {
 	if c.RollupRpc == "" {
 		return ErrMissingRollupRpc
 	}
+	if c.L2Rpc == "" {
+		return ErrMissingL2Rpc
+	}
 	if c.GameFactoryAddress == (common.Address{}) {
 		return ErrMissingGameFactoryAddress
 	}
@@ -244,9 +251,6 @@ func (c Config) Check() error {
 		if c.CannonAbsolutePreState != "" && c.CannonAbsolutePreStateBaseURL != nil {
 			return ErrCannonAbsolutePreStateAndBaseURL
 		}
-		if c.L2Rpc == "" {
-			return ErrMissingL2Rpc
-		}
 		if c.CannonSnapshotFreq == 0 {
 			return ErrMissingCannonSnapshotFreq
 		}
@@ -285,9 +289,6 @@ func (c Config) Check() error {
 		if c.AsteriscAbsolutePreState != "" && c.AsteriscAbsolutePreStateBaseURL != nil {
 			return ErrAsteriscAbsolutePreStateAndBaseURL
 		}
-		if c.L2Rpc == "" {
-			return ErrMissingL2Rpc
-		}
 		if c.AsteriscSnapshotFreq == 0 {
 			return ErrMissingAsteriscSnapshotFreq
 		}

op-challenger/config/config_test.go

@@ -40,26 +40,23 @@ func applyValidConfigForCannon(cfg *Config) {
 	cfg.CannonServer = validCannonOpProgramBin
 	cfg.CannonAbsolutePreStateBaseURL = validCannonAbsolutPreStateBaseURL
 	cfg.CannonNetwork = validCannonNetwork
-	cfg.L2Rpc = validL2Rpc
 }
 
 func applyValidConfigForAsterisc(cfg *Config) {
 	cfg.AsteriscBin = validAsteriscBin
 	cfg.AsteriscServer = validAsteriscOpProgramBin
 	cfg.AsteriscAbsolutePreStateBaseURL = validAsteriscAbsolutPreStateBaseURL
 	cfg.AsteriscNetwork = validAsteriscNetwork
-	cfg.L2Rpc = validL2Rpc
 }
 
 func validConfig(traceType TraceType) Config {
-	cfg := NewConfig(validGameFactoryAddress, validL1EthRpc, validL1BeaconUrl, validDatadir, traceType)
+	cfg := NewConfig(validGameFactoryAddress, validL1EthRpc, validL1BeaconUrl, validRollupRpc, validL2Rpc, validDatadir, traceType)
 	if traceType == TraceTypeCannon || traceType == TraceTypePermissioned {
 		applyValidConfigForCannon(&cfg)
 	}
 	if traceType == TraceTypeAsterisc {
 		applyValidConfigForAsterisc(&cfg)
 	}
-	cfg.RollupRpc = validRollupRpc
 	return cfg
 }
 

op-challenger/flags/flags.go

@@ -296,10 +296,6 @@ func CheckCannonFlags(ctx *cli.Context) error {
 	if !ctx.IsSet(CannonPreStateFlag.Name) && !ctx.IsSet(CannonPreStatesURLFlag.Name) {
 		return fmt.Errorf("flag %s or %s is required", CannonPreStatesURLFlag.Name, CannonPreStateFlag.Name)
 	}
-	// CannonL2Flag is checked because it is an alias with L2EthRpcFlag
-	if !ctx.IsSet(CannonL2Flag.Name) && !ctx.IsSet(L2EthRpcFlag.Name) {
-		return fmt.Errorf("flag %s is required", L2EthRpcFlag.Name)
-	}
 	return nil
 }
 
@@ -323,10 +319,6 @@ func CheckAsteriscFlags(ctx *cli.Context) error {
 	if !ctx.IsSet(AsteriscPreStateFlag.Name) && !ctx.IsSet(AsteriscPreStatesURLFlag.Name) {
 		return fmt.Errorf("flag %s or %s is required", AsteriscPreStatesURLFlag.Name, AsteriscPreStateFlag.Name)
 	}
-	// CannonL2Flag is checked because it is an alias with L2EthRpcFlag
-	if !ctx.IsSet(CannonL2Flag.Name) && !ctx.IsSet(L2EthRpcFlag.Name) {
-		return fmt.Errorf("flag %s is required", L2EthRpcFlag.Name)
-	}
 	return nil
 }
 
@@ -336,6 +328,10 @@ func CheckRequired(ctx *cli.Context, traceTypes []config.TraceType) error {
 			return fmt.Errorf("flag %s is required", f.Names()[0])
 		}
 	}
+	// CannonL2Flag is checked because it is an alias with L2EthRpcFlag
+	if !ctx.IsSet(CannonL2Flag.Name) && !ctx.IsSet(L2EthRpcFlag.Name) {
+		return fmt.Errorf("flag %s is required", L2EthRpcFlag.Name)
+	}
 	for _, traceType := range traceTypes {
 		switch traceType {
 		case config.TraceTypeCannon, config.TraceTypePermissioned:

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

@@ -20,7 +20,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", "http://localhost:9000", tempDir, config.TraceTypeAsterisc)
+	cfg := config.NewConfig(common.Address{0xbb}, "http://localhost:8888", "http://localhost:9000", "http://localhost:9096", "http://localhost:9095", tempDir, config.TraceTypeAsterisc)
 	cfg.L2Rpc = "http://localhost:9999"
 	cfg.AsteriscAbsolutePreState = "pre.json"
 	cfg.AsteriscBin = "./bin/asterisc"

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

@@ -25,7 +25,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", "http://localhost:9000", tempDir, config.TraceTypeCannon)
+	cfg := config.NewConfig(common.Address{0xbb}, "http://localhost:8888", "http://localhost:9000", "http://localhost:9096", "http://localhost:9095", tempDir, config.TraceTypeCannon)
 	cfg.L2Rpc = "http://localhost:9999"
 	cfg.CannonAbsolutePreState = "pre.json"
 	cfg.CannonBin = "./bin/cannon"

op-e2e/e2eutils/challenger/helper.go

@@ -29,6 +29,7 @@ import (
 
 type EndpointProvider interface {
 	NodeEndpoint(name string) string
+	RollupEndpoint(name string) string
 	L1BeaconEndpoint() string
 }
 
@@ -95,15 +96,8 @@ func FindMonorepoRoot(t *testing.T) string {
 	return ""
 }
 
-func applyCannonConfig(
-	c *config.Config,
-	t *testing.T,
-	rollupCfg *rollup.Config,
-	l2Genesis *core.Genesis,
-	l2Endpoint string,
-) {
+func applyCannonConfig(c *config.Config, t *testing.T, rollupCfg *rollup.Config, l2Genesis *core.Genesis) {
 	require := require.New(t)
-	c.L2Rpc = l2Endpoint
 	root := FindMonorepoRoot(t)
 	c.CannonBin = root + "cannon/bin/cannon"
 	c.CannonServer = root + "op-program/bin/op-program"
@@ -123,31 +117,23 @@ func applyCannonConfig(
 	c.CannonRollupConfigPath = rollupFile
 }
 
-func WithCannon(
-	t *testing.T,
-	rollupCfg *rollup.Config,
-	l2Genesis *core.Genesis,
-	rollupEndpoint string,
-	l2Endpoint string,
-) Option {
+func WithCannon(t *testing.T, rollupCfg *rollup.Config, l2Genesis *core.Genesis) Option {
 	return func(c *config.Config) {
 		c.TraceTypes = append(c.TraceTypes, config.TraceTypeCannon)
-		c.RollupRpc = rollupEndpoint
-		applyCannonConfig(c, t, rollupCfg, l2Genesis, l2Endpoint)
+		applyCannonConfig(c, t, rollupCfg, l2Genesis)
 	}
 }
 
-func WithAlphabet(rollupEndpoint string) Option {
+func WithAlphabet() Option {
 	return func(c *config.Config) {
 		c.TraceTypes = append(c.TraceTypes, config.TraceTypeAlphabet)
-		c.RollupRpc = rollupEndpoint
 	}
 }
 
 func NewChallenger(t *testing.T, ctx context.Context, sys EndpointProvider, name string, options ...Option) *Helper {
 	log := testlog.Logger(t, log.LevelDebug).New("role", name)
 	log.Info("Creating challenger")
-	cfg := NewChallengerConfig(t, sys, options...)
+	cfg := NewChallengerConfig(t, sys, "sequencer", options...)
 	chl, err := challenger.Main(ctx, log, cfg)
 	require.NoError(t, err, "must init challenger")
 	require.NoError(t, chl.Start(ctx), "must start challenger")
@@ -161,11 +147,11 @@ func NewChallenger(t *testing.T, ctx context.Context, sys EndpointProvider, name
 	}
 }
 
-func NewChallengerConfig(t *testing.T, sys EndpointProvider, options ...Option) *config.Config {
+func NewChallengerConfig(t *testing.T, sys EndpointProvider, l2NodeName string, options ...Option) *config.Config {
 	// Use the NewConfig method to ensure we pick up any defaults that are set.
 	l1Endpoint := sys.NodeEndpoint("l1")
 	l1Beacon := sys.L1BeaconEndpoint()
-	cfg := config.NewConfig(common.Address{}, l1Endpoint, l1Beacon, t.TempDir())
+	cfg := config.NewConfig(common.Address{}, l1Endpoint, l1Beacon, sys.RollupEndpoint(l2NodeName), sys.NodeEndpoint(l2NodeName), t.TempDir())
 	// The devnet can't set the absolute prestate output root because the contracts are deployed in L1 genesis
 	// before the L2 genesis is known.
 	cfg.AllowInvalidPrestate = true

op-e2e/e2eutils/disputegame/alphabet_helper.go

@@ -17,7 +17,7 @@ func (g *AlphabetGameHelper) StartChallenger(ctx context.Context, sys challenger
 	opts := []challenger.Option{
 		challenger.WithFactoryAddress(g.factoryAddr),
 		challenger.WithGameAddress(g.addr),
-		challenger.WithAlphabet(g.system.RollupEndpoint("sequencer")),
+		challenger.WithAlphabet(),
 	}
 	opts = append(opts, options...)
 	c := challenger.NewChallenger(g.t, ctx, sys, name, opts...)

op-e2e/e2eutils/disputegame/game_helper.go

@@ -25,7 +25,6 @@ type FaultGameHelper struct {
 	game        *bindings.FaultDisputeGame
 	factoryAddr common.Address
 	addr        common.Address
-	system      DisputeSystem
 }
 
 func (g *FaultGameHelper) Addr() common.Address {

op-e2e/e2eutils/disputegame/output_alphabet_helper.go

@@ -24,7 +24,7 @@ func (g *OutputAlphabetGameHelper) StartChallenger(
 	options ...challenger.Option,
 ) *challenger.Helper {
 	opts := []challenger.Option{
-		challenger.WithAlphabet(g.System.RollupEndpoint(l2Node)),
+		challenger.WithAlphabet(),
 		challenger.WithFactoryAddress(g.FactoryAddr),
 		challenger.WithGameAddress(g.Addr),
 	}