Merge branch 'develop' into cleanup/ctb-remove-glob-dep

.circleci/config.yml

@@ -1634,7 +1634,7 @@ workflows:
           docker_file: ./ops/docker/ci-builder/Dockerfile
           docker_name: ci-builder
           docker_tags: <<pipeline.git.revision>>,latest
-          docker_context: ./ops/docker/ci-builder
+          docker_context: .
           context:
             - oplabs-gcr
           requires:

op-challenger/challenger.go

+package op_challenger
+
+import (
+	"github.com/ethereum-optimism/optimism/op-challenger/config"
+	"github.com/ethereum/go-ethereum/log"
+)
+
+// Main is the programmatic entry-point for running op-challenger
+func Main(logger log.Logger, cfg *config.Config) error {
+	logger.Info("Fault game started")
+	return nil
+}

op-challenger/cmd/main.go

 package main
 
 import (
+	"fmt"
 	"os"
 
+	op_challenger "github.com/ethereum-optimism/optimism/op-challenger"
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/urfave/cli/v2"
 
@@ -33,6 +35,15 @@ var VersionWithMeta = func() string {
 }()
 
 func main() {
+	args := os.Args
+	if err := run(args, op_challenger.Main); err != nil {
+		log.Crit("Application failed", "err", err)
+	}
+}
+
+type ConfigAction func(log log.Logger, config *config.Config) error
+
+func run(args []string, action ConfigAction) error {
 	oplog.SetupDefaults()
 
 	app := cli.NewApp()
@@ -42,24 +53,26 @@ func main() {
 	app.Usage = "Challenge outputs"
 	app.Description = "Ensures that on chain outputs are correct."
 	app.Action = func(ctx *cli.Context) error {
-		return FaultGame(VersionWithMeta, ctx)
-	}
-	if err := app.Run(os.Args); err != nil {
-		log.Crit("Application failed", "message", err)
+		logger, err := setupLogging(ctx)
+		if err != nil {
+			return err
 		}
-}
-
-type ConfigAction func(log log.Logger, version string, config *config.Config) error
+		logger.Info("Starting op-challenger", "version", VersionWithMeta)
 
-func FaultGame(version string, cliCtx *cli.Context) error {
-	cfg, err := config.NewConfigFromCLI(cliCtx)
+		cfg, err := config.NewConfigFromCLI(ctx)
 		if err != nil {
 			return err
 		}
-	if err := cfg.Check(); err != nil {
-		return err
+		return action(logger, cfg)
+	}
+	return app.Run(args)
+}
+
+func setupLogging(ctx *cli.Context) (log.Logger, error) {
+	logCfg := oplog.ReadCLIConfig(ctx)
+	if err := logCfg.Check(); err != nil {
+		return nil, fmt.Errorf("log config error: %w", err)
 	}
-	log := oplog.NewLogger(cfg.LogConfig)
-	log.Info("Fault game started")
-	return nil
+	logger := oplog.NewLogger(logCfg)
+	return logger, nil
 }

op-challenger/cmd/main_test.go

+package main
+
+import (
+	"testing"
+
+	"github.com/ethereum-optimism/optimism/op-challenger/config"
+	"github.com/ethereum-optimism/optimism/op-service/txmgr"
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/log"
+	"github.com/stretchr/testify/require"
+)
+
+var (
+	l1EthRpc         = "http://example.com:8545"
+	gameAddressValue = "0xaa00000000000000000000000000000000000000"
+	alphabetTrace    = "abcdefghijz"
+)
+
+func TestLogLevel(t *testing.T) {
+	t.Run("RejectInvalid", func(t *testing.T) {
+		verifyArgsInvalid(t, "unknown level: foo", addRequiredArgs("--log.level=foo"))
+	})
+
+	for _, lvl := range []string{"trace", "debug", "info", "error", "crit"} {
+		lvl := lvl
+		t.Run("AcceptValid_"+lvl, func(t *testing.T) {
+			logger, _, err := runWithArgs(addRequiredArgs("--log.level", lvl))
+			require.NoError(t, err)
+			require.NotNil(t, logger)
+		})
+	}
+}
+
+func TestDefaultCLIOptionsMatchDefaultConfig(t *testing.T) {
+	cfg := configForArgs(t, addRequiredArgs())
+	defaultCfg := config.NewConfig(l1EthRpc, common.HexToAddress(gameAddressValue), alphabetTrace)
+	require.Equal(t, defaultCfg, cfg)
+}
+
+func TestDefaultConfigIsValid(t *testing.T) {
+	cfg := config.NewConfig(l1EthRpc, common.HexToAddress(gameAddressValue), alphabetTrace)
+	require.NoError(t, cfg.Check())
+}
+
+func TestL1ETHRPCAddress(t *testing.T) {
+	t.Run("Required", func(t *testing.T) {
+		verifyArgsInvalid(t, "flag l1-eth-rpc is required", addRequiredArgsExcept("--l1-eth-rpc"))
+	})
+
+	t.Run("Valid", func(t *testing.T) {
+		url := "http://example.com:8888"
+		cfg := configForArgs(t, addRequiredArgsExcept("--l1-eth-rpc", "--l1-eth-rpc="+url))
+		require.Equal(t, url, cfg.L1EthRpc)
+		require.Equal(t, url, cfg.TxMgrConfig.L1RPCURL)
+	})
+}
+
+func TestAlphabetTrace(t *testing.T) {
+	t.Run("Required", func(t *testing.T) {
+		verifyArgsInvalid(t, "flag alphabet is required", addRequiredArgsExcept("--alphabet"))
+	})
+
+	t.Run("Valid", func(t *testing.T) {
+		value := "abcde"
+		cfg := configForArgs(t, addRequiredArgsExcept("--alphabet", "--alphabet="+value))
+		require.Equal(t, value, cfg.AlphabetTrace)
+	})
+}
+
+func TestGameAddress(t *testing.T) {
+	t.Run("Required", func(t *testing.T) {
+		verifyArgsInvalid(t, "flag game-address is required", addRequiredArgsExcept("--game-address"))
+	})
+
+	t.Run("Valid", func(t *testing.T) {
+		addr := common.Address{0xbb, 0xcc, 0xdd}
+		cfg := configForArgs(t, addRequiredArgsExcept("--game-address", "--game-address="+addr.Hex()))
+		require.Equal(t, addr, cfg.GameAddress)
+	})
+
+	t.Run("Invalid", func(t *testing.T) {
+		verifyArgsInvalid(t, "invalid address: foo", addRequiredArgsExcept("--game-address", "--game-address=foo"))
+	})
+}
+
+func TestTxManagerFlagsSupported(t *testing.T) {
+	// Not a comprehensive list of flags, just enough to sanity check the txmgr.CLIFlags were defined
+	cfg := configForArgs(t, addRequiredArgs("--"+txmgr.NumConfirmationsFlagName, "7"))
+	require.Equal(t, uint64(7), cfg.TxMgrConfig.NumConfirmations)
+}
+
+func verifyArgsInvalid(t *testing.T, messageContains string, cliArgs []string) {
+	_, _, err := runWithArgs(cliArgs)
+	require.ErrorContains(t, err, messageContains)
+}
+
+func configForArgs(t *testing.T, cliArgs []string) config.Config {
+	_, cfg, err := runWithArgs(cliArgs)
+	require.NoError(t, err)
+	return cfg
+}
+
+func runWithArgs(cliArgs []string) (log.Logger, config.Config, error) {
+	cfg := new(config.Config)
+	var logger log.Logger
+	fullArgs := append([]string{"op-program"}, cliArgs...)
+	err := run(fullArgs, func(log log.Logger, config *config.Config) error {
+		logger = log
+		cfg = config
+		return nil
+	})
+	return logger, *cfg, err
+}
+
+func addRequiredArgs(args ...string) []string {
+	req := requiredArgs()
+	combined := toArgList(req)
+	return append(combined, args...)
+}
+
+func addRequiredArgsExcept(name string, optionalArgs ...string) []string {
+	req := requiredArgs()
+	delete(req, name)
+	return append(toArgList(req), optionalArgs...)
+}
+
+func requiredArgs() map[string]string {
+	return map[string]string{
+		"--l1-eth-rpc":   l1EthRpc,
+		"--game-address": gameAddressValue,
+		"--alphabet":     alphabetTrace,
+	}
+}
+
+func toArgList(req map[string]string) []string {
+	var combined []string
+	for name, value := range req {
+		combined = append(combined, name)
+		combined = append(combined, value)
+	}
+	return combined
+}

op-challenger/config/config.go

@@ -8,10 +8,6 @@ import (
 
 	"github.com/ethereum-optimism/optimism/op-challenger/flags"
 	opservice "github.com/ethereum-optimism/optimism/op-service"
-	oplog "github.com/ethereum-optimism/optimism/op-service/log"
-	opmetrics "github.com/ethereum-optimism/optimism/op-service/metrics"
-	oppprof "github.com/ethereum-optimism/optimism/op-service/pprof"
-	oprpc "github.com/ethereum-optimism/optimism/op-service/rpc"
 	"github.com/ethereum-optimism/optimism/op-service/txmgr"
 )
 
@@ -30,30 +26,17 @@ type Config struct {
 	AlphabetTrace string         // String for the AlphabetTraceProvider
 
 	TxMgrConfig txmgr.CLIConfig
-	RPCConfig     oprpc.CLIConfig
-	LogConfig     oplog.CLIConfig
-	MetricsConfig opmetrics.CLIConfig
-	PprofConfig   oppprof.CLIConfig
 }
 
-func NewConfig(L1EthRpc string,
+func NewConfig(l1EthRpc string,
 	GameAddress common.Address,
 	AlphabetTrace string,
-	TxMgrConfig txmgr.CLIConfig,
-	RPCConfig oprpc.CLIConfig,
-	LogConfig oplog.CLIConfig,
-	MetricsConfig opmetrics.CLIConfig,
-	PprofConfig oppprof.CLIConfig,
 ) Config {
 	return Config{
-		L1EthRpc,
-		GameAddress,
-		AlphabetTrace,
-		TxMgrConfig,
-		RPCConfig,
-		LogConfig,
-		MetricsConfig,
-		PprofConfig,
+		L1EthRpc:      l1EthRpc,
+		GameAddress:   GameAddress,
+		AlphabetTrace: AlphabetTrace,
+		TxMgrConfig:   txmgr.NewCLIConfig(l1EthRpc),
 	}
 }
 
@@ -67,18 +50,6 @@ func (c Config) Check() error {
 	if c.AlphabetTrace == "" {
 		return ErrMissingAlphabetTrace
 	}
-	if err := c.RPCConfig.Check(); err != nil {
-		return err
-	}
-	if err := c.LogConfig.Check(); err != nil {
-		return err
-	}
-	if err := c.MetricsConfig.Check(); err != nil {
-		return err
-	}
-	if err := c.PprofConfig.Check(); err != nil {
-		return err
-	}
 	if err := c.TxMgrConfig.Check(); err != nil {
 		return err
 	}
@@ -96,10 +67,6 @@ func NewConfigFromCLI(ctx *cli.Context) (*Config, error) {
 	}
 
 	txMgrConfig := txmgr.ReadCLIConfig(ctx)
-	rpcConfig := oprpc.ReadCLIConfig(ctx)
-	logConfig := oplog.ReadCLIConfig(ctx)
-	metricsConfig := opmetrics.ReadCLIConfig(ctx)
-	pprofConfig := oppprof.ReadCLIConfig(ctx)
 
 	return &Config{
 		// Required Flags
@@ -107,10 +74,5 @@ func NewConfigFromCLI(ctx *cli.Context) (*Config, error) {
 		GameAddress:   dgfAddress,
 		AlphabetTrace: ctx.String(flags.AlphabetFlag.Name),
 		TxMgrConfig:   txMgrConfig,
-		// Optional Flags
-		RPCConfig:     rpcConfig,
-		LogConfig:     logConfig,
-		MetricsConfig: metricsConfig,
-		PprofConfig:   pprofConfig,
 	}, nil
 }

op-challenger/config/config_test.go

@@ -2,15 +2,8 @@ package config
 
 import (
 	"testing"
-	"time"
 
-	oplog "github.com/ethereum-optimism/optimism/op-service/log"
-	opmetrics "github.com/ethereum-optimism/optimism/op-service/metrics"
-	oppprof "github.com/ethereum-optimism/optimism/op-service/pprof"
-	oprpc "github.com/ethereum-optimism/optimism/op-service/rpc"
 	"github.com/ethereum-optimism/optimism/op-service/txmgr"
-	"github.com/ethereum-optimism/optimism/op-signer/client"
-
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/stretchr/testify/require"
 )
@@ -18,49 +11,11 @@ import (
 var (
 	validL1EthRpc      = "http://localhost:8545"
 	validGameAddress   = common.HexToAddress("0x7bdd3b028C4796eF0EAf07d11394d0d9d8c24139")
-	validNetworkTimeout = time.Duration(5) * time.Second
 	validAlphabetTrace = "abcdefgh"
 )
 
-var validTxMgrConfig = txmgr.CLIConfig{
-	L1RPCURL:                  validL1EthRpc,
-	NumConfirmations:          10,
-	NetworkTimeout:            validNetworkTimeout,
-	ResubmissionTimeout:       time.Duration(5) * time.Second,
-	ReceiptQueryInterval:      time.Duration(5) * time.Second,
-	TxNotInMempoolTimeout:     time.Duration(5) * time.Second,
-	SafeAbortNonceTooLowCount: 10,
-	SignerCLIConfig: client.CLIConfig{
-		Endpoint: "http://localhost:8547",
-		// First address for the default hardhat mnemonic
-		Address: "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266",
-	},
-}
-
-var validRPCConfig = oprpc.CLIConfig{
-	ListenAddr: "localhost:8547",
-	ListenPort: 8547,
-}
-
-var validLogConfig = oplog.DefaultCLIConfig()
-
-var validMetricsConfig = opmetrics.CLIConfig{
-	Enabled: false,
-}
-
-var validPprofConfig = oppprof.CLIConfig{
-	Enabled: false,
-}
-
 func validConfig() Config {
-	cfg := NewConfig(validL1EthRpc,
-		validGameAddress,
-		validAlphabetTrace,
-		validTxMgrConfig,
-		validRPCConfig,
-		validLogConfig,
-		validMetricsConfig,
-		validPprofConfig)
+	cfg := NewConfig(validL1EthRpc, validGameAddress, validAlphabetTrace)
 	return cfg
 }
 

op-challenger/flags/flags.go

@@ -7,9 +7,6 @@ import (
 
 	opservice "github.com/ethereum-optimism/optimism/op-service"
 	oplog "github.com/ethereum-optimism/optimism/op-service/log"
-	opmetrics "github.com/ethereum-optimism/optimism/op-service/metrics"
-	oppprof "github.com/ethereum-optimism/optimism/op-service/pprof"
-	oprpc "github.com/ethereum-optimism/optimism/op-service/rpc"
 	txmgr "github.com/ethereum-optimism/optimism/op-service/txmgr"
 )
 
@@ -50,10 +47,7 @@ var requiredFlags = []cli.Flag{
 var optionalFlags = []cli.Flag{}
 
 func init() {
-	optionalFlags = append(optionalFlags, oprpc.CLIFlags(envVarPrefix)...)
 	optionalFlags = append(optionalFlags, oplog.CLIFlags(envVarPrefix)...)
-	optionalFlags = append(optionalFlags, opmetrics.CLIFlags(envVarPrefix)...)
-	optionalFlags = append(optionalFlags, oppprof.CLIFlags(envVarPrefix)...)
 	optionalFlags = append(optionalFlags, txmgr.CLIFlags(envVarPrefix)...)
 
 	Flags = append(requiredFlags, optionalFlags...)

op-program/client/driver/driver.go

@@ -61,6 +61,11 @@ func (d *Driver) Step(ctx context.Context) error {
 		}
 		d.logger.Debug("Data is lacking")
 		return nil
+	} else if errors.Is(err, derive.ErrTemporary) {
+		// While most temporary errors are due to requests for external data failing which can't happen,
+		// they may also be returned due to other events like channels timing out so need to be handled
+		d.logger.Warn("Temporary error in derivation", "err", err)
+		return nil
 	} else if err != nil {
 		return fmt.Errorf("pipeline err: %w", err)
 	}

op-program/client/driver/driver_test.go

@@ -23,7 +23,7 @@ func TestDerivationComplete(t *testing.T) {
 func TestTemporaryError(t *testing.T) {
 	driver := createDriver(t, fmt.Errorf("whoopsie: %w", derive.ErrTemporary))
 	err := driver.Step(context.Background())
-	require.ErrorIs(t, err, derive.ErrTemporary)
+	require.NoError(t, err, "should allow derivation to continue after temporary error")
 }
 
 func TestNotEnoughDataError(t *testing.T) {

op-service/tls/cli.go

@@ -23,6 +23,12 @@ func CLIFlags(envPrefix string) []cli.Flag {
 	return CLIFlagsWithFlagPrefix(envPrefix, "")
 }
 
+var (
+	defaultTLSCaCert = "tls/ca.crt"
+	defaultTLSCert   = "tls/tls.crt"
+	defaultTLSKey    = "tls/tls.key"
+)
+
 // CLIFlagsWithFlagPrefix returns flags with env var and cli flag prefixes
 // Should be used for client TLS configs when different from server on the same process
 func CLIFlagsWithFlagPrefix(envPrefix string, flagPrefix string) []cli.Flag {
@@ -36,19 +42,19 @@ func CLIFlagsWithFlagPrefix(envPrefix string, flagPrefix string) []cli.Flag {
 		&cli.StringFlag{
 			Name:    prefixFunc(TLSCaCertFlagName),
 			Usage:   "tls ca cert path",
-			Value:   "tls/ca.crt",
+			Value:   defaultTLSCaCert,
 			EnvVars: prefixEnvVars("TLS_CA"),
 		},
 		&cli.StringFlag{
 			Name:    prefixFunc(TLSCertFlagName),
 			Usage:   "tls cert path",
-			Value:   "tls/tls.crt",
+			Value:   defaultTLSCert,
 			EnvVars: prefixEnvVars("TLS_CERT"),
 		},
 		&cli.StringFlag{
 			Name:    prefixFunc(TLSKeyFlagName),
 			Usage:   "tls key",
-			Value:   "tls/tls.key",
+			Value:   defaultTLSKey,
 			EnvVars: prefixEnvVars("TLS_KEY"),
 		},
 	}
@@ -60,6 +66,14 @@ type CLIConfig struct {
 	TLSKey    string
 }
 
+func NewCLIConfig() CLIConfig {
+	return CLIConfig{
+		TLSCaCert: defaultTLSCaCert,
+		TLSCert:   defaultTLSCert,
+		TLSKey:    defaultTLSKey,
+	}
+}
+
 func (c CLIConfig) Check() error {
 	if c.TLSEnabled() && (c.TLSCaCert == "" || c.TLSCert == "" || c.TLSKey == "") {
 		return errors.New("all tls flags must be set if at least one is set")

op-service/tls/cli_test.go

+package tls
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"github.com/urfave/cli/v2"
+)
+
+func TestDefaultCLIOptionsMatchDefaultConfig(t *testing.T) {
+	cfg := configForArgs()
+	defaultCfg := NewCLIConfig()
+	require.Equal(t, defaultCfg, cfg)
+}
+
+func TestDefaultConfigIsValid(t *testing.T) {
+	err := NewCLIConfig().Check()
+	require.NoError(t, err)
+}
+
+func TestInvalidConfig(t *testing.T) {
+	tests := []struct {
+		name         string
+		configChange func(config *CLIConfig)
+	}{
+		{"MissingCaCert", func(config *CLIConfig) {
+			config.TLSCaCert = ""
+		}},
+		{"MissingCert", func(config *CLIConfig) {
+			config.TLSCert = ""
+		}},
+		{"MissingKey", func(config *CLIConfig) {
+			config.TLSKey = ""
+		}},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			cfg := NewCLIConfig()
+			test.configChange(&cfg)
+			err := cfg.Check()
+			require.ErrorContains(t, err, "all tls flags must be set if at least one is set")
+		})
+	}
+}
+
+func configForArgs(args ...string) CLIConfig {
+	app := cli.NewApp()
+	app.Flags = CLIFlagsWithFlagPrefix("TEST_", "test")
+	app.Name = "test"
+	var config CLIConfig
+	app.Action = func(ctx *cli.Context) error {
+		config = ReadCLIConfigWithPrefix(ctx, "test")
+		return nil
+	}
+	_ = app.Run(args)
+	return config
+}

op-service/txmgr/cli.go

@@ -48,6 +48,16 @@ var (
 	}
 )
 
+var (
+	defaultNumConfirmations          = uint64(10)
+	defaultSafeAbortNonceTooLowCount = uint64(3)
+	defaultResubmissionTimeout       = 48 * time.Second
+	defaultNetworkTimeout            = 2 * time.Second
+	defaultTxSendTimeout             = 0 * time.Second
+	defaultTxNotInMempoolTimeout     = 2 * time.Minute
+	defaultReceiptQueryInterval      = 12 * time.Second
+)
+
 func CLIFlags(envPrefix string) []cli.Flag {
 	prefixEnvVars := func(name string) []string {
 		return opservice.PrefixEnvVar(envPrefix, name)
@@ -71,43 +81,43 @@ func CLIFlags(envPrefix string) []cli.Flag {
 		&cli.Uint64Flag{
 			Name:    NumConfirmationsFlagName,
 			Usage:   "Number of confirmations which we will wait after sending a transaction",
-			Value:   10,
+			Value:   defaultNumConfirmations,
 			EnvVars: prefixEnvVars("NUM_CONFIRMATIONS"),
 		},
 		&cli.Uint64Flag{
 			Name:    SafeAbortNonceTooLowCountFlagName,
 			Usage:   "Number of ErrNonceTooLow observations required to give up on a tx at a particular nonce without receiving confirmation",
-			Value:   3,
+			Value:   defaultSafeAbortNonceTooLowCount,
 			EnvVars: prefixEnvVars("SAFE_ABORT_NONCE_TOO_LOW_COUNT"),
 		},
 		&cli.DurationFlag{
 			Name:    ResubmissionTimeoutFlagName,
 			Usage:   "Duration we will wait before resubmitting a transaction to L1",
-			Value:   48 * time.Second,
+			Value:   defaultResubmissionTimeout,
 			EnvVars: prefixEnvVars("RESUBMISSION_TIMEOUT"),
 		},
 		&cli.DurationFlag{
 			Name:    NetworkTimeoutFlagName,
 			Usage:   "Timeout for all network operations",
-			Value:   2 * time.Second,
+			Value:   defaultNetworkTimeout,
 			EnvVars: prefixEnvVars("NETWORK_TIMEOUT"),
 		},
 		&cli.DurationFlag{
 			Name:    TxSendTimeoutFlagName,
 			Usage:   "Timeout for sending transactions. If 0 it is disabled.",
-			Value:   0,
+			Value:   defaultTxSendTimeout,
 			EnvVars: prefixEnvVars("TXMGR_TX_SEND_TIMEOUT"),
 		},
 		&cli.DurationFlag{
 			Name:    TxNotInMempoolTimeoutFlagName,
 			Usage:   "Timeout for aborting a tx send if the tx does not make it to the mempool.",
-			Value:   2 * time.Minute,
+			Value:   defaultTxNotInMempoolTimeout,
 			EnvVars: prefixEnvVars("TXMGR_TX_NOT_IN_MEMPOOL_TIMEOUT"),
 		},
 		&cli.DurationFlag{
 			Name:    ReceiptQueryIntervalFlagName,
 			Usage:   "Frequency to poll for receipts",
-			Value:   12 * time.Second,
+			Value:   defaultReceiptQueryInterval,
 			EnvVars: prefixEnvVars("TXMGR_RECEIPT_QUERY_INTERVAL"),
 		},
 	}, client.CLIFlags(envPrefix)...)
@@ -130,6 +140,20 @@ type CLIConfig struct {
 	TxNotInMempoolTimeout     time.Duration
 }
 
+func NewCLIConfig(l1RPCURL string) CLIConfig {
+	return CLIConfig{
+		L1RPCURL:                  l1RPCURL,
+		NumConfirmations:          defaultNumConfirmations,
+		SafeAbortNonceTooLowCount: defaultSafeAbortNonceTooLowCount,
+		ResubmissionTimeout:       defaultResubmissionTimeout,
+		NetworkTimeout:            defaultNetworkTimeout,
+		TxSendTimeout:             defaultTxSendTimeout,
+		TxNotInMempoolTimeout:     defaultTxNotInMempoolTimeout,
+		ReceiptQueryInterval:      defaultReceiptQueryInterval,
+		SignerCLIConfig:           client.NewCLIConfig(),
+	}
+}
+
 func (m CLIConfig) Check() error {
 	if m.L1RPCURL == "" {
 		return errors.New("must provide a L1 RPC url")

op-service/txmgr/cli_test.go

+package txmgr
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"github.com/urfave/cli/v2"
+)
+
+var (
+	l1EthRpcValue = "http://localhost:9546"
+)
+
+func TestDefaultCLIOptionsMatchDefaultConfig(t *testing.T) {
+	cfg := configForArgs()
+	defaultCfg := NewCLIConfig(l1EthRpcValue)
+	require.Equal(t, defaultCfg, cfg)
+}
+
+func TestDefaultConfigIsValid(t *testing.T) {
+	cfg := NewCLIConfig(l1EthRpcValue)
+	require.NoError(t, cfg.Check())
+}
+
+func configForArgs(args ...string) CLIConfig {
+	app := cli.NewApp()
+	// txmgr expects the --l1-eth-rpc option to be declared externally
+	flags := append(CLIFlags("TEST_"), &cli.StringFlag{
+		Name:  L1RPCFlagName,
+		Value: l1EthRpcValue,
+	})
+	app.Flags = flags
+	app.Name = "test"
+	var config CLIConfig
+	app.Action = func(ctx *cli.Context) error {
+		config = ReadCLIConfig(ctx)
+		return nil
+	}
+	_ = app.Run(args)
+	return config
+}

op-signer/client/config.go

@@ -38,6 +38,12 @@ type CLIConfig struct {
 	TLSConfig optls.CLIConfig
 }
 
+func NewCLIConfig() CLIConfig {
+	return CLIConfig{
+		TLSConfig: optls.NewCLIConfig(),
+	}
+}
+
 func (c CLIConfig) Check() error {
 	if err := c.TLSConfig.Check(); err != nil {
 		return err

op-signer/client/config_test.go

+package client
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"github.com/urfave/cli/v2"
+)
+
+func TestDefaultCLIOptionsMatchDefaultConfig(t *testing.T) {
+	cfg := configForArgs()
+	defaultCfg := NewCLIConfig()
+	require.Equal(t, defaultCfg, cfg)
+}
+
+func TestDefaultConfigIsValid(t *testing.T) {
+	err := NewCLIConfig().Check()
+	require.NoError(t, err)
+}
+
+func TestInvalidConfig(t *testing.T) {
+	tests := []struct {
+		name         string
+		expected     string
+		configChange func(config *CLIConfig)
+	}{
+		{
+			name:     "MissingEndpoint",
+			expected: "signer endpoint and address must both be set or not set",
+			configChange: func(config *CLIConfig) {
+				config.Address = "0x1234"
+			},
+		},
+		{
+			name:     "MissingAddress",
+			expected: "signer endpoint and address must both be set or not set",
+			configChange: func(config *CLIConfig) {
+				config.Endpoint = "http://localhost"
+			},
+		},
+		{
+			name:     "InvalidTLSConfig",
+			expected: "all tls flags must be set if at least one is set",
+			configChange: func(config *CLIConfig) {
+				config.TLSConfig.TLSKey = ""
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			cfg := NewCLIConfig()
+			test.configChange(&cfg)
+			err := cfg.Check()
+			require.ErrorContains(t, err, test.expected)
+		})
+	}
+}
+
+func configForArgs(args ...string) CLIConfig {
+	app := cli.NewApp()
+	app.Flags = CLIFlags("TEST_")
+	app.Name = "test"
+	var config CLIConfig
+	app.Action = func(ctx *cli.Context) error {
+		config = ReadCLIConfig(ctx)
+		return nil
+	}
+	_ = app.Run(args)
+	return config
+}

ops/docker/ci-builder/Dockerfile

@@ -11,14 +11,14 @@ RUN apt-get update && \
     chmod +x ./rustup.sh && \
     ./rustup.sh -y
 
-# move the foundryrc file to the foundry dir
-WORKDIR /opt/foundry
-COPY ../../.foundryrc ./.foundryrc
+COPY ./.foundryrc ./.foundryrc
 
 # Only diff from upstream docker image is this clone instead
 # of COPY. We select a specific commit to use.
-RUN git clone https://github.com/foundry-rs/foundry.git . \
-  && git checkout $(cat .foundryrc)
+RUN git clone https://github.com/foundry-rs/foundry.git ./foundry \
+  && cd foundry && git checkout $(cat ../.foundryrc)
+
+WORKDIR /opt/foundry
 
 RUN source $HOME/.profile && \
     cargo build --release && \
@@ -54,14 +54,9 @@ RUN apt-get update && \
   pip install slither-analyzer==0.9.3 && \
   go install gotest.tools/gotestsum@latest && \
   curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.48.0 && \
-  curl -fLSs https://raw.githubusercontent.com/CircleCI-Public/circleci-cli/master/install.sh | bash &&
-  chmox +x /usr/local/bin/check-changed
-
-RUN echo "downloading pnpm" && \
-  curl -o /pnpm-install.sh -L https://get.pnpm.io/install.sh && \
-  chmod +x /pnpm-install.sh && \
-  /pnpm-install.sh && \
-  rm /pnpm-install.sh
+  curl -fLSs https://raw.githubusercontent.com/CircleCI-Public/circleci-cli/master/install.sh | bash
+
+RUN echo "downloading pnpm" && npm i -g pnpm
 
 RUN echo "downloading solidity compilers" && \
   curl -o solc-linux-amd64-v0.5.17+commit.d19bba13 -sL https://binaries.soliditylang.org/linux-amd64/solc-linux-amd64-v0.5.17+commit.d19bba13 && \

packages/contracts-bedrock/.gas-snapshot

@@ -32,12 +32,12 @@ DisputeGameFactory_SetImplementation_Test:test_setImplementation_notOwner_revert
 DisputeGameFactory_SetImplementation_Test:test_setImplementation_succeeds() (gas: 44243)
 DisputeGameFactory_TransferOwnership_Test:test_transferOwnership_notOwner_reverts() (gas: 15950)
 DisputeGameFactory_TransferOwnership_Test:test_transferOwnership_succeeds() (gas: 18642)
-FaultDisputeGame_ResolvesCorrectly_CorrectRoot2:test_resolvesCorrectly_succeeds() (gas: 502174)
-FaultDisputeGame_ResolvesCorrectly_CorrectRoot3:test_resolvesCorrectly_succeeds() (gas: 504053)
-FaultDisputeGame_ResolvesCorrectly_CorrectRoot:test_resolvesCorrectly_succeeds() (gas: 491517)
-FaultDisputeGame_ResolvesCorrectly_IncorrectRoot2:test_resolvesCorrectly_succeeds() (gas: 500937)
-FaultDisputeGame_ResolvesCorrectly_IncorrectRoot3:test_resolvesCorrectly_succeeds() (gas: 502816)
-FaultDisputeGame_ResolvesCorrectly_IncorrectRoot:test_resolvesCorrectly_succeeds() (gas: 490280)
+FaultDisputeGame_ResolvesCorrectly_CorrectRoot1:test_resolvesCorrectly_succeeds() (gas: 491839)
+FaultDisputeGame_ResolvesCorrectly_CorrectRoot2:test_resolvesCorrectly_succeeds() (gas: 495751)
+FaultDisputeGame_ResolvesCorrectly_CorrectRoot3:test_resolvesCorrectly_succeeds() (gas: 495049)
+FaultDisputeGame_ResolvesCorrectly_IncorrectRoot1:test_resolvesCorrectly_succeeds() (gas: 490600)
+FaultDisputeGame_ResolvesCorrectly_IncorrectRoot2:test_resolvesCorrectly_succeeds() (gas: 494512)
+FaultDisputeGame_ResolvesCorrectly_IncorrectRoot3:test_resolvesCorrectly_succeeds() (gas: 493810)
 FaultDisputeGame_Test:test_extraData_succeeds() (gas: 17426)
 FaultDisputeGame_Test:test_gameData_succeeds() (gas: 17917)
 FaultDisputeGame_Test:test_gameStart_succeeds() (gas: 10315)
@@ -51,12 +51,12 @@ FaultDisputeGame_Test:test_move_gameDepthExceeded_reverts() (gas: 408100)
 FaultDisputeGame_Test:test_move_gameNotInProgress_reverts() (gas: 10968)
 FaultDisputeGame_Test:test_move_nonExistentParent_reverts() (gas: 24655)
 FaultDisputeGame_Test:test_move_simpleAttack_succeeds() (gas: 107344)
-FaultDisputeGame_Test:test_resolve_challengeContested_succeeds() (gas: 224789)
+FaultDisputeGame_Test:test_resolve_challengeContested_succeeds() (gas: 224784)
 FaultDisputeGame_Test:test_resolve_notInProgress_reverts() (gas: 9657)
-FaultDisputeGame_Test:test_resolve_rootContested_succeeds() (gas: 109754)
+FaultDisputeGame_Test:test_resolve_rootContested_succeeds() (gas: 109749)
 FaultDisputeGame_Test:test_resolve_rootUncontestedClockNotExpired_succeeds() (gas: 21422)
-FaultDisputeGame_Test:test_resolve_rootUncontested_succeeds() (gas: 27256)
-FaultDisputeGame_Test:test_resolve_teamDeathmatch_succeeds() (gas: 395447)
+FaultDisputeGame_Test:test_resolve_rootUncontested_succeeds() (gas: 27251)
+FaultDisputeGame_Test:test_resolve_teamDeathmatch_succeeds() (gas: 395442)
 FaultDisputeGame_Test:test_rootClaim_succeeds() (gas: 8181)
 FeeVault_Test:test_constructor_succeeds() (gas: 18185)
 GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 352113)

packages/contracts-bedrock/contracts/dispute/interfaces/IFaultDisputeGame.sol

@@ -20,32 +20,33 @@ interface IFaultDisputeGame is IDisputeGame {
 
     /// @notice Emitted when a new claim is added to the DAG by `claimant`
     /// @param parentIndex The index within the `claimData` array of the parent claim
-    /// @param pivot The claim being added
+    /// @param claim The claim being added
     /// @param claimant The address of the claimant
-    event Move(uint256 indexed parentIndex, Claim indexed pivot, address indexed claimant);
+    event Move(uint256 indexed parentIndex, Claim indexed claim, address indexed claimant);
 
     /// @notice Attack a disagreed upon `Claim`.
     /// @param _parentIndex Index of the `Claim` to attack in `claimData`.
-    /// @param _pivot The `Claim` at the relative attack position.
-    function attack(uint256 _parentIndex, Claim _pivot) external payable;
+    /// @param _claim The `Claim` at the relative attack position.
+    function attack(uint256 _parentIndex, Claim _claim) external payable;
 
     /// @notice Defend an agreed upon `Claim`.
     /// @param _parentIndex Index of the claim to defend in `claimData`.
-    /// @param _pivot The `Claim` at the relative defense position.
-    function defend(uint256 _parentIndex, Claim _pivot) external payable;
+    /// @param _claim The `Claim` at the relative defense position.
+    function defend(uint256 _parentIndex, Claim _claim) external payable;
 
     /// @notice Perform the final step via an on-chain fault proof processor
     /// @dev This function should point to a fault proof processor in order to execute
     ///      a step in the fault proof program on-chain. The interface of the fault proof
     ///      processor contract should be generic enough such that we can use different
     ///      fault proof VMs (MIPS, RiscV5, etc.)
-    /// @param _stateIndex The index of the pre/post state of the step within `claimData`.
     /// @param _claimIndex The index of the challenged claim within `claimData`.
     /// @param _isAttack Whether or not the step is an attack or a defense.
-    /// @param _stateData The stateData of the step is the preimage of the claim @ `prestateIndex`
+    /// @param _stateData The stateData of the step is the preimage of the claim at the given
+    ///        prestate, which is at `_stateIndex` if the move is an attack and `_claimIndex` if
+    ///        the move is a defense. If the step is an attack on the first instruction, it is
+    ///        the absolute prestate of the fault proof VM.
     /// @param _proof Proof to access memory leaf nodes in the VM.
     function step(
-        uint256 _stateIndex,
         uint256 _claimIndex,
         bool _isAttack,
         bytes calldata _stateData,

packages/contracts-bedrock/contracts/dispute/lib/LibPosition.sol

@@ -124,6 +124,27 @@ library LibPosition {
         }
     }
 
+    /// @notice Gets the position of the highest ancestor of `_position` that commits to the same
+    ///         trace index.
+    /// @param _position The position to get the highest ancestor of.
+    /// @return ancestor_ The highest ancestor of `position` that commits to the same trace index.
+    function traceAncestor(Position _position) internal pure returns (Position ancestor_) {
+        // Create a field with only the lowest unset bit of `_position` set.
+        Position lsb;
+        assembly {
+            lsb := and(not(_position), add(_position, 1))
+        }
+        // Find the index of the lowest unset bit within the field.
+        uint256 msb = depth(lsb);
+        // The highest ancestor that commits to the same trace index is the original position
+        // shifted right by the index of the lowest unset bit.
+        assembly {
+            let a := shr(msb, _position)
+            // Bound the ancestor to the minimum gindex, 1.
+            ancestor_ := or(a, iszero(a))
+        }
+    }
+
     /// @notice Get the move position of `_position`, which is the left child of:
     ///         1. `_position + 1` if `_isAttack` is true.
     ///         1. `_position` if `_isAttack` is false.

packages/contracts-bedrock/contracts/test/FaultDisputeGame.t.sol

@@ -424,7 +424,6 @@ contract GamePlayer {
 
         // If we are past the maximum depth, break the recursion and step.
         if (movePos.depth() > maxDepth) {
-            uint256 stateIndex;
             bytes memory preStateTrace;
 
             // First, we need to find the pre/post state index depending on whether we
@@ -435,27 +434,7 @@ contract GamePlayer {
                 Position leafPos = isAttack
                     ? Position.wrap(Position.unwrap(parentPos) - 1)
                     : Position.wrap(Position.unwrap(parentPos) + 1);
-                Position statePos = leafPos;
-
-                // Walk up until the valid position that commits to the prestate's
-                // trace index is found.
-                while (
-                    Position.unwrap(statePos.parent().rightIndex(maxDepth)) ==
-                    Position.unwrap(leafPos)
-                ) {
-                    statePos = statePos.parent();
-                }
-
-                // Now, search for the index of the claim that commits to the prestate's trace
-                // index.
-                uint256 len = gameProxy.claimDataLen();
-                for (uint256 i = 0; i < len; i++) {
-                    (, , , Position pos, ) = gameProxy.claimData(i);
-                    if (Position.unwrap(pos) == Position.unwrap(statePos)) {
-                        stateIndex = i;
-                        break;
-                    }
-                }
+                Position statePos = leafPos.traceAncestor();
 
                 // Grab the trace up to the prestate's trace index.
                 if (isAttack) {
@@ -463,10 +442,12 @@ contract GamePlayer {
                 } else {
                     preStateTrace = abi.encode(parentPos.traceIndex(maxDepth), traceAt(parentPos));
                 }
+            } else {
+                preStateTrace = abi.encode(15);
             }
 
             // Perform the step and halt recursion.
-            try gameProxy.step(stateIndex, _parentIndex, isAttack, preStateTrace, hex"") {
+            try gameProxy.step(_parentIndex, isAttack, preStateTrace, hex"") {
                 // Do nothing, step succeeded.
             } catch {
                 failedToStep = true;
@@ -523,7 +504,9 @@ contract GamePlayer {
 
 contract OneVsOne_Arena is FaultDisputeGame_Init {
     /// @dev The absolute prestate of the trace.
-    Claim internal constant ABSOLUTE_PRESTATE = Claim.wrap(bytes32(uint256(15)));
+    bytes ABSOLUTE_PRESTATE = abi.encode(15);
+    /// @dev The absolute prestate claim.
+    Claim internal constant ABSOLUTE_PRESTATE_CLAIM = Claim.wrap(keccak256(abi.encode(15)));
     /// @dev The defender.
     GamePlayer internal defender;
     /// @dev The challenger.
@@ -531,7 +514,7 @@ contract OneVsOne_Arena is FaultDisputeGame_Init {
 
     function init(GamePlayer _defender, GamePlayer _challenger) public {
         Claim rootClaim = Claim.wrap(keccak256(abi.encode(15, _defender.traceAt(15))));
-        super.init(rootClaim, ABSOLUTE_PRESTATE);
+        super.init(rootClaim, ABSOLUTE_PRESTATE_CLAIM);
         defender = _defender;
         challenger = _challenger;
 
@@ -545,7 +528,7 @@ contract OneVsOne_Arena is FaultDisputeGame_Init {
     }
 }
 
-contract FaultDisputeGame_ResolvesCorrectly_IncorrectRoot is OneVsOne_Arena {
+contract FaultDisputeGame_ResolvesCorrectly_IncorrectRoot1 is OneVsOne_Arena {
     function setUp() public override {
         GamePlayer honest = new HonestPlayer(ABSOLUTE_PRESTATE);
         GamePlayer dishonest = new FullyDivergentPlayer(ABSOLUTE_PRESTATE);
@@ -566,7 +549,7 @@ contract FaultDisputeGame_ResolvesCorrectly_IncorrectRoot is OneVsOne_Arena {
     }
 }
 
-contract FaultDisputeGame_ResolvesCorrectly_CorrectRoot is OneVsOne_Arena {
+contract FaultDisputeGame_ResolvesCorrectly_CorrectRoot1 is OneVsOne_Arena {
     function setUp() public override {
         GamePlayer honest = new HonestPlayer(ABSOLUTE_PRESTATE);
         GamePlayer dishonest = new FullyDivergentPlayer(ABSOLUTE_PRESTATE);
@@ -676,8 +659,8 @@ contract FaultDisputeGame_ResolvesCorrectly_CorrectRoot3 is OneVsOne_Arena {
 ////////////////////////////////////////////////////////////////
 
 contract HonestPlayer is GamePlayer {
-    constructor(Claim _absolutePrestate) {
-        uint8 absolutePrestate = uint8(uint256(Claim.unwrap(_absolutePrestate)));
+    constructor(bytes memory _absolutePrestate) {
+        uint8 absolutePrestate = uint8(_absolutePrestate[31]);
         bytes memory honestTrace = new bytes(16);
         for (uint8 i = 0; i < honestTrace.length; i++) {
             honestTrace[i] = bytes1(absolutePrestate + i + 1);
@@ -687,8 +670,8 @@ contract HonestPlayer is GamePlayer {
 }
 
 contract FullyDivergentPlayer is GamePlayer {
-    constructor(Claim _absolutePrestate) {
-        uint8 absolutePrestate = uint8(uint256(Claim.unwrap(_absolutePrestate)));
+    constructor(bytes memory _absolutePrestate) {
+        uint8 absolutePrestate = uint8(_absolutePrestate[31]);
         bytes memory dishonestTrace = new bytes(16);
         for (uint8 i = 0; i < dishonestTrace.length; i++) {
             // Offset the honest trace by 1.
@@ -699,8 +682,8 @@ contract FullyDivergentPlayer is GamePlayer {
 }
 
 contract HalfDivergentPlayer is GamePlayer {
-    constructor(Claim _absolutePrestate) {
-        uint8 absolutePrestate = uint8(uint256(Claim.unwrap(_absolutePrestate)));
+    constructor(bytes memory _absolutePrestate) {
+        uint8 absolutePrestate = uint8(_absolutePrestate[31]);
         bytes memory dishonestTrace = new bytes(16);
         for (uint8 i = 0; i < dishonestTrace.length; i++) {
             // Offset the trace after the first half.
@@ -711,11 +694,11 @@ contract HalfDivergentPlayer is GamePlayer {
 }
 
 contract EarlyDivergentPlayer is GamePlayer {
-    constructor(Claim _absolutePrestate) {
-        uint8 absolutePrestate = uint8(uint256(Claim.unwrap(_absolutePrestate)));
+    constructor(bytes memory _absolutePrestate) {
+        uint8 absolutePrestate = uint8(_absolutePrestate[31]);
         bytes memory dishonestTrace = new bytes(16);
         for (uint8 i = 0; i < dishonestTrace.length; i++) {
-            // Offset the trace after the first half.
+            // Offset the trace after the first 3 instructions.
             dishonestTrace[i] = i > 2 ? bytes1(i) : bytes1(absolutePrestate + i + 1);
         }
         trace = dishonestTrace;
@@ -741,10 +724,10 @@ contract AlphabetVM is IBigStepper {
     {
         uint256 traceIndex;
         uint256 claim;
-        if (_stateData.length == 0) {
+        if (keccak256(_stateData) == Claim.unwrap(ABSOLUTE_PRESTATE)) {
             // If the state data is empty, then the absolute prestate is the claim.
             traceIndex = 0;
-            claim = uint256(Claim.unwrap(ABSOLUTE_PRESTATE));
+            (claim) = abi.decode(_stateData, (uint256));
         } else {
             // Otherwise, decode the state data.
             (traceIndex, claim) = abi.decode(_stateData, (uint256, uint256));

packages/contracts-bedrock/contracts/test/LibPosition.t.sol

@@ -74,6 +74,24 @@ contract LibPosition_Test is Test {
         assertEq(parent.indexAtDepth(), _indexAtDepth / 2);
     }
 
+    /// @notice Tests that the `traceAncestor` function correctly computes the position of the
+    ///         highest ancestor that commits to the same trace index.
+    function testFuzz_traceAncestor_correctness_succeeds(uint8 _depth, uint64 _indexAtDepth)
+        public
+    {
+        _depth = uint8(bound(_depth, 1, MAX_DEPTH));
+        _indexAtDepth = boundIndexAtDepth(_depth, _indexAtDepth);
+
+        Position position = LibPosition.wrap(_depth, _indexAtDepth);
+        Position ancestor = position.traceAncestor();
+        Position loopAncestor = position;
+        while (loopAncestor.parent().traceIndex(MAX_DEPTH) == position.traceIndex(MAX_DEPTH)) {
+            loopAncestor = loopAncestor.parent();
+        }
+
+        assertEq(Position.unwrap(ancestor), Position.unwrap(loopAncestor));
+    }
+
     /// @notice Tests that the `rightIndex` function correctly computes the deepest, right most index relative
     ///         to a given position.
     function testFuzz_rightIndex_correctness_succeeds(