Skip to content

Commit

Permalink
perf!: Add HasAccount to the AuthKeeper to save protobuf decoding time (
Browse files Browse the repository at this point in the history
#10022)

* 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

* Update Spec

* Add Changelog entry

* Fix lint & use speedup in SendCoins

* Update x/auth/keeper/account.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
  • Loading branch information
ValarDragon and fedekunze committed Aug 31, 2021
1 parent 78c3c4e commit 7273bd3
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.

Expand Down
6 changes: 6 additions & 0 deletions x/auth/keeper/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ 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)
return store.Has(types.AddressStoreKey(addr))
}

// GetAccount implements AccountKeeperI.
func (ak AccountKeeper) GetAccount(ctx sdk.Context, addr sdk.AccAddress) types.AccountI {
store := ctx.KVStore(ak.key)
Expand Down
3 changes: 3 additions & 0 deletions x/auth/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,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

Expand Down
3 changes: 3 additions & 0 deletions x/auth/spec/04_keepers.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 4 additions & 4 deletions x/bank/keeper/send.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand All @@ -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))
}
Expand Down
1 change: 1 addition & 0 deletions x/bank/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 7273bd3

Please sign in to comment.