From 35544bd2fba1c3ca438802ec7c13b5b50eed00dd Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Sat, 28 Aug 2021 16:40:38 -0400 Subject: [PATCH 1/5] Add HasAccount to the AuthKeeper to save protobuf decoding time We found in the Osmosis epoch time, the many accesses to GetAccount's proto unmarshalling was a significant slowdown. This adds a HasAccount method to the AuthKeeper, and fixes one unnecessary spot that it appears within in SendCoins --- x/auth/keeper/account.go | 11 +++++++++++ x/auth/keeper/keeper.go | 3 +++ x/bank/keeper/send.go | 4 ++-- x/bank/types/expected_keepers.go | 1 + 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/x/auth/keeper/account.go b/x/auth/keeper/account.go index a26ea6427bc4..57a6d9705260 100644 --- a/x/auth/keeper/account.go +++ b/x/auth/keeper/account.go @@ -25,6 +25,17 @@ func (ak AccountKeeper) NewAccount(ctx sdk.Context, acc types.AccountI) types.Ac return acc } +// HasAccount implements AccountKeeperI. +func (ak AccountKeeper) HasAccount(ctx sdk.Context, addr sdk.AccAddress) bool { + store := ctx.KVStore(ak.key) + bz := store.Get(types.AddressStoreKey(addr)) + if bz == nil { + return false + } + + return true +} + // GetAccount implements AccountKeeperI. func (ak AccountKeeper) GetAccount(ctx sdk.Context, addr sdk.AccAddress) types.AccountI { store := ctx.KVStore(ak.key) diff --git a/x/auth/keeper/keeper.go b/x/auth/keeper/keeper.go index 8b8ba835c769..5dffe88f7f59 100644 --- a/x/auth/keeper/keeper.go +++ b/x/auth/keeper/keeper.go @@ -28,6 +28,9 @@ type AccountKeeperI interface { // Retrieve an account from the store. GetAccount(sdk.Context, sdk.AccAddress) types.AccountI + // Check if an account exists in the store. + HasAccount(sdk.Context, sdk.AccAddress) bool + // Set an account in the store. SetAccount(sdk.Context, types.AccountI) diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index 17427cabba7c..fba591c2f7bc 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -120,8 +120,8 @@ func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, inputs []types.Input, // // NOTE: This should ultimately be removed in favor a more flexible approach // such as delegated fee messages. - acc := k.ak.GetAccount(ctx, outAddress) - if acc == nil { + accExists := k.ak.HasAccount(ctx, outAddress) + if !accExists { defer telemetry.IncrCounter(1, "new", "account") k.ak.SetAccount(ctx, k.ak.NewAccountWithAddress(ctx, outAddress)) } diff --git a/x/bank/types/expected_keepers.go b/x/bank/types/expected_keepers.go index 7307864b86e6..23ca020a34b0 100644 --- a/x/bank/types/expected_keepers.go +++ b/x/bank/types/expected_keepers.go @@ -13,6 +13,7 @@ type AccountKeeper interface { GetAccount(ctx sdk.Context, addr sdk.AccAddress) types.AccountI GetAllAccounts(ctx sdk.Context) []types.AccountI + HasAccount(ctx sdk.Context, addr sdk.AccAddress) bool SetAccount(ctx sdk.Context, acc types.AccountI) IterateAccounts(ctx sdk.Context, process func(types.AccountI) bool) From 177a6d041f157e5287fb8094bd1d7ba8ee47d725 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Sat, 28 Aug 2021 16:51:49 -0400 Subject: [PATCH 2/5] Update Spec --- x/auth/keeper/keeper.go | 6 +++--- x/auth/spec/04_keepers.md | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/x/auth/keeper/keeper.go b/x/auth/keeper/keeper.go index 5dffe88f7f59..10ce4f0be2aa 100644 --- a/x/auth/keeper/keeper.go +++ b/x/auth/keeper/keeper.go @@ -25,12 +25,12 @@ type AccountKeeperI interface { // Return a new account with the next account number. Does not save the new account to the store. NewAccount(sdk.Context, types.AccountI) types.AccountI - // Retrieve an account from the store. - GetAccount(sdk.Context, sdk.AccAddress) types.AccountI - // Check if an account exists in the store. HasAccount(sdk.Context, sdk.AccAddress) bool + // Retrieve an account from the store. + GetAccount(sdk.Context, sdk.AccAddress) types.AccountI + // Set an account in the store. SetAccount(sdk.Context, types.AccountI) diff --git a/x/auth/spec/04_keepers.md b/x/auth/spec/04_keepers.md index 40882e24e571..268ced5c3eb8 100644 --- a/x/auth/spec/04_keepers.md +++ b/x/auth/spec/04_keepers.md @@ -20,6 +20,9 @@ type AccountKeeperI interface { // Return a new account with the next account number. Does not save the new account to the store. NewAccount(sdk.Context, types.AccountI) types.AccountI + // Check if an account exists in the store. + HasAccount(sdk.Context, sdk.AccAddress) bool + // Retrieve an account from the store. GetAccount(sdk.Context, sdk.AccAddress) types.AccountI From 2dd181344f36ae5380d445c7afba39aa78cd535b Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Sat, 28 Aug 2021 16:54:34 -0400 Subject: [PATCH 3/5] Add Changelog entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3ee0f67bd58..62f0ae124186 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking Changes +* [\#10022](https://github.com/cosmos/cosmos-sdk/pull/10022) `AuthKeeper` interface in `x/auth` now includes a function `HasAccount`. * [\#9759](https://github.com/cosmos/cosmos-sdk/pull/9759) `NewAccountKeeeper` in `x/auth` now takes an additional `bech32Prefix` argument that represents `sdk.Bech32MainPrefix`. * [\#9628](https://github.com/cosmos/cosmos-sdk/pull/9628) Rename `x/{mod}/legacy` to `x/{mod}/migrations`. * [\#9571](https://github.com/cosmos/cosmos-sdk/pull/9571) Implemented error handling for staking hooks, which now return an error on failure. @@ -85,6 +86,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* (x/bank) [\#10022](https://github.com/cosmos/cosmos-sdk/pull/10022) `BankKeeper.SendCoins` now takes less execution time. * (deps) [\#9956](https://github.com/cosmos/cosmos-sdk/pull/9956) Bump Tendermint to [v0.34.12](https://github.com/tendermint/tendermint/releases/tag/v0.34.12). * (cli) [\#9856](https://github.com/cosmos/cosmos-sdk/pull/9856) Overwrite `--sequence` and `--account-number` flags with default flag values when used with `offline=false` in `sign-batch` command. From f2967a0a026850456025f7dd373dd372ce9c40f5 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Sat, 28 Aug 2021 23:48:12 -0400 Subject: [PATCH 4/5] Fix lint & use speedup in SendCoins --- x/auth/keeper/account.go | 6 +----- x/bank/keeper/send.go | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/x/auth/keeper/account.go b/x/auth/keeper/account.go index 57a6d9705260..43b643e4c20f 100644 --- a/x/auth/keeper/account.go +++ b/x/auth/keeper/account.go @@ -29,11 +29,7 @@ func (ak AccountKeeper) NewAccount(ctx sdk.Context, acc types.AccountI) types.Ac func (ak AccountKeeper) HasAccount(ctx sdk.Context, addr sdk.AccAddress) bool { store := ctx.KVStore(ak.key) bz := store.Get(types.AddressStoreKey(addr)) - if bz == nil { - return false - } - - return true + return bz != nil } // GetAccount implements AccountKeeperI. diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index fba591c2f7bc..b0c38a320e0f 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -147,8 +147,8 @@ func (k BaseSendKeeper) SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAd // // NOTE: This should ultimately be removed in favor a more flexible approach // such as delegated fee messages. - acc := k.ak.GetAccount(ctx, toAddr) - if acc == nil { + accExists := k.ak.HasAccount(ctx, toAddr) + if !accExists { defer telemetry.IncrCounter(1, "new", "account") k.ak.SetAccount(ctx, k.ak.NewAccountWithAddress(ctx, toAddr)) } From ff7e8e9a4b2f308b6a8e22ad6444e01cec2b2d56 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Mon, 30 Aug 2021 12:50:38 -0400 Subject: [PATCH 5/5] Update x/auth/keeper/account.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> --- x/auth/keeper/account.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/auth/keeper/account.go b/x/auth/keeper/account.go index 43b643e4c20f..7474e93a5469 100644 --- a/x/auth/keeper/account.go +++ b/x/auth/keeper/account.go @@ -28,8 +28,7 @@ func (ak AccountKeeper) NewAccount(ctx sdk.Context, acc types.AccountI) types.Ac // HasAccount implements AccountKeeperI. func (ak AccountKeeper) HasAccount(ctx sdk.Context, addr sdk.AccAddress) bool { store := ctx.KVStore(ak.key) - bz := store.Get(types.AddressStoreKey(addr)) - return bz != nil + return store.Has(types.AddressStoreKey(addr)) } // GetAccount implements AccountKeeperI.