From 20337d90533cbf61cb9f844980a5127780ec4a7a Mon Sep 17 00:00:00 2001 From: Marko Date: Tue, 28 Nov 2023 15:27:42 +0100 Subject: [PATCH] fix: prevent nil lastcommitid hash (#18563) --- baseapp/abci.go | 17 ++--------------- baseapp/abci_test.go | 14 +++++++------- baseapp/baseapp_test.go | 10 ++++++++-- go.mod | 4 +--- go.sum | 2 -- store/CHANGELOG.md | 12 +++++++++--- store/rootmulti/store.go | 12 ++++++++++++ store/rootmulti/store_test.go | 17 +++++++++++++---- store/types/commit_info.go | 11 ++++++++++- 9 files changed, 62 insertions(+), 37 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 2c1aab4f95..a78fc700bc 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -2,7 +2,6 @@ package baseapp import ( "context" - "crypto/sha256" "fmt" "sort" "strings" @@ -126,28 +125,16 @@ func (app *BaseApp) InitChain(req *abci.RequestInitChain) (*abci.ResponseInitCha } } - // In the case of a new chain, AppHash will be the hash of an empty string. - // During an upgrade, it'll be the hash of the last committed block. - var appHash []byte - if !app.LastCommitID().IsZero() { - appHash = app.LastCommitID().Hash - } else { - // $ echo -n '' | sha256sum - // e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 - emptyHash := sha256.Sum256([]byte{}) - appHash = emptyHash[:] - } - // NOTE: We don't commit, but FinalizeBlock for block InitialHeight starts from // this FinalizeBlockState. return &abci.ResponseInitChain{ ConsensusParams: res.ConsensusParams, Validators: res.Validators, - AppHash: appHash, + AppHash: app.LastCommitID().Hash, }, nil } -func (app *BaseApp) Info(req *abci.RequestInfo) (*abci.ResponseInfo, error) { +func (app *BaseApp) Info(_ *abci.RequestInfo) (*abci.ResponseInfo, error) { lastCommitID := app.cms.LastCommitID() return &abci.ResponseInfo{ diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 179c732acd..d368b9e81d 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -3,6 +3,7 @@ package baseapp_test import ( "bytes" "context" + "crypto/sha256" "encoding/binary" "encoding/hex" "errors" @@ -49,10 +50,12 @@ func TestABCI_Info(t *testing.T) { res, err := suite.baseApp.Info(&reqInfo) require.NoError(t, err) + emptyHash := sha256.Sum256([]byte{}) + appHash := emptyHash[:] require.Equal(t, "", res.Version) require.Equal(t, t.Name(), res.GetData()) require.Equal(t, int64(0), res.LastBlockHeight) - require.Equal(t, []uint8(nil), res.LastBlockAppHash) + require.Equal(t, appHash, res.LastBlockAppHash) require.Equal(t, suite.baseApp.AppVersion(), res.AppVersion) } @@ -121,14 +124,11 @@ func TestABCI_InitChain(t *testing.T) { // The AppHash returned by a new chain is the sha256 hash of "". // $ echo -n '' | sha256sum // e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 - apphash, err := hex.DecodeString("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") + emptyHash := sha256.Sum256([]byte{}) + appHash := emptyHash[:] require.NoError(t, err) - require.Equal( - t, - apphash, - initChainRes.AppHash, - ) + require.Equal(t, appHash, initChainRes.AppHash) // assert that chainID is set correctly in InitChain chainID := getFinalizeBlockStateCtx(app).ChainID() diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index ece2bf0af1..ea082b8b04 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -2,6 +2,7 @@ package baseapp_test import ( "context" + "crypto/sha256" "fmt" "math/rand" "testing" @@ -199,7 +200,9 @@ func TestLoadVersion(t *testing.T) { err := app.LoadLatestVersion() // needed to make stores non-nil require.Nil(t, err) - emptyCommitID := storetypes.CommitID{} + emptyHash := sha256.Sum256([]byte{}) + appHash := emptyHash[:] + emptyCommitID := storetypes.CommitID{Hash: appHash} // fresh store has zero/empty last commit lastHeight := app.LastBlockHeight() @@ -737,7 +740,10 @@ func TestLoadVersionPruning(t *testing.T) { err := app.LoadLatestVersion() // needed to make stores non-nil require.Nil(t, err) - emptyCommitID := storetypes.CommitID{} + emptyHash := sha256.Sum256([]byte{}) + emptyCommitID := storetypes.CommitID{ + Hash: emptyHash[:], + } // fresh store has zero/empty last commit lastHeight := app.LastBlockHeight() diff --git a/go.mod b/go.mod index 21120aa17e..e5380e2326 100644 --- a/go.mod +++ b/go.mod @@ -164,9 +164,7 @@ require ( // Here are the short-lived replace from the Cosmos SDK // Replace here are pending PRs, or version to be tagged -// replace ( -// -// ) +replace cosmossdk.io/store => ./store // TODO: REMOVE prior to release // Below are the long-lived replace of the Cosmos SDK replace ( diff --git a/go.sum b/go.sum index 361d798d82..d5f1124327 100644 --- a/go.sum +++ b/go.sum @@ -49,8 +49,6 @@ cosmossdk.io/log v1.2.1 h1:Xc1GgTCicniwmMiKwDxUjO4eLhPxoVdI9vtMW8Ti/uk= cosmossdk.io/log v1.2.1/go.mod h1:GNSCc/6+DhFIj1aLn/j7Id7PaO8DzNylUZoOYBL9+I4= cosmossdk.io/math v1.2.0 h1:8gudhTkkD3NxOP2YyyJIYYmt6dQ55ZfJkDOaxXpy7Ig= cosmossdk.io/math v1.2.0/go.mod h1:l2Gnda87F0su8a/7FEKJfFdJrM0JZRXQaohlgJeyQh0= -cosmossdk.io/store v1.0.0 h1:6tnPgTpTSIskaTmw/4s5C9FARdgFflycIc9OX8i1tOI= -cosmossdk.io/store v1.0.0/go.mod h1:ABMprwjvx6IpMp8l06TwuMrj6694/QP5NIW+X6jaTYc= cosmossdk.io/x/tx v0.12.0 h1:Ry2btjQdrfrje9qZ3iZeZSmDArjgxUJMMcLMrX4wj5U= cosmossdk.io/x/tx v0.12.0/go.mod h1:qTth2coAGkwCwOCjqQ8EAQg+9udXNRzcnSbMgGKGEI0= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= diff --git a/store/CHANGELOG.md b/store/CHANGELOG.md index b8cffc0ad6..0df26e3cc4 100644 --- a/store/CHANGELOG.md +++ b/store/CHANGELOG.md @@ -23,15 +23,21 @@ Ref: https://keepachangelog.com/en/1.0.0/ # Changelog +## Unreleased + +### Bug Fixes + +* [#18563](https://github.com/cosmos/cosmos-sdk/pull/18563) `LastCommitID().Hash` will always return `sha256([]byte{})` if the store is empty. + ## v1.0.0 (October 31, 2023) ### Features * [#17294](https://github.com/cosmos/cosmos-sdk/pull/17294) Add snapshot manager Close method. * [#15568](https://github.com/cosmos/cosmos-sdk/pull/15568) Migrate the `iavl` to the new key format. - * Remove `DeleteVersion`, `DeleteVersions`, `LazyLoadVersionForOverwriting` from `iavl` tree API. - * Add `DeleteVersionsTo` and `SaveChangeSet`, since it will keep versions sequentially like `fromVersion` to `toVersion`. - * Refactor the pruning manager to use `DeleteVersionsTo`. + * Remove `DeleteVersion`, `DeleteVersions`, `LazyLoadVersionForOverwriting` from `iavl` tree API. + * Add `DeleteVersionsTo` and `SaveChangeSet`, since it will keep versions sequentially like `fromVersion` to `toVersion`. + * Refactor the pruning manager to use `DeleteVersionsTo`. * [#15712](https://github.com/cosmos/cosmos-sdk/pull/15712) Add `WorkingHash` function to the store interface to get the current app hash before commit. * [#14645](https://github.com/cosmos/cosmos-sdk/pull/14645) Add limit to the length of key and value. * [#15683](https://github.com/cosmos/cosmos-sdk/pull/15683) `rootmulti.Store.CacheMultiStoreWithVersion` now can handle loading archival states that don't persist any of the module stores the current state has. diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index 7b2c05ff77..551a53e635 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -1,6 +1,7 @@ package rootmulti import ( + "crypto/sha256" "errors" "fmt" "io" @@ -438,8 +439,19 @@ func (rs *Store) LatestVersion() int64 { // LastCommitID implements Committer/CommitStore. func (rs *Store) LastCommitID() types.CommitID { if rs.lastCommitInfo == nil { + emptyHash := sha256.Sum256([]byte{}) + appHash := emptyHash[:] return types.CommitID{ Version: GetLatestVersion(rs.db), + Hash: appHash, // set empty apphash to sha256([]byte{}) if info is nil + } + } + if len(rs.lastCommitInfo.CommitID().Hash) == 0 { + emptyHash := sha256.Sum256([]byte{}) + appHash := emptyHash[:] + return types.CommitID{ + Version: rs.lastCommitInfo.Version, + Hash: appHash, // set empty apphash to sha256([]byte{}) if hash is nil } } diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go index df69e2ab73..2702f3e086 100644 --- a/store/rootmulti/store_test.go +++ b/store/rootmulti/store_test.go @@ -2,6 +2,7 @@ package rootmulti import ( "bytes" + "crypto/sha256" "fmt" "testing" "time" @@ -72,7 +73,9 @@ func TestCacheMultiStoreWithVersion(t *testing.T) { err := ms.LoadLatestVersion() require.Nil(t, err) - commitID := types.CommitID{} + emptyHash := sha256.Sum256([]byte{}) + appHash := emptyHash[:] + commitID := types.CommitID{Hash: appHash} checkStore(t, ms, commitID, commitID) k, v := []byte("wind"), []byte("blows") @@ -120,7 +123,9 @@ func TestHashStableWithEmptyCommit(t *testing.T) { err := ms.LoadLatestVersion() require.Nil(t, err) - commitID := types.CommitID{} + emptyHash := sha256.Sum256([]byte{}) + appHash := emptyHash[:] + commitID := types.CommitID{Hash: appHash} checkStore(t, ms, commitID, commitID) k, v := []byte("wind"), []byte("blows") @@ -148,8 +153,10 @@ func TestMultistoreCommitLoad(t *testing.T) { err := store.LoadLatestVersion() require.Nil(t, err) + emptyHash := sha256.Sum256([]byte{}) + appHash := emptyHash[:] // New store has empty last commit. - commitID := types.CommitID{} + commitID := types.CommitID{Hash: appHash} checkStore(t, store, commitID, commitID) // Make sure we can get stores by name. @@ -744,7 +751,9 @@ func TestCommitOrdered(t *testing.T) { err := multi.LoadLatestVersion() require.Nil(t, err) - commitID := types.CommitID{} + emptyHash := sha256.Sum256([]byte{}) + appHash := emptyHash[:] + commitID := types.CommitID{Hash: appHash} checkStore(t, multi, commitID, commitID) k, v := []byte("wind"), []byte("blows") diff --git a/store/types/commit_info.go b/store/types/commit_info.go index 125111a0c2..249d0986d4 100644 --- a/store/types/commit_info.go +++ b/store/types/commit_info.go @@ -1,6 +1,8 @@ package types import ( + "crypto/sha256" + cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto" "cosmossdk.io/store/internal/maps" @@ -30,10 +32,17 @@ func (ci CommitInfo) toMap() map[string][]byte { func (ci CommitInfo) Hash() []byte { // we need a special case for empty set, as SimpleProofsFromMap requires at least one entry if len(ci.StoreInfos) == 0 { - return nil + emptyHash := sha256.Sum256([]byte{}) + return emptyHash[:] } rootHash, _, _ := maps.ProofsFromMap(ci.toMap()) + + if len(rootHash) == 0 { + emptyHash := sha256.Sum256([]byte{}) + return emptyHash[:] + } + return rootHash }