Skip to content

Commit

Permalink
fix: app-hash mismatch if upgrade migration commit is interrupted(bac…
Browse files Browse the repository at this point in the history
…kport 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
  • Loading branch information
jaeseung-bae authored and mergify[bot] committed Mar 27, 2024
1 parent 971875b commit 1139b35
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 1 deletion.
19 changes: 19 additions & 0 deletions CHANGELOG.md
Expand Up @@ -43,11 +43,30 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Improvements

### Bug Fixes
<<<<<<< HEAD
* (x/auth) [#1281](https://github.com/Finschia/finschia-sdk/pull/1281) `ModuleAccount.Validate` now reports a nil `.BaseAccount` instead of panicking. (backport #1274)
* (x/foundation) [\#1283](https://github.com/Finschia/finschia-sdk/pull/1283) add init logic of foundation module accounts to InitGenesis in order to eliminate potential panic (backport #1277)
* (x/collection) [\#1282](https://github.com/Finschia/finschia-sdk/pull/1282) eliminates potential risk for Insufficient Sanity Check of tokenID in Genesis (backport #1276)
* (x/collection) [\#1290](https://github.com/Finschia/finschia-sdk/pull/1290) export x/collection params into genesis (backport #1268)
* (x/foundation) [\#1295](https://github.com/Finschia/finschia-sdk/pull/1295) add missing error handling for migration
=======
* chore(deps) [\#1141](https://github.com/Finschia/finschia-sdk/pull/1141) Bump github.com/cosmos/ledger-cosmos-go from 0.12.2 to 0.13.2 to fix ledger signing issue
* (x/auth, x/slashing) [\#1179](https://github.com/Finschia/finschia-sdk/pull/1179) modify missing changes of converting to tendermint
* (x/auth) [#1274](https://github.com/Finschia/finschia-sdk/pull/1274) `ModuleAccount.Validate` now reports a nil `.BaseAccount` instead of panicking.
* (x/collection) [\#1276](https://github.com/Finschia/finschia-sdk/pull/1276) eliminates potential risk for Insufficient Sanity Check of tokenID in Genesis
* (x/foundation) [\#1277](https://github.com/Finschia/finschia-sdk/pull/1277) add init logic of foundation module accounts to InitGenesis in order to eliminate potential panic
* (x/collection, x/token) [\#1288](https://github.com/Finschia/finschia-sdk/pull/1288) use accAddress to compare in validatebasic function in collection & token modules
* (x/collection) [\#1268](https://github.com/Finschia/finschia-sdk/pull/1268) export x/collection params into genesis
* (x/collection) [\#1294](https://github.com/Finschia/finschia-sdk/pull/1294) reject NFT coins on FT APIs
* (sec) [\#1302](https://github.com/Finschia/finschia-sdk/pull/1302) remove map iteration non-determinism with keys + sorting
* (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)
* (x/gov) [\#1304](https://github.com/Finschia/finschia-sdk/pull/1304) fetch a failed proposal tally from proposal.FinalTallyResult in the gprc query
* (x/staking) [\#1306](https://github.com/Finschia/finschia-sdk/pull/1306) add validation for potential slashing evasion during re-delegation
* (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)
>>>>>>> 9105ff4be (fix: app-hash mismatch if upgrade migration commit is interrupted(backport cosmos/cosmos-sdk#13530) (#1310))
### Removed

Expand Down
10 changes: 9 additions & 1 deletion store/rootmulti/store.go
Expand Up @@ -968,8 +968,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
}
Expand Down
75 changes: 75 additions & 0 deletions store/rootmulti/store_test.go
Expand Up @@ -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)
})
}
}

0 comments on commit 1139b35

Please sign in to comment.