Skip to content

Commit

Permalink
cmd: optimize parse state scheme in cli and config (#2220)
Browse files Browse the repository at this point in the history
  • Loading branch information
sysvm authored Feb 20, 2024
1 parent 40cae45 commit 5378df3
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 188 deletions.
65 changes: 6 additions & 59 deletions cmd/utils/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
package utils

import (
"bufio"
"context"
"crypto/ecdsa"
"encoding/hex"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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 ""
}
128 changes: 0 additions & 128 deletions cmd/utils/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
})
}
}
2 changes: 1 addition & 1 deletion core/rawdb/accessors_trie.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 5378df3

Please sign in to comment.