From ddfd9ccd9c4f217481bd6219bb5589d753859ad6 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 25 Oct 2022 00:02:51 +0800 Subject: [PATCH 1/5] fix: possible app-hash mismatch(cherry-pick cosmos-sdk #13530) --- store/rootmulti/store.go | 10 ++++- store/rootmulti/store_test.go | 73 +++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index 3aef50bcca..61063a0da0 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -984,8 +984,16 @@ 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() + // 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 + commitID = last + } else { + commitID = store.Commit() + } if store.GetStoreType() == types.StoreTypeTransient { continue } diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go index da1c90842a..d38bd37ad7 100644 --- a/store/rootmulti/store_test.go +++ b/store/rootmulti/store_test.go @@ -858,3 +858,76 @@ func TestSetIAVLDIsableFastNode(t *testing.T) { multi.SetIAVLDisableFastNode(false) require.Equal(t, multi.iavlDisableFastNode, false) } + +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 fddddff2b059504cc305011a1c046c0b775dcc16 Mon Sep 17 00:00:00 2001 From: "jaeseung.bae" Date: Wed, 27 Mar 2024 09:48:51 +0900 Subject: [PATCH 2/5] chore: fix testcase --- store/rootmulti/store_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go index d38bd37ad7..097dd173d5 100644 --- a/store/rootmulti/store_test.go +++ b/store/rootmulti/store_test.go @@ -921,8 +921,7 @@ func TestCommitStores(t *testing.T) { } store.Committed = 0 var version int64 = 1 - removalMap := map[types.StoreKey]bool{} - res := commitStores(version, storeMap, removalMap) + res := commitStores(version, storeMap) for _, s := range res.StoreInfos { require.Equal(t, version, s.CommitId.Version) } From f4f3d9be6578cebf42bed62e51fe6247d5de8da4 Mon Sep 17 00:00:00 2001 From: "jaeseung.bae" Date: Wed, 27 Mar 2024 09:49:25 +0900 Subject: [PATCH 3/5] chore: update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cbb973ea6..728e18fb00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Breaking Changes * (consensus) [\#1178](https://github.com/Finschia/finschia-sdk/pull/1178) change the consensus from Ostracon to Tendermint v0.34.24 +* (consensus) [\#1310](https://github.com/Finschia/finschia-sdk/pull/1310) fix app-hash mismatch if upgrade migration commit is interrupted(backport cosmos/cosmos-sdk#13530) ### State Machine Breaking From 55264b13df171e0076cd6e3135e9c8074a0d233a Mon Sep 17 00:00:00 2001 From: "jaeseung.bae" Date: Wed, 27 Mar 2024 09:59:06 +0900 Subject: [PATCH 4/5] chore: lint fix --- store/rootmulti/store_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go index 097dd173d5..944075c803 100644 --- a/store/rootmulti/store_test.go +++ b/store/rootmulti/store_test.go @@ -876,7 +876,10 @@ func prepareStoreMap() map[types.StoreKey]types.CommitKVStore { 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() + err := store.LoadLatestVersion() + if err != nil { + panic(err) + } return map[types.StoreKey]types.CommitKVStore{ testStoreKey1: &commitKVStoreStub{ CommitKVStore: store.GetStoreByName("iavl1").(types.CommitKVStore), From 8fe975488c20be9cf62f68163fae2f704fc78a9c Mon Sep 17 00:00:00 2001 From: "jaeseung.bae" Date: Wed, 27 Mar 2024 10:55:07 +0900 Subject: [PATCH 5/5] chore: not a breaking change --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 728e18fb00..15b12faba2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,12 +64,12 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (client) [\#1303](https://github.com/Finschia/finschia-sdk/pull/1303) fix possible overflow in BuildUnsignedTx * (types) [\#1299](https://github.com/Finschia/finschia-sdk/pull/1299) add missing nil checks * (x/staking) [\#1301](https://github.com/Finschia/finschia-sdk/pull/1301) Use bytes instead of string comparison in delete validator queue (backport cosmos/cosmos-sdk#12303) +* (store) [\#1310](https://github.com/Finschia/finschia-sdk/pull/1310) fix app-hash mismatch if upgrade migration commit is interrupted(backport cosmos/cosmos-sdk#13530) ### Removed ### Breaking Changes -* (consensus) [\#1178](https://github.com/Finschia/finschia-sdk/pull/1178) change the consensus from Ostracon to Tendermint v0.34.24 -* (consensus) [\#1310](https://github.com/Finschia/finschia-sdk/pull/1310) fix app-hash mismatch if upgrade migration commit is interrupted(backport cosmos/cosmos-sdk#13530) +* (consensus) [\#1178](https://github.com/Finschia/finschia-sdk/pull/1178) change the consensus from Ostracon to Tendermint v0.34.24 ### State Machine Breaking