op-challenger: Support running multiple prestates the same game type in run-trace (#12443)

op-challenger/cmd/run_trace.go

@@ -4,11 +4,11 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"net/url"
+	"slices"
+	"strings"
 
-	"github.com/ethereum-optimism/optimism/op-challenger/config"
 	"github.com/ethereum-optimism/optimism/op-challenger/flags"
-	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/trace/vm"
+	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/types"
 	"github.com/ethereum-optimism/optimism/op-challenger/runner"
 	opservice "github.com/ethereum-optimism/optimism/op-service"
 	"github.com/ethereum-optimism/optimism/op-service/cliapp"
@@ -16,8 +16,12 @@ import (
 	"github.com/urfave/cli/v2"
 )
 
-func RunTrace(ctx *cli.Context, _ context.CancelCauseFunc) (cliapp.Lifecycle, error) {
+var (
+	ErrUnknownTraceType    = errors.New("unknown trace type")
+	ErrInvalidPrestateHash = errors.New("invalid prestate hash")
+)
 
+func RunTrace(ctx *cli.Context, _ context.CancelCauseFunc) (cliapp.Lifecycle, error) {
 	logger, err := setupLogging(ctx)
 	if err != nil {
 		return nil, err
@@ -31,36 +35,21 @@ func RunTrace(ctx *cli.Context, _ context.CancelCauseFunc) (cliapp.Lifecycle, er
 	if err := cfg.Check(); err != nil {
 		return nil, err
 	}
-	if err := checkMTCannonFlags(ctx, cfg); err != nil {
-		return nil, err
-	}
-
-	var mtPrestate common.Hash
-	var mtPrestateURL *url.URL
-	if ctx.IsSet(addMTCannonPrestateFlag.Name) {
-		mtPrestate = common.HexToHash(ctx.String(addMTCannonPrestateFlag.Name))
-		mtPrestateURL, err = url.Parse(ctx.String(addMTCannonPrestateURLFlag.Name))
+	runConfigs, err := parseRunArgs(ctx.StringSlice(RunTraceRunFlag.Name))
 	if err != nil {
-			return nil, fmt.Errorf("invalid mt-cannon prestate url (%v): %w", ctx.String(addMTCannonPrestateFlag.Name), err)
-		}
-	}
-	return runner.NewRunner(logger, cfg, mtPrestate, mtPrestateURL), nil
-}
-
-func checkMTCannonFlags(ctx *cli.Context, cfg *config.Config) error {
-	if ctx.IsSet(addMTCannonPrestateFlag.Name) || ctx.IsSet(addMTCannonPrestateURLFlag.Name) {
-		if ctx.IsSet(addMTCannonPrestateFlag.Name) != ctx.IsSet(addMTCannonPrestateURLFlag.Name) {
-			return fmt.Errorf("both flag %v and %v must be set when running MT-Cannon traces", addMTCannonPrestateURLFlag.Name, addMTCannonPrestateFlag.Name)
+		return nil, err
 	}
-		if cfg.Cannon == (vm.Config{}) {
-			return errors.New("required Cannon vm configuration for mt-cannon traces is missing")
+	if len(runConfigs) == 0 {
+		// Default to running on-chain version of each enabled trace type
+		for _, traceType := range cfg.TraceTypes {
+			runConfigs = append(runConfigs, runner.RunConfig{TraceType: traceType})
 		}
 	}
-	return nil
+	return runner.NewRunner(logger, cfg, runConfigs), nil
 }
 
 func runTraceFlags() []cli.Flag {
-	return append(flags.Flags, addMTCannonPrestateFlag, addMTCannonPrestateURLFlag)
+	return append(flags.Flags, RunTraceRunFlag)
 }
 
 var RunTraceCommand = &cli.Command{
@@ -72,14 +61,50 @@ var RunTraceCommand = &cli.Command{
 }
 
 var (
-	addMTCannonPrestateFlag = &cli.StringFlag{
-		Name:    "add-mt-cannon-prestate",
-		Usage:   "Use this prestate to run MT-Cannon compatibility tests",
-		EnvVars: opservice.PrefixEnvVar(flags.EnvVarPrefix, "ADD_MT_CANNON_PRESTATE"),
-	}
-	addMTCannonPrestateURLFlag = &cli.StringFlag{
-		Name:    "add-mt-cannon-prestate-url",
-		Usage:   "Use this prestate URL to run MT-Cannon compatibility tests",
-		EnvVars: opservice.PrefixEnvVar(flags.EnvVarPrefix, "ADD_MT_CANNON_PRESTATE_URL"),
+	RunTraceRunFlag = &cli.StringSliceFlag{
+		Name: "run",
+		Usage: "Specify a trace to run. Format is traceType/name/prestateHash where " +
+			"traceType is the trace type to use with the prestate (e.g cannon or asterisc-kona), " +
+			"name is an arbitrary name for the prestate to use when reporting metrics and" +
+			"prestateHash is the hex encoded absolute prestate commitment to use. " +
+			"If name is omitted the trace type name is used." +
+			"If the prestateHash is omitted, the absolute prestate hash used for new games on-chain.",
+		EnvVars: opservice.PrefixEnvVar(flags.EnvVarPrefix, "RUN"),
 	}
 )
+
+func parseRunArgs(args []string) ([]runner.RunConfig, error) {
+	cfgs := make([]runner.RunConfig, len(args))
+	for i, arg := range args {
+		cfg, err := parseRunArg(arg)
+		if err != nil {
+			return nil, err
+		}
+		cfgs[i] = cfg
+	}
+	return cfgs, nil
+}
+
+func parseRunArg(arg string) (runner.RunConfig, error) {
+	cfg := runner.RunConfig{}
+	opts := strings.SplitN(arg, "/", 3)
+	if len(opts) == 0 {
+		return runner.RunConfig{}, fmt.Errorf("invalid run config %q", arg)
+	}
+	cfg.TraceType = types.TraceType(opts[0])
+	if !slices.Contains(types.TraceTypes, cfg.TraceType) {
+		return runner.RunConfig{}, fmt.Errorf("%w %q for run config %q", ErrUnknownTraceType, opts[0], arg)
+	}
+	if len(opts) > 1 {
+		cfg.Name = opts[1]
+	} else {
+		cfg.Name = cfg.TraceType.String()
+	}
+	if len(opts) > 2 {
+		cfg.Prestate = common.HexToHash(opts[2])
+		if cfg.Prestate == (common.Hash{}) {
+			return runner.RunConfig{}, fmt.Errorf("%w %q for run config %q", ErrInvalidPrestateHash, opts[2], arg)
+		}
+	}
+	return cfg, nil
+}

op-challenger/cmd/run_trace_test.go

+package main
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/types"
+	"github.com/ethereum-optimism/optimism/op-challenger/runner"
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseRunArg(t *testing.T) {
+	tests := []struct {
+		arg      string
+		expected runner.RunConfig
+		err      error
+	}{
+		{arg: "unknown/test1/0x1234", err: ErrUnknownTraceType},
+		{arg: "cannon", expected: runner.RunConfig{TraceType: types.TraceTypeCannon, Name: types.TraceTypeCannon.String()}},
+		{arg: "asterisc", expected: runner.RunConfig{TraceType: types.TraceTypeAsterisc, Name: types.TraceTypeAsterisc.String()}},
+		{arg: "cannon/test1", expected: runner.RunConfig{TraceType: types.TraceTypeCannon, Name: "test1"}},
+		{arg: "cannon/test1/0x1234", expected: runner.RunConfig{TraceType: types.TraceTypeCannon, Name: "test1", Prestate: common.HexToHash("0x1234")}},
+		{arg: "cannon/test1/invalid", err: ErrInvalidPrestateHash},
+	}
+	for _, test := range tests {
+		test := test
+		// Slash characters in test names confuse some things that parse the output as it looks like a subtest
+		t.Run(strings.ReplaceAll(test.arg, "/", "_"), func(t *testing.T) {
+			actual, err := parseRunArg(test.arg)
+			require.ErrorIs(t, err, test.err)
+			require.Equal(t, test.expected, actual)
+		})
+	}
+}

op-challenger/runner/factory.go

@@ -60,28 +60,6 @@ func createTraceProvider(
 	return nil, errors.New("invalid trace type")
 }
 
-func createMTTraceProvider(
-	ctx context.Context,
-	logger log.Logger,
-	m vm.Metricer,
-	vmConfig vm.Config,
-	prestateHash common.Hash,
-	absolutePrestateBaseURL *url.URL,
-	localInputs utils.LocalGameInputs,
-	dir string,
-) (types.TraceProvider, error) {
-	executor := vm.NewOpProgramServerExecutor(logger)
-	stateConverter := cannon.NewStateConverter(vmConfig)
-
-	prestateSource := prestates.NewMultiPrestateProvider(absolutePrestateBaseURL, filepath.Join(dir, "prestates"), stateConverter)
-	prestatePath, err := prestateSource.PrestatePath(ctx, prestateHash)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get prestate %v: %w", prestateHash, err)
-	}
-	prestateProvider := vm.NewPrestateProvider(prestatePath, stateConverter)
-	return cannon.NewTraceProvider(logger, m, vmConfig, executor, prestateProvider, prestatePath, localInputs, dir, 42), nil
-}
-
 func getPrestate(ctx context.Context, prestateHash common.Hash, prestateBaseUrl *url.URL, prestatePath string, dataDir string, stateConverter vm.StateConverter) (string, error) {
 	prestateSource := prestates.NewPrestateSource(
 		prestateBaseUrl,

op-challenger/runner/runner.go

@@ -5,9 +5,9 @@ import (
 	"errors"
 	"fmt"
 	"math/big"
-	"net/url"
 	"os"
 	"path/filepath"
+	"regexp"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -29,8 +29,6 @@ import (
 	"github.com/ethereum/go-ethereum/log"
 )
 
-const mtCannonType = "mt-cannon"
-
 var (
 	ErrUnexpectedStatusCode = errors.New("unexpected status code")
 )
@@ -45,11 +43,16 @@ type Metricer interface {
 	RecordSuccess(vmType string)
 }
 
+type RunConfig struct {
+	TraceType types.TraceType
+	Name      string
+	Prestate  common.Hash
+}
+
 type Runner struct {
 	log        log.Logger
 	cfg        *config.Config
-	addMTCannonPrestate    common.Hash
-	addMTCannonPrestateURL *url.URL
+	runConfigs []RunConfig
 	m          Metricer
 
 	running    atomic.Bool
@@ -59,12 +62,11 @@ type Runner struct {
 	metricsSrv *httputil.HTTPServer
 }
 
-func NewRunner(logger log.Logger, cfg *config.Config, mtCannonPrestate common.Hash, mtCannonPrestateURL *url.URL) *Runner {
+func NewRunner(logger log.Logger, cfg *config.Config, runConfigs []RunConfig) *Runner {
 	return &Runner{
 		log:        logger,
 		cfg:        cfg,
-		addMTCannonPrestate:    mtCannonPrestate,
-		addMTCannonPrestateURL: mtCannonPrestateURL,
+		runConfigs: runConfigs,
 		m:          NewMetrics(),
 	}
 }
@@ -91,21 +93,21 @@ func (r *Runner) Start(ctx context.Context) error {
 	}
 	caller := batching.NewMultiCaller(l1Client, batching.DefaultBatchSize)
 
-	for _, traceType := range r.cfg.TraceTypes {
+	for _, runConfig := range r.runConfigs {
 		r.wg.Add(1)
-		go r.loop(ctx, traceType, rollupClient, caller)
+		go r.loop(ctx, runConfig, rollupClient, caller)
 	}
 
-	r.log.Info("Runners started")
+	r.log.Info("Runners started", "num", len(r.runConfigs))
 	return nil
 }
 
-func (r *Runner) loop(ctx context.Context, traceType types.TraceType, client *sources.RollupClient, caller *batching.MultiCaller) {
+func (r *Runner) loop(ctx context.Context, runConfig RunConfig, client *sources.RollupClient, caller *batching.MultiCaller) {
 	defer r.wg.Done()
 	t := time.NewTicker(1 * time.Minute)
 	defer t.Stop()
 	for {
-		r.runAndRecordOnce(ctx, traceType, client, caller)
+		r.runAndRecordOnce(ctx, runConfig, client, caller)
 		select {
 		case <-t.C:
 		case <-ctx.Done():
@@ -114,80 +116,50 @@ func (r *Runner) loop(ctx context.Context, traceType types.TraceType, client *so
 	}
 }
 
-func (r *Runner) runAndRecordOnce(ctx context.Context, traceType types.TraceType, client *sources.RollupClient, caller *batching.MultiCaller) {
+func (r *Runner) runAndRecordOnce(ctx context.Context, runConfig RunConfig, client *sources.RollupClient, caller *batching.MultiCaller) {
 	recordError := func(err error, traceType string, m Metricer, log log.Logger) {
 		if errors.Is(err, ErrUnexpectedStatusCode) {
-			log.Error("Incorrect status code", "type", traceType, "err", err)
+			log.Error("Incorrect status code", "type", runConfig.Name, "err", err)
 			m.RecordInvalid(traceType)
 		} else if err != nil {
-			log.Error("Failed to run", "type", traceType, "err", err)
+			log.Error("Failed to run", "type", runConfig.Name, "err", err)
 			m.RecordFailure(traceType)
 		} else {
-			log.Info("Successfully verified output root", "type", traceType)
+			log.Info("Successfully verified output root", "type", runConfig.Name)
 			m.RecordSuccess(traceType)
 		}
 	}
 
-	prestateHash, err := r.getPrestateHash(ctx, traceType, caller)
+	prestateHash := runConfig.Prestate
+	if prestateHash == (common.Hash{}) {
+		hash, err := r.getPrestateHash(ctx, runConfig.TraceType, caller)
 		if err != nil {
-		recordError(err, traceType.String(), r.m, r.log)
+			recordError(err, runConfig.Name, r.m, r.log)
 			return
 		}
+		prestateHash = hash
+	}
 
 	localInputs, err := r.createGameInputs(ctx, client)
 	if err != nil {
-		recordError(err, traceType.String(), r.m, r.log)
+		recordError(err, runConfig.Name, r.m, r.log)
 		return
 	}
 
 	inputsLogger := r.log.New("l1", localInputs.L1Head, "l2", localInputs.L2Head, "l2Block", localInputs.L2BlockNumber, "claim", localInputs.L2Claim)
-	var wg sync.WaitGroup
-	wg.Add(1)
-	go func() {
-		defer wg.Done()
-		dir, err := r.prepDatadir(traceType.String())
+	// Sanitize the directory name.
+	safeName := regexp.MustCompile("[^a-zA-Z0-9_-]").ReplaceAllString(runConfig.Name, "")
+	dir, err := r.prepDatadir(safeName)
 	if err != nil {
-			recordError(err, traceType.String(), r.m, r.log)
+		recordError(err, runConfig.Name, r.m, r.log)
 		return
 	}
-		err = r.runOnce(ctx, inputsLogger.With("type", traceType), traceType, prestateHash, localInputs, dir)
-		recordError(err, traceType.String(), r.m, r.log)
-	}()
-
-	if traceType == types.TraceTypeCannon && r.addMTCannonPrestate != (common.Hash{}) && r.addMTCannonPrestateURL != nil {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
-			dir, err := r.prepDatadir(mtCannonType)
-			if err != nil {
-				recordError(err, mtCannonType, r.m, r.log)
-				return
-			}
-			logger := inputsLogger.With("type", mtCannonType)
-			err = r.runMTOnce(ctx, logger, localInputs, dir)
-			recordError(err, mtCannonType, r.m, r.log.With(mtCannonType, true))
-		}()
-	}
-	wg.Wait()
-}
-
-func (r *Runner) runOnce(ctx context.Context, logger log.Logger, traceType types.TraceType, prestateHash common.Hash, localInputs utils.LocalGameInputs, dir string) error {
-	provider, err := createTraceProvider(ctx, logger, metrics.NewVmMetrics(r.m, traceType.String()), r.cfg, prestateHash, traceType, localInputs, dir)
-	if err != nil {
-		return fmt.Errorf("failed to create trace provider: %w", err)
-	}
-	hash, err := provider.Get(ctx, types.RootPosition)
-	if err != nil {
-		return fmt.Errorf("failed to execute trace provider: %w", err)
-	}
-	if hash[0] != mipsevm.VMStatusValid {
-		return fmt.Errorf("%w: %v", ErrUnexpectedStatusCode, hash)
-	}
-	return nil
+	err = r.runOnce(ctx, inputsLogger.With("type", runConfig.Name), runConfig.Name, runConfig.TraceType, prestateHash, localInputs, dir)
+	recordError(err, runConfig.Name, r.m, r.log)
 }
 
-func (r *Runner) runMTOnce(ctx context.Context, logger log.Logger, localInputs utils.LocalGameInputs, dir string) error {
-	provider, err := createMTTraceProvider(ctx, logger, metrics.NewVmMetrics(r.m, mtCannonType), r.cfg.Cannon, r.addMTCannonPrestate, r.addMTCannonPrestateURL, localInputs, dir)
+func (r *Runner) runOnce(ctx context.Context, logger log.Logger, name string, traceType types.TraceType, prestateHash common.Hash, localInputs utils.LocalGameInputs, dir string) error {
+	provider, err := createTraceProvider(ctx, logger, metrics.NewVmMetrics(r.m, name), r.cfg, prestateHash, traceType, localInputs, dir)
 	if err != nil {
 		return fmt.Errorf("failed to create trace provider: %w", err)
 	}
@@ -201,8 +173,8 @@ func (r *Runner) runMTOnce(ctx context.Context, logger log.Logger, localInputs u
 	return nil
 }
 
-func (r *Runner) prepDatadir(traceType string) (string, error) {
-	dir := filepath.Join(r.cfg.Datadir, traceType)
+func (r *Runner) prepDatadir(name string) (string, error) {
+	dir := filepath.Join(r.cfg.Datadir, name)
 	if err := os.RemoveAll(dir); err != nil {
 		return "", fmt.Errorf("failed to remove old dir: %w", err)
 	}