From 1ab5d9c0ff7027059b0c7d239bccfda02d0ed1eb Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Tue, 18 Jun 2019 16:14:19 -0700 Subject: [PATCH 1/4] add expected keeper for account keeper in bank module --- x/bank/internal/keeper/invariants.go | 6 ++-- x/bank/internal/keeper/keeper.go | 34 +++++++++++------------ x/bank/internal/types/expected_keepers.go | 17 ++++++++++++ x/bank/module.go | 6 ++-- x/bank/simulation.go | 13 ++++----- 5 files changed, 46 insertions(+), 30 deletions(-) create mode 100644 x/bank/internal/types/expected_keepers.go diff --git a/x/bank/internal/keeper/invariants.go b/x/bank/internal/keeper/invariants.go index d401c27f5073..ac53e5040199 100644 --- a/x/bank/internal/keeper/invariants.go +++ b/x/bank/internal/keeper/invariants.go @@ -10,13 +10,13 @@ import ( ) // register bank invariants -func RegisterInvariants(ir sdk.InvariantRegistry, ak auth.AccountKeeper) { +func RegisterInvariants(ir sdk.InvariantRegistry, ak types.AccountKeeper) { ir.RegisterRoute(types.ModuleName, "nonnegative-outstanding", NonnegativeBalanceInvariant(ak)) } // NonnegativeBalanceInvariant checks that all accounts in the application have non-negative balances -func NonnegativeBalanceInvariant(ak auth.AccountKeeper) sdk.Invariant { +func NonnegativeBalanceInvariant(ak types.AccountKeeper) sdk.Invariant { return func(ctx sdk.Context) error { accts := ak.GetAllAccounts(ctx) for _, acc := range accts { @@ -33,7 +33,7 @@ func NonnegativeBalanceInvariant(ak auth.AccountKeeper) sdk.Invariant { // TotalCoinsInvariant checks that the sum of the coins across all accounts // is what is expected -func TotalCoinsInvariant(ak auth.AccountKeeper, totalSupplyFn func() sdk.Coins) sdk.Invariant { +func TotalCoinsInvariant(ak types.AccountKeeper, totalSupplyFn func() sdk.Coins) sdk.Invariant { return func(ctx sdk.Context) error { totalCoins := sdk.NewCoins() diff --git a/x/bank/internal/keeper/keeper.go b/x/bank/internal/keeper/keeper.go index 718569fadaef..89ecbad56d0c 100644 --- a/x/bank/internal/keeper/keeper.go +++ b/x/bank/internal/keeper/keeper.go @@ -30,12 +30,12 @@ type Keeper interface { type BaseKeeper struct { BaseSendKeeper - ak auth.AccountKeeper + ak types.AccountKeeper paramSpace params.Subspace } // NewBaseKeeper returns a new BaseKeeper -func NewBaseKeeper(ak auth.AccountKeeper, +func NewBaseKeeper(ak types.AccountKeeper, paramSpace params.Subspace, codespace sdk.CodespaceType) BaseKeeper { @@ -133,12 +133,12 @@ var _ SendKeeper = (*BaseSendKeeper)(nil) type BaseSendKeeper struct { BaseViewKeeper - ak auth.AccountKeeper + ak types.AccountKeeper paramSpace params.Subspace } // NewBaseSendKeeper returns a new BaseSendKeeper. -func NewBaseSendKeeper(ak auth.AccountKeeper, +func NewBaseSendKeeper(ak types.AccountKeeper, paramSpace params.Subspace, codespace sdk.CodespaceType) BaseSendKeeper { return BaseSendKeeper{ @@ -185,12 +185,12 @@ type ViewKeeper interface { // BaseViewKeeper implements a read only keeper implementation of ViewKeeper. type BaseViewKeeper struct { - ak auth.AccountKeeper + ak types.AccountKeeper codespace sdk.CodespaceType } // NewBaseViewKeeper returns a new BaseViewKeeper. -func NewBaseViewKeeper(ak auth.AccountKeeper, codespace sdk.CodespaceType) BaseViewKeeper { +func NewBaseViewKeeper(ak types.AccountKeeper, codespace sdk.CodespaceType) BaseViewKeeper { return BaseViewKeeper{ak: ak, codespace: codespace} } @@ -209,7 +209,7 @@ func (keeper BaseViewKeeper) Codespace() sdk.CodespaceType { return keeper.codespace } -func getCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress) sdk.Coins { +func getCoins(ctx sdk.Context, am types.AccountKeeper, addr sdk.AccAddress) sdk.Coins { acc := am.GetAccount(ctx, addr) if acc == nil { return sdk.NewCoins() @@ -217,7 +217,7 @@ func getCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress) sdk.C return acc.GetCoins() } -func setCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) sdk.Error { +func setCoins(ctx sdk.Context, am types.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) sdk.Error { if !amt.IsValid() { return sdk.ErrInvalidCoins(amt.String()) } @@ -235,22 +235,22 @@ func setCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt s } // HasCoins returns whether or not an account has at least amt coins. -func hasCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) bool { +func hasCoins(ctx sdk.Context, am types.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) bool { return getCoins(ctx, am, addr).IsAllGTE(amt) } -func getAccount(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress) auth.Account { +func getAccount(ctx sdk.Context, ak types.AccountKeeper, addr sdk.AccAddress) auth.Account { return ak.GetAccount(ctx, addr) } -func setAccount(ctx sdk.Context, ak auth.AccountKeeper, acc auth.Account) { +func setAccount(ctx sdk.Context, ak types.AccountKeeper, acc auth.Account) { ak.SetAccount(ctx, acc) } // subtractCoins subtracts amt coins from an account with the given address addr. // // CONTRACT: If the account is a vesting account, the amount has to be spendable. -func subtractCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Error) { +func subtractCoins(ctx sdk.Context, ak types.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Error) { if !amt.IsValid() { return nil, sdk.ErrInvalidCoins(amt.String()) @@ -280,7 +280,7 @@ func subtractCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, } // AddCoins adds amt to the coins at the addr. -func addCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Error) { +func addCoins(ctx sdk.Context, am types.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Error) { if !amt.IsValid() { return nil, sdk.ErrInvalidCoins(amt.String()) @@ -302,7 +302,7 @@ func addCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt s // SendCoins moves coins from one account to another // Returns ErrInvalidCoins if amt is invalid. -func sendCoins(ctx sdk.Context, am auth.AccountKeeper, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) sdk.Error { +func sendCoins(ctx sdk.Context, am types.AccountKeeper, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) sdk.Error { // Safety check ensuring that when sending coins the keeper must maintain the if !amt.IsValid() { return sdk.ErrInvalidCoins(amt.String()) @@ -323,7 +323,7 @@ func sendCoins(ctx sdk.Context, am auth.AccountKeeper, fromAddr sdk.AccAddress, // InputOutputCoins handles a list of inputs and outputs // NOTE: Make sure to revert state changes from tx on error -func inputOutputCoins(ctx sdk.Context, am auth.AccountKeeper, inputs []types.Input, outputs []types.Output) (sdk.Tags, sdk.Error) { +func inputOutputCoins(ctx sdk.Context, am types.AccountKeeper, inputs []types.Input, outputs []types.Output) (sdk.Tags, sdk.Error) { // Safety check ensuring that when sending coins the keeper must maintain the // Check supply invariant and validity of Coins. if err := types.ValidateInputsOutputs(inputs, outputs); err != nil { @@ -357,7 +357,7 @@ func inputOutputCoins(ctx sdk.Context, am auth.AccountKeeper, inputs []types.Inp } func delegateCoins( - ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins, + ctx sdk.Context, ak types.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins, ) (sdk.Tags, sdk.Error) { if !amt.IsValid() { @@ -391,7 +391,7 @@ func delegateCoins( } func undelegateCoins( - ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins, + ctx sdk.Context, ak types.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins, ) (sdk.Tags, sdk.Error) { if !amt.IsValid() { diff --git a/x/bank/internal/types/expected_keepers.go b/x/bank/internal/types/expected_keepers.go new file mode 100644 index 000000000000..a1a85ed23b65 --- /dev/null +++ b/x/bank/internal/types/expected_keepers.go @@ -0,0 +1,17 @@ +package types + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth" +) + +// expected account keeper +type AccountKeeper interface { + NewAccountWithAddress(ctx sdk.Context, addr sdk.AccAddress) auth.Account + + GetAccount(ctx sdk.Context, addr sdk.AccAddress) auth.Account + GetAllAccounts(ctx sdk.Context) []auth.Account + SetAccount(ctx sdk.Context, acc auth.Account) + + IterateAccounts(ctx sdk.Context, process func(auth.Account) bool) +} diff --git a/x/bank/module.go b/x/bank/module.go index b75ed286c6a8..bcb415a90681 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -12,10 +12,10 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" - "github.com/cosmos/cosmos-sdk/x/auth" "github.com/cosmos/cosmos-sdk/x/bank/client/cli" "github.com/cosmos/cosmos-sdk/x/bank/client/rest" "github.com/cosmos/cosmos-sdk/x/bank/internal/keeper" + "github.com/cosmos/cosmos-sdk/x/bank/internal/types" ) var ( @@ -65,11 +65,11 @@ func (AppModuleBasic) GetQueryCmd(_ *codec.Codec) *cobra.Command { return nil } type AppModule struct { AppModuleBasic keeper Keeper - accountKeeper auth.AccountKeeper + accountKeeper types.AccountKeeper } // NewAppModule creates a new AppModule object -func NewAppModule(keeper Keeper, accountKeeper auth.AccountKeeper) AppModule { +func NewAppModule(keeper Keeper, accountKeeper types.AccountKeeper) AppModule { return AppModule{ AppModuleBasic: AppModuleBasic{}, keeper: keeper, diff --git a/x/bank/simulation.go b/x/bank/simulation.go index 3fe611a9d067..85532502eaab 100644 --- a/x/bank/simulation.go +++ b/x/bank/simulation.go @@ -8,7 +8,6 @@ import ( "github.com/cosmos/cosmos-sdk/baseapp" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/auth" "github.com/cosmos/cosmos-sdk/x/bank/internal/keeper" "github.com/cosmos/cosmos-sdk/x/bank/internal/types" "github.com/cosmos/cosmos-sdk/x/mock" @@ -17,7 +16,7 @@ import ( // SendTx tests and runs a single msg send where both // accounts already exist. -func SimulateMsgSend(mapper auth.AccountKeeper, bk keeper.Keeper) simulation.Operation { +func SimulateMsgSend(mapper types.AccountKeeper, bk keeper.Keeper) simulation.Operation { handler := NewHandler(bk) return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simulation.Account) ( opMsg simulation.OperationMsg, fOps []simulation.FutureOperation, err error) { @@ -35,7 +34,7 @@ func SimulateMsgSend(mapper auth.AccountKeeper, bk keeper.Keeper) simulation.Ope } } -func createMsgSend(r *rand.Rand, ctx sdk.Context, accs []simulation.Account, mapper auth.AccountKeeper) ( +func createMsgSend(r *rand.Rand, ctx sdk.Context, accs []simulation.Account, mapper types.AccountKeeper) ( fromAcc simulation.Account, comment string, msg types.MsgSend, ok bool) { fromAcc = simulation.RandomAcc(r, accs) @@ -65,7 +64,7 @@ func createMsgSend(r *rand.Rand, ctx sdk.Context, accs []simulation.Account, map } // Sends and verifies the transition of a msg send. -func sendAndVerifyMsgSend(app *baseapp.BaseApp, mapper auth.AccountKeeper, msg types.MsgSend, ctx sdk.Context, privkeys []crypto.PrivKey, handler sdk.Handler) error { +func sendAndVerifyMsgSend(app *baseapp.BaseApp, mapper types.AccountKeeper, msg types.MsgSend, ctx sdk.Context, privkeys []crypto.PrivKey, handler sdk.Handler) error { fromAcc := mapper.GetAccount(ctx, msg.FromAddress) AccountNumbers := []uint64{fromAcc.GetAccountNumber()} SequenceNumbers := []uint64{fromAcc.GetSequence()} @@ -111,7 +110,7 @@ func sendAndVerifyMsgSend(app *baseapp.BaseApp, mapper auth.AccountKeeper, msg t // SingleInputSendMsg tests and runs a single msg multisend, with one input and one output, where both // accounts already exist. -func SimulateSingleInputMsgMultiSend(mapper auth.AccountKeeper, bk keeper.Keeper) simulation.Operation { +func SimulateSingleInputMsgMultiSend(mapper types.AccountKeeper, bk keeper.Keeper) simulation.Operation { handler := NewHandler(bk) return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simulation.Account) ( opMsg simulation.OperationMsg, fOps []simulation.FutureOperation, err error) { @@ -129,7 +128,7 @@ func SimulateSingleInputMsgMultiSend(mapper auth.AccountKeeper, bk keeper.Keeper } } -func createSingleInputMsgMultiSend(r *rand.Rand, ctx sdk.Context, accs []simulation.Account, mapper auth.AccountKeeper) ( +func createSingleInputMsgMultiSend(r *rand.Rand, ctx sdk.Context, accs []simulation.Account, mapper types.AccountKeeper) ( fromAcc simulation.Account, comment string, msg types.MsgMultiSend, ok bool) { fromAcc = simulation.RandomAcc(r, accs) @@ -164,7 +163,7 @@ func createSingleInputMsgMultiSend(r *rand.Rand, ctx sdk.Context, accs []simulat // Sends and verifies the transition of a msg multisend. This fails if there are repeated inputs or outputs // pass in handler as nil to handle txs, otherwise handle msgs -func sendAndVerifyMsgMultiSend(app *baseapp.BaseApp, mapper auth.AccountKeeper, msg types.MsgMultiSend, +func sendAndVerifyMsgMultiSend(app *baseapp.BaseApp, mapper types.AccountKeeper, msg types.MsgMultiSend, ctx sdk.Context, privkeys []crypto.PrivKey, handler sdk.Handler) error { initialInputAddrCoins := make([]sdk.Coins, len(msg.Inputs)) From 6a6b988541811706f5a12bd8c56b1a22e8622284 Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Tue, 18 Jun 2019 16:22:17 -0700 Subject: [PATCH 2/4] add pending log --- .pending/improvements/sdk/4584-Update-bank-Kee | 1 + 1 file changed, 1 insertion(+) create mode 100644 .pending/improvements/sdk/4584-Update-bank-Kee diff --git a/.pending/improvements/sdk/4584-Update-bank-Kee b/.pending/improvements/sdk/4584-Update-bank-Kee new file mode 100644 index 000000000000..90b030075fb6 --- /dev/null +++ b/.pending/improvements/sdk/4584-Update-bank-Kee @@ -0,0 +1 @@ +#4584 Update bank Keeper to use expected keeper interface of the AccountKeeper. \ No newline at end of file From 14d02720e60dde97c6c8b4af877ac13536c9c4f8 Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Wed, 19 Jun 2019 12:01:28 -0700 Subject: [PATCH 3/4] update auth to authtypes --- x/bank/internal/keeper/invariants.go | 4 ++-- x/bank/internal/keeper/keeper.go | 14 +++++++------- x/bank/internal/types/expected_keepers.go | 12 ++++++------ 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/x/bank/internal/keeper/invariants.go b/x/bank/internal/keeper/invariants.go index ac53e5040199..8ab471580927 100644 --- a/x/bank/internal/keeper/invariants.go +++ b/x/bank/internal/keeper/invariants.go @@ -5,7 +5,7 @@ import ( "fmt" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/auth" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/cosmos/cosmos-sdk/x/bank/internal/types" ) @@ -37,7 +37,7 @@ func TotalCoinsInvariant(ak types.AccountKeeper, totalSupplyFn func() sdk.Coins) return func(ctx sdk.Context) error { totalCoins := sdk.NewCoins() - chkAccount := func(acc auth.Account) bool { + chkAccount := func(acc authtypes.Account) bool { coins := acc.GetCoins() totalCoins = totalCoins.Add(coins) return false diff --git a/x/bank/internal/keeper/keeper.go b/x/bank/internal/keeper/keeper.go index 89ecbad56d0c..1721561c7158 100644 --- a/x/bank/internal/keeper/keeper.go +++ b/x/bank/internal/keeper/keeper.go @@ -5,7 +5,7 @@ import ( "time" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/auth" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/cosmos/cosmos-sdk/x/bank/internal/types" "github.com/cosmos/cosmos-sdk/x/params" ) @@ -239,11 +239,11 @@ func hasCoins(ctx sdk.Context, am types.AccountKeeper, addr sdk.AccAddress, amt return getCoins(ctx, am, addr).IsAllGTE(amt) } -func getAccount(ctx sdk.Context, ak types.AccountKeeper, addr sdk.AccAddress) auth.Account { +func getAccount(ctx sdk.Context, ak types.AccountKeeper, addr sdk.AccAddress) authtypes.Account { return ak.GetAccount(ctx, addr) } -func setAccount(ctx sdk.Context, ak types.AccountKeeper, acc auth.Account) { +func setAccount(ctx sdk.Context, ak types.AccountKeeper, acc authtypes.Account) { ak.SetAccount(ctx, acc) } @@ -416,8 +416,8 @@ func undelegateCoins( } // CONTRACT: assumes that amt is valid. -func trackDelegation(acc auth.Account, blockTime time.Time, amt sdk.Coins) error { - vacc, ok := acc.(auth.VestingAccount) +func trackDelegation(acc authtypes.Account, blockTime time.Time, amt sdk.Coins) error { + vacc, ok := acc.(authtypes.VestingAccount) if ok { vacc.TrackDelegation(blockTime, amt) return nil @@ -427,8 +427,8 @@ func trackDelegation(acc auth.Account, blockTime time.Time, amt sdk.Coins) error } // CONTRACT: assumes that amt is valid. -func trackUndelegation(acc auth.Account, amt sdk.Coins) error { - vacc, ok := acc.(auth.VestingAccount) +func trackUndelegation(acc authtypes.Account, amt sdk.Coins) error { + vacc, ok := acc.(authtypes.VestingAccount) if ok { vacc.TrackUndelegation(amt) return nil diff --git a/x/bank/internal/types/expected_keepers.go b/x/bank/internal/types/expected_keepers.go index a1a85ed23b65..ff6aa0c1985c 100644 --- a/x/bank/internal/types/expected_keepers.go +++ b/x/bank/internal/types/expected_keepers.go @@ -2,16 +2,16 @@ package types import ( sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/auth" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" ) // expected account keeper type AccountKeeper interface { - NewAccountWithAddress(ctx sdk.Context, addr sdk.AccAddress) auth.Account + NewAccountWithAddress(ctx sdk.Context, addr sdk.AccAddress) authtypes.Account - GetAccount(ctx sdk.Context, addr sdk.AccAddress) auth.Account - GetAllAccounts(ctx sdk.Context) []auth.Account - SetAccount(ctx sdk.Context, acc auth.Account) + GetAccount(ctx sdk.Context, addr sdk.AccAddress) authtypes.Account + GetAllAccounts(ctx sdk.Context) []authtypes.Account + SetAccount(ctx sdk.Context, acc authtypes.Account) - IterateAccounts(ctx sdk.Context, process func(auth.Account) bool) + IterateAccounts(ctx sdk.Context, process func(authtypes.Account) bool) } From abe0591337916a1077370ed75d72caf95fa2f91e Mon Sep 17 00:00:00 2001 From: colin axner Date: Wed, 19 Jun 2019 13:07:52 -0700 Subject: [PATCH 4/4] Update account keeper doc Co-Authored-By: Federico Kunze <31522760+fedekunze@users.noreply.github.com> --- x/bank/internal/types/expected_keepers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/bank/internal/types/expected_keepers.go b/x/bank/internal/types/expected_keepers.go index ff6aa0c1985c..3c07e3612fc3 100644 --- a/x/bank/internal/types/expected_keepers.go +++ b/x/bank/internal/types/expected_keepers.go @@ -5,7 +5,7 @@ import ( authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" ) -// expected account keeper +// AccountKeeper defines the expected account keeper type AccountKeeper interface { NewAccountWithAddress(ctx sdk.Context, addr sdk.AccAddress) authtypes.Account