op-node,batcher,proposer,challenger: Add env var flag tests (#8596)

This creates a standardized conversion between env vars and flag names and asserts
that most existing flags and all new flags satisfy the format. Existing flags that
don't match are skipped. This also adds general flag tests for op-batcher and
op-proposer.

op-batcher/flags/flags_test.go

+package flags
+
+import (
+	"slices"
+	"strings"
+	"testing"
+
+	opservice "github.com/ethereum-optimism/optimism/op-service"
+	"github.com/ethereum-optimism/optimism/op-service/txmgr"
+
+	"github.com/stretchr/testify/require"
+	"github.com/urfave/cli/v2"
+)
+
+// TestOptionalFlagsDontSetRequired asserts that all flags deemed optional set
+// the Required field to false.
+func TestOptionalFlagsDontSetRequired(t *testing.T) {
+	for _, flag := range optionalFlags {
+		reqFlag, ok := flag.(cli.RequiredFlag)
+		require.True(t, ok)
+		require.False(t, reqFlag.IsRequired())
+	}
+}
+
+// TestUniqueFlags asserts that all flag names are unique, to avoid accidental conflicts between the many flags.
+func TestUniqueFlags(t *testing.T) {
+	seenCLI := make(map[string]struct{})
+	for _, flag := range Flags {
+		name := flag.Names()[0]
+		if _, ok := seenCLI[name]; ok {
+			t.Errorf("duplicate flag %s", name)
+			continue
+		}
+		seenCLI[name] = struct{}{}
+	}
+}
+
+// TestBetaFlags test that all flags starting with "beta." have "BETA_" in the env var, and vice versa.
+func TestBetaFlags(t *testing.T) {
+	for _, flag := range Flags {
+		envFlag, ok := flag.(interface {
+			GetEnvVars() []string
+		})
+		if !ok || len(envFlag.GetEnvVars()) == 0 { // skip flags without env-var support
+			continue
+		}
+		name := flag.Names()[0]
+		envName := envFlag.GetEnvVars()[0]
+		if strings.HasPrefix(name, "beta.") {
+			require.Contains(t, envName, "BETA_", "%q flag must contain BETA in env var to match \"beta.\" flag name", name)
+		}
+		if strings.Contains(envName, "BETA_") {
+			require.True(t, strings.HasPrefix(name, "beta."), "%q flag must start with \"beta.\" in flag name to match \"BETA_\" env var", name)
+		}
+	}
+}
+
+func TestHasEnvVar(t *testing.T) {
+	for _, flag := range Flags {
+		flag := flag
+		flagName := flag.Names()[0]
+
+		t.Run(flagName, func(t *testing.T) {
+			envFlagGetter, ok := flag.(interface {
+				GetEnvVars() []string
+			})
+			envFlags := envFlagGetter.GetEnvVars()
+			require.True(t, ok, "must be able to cast the flag to an EnvVar interface")
+			require.Equal(t, 1, len(envFlags), "flags should have exactly one env var")
+		})
+	}
+}
+
+func TestEnvVarFormat(t *testing.T) {
+	for _, flag := range Flags {
+		flag := flag
+		flagName := flag.Names()[0]
+
+		skippedFlags := []string{
+			txmgr.FeeLimitMultiplierFlagName,
+			txmgr.TxSendTimeoutFlagName,
+			txmgr.TxNotInMempoolTimeoutFlagName,
+		}
+
+		t.Run(flagName, func(t *testing.T) {
+			if slices.Contains(skippedFlags, flagName) {
+				t.Skipf("Skipping flag %v which is known to not have a standard flag name <-> env var conversion", flagName)
+			}
+			envFlagGetter, ok := flag.(interface {
+				GetEnvVars() []string
+			})
+			envFlags := envFlagGetter.GetEnvVars()
+			require.True(t, ok, "must be able to cast the flag to an EnvVar interface")
+			require.Equal(t, 1, len(envFlags), "flags should have exactly one env var")
+			expectedEnvVar := opservice.FlagNameToEnvVarName(flagName, "OP_BATCHER")
+			require.Equal(t, expectedEnvVar, envFlags[0])
+		})
+	}
+}

op-challenger/flags/flags_test.go

@@ -2,9 +2,14 @@ package flags
 
 import (
 	"reflect"
+	"slices"
 	"strings"
 	"testing"
 
+	opservice "github.com/ethereum-optimism/optimism/op-service"
+	"github.com/ethereum-optimism/optimism/op-service/txmgr"
+
+	"github.com/stretchr/testify/require"
 	"github.com/urfave/cli/v2"
 )
 
@@ -57,3 +62,30 @@ func envVarForFlag(flag cli.Flag) string {
 	}
 	return envVarValue.Index(0).String()
 }
+
+func TestEnvVarFormat(t *testing.T) {
+	for _, flag := range Flags {
+		flag := flag
+		flagName := flag.Names()[0]
+
+		skippedFlags := []string{
+			txmgr.FeeLimitMultiplierFlagName,
+			txmgr.TxSendTimeoutFlagName,
+			txmgr.TxNotInMempoolTimeoutFlagName,
+		}
+
+		t.Run(flagName, func(t *testing.T) {
+			if slices.Contains(skippedFlags, flagName) {
+				t.Skipf("Skipping flag %v which is known to not have a standard flag name <-> env var conversion", flagName)
+			}
+			envFlagGetter, ok := flag.(interface {
+				GetEnvVars() []string
+			})
+			envFlags := envFlagGetter.GetEnvVars()
+			require.True(t, ok, "must be able to cast the flag to an EnvVar interface")
+			require.Equal(t, 1, len(envFlags), "flags should have exactly one env var")
+			expectedEnvVar := opservice.FlagNameToEnvVarName(flagName, "OP_CHALLENGER")
+			require.Equal(t, expectedEnvVar, envFlags[0])
+		})
+	}
+}

op-node/flags/flags_test.go

 package flags
 
 import (
+	"slices"
 	"strings"
 	"testing"
 
+	opservice "github.com/ethereum-optimism/optimism/op-service"
+
 	"github.com/stretchr/testify/require"
 	"github.com/urfave/cli/v2"
 )
@@ -85,3 +88,48 @@ func TestHasEnvVar(t *testing.T) {
 		})
 	}
 }
+
+func TestEnvVarFormat(t *testing.T) {
+	for _, flag := range Flags {
+		flag := flag
+		flagName := flag.Names()[0]
+
+		skippedFlags := []string{
+			L1NodeAddr.Name,
+			L2EngineAddr.Name,
+			L2EngineJWTSecret.Name,
+			L1TrustRPC.Name,
+			L1RPCProviderKind.Name,
+			SnapshotLog.Name,
+			BackupL2UnsafeSyncRPC.Name,
+			BackupL2UnsafeSyncRPCTrustRPC.Name,
+			"p2p.scoring",
+			"p2p.ban.peers",
+			"p2p.ban.threshold",
+			"p2p.ban.duration",
+			"p2p.listen.tcp",
+			"p2p.listen.udp",
+			"p2p.useragent",
+			"p2p.gossip.mesh.lo",
+			"p2p.gossip.mesh.floodpublish",
+			"l2.engine-sync",
+		}
+
+		t.Run(flagName, func(t *testing.T) {
+			if slices.Contains(skippedFlags, flagName) {
+				t.Skipf("Skipping flag %v which is known to not have a standard flag name <-> env var conversion", flagName)
+			}
+			if flagName == PeerScoringName || flagName == PeerScoreBandsName || flagName == TopicScoringName {
+				t.Skipf("Skipping flag %v which is known to have no env vars", flagName)
+			}
+			envFlagGetter, ok := flag.(interface {
+				GetEnvVars() []string
+			})
+			envFlags := envFlagGetter.GetEnvVars()
+			require.True(t, ok, "must be able to cast the flag to an EnvVar interface")
+			require.Equal(t, 1, len(envFlags), "flags should have exactly one env var")
+			expectedEnvVar := opservice.FlagNameToEnvVarName(flagName, "OP_NODE")
+			require.Equal(t, expectedEnvVar, envFlags[0])
+		})
+	}
+}

op-proposer/flags/flags_test.go

+package flags
+
+import (
+	"slices"
+	"strings"
+	"testing"
+
+	opservice "github.com/ethereum-optimism/optimism/op-service"
+	"github.com/ethereum-optimism/optimism/op-service/txmgr"
+
+	"github.com/stretchr/testify/require"
+	"github.com/urfave/cli/v2"
+)
+
+// TestOptionalFlagsDontSetRequired asserts that all flags deemed optional set
+// the Required field to false.
+func TestOptionalFlagsDontSetRequired(t *testing.T) {
+	for _, flag := range optionalFlags {
+		reqFlag, ok := flag.(cli.RequiredFlag)
+		require.True(t, ok)
+		require.False(t, reqFlag.IsRequired())
+	}
+}
+
+// TestUniqueFlags asserts that all flag names are unique, to avoid accidental conflicts between the many flags.
+func TestUniqueFlags(t *testing.T) {
+	seenCLI := make(map[string]struct{})
+	for _, flag := range Flags {
+		name := flag.Names()[0]
+		if _, ok := seenCLI[name]; ok {
+			t.Errorf("duplicate flag %s", name)
+			continue
+		}
+		seenCLI[name] = struct{}{}
+	}
+}
+
+// TestBetaFlags test that all flags starting with "beta." have "BETA_" in the env var, and vice versa.
+func TestBetaFlags(t *testing.T) {
+	for _, flag := range Flags {
+		envFlag, ok := flag.(interface {
+			GetEnvVars() []string
+		})
+		if !ok || len(envFlag.GetEnvVars()) == 0 { // skip flags without env-var support
+			continue
+		}
+		name := flag.Names()[0]
+		envName := envFlag.GetEnvVars()[0]
+		if strings.HasPrefix(name, "beta.") {
+			require.Contains(t, envName, "BETA_", "%q flag must contain BETA in env var to match \"beta.\" flag name", name)
+		}
+		if strings.Contains(envName, "BETA_") {
+			require.True(t, strings.HasPrefix(name, "beta."), "%q flag must start with \"beta.\" in flag name to match \"BETA_\" env var", name)
+		}
+	}
+}
+
+func TestHasEnvVar(t *testing.T) {
+	for _, flag := range Flags {
+		flag := flag
+		flagName := flag.Names()[0]
+
+		t.Run(flagName, func(t *testing.T) {
+			envFlagGetter, ok := flag.(interface {
+				GetEnvVars() []string
+			})
+			envFlags := envFlagGetter.GetEnvVars()
+			require.True(t, ok, "must be able to cast the flag to an EnvVar interface")
+			require.Equal(t, 1, len(envFlags), "flags should have exactly one env var")
+		})
+	}
+}
+
+func TestEnvVarFormat(t *testing.T) {
+	for _, flag := range Flags {
+		flag := flag
+		flagName := flag.Names()[0]
+
+		skippedFlags := []string{
+			txmgr.FeeLimitMultiplierFlagName,
+			txmgr.TxSendTimeoutFlagName,
+			txmgr.TxNotInMempoolTimeoutFlagName,
+		}
+
+		t.Run(flagName, func(t *testing.T) {
+			if slices.Contains(skippedFlags, flagName) {
+				t.Skipf("Skipping flag %v which is known to not have a standard flag name <-> env var conversion", flagName)
+			}
+			envFlagGetter, ok := flag.(interface {
+				GetEnvVars() []string
+			})
+			envFlags := envFlagGetter.GetEnvVars()
+			require.True(t, ok, "must be able to cast the flag to an EnvVar interface")
+			require.Equal(t, 1, len(envFlags), "flags should have exactly one env var")
+			expectedEnvVar := opservice.FlagNameToEnvVarName(flagName, "OP_PROPOSER")
+			require.Equal(t, expectedEnvVar, envFlags[0])
+		})
+	}
+}

op-service/util.go

@@ -32,6 +32,11 @@ func ValidateEnvVars(prefix string, flags []cli.Flag, log log.Logger) {
 	}
 }
 
+func FlagNameToEnvVarName(f string, prefix string) string {
+	f = strings.ReplaceAll(strings.ReplaceAll(strings.ToUpper(f), ".", "_"), "-", "_")
+	return fmt.Sprintf("%s_%s", prefix, f)
+}
+
 func cliFlagsToEnvVars(flags []cli.Flag) map[string]struct{} {
 	definedEnvVars := make(map[string]struct{})
 	for _, flag := range flags {