From 2580480c580e6f25b88953742989b53691d99317 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Mon, 27 Nov 2023 11:44:33 +0100 Subject: [PATCH 1/8] remove the notion of nil lastcommitidhash --- baseapp/abci.go | 20 ++++---------------- baseapp/abci_test.go | 14 +++++++------- baseapp/baseapp_test.go | 5 ++++- go.mod | 4 +--- go.sum | 2 -- store/rootmulti/store.go | 13 +++++++++++++ 6 files changed, 29 insertions(+), 29 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 2c1aab4f955c..f6c785a5fa27 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,17 @@ 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[:] - } - + fmt.Println(app.LastCommitID().Hash) // 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{ @@ -732,7 +720,7 @@ func (app *BaseApp) internalFinalizeBlock(ctx context.Context, req *abci.Request Height: req.Height, Time: req.Time, Hash: req.Hash, - AppHash: app.LastCommitID().Hash, + // AppHash: app.LastCommitID().Hash, }). WithConsensusParams(app.GetConsensusParams(app.finalizeBlockState.ctx)). WithVoteInfos(req.DecidedLastCommit.Votes). diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 179c732acdc6..d368b9e81d29 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 d8371ead0d12..927d9f037e62 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() diff --git a/go.mod b/go.mod index 4375ca8dacab..e8d337ff82a9 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 // Below are the long-lived replace of the Cosmos SDK replace ( diff --git a/go.sum b/go.sum index 784e3e7f44a4..0ad229accfd2 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/rootmulti/store.go b/store/rootmulti/store.go index 7b2c05ff7761..58ff94f0cf8c 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,20 @@ 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 rs.lastCommitInfo.CommitID().Hash == nil { + 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 } } From a02cdacfb1354eaafe08c940525fdb5bfb716dd0 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Mon, 27 Nov 2023 11:47:57 +0100 Subject: [PATCH 2/8] remove print --- baseapp/abci.go | 1 - 1 file changed, 1 deletion(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index f6c785a5fa27..bea5eeb41a30 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -125,7 +125,6 @@ func (app *BaseApp) InitChain(req *abci.RequestInitChain) (*abci.ResponseInitCha } } - fmt.Println(app.LastCommitID().Hash) // NOTE: We don't commit, but FinalizeBlock for block InitialHeight starts from // this FinalizeBlockState. return &abci.ResponseInitChain{ From 0be2dcca182e2a364463dcf7bb0cbfc236344212 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Mon, 27 Nov 2023 11:48:42 +0100 Subject: [PATCH 3/8] revert comment --- baseapp/abci.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index bea5eeb41a30..a78fc700bc14 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -719,7 +719,7 @@ func (app *BaseApp) internalFinalizeBlock(ctx context.Context, req *abci.Request Height: req.Height, Time: req.Time, Hash: req.Hash, - // AppHash: app.LastCommitID().Hash, + AppHash: app.LastCommitID().Hash, }). WithConsensusParams(app.GetConsensusParams(app.finalizeBlockState.ctx)). WithVoteInfos(req.DecidedLastCommit.Votes). From f2725499fc6f6231872810b39b80faf847becdf3 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Mon, 27 Nov 2023 12:13:43 +0100 Subject: [PATCH 4/8] changelog and todo in go.mod --- go.mod | 2 +- store/CHANGELOG.md | 7 ++++--- store/rootmulti/store_test.go | 17 +++++++++++++---- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index e8d337ff82a9..f8632ca0b1c9 100644 --- a/go.mod +++ b/go.mod @@ -164,7 +164,7 @@ require ( // Here are the short-lived replace from the Cosmos SDK // Replace here are pending PRs, or version to be tagged -replace cosmossdk.io/store => ./store +replace cosmossdk.io/store => ./store // TODO: REMOVE prior to release // Below are the long-lived replace of the Cosmos SDK replace ( diff --git a/store/CHANGELOG.md b/store/CHANGELOG.md index b8cffc0ad6ec..54abe5e39b4d 100644 --- a/store/CHANGELOG.md +++ b/store/CHANGELOG.md @@ -29,9 +29,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#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. @@ -46,3 +46,4 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking Changes * [#16321](https://github.com/cosmos/cosmos-sdk/pull/16321) QueryInterface defines its own request and response types instead of relying on comet/abci & returns an error +* [#18563](https://github.com/cosmos/cosmos-sdk/pull/18563) `LastCommitID().Hash` will always return `sha256([]byte{})` if the store is empty. diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go index df69e2ab734a..2702f3e08623 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") From 57d15a63023dd52b13fd2c9effb140fcb0435a92 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Mon, 27 Nov 2023 15:01:06 +0100 Subject: [PATCH 5/8] fix working hash --- baseapp/baseapp_test.go | 5 ++++- store/rootmulti/store.go | 12 +++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 927d9f037e62..6d941e9bbb7c 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -729,7 +729,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/store/rootmulti/store.go b/store/rootmulti/store.go index 58ff94f0cf8c..fa9a098494e7 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -446,8 +446,7 @@ func (rs *Store) LastCommitID() types.CommitID { Hash: appHash, // set empty apphash to sha256([]byte{}) if info is nil } } - - if rs.lastCommitInfo.CommitID().Hash == nil { + if len(rs.lastCommitInfo.CommitID().Hash) == 0 { emptyHash := sha256.Sum256([]byte{}) appHash := emptyHash[:] return types.CommitID{ @@ -534,7 +533,14 @@ func (rs *Store) WorkingHash() []byte { return storeInfos[i].Name < storeInfos[j].Name }) - return types.CommitInfo{StoreInfos: storeInfos}.Hash() + apphash := types.CommitInfo{StoreInfos: storeInfos}.Hash() + + if len(apphash) == 0 { + emptyHash := sha256.Sum256([]byte{}) + return emptyHash[:] + } + + return apphash } // CacheWrap implements CacheWrapper/Store/CommitStore. From 624b764069226ee496cc853abff9731d21ff29e6 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Mon, 27 Nov 2023 19:41:52 +0100 Subject: [PATCH 6/8] fix hash call --- store/rootmulti/store.go | 9 +-------- store/types/commit_info.go | 8 ++++++++ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index fa9a098494e7..551a53e635c8 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -533,14 +533,7 @@ func (rs *Store) WorkingHash() []byte { return storeInfos[i].Name < storeInfos[j].Name }) - apphash := types.CommitInfo{StoreInfos: storeInfos}.Hash() - - if len(apphash) == 0 { - emptyHash := sha256.Sum256([]byte{}) - return emptyHash[:] - } - - return apphash + return types.CommitInfo{StoreInfos: storeInfos}.Hash() } // CacheWrap implements CacheWrapper/Store/CommitStore. diff --git a/store/types/commit_info.go b/store/types/commit_info.go index 125111a0c228..e23d1df6b96e 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" @@ -34,6 +36,12 @@ func (ci CommitInfo) Hash() []byte { } rootHash, _, _ := maps.ProofsFromMap(ci.toMap()) + + if len(rootHash) == 0 { + emptyHash := sha256.Sum256([]byte{}) + return emptyHash[:] + } + return rootHash } From c869df80cc7d5aa9c1e975e0653a4f2be53a2941 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Mon, 27 Nov 2023 19:43:03 +0100 Subject: [PATCH 7/8] changelog add --- store/CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/store/CHANGELOG.md b/store/CHANGELOG.md index 54abe5e39b4d..0df26e3cc4bc 100644 --- a/store/CHANGELOG.md +++ b/store/CHANGELOG.md @@ -23,6 +23,12 @@ 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 @@ -46,4 +52,3 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking Changes * [#16321](https://github.com/cosmos/cosmos-sdk/pull/16321) QueryInterface defines its own request and response types instead of relying on comet/abci & returns an error -* [#18563](https://github.com/cosmos/cosmos-sdk/pull/18563) `LastCommitID().Hash` will always return `sha256([]byte{})` if the store is empty. From 5f718a1f1fae8d0ca527ae803bba2c45c7588c93 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Mon, 27 Nov 2023 21:28:08 +0100 Subject: [PATCH 8/8] replace nil check --- store/types/commit_info.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/store/types/commit_info.go b/store/types/commit_info.go index e23d1df6b96e..249d0986d403 100644 --- a/store/types/commit_info.go +++ b/store/types/commit_info.go @@ -32,7 +32,8 @@ 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())