From 1139b356f8ac21c228adbd7e2cef9633357b95b8 Mon Sep 17 00:00:00 2001 From: jaeseung-bae <119839167+jaeseung-bae@users.noreply.github.com> Date: Wed, 27 Mar 2024 19:55:09 +0900 Subject: [PATCH 1/2] 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 (cherry picked from commit 9105ff4be9ae75612df62bc89944e4e310057c0b) # Conflicts: # CHANGELOG.md --- CHANGELOG.md | 19 +++++++++ store/rootmulti/store.go | 10 ++++- store/rootmulti/store_test.go | 75 +++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb197ca3e7..168f6969f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index 8165debd66..5a971ea114 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -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 } diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go index 94df8e8424..0d457402b0 100644 --- 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) + }) + } +} From 1d9e552c03524131de92cf668eb9d86640cce41b Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Fri, 29 Mar 2024 02:31:16 +0000 Subject: [PATCH 2/2] Update CHANGELOG.md --- CHANGELOG.md | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 168f6969f7..3b91dba7ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,30 +43,12 @@ 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