From 5378df37022e6a25c9568b54ad136a6ccf7e34bb Mon Sep 17 00:00:00 2001 From: VM <112189277+sysvm@users.noreply.github.com> Date: Tue, 20 Feb 2024 17:22:27 +0800 Subject: [PATCH] cmd: optimize parse state scheme in cli and config (#2220) --- cmd/utils/flags.go | 65 ++---------------- cmd/utils/flags_test.go | 128 ----------------------------------- core/rawdb/accessors_trie.go | 2 +- 3 files changed, 7 insertions(+), 188 deletions(-) diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index 3b79a54029..e7dac8e4ce 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -18,7 +18,6 @@ package utils import ( - "bufio" "context" "crypto/ecdsa" "encoding/hex" @@ -1884,7 +1883,7 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) { if ctx.IsSet(StateHistoryFlag.Name) { cfg.StateHistory = ctx.Uint64(StateHistoryFlag.Name) } - scheme, err := CompareStateSchemeCLIWithConfig(ctx) + scheme, err := ParseCLIAndConfigStateScheme(ctx.String(StateSchemeFlag.Name), cfg.StateScheme) if err != nil { Fatalf("%v", err) } @@ -2353,11 +2352,7 @@ func MakeChain(ctx *cli.Context, stack *node.Node, readonly bool) (*core.BlockCh if gcmode := ctx.String(GCModeFlag.Name); gcmode != "full" && gcmode != "archive" { Fatalf("--%s must be either 'full' or 'archive'", GCModeFlag.Name) } - provided, err := CompareStateSchemeCLIWithConfig(ctx) - if err != nil { - Fatalf("%v", err) - } - scheme, err := rawdb.ParseStateScheme(provided, chainDb) + scheme, err := rawdb.ParseStateScheme(ctx.String(StateSchemeFlag.Name), chainDb) if err != nil { Fatalf("%v", err) } @@ -2425,11 +2420,7 @@ func MakeTrieDatabase(ctx *cli.Context, disk ethdb.Database, preimage bool, read config := &trie.Config{ Preimages: preimage, } - provided, err := CompareStateSchemeCLIWithConfig(ctx) - if err != nil { - Fatalf("%v", err) - } - scheme, err := rawdb.ParseStateScheme(provided, disk) + scheme, err := rawdb.ParseStateScheme(ctx.String(StateSchemeFlag.Name), disk) if err != nil { Fatalf("%v", err) } @@ -2448,27 +2439,15 @@ func MakeTrieDatabase(ctx *cli.Context, disk ethdb.Database, preimage bool, read return trie.NewDatabase(disk, config) } -// CompareStateSchemeCLIWithConfig compare state scheme in CLI with config whether are equal. -func CompareStateSchemeCLIWithConfig(ctx *cli.Context) (string, error) { - var ( - cfgScheme string - err error - ) - if file := ctx.String("config"); file != "" { - // we don't validate cfgScheme because it's already checked in cmd/geth/loadBaseConfig - if cfgScheme, err = scanConfigForStateScheme(file); err != nil { - log.Error("Failed to parse config file", "error", err) - return "", err - } - } - if !ctx.IsSet(StateSchemeFlag.Name) { +// ParseCLIAndConfigStateScheme parses state scheme in CLI and config. +func ParseCLIAndConfigStateScheme(cliScheme, cfgScheme string) (string, error) { + if cliScheme == "" { if cfgScheme != "" { log.Info("Use config state scheme", "config", cfgScheme) } return cfgScheme, nil } - cliScheme := ctx.String(StateSchemeFlag.Name) if !rawdb.ValidateStateScheme(cliScheme) { return "", fmt.Errorf("invalid state scheme in CLI: %s", cliScheme) } @@ -2478,35 +2457,3 @@ func CompareStateSchemeCLIWithConfig(ctx *cli.Context) (string, error) { } return "", fmt.Errorf("incompatible state scheme, CLI: %s, config: %s", cliScheme, cfgScheme) } - -func scanConfigForStateScheme(file string) (string, error) { - f, err := os.Open(file) - if err != nil { - return "", err - } - defer f.Close() - - scanner := bufio.NewScanner(f) - targetStr := "StateScheme" - for scanner.Scan() { - line := scanner.Text() - if strings.Contains(line, targetStr) { - return indexStateScheme(line), nil - } - } - - if err = scanner.Err(); err != nil { - return "", err - } - return "", nil -} - -func indexStateScheme(str string) string { - i1 := strings.Index(str, "\"") - i2 := strings.LastIndex(str, "\"") - - if i1 != -1 && i2 != -1 && i1 < i2 { - return str[i1+1 : i2] - } - return "" -} diff --git a/cmd/utils/flags_test.go b/cmd/utils/flags_test.go index f8bcd96cf8..adfdd0903e 100644 --- a/cmd/utils/flags_test.go +++ b/cmd/utils/flags_test.go @@ -18,13 +18,8 @@ package utils import ( - "os" "reflect" "testing" - - "github.com/stretchr/testify/assert" - - "github.com/ethereum/go-ethereum/core/rawdb" ) func Test_SplitTagsFlag(t *testing.T) { @@ -67,126 +62,3 @@ func Test_SplitTagsFlag(t *testing.T) { }) } } - -func Test_parseConfig(t *testing.T) { - tests := []struct { - name string - fn func() string - wantedResult string - wantedIsErr bool - wantedErrStr string - }{ - { - name: "path", - fn: func() string { - tomlString := `[Eth]NetworkId = 56StateScheme = "path"` - return createTempTomlFile(t, tomlString) - }, - wantedResult: rawdb.PathScheme, - wantedIsErr: false, - wantedErrStr: "", - }, - { - name: "hash", - fn: func() string { - tomlString := `[Eth]NetworkId = 56StateScheme = "hash"` - return createTempTomlFile(t, tomlString) - }, - wantedResult: rawdb.HashScheme, - wantedIsErr: false, - wantedErrStr: "", - }, - { - name: "empty state scheme", - fn: func() string { - tomlString := `[Eth]NetworkId = 56StateScheme = ""` - return createTempTomlFile(t, tomlString) - }, - wantedResult: "", - wantedIsErr: false, - wantedErrStr: "", - }, - { - name: "unset state scheme", - fn: func() string { - tomlString := `[Eth]NetworkId = 56` - return createTempTomlFile(t, tomlString) - }, - wantedResult: "", - wantedIsErr: false, - wantedErrStr: "", - }, - { - name: "failed to open file", - fn: func() string { return "" }, - wantedResult: "", - wantedIsErr: true, - wantedErrStr: "open : no such file or directory", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result, err := scanConfigForStateScheme(tt.fn()) - if tt.wantedIsErr { - assert.Contains(t, err.Error(), tt.wantedErrStr) - } else { - assert.Nil(t, err) - } - assert.Equal(t, tt.wantedResult, result) - }) - } -} - -// createTempTomlFile is a helper function to create a temp file with the provided TOML content -func createTempTomlFile(t *testing.T, content string) string { - t.Helper() - - dir := t.TempDir() - file, err := os.CreateTemp(dir, "config.toml") - if err != nil { - t.Fatalf("Unable to create temporary file: %v", err) - } - defer file.Close() - - _, err = file.WriteString(content) - if err != nil { - t.Fatalf("Unable to write to temporary file: %v", err) - } - return file.Name() -} - -func Test_parseString(t *testing.T) { - tests := []struct { - name string - arg string - wantResult string - }{ - { - name: "hash string", - arg: "\"hash\"", - wantResult: rawdb.HashScheme, - }, - { - name: "path string", - arg: "\"path\"", - wantResult: rawdb.PathScheme, - }, - { - name: "empty string", - arg: "", - wantResult: "", - }, - { - name: "empty string", - arg: "\"\"", - wantResult: "", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := indexStateScheme(tt.arg); got != tt.wantResult { - t.Errorf("parseString() = %v, want %v", got, tt.wantResult) - } - }) - } -} diff --git a/core/rawdb/accessors_trie.go b/core/rawdb/accessors_trie.go index 5248fbecab..e392809924 100644 --- a/core/rawdb/accessors_trie.go +++ b/core/rawdb/accessors_trie.go @@ -335,7 +335,7 @@ func ParseStateScheme(provided string, disk ethdb.Database) (string, error) { if stored == "" { // use default scheme for empty database, flip it when // path mode is chosen as default - log.Info("State schema set to default", "scheme", "hash") + log.Info("State scheme set to default", "scheme", "hash") return HashScheme, nil } log.Info("State scheme set to already existing disk db", "scheme", stored)