Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend DiffKVStores to return a list of KVPairs #4729

Merged
merged 4 commits into from
Jul 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .pending/improvements/sdk/4640-extend-DiffKVSt
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
Original file line number Diff line number Diff line change
Expand Up @@ -498,9 +498,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
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
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
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
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