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

refactor simulation decoder #6037

Merged
merged 8 commits into from
Apr 21, 2020
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ older clients.

### API Breaking Changes

* (modules) [\#5664](https://github.com/cosmos/cosmos-sdk/pull/5664) Remove amino `Codec` from simulation `StoreDecoder`, which now returns a function closure in order to unmarshal the key-value pairs.
* (x/auth) [\#6029](https://github.com/cosmos/cosmos-sdk/pull/6029) Module accounts have been moved from `x/supply` to `x/auth`.
* (x/supply) [\#6010](https://github.com/cosmos/cosmos-sdk/pull/6010) All `x/supply` types and APIs have been moved to `x/bank`.
* (baseapp) [\#5865](https://github.com/cosmos/cosmos-sdk/pull/5865) The `SimulationResponse` returned from tx simulation is now JSON encoded instead of Amino binary.
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ test-ledger: test-ledger-mock
@go test -mod=readonly -v `go list github.com/cosmos/cosmos-sdk/crypto` -tags='cgo ledger'

test-unit:
@VERSION=$(VERSION) go test -mod=readonly $(PACKAGES_NOSIMULATION) -tags='ledger test_ledger_mock'
@VERSION=$(VERSION) go test -mod=readonly ./... -tags='ledger test_ledger_mock'

test-race:
@VERSION=$(VERSION) go test -mod=readonly -race $(PACKAGES_NOSIMULATION)
Expand Down
28 changes: 14 additions & 14 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,15 +261,15 @@ func NewSimApp(
// must be passed by reference here.
app.mm = module.NewManager(
genutil.NewAppModule(app.AccountKeeper, app.StakingKeeper, app.BaseApp.DeliverTx),
auth.NewAppModule(app.AccountKeeper),
bank.NewAppModule(app.BankKeeper, app.AccountKeeper),
auth.NewAppModule(appCodec, app.AccountKeeper),
bank.NewAppModule(appCodec, app.BankKeeper, app.AccountKeeper),
capability.NewAppModule(*app.CapabilityKeeper),
crisis.NewAppModule(&app.CrisisKeeper),
gov.NewAppModule(app.GovKeeper, app.AccountKeeper, app.BankKeeper),
mint.NewAppModule(app.MintKeeper, app.AccountKeeper),
slashing.NewAppModule(app.SlashingKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper),
distr.NewAppModule(app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper),
staking.NewAppModule(app.StakingKeeper, app.AccountKeeper, app.BankKeeper),
gov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper),
mint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper),
slashing.NewAppModule(appCodec, app.SlashingKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper),
distr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper),
staking.NewAppModule(appCodec, app.StakingKeeper, app.AccountKeeper, app.BankKeeper),
upgrade.NewAppModule(app.UpgradeKeeper),
evidence.NewAppModule(app.EvidenceKeeper),
ibc.NewAppModule(app.IBCKeeper),
Expand Down Expand Up @@ -302,13 +302,13 @@ func NewSimApp(
// NOTE: this is not required apps that don't use the simulator for fuzz testing
// transactions
app.sm = module.NewSimulationManager(
auth.NewAppModule(app.AccountKeeper),
bank.NewAppModule(app.BankKeeper, app.AccountKeeper),
gov.NewAppModule(app.GovKeeper, app.AccountKeeper, app.BankKeeper),
mint.NewAppModule(app.MintKeeper, app.AccountKeeper),
staking.NewAppModule(app.StakingKeeper, app.AccountKeeper, app.BankKeeper),
distr.NewAppModule(app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper),
slashing.NewAppModule(app.SlashingKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper),
auth.NewAppModule(appCodec, app.AccountKeeper),
bank.NewAppModule(appCodec, app.BankKeeper, app.AccountKeeper),
gov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper),
mint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper),
staking.NewAppModule(appCodec, app.StakingKeeper, app.AccountKeeper, app.BankKeeper),
distr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper),
slashing.NewAppModule(appCodec, app.SlashingKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper),
params.NewAppModule(app.ParamsKeeper),
)

Expand Down
2 changes: 1 addition & 1 deletion simapp/sim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func TestAppImportExport(t *testing.T) {
require.Equal(t, len(failedKVAs), len(failedKVBs), "unequal sets of key-values to compare")

fmt.Printf("compared %d different key/value pairs between %s and %s\n", len(failedKVAs), skp.A, skp.B)
require.Equal(t, len(failedKVAs), 0, GetSimulationLog(skp.A.Name(), app.SimulationManager().StoreDecoders, app.Codec(), failedKVAs, failedKVBs))
require.Equal(t, len(failedKVAs), 0, GetSimulationLog(skp.A.Name(), app.SimulationManager().StoreDecoders, failedKVAs, failedKVBs))
}
}

Expand Down
7 changes: 3 additions & 4 deletions simapp/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,21 +109,20 @@ func PrintStats(db dbm.DB) {

// 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, sdr sdk.StoreDecoderRegistry, cdc *codec.Codec, kvAs, kvBs []tmkv.Pair) (log string) {
func GetSimulationLog(storeName string, sdr sdk.StoreDecoderRegistry, kvAs, kvBs []tmkv.Pair) (log string) {
for i := 0; i < len(kvAs); i++ {

if len(kvAs[i].Value) == 0 && len(kvBs[i].Value) == 0 {
// skip if the value doesn't have any bytes
continue
}

decoder, ok := sdr[storeName]
if ok {
log += decoder(cdc, kvAs[i], kvBs[i])
log += decoder(kvAs[i], kvBs[i])
} else {
log += fmt.Sprintf("store A %X => %X\nstore B %X => %X\n", kvAs[i].Key, kvAs[i].Value, kvBs[i].Key, kvBs[i].Value)
}
}

return
return log
}
5 changes: 2 additions & 3 deletions simapp/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"github.com/stretchr/testify/require"
tmkv "github.com/tendermint/tendermint/libs/kv"

"github.com/cosmos/cosmos-sdk/codec"
codecstd "github.com/cosmos/cosmos-sdk/codec/std"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth"
Expand All @@ -17,7 +16,7 @@ func TestGetSimulationLog(t *testing.T) {
cdc := codecstd.MakeCodec(ModuleBasics)

decoders := make(sdk.StoreDecoderRegistry)
decoders[auth.StoreKey] = func(cdc *codec.Codec, kvAs, kvBs tmkv.Pair) string { return "10" }
decoders[auth.StoreKey] = func(kvAs, kvBs tmkv.Pair) string { return "10" }

tests := []struct {
store string
Expand All @@ -44,7 +43,7 @@ func TestGetSimulationLog(t *testing.T) {
for _, tt := range tests {
tt := tt
t.Run(tt.store, func(t *testing.T) {
require.Equal(t, tt.expectedLog, GetSimulationLog(tt.store, decoders, cdc, tt.kvPairs, tt.kvPairs), tt.store)
require.Equal(t, tt.expectedLog, GetSimulationLog(tt.store, decoders, tt.kvPairs, tt.kvPairs), tt.store)
})
}
}
3 changes: 1 addition & 2 deletions types/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package types
import (
tmkv "github.com/tendermint/tendermint/libs/kv"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/store/types"
)

Expand All @@ -28,7 +27,7 @@ type (

// StoreDecoderRegistry defines each of the modules store decoders. Used for ImportExport
// simulation.
type StoreDecoderRegistry map[string]func(cdc *codec.Codec, kvA, kvB tmkv.Pair) string
type StoreDecoderRegistry map[string]func(kvA, kvB tmkv.Pair) string

// Iterator over all the keys with a certain prefix in ascending order
func KVStorePrefixIterator(kvs KVStore, prefix []byte) Iterator {
Expand Down
12 changes: 7 additions & 5 deletions x/auth/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ var (
)

// AppModuleBasic defines the basic application module used by the auth module.
type AppModuleBasic struct{}
type AppModuleBasic struct {
cdc Codec
}

// Name returns the auth module's name.
func (AppModuleBasic) Name() string {
Expand Down Expand Up @@ -80,9 +82,9 @@ type AppModule struct {
}

// NewAppModule creates a new AppModule object
func NewAppModule(accountKeeper AccountKeeper) AppModule {
func NewAppModule(cdc Codec, accountKeeper AccountKeeper) AppModule {
return AppModule{
AppModuleBasic: AppModuleBasic{},
AppModuleBasic: AppModuleBasic{cdc: cdc},
accountKeeper: accountKeeper,
}
}
Expand Down Expand Up @@ -156,8 +158,8 @@ func (AppModule) RandomizedParams(r *rand.Rand) []simtypes.ParamChange {
}

// RegisterStoreDecoder registers a decoder for auth module's types
func (AppModule) RegisterStoreDecoder(sdr sdk.StoreDecoderRegistry) {
sdr[StoreKey] = simulation.DecodeStore
func (am AppModule) RegisterStoreDecoder(sdr sdk.StoreDecoderRegistry) {
sdr[StoreKey] = simulation.NewDecodeStore(am.cdc)
}

// WeightedOperations doesn't return any auth module operation.
Expand Down
48 changes: 29 additions & 19 deletions x/auth/simulation/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,39 @@ import (
"bytes"
"fmt"

gogotypes "github.com/gogo/protobuf/types"
tmkv "github.com/tendermint/tendermint/libs/kv"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/x/auth/exported"
"github.com/cosmos/cosmos-sdk/x/auth/types"
)

// DecodeStore unmarshals the KVPair's Value to the corresponding auth type
func DecodeStore(cdc *codec.Codec, kvA, kvB tmkv.Pair) string {
switch {
case bytes.Equal(kvA.Key[:1], types.AddressStoreKeyPrefix):
var accA, accB exported.Account
cdc.MustUnmarshalBinaryBare(kvA.Value, &accA)
cdc.MustUnmarshalBinaryBare(kvB.Value, &accB)
return fmt.Sprintf("%v\n%v", accA, accB)

case bytes.Equal(kvA.Key, types.GlobalAccountNumberKey):
var globalAccNumberA, globalAccNumberB uint64
cdc.MustUnmarshalBinaryBare(kvA.Value, &globalAccNumberA)
cdc.MustUnmarshalBinaryBare(kvB.Value, &globalAccNumberB)
return fmt.Sprintf("GlobalAccNumberA: %d\nGlobalAccNumberB: %d", globalAccNumberA, globalAccNumberB)

default:
panic(fmt.Sprintf("invalid account key %X", kvA.Key))
// NewDecodeStore returns a decoder function closure that unmarshals the KVPair's
// Value to the corresponding auth type.
func NewDecodeStore(cdc types.Codec) func(kvA, kvB tmkv.Pair) string {
return func(kvA, kvB tmkv.Pair) string {
switch {
case bytes.Equal(kvA.Key[:1], types.AddressStoreKeyPrefix):
accA, err := cdc.UnmarshalAccount(kvA.Value)
if err != nil {
panic(err)
}

accB, err := cdc.UnmarshalAccount(kvB.Value)
if err != nil {
panic(err)
}

return fmt.Sprintf("%v\n%v", accA, accB)

case bytes.Equal(kvA.Key, types.GlobalAccountNumberKey):
var globalAccNumberA, globalAccNumberB gogotypes.UInt64Value
cdc.MustUnmarshalBinaryBare(kvA.Value, &globalAccNumberA)
cdc.MustUnmarshalBinaryBare(kvB.Value, &globalAccNumberB)

return fmt.Sprintf("GlobalAccNumberA: %d\nGlobalAccNumberB: %d", globalAccNumberA, globalAccNumberB)

default:
panic(fmt.Sprintf("unexpected %s key %X (%s)", types.ModuleName, kvA.Key, kvA.Key))
}
}
}
44 changes: 26 additions & 18 deletions x/auth/simulation/decoder_test.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
package simulation
package simulation_test

import (
"fmt"
"testing"

gogotypes "github.com/gogo/protobuf/types"
"github.com/stretchr/testify/require"

"github.com/tendermint/tendermint/crypto/ed25519"
tmkv "github.com/tendermint/tendermint/libs/kv"

"github.com/cosmos/cosmos-sdk/codec"
codecstd "github.com/cosmos/cosmos-sdk/codec/std"
"github.com/cosmos/cosmos-sdk/simapp"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth/simulation"
"github.com/cosmos/cosmos-sdk/x/auth/types"
)

Expand All @@ -19,23 +21,29 @@ var (
delAddr1 = sdk.AccAddress(delPk1.Address())
)

func makeTestCodec() (cdc *codec.Codec) {
cdc = codec.New()
sdk.RegisterCodec(cdc)
codec.RegisterCrypto(cdc)
types.RegisterCodec(cdc)
return
}

func TestDecodeStore(t *testing.T) {
cdc := makeTestCodec()
cdc := codecstd.NewAppCodec(codecstd.MakeCodec(simapp.ModuleBasics))
acc := types.NewBaseAccountWithAddress(delAddr1)
globalAccNumber := uint64(10)
dec := simulation.NewDecodeStore(cdc)

accBz, err := cdc.MarshalAccount(acc)
require.NoError(t, err)

globalAccNumber := gogotypes.UInt64Value{Value: 10}

kvPairs := tmkv.Pairs{
tmkv.Pair{Key: types.AddressStoreKey(delAddr1), Value: cdc.MustMarshalBinaryBare(acc)},
tmkv.Pair{Key: types.GlobalAccountNumberKey, Value: cdc.MustMarshalBinaryBare(globalAccNumber)},
tmkv.Pair{Key: []byte{0x99}, Value: []byte{0x99}},
tmkv.Pair{
Key: types.AddressStoreKey(delAddr1),
Value: accBz,
},
tmkv.Pair{
Key: types.GlobalAccountNumberKey,
Value: cdc.MustMarshalBinaryBare(&globalAccNumber),
},
tmkv.Pair{
Key: []byte{0x99},
Value: []byte{0x99},
},
}
tests := []struct {
name string
Expand All @@ -51,9 +59,9 @@ func TestDecodeStore(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
switch i {
case len(tests) - 1:
require.Panics(t, func() { DecodeStore(cdc, kvPairs[i], kvPairs[i]) }, tt.name)
require.Panics(t, func() { dec(kvPairs[i], kvPairs[i]) }, tt.name)
default:
require.Equal(t, tt.expectedLog, DecodeStore(cdc, kvPairs[i], kvPairs[i]), tt.name)
require.Equal(t, tt.expectedLog, dec(kvPairs[i], kvPairs[i]), tt.name)
}
})
}
Expand Down
12 changes: 7 additions & 5 deletions x/bank/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ var (
)

// AppModuleBasic defines the basic application module used by the bank module.
type AppModuleBasic struct{}
type AppModuleBasic struct {
cdc Codec
}

// Name returns the bank module's name.
func (AppModuleBasic) Name() string { return ModuleName }
Expand Down Expand Up @@ -78,9 +80,9 @@ type AppModule struct {
}

// NewAppModule creates a new AppModule object
func NewAppModule(keeper Keeper, accountKeeper types.AccountKeeper) AppModule {
func NewAppModule(cdc Codec, keeper Keeper, accountKeeper types.AccountKeeper) AppModule {
return AppModule{
AppModuleBasic: AppModuleBasic{},
AppModuleBasic: AppModuleBasic{cdc: cdc},
keeper: keeper,
accountKeeper: accountKeeper,
}
Expand Down Expand Up @@ -153,8 +155,8 @@ func (AppModule) RandomizedParams(r *rand.Rand) []simtypes.ParamChange {
}

// RegisterStoreDecoder registers a decoder for supply module's types
func (AppModule) RegisterStoreDecoder(sdr sdk.StoreDecoderRegistry) {
sdr[StoreKey] = simulation.DecodeStore
func (am AppModule) RegisterStoreDecoder(sdr sdk.StoreDecoderRegistry) {
sdr[StoreKey] = simulation.NewDecodeStore(am.cdc)
}

// WeightedOperations returns the all the gov module operations with their respective weights.
Expand Down
31 changes: 20 additions & 11 deletions x/bank/simulation/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,29 @@ import (

tmkv "github.com/tendermint/tendermint/libs/kv"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/x/bank/types"
)

// DecodeStore unmarshals the KVPair's values to the corresponding types.
func DecodeStore(cdc *codec.Codec, kvA, kvB tmkv.Pair) string {
switch {
case bytes.Equal(kvA.Key[:1], types.SupplyKey):
var supplyA, supplyB types.Supply
cdc.MustUnmarshalBinaryBare(kvA.Value, &supplyA)
cdc.MustUnmarshalBinaryBare(kvB.Value, &supplyB)
return fmt.Sprintf("%v\n%v", supplyB, supplyB)
// NewDecodeStore returns a function closure that unmarshals the KVPair's values
// to the corresponding types.
func NewDecodeStore(cdc types.Codec) func(kvA, kvB tmkv.Pair) string {
return func(kvA, kvB tmkv.Pair) string {
switch {
case bytes.Equal(kvA.Key[:1], types.SupplyKey):
supplyA, err := cdc.UnmarshalSupply(kvA.Value)
if err != nil {
panic(err)
}

default:
panic(fmt.Sprintf("unknown %s key %X (%s)", types.ModuleName, kvA.Key, kvA.Key))
supplyB, err := cdc.UnmarshalSupply(kvB.Value)
if err != nil {
panic(err)
}

return fmt.Sprintf("%v\n%v", supplyA, supplyB)

default:
panic(fmt.Sprintf("unexpected %s key %X (%s)", types.ModuleName, kvA.Key, kvA.Key))
}
}
}
Loading