From bdf6d263c4d83e040842e036d67fb3e707613637 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Tue, 31 Aug 2021 01:07:31 -0400 Subject: [PATCH] perf!: Add HasAccount to the AuthKeeper to save protobuf decoding time (#10022) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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> (cherry picked from commit 7273bd39e713be8c9dd17bae71b85df495b98be0) # Conflicts: # CHANGELOG.md --- CHANGELOG.md | 30 ++++++++++++++++++++++++++++++ x/auth/keeper/account.go | 6 ++++++ x/auth/keeper/keeper.go | 3 +++ x/auth/spec/04_keepers.md | 3 +++ x/bank/keeper/send.go | 8 ++++---- x/bank/types/expected_keepers.go | 1 + 6 files changed, 47 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 988705fe6c2..a2181fd2fd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking Changes +<<<<<<< HEAD * [\#10561](https://github.com/cosmos/cosmos-sdk/pull/10561) The `CommitMultiStore` interface contains a new `SetIAVLCacheSize` method ### Bug Fixes @@ -121,6 +122,34 @@ Security Release. No breaking changes related to 0.44.x. * [\#9969](https://github.com/cosmos/cosmos-sdk/pull/9969) fix: use keyring in config for add-genesis-account cmd. * (x/genutil) [#10104](https://github.com/cosmos/cosmos-sdk/pull/10104) Ensure the `init` command reads the `--home` flag value correctly. * (x/feegrant) [\#10049](https://github.com/cosmos/cosmos-sdk/issues/10049) Fixed the error message when `period` or `period-limit` flag is not set on a feegrant grant transaction. +======= +* [\#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. +* [\#9427](https://github.com/cosmos/cosmos-sdk/pull/9427) Move simapp `FundAccount` and `FundModuleAccount` to `x/bank/testutil` +* (client/tx) [\#9421](https://github.com/cosmos/cosmos-sdk/pull/9421/) `BuildUnsignedTx`, `BuildSimTx`, `PrintUnsignedStdTx` functions are moved to + the Tx Factory as methods. +* (client/keys) [\#9407](https://github.com/cosmos/cosmos-sdk/pull/9601) Added `keys rename` CLI command and `Keyring.Rename` interface method to rename a key in the keyring. +* (x/slashing) [\#9458](https://github.com/cosmos/cosmos-sdk/pull/9458) Coins burned from slashing is now returned from Slash function and included in Slash event. +* [\#9246](https://github.com/cosmos/cosmos-sdk/pull/9246) The `New` method for the network package now returns an error. +* [\#9519](https://github.com/cosmos/cosmos-sdk/pull/9519) `DeleteDeposits` renamed to `DeleteAndBurnDeposits`, `RefundDeposits` renamed to `RefundAndDeleteDeposits` +* (codec) [\#9521](https://github.com/cosmos/cosmos-sdk/pull/9521) Removed deprecated `clientCtx.JSONCodec` from `client.Context`. +* (codec) [\#9521](https://github.com/cosmos/cosmos-sdk/pull/9521) Rename `EncodingConfig.Marshaler` to `Codec`. +* [\#9594](https://github.com/cosmos/cosmos-sdk/pull/9594) `RESTHandlerFn` argument is removed from the `gov/NewProposalHandler`. +* [\#9594](https://github.com/cosmos/cosmos-sdk/pull/9594) `types/rest` package moved to `testutil/rest`. +* [\#9432](https://github.com/cosmos/cosmos-sdk/pull/9432) `ConsensusParamsKeyTable` moved from `params/keeper` to `params/types` +* [\#9576](https://github.com/cosmos/cosmos-sdk/pull/9576) Add debug error message to `sdkerrors.QueryResult` when enabled +* [\#9650](https://github.com/cosmos/cosmos-sdk/pull/9650) Removed deprecated message handler implementation from the SDK modules. +* (x/bank) [\#9832] (https://github.com/cosmos/cosmos-sdk/pull/9832) `AddressFromBalancesStore` renamed to `AddressAndDenomFromBalancesStore`. +* (tests) [\#9938](https://github.com/cosmos/cosmos-sdk/pull/9938) `simapp.Setup` accepts additional `testing.T` argument. +* (baseapp) [\#9920](https://github.com/cosmos/cosmos-sdk/pull/9920) BaseApp `{Check,Deliver,Simulate}Tx` methods are now replaced by a middleware stack. + * Replace the Antehandler interface with the `tx.Handler` and `tx.Middleware` interfaces. + * Replace `baseapp.SetAnteHandler` with `baseapp.SetTxHandler`. + * Move Msg routers from BaseApp to middlewares. + * Move Baseapp panic recovery into a middleware. + * Rename simulation helper methods `baseapp.{Check,Deliver}` to `baseapp.Sim{Check,Deliver}`. +>>>>>>> 7273bd39e (perf!: Add HasAccount to the AuthKeeper to save protobuf decoding time (#10022)) ### Client Breaking Changes @@ -134,6 +163,7 @@ Security Release. No breaking changes related to 0.44.x. ### 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). ### Deprecated diff --git a/x/auth/keeper/account.go b/x/auth/keeper/account.go index a26ea6427bc..7474e93a546 100644 --- a/x/auth/keeper/account.go +++ b/x/auth/keeper/account.go @@ -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) diff --git a/x/auth/keeper/keeper.go b/x/auth/keeper/keeper.go index b690c33c37c..a73f97dc344 100644 --- a/x/auth/keeper/keeper.go +++ b/x/auth/keeper/keeper.go @@ -22,6 +22,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 diff --git a/x/auth/spec/04_keepers.md b/x/auth/spec/04_keepers.md index 8e4466cd68a..fcd99544754 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 diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index 369fa631c44..80fdb555b33 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -118,8 +118,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)) } @@ -145,8 +145,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)) } diff --git a/x/bank/types/expected_keepers.go b/x/bank/types/expected_keepers.go index 7307864b86e..23ca020a34b 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)