Skip to content

Commit

Permalink
fix: prevent nil lastcommitid hash (#18563)
Browse files Browse the repository at this point in the history
  • Loading branch information
tac0turtle authored Nov 28, 2023
1 parent 67fecca commit 20337d9
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 37 deletions.
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
10 changes: 8 additions & 2 deletions 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 Expand Up @@ -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()
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
12 changes: 9 additions & 3 deletions store/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 12 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,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
}
}

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
11 changes: 10 additions & 1 deletion store/types/commit_info.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package types

import (
"crypto/sha256"

cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto"

"cosmossdk.io/store/internal/maps"
Expand Down Expand Up @@ -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
}

Expand Down

0 comments on commit 20337d9

Please sign in to comment.