From 3ed7a8e347178ec22bff7c1da35f762c70ae7835 Mon Sep 17 00:00:00 2001 From: Sergio Mena Date: Fri, 1 Sep 2023 15:17:59 +0200 Subject: [PATCH 1/6] Self-contained solution --- node/setup.go | 67 +++++++++++++++++++++------------------------------ 1 file changed, 27 insertions(+), 40 deletions(-) diff --git a/node/setup.go b/node/setup.go index 206f5435346..bde9f03fe35 100644 --- a/node/setup.go +++ b/node/setup.go @@ -3,7 +3,6 @@ package node import ( "bytes" "context" - "errors" "fmt" "net" "strings" @@ -18,6 +17,7 @@ import ( cfg "github.com/cometbft/cometbft/config" cs "github.com/cometbft/cometbft/consensus" "github.com/cometbft/cometbft/crypto" + "github.com/cometbft/cometbft/crypto/tmhash" "github.com/cometbft/cometbft/evidence" "github.com/cometbft/cometbft/statesync" @@ -530,6 +530,7 @@ func startStateSync( //------------------------------------------------------------------------------ var genesisDocKey = []byte("genesisDoc") +var genesisDocHashKey = []byte("genesisDocHash") // LoadStateFromDBOrGenesisDocProvider attempts to load the state from the // database, or creates one using the given genesisDocProvider. On success this also @@ -538,22 +539,34 @@ func LoadStateFromDBOrGenesisDocProvider( stateDB dbm.DB, genesisDocProvider GenesisDocProvider, ) (sm.State, *types.GenesisDoc, error) { - // Get genesis doc - genDoc, err := loadGenesisDoc(stateDB) + // Get genesis doc hash + genDocHash, err := stateDB.Get(genesisDocHashKey) if err != nil { - genDoc, err = genesisDocProvider() - if err != nil { - return sm.State{}, nil, err - } + return sm.State{}, nil, fmt.Errorf("error retrieving genesis doc hash: %w", err) + } + genDoc, err := genesisDocProvider() + if err != nil { + return sm.State{}, nil, err + } - err = genDoc.ValidateAndComplete() - if err != nil { - return sm.State{}, nil, fmt.Errorf("error in genesis doc: %w", err) + if err := genDoc.ValidateAndComplete(); err != nil { + return sm.State{}, nil, fmt.Errorf("error in genesis doc: %w", err) + } + + genDocBytes, err := cmtjson.Marshal(genDoc) + if err != nil { + return sm.State{}, nil, fmt.Errorf("failed to save genesis doc hash due to marshaling error: %w", err) + } + + incomingGenDocHash := tmhash.Sum(genDocBytes) + if len(genDocHash) == 0 { + // Save the genDoc hash in the store if it doesn't already exist for future verification + if err := stateDB.SetSync(genesisDocHashKey, incomingGenDocHash); err != nil { + return sm.State{}, nil, fmt.Errorf("failed to save genesis doc hash to db: %w", err) } - // save genesis doc to prevent a certain class of user errors (e.g. when it - // was changed, accidentally or not). Also good for audit trail. - if err := saveGenesisDoc(stateDB, genDoc); err != nil { - return sm.State{}, nil, err + } else { + if !bytes.Equal(genDocHash, incomingGenDocHash) { + return sm.State{}, nil, fmt.Errorf("genesis doc hash in db does not match loaded genesis doc") } } stateStore := sm.NewStore(stateDB, sm.StoreOptions{ @@ -566,32 +579,6 @@ func LoadStateFromDBOrGenesisDocProvider( return state, genDoc, nil } -// panics if failed to unmarshal bytes -func loadGenesisDoc(db dbm.DB) (*types.GenesisDoc, error) { - b, err := db.Get(genesisDocKey) - if err != nil { - panic(err) - } - if len(b) == 0 { - return nil, errors.New("genesis doc not found") - } - var genDoc *types.GenesisDoc - err = cmtjson.Unmarshal(b, &genDoc) - if err != nil { - panic(fmt.Sprintf("Failed to load genesis doc due to unmarshaling error: %v (bytes: %X)", err, b)) - } - return genDoc, nil -} - -// panics if failed to marshal the given genesis document -func saveGenesisDoc(db dbm.DB, genDoc *types.GenesisDoc) error { - b, err := cmtjson.Marshal(genDoc) - if err != nil { - return fmt.Errorf("failed to save genesis doc due to marshaling error: %w", err) - } - return db.SetSync(genesisDocKey, b) -} - func createAndStartPrivValidatorSocketClient( listenAddr, chainID string, From 29c4a05f94671d62fe834bd655774d3b7ac6603c Mon Sep 17 00:00:00 2001 From: Sergio Mena Date: Fri, 1 Sep 2023 16:17:47 +0200 Subject: [PATCH 2/6] Retrieve new UT from #1017 --- node/node_test.go | 70 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/node/node_test.go b/node/node_test.go index c963f383bab..32496f64d6a 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -18,9 +18,12 @@ import ( "github.com/cometbft/cometbft/abci/example/kvstore" cfg "github.com/cometbft/cometbft/config" "github.com/cometbft/cometbft/crypto/ed25519" + "github.com/cometbft/cometbft/crypto/tmhash" "github.com/cometbft/cometbft/evidence" "github.com/cometbft/cometbft/internal/test" + cmtjson "github.com/cometbft/cometbft/libs/json" "github.com/cometbft/cometbft/libs/log" + cmtos "github.com/cometbft/cometbft/libs/os" cmtrand "github.com/cometbft/cometbft/libs/rand" mempl "github.com/cometbft/cometbft/mempool" "github.com/cometbft/cometbft/p2p" @@ -465,6 +468,73 @@ func TestNodeNewNodeCustomReactors(t *testing.T) { assert.Contains(t, channels, cr.Channels[0].ID) } +func TestNodeNewNodeGenesisHashMismatch(t *testing.T) { + config := test.ResetTestRoot("node_new_node_genesis_hash") + defer os.RemoveAll(config.RootDir) + + // Use goleveldb so we can reuse the same db for the second NewNode() + config.DBBackend = string(dbm.GoLevelDBBackend) + + nodeKey, err := p2p.LoadOrGenNodeKey(config.NodeKeyFile()) + require.NoError(t, err) + + n, err := NewNode( + 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), + log.TestingLogger(), + ) + require.NoError(t, err) + + // Start and stop to close the db for later reading + err = n.Start() + require.NoError(t, err) + + err = n.Stop() + require.NoError(t, err) + + // Ensure the genesis doc hash is saved to db + stateDB, err := cfg.DefaultDBProvider(&cfg.DBContext{ID: "state", Config: config}) + require.NoError(t, err) + + genDocHash, err := stateDB.Get(genesisDocHashKey) + require.NoError(t, err) + require.NotNil(t, genDocHash, "genesis doc hash should be saved in db") + require.Len(t, genDocHash, tmhash.Size) + + err = stateDB.Close() + require.NoError(t, err) + + // Modify the genesis file chain ID to get a different hash + genBytes := cmtos.MustReadFile(config.GenesisFile()) + var genesisDoc types.GenesisDoc + err = cmtjson.Unmarshal(genBytes, &genesisDoc) + require.NoError(t, err) + + genesisDoc.ChainID = "different-chain-id" + err = genesisDoc.SaveAs(config.GenesisFile()) + require.NoError(t, err) + + _, err = NewNode( + 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), + log.TestingLogger(), + ) + require.Error(t, err, "NewNode should error when genesisDoc is changed") + require.Equal(t, "genesis doc hash in db does not match loaded genesis doc", err.Error()) +} + func state(nVals int, height int64) (sm.State, dbm.DB, []types.PrivValidator) { privVals := make([]types.PrivValidator, nVals) vals := make([]types.GenesisValidator, nVals) From 75b924ccdeded65e7e48f9577c691641b52e3811 Mon Sep 17 00:00:00 2001 From: Sergio Mena Date: Fri, 1 Sep 2023 16:18:24 +0200 Subject: [PATCH 3/6] Keep changelog from #1017 --- .../improvements/1017-remove-genesis-persistence-in-state-db.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/1017-remove-genesis-persistence-in-state-db.md diff --git a/.changelog/unreleased/improvements/1017-remove-genesis-persistence-in-state-db.md b/.changelog/unreleased/improvements/1017-remove-genesis-persistence-in-state-db.md new file mode 100644 index 00000000000..1db7e014003 --- /dev/null +++ b/.changelog/unreleased/improvements/1017-remove-genesis-persistence-in-state-db.md @@ -0,0 +1,2 @@ +- `[node]` Remove genesis persistence in state db, replaced by a hash + ([cometbft/cometbft\#1017](https://github.com/cometbft/cometbft/pull/1017)) From cfddfaa3a9f87f2597f8e4412986718c43b100a5 Mon Sep 17 00:00:00 2001 From: Sergio Mena Date: Fri, 1 Sep 2023 16:26:50 +0200 Subject: [PATCH 4/6] Updated changelog --- .../1017-remove-genesis-persistence-in-state-db.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.changelog/unreleased/improvements/1017-remove-genesis-persistence-in-state-db.md b/.changelog/unreleased/improvements/1017-remove-genesis-persistence-in-state-db.md index 1db7e014003..13917825084 100644 --- a/.changelog/unreleased/improvements/1017-remove-genesis-persistence-in-state-db.md +++ b/.changelog/unreleased/improvements/1017-remove-genesis-persistence-in-state-db.md @@ -1,2 +1,3 @@ - `[node]` Remove genesis persistence in state db, replaced by a hash - ([cometbft/cometbft\#1017](https://github.com/cometbft/cometbft/pull/1017)) + ([cometbft/cometbft\#1017](https://github.com/cometbft/cometbft/pull/1017), + [cometbft/cometbft\#1295](https://github.com/cometbft/cometbft/pull/1295)) From db053f48481a920f52cbb884bf2f10f5840dfd16 Mon Sep 17 00:00:00 2001 From: Sergio Mena Date: Fri, 1 Sep 2023 16:32:32 +0200 Subject: [PATCH 5/6] Appease linter --- node/setup.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/setup.go b/node/setup.go index bde9f03fe35..c01bfa66651 100644 --- a/node/setup.go +++ b/node/setup.go @@ -529,7 +529,7 @@ func startStateSync( //------------------------------------------------------------------------------ -var genesisDocKey = []byte("genesisDoc") +//var genesisDocKey = []byte("genesisDoc") var genesisDocHashKey = []byte("genesisDocHash") // LoadStateFromDBOrGenesisDocProvider attempts to load the state from the From 1a469280722559187840f77af9dccfba314bb247 Mon Sep 17 00:00:00 2001 From: Sergio Mena Date: Fri, 1 Sep 2023 16:37:29 +0200 Subject: [PATCH 6/6] Appease linter 2 --- node/setup.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/setup.go b/node/setup.go index c01bfa66651..7ea0d29a673 100644 --- a/node/setup.go +++ b/node/setup.go @@ -529,7 +529,7 @@ func startStateSync( //------------------------------------------------------------------------------ -//var genesisDocKey = []byte("genesisDoc") +// var genesisDocKey = []byte("genesisDoc") var genesisDocHashKey = []byte("genesisDocHash") // LoadStateFromDBOrGenesisDocProvider attempts to load the state from the