From 0f9feeb3fef5f89eaaf10f4c26386ed668dc5699 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 13 Oct 2022 11:17:46 +0800 Subject: [PATCH 1/4] avoid append commit when upgrade get interrupted --- store/rootmulti/store.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index 0f652d24bab5..5fe10d2604b5 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -1035,8 +1035,14 @@ func commitStores(version int64, storeMap map[types.StoreKey]types.CommitKVStore storeInfos := make([]types.StoreInfo, 0, len(storeMap)) for key, store := range storeMap { - commitID := store.Commit() - + last := store.LastCommitID() + var commitID types.CommitID + if last.Version >= version { + last.Version = version + commitID = last + } else { + commitID = store.Commit() + } if store.GetStoreType() == types.StoreTypeTransient { continue } From a61dbd1368ddffd1c766df532b16a7f2f05158b7 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 14 Oct 2022 09:22:37 +0800 Subject: [PATCH 2/4] add commitStores test --- store/rootmulti/store_test.go | 73 +++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go index a219593fbb86..1038bdef93db 100644 --- a/store/rootmulti/store_test.go +++ b/store/rootmulti/store_test.go @@ -961,3 +961,76 @@ func TestStateListeners(t *testing.T) { cacheMulti.Write() require.Equal(t, 1, len(listener.stateCache)) } + +type commitKVStoreStub struct { + types.CommitKVStore + Committed int +} + +func (stub *commitKVStoreStub) Commit() types.CommitID { + commitID := stub.CommitKVStore.Commit() + stub.Committed += 1 + return commitID +} + +func prepareStoreMap() map[types.StoreKey]types.CommitKVStore { + var db dbm.DB = dbm.NewMemDB() + store := NewStore(db, log.NewNopLogger()) + store.MountStoreWithDB(types.NewKVStoreKey("iavl1"), types.StoreTypeIAVL, nil) + store.MountStoreWithDB(types.NewKVStoreKey("iavl2"), types.StoreTypeIAVL, nil) + store.MountStoreWithDB(types.NewTransientStoreKey("trans1"), types.StoreTypeTransient, nil) + store.LoadLatestVersion() + return map[types.StoreKey]types.CommitKVStore{ + testStoreKey1: &commitKVStoreStub{ + CommitKVStore: store.GetStoreByName("iavl1").(types.CommitKVStore), + }, + testStoreKey2: &commitKVStoreStub{ + CommitKVStore: store.GetStoreByName("iavl2").(types.CommitKVStore), + }, + testStoreKey3: &commitKVStoreStub{ + CommitKVStore: store.GetStoreByName("trans1").(types.CommitKVStore), + }, + } +} + +func TestCommitStores(t *testing.T) { + testCases := []struct { + name string + committed int + exptectCommit int + }{ + { + "when upgrade not get interrupted", + 0, + 1, + }, + { + "when upgrade get interrupted once", + 1, + 0, + }, + { + "when upgrade get interrupted twice", + 2, + 0, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + storeMap := prepareStoreMap() + store := storeMap[testStoreKey1].(*commitKVStoreStub) + for i := tc.committed; i > 0; i-- { + store.Commit() + } + store.Committed = 0 + var version int64 = 1 + removalMap := map[types.StoreKey]bool{} + res := commitStores(version, storeMap, removalMap) + for _, s := range res.StoreInfos { + require.Equal(t, version, s.CommitId.Version) + } + require.Equal(t, version, res.Version) + require.Equal(t, tc.exptectCommit, store.Committed) + }) + } +} From 358117ce5bacdfefe6f7b73659bc60689ce8df47 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 14 Oct 2022 09:27:18 +0800 Subject: [PATCH 3/4] add change doc --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd789a2602ae..afcc8c83f1f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -175,6 +175,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/gov) [#13045](https://github.com/cosmos/cosmos-sdk/pull/13045) Fix gov migrations for v3(0.46). * (snapshot) [#13400](https://github.com/cosmos/cosmos-sdk/pull/13400) Fix snapshot checksum issue in golang 1.19. * (store) [#13459](https://github.com/cosmos/cosmos-sdk/pull/13459) Don't let state listener observe the uncommitted writes. +* (store) [#13530](https://github.com/cosmos/cosmos-sdk/pull/13530) Fix app-hash mismatch if upgrade migration commit is interrupted. ### Deprecated From 907c2375fbbc4d51ced7e06f6869c48d1d7c9e5d Mon Sep 17 00:00:00 2001 From: yihuang Date: Wed, 19 Oct 2022 08:51:39 +0800 Subject: [PATCH 4/4] Update store/rootmulti/store.go --- store/rootmulti/store.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index 5fe10d2604b5..260811478105 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -1036,6 +1036,8 @@ func commitStores(version int64, storeMap map[types.StoreKey]types.CommitKVStore for key, store := range storeMap { last := store.LastCommitID() + + // If a commit event execution is interrupted, a new iavl store's version will be larger than the rootmulti's metadata, when the block is replayed, we should avoid committing that iavl store again. var commitID types.CommitID if last.Version >= version { last.Version = version