fix: app-hash mismatch if upgrade migration commit is interrupted(bac…

…kport cosmos/cosmos-sdk#13530) (backport #1310) (#1318) * fix: app-hash mismatch if upgrade migration commit is interrupted(backport cosmos/cosmos-sdk#13530) (#1310) * fix: possible app-hash mismatch(cherry-pick cosmos-sdk #13530) * chore: fix testcase * chore: update changelog * chore: lint fix * chore: not a breaking change --------- Co-authored-by: mmsqe <mavis@crypto.com> (cherry picked from commit 9105ff4) # Conflicts: # CHANGELOG.md * Update CHANGELOG.md --------- Co-authored-by: jaeseung-bae <119839167+jaeseung-bae@users.noreply.github.com> Co-authored-by: Youngtaek Yoon <noreply@yoon.mailer.me>
Finschia · Mar 29, 2024 · feb8821 · feb8821
1 parent 581d50c
commit feb8821
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -54,6 +54,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (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)
 * (client/keys) [#1312](https://github.com/Finschia/finschia-sdk/pull/1312) ignore error when key not found in `keys delete`
+* (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
 

diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go
@@ -985,8 +985,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
@@ -918,3 +918,78 @@ 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)
+ err := store.LoadLatestVersion()
+ if err != nil {
+ panic(err)
+ }
+ 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
+ res := commitStores(version, storeMap)
+ 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)
+ })
+ }
+}