Skip to content

Commit

Permalink
fix: Return error instead of empty values when pruned height is queri…
Browse files Browse the repository at this point in the history
…ed (cosmos#13896)

* fix: return err instead of empty values when pruned height is queried

* fix tests

* fix more tests

* final fixes

* add changelog
  • Loading branch information
likhita-809 authored and JeancarloBarrios committed Sep 28, 2024
1 parent 331ef2a commit efeee5e
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 18 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

* [#13896](https://github.com/cosmos/cosmos-sdk/pull/13896) Queries on pruned height returns error instead of empty values.
* (deps) Bump Tendermint version to [v0.34.23](https://github.com/tendermint/tendermint/releases/tag/v0.34.23).
* (deps) Bump IAVL version to [v0.19.4](https://github.com/cosmos/iavl/releases/tag/v0.19.4).
* (snapshot) [#13400](https://github.com/cosmos/cosmos-sdk/pull/13400) Fix snapshot checksum issue in golang 1.19.
Expand Down
2 changes: 1 addition & 1 deletion baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func TestLoadVersionPruning(t *testing.T) {

for _, v := range []int64{1, 2, 4} {
_, err = app.cms.CacheMultiStoreWithVersion(v)
require.NoError(t, err)
require.Error(t, err)
}

for _, v := range []int64{3, 5, 6, 7} {
Expand Down
2 changes: 1 addition & 1 deletion store/iavl/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func UnsafeNewStore(tree *iavl.MutableTree) *Store {
// Any mutable operations executed will result in a panic.
func (st *Store) GetImmutable(version int64) (*Store, error) {
if !st.VersionExists(version) {
return nil, errors.New("version mismatch on immutable IAVL tree; version does not exist. Version has either been pruned, or is for a future block height")
return &Store{tree: &immutableTree{&iavl.ImmutableTree{}}}, fmt.Errorf("version mismatch on immutable IAVL tree; version does not exist. Version has either been pruned, or is for a future block height")
}

iTree, err := st.tree.GetImmutable(version)
Expand Down
31 changes: 15 additions & 16 deletions store/rootmulti/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func TestCacheMultiStoreWithVersion(t *testing.T) {
cID := ms.Commit()
require.Equal(t, int64(1), cID.Version)

// require no failure when given an invalid or pruned version
// require error when given an invalid or pruned version
_, err = ms.CacheMultiStoreWithVersion(cID.Version + 1)
require.Error(t, err)

Expand Down Expand Up @@ -535,11 +535,11 @@ func TestMultiStore_Pruning(t *testing.T) {
deleted []int64
saved []int64
}{
{"prune nothing", 10, pruningtypes.NewPruningOptions(pruningtypes.PruningNothing), nil, []int64{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}},
{"prune everything", 12, pruningtypes.NewPruningOptions(pruningtypes.PruningEverything), []int64{1, 2, 3, 4, 5, 6, 7}, []int64{8, 9, 10, 11, 12}},
{"prune some; no batch", 10, pruningtypes.NewCustomPruningOptions(2, 1), []int64{1, 2, 3, 4, 6, 5, 7}, []int64{8, 9, 10}},
{"prune some; small batch", 10, pruningtypes.NewCustomPruningOptions(2, 3), []int64{1, 2, 3, 4, 5, 6}, []int64{7, 8, 9, 10}},
{"prune some; large batch", 10, pruningtypes.NewCustomPruningOptions(2, 11), nil, []int64{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}},
{"prune nothing", 10, types.PruneNothing, nil, []int64{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}},
{"prune everything", 10, types.PruneEverything, []int64{1, 2, 3, 4, 5, 6, 7}, []int64{8, 9, 10}},
{"prune some; no batch", 10, types.NewPruningOptions(2, 3, 1), []int64{1, 2, 4, 5, 7}, []int64{3, 6, 8, 9, 10}},
{"prune some; small batch", 10, types.NewPruningOptions(2, 3, 3), []int64{1, 2, 4, 5}, []int64{3, 6, 7, 8, 9, 10}},
{"prune some; large batch", 10, types.NewPruningOptions(2, 3, 11), nil, []int64{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}},
}

for _, tc := range testCases {
Expand All @@ -552,18 +552,14 @@ func TestMultiStore_Pruning(t *testing.T) {
ms.Commit()
}

for _, v := range tc.deleted {
// Ensure async pruning is done
checkErr := func() bool {
_, err := ms.CacheMultiStoreWithVersion(v)
return err != nil
}
require.Eventually(t, checkErr, 1*time.Second, 10*time.Millisecond, "expected error when loading height: %d", v)
for _, v := range tc.saved {
_, err := ms.CacheMultiStoreWithVersion(v)
require.NoError(t, err, "expected no error when loading height: %d", v)
}

for _, v := range tc.saved {
_, err := ms.CacheMultiStoreWithVersion(v)
require.NoError(t, err, "expected no error when loading height: %d", v)
require.Error(t, err, "expected error when loading height: %d", v)
}
})
}
Expand Down Expand Up @@ -641,8 +637,11 @@ func TestMultiStore_PruningRestart(t *testing.T) {
// commit one more block and ensure the heights have been pruned
ms.Commit()

actualHeightToPrune = ms.pruningManager.GetPruningHeight(ms.LatestVersion())
require.Equal(t, int64(8), actualHeightToPrune)
for _, v := range pruneHeights {
_, err := ms.CacheMultiStoreWithVersion(v)
require.Error(t, err, "expected error when loading height: %d", v)
}
}

func TestSetInitialVersion(t *testing.T) {
db := coretesting.NewMemDB()
Expand Down

0 comments on commit efeee5e

Please sign in to comment.