diff --git a/.pending/improvements/sdk/4640-extend-DiffKVSt b/.pending/improvements/sdk/4640-extend-DiffKVSt new file mode 100644 index 000000000..821dd5c40 --- /dev/null +++ b/.pending/improvements/sdk/4640-extend-DiffKVSt @@ -0,0 +1,2 @@ +#4640 improve import/export simulation errors by extending `DiffKVStores` +to return an array of `KVPairs` that are then compared to check for inconsistencies. \ No newline at end of file diff --git a/simapp/sim_test.go b/simapp/sim_test.go index 59a9b6d4a..9a45bbd4c 100644 --- a/simapp/sim_test.go +++ b/simapp/sim_test.go @@ -499,9 +499,9 @@ func TestAppImportExport(t *testing.T) { prefixes := storeKeysPrefix.Prefixes storeA := ctxA.KVStore(storeKeyA) storeB := ctxB.KVStore(storeKeyB) - kvA, kvB, count, equal := sdk.DiffKVStores(storeA, storeB, prefixes) - fmt.Printf("Compared %d key/value pairs between %s and %s\n", count, storeKeyA, storeKeyB) - require.True(t, equal, GetSimulationLog(storeKeyA.Name(), app.cdc, newApp.cdc, kvA, kvB)) + failedKVs := sdk.DiffKVStores(storeA, storeB, prefixes) + fmt.Printf("Compared %d key/value pairs between %s and %s\n", len(failedKVs)/2, storeKeyA, storeKeyB) + require.Len(t, failedKVs, 0, GetSimulationLog(storeKeyA.Name(), app.cdc, newApp.cdc, failedKVs)) } } diff --git a/simapp/utils.go b/simapp/utils.go index 0624b4e9f..a19de40f3 100644 --- a/simapp/utils.go +++ b/simapp/utils.go @@ -496,31 +496,38 @@ func GenStakingGenesisState( // GetSimulationLog unmarshals the KVPair's Value to the corresponding type based on the // each's module store key and the prefix bytes of the KVPair's key. -func GetSimulationLog(storeName string, cdcA, cdcB *codec.Codec, kvA, kvB cmn.KVPair) (log string) { - log = fmt.Sprintf("store A %X => %X\nstore B %X => %X\n", kvA.Key, kvA.Value, kvB.Key, kvB.Value) +func GetSimulationLog(storeName string, cdcA, cdcB *codec.Codec, kvs []cmn.KVPair) (log string) { + var kvA, kvB cmn.KVPair + for i := 0; i < len(kvs); i += 2 { + kvA = kvs[i] + kvB = kvs[i+1] - if len(kvA.Value) == 0 && len(kvB.Value) == 0 { - return + if len(kvA.Value) == 0 && len(kvB.Value) == 0 { + // skip if the value doesn't have any bytes + continue + } + + switch storeName { + case auth.StoreKey: + log += DecodeAccountStore(cdcA, cdcB, kvA, kvB) + case mint.StoreKey: + log += DecodeMintStore(cdcA, cdcB, kvA, kvB) + case staking.StoreKey: + log += DecodeStakingStore(cdcA, cdcB, kvA, kvB) + case slashing.StoreKey: + log += DecodeSlashingStore(cdcA, cdcB, kvA, kvB) + case gov.StoreKey: + log += DecodeGovStore(cdcA, cdcB, kvA, kvB) + case distribution.StoreKey: + log += DecodeDistributionStore(cdcA, cdcB, kvA, kvB) + case supply.StoreKey: + log += DecodeSupplyStore(cdcA, cdcB, kvA, kvB) + default: + log += fmt.Sprintf("store A %X => %X\nstore B %X => %X\n", kvA.Key, kvA.Value, kvB.Key, kvB.Value) + } } - switch storeName { - case auth.StoreKey: - return DecodeAccountStore(cdcA, cdcB, kvA, kvB) - case mint.StoreKey: - return DecodeMintStore(cdcA, cdcB, kvA, kvB) - case staking.StoreKey: - return DecodeStakingStore(cdcA, cdcB, kvA, kvB) - case slashing.StoreKey: - return DecodeSlashingStore(cdcA, cdcB, kvA, kvB) - case gov.StoreKey: - return DecodeGovStore(cdcA, cdcB, kvA, kvB) - case distribution.StoreKey: - return DecodeDistributionStore(cdcA, cdcB, kvA, kvB) - case supply.StoreKey: - return DecodeSupplyStore(cdcA, cdcB, kvA, kvB) - default: - return - } + return } // DecodeAccountStore unmarshals the KVPair's Value to the corresponding auth type diff --git a/simapp/utils_test.go b/simapp/utils_test.go index b76189728..2fb0fbab8 100644 --- a/simapp/utils_test.go +++ b/simapp/utils_test.go @@ -48,23 +48,47 @@ func TestGetSimulationLog(t *testing.T) { cdc := makeTestCodec() tests := []struct { - store string - kvPair cmn.KVPair + store string + kvPairs []cmn.KVPair }{ - {auth.StoreKey, cmn.KVPair{Key: auth.AddressStoreKey(delAddr1), Value: cdc.MustMarshalBinaryBare(auth.BaseAccount{})}}, - {mint.StoreKey, cmn.KVPair{Key: mint.MinterKey, Value: cdc.MustMarshalBinaryLengthPrefixed(mint.Minter{})}}, - {staking.StoreKey, cmn.KVPair{Key: staking.LastValidatorPowerKey, Value: valAddr1.Bytes()}}, - {gov.StoreKey, cmn.KVPair{Key: gov.VoteKey(1, delAddr1), Value: cdc.MustMarshalBinaryLengthPrefixed(gov.Vote{})}}, - {distribution.StoreKey, cmn.KVPair{Key: distr.ProposerKey, Value: consAddr1.Bytes()}}, - {slashing.StoreKey, cmn.KVPair{Key: slashing.GetValidatorMissedBlockBitArrayKey(consAddr1, 6), Value: cdc.MustMarshalBinaryLengthPrefixed(true)}}, - {supply.StoreKey, cmn.KVPair{Key: supply.SupplyKey, Value: cdc.MustMarshalBinaryLengthPrefixed(supply.NewSupply(sdk.Coins{}))}}, - {"Empty", cmn.KVPair{}}, - {"OtherStore", cmn.KVPair{Key: []byte("key"), Value: []byte("value")}}, + {auth.StoreKey, []cmn.KVPair{ + cmn.KVPair{Key: auth.AddressStoreKey(delAddr1), Value: cdc.MustMarshalBinaryBare(auth.BaseAccount{})}, + cmn.KVPair{Key: auth.AddressStoreKey(delAddr1), Value: cdc.MustMarshalBinaryBare(auth.BaseAccount{})}, + }}, + {mint.StoreKey, []cmn.KVPair{ + cmn.KVPair{Key: mint.MinterKey, Value: cdc.MustMarshalBinaryLengthPrefixed(mint.Minter{})}, + cmn.KVPair{Key: mint.MinterKey, Value: cdc.MustMarshalBinaryLengthPrefixed(mint.Minter{})}, + }}, + {staking.StoreKey, []cmn.KVPair{ + cmn.KVPair{Key: staking.LastValidatorPowerKey, Value: valAddr1.Bytes()}, + cmn.KVPair{Key: staking.LastValidatorPowerKey, Value: valAddr1.Bytes()}, + }}, + {gov.StoreKey, []cmn.KVPair{ + cmn.KVPair{Key: gov.VoteKey(1, delAddr1), Value: cdc.MustMarshalBinaryLengthPrefixed(gov.Vote{})}, + cmn.KVPair{Key: gov.VoteKey(1, delAddr1), Value: cdc.MustMarshalBinaryLengthPrefixed(gov.Vote{})}, + }}, + {distribution.StoreKey, []cmn.KVPair{ + cmn.KVPair{Key: distr.ProposerKey, Value: consAddr1.Bytes()}, + cmn.KVPair{Key: distr.ProposerKey, Value: consAddr1.Bytes()}, + }}, + {slashing.StoreKey, []cmn.KVPair{ + cmn.KVPair{Key: slashing.GetValidatorMissedBlockBitArrayKey(consAddr1, 6), Value: cdc.MustMarshalBinaryLengthPrefixed(true)}, + cmn.KVPair{Key: slashing.GetValidatorMissedBlockBitArrayKey(consAddr1, 6), Value: cdc.MustMarshalBinaryLengthPrefixed(true)}, + }}, + {supply.StoreKey, []cmn.KVPair{ + cmn.KVPair{Key: supply.SupplyKey, Value: cdc.MustMarshalBinaryLengthPrefixed(supply.NewSupply(sdk.Coins{}))}, + cmn.KVPair{Key: supply.SupplyKey, Value: cdc.MustMarshalBinaryLengthPrefixed(supply.NewSupply(sdk.Coins{}))}, + }}, + {"Empty", []cmn.KVPair{cmn.KVPair{}, cmn.KVPair{}}}, + {"OtherStore", []cmn.KVPair{ + cmn.KVPair{Key: []byte("key"), Value: []byte("value")}, + cmn.KVPair{Key: []byte("key"), Value: []byte("other_value")}, + }}, } for _, tt := range tests { t.Run(tt.store, func(t *testing.T) { - require.NotPanics(t, func() { GetSimulationLog(tt.store, cdc, cdc, tt.kvPair, tt.kvPair) }, tt.store) + require.NotPanics(t, func() { GetSimulationLog(tt.store, cdc, cdc, tt.kvPairs) }, tt.store) }) } } diff --git a/store/types/utils.go b/store/types/utils.go index 20e7d2603..39bc7da66 100644 --- a/store/types/utils.go +++ b/store/types/utils.go @@ -16,13 +16,12 @@ func KVStoreReversePrefixIterator(kvs KVStore, prefix []byte) Iterator { return kvs.ReverseIterator(prefix, PrefixEndBytes(prefix)) } -// Compare two KVstores, return either the first key/value pair -// at which they differ and whether or not they are equal, skipping -// value comparison for a set of provided prefixes -func DiffKVStores(a KVStore, b KVStore, prefixesToSkip [][]byte) (kvA cmn.KVPair, kvB cmn.KVPair, count int64, equal bool) { +// DiffKVStores compares two KVstores and returns all the key/value pairs +// that differ from one another. It also skips value comparison for a set of provided prefixes +func DiffKVStores(a KVStore, b KVStore, prefixesToSkip [][]byte) (kvs []cmn.KVPair) { iterA := a.Iterator(nil, nil) iterB := b.Iterator(nil, nil) - count = int64(0) + for { if !iterA.Valid() && !iterB.Valid() { break @@ -37,7 +36,7 @@ func DiffKVStores(a KVStore, b KVStore, prefixesToSkip [][]byte) (kvA cmn.KVPair iterB.Next() } if !bytes.Equal(kvA.Key, kvB.Key) { - return kvA, kvB, count, false + kvs = append(kvs, []cmn.KVPair{kvA, kvB}...) } compareValue := true for _, prefix := range prefixesToSkip { @@ -47,11 +46,10 @@ func DiffKVStores(a KVStore, b KVStore, prefixesToSkip [][]byte) (kvA cmn.KVPair } } if compareValue && !bytes.Equal(kvA.Value, kvB.Value) { - return kvA, kvB, count, false + kvs = append(kvs, []cmn.KVPair{kvA, kvB}...) } - count++ } - return cmn.KVPair{}, cmn.KVPair{}, count, true + return } // PrefixEndBytes returns the []byte that would end a diff --git a/types/store.go b/types/store.go index 1b5b20213..c7e657614 100644 --- a/types/store.go +++ b/types/store.go @@ -34,10 +34,9 @@ func KVStoreReversePrefixIterator(kvs KVStore, prefix []byte) Iterator { return types.KVStoreReversePrefixIterator(kvs, prefix) } -// Compare two KVstores, return either the first key/value pair -// at which they differ and whether or not they are equal, skipping -// value comparison for a set of provided prefixes -func DiffKVStores(a KVStore, b KVStore, prefixesToSkip [][]byte) (kvA cmn.KVPair, kvB cmn.KVPair, count int64, equal bool) { +// DiffKVStores compares two KVstores and returns all the key/value pairs +// that differ from one another. It also skips value comparison for a set of provided prefixes +func DiffKVStores(a KVStore, b KVStore, prefixesToSkip [][]byte) []cmn.KVPair { return types.DiffKVStores(a, b, prefixesToSkip) }