refactor(config/node)!: Remove genesisHash from config (cometbft#3595)

Closes cometbft#3547 .

This PR removes the `GenesisHash` from the config file because this
field is not meant to be updated by the operator. It was only introduced
to avoid breaking an API and used to pass the genesis hash provided via
cli to the node constructor.


As this feature is going into an unreleased version, we decided to break
the API of `DefaultNewNode`. We maintain the signature of `NewNode` as
this is probably used by the community more and introduce
`NewNodeWithCliParams`.

`NewNodeWithCliParams` allows for passing different parameters from the
cli to the node.

The PR also introduces a new type `CliParams`. At the moment this struct
contains just the `GenesisHash` but it was done this way so that we can
in the future extend it with new parameters without breaking APIs.

---------

Co-authored-by: Daniel <[email protected]>

.changelog/unreleased/breaking-changes/3595-node-go-api-defaultnewnode.md

@@ -0,0 +1,3 @@
+- `[node]` Go API breaking change to `DefaultNewNode`. The function passes 
+`CliParams` to a node now.
+  ([\#3595](https://github.com/cometbft/cometbft/pull/3595))

.changelog/unreleased/breaking-changes/3595-node-go-api-provider.md

@@ -0,0 +1,3 @@
+- `[node]` Go API breaking change to `Provider`. The function takes  
+`CliParams` as a parameter now.
+  ([\#3595](https://github.com/cometbft/cometbft/pull/3595))

.changelog/unreleased/improvements/3595-config-remove-genesishash.md

@@ -0,0 +1,2 @@
+- `[config]` Remove unused `GenesisHash` flag
+  ([\#3595](https://github.com/cometbft/cometbft/pull/3595))

cmd/cometbft/commands/run_node.go

@@ -1,7 +1,6 @@
 package commands
 
 import (
-	"encoding/hex"
 	"fmt"
 
 	"github.com/spf13/cobra"
@@ -10,7 +9,7 @@ import (
 	nm "github.com/cometbft/cometbft/node"
 )
 
-var genesisHash []byte
+var cliParams nm.CliParams
 
 // AddNodeFlags exposes some common configuration options on the command-line
 // These are exposed for convenience of commands embedding a CometBFT node.
@@ -26,7 +25,7 @@ func AddNodeFlags(cmd *cobra.Command) {
 
 	// node flags
 	cmd.Flags().BytesHexVar(
-		&genesisHash,
+		&cliParams.GenesisHash,
 		"genesis_hash",
 		[]byte{},
 		"optional SHA-256 hash of the genesis file")
@@ -90,11 +89,7 @@ func NewRunNodeCmd(nodeProvider nm.Provider) *cobra.Command {
 		Aliases: []string{"node", "run"},
 		Short:   "Run the CometBFT node",
 		RunE: func(_ *cobra.Command, _ []string) error {
-			if len(genesisHash) != 0 {
-				config.Storage.GenesisHash = hex.EncodeToString(genesisHash)
-			}
-
-			n, err := nodeProvider(config, logger)
+			n, err := nodeProvider(config, logger, cliParams)
 			if err != nil {
 				return fmt.Errorf("failed to create node: %w", err)
 			}

config/config.go

@@ -1330,13 +1330,6 @@ type StorageConfig struct {
 	// large multiple of your retain height as it might occur bigger overheads.
 	// 1000 by default.
 	CompactionInterval int64 `mapstructure:"compaction_interval"`
-	// Hex representation of the hash of the genesis file.
-	// This is an optional parameter set when an operator provides
-	// a hash via the command line.
-	// It is used to verify the hash of the actual genesis file.
-	// Note that if the provided has does not match the hash of the genesis file
-	// the node will report an error and not boot.
-	GenesisHash string `mapstructure:"genesis_hash"`
 
 	// The representation of keys in the database.
 	// The current representation of keys in Comet's stores is considered to be v1
@@ -1354,7 +1347,6 @@ func DefaultStorageConfig() *StorageConfig {
 		Pruning:               DefaultPruningConfig(),
 		Compact:               false,
 		CompactionInterval:    1000,
-		GenesisHash:           "",
 		ExperimentalKeyLayout: "v1",
 	}
 }
@@ -1365,7 +1357,6 @@ func TestStorageConfig() *StorageConfig {
 	return &StorageConfig{
 		DiscardABCIResponses: false,
 		Pruning:              TestPruningConfig(),
-		GenesisHash:          "",
 	}
 }
 

config/config.toml.tpl

@@ -504,11 +504,6 @@ compact = {{ .Storage.Compact }}
 # large multiple of your retain height as it might occur bigger overheads.
 compaction_interval = "{{ .Storage.CompactionInterval }}"
 
-# Hash of the Genesis file (as hex string), passed to CometBFT via the command line.
-# If this hash mismatches the hash that CometBFT computes on the genesis file,
-# the node is not able to boot.
-genesis_hash = "{{ .Storage.GenesisHash }}"
-
 [storage.pruning]
 
 # The time period between automated background pruning operations.

docs/references/config/config.toml.md

@@ -2025,18 +2025,6 @@ initial_block_results_retain_height = 0
 |:--------------------|:--------|
 | **Possible values** | >= 0 |
 
-### storage.pruning.data_companion.genesis_hash
-Hash of the Genesis file, passed to CometBFT via the command line.
-```toml
-genesis_hash = ""
-```
-
-| Value type          | string             |
-|:--------------------|:-------------------|
-| **Possible values** | hex-encoded number |
-|                     | `""`               |
-
-If this hash mismatches the hash that CometBFT computes on the genesis file, the node is not able to boot.
 
 ## Transaction indexer
 Transaction indexer settings.

node/node.go

@@ -3,6 +3,7 @@ package node
 import (
 	"bytes"
 	"context"
+	"encoding/hex"
 	"errors"
 	"fmt"
 	"net"
@@ -213,7 +214,7 @@ func BootstrapState(ctx context.Context, config *cfg.Config, dbProvider cfg.DBPr
 	}
 
 	// The state store will use the DBKeyLayout set in config or already existing in the DB.
-	genState, _, err := LoadStateFromDBOrGenesisDocProvider(stateDB, genProvider, config.Storage.GenesisHash)
+	genState, _, err := LoadStateFromDBOrGenesisDocProvider(stateDB, genProvider, "")
 	if err != nil {
 		return err
 	}
@@ -286,13 +287,46 @@ func NewNode(ctx context.Context,
 	metricsProvider MetricsProvider,
 	logger log.Logger,
 	options ...Option,
+) (*Node, error) {
+	return NewNodeWithCliParams(ctx,
+		config,
+		privValidator,
+		nodeKey,
+		clientCreator,
+		genesisDocProvider,
+		dbProvider,
+		metricsProvider,
+		logger,
+		CliParams{},
+		options...)
+}
+
+// NewNodeWithCliParams returns a new, ready to go, CometBFT node
+// where we check the hash of the provided genesis file against
+// a hash provided by the operator via cli.
+
+func NewNodeWithCliParams(ctx context.Context,
+	config *cfg.Config,
+	privValidator types.PrivValidator,
+	nodeKey *p2p.NodeKey,
+	clientCreator proxy.ClientCreator,
+	genesisDocProvider GenesisDocProvider,
+	dbProvider cfg.DBProvider,
+	metricsProvider MetricsProvider,
+	logger log.Logger,
+	cliParams CliParams,
+	options ...Option,
 ) (*Node, error) {
 	blockStoreDB, stateDB, err := initDBs(config, dbProvider)
 	if err != nil {
 		return nil, err
 	}
 
-	state, genDoc, err := LoadStateFromDBOrGenesisDocProvider(stateDB, genesisDocProvider, config.Storage.GenesisHash)
+	var genesisHash string
+	if len(cliParams.GenesisHash) != 0 {
+		genesisHash = hex.EncodeToString(cliParams.GenesisHash)
+	}
+	state, genDoc, err := LoadStateFromDBOrGenesisDocProvider(stateDB, genesisDocProvider, genesisHash)
 	if err != nil {
 		return nil, err
 	}

node/node_test.go

@@ -3,7 +3,6 @@ package node
 import (
 	"bytes"
 	"context"
-	"encoding/hex"
 	"fmt"
 	"net"
 	"net/http"
@@ -43,7 +42,7 @@ func TestNodeStartStop(t *testing.T) {
 	defer os.RemoveAll(config.RootDir)
 
 	// create & start node
-	n, err := DefaultNewNode(config, log.TestingLogger())
+	n, err := DefaultNewNode(config, log.TestingLogger(), CliParams{})
 	require.NoError(t, err)
 	err = n.Start()
 	require.NoError(t, err)
@@ -106,7 +105,7 @@ func TestCompanionInitialHeightSetup(t *testing.T) {
 	config.Storage.Pruning.DataCompanion.Enabled = true
 	config.Storage.Pruning.DataCompanion.InitialBlockRetainHeight = 1
 	// create & start node
-	n, err := DefaultNewNode(config, log.TestingLogger())
+	n, err := DefaultNewNode(config, log.TestingLogger(), CliParams{})
 	require.NoError(t, err)
 
 	companionRetainHeight, err := n.stateStore.GetCompanionBlockRetainHeight()
@@ -120,7 +119,7 @@ func TestNodeDelayedStart(t *testing.T) {
 	now := cmttime.Now()
 
 	// create & start node
-	n, err := DefaultNewNode(config, log.TestingLogger())
+	n, err := DefaultNewNode(config, log.TestingLogger(), CliParams{})
 	n.GenesisDoc().GenesisTime = now.Add(2 * time.Second)
 	require.NoError(t, err)
 
@@ -137,7 +136,7 @@ func TestNodeSetAppVersion(t *testing.T) {
 	defer os.RemoveAll(config.RootDir)
 
 	// create & start node
-	n, err := DefaultNewNode(config, log.TestingLogger())
+	n, err := DefaultNewNode(config, log.TestingLogger(), CliParams{})
 	require.NoError(t, err)
 
 	// default config uses the kvstore app
@@ -161,7 +160,7 @@ func TestPprofServer(t *testing.T) {
 	_, err := http.Get("http://" + config.RPC.PprofListenAddress) //nolint: bodyclose
 	require.Error(t, err)
 
-	n, err := DefaultNewNode(config, log.TestingLogger())
+	n, err := DefaultNewNode(config, log.TestingLogger(), CliParams{})
 	require.NoError(t, err)
 	require.NoError(t, n.Start())
 	defer func() {
@@ -203,7 +202,7 @@ func TestNodeSetPrivValTCP(t *testing.T) {
 	}()
 	defer signerServer.Stop() //nolint:errcheck // ignore for tests
 
-	n, err := DefaultNewNode(config, log.TestingLogger())
+	n, err := DefaultNewNode(config, log.TestingLogger(), CliParams{})
 	require.NoError(t, err)
 	assert.IsType(t, &privval.RetrySignerClient{}, n.PrivValidator())
 }
@@ -216,7 +215,7 @@ func TestPrivValidatorListenAddrNoProtocol(t *testing.T) {
 	defer os.RemoveAll(config.RootDir)
 	config.BaseConfig.PrivValidatorListenAddr = addrNoPrefix
 
-	_, err := DefaultNewNode(config, log.TestingLogger())
+	_, err := DefaultNewNode(config, log.TestingLogger(), CliParams{})
 	require.ErrorAs(t, err, &ErrPrivValidatorSocketClient{})
 }
 
@@ -247,7 +246,7 @@ func TestNodeSetPrivValIPC(t *testing.T) {
 	}()
 	defer pvsc.Stop() //nolint:errcheck // ignore for tests
 
-	n, err := DefaultNewNode(config, log.TestingLogger())
+	n, err := DefaultNewNode(config, log.TestingLogger(), CliParams{})
 	require.NoError(t, err)
 	assert.IsType(t, &privval.RetrySignerClient{}, n.PrivValidator())
 }
@@ -617,10 +616,11 @@ func TestNodeGenesisHashFlagMatch(t *testing.T) {
 	jsonBlob, err := os.ReadFile(config.GenesisFile())
 	require.NoError(t, err)
 
+	// Set the cli params variable to the correct hash
 	incomingChecksum := tmhash.Sum(jsonBlob)
-	// Set genesis flag value to incorrect hash
-	config.Storage.GenesisHash = hex.EncodeToString(incomingChecksum)
-	_, err = NewNode(
+	cliParams := CliParams{GenesisHash: incomingChecksum}
+
+	_, err = NewNodeWithCliParams(
 		context.Background(),
 		config,
 		privval.LoadOrGenFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile()),
@@ -630,6 +630,7 @@ func TestNodeGenesisHashFlagMatch(t *testing.T) {
 		cfg.DefaultDBProvider,
 		DefaultMetricsProvider(config.Instrumentation),
 		log.TestingLogger(),
+		cliParams,
 	)
 	require.NoError(t, err)
 }
@@ -650,9 +651,9 @@ func TestNodeGenesisHashFlagMismatch(t *testing.T) {
 	flagHash := tmhash.Sum(f)
 
 	// Set genesis flag value to incorrect hash
-	config.Storage.GenesisHash = hex.EncodeToString(flagHash)
+	cliParams := CliParams{GenesisHash: flagHash}
 
-	_, err = NewNode(
+	_, err = NewNodeWithCliParams(
 		context.Background(),
 		config,
 		privval.LoadOrGenFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile()),
@@ -662,6 +663,7 @@ func TestNodeGenesisHashFlagMismatch(t *testing.T) {
 		cfg.DefaultDBProvider,
 		DefaultMetricsProvider(config.Instrumentation),
 		log.TestingLogger(),
+		cliParams,
 	)
 	require.ErrorIs(t, err, ErrPassedGenesisHashMismatch, "NewNode should error when genesis flag value is incorrectly set")
 
@@ -710,7 +712,7 @@ func TestLoadStateFromDBOrGenesisDocProviderWithConfig(t *testing.T) {
 		_, _, err = LoadStateFromDBOrGenesisDocProviderWithConfig(
 			stateDB,
 			genDocProvider,
-			config.Storage.GenesisHash,
+			"",
 			nil,
 		)
 
@@ -724,7 +726,7 @@ func TestLoadStateFromDBOrGenesisDocProviderWithConfig(t *testing.T) {
 		_, _, err = LoadStateFromDBOrGenesisDocProviderWithConfig(
 			stateDB,
 			genDocProvider,
-			config.Storage.GenesisHash,
+			"",
 			nil,
 		)
 

node/setup.go

@@ -48,6 +48,19 @@ type ChecksummedGenesisDoc struct {
 	Sha256Checksum []byte
 }
 
+// Introduced to store parameters passed via cli and needed to start the node.
+// This parameters should not be stored or persisted in the config file.
+// This can then be further extended to include additional flags without further
+// API breaking changes.
+type CliParams struct {
+	// SHA-256 hash of the genesis file provided via the command line.
+	// This hash is used is compared against the computed hash of the
+	// actual genesis file or the hash stored in the database.
+	// If there is a mismatch between the hash provided via cli and the
+	// hash of the genesis file or the hash in the DB, the node will not boot.
+	GenesisHash []byte
+}
+
 // GenesisDocProvider returns a GenesisDoc together with its SHA256 checksum.
 // It allows the GenesisDoc to be pulled from sources other than the
 // filesystem, for instance from a distributed key-value store cluster.
@@ -74,25 +87,26 @@ func DefaultGenesisDocProviderFunc(config *cfg.Config) GenesisDocProvider {
 }
 
 // Provider takes a config and a logger and returns a ready to go Node.
-type Provider func(*cfg.Config, log.Logger) (*Node, error)
+type Provider func(*cfg.Config, log.Logger, CliParams) (*Node, error)
 
 // DefaultNewNode returns a CometBFT node with default settings for the
 // PrivValidator, ClientCreator, GenesisDoc, and DBProvider.
-// It implements NodeProvider.
-func DefaultNewNode(config *cfg.Config, logger log.Logger) (*Node, error) {
+// It implements Provider.
+func DefaultNewNode(config *cfg.Config, logger log.Logger, cliParams CliParams) (*Node, error) {
 	nodeKey, err := p2p.LoadOrGenNodeKey(config.NodeKeyFile())
 	if err != nil {
 		return nil, ErrorLoadOrGenNodeKey{Err: err, NodeKeyFile: config.NodeKeyFile()}
 	}
 
-	return NewNode(context.Background(), config,
+	return NewNodeWithCliParams(context.Background(), config,
 		privval.LoadOrGenFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile()),
 		nodeKey,
 		proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()),
 		DefaultGenesisDocProviderFunc(config),
 		cfg.DefaultDBProvider,
 		DefaultMetricsProvider(config.Instrumentation),
 		logger,
+		cliParams,
 	)
 }