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.

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/geth: add check func to validate state scheme #2067

Merged
merged 2 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions cmd/geth/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ func loadBaseConfig(ctx *cli.Context) gethConfig {
utils.Fatalf("%v", err)
}
}
if !utils.ValidateStateScheme(cfg.Eth.StateScheme) {
utils.Fatalf("invalid state scheme param in config: %s", cfg.Eth.StateScheme)
}

// Apply flags.
utils.SetNodeConfig(ctx, &cfg.Node)
Expand Down
63 changes: 60 additions & 3 deletions cmd/utils/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ import (
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/crypto/kzg4844"
"github.com/ethereum/go-ethereum/eth"

"github.com/ethereum/go-ethereum/eth/downloader"
"github.com/ethereum/go-ethereum/eth/ethconfig"
"github.com/ethereum/go-ethereum/eth/filters"
Expand Down Expand Up @@ -1941,7 +1940,7 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) {
}
// Parse state scheme, abort the process if it's not compatible.
chaindb := tryMakeReadOnlyDatabase(ctx, stack)
scheme, err := ParseStateScheme(ctx, chaindb)
scheme, err := ResolveStateScheme(ctx, cfg.StateScheme, chaindb)
chaindb.Close()
if err != nil {
Fatalf("%v", err)
Expand Down Expand Up @@ -2487,6 +2486,52 @@ func MakeConsolePreloads(ctx *cli.Context) []string {
return preloads
}

// ResolveStateScheme resolve state scheme from CLI flag, config file and persistent state.
// The differences between ResolveStateScheme and ParseStateScheme are:
// - ResolveStateScheme adds config to compare with CLI and persistent state to ensure correctness.
// - ResolveStateScheme is only used in SetEthConfig function.
//
// 1. If config isn't provided, write hash mode to config by default, so in current function, config is nonempty.
// 2. If persistent state and cli is empty, use config param.
// 3. If persistent state is empty, provide CLI flag and config, choose CLI to return.
// 4. If persistent state is nonempty and CLI isn't provided, persistent state should be equal to config.
// 5. If all three items are provided: if any two of the three are not equal, return error.
func ResolveStateScheme(ctx *cli.Context, stateSchemeCfg string, disk ethdb.Database) (string, error) {
stored := rawdb.ReadStateScheme(disk)
if stored == "" {
// there is no persistent state data in disk db(e.g. geth init)
if !ctx.IsSet(StateSchemeFlag.Name) {
log.Info("State scheme set by config", "scheme", stateSchemeCfg)
return stateSchemeCfg, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need judge if the stateSchemeCfg is not setted

Copy link
Contributor Author

@sysvm sysvm Dec 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stateSchemeCfg is impossible to be empty in ResolveStateScheme function, because there is a default value in caller function loadBaseConfig.

}
// if both CLI flag and config are set, choose CLI
scheme := ctx.String(StateSchemeFlag.Name)
if !ValidateStateScheme(scheme) {
return "", fmt.Errorf("invalid state scheme param in CLI: %s", scheme)
}
log.Info("State scheme set by CLI", "scheme", scheme)
return scheme, nil
}
if !ctx.IsSet(StateSchemeFlag.Name) {
if stored != stateSchemeCfg {
return "", fmt.Errorf("incompatible state scheme, stored: %s, config: %s", stored, stateSchemeCfg)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should not return error if stateSchemeCfg is empty

Copy link
Contributor Author

@sysvm sysvm Dec 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto. stateSchemeCfg is impossible to be empty in ResolveStateScheme function.

}
log.Info("State scheme set to already existing", "scheme", stored)
return stored, nil
}
scheme := ctx.String(StateSchemeFlag.Name)
if !ValidateStateScheme(scheme) {
return "", fmt.Errorf("invalid state scheme param in CLI: %s", scheme)
}
// if there is persistent state data in disk db, and CLI flag, config are set,
// when they all are different, return error
if scheme != stored || scheme != stateSchemeCfg || stored != stateSchemeCfg {
return "", fmt.Errorf("incompatible state scheme, stored: %s, config: %s, CLI: %s", stored, stateSchemeCfg, scheme)
}
log.Info("All are provided, state scheme set to already existing", "scheme", stored)
return stored, nil
}

// ParseStateScheme resolves scheme identifier from CLI flag. If the provided
// state scheme is not compatible with the one of persistent scheme, an error
// will be returned.
Expand All @@ -2506,7 +2551,7 @@ func ParseStateScheme(ctx *cli.Context, 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 rawdb.HashScheme, nil
}
log.Info("State scheme set to already existing", "scheme", stored)
Expand All @@ -2515,6 +2560,9 @@ func ParseStateScheme(ctx *cli.Context, disk ethdb.Database) (string, error) {
// If state scheme is specified, ensure it's compatible with
// persistent state.
scheme := ctx.String(StateSchemeFlag.Name)
if !ValidateStateScheme(scheme) {
return "", fmt.Errorf("invalid state scheme param in CLI: %s", scheme)
}
if stored == "" || scheme == stored {
log.Info("State scheme set by user", "scheme", scheme)
return scheme, nil
Expand Down Expand Up @@ -2545,3 +2593,12 @@ func MakeTrieDatabase(ctx *cli.Context, disk ethdb.Database, preimage bool, read
}
return trie.NewDatabase(disk, config)
}

// ValidateStateScheme used to check state scheme whether is valid.
// Valid state scheme: hash and path.
func ValidateStateScheme(stateScheme string) bool {
if stateScheme == rawdb.HashScheme || stateScheme == rawdb.PathScheme {
return true
}
return false
}
33 changes: 33 additions & 0 deletions cmd/utils/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ package utils
import (
"reflect"
"testing"

"github.com/ethereum/go-ethereum/core/rawdb"
)

func Test_SplitTagsFlag(t *testing.T) {
Expand Down Expand Up @@ -62,3 +64,34 @@ func Test_SplitTagsFlag(t *testing.T) {
})
}
}

func TestValidateStateScheme(t *testing.T) {
tests := []struct {
name string
arg string
wantResult bool
}{
{
name: "hash scheme",
arg: rawdb.HashScheme,
wantResult: true,
},
{
name: "path scheme",
arg: rawdb.PathScheme,
wantResult: true,
},
{
name: "invalid scheme",
arg: "mockScheme",
wantResult: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := ValidateStateScheme(tt.arg); got != tt.wantResult {
t.Errorf("ValidateStateScheme() = %v, want %v", got, tt.wantResult)
}
})
}
}