From 41e44fe866d62079041eac3f218087db23ea4805 Mon Sep 17 00:00:00 2001
From: HuangYi <huang@crypto.com>
Date: Wed, 23 Oct 2024 12:13:13 +0800
Subject: [PATCH] Problem: subUnlockedCoins is not optimal

---
 CHANGELOG.md          |  1 +
 types/coin.go         |  8 ++++++++
 x/bank/keeper/send.go | 30 +++++++++++++++---------------
 3 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2ba20e167f9e..2c94d226505d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -54,6 +54,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * [#261](https://github.com/crypto-org-chain/cosmos-sdk/pull/261) `ctx.BlockHeader` don't do protobuf deep copy, shallow copy seems enough, reduce scope of mutex in `PriorityNonceMempool.Remove`.
 * [#507](https://github.com/crypto-org-chain/cosmos-sdk/pull/507) mempool respect gas wanted returned by ante handler
 * [#744](https://github.com/crypto-org-chain/cosmos-sdk/pull/744) Pass raw transactions to tx executor so it can do pre-estimations.
+* [#]() Optimize subUnlockedCoins implementation.
 
 ## [Unreleased]
 
diff --git a/types/coin.go b/types/coin.go
index 136295eed295..6103b761d032 100644
--- a/types/coin.go
+++ b/types/coin.go
@@ -29,6 +29,14 @@ func NewCoin(denom string, amount math.Int) Coin {
 	return coin
 }
 
+// UnsafeNewCoin returns a new coin without validate inputs.
+func UnsafeNewCoin(denom string, amount math.Int) Coin {
+	return Coin{
+		Denom:  denom,
+		Amount: amount,
+	}
+}
+
 // NewInt64Coin returns a new coin with a denomination and amount. It will panic
 // if the amount is negative.
 func NewInt64Coin(denom string, amount int64) Coin {
diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go
index 9ab40e8a6bbc..200eb11d5cb8 100644
--- a/x/bank/keeper/send.go
+++ b/x/bank/keeper/send.go
@@ -8,7 +8,7 @@ import (
 	"cosmossdk.io/core/store"
 	errorsmod "cosmossdk.io/errors"
 	"cosmossdk.io/log"
-	"cosmossdk.io/math"
+	sdkmath "cosmossdk.io/math"
 	storetypes "cosmossdk.io/store/types"
 
 	"github.com/cosmos/cosmos-sdk/codec"
@@ -274,28 +274,28 @@ func (k BaseSendKeeper) subUnlockedCoins(ctx context.Context, addr sdk.AccAddres
 	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))
+		balance := k.GetBalance(ctx, addr, coin.Denom).Amount.BigIntMut()
+		locked := lockedCoins.AmountOf(coin.Denom).BigIntMut()
+		amount := coin.Amount.BigIntMut()
 
-		spendable, hasNeg := sdk.Coins{balance}.SafeSub(locked)
-		if hasNeg {
-			return errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds,
-				"locked amount exceeds account balance funds: %s > %s", locked, balance)
+		if locked.Sign() == -1 {
+			return errorsmod.Wrapf(
+				sdkerrors.ErrInsufficientFunds,
+				"locked balance %s%s is negative",
+				locked, coin.Denom,
+			)
 		}
 
-		if _, hasNeg := spendable.SafeSub(coin); hasNeg {
-			if len(spendable) == 0 {
-				spendable = sdk.Coins{sdk.NewCoin(coin.Denom, math.ZeroInt())}
-			}
+		if balance.Cmp(locked.Add(locked, amount)) == -1 {
+			spendable := balance.Sub(balance, locked)
 			return errorsmod.Wrapf(
 				sdkerrors.ErrInsufficientFunds,
-				"spendable balance %s is smaller than %s",
-				spendable, coin,
+				"spendable balance %s%s is smaller than %s%s",
+				spendable, coin.Denom, amount, coin.Denom,
 			)
 		}
 
-		newBalance := balance.Sub(coin)
-
+		newBalance := sdk.UnsafeNewCoin(coin.Denom, sdkmath.NewIntFromBigIntMut(balance.Sub(balance, amount)))
 		if err := k.setBalance(ctx, addr, newBalance); err != nil {
 			return err
 		}