From 6a07568e30583e9c250c8f2f2d93d6927a338027 Mon Sep 17 00:00:00 2001 From: Tuan Tran Date: Wed, 29 May 2024 03:20:54 +0700 Subject: [PATCH] perf: reduce the number of ValidateDenom calls in bank.SendCoins (#20354) --- CHANGELOG.md | 2 +- types/coin.go | 2 -- x/bank/keeper/keeper.go | 7 +++++++ x/bank/keeper/send.go | 35 ++++++++++++++++++++--------------- 4 files changed, 28 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dcbae6d38f..3773d6e78a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,7 +64,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i * (debug) [#20328](https://github.com/cosmos/cosmos-sdk/pull/20328) Add consensus address for debug cmd. ### Improvements - +* (bank) [#20354](https://github.com/cosmos/cosmos-sdk/pull/20354) Reduce the number of `ValidateDenom` calls in `bank.SendCoins`. * (types) [#19869](https://github.com/cosmos/cosmos-sdk/pull/19869) Removed `Any` type from `codec/types` and replaced it with an alias for `cosmos/gogoproto/types/any`. * (server) [#19854](https://github.com/cosmos/cosmos-sdk/pull/19854) Add customizability to start command. * Add `StartCmdOptions` in `server.AddCommands` instead of `servertypes.ModuleInitFlags`. To set custom flags set them in the `StartCmdOptions` struct on the `AddFlags` field. diff --git a/types/coin.go b/types/coin.go index 12a05d006c..99e74ee65d 100644 --- a/types/coin.go +++ b/types/coin.go @@ -865,8 +865,6 @@ func SetCoinDenomRegex(reFn func() string) { // ValidateDenom is the default validation function for Coin.Denom. func ValidateDenom(denom string) error { if reDnm == nil || reDecCoin == nil { - // Convert the string to a byte slice as required by the Ragel-generated function. - // Call the Ragel-generated function. if !MatchDenom(denom) { return fmt.Errorf("invalid denom: %s", denom) diff --git a/x/bank/keeper/keeper.go b/x/bank/keeper/keeper.go index 7b8255194b..31e4720bbd 100644 --- a/x/bank/keeper/keeper.go +++ b/x/bank/keeper/keeper.go @@ -360,6 +360,10 @@ func (k BaseKeeper) MintCoins(ctx context.Context, moduleName string, amounts sd return errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "module account %s does not have permissions to mint tokens", moduleName) } + if !amounts.IsValid() { + return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, amounts.String()) + } + err = k.addCoins(ctx, acc.GetAddress(), amounts) if err != nil { return err @@ -399,6 +403,9 @@ func (k BaseKeeper) BurnCoins(ctx context.Context, address []byte, amounts sdk.C return errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "account %x does not have permissions to burn tokens", address) } } + if !amounts.IsValid() { + return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, amounts.String()) + } err := k.subUnlockedCoins(ctx, acc.GetAddress(), amounts) if err != nil { diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index 8055542754..47321da51b 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -184,6 +184,10 @@ func (k BaseSendKeeper) InputOutputCoins(ctx context.Context, input types.Input, // SendCoins transfers amt coins from a sending account to a receiving account. // An error is returned upon failure. func (k BaseSendKeeper) SendCoins(ctx context.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) error { + if !amt.IsValid() { + return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, amt.String()) + } + var err error err = k.subUnlockedCoins(ctx, fromAddr, amt) if err != nil { @@ -217,19 +221,21 @@ func (k BaseSendKeeper) SendCoins(ctx context.Context, fromAddr, toAddr sdk.AccA ) } -// subUnlockedCoins removes the unlocked amt coins of the given account. An error is -// returned if the resulting balance is negative or the initial amount is invalid. -// A coin_spent event is emitted after. +// subUnlockedCoins removes the unlocked amt coins of the given account. +// An error is returned if the resulting balance is negative. +// +// CONTRACT: The provided amount (amt) must be valid, non-negative coins. +// +// A coin_spent event is emitted after the operation. func (k BaseSendKeeper) subUnlockedCoins(ctx context.Context, addr sdk.AccAddress, amt sdk.Coins) error { - if !amt.IsValid() { - return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, amt.String()) - } - lockedCoins := k.LockedCoins(ctx, addr) for _, coin := range amt { balance := k.GetBalance(ctx, addr, coin.Denom) - locked := sdk.NewCoin(coin.Denom, lockedCoins.AmountOf(coin.Denom)) + ok, locked := lockedCoins.Find(coin.Denom) + if !ok { + locked = sdk.Coin{Denom: coin.Denom, Amount: math.ZeroInt()} + } spendable, hasNeg := sdk.Coins{balance}.SafeSub(locked) if hasNeg { @@ -239,7 +245,7 @@ func (k BaseSendKeeper) subUnlockedCoins(ctx context.Context, addr sdk.AccAddres if _, hasNeg := spendable.SafeSub(coin); hasNeg { if len(spendable) == 0 { - spendable = sdk.Coins{sdk.NewCoin(coin.Denom, math.ZeroInt())} + spendable = sdk.Coins{sdk.Coin{Denom: coin.Denom, Amount: math.ZeroInt()}} } return errorsmod.Wrapf( sdkerrors.ErrInsufficientFunds, @@ -267,13 +273,12 @@ func (k BaseSendKeeper) subUnlockedCoins(ctx context.Context, addr sdk.AccAddres ) } -// addCoins increase the addr balance by the given amt. Fails if the provided -// amt is invalid. It emits a coin received event. +// addCoins increases the balance of the given address by the specified amount. +// +// CONTRACT: The provided amount (amt) must be valid, non-negative coins. +// +// It emits a coin_received event after the operation. func (k BaseSendKeeper) addCoins(ctx context.Context, addr sdk.AccAddress, amt sdk.Coins) error { - if !amt.IsValid() { - return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, amt.String()) - } - for _, coin := range amt { balance := k.GetBalance(ctx, addr, coin.Denom) newBalance := balance.Add(coin)