Skip to content

Commit

Permalink
Merge PR #4729: Extend DiffKVStores to return a list of KVPairs
Browse files Browse the repository at this point in the history
  • Loading branch information
@fedekunze @alexanderbez
fedekunze authored and alexanderbez committed Jul 19, 2019
1 parent b45a94a commit b17f8f8
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 50 deletions.
2 changes: 2 additions & 0 deletions .pending/improvements/sdk/4640-extend-DiffKVSt
View file
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 3 additions & 3 deletions simapp/sim_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

}
Expand Down
51 changes: 29 additions & 22 deletions simapp/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
// skip if the value doesn't have any bytes
continue
}

if len(kvA.Value) == 0 && len(kvB.Value) == 0 {
return
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
Expand Down
48 changes: 36 additions & 12 deletions simapp/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
Expand Down
16 changes: 7 additions & 9 deletions store/types/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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
Expand Down
7 changes: 3 additions & 4 deletions types/store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down

0 comments on commit b17f8f8

Please sign in to comment.