Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prevent nil lastcommitid hash #18563

Merged
merged 8 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 2 additions & 15 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package baseapp

import (
"context"
"crypto/sha256"
"fmt"
"sort"
"strings"
Expand Down Expand Up @@ -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{
Expand Down
14 changes: 7 additions & 7 deletions baseapp/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package baseapp_test
import (
"bytes"
"context"
"crypto/sha256"
"encoding/binary"
"encoding/hex"
"errors"
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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()
Expand Down
5 changes: 4 additions & 1 deletion baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package baseapp_test

import (
"context"
"crypto/sha256"
"fmt"
"math/rand"
"testing"
Expand Down Expand Up @@ -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()
Expand Down
4 changes: 1 addition & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
// <temporary replace>
// )
replace cosmossdk.io/store => ./store // TODO: REMOVE prior to release

// Below are the long-lived replace of the Cosmos SDK
replace (
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
7 changes: 4 additions & 3 deletions store/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ Ref: https://keepachangelog.com/en/1.0.0/

julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
* [#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.
Expand All @@ -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.
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
13 changes: 13 additions & 0 deletions store/rootmulti/store.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package rootmulti

import (
"crypto/sha256"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -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
}
}

Expand Down
17 changes: 13 additions & 4 deletions store/rootmulti/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package rootmulti

import (
"bytes"
"crypto/sha256"
"fmt"
"testing"
"time"
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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")
Expand Down
Loading