Skip to content

Commit

Permalink
perf!: Add HasAccount to the AuthKeeper to save protobuf decoding tim…
Browse files Browse the repository at this point in the history
…e (backport cosmos#10022) (cosmos#10847)

* perf!: Add HasAccount to the AuthKeeper to save protobuf decoding time (cosmos#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>
(cherry picked from commit 7273bd3)

# Conflicts:
#	CHANGELOG.md

* conflicts

* changelog

* changelog

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Federico Kunze Küllmer <federico.kunze94@gmail.com>
  • Loading branch information
3 people authored and JeancarloBarrios committed Sep 28, 2024
1 parent bcc086f commit dbef1a9
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 15 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (store) [#10218](https://github.com/cosmos/cosmos-sdk/pull/10218) Charge gas even when there are no entries while seeking.
* (store) [#10247](https://github.com/cosmos/cosmos-sdk/pull/10247) Charge gas for the key length in gas meter.

### API Breaking Changes

* (auth) [\#10022](https://github.com/cosmos/cosmos-sdk/pull/10022) `AuthKeeper` interface in `x/auth` now includes a function `HasAccount`.

### Improvements

* (store) [\#10741](https://github.com/cosmos/cosmos-sdk/pull/10741) Significantly speedup iterator creation after delete heavy workloads. Significantly improves IBC migration times.
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 @@ -29,6 +29,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 @@ -30,6 +30,9 @@ type AccountKeeper interface {
// Check if an account exists in the store.
HasAccount(context.Context, sdk.AccAddress) bool

// Check if an account exists in the store.
HasAccount(sdk.Context, sdk.AccAddress) bool

// Retrieve an account from the store.
GetAccount(context.Context, sdk.AccAddress) sdk.AccountI

Expand Down
38 changes: 24 additions & 14 deletions x/bank/keeper/send.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,17 +165,22 @@ func (k BaseSendKeeper) InputOutputCoins(ctx context.Context, input types.Input,
return err
}

if err := k.addCoins(ctx, outAddress, out.Coins); err != nil {
return err
}

if err := k.EventService.EventManager(ctx).EmitKV(
types.EventTypeTransfer,
event.NewAttribute(types.AttributeKeyRecipient, out.Address),
event.NewAttribute(types.AttributeKeySender, input.Address),
event.NewAttribute(sdk.AttributeKeyAmount, out.Coins.String()),
); err != nil {
return err
ctx.EventManager().EmitEvent(
sdk.NewEvent(
types.EventTypeTransfer,
sdk.NewAttribute(types.AttributeKeyRecipient, out.Address),
sdk.NewAttribute(sdk.AttributeKeyAmount, out.Coins.String()),
),
)

// Create account if recipient does not exist.
//
// NOTE: This should ultimately be removed in favor a more flexible approach
// such as delegated fee messages.
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 @@ -200,9 +205,14 @@ func (k BaseSendKeeper) SendCoins(ctx context.Context, fromAddr, toAddr sdk.AccA
return err
}

err = k.addCoins(ctx, toAddr, amt)
if err != nil {
return err
// Create account if recipient does not exist.
//
// NOTE: This should ultimately be removed in favor a more flexible approach
// such as delegated fee messages.
accExists := k.ak.HasAccount(ctx, toAddr)
if !accExists {
defer telemetry.IncrCounter(1, "new", "account")
k.ak.SetAccount(ctx, k.ak.NewAccountWithAddress(ctx, toAddr))
}

fromAddrString, err := k.ak.AddressCodec().BytesToString(fromAddr)
Expand Down
8 changes: 7 additions & 1 deletion x/bank/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,13 @@ import (
// AccountKeeper defines the account contract that must be fulfilled when
// creating a x/bank keeper.
type AccountKeeper interface {
AddressCodec() address.Codec
NewAccount(sdk.Context, types.AccountI) types.AccountI
NewAccountWithAddress(ctx sdk.Context, addr sdk.AccAddress) types.AccountI

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)

NewAccount(context.Context, sdk.AccountI) sdk.AccountI
NewAccountWithAddress(ctx context.Context, addr sdk.AccAddress) sdk.AccountI
Expand Down

0 comments on commit dbef1a9

Please sign in to comment.