Skip to content

Commit

Permalink
op-challenger: Improve GameType typing (ethereum-optimism#11012)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Inphi
Inphi authored Jun 27, 2024
1 parent 1440a51 commit 9803860
Show file tree
Hide file tree
Showing 14 changed files with 256 additions and 204 deletions.
4 changes: 2 additions & 2 deletions op-challenger/cmd/create_game.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import (
"context"
"fmt"

"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/contracts"
contractMetrics "github.com/ethereum-optimism/optimism/op-challenger/game/fault/contracts/metrics"
"github.com/ethereum-optimism/optimism/op-challenger/game/fault/types"
"github.com/ethereum-optimism/optimism/op-challenger/tools"
opservice "github.com/ethereum-optimism/optimism/op-service"
oplog "github.com/ethereum-optimism/optimism/op-service/log"
Expand All @@ -22,7 +22,7 @@ var (
Name: "trace-type",
Usage: "Trace types to support.",
EnvVars: opservice.PrefixEnvVar(flags.EnvVarPrefix, "TRACE_TYPE"),
Value: config.TraceTypeCannon.String(),
Value: types.TraceTypeCannon.String(),
}
OutputRootFlag = &cli.StringFlag{
Name: "output-root",
Expand Down
171 changes: 86 additions & 85 deletions op-challenger/cmd/main_test.go
View file

Large diffs are not rendered by default.

54 changes: 8 additions & 46 deletions op-challenger/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

"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-node/chaincfg"
opmetrics "github.com/ethereum-optimism/optimism/op-service/metrics"
"github.com/ethereum-optimism/optimism/op-service/oppprof"
Expand Down Expand Up @@ -50,45 +51,6 @@ var (
ErrAsteriscNetworkUnknown = errors.New("unknown asterisc network")
)

type TraceType string

const (
TraceTypeAlphabet TraceType = "alphabet"
TraceTypeFast TraceType = "fast"
TraceTypeCannon TraceType = "cannon"
TraceTypeAsterisc TraceType = "asterisc"
TraceTypePermissioned TraceType = "permissioned"
)

var TraceTypes = []TraceType{TraceTypeAlphabet, TraceTypeCannon, TraceTypePermissioned, TraceTypeAsterisc, TraceTypeFast}

func (t TraceType) String() string {
return string(t)
}

// Set implements the Set method required by the [cli.Generic] interface.
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 (t *TraceType) Clone() any {
cpy := *t
return &cpy
}

func ValidTraceType(value TraceType) bool {
for _, t := range TraceTypes {
if t == value {
return true
}
}
return false
}

const (
DefaultPollInterval = time.Second * 12
DefaultCannonSnapshotFreq = uint(1_000_000_000)
Expand Down Expand Up @@ -122,7 +84,7 @@ type Config struct {

SelectiveClaimResolution bool // Whether to only resolve claims for the claimants in AdditionalBondClaimants union [TxSender.From()]

TraceTypes []TraceType // Type of traces supported
TraceTypes []types.TraceType // Type of traces supported

RollupRpc string // L2 Rollup RPC Url

Expand Down Expand Up @@ -152,7 +114,7 @@ func NewConfig(
l2RollupRpc string,
l2EthRpc string,
datadir string,
supportedTraceTypes ...TraceType,
supportedTraceTypes ...types.TraceType,
) Config {
return Config{
L1EthRpc: l1EthRpc,
Expand All @@ -174,15 +136,15 @@ func NewConfig(
Datadir: datadir,

Cannon: vm.Config{
VmType: TraceTypeCannon.String(),
VmType: types.TraceTypeCannon,
L1: l1EthRpc,
L1Beacon: l1BeaconApi,
L2: l2EthRpc,
SnapshotFreq: DefaultCannonSnapshotFreq,
InfoFreq: DefaultCannonInfoFreq,
},
Asterisc: vm.Config{
VmType: TraceTypeAsterisc.String(),
VmType: types.TraceTypeAsterisc,
L1: l1EthRpc,
L1Beacon: l1BeaconApi,
L2: l2EthRpc,
Expand All @@ -193,7 +155,7 @@ func NewConfig(
}
}

func (c Config) TraceTypeEnabled(t TraceType) bool {
func (c Config) TraceTypeEnabled(t types.TraceType) bool {
return slices.Contains(c.TraceTypes, t)
}

Expand Down Expand Up @@ -222,7 +184,7 @@ func (c Config) Check() error {
if c.MaxConcurrency == 0 {
return ErrMaxConcurrencyZero
}
if c.TraceTypeEnabled(TraceTypeCannon) || c.TraceTypeEnabled(TraceTypePermissioned) {
if c.TraceTypeEnabled(types.TraceTypeCannon) || c.TraceTypeEnabled(types.TraceTypePermissioned) {
if c.Cannon.VmBin == "" {
return ErrMissingCannonBin
}
Expand Down Expand Up @@ -260,7 +222,7 @@ func (c Config) Check() error {
return ErrMissingCannonInfoFreq
}
}
if c.TraceTypeEnabled(TraceTypeAsterisc) {
if c.TraceTypeEnabled(types.TraceTypeAsterisc) {
if c.Asterisc.VmBin == "" {
return ErrMissingAsteriscBin
}
Expand Down
47 changes: 24 additions & 23 deletions op-challenger/config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/ethereum/go-ethereum/common"
"github.com/stretchr/testify/require"

"github.com/ethereum-optimism/optimism/op-challenger/game/fault/types"
"github.com/ethereum-optimism/optimism/op-service/txmgr"
)

Expand All @@ -32,8 +33,8 @@ var (
validAsteriscAbsolutPreStateBaseURL, _ = url.Parse("http://localhost/bar/")
)

var cannonTraceTypes = []TraceType{TraceTypeCannon, TraceTypePermissioned}
var asteriscTraceTypes = []TraceType{TraceTypeAsterisc}
var cannonTraceTypes = []types.TraceType{types.TraceTypeCannon, types.TraceTypePermissioned}
var asteriscTraceTypes = []types.TraceType{types.TraceTypeAsterisc}

func applyValidConfigForCannon(cfg *Config) {
cfg.Cannon.VmBin = validCannonBin
Expand All @@ -49,20 +50,20 @@ func applyValidConfigForAsterisc(cfg *Config) {
cfg.Asterisc.Network = validAsteriscNetwork
}

func validConfig(traceType TraceType) Config {
func validConfig(traceType types.TraceType) Config {
cfg := NewConfig(validGameFactoryAddress, validL1EthRpc, validL1BeaconUrl, validRollupRpc, validL2Rpc, validDatadir, traceType)
if traceType == TraceTypeCannon || traceType == TraceTypePermissioned {
if traceType == types.TraceTypeCannon || traceType == types.TraceTypePermissioned {
applyValidConfigForCannon(&cfg)
}
if traceType == TraceTypeAsterisc {
if traceType == types.TraceTypeAsterisc {
applyValidConfigForAsterisc(&cfg)
}
return cfg
}

// TestValidConfigIsValid checks that the config provided by validConfig is actually valid
func TestValidConfigIsValid(t *testing.T) {
for _, traceType := range TraceTypes {
for _, traceType := range types.TraceTypes {
traceType := traceType
t.Run(traceType.String(), func(t *testing.T) {
err := validConfig(traceType).Check()
Expand All @@ -73,38 +74,38 @@ func TestValidConfigIsValid(t *testing.T) {

func TestTxMgrConfig(t *testing.T) {
t.Run("Invalid", func(t *testing.T) {
config := validConfig(TraceTypeCannon)
config := validConfig(types.TraceTypeCannon)
config.TxMgrConfig = txmgr.CLIConfig{}
require.Equal(t, config.Check().Error(), "must provide a L1 RPC url")
})
}

func TestL1EthRpcRequired(t *testing.T) {
config := validConfig(TraceTypeCannon)
config := validConfig(types.TraceTypeCannon)
config.L1EthRpc = ""
require.ErrorIs(t, config.Check(), ErrMissingL1EthRPC)
}

func TestL1BeaconRequired(t *testing.T) {
config := validConfig(TraceTypeCannon)
config := validConfig(types.TraceTypeCannon)
config.L1Beacon = ""
require.ErrorIs(t, config.Check(), ErrMissingL1Beacon)
}

func TestGameFactoryAddressRequired(t *testing.T) {
config := validConfig(TraceTypeCannon)
config := validConfig(types.TraceTypeCannon)
config.GameFactoryAddress = common.Address{}
require.ErrorIs(t, config.Check(), ErrMissingGameFactoryAddress)
}

func TestSelectiveClaimResolutionNotRequired(t *testing.T) {
config := validConfig(TraceTypeCannon)
config := validConfig(types.TraceTypeCannon)
require.Equal(t, false, config.SelectiveClaimResolution)
require.NoError(t, config.Check())
}

func TestGameAllowlistNotRequired(t *testing.T) {
config := validConfig(TraceTypeCannon)
config := validConfig(types.TraceTypeCannon)
config.GameAllowlist = []common.Address{}
require.NoError(t, config.Check())
}
Expand Down Expand Up @@ -322,33 +323,33 @@ func TestAsteriscRequiredArgs(t *testing.T) {
}

func TestDatadirRequired(t *testing.T) {
config := validConfig(TraceTypeAlphabet)
config := validConfig(types.TraceTypeAlphabet)
config.Datadir = ""
require.ErrorIs(t, config.Check(), ErrMissingDatadir)
}

func TestMaxConcurrency(t *testing.T) {
t.Run("Required", func(t *testing.T) {
config := validConfig(TraceTypeAlphabet)
config := validConfig(types.TraceTypeAlphabet)
config.MaxConcurrency = 0
require.ErrorIs(t, config.Check(), ErrMaxConcurrencyZero)
})

t.Run("DefaultToNumberOfCPUs", func(t *testing.T) {
config := validConfig(TraceTypeAlphabet)
config := validConfig(types.TraceTypeAlphabet)
require.EqualValues(t, runtime.NumCPU(), config.MaxConcurrency)
})
}

func TestHttpPollInterval(t *testing.T) {
t.Run("Default", func(t *testing.T) {
config := validConfig(TraceTypeAlphabet)
config := validConfig(types.TraceTypeAlphabet)
require.EqualValues(t, DefaultPollInterval, config.PollInterval)
})
}

func TestRollupRpcRequired(t *testing.T) {
for _, traceType := range TraceTypes {
for _, traceType := range types.TraceTypes {
traceType := traceType
t.Run(traceType.String(), func(t *testing.T) {
config := validConfig(traceType)
Expand All @@ -359,8 +360,8 @@ func TestRollupRpcRequired(t *testing.T) {
}

func TestRequireConfigForMultipleTraceTypesForCannon(t *testing.T) {
cfg := validConfig(TraceTypeCannon)
cfg.TraceTypes = []TraceType{TraceTypeCannon, TraceTypeAlphabet}
cfg := validConfig(types.TraceTypeCannon)
cfg.TraceTypes = []types.TraceType{types.TraceTypeCannon, types.TraceTypeAlphabet}
// Set all required options and check its valid
cfg.RollupRpc = validRollupRpc
require.NoError(t, cfg.Check())
Expand All @@ -377,8 +378,8 @@ func TestRequireConfigForMultipleTraceTypesForCannon(t *testing.T) {
}

func TestRequireConfigForMultipleTraceTypesForAsterisc(t *testing.T) {
cfg := validConfig(TraceTypeAsterisc)
cfg.TraceTypes = []TraceType{TraceTypeAsterisc, TraceTypeAlphabet}
cfg := validConfig(types.TraceTypeAsterisc)
cfg.TraceTypes = []types.TraceType{types.TraceTypeAsterisc, types.TraceTypeAlphabet}
// Set all required options and check its valid
cfg.RollupRpc = validRollupRpc
require.NoError(t, cfg.Check())
Expand All @@ -395,10 +396,10 @@ func TestRequireConfigForMultipleTraceTypesForAsterisc(t *testing.T) {
}

func TestRequireConfigForMultipleTraceTypesForCannonAndAsterisc(t *testing.T) {
cfg := validConfig(TraceTypeCannon)
cfg := validConfig(types.TraceTypeCannon)
applyValidConfigForAsterisc(&cfg)

cfg.TraceTypes = []TraceType{TraceTypeCannon, TraceTypeAsterisc, TraceTypeAlphabet, TraceTypeFast}
cfg.TraceTypes = []types.TraceType{types.TraceTypeCannon, types.TraceTypeAsterisc, types.TraceTypeAlphabet, types.TraceTypeFast}
// Set all required options and check its valid
cfg.RollupRpc = validRollupRpc
require.NoError(t, cfg.Check())
Expand Down
25 changes: 13 additions & 12 deletions op-challenger/flags/flags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"strings"

"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-service/flags"
"github.com/ethereum-optimism/superchain-registry/superchain"
"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -61,9 +62,9 @@ var (
}
TraceTypeFlag = &cli.StringSliceFlag{
Name: "trace-type",
Usage: "The trace types to support. Valid options: " + openum.EnumString(config.TraceTypes),
Usage: "The trace types to support. Valid options: " + openum.EnumString(types.TraceTypes),
EnvVars: prefixEnvVars("TRACE_TYPE"),
Value: cli.NewStringSlice(config.TraceTypeCannon.String()),
Value: cli.NewStringSlice(types.TraceTypeCannon.String()),
}
DatadirFlag = &cli.StringFlag{
Name: "datadir",
Expand Down Expand Up @@ -339,7 +340,7 @@ func CheckAsteriscFlags(ctx *cli.Context) error {
return nil
}

func CheckRequired(ctx *cli.Context, traceTypes []config.TraceType) error {
func CheckRequired(ctx *cli.Context, traceTypes []types.TraceType) error {
for _, f := range requiredFlags {
if !ctx.IsSet(f.Names()[0]) {
return fmt.Errorf("flag %s is required", f.Names()[0])
Expand All @@ -351,26 +352,26 @@ func CheckRequired(ctx *cli.Context, traceTypes []config.TraceType) error {
}
for _, traceType := range traceTypes {
switch traceType {
case config.TraceTypeCannon, config.TraceTypePermissioned:
case types.TraceTypeCannon, types.TraceTypePermissioned:
if err := CheckCannonFlags(ctx); err != nil {
return err
}
case config.TraceTypeAsterisc:
case types.TraceTypeAsterisc:
if err := CheckAsteriscFlags(ctx); err != nil {
return err
}
case config.TraceTypeAlphabet, config.TraceTypeFast:
case types.TraceTypeAlphabet, types.TraceTypeFast:
default:
return fmt.Errorf("invalid trace type. must be one of %v", config.TraceTypes)
return fmt.Errorf("invalid trace type. must be one of %v", types.TraceTypes)
}
}
return nil
}

func parseTraceTypes(ctx *cli.Context) ([]config.TraceType, error) {
var traceTypes []config.TraceType
func parseTraceTypes(ctx *cli.Context) ([]types.TraceType, error) {
var traceTypes []types.TraceType
for _, typeName := range ctx.StringSlice(TraceTypeFlag.Name) {
traceType := new(config.TraceType)
traceType := new(types.TraceType)
if err := traceType.Set(typeName); err != nil {
return nil, err
}
Expand Down Expand Up @@ -514,7 +515,7 @@ func NewConfigFromCLI(ctx *cli.Context, logger log.Logger) (*config.Config, erro
AdditionalBondClaimants: claimants,
RollupRpc: ctx.String(RollupRpcFlag.Name),
Cannon: vm.Config{
VmType: config.TraceTypeCannon.String(),
VmType: types.TraceTypeCannon,
L1: l1EthRpc,
L1Beacon: l1Beacon,
L2: l2Rpc,
Expand All @@ -530,7 +531,7 @@ func NewConfigFromCLI(ctx *cli.Context, logger log.Logger) (*config.Config, erro
CannonAbsolutePreStateBaseURL: cannonPrestatesURL,
Datadir: ctx.String(DatadirFlag.Name),
Asterisc: vm.Config{
VmType: config.TraceTypeAsterisc.String(),
VmType: types.TraceTypeAsterisc,
L1: l1EthRpc,
L1Beacon: l1Beacon,
L2: l2Rpc,
Expand Down
3 changes: 2 additions & 1 deletion op-challenger/game/fault/contracts/gamefactory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"math/big"

"github.com/ethereum-optimism/optimism/op-challenger/game/fault/contracts/metrics"
faultTypes "github.com/ethereum-optimism/optimism/op-challenger/game/fault/types"
"github.com/ethereum-optimism/optimism/op-challenger/game/types"
"github.com/ethereum-optimism/optimism/op-service/sources/batching"
"github.com/ethereum-optimism/optimism/op-service/sources/batching/rpcblock"
Expand Down Expand Up @@ -76,7 +77,7 @@ func (f *DisputeGameFactoryContract) GetGame(ctx context.Context, idx uint64, bl
return f.decodeGame(idx, result), nil
}

func (f *DisputeGameFactoryContract) GetGameImpl(ctx context.Context, gameType uint32) (common.Address, error) {
func (f *DisputeGameFactoryContract) GetGameImpl(ctx context.Context, gameType faultTypes.GameType) (common.Address, error) {
defer f.metrics.StartContractRequest("GetGameImpl")()
result, err := f.multiCaller.SingleCall(ctx, rpcblock.Latest, f.contract.Call(methodGameImpls, gameType))
if err != nil {
Expand Down
Loading

0 comments on commit 9803860

Please sign in to comment.