From 8613ff1c0641d4342cb7b5e043b7cc43cf8ab2f6 Mon Sep 17 00:00:00 2001 From: Manav Darji Date: Thu, 21 Sep 2023 12:52:59 +0530 Subject: [PATCH] internal/cli: add support to overwrite config.toml values via cli flags (#1008) * internal/cli: add support to overwrite config.toml via cli flags * fix lint and refactor * add extensive tests for flagset * fix type conversion for big.Int * add more tests for coverage * add t.parallel * internal/cli/flagset: handle flag conversion using interface * internal/cli/flagset: fix test --- internal/cli/dumpconfig.go | 2 +- internal/cli/flagset/flagset.go | 154 ++++++++++++++-- internal/cli/flagset/flagset_test.go | 212 ++++++++++++++++++++-- internal/cli/server/command.go | 95 ++++++---- internal/cli/server/command_test.go | 140 +++++++++++--- internal/cli/server/config_legacy_test.go | 22 ++- internal/cli/server/flags.go | 8 +- internal/cli/server/testdata/test.toml | 32 ++-- scripts/getconfig.go | 2 +- 9 files changed, 550 insertions(+), 117 deletions(-) diff --git a/internal/cli/dumpconfig.go b/internal/cli/dumpconfig.go index 0cd0958ae9..c735f040fa 100644 --- a/internal/cli/dumpconfig.go +++ b/internal/cli/dumpconfig.go @@ -42,7 +42,7 @@ func (c *DumpconfigCommand) Synopsis() string { func (c *DumpconfigCommand) Run(args []string) int { // Initialize an empty command instance to get flags command := server.Command{} - flags := command.Flags() + flags := command.Flags(nil) if err := flags.Parse(args); err != nil { c.UI.Error(err.Error()) diff --git a/internal/cli/flagset/flagset.go b/internal/cli/flagset/flagset.go index 25b7b4c3c3..679c78c4a7 100644 --- a/internal/cli/flagset/flagset.go +++ b/internal/cli/flagset/flagset.go @@ -5,33 +5,42 @@ import ( "fmt" "math/big" "sort" + "strconv" "strings" "time" ) type Flagset struct { - flags []*FlagVar + flags map[string]*FlagVar set *flag.FlagSet } func NewFlagSet(name string) *Flagset { f := &Flagset{ - flags: []*FlagVar{}, + flags: make(map[string]*FlagVar, 0), set: flag.NewFlagSet(name, flag.ContinueOnError), } return f } +// Updatable is a minimalistic representation of a flag which has +// the method `UpdateValue` implemented which can be called while +// overwriting flags. +type Updatable interface { + UpdateValue(string) +} + type FlagVar struct { Name string Usage string Group string Default any + Value Updatable } func (f *Flagset) addFlag(fl *FlagVar) { - f.flags = append(f.flags, fl) + f.flags[fl.Name] = fl } func (f *Flagset) Help() string { @@ -51,9 +60,12 @@ func (f *Flagset) Help() string { } func (f *Flagset) GetAllFlags() []string { - flags := []string{} - for _, flag := range f.flags { - flags = append(flags, flag.Name) + i := 0 + flags := make([]string, 0, len(f.flags)) + + for name := range f.flags { + flags[i] = name + i++ } return flags @@ -110,6 +122,33 @@ func (f *Flagset) Args() []string { return f.set.Args() } +// UpdateValue updates the underlying value of a flag +// given the flag name and value to update using pointer. +func (f *Flagset) UpdateValue(names []string, values []string) { + for i, name := range names { + if flag, ok := f.flags[name]; ok { + value := values[i] + + // Call the underlying flag's `UpdateValue` method + flag.Value.UpdateValue(value) + } + } +} + +// Visit visits all the set flags and returns the name and value +// in string to set later. +func (f *Flagset) Visit() ([]string, []string) { + names := make([]string, 0, len(f.flags)) + values := make([]string, 0, len(f.flags)) + + f.set.Visit(func(flag *flag.Flag) { + names = append(names, flag.Name) + values = append(values, flag.Value.String()) + }) + + return names, values +} + type BoolFlag struct { Name string Usage string @@ -118,12 +157,19 @@ type BoolFlag struct { Group string } +func (b *BoolFlag) UpdateValue(value string) { + v, _ := strconv.ParseBool(value) + + *b.Value = v +} + func (f *Flagset) BoolFlag(b *BoolFlag) { f.addFlag(&FlagVar{ Name: b.Name, Usage: b.Usage, Group: b.Group, Default: b.Default, + Value: b, }) f.set.BoolVar(b.Value, b.Name, b.Default, b.Usage) } @@ -137,6 +183,10 @@ type StringFlag struct { HideDefaultFromDoc bool } +func (b *StringFlag) UpdateValue(value string) { + *b.Value = value +} + func (f *Flagset) StringFlag(b *StringFlag) { if b.Default == "" || b.HideDefaultFromDoc { f.addFlag(&FlagVar{ @@ -144,6 +194,7 @@ func (f *Flagset) StringFlag(b *StringFlag) { Usage: b.Usage, Group: b.Group, Default: nil, + Value: b, }) } else { f.addFlag(&FlagVar{ @@ -151,6 +202,7 @@ func (f *Flagset) StringFlag(b *StringFlag) { Usage: b.Usage, Group: b.Group, Default: b.Default, + Value: b, }) } @@ -165,12 +217,19 @@ type IntFlag struct { Group string } +func (b *IntFlag) UpdateValue(value string) { + v, _ := strconv.ParseInt(value, 10, 64) + + *b.Value = int(v) +} + func (f *Flagset) IntFlag(i *IntFlag) { f.addFlag(&FlagVar{ Name: i.Name, Usage: i.Usage, Group: i.Group, Default: i.Default, + Value: i, }) f.set.IntVar(i.Value, i.Name, i.Default, i.Usage) } @@ -183,12 +242,19 @@ type Uint64Flag struct { Group string } +func (b *Uint64Flag) UpdateValue(value string) { + v, _ := strconv.ParseUint(value, 10, 64) + + *b.Value = v +} + func (f *Flagset) Uint64Flag(i *Uint64Flag) { f.addFlag(&FlagVar{ Name: i.Name, Usage: i.Usage, Group: i.Group, Default: fmt.Sprintf("%d", i.Default), + Value: i, }) f.set.Uint64Var(i.Value, i.Name, i.Default, i.Usage) } @@ -209,31 +275,47 @@ func (b *BigIntFlag) String() string { return b.Value.String() } -func (b *BigIntFlag) Set(value string) error { +func parseBigInt(value string) *big.Int { num := new(big.Int) - var ok bool if strings.HasPrefix(value, "0x") { - num, ok = num.SetString(value[2:], 16) - *b.Value = *num + num, _ = num.SetString(value[2:], 16) } else { - num, ok = num.SetString(value, 10) - *b.Value = *num + num, _ = num.SetString(value, 10) } - if !ok { + return num +} + +func (b *BigIntFlag) Set(value string) error { + num := parseBigInt(value) + + if num == nil { return fmt.Errorf("failed to set big int") } + *b.Value = *num + return nil } +func (b *BigIntFlag) UpdateValue(value string) { + num := parseBigInt(value) + + if num == nil { + return + } + + *b.Value = *num +} + func (f *Flagset) BigIntFlag(b *BigIntFlag) { f.addFlag(&FlagVar{ Name: b.Name, Usage: b.Usage, Group: b.Group, Default: b.Default, + Value: b, }) f.set.Var(b, b.Name, b.Usage) } @@ -273,6 +355,10 @@ func (i *SliceStringFlag) Set(value string) error { return nil } +func (i *SliceStringFlag) UpdateValue(value string) { + *i.Value = SplitAndTrim(value) +} + func (f *Flagset) SliceStringFlag(s *SliceStringFlag) { if s.Default == nil || len(s.Default) == 0 { f.addFlag(&FlagVar{ @@ -280,6 +366,7 @@ func (f *Flagset) SliceStringFlag(s *SliceStringFlag) { Usage: s.Usage, Group: s.Group, Default: nil, + Value: s, }) } else { f.addFlag(&FlagVar{ @@ -287,6 +374,7 @@ func (f *Flagset) SliceStringFlag(s *SliceStringFlag) { Usage: s.Usage, Group: s.Group, Default: strings.Join(s.Default, ","), + Value: s, }) } @@ -301,12 +389,19 @@ type DurationFlag struct { Group string } +func (d *DurationFlag) UpdateValue(value string) { + v, _ := time.ParseDuration(value) + + *d.Value = v +} + func (f *Flagset) DurationFlag(d *DurationFlag) { f.addFlag(&FlagVar{ Name: d.Name, Usage: d.Usage, Group: d.Group, Default: d.Default, + Value: d, }) f.set.DurationVar(d.Value, d.Name, d.Default, "") } @@ -336,24 +431,38 @@ func (m *MapStringFlag) String() string { return formatMapString(*m.Value) } -func (m *MapStringFlag) Set(value string) error { - if m.Value == nil { - m.Value = &map[string]string{} - } +func parseMap(value string) map[string]string { + m := make(map[string]string) for _, t := range strings.Split(value, ",") { if t != "" { kv := strings.Split(t, "=") if len(kv) == 2 { - (*m.Value)[kv[0]] = kv[1] + m[kv[0]] = kv[1] } } } + return m +} + +func (m *MapStringFlag) Set(value string) error { + if m.Value == nil { + m.Value = &map[string]string{} + } + + m2 := parseMap(value) + *m.Value = m2 + return nil } +func (m *MapStringFlag) UpdateValue(value string) { + m2 := parseMap(value) + *m.Value = m2 +} + func (f *Flagset) MapStringFlag(m *MapStringFlag) { if m.Default == nil || len(m.Default) == 0 { f.addFlag(&FlagVar{ @@ -361,6 +470,7 @@ func (f *Flagset) MapStringFlag(m *MapStringFlag) { Usage: m.Usage, Group: m.Group, Default: nil, + Value: m, }) } else { f.addFlag(&FlagVar{ @@ -368,6 +478,7 @@ func (f *Flagset) MapStringFlag(m *MapStringFlag) { Usage: m.Usage, Group: m.Group, Default: formatMapString(m.Default), + Value: m, }) } @@ -382,12 +493,19 @@ type Float64Flag struct { Group string } +func (f *Float64Flag) UpdateValue(value string) { + v, _ := strconv.ParseFloat(value, 64) + + *f.Value = v +} + func (f *Flagset) Float64Flag(i *Float64Flag) { f.addFlag(&FlagVar{ Name: i.Name, Usage: i.Usage, Group: i.Group, Default: i.Default, + Value: i, }) f.set.Float64Var(i.Value, i.Name, i.Default, "") } diff --git a/internal/cli/flagset/flagset_test.go b/internal/cli/flagset/flagset_test.go index 118361320d..96d462de31 100644 --- a/internal/cli/flagset/flagset_test.go +++ b/internal/cli/flagset/flagset_test.go @@ -1,26 +1,176 @@ package flagset import ( + "math/big" "testing" "time" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestFlagsetBool(t *testing.T) { + t.Parallel() + f := NewFlagSet("") - value := false + value := true f.BoolFlag(&BoolFlag{ Name: "flag", Value: &value, }) - assert.NoError(t, f.Parse([]string{"--flag", "true"})) - assert.Equal(t, value, true) + // Parse no value, should have default (of datatype) + require.NoError(t, f.Parse([]string{})) + require.Equal(t, false, value) + + // Parse --flag true + require.NoError(t, f.Parse([]string{"--flag", "true"})) + require.Equal(t, true, value) + + // Parse --flag=true + require.NoError(t, f.Parse([]string{"--flag=true"})) + require.Equal(t, true, value) + + // Parse --flag false: won't parse false + require.NoError(t, f.Parse([]string{"--flag", "false"})) + require.Equal(t, true, value) + + // Parse --flag=false + require.NoError(t, f.Parse([]string{"--flag=false"})) + require.Equal(t, false, value) + + // Parse --flag + require.NoError(t, f.Parse([]string{"--flag"})) + require.Equal(t, true, value) +} + +func TestFlagsetString(t *testing.T) { + t.Parallel() + + f := NewFlagSet("") + + value := "hello" + f.StringFlag(&StringFlag{ + Name: "flag", + Value: &value, + }) + + // Parse no value, should have default + require.NoError(t, f.Parse([]string{})) + require.Equal(t, "", value) + + // Parse --flag value + require.NoError(t, f.Parse([]string{"--flag", "value"})) + require.Equal(t, "value", value) + + // Parse --flag "" + require.NoError(t, f.Parse([]string{"--flag", ""})) + require.Equal(t, "", value) + + // Parse --flag=newvalue + require.NoError(t, f.Parse([]string{"--flag=newvalue"})) + require.Equal(t, "newvalue", value) + + // Parse --flag: should fail due to no args + require.Error(t, f.Parse([]string{"--flag"})) +} + +func TestFlagsetInt(t *testing.T) { + t.Parallel() + + f := NewFlagSet("") + + value := 10 + f.IntFlag(&IntFlag{ + Name: "flag", + Value: &value, + }) + + // Parse no value, should have default + require.NoError(t, f.Parse([]string{})) + require.Equal(t, 0, value) + + // Parse --flag 20 + require.NoError(t, f.Parse([]string{"--flag", "20"})) + require.Equal(t, 20, value) + + // Parse --flag 0 + require.NoError(t, f.Parse([]string{"--flag", "0"})) + require.Equal(t, 0, value) + + // Parse --flag=30 + require.NoError(t, f.Parse([]string{"--flag=30"})) + require.Equal(t, 30, value) + + // Parse --flag: should fail due to no args + require.Error(t, f.Parse([]string{"--flag"})) +} + +func TestFlagsetFloat64(t *testing.T) { + t.Parallel() + + f := NewFlagSet("") + + value := 10.0 + f.Float64Flag(&Float64Flag{ + Name: "flag", + Value: &value, + }) + + // Parse no value, should have default + require.NoError(t, f.Parse([]string{})) + require.Equal(t, 0.0, value) + + // Parse --flag 20 + require.NoError(t, f.Parse([]string{"--flag", "20.1"})) + require.Equal(t, 20.1, value) + + // Parse --flag 0 + require.NoError(t, f.Parse([]string{"--flag", "0"})) + require.Equal(t, 0.0, value) + + // Parse --flag 0.0 + require.NoError(t, f.Parse([]string{"--flag", "0.0"})) + require.Equal(t, 0.0, value) + + // Parse --flag=30.1 + require.NoError(t, f.Parse([]string{"--flag=30.1"})) + require.Equal(t, 30.1, value) + + // Parse --flag: should fail due to no args + require.Error(t, f.Parse([]string{"--flag"})) +} + +func TestFlagsetBigInt(t *testing.T) { + t.Parallel() + + f := NewFlagSet("") + + value := big.NewInt(0) + f.BigIntFlag(&BigIntFlag{ + Name: "flag", + Value: value, + }) + + // Parse no value, should have initial value (0 here) + require.NoError(t, f.Parse([]string{})) + require.Equal(t, big.NewInt(0), value) + + // Parse --flag 20 + require.NoError(t, f.Parse([]string{"--flag", "20"})) + require.Equal(t, big.NewInt(20), value) + + // Parse --flag=30 + require.NoError(t, f.Parse([]string{"--flag=30"})) + require.Equal(t, big.NewInt(30), value) + + // Parse --flag: should fail due to no args + require.Error(t, f.Parse([]string{"--flag"})) } func TestFlagsetSliceString(t *testing.T) { + t.Parallel() + f := NewFlagSet("") value := []string{"a", "b", "c"} @@ -30,13 +180,25 @@ func TestFlagsetSliceString(t *testing.T) { Default: value, }) - assert.NoError(t, f.Parse([]string{})) - assert.Equal(t, value, []string{"a", "b", "c"}) - assert.NoError(t, f.Parse([]string{"--flag", "a,b"})) - assert.Equal(t, value, []string{"a", "b"}) + // Parse no value, should have initial value + require.NoError(t, f.Parse([]string{})) + require.Equal(t, []string{"a", "b", "c"}, value) + + // Parse --flag a,b + require.NoError(t, f.Parse([]string{"--flag", "a,b"})) + require.Equal(t, []string{"a", "b"}, value) + + // Parse --flag "" + require.NoError(t, f.Parse([]string{"--flag", ""})) + require.Equal(t, []string(nil), value) + + // Parse --flag: should fail due to no args + require.Error(t, f.Parse([]string{"--flag"})) } func TestFlagsetDuration(t *testing.T) { + t.Parallel() + f := NewFlagSet("") value := time.Duration(0) @@ -45,11 +207,25 @@ func TestFlagsetDuration(t *testing.T) { Value: &value, }) - assert.NoError(t, f.Parse([]string{"--flag", "1m"})) - assert.Equal(t, value, 1*time.Minute) + // Parse no value, should have initial value + require.NoError(t, f.Parse([]string{})) + require.Equal(t, time.Duration(0), value) + + // Parse --flag 1m + require.NoError(t, f.Parse([]string{"--flag", "1m"})) + require.Equal(t, time.Minute, value) + + // Parse --flag=1h + require.NoError(t, f.Parse([]string{"--flag=1h"})) + require.Equal(t, time.Hour, value) + + // Parse --flag: should fail due to no args + require.Error(t, f.Parse([]string{"--flag"})) } func TestFlagsetMapString(t *testing.T) { + t.Parallel() + f := NewFlagSet("") value := map[string]string{} @@ -58,6 +234,18 @@ func TestFlagsetMapString(t *testing.T) { Value: &value, }) - assert.NoError(t, f.Parse([]string{"--flag", "a=b,c=d"})) - assert.Equal(t, value, map[string]string{"a": "b", "c": "d"}) + // Parse no value, should have initial value + require.NoError(t, f.Parse([]string{})) + require.Equal(t, map[string]string{}, value) + + // Parse --flag a=b,c=d + require.NoError(t, f.Parse([]string{"--flag", "a=b,c=d"})) + require.Equal(t, map[string]string{"a": "b", "c": "d"}, value) + + // Parse --flag=x=y + require.NoError(t, f.Parse([]string{"--flag=x=y"})) + require.Equal(t, map[string]string{"x": "y"}, value) + + // Parse --flag: should fail due to no args + require.Error(t, f.Parse([]string{"--flag"})) } diff --git a/internal/cli/server/command.go b/internal/cli/server/command.go index 4d7ab6f108..817c09b670 100644 --- a/internal/cli/server/command.go +++ b/internal/cli/server/command.go @@ -35,7 +35,7 @@ func (c *Command) MarkDown() string { items := []string{ "# Server", "The ```bor server``` command runs the Bor client.", - c.Flags().MarkDown(), + c.Flags(nil).MarkDown(), } return strings.Join(items, "\n\n") @@ -46,7 +46,7 @@ func (c *Command) Help() string { return `Usage: bor [options] Run the Bor server. - ` + c.Flags().Help() + ` + c.Flags(nil).Help() } // Synopsis implements the cli.Command interface @@ -54,40 +54,69 @@ func (c *Command) Synopsis() string { return "Run the Bor server" } -func (c *Command) extractFlags(args []string) error { - config := *DefaultConfig() - - flags := c.Flags() - if err := flags.Parse(args); err != nil { - c.UI.Error(err.Error()) - c.config = &config +// checkConfigFlag checks if the config flag is set or not. If set, +// it returns the value else an empty string. +func checkConfigFlag(args []string) string { + for i := 0; i < len(args); i++ { + arg := args[i] + + // Check for single or double dashes + if strings.HasPrefix(arg, "-config") || strings.HasPrefix(arg, "--config") { + parts := strings.SplitN(arg, "=", 2) + if len(parts) == 2 { + return parts[1] + } - return err + // If there's no equal sign, check the next argument + if i+1 < len(args) { + return args[i+1] + } + } } - // TODO: Check if this can be removed or not - // read cli flags - if err := config.Merge(c.cliConfig); err != nil { - c.UI.Error(err.Error()) - c.config = &config + return "" +} - return err - } - // read if config file is provided, this will overwrite the cli flags, if provided - if c.configFile != "" { - log.Warn("Config File provided, this will overwrite the cli flags", "path", c.configFile) +func (c *Command) extractFlags(args []string) error { + // Check if config file is provided or not + configFilePath := checkConfigFlag(args) + + if configFilePath != "" { + log.Info("Reading config file", "path", configFilePath) - cfg, err := readConfigFile(c.configFile) + // Parse the config file + cfg, err := readConfigFile(configFilePath) if err != nil { c.UI.Error(err.Error()) - c.config = &config return err } - if err := config.Merge(cfg); err != nil { + log.Warn("Config set via config file will be overridden by cli flags") + + // Initialse a flagset based on the config created above + flags := c.Flags(cfg) + + // Check for explicit cli args + cmd := Command{} // use a new variable to keep the original config intact + + cliFlags := cmd.Flags(nil) + if err := cliFlags.Parse(args); err != nil { + c.UI.Error(err.Error()) + + return err + } + + // Get the list of flags set explicitly + names, values := cliFlags.Visit() + + // Set these flags using the flagset created earlier + flags.UpdateValue(names, values) + } else { + flags := c.Flags(nil) + + if err := flags.Parse(args); err != nil { c.UI.Error(err.Error()) - c.config = &config return err } @@ -95,33 +124,33 @@ func (c *Command) extractFlags(args []string) error { // nolint: nestif // check for log-level and verbosity here - if c.configFile != "" { - data, _ := toml.LoadFile(c.configFile) + if configFilePath != "" { + data, _ := toml.LoadFile(configFilePath) if data.Has("verbosity") && data.Has("log-level") { log.Warn("Config contains both, verbosity and log-level, log-level will be deprecated soon. Use verbosity only.", "using", data.Get("verbosity")) } else if !data.Has("verbosity") && data.Has("log-level") { log.Warn("Config contains log-level only, note that log-level will be deprecated soon. Use verbosity instead.", "using", data.Get("log-level")) - config.Verbosity = VerbosityStringToInt(strings.ToLower(data.Get("log-level").(string))) + c.cliConfig.Verbosity = VerbosityStringToInt(strings.ToLower(data.Get("log-level").(string))) } } else { tempFlag := 0 for _, val := range args { - if (strings.HasPrefix(val, "-verbosity") || strings.HasPrefix(val, "--verbosity")) && config.LogLevel != "" { + if (strings.HasPrefix(val, "-verbosity") || strings.HasPrefix(val, "--verbosity")) && c.cliConfig.LogLevel != "" { tempFlag = 1 break } } if tempFlag == 1 { - log.Warn("Both, verbosity and log-level flags are provided, log-level will be deprecated soon. Use verbosity only.", "using", config.Verbosity) - } else if tempFlag == 0 && config.LogLevel != "" { - log.Warn("Only log-level flag is provided, note that log-level will be deprecated soon. Use verbosity instead.", "using", config.LogLevel) - config.Verbosity = VerbosityStringToInt(strings.ToLower(config.LogLevel)) + log.Warn("Both, verbosity and log-level flags are provided, log-level will be deprecated soon. Use verbosity only.", "using", c.cliConfig.Verbosity) + } else if tempFlag == 0 && c.cliConfig.LogLevel != "" { + log.Warn("Only log-level flag is provided, note that log-level will be deprecated soon. Use verbosity instead.", "using", c.cliConfig.LogLevel) + c.cliConfig.Verbosity = VerbosityStringToInt(strings.ToLower(c.cliConfig.LogLevel)) } } - c.config = &config + c.config = c.cliConfig return nil } diff --git a/internal/cli/server/command_test.go b/internal/cli/server/command_test.go index ab28de5ee6..9f5468f3d3 100644 --- a/internal/cli/server/command_test.go +++ b/internal/cli/server/command_test.go @@ -8,43 +8,135 @@ import ( "github.com/stretchr/testify/require" ) -func TestFlags(t *testing.T) { +// TestFlagsWithoutConfig tests all types of flags passed only +// via cli args. +func TestFlagsWithoutConfig(t *testing.T) { t.Parallel() var c Command args := []string{ - "--txpool.rejournal", "30m0s", - "--txpool.lifetime", "30m0s", - "--miner.gasprice", "20000000000", - "--gpo.maxprice", "70000000000", - "--gpo.ignoreprice", "1", - "--cache.trie.rejournal", "40m0s", - "--dev", - "--dev.period", "2", + "--identity", "", "--datadir", "./data", - "--maxpeers", "30", + "--verbosity", "3", + "--rpc.batchlimit", "0", + "--snapshot", + "--bor.logs=false", "--eth.requiredblocks", "a=b", - "--http.api", "eth,web3,bor", + "--miner.gasprice", "30000000000", + "--miner.recommit", "20s", + "--rpc.evmtimeout", "5s", + "--rpc.txfeecap", "6.0", + "--http.api", "eth,bor", + "--ws.api", "", + "--gpo.maxprice", "5000000000000", } + err := c.extractFlags(args) require.NoError(t, err) - txRe, _ := time.ParseDuration("30m0s") - txLt, _ := time.ParseDuration("30m0s") - caRe, _ := time.ParseDuration("40m0s") + recommit, _ := time.ParseDuration("20s") + evmTimeout, _ := time.ParseDuration("5s") + require.Equal(t, c.config.Identity, "") require.Equal(t, c.config.DataDir, "./data") - require.Equal(t, c.config.Developer.Enabled, true) - require.Equal(t, c.config.Developer.Period, uint64(2)) - require.Equal(t, c.config.TxPool.Rejournal, txRe) - require.Equal(t, c.config.TxPool.LifeTime, txLt) - require.Equal(t, c.config.Sealer.GasPrice, big.NewInt(20000000000)) - require.Equal(t, c.config.Gpo.MaxPrice, big.NewInt(70000000000)) - require.Equal(t, c.config.Gpo.IgnorePrice, big.NewInt(1)) - require.Equal(t, c.config.Cache.Rejournal, caRe) - require.Equal(t, c.config.P2P.MaxPeers, uint64(30)) + require.Equal(t, c.config.Verbosity, 3) + require.Equal(t, c.config.RPCBatchLimit, uint64(0)) + require.Equal(t, c.config.Snapshot, true) + require.Equal(t, c.config.BorLogs, false) require.Equal(t, c.config.RequiredBlocks, map[string]string{"a": "b"}) - require.Equal(t, c.config.JsonRPC.Http.API, []string{"eth", "web3", "bor"}) + require.Equal(t, c.config.Sealer.GasPrice, big.NewInt(30000000000)) + require.Equal(t, c.config.Sealer.Recommit, recommit) + require.Equal(t, c.config.JsonRPC.RPCEVMTimeout, evmTimeout) + require.Equal(t, c.config.JsonRPC.Http.API, []string{"eth", "bor"}) + require.Equal(t, c.config.JsonRPC.Ws.API, []string(nil)) + require.Equal(t, c.config.Gpo.MaxPrice, big.NewInt(5000000000000)) +} + +// TestFlagsWithoutConfig tests all types of flags passed only +// via config file. +func TestFlagsWithConfig(t *testing.T) { + t.Parallel() + + var c Command + + args := []string{ + "--config", "./testdata/test.toml", + } + + err := c.extractFlags(args) + + require.NoError(t, err) + + recommit, _ := time.ParseDuration("20s") + evmTimeout, _ := time.ParseDuration("5s") + + require.Equal(t, c.config.Identity, "") + require.Equal(t, c.config.DataDir, "./data") + require.Equal(t, c.config.Verbosity, 3) + require.Equal(t, c.config.RPCBatchLimit, uint64(0)) + require.Equal(t, c.config.Snapshot, true) + require.Equal(t, c.config.BorLogs, false) + require.Equal(t, c.config.RequiredBlocks, + map[string]string{ + "31000000": "0x2087b9e2b353209c2c21e370c82daa12278efd0fe5f0febe6c29035352cf050e", + "32000000": "0x875500011e5eecc0c554f95d07b31cf59df4ca2505f4dbbfffa7d4e4da917c68", + }, + ) + require.Equal(t, c.config.Sealer.GasPrice, big.NewInt(30000000000)) + require.Equal(t, c.config.Sealer.Recommit, recommit) + require.Equal(t, c.config.JsonRPC.RPCEVMTimeout, evmTimeout) + require.Equal(t, c.config.JsonRPC.Http.API, []string{"eth", "bor"}) + require.Equal(t, c.config.JsonRPC.Ws.API, []string{""}) + require.Equal(t, c.config.Gpo.MaxPrice, big.NewInt(5000000000000)) +} + +// TestFlagsWithConfig tests all types of flags passed via both +// config file and cli args. The cli args should overwrite the +// value of flag. +func TestFlagsWithConfigAndFlags(t *testing.T) { + t.Parallel() + + var c Command + + // Set the config and also override + args := []string{ + "--config", "./testdata/test.toml", + "--identity", "Anon", + "--datadir", "", + "--verbosity", "0", + "--rpc.batchlimit", "5", + "--snapshot=false", + "--bor.logs=true", + "--eth.requiredblocks", "x=y", + "--miner.gasprice", "60000000000", + "--miner.recommit", "30s", + "--rpc.evmtimeout", "0s", + "--rpc.txfeecap", "0", + "--http.api", "", + "--ws.api", "eth,bor,web3", + "--gpo.maxprice", "0", + } + + err := c.extractFlags(args) + + require.NoError(t, err) + + recommit, _ := time.ParseDuration("30s") + evmTimeout, _ := time.ParseDuration("0s") + + require.Equal(t, c.config.Identity, "Anon") + require.Equal(t, c.config.DataDir, "") + require.Equal(t, c.config.Verbosity, 0) + require.Equal(t, c.config.RPCBatchLimit, uint64(5)) + require.Equal(t, c.config.Snapshot, false) + require.Equal(t, c.config.BorLogs, true) + require.Equal(t, c.config.RequiredBlocks, map[string]string{"x": "y"}) + require.Equal(t, c.config.Sealer.GasPrice, big.NewInt(60000000000)) + require.Equal(t, c.config.Sealer.Recommit, recommit) + require.Equal(t, c.config.JsonRPC.RPCEVMTimeout, evmTimeout) + require.Equal(t, c.config.JsonRPC.Http.API, []string(nil)) + require.Equal(t, c.config.JsonRPC.Ws.API, []string{"eth", "bor", "web3"}) + require.Equal(t, c.config.Gpo.MaxPrice, big.NewInt(0)) } diff --git a/internal/cli/server/config_legacy_test.go b/internal/cli/server/config_legacy_test.go index 8ac61b094f..e0b1a76577 100644 --- a/internal/cli/server/config_legacy_test.go +++ b/internal/cli/server/config_legacy_test.go @@ -15,21 +15,23 @@ func TestConfigLegacy(t *testing.T) { testConfig := DefaultConfig() + testConfig.Identity = "" testConfig.DataDir = "./data" - testConfig.Snapshot = false + testConfig.Verbosity = 3 + testConfig.RPCBatchLimit = 0 + testConfig.Snapshot = true + testConfig.BorLogs = false testConfig.RequiredBlocks = map[string]string{ "31000000": "0x2087b9e2b353209c2c21e370c82daa12278efd0fe5f0febe6c29035352cf050e", "32000000": "0x875500011e5eecc0c554f95d07b31cf59df4ca2505f4dbbfffa7d4e4da917c68", } - testConfig.P2P.MaxPeers = 30 - testConfig.TxPool.Locals = []string{} - testConfig.TxPool.LifeTime = time.Second - testConfig.Sealer.Enabled = true - testConfig.Sealer.GasCeil = 30000000 - testConfig.Sealer.GasPrice = big.NewInt(1000000000) - testConfig.Gpo.IgnorePrice = big.NewInt(4) - testConfig.Cache.Cache = 1024 - testConfig.Cache.Rejournal = time.Second + testConfig.Sealer.GasPrice = big.NewInt(30000000000) + testConfig.Sealer.Recommit = 20 * time.Second + testConfig.JsonRPC.RPCEVMTimeout = 5 * time.Second + testConfig.JsonRPC.TxFeeCap = 6.0 + testConfig.JsonRPC.Http.API = []string{"eth", "bor"} + testConfig.JsonRPC.Ws.API = []string{""} + testConfig.Gpo.MaxPrice = big.NewInt(5000000000000) assert.Equal(t, expectedConfig, testConfig) } diff --git a/internal/cli/server/flags.go b/internal/cli/server/flags.go index 1edacc9ac0..12995977ff 100644 --- a/internal/cli/server/flags.go +++ b/internal/cli/server/flags.go @@ -4,8 +4,12 @@ import ( "github.com/ethereum/go-ethereum/internal/cli/flagset" ) -func (c *Command) Flags() *flagset.Flagset { - c.cliConfig = DefaultConfig() +func (c *Command) Flags(config *Config) *flagset.Flagset { + if config != nil { + c.cliConfig = config + } else { + c.cliConfig = DefaultConfig() + } f := flagset.NewFlagSet("server") diff --git a/internal/cli/server/testdata/test.toml b/internal/cli/server/testdata/test.toml index 4ccc644ee9..522322b2a3 100644 --- a/internal/cli/server/testdata/test.toml +++ b/internal/cli/server/testdata/test.toml @@ -1,25 +1,25 @@ +identity = "" datadir = "./data" -snapshot = false +verbosity = 3 +"rpc.batchlimit" = 0 +snapshot = true +"bor.logs" = false ["eth.requiredblocks"] "31000000" = "0x2087b9e2b353209c2c21e370c82daa12278efd0fe5f0febe6c29035352cf050e" "32000000" = "0x875500011e5eecc0c554f95d07b31cf59df4ca2505f4dbbfffa7d4e4da917c68" -[p2p] -maxpeers = 30 - -[txpool] -locals = [] -lifetime = "1s" - [miner] -mine = true -gaslimit = 30000000 -gasprice = "1000000000" + gasprice = "30000000000" + recommit = "20s" -[gpo] -ignoreprice = "4" +[jsonrpc] + evmtimeout = "5s" + txfeecap = 6.0 + [jsonrpc.http] + api = ["eth", "bor"] + [jsonrpc.ws] + api = [""] -[cache] -cache = 1024 -rejournal = "1s" +[gpo] + maxprice = "5000000000000" diff --git a/scripts/getconfig.go b/scripts/getconfig.go index 0185588760..9a7476c0eb 100644 --- a/scripts/getconfig.go +++ b/scripts/getconfig.go @@ -699,7 +699,7 @@ func main() { args, ignoreForNow := beautifyArgs(args) c := server.Command{} - flags := c.Flags() + flags := c.Flags(nil) allFlags := flags.GetAllFlags() flagsToCheck := getFlagsToCheck(args)