Skip to content

cmd/geth: add check func to validate state scheme#2067

Merged
sysvm merged 2 commits intobnb-chain:developfrom
sysvm:fix-pbssconfig
Dec 18, 2023
Merged

cmd/geth: add check func to validate state scheme#2067
sysvm merged 2 commits intobnb-chain:developfrom
sysvm:fix-pbssconfig

Conversation

@sysvm
Copy link
Copy Markdown
Contributor

@sysvm sysvm commented Dec 14, 2023

Description

Provide explicit for which state scheme geth can use to start or init. And add check function to validate provided state scheme.

Rationale

Users may add StateScheme = "path" in config.toml and want to start geth without persistent state in disk, but geth is started in hash mode. For solving these cases, add a function, obey the following rules:

  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.

Example

  1. nohup ./geth --config config.toml --cache 8000 --rpc.allow-unprotected-txs --history.transactions 0 >> nohup.out 2>&1& and add StateScheme = "path" in config.toml, geth is started in path mode.

Changes

Notable changes:

  • N/A

@sysvm sysvm added the r4r ready for review label Dec 14, 2023
joey0612
joey0612 previously approved these changes Dec 14, 2023
@joey0612
Copy link
Copy Markdown
Contributor

LGTM

// 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
Copy Markdown
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
Copy Markdown
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 !ctx.IsSet(StateSchemeFlag.Name) {
if stored != stateSchemeCfg {
return "", fmt.Errorf("incompatible state scheme, stored: %s, config: %s", stored, stateSchemeCfg)
Copy link
Copy Markdown
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
Copy Markdown
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.

flywukong
flywukong previously approved these changes Dec 15, 2023
Copy link
Copy Markdown

@RenRick RenRick left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@flywukong flywukong left a comment

Choose a reason for hiding this comment

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

LGTM

@sysvm sysvm merged commit 07c46ab into bnb-chain:develop Dec 18, 2023
@sysvm sysvm deleted the fix-pbssconfig branch December 18, 2023 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

r4r ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Node can't read StateScheme entries in configuration file

4 participants