Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd: optimize parse state scheme in cli and config #2220

Merged
merged 2 commits into from
Feb 20, 2024
Merged

cmd: optimize parse state scheme in cli and config #2220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e01c375
cmd: optimize parse state scheme in cli and config
Feb 2, 2024
51392c8
core: fix parse state scheme log
Feb 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 6 additions & 59 deletions cmd/utils/flags.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading