From 5fd8012e4564c9185b273f7da8a1f3966a771bb4 Mon Sep 17 00:00:00 2001 From: codchen Date: Thu, 1 Feb 2024 15:57:22 +0800 Subject: [PATCH] Allow balance to go negative in SendCoinsAndWei (#417) ## Describe your changes and provide context Bypass negative check for sends involving Wei escrow accounts, since they may temporarily go negative during tx processing (but will be settled back to 0 in EndBlock) ## Testing performed to validate your change tested with the corresponding sei change --- types/coin.go | 8 +++++++ x/bank/keeper/keeper.go | 16 ++++++------- x/bank/keeper/send.go | 53 ++++++++++++++++++++++------------------- 3 files changed, 45 insertions(+), 32 deletions(-) diff --git a/types/coin.go b/types/coin.go index 0c33ed757..f96996c13 100644 --- a/types/coin.go +++ b/types/coin.go @@ -120,6 +120,14 @@ func (coin Coin) Sub(coinB Coin) Coin { return res } +func (coin Coin) SubUnsafe(coinB Coin) Coin { + if coin.Denom != coinB.Denom { + panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, coinB.Denom)) + } + + return Coin{coin.Denom, coin.Amount.Sub(coinB.Amount)} +} + // SubAmount subtracts an amount from the Coin. func (coin Coin) SubAmount(amount Int) Coin { res := Coin{coin.Denom, coin.Amount.Sub(amount)} diff --git a/x/bank/keeper/keeper.go b/x/bank/keeper/keeper.go index 8b0bd2f03..e74d868a1 100644 --- a/x/bank/keeper/keeper.go +++ b/x/bank/keeper/keeper.go @@ -200,7 +200,7 @@ func (k BaseKeeper) DelegateCoins(ctx sdk.Context, delegatorAddr, moduleAccAddr } balances = balances.Add(balance) - err := k.setBalance(ctx, delegatorAddr, balance.Sub(coin)) + err := k.setBalance(ctx, delegatorAddr, balance.Sub(coin), true) if err != nil { return err } @@ -214,7 +214,7 @@ func (k BaseKeeper) DelegateCoins(ctx sdk.Context, delegatorAddr, moduleAccAddr types.NewCoinSpentEvent(delegatorAddr, amt), ) - err := k.addCoins(ctx, moduleAccAddr, amt) + err := k.addCoins(ctx, moduleAccAddr, amt, true) if err != nil { return err } @@ -237,7 +237,7 @@ func (k BaseKeeper) UndelegateCoins(ctx sdk.Context, moduleAccAddr, delegatorAdd return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, amt.String()) } - err := k.subUnlockedCoins(ctx, moduleAccAddr, amt) + err := k.subUnlockedCoins(ctx, moduleAccAddr, amt, true) if err != nil { return err } @@ -246,7 +246,7 @@ func (k BaseKeeper) UndelegateCoins(ctx sdk.Context, moduleAccAddr, delegatorAdd return sdkerrors.Wrap(err, "failed to track undelegation") } - err = k.addCoins(ctx, delegatorAddr, amt) + err = k.addCoins(ctx, delegatorAddr, amt, true) if err != nil { return err } @@ -410,7 +410,7 @@ func (k BaseKeeper) DeferredSendCoinsFromAccountToModule( panic("bank keeper created without deferred cache") } // Deducts Fees from the Sender Account - err := k.subUnlockedCoins(ctx, senderAddr, amount) + err := k.subUnlockedCoins(ctx, senderAddr, amount, true) if err != nil { return err } @@ -468,7 +468,7 @@ func (k BaseKeeper) WriteDeferredBalances(ctx sdk.Context) []abci.Event { ctx.Logger().Error(err.Error()) panic(err) } - err := k.addCoins(ctx, sdk.MustAccAddressFromBech32(moduleBech32Addr), amount) + err := k.addCoins(ctx, sdk.MustAccAddressFromBech32(moduleBech32Addr), amount, true) if err != nil { ctx.Logger().Error(fmt.Sprintf("Failed to add coin=%s to module address=%s, error is: %s", amount, moduleBech32Addr, err)) panic(err) @@ -570,7 +570,7 @@ func (k BaseKeeper) MintCoins(ctx sdk.Context, moduleName string, amounts sdk.Co if acc == nil { return errors.New(fmt.Sprintf("module account for %s not found", moduleName)) } - return k.addCoins(ctx, acc.GetAddress(), amounts) + return k.addCoins(ctx, acc.GetAddress(), amounts, true) } err := k.createCoins(ctx, moduleName, amounts, addFn) @@ -616,7 +616,7 @@ func (k BaseKeeper) destroyCoins(ctx sdk.Context, moduleName string, amounts sdk func (k BaseKeeper) BurnCoins(ctx sdk.Context, moduleName string, amounts sdk.Coins) error { subFn := func(ctx sdk.Context, moduleName string, amounts sdk.Coins) error { acc := k.ak.GetModuleAccount(ctx, moduleName) - return k.subUnlockedCoins(ctx, acc.GetAddress(), amounts) + return k.subUnlockedCoins(ctx, acc.GetAddress(), amounts, true) } err := k.destroyCoins(ctx, moduleName, amounts, subFn) diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index 93ac398dd..401a815ba 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -89,7 +89,7 @@ func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, inputs []types.Input, return err } - err = k.subUnlockedCoins(ctx, inAddress, in.Coins) + err = k.subUnlockedCoins(ctx, inAddress, in.Coins, true) if err != nil { return err } @@ -107,7 +107,7 @@ func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, inputs []types.Input, if err != nil { return err } - err = k.addCoins(ctx, outAddress, out.Coins) + err = k.addCoins(ctx, outAddress, out.Coins, true) if err != nil { return err } @@ -155,12 +155,16 @@ func (k BaseSendKeeper) SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAd } func (k BaseSendKeeper) SendCoinsWithoutAccCreation(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error { - err := k.subUnlockedCoins(ctx, fromAddr, amt) + return k.sendCoinsWithoutAccCreation(ctx, fromAddr, toAddr, amt, true) +} + +func (k BaseSendKeeper) sendCoinsWithoutAccCreation(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins, checkNeg bool) error { + err := k.subUnlockedCoins(ctx, fromAddr, amt, checkNeg) if err != nil { return err } - err = k.addCoins(ctx, toAddr, amt) + err = k.addCoins(ctx, toAddr, amt, checkNeg) if err != nil { return err } @@ -184,7 +188,7 @@ func (k BaseSendKeeper) SendCoinsWithoutAccCreation(ctx sdk.Context, fromAddr sd // 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. -func (k BaseSendKeeper) subUnlockedCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) error { +func (k BaseSendKeeper) subUnlockedCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins, checkNeg bool) error { if !amt.IsValid() { return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, amt.String()) } @@ -193,17 +197,24 @@ func (k BaseSendKeeper) subUnlockedCoins(ctx sdk.Context, addr sdk.AccAddress, a for _, coin := range amt { balance := k.GetBalance(ctx, addr, coin.Denom) - locked := sdk.NewCoin(coin.Denom, lockedCoins.AmountOf(coin.Denom)) - spendable := balance.Sub(locked) + if checkNeg { + locked := sdk.NewCoin(coin.Denom, lockedCoins.AmountOf(coin.Denom)) + spendable := balance.Sub(locked) - _, hasNeg := sdk.Coins{spendable}.SafeSub(sdk.Coins{coin}) - if hasNeg { - return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "%s is smaller than %s", spendable, coin) + _, hasNeg := sdk.Coins{spendable}.SafeSub(sdk.Coins{coin}) + if hasNeg { + return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "%s is smaller than %s", spendable, coin) + } } - newBalance := balance.Sub(coin) + var newBalance sdk.Coin + if checkNeg { + newBalance = balance.Sub(coin) + } else { + newBalance = balance.SubUnsafe(coin) + } - err := k.setBalance(ctx, addr, newBalance) + err := k.setBalance(ctx, addr, newBalance, checkNeg) if err != nil { return err } @@ -218,7 +229,7 @@ func (k BaseSendKeeper) subUnlockedCoins(ctx sdk.Context, addr sdk.AccAddress, a // addCoins increase the addr balance by the given amt. Fails if the provided amt is invalid. // It emits a coin received event. -func (k BaseSendKeeper) addCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) error { +func (k BaseSendKeeper) addCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins, checkNeg bool) error { if !amt.IsValid() { return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, amt.String()) } @@ -227,7 +238,7 @@ func (k BaseSendKeeper) addCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.C balance := k.GetBalance(ctx, addr, coin.Denom) newBalance := balance.Add(coin) - err := k.setBalance(ctx, addr, newBalance) + err := k.setBalance(ctx, addr, newBalance, checkNeg) if err != nil { return err } @@ -262,8 +273,8 @@ func (k BaseSendKeeper) initBalances(ctx sdk.Context, addr sdk.AccAddress, balan } // setBalance sets the coin balance for an account by address. -func (k BaseSendKeeper) setBalance(ctx sdk.Context, addr sdk.AccAddress, balance sdk.Coin) error { - if !balance.IsValid() { +func (k BaseSendKeeper) setBalance(ctx sdk.Context, addr sdk.AccAddress, balance sdk.Coin, checkNeg bool) error { + if checkNeg && !balance.IsValid() { return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, balance.String()) } @@ -343,7 +354,7 @@ func (k BaseSendKeeper) SendCoinsAndWei(ctx sdk.Context, from sdk.AccAddress, to return err } // need to send one sei to escrow because wei balance is insufficient - if err := k.SendCoinsWithoutAccCreation(ctx, from, escrow, sdk.NewCoins(sdk.NewCoin(denom, sdk.OneInt()))); err != nil { + if err := k.sendCoinsWithoutAccCreation(ctx, from, escrow, sdk.NewCoins(sdk.NewCoin(denom, sdk.OneInt())), false); err != nil { return err } } @@ -359,13 +370,7 @@ func (k BaseSendKeeper) SendCoinsAndWei(ctx sdk.Context, from sdk.AccAddress, to } // need to redeem one sei from escrow because wei balance overflowed one := sdk.NewCoins(sdk.NewCoin(denom, sdk.OneInt())) - if err := k.SendCoinsWithoutAccCreation(ctx, escrow, to, one); err != nil { - if sdkerrors.ErrInsufficientFunds.Is(err) && customEscrow != nil { - // custom escrow does not have enough balance to redeem. Try the global escrow instead - if err := k.SendCoinsWithoutAccCreation(ctx, k.ak.GetModuleAddress(types.WeiEscrowName), to, one); err != nil { - return err - } - } + if err := k.sendCoinsWithoutAccCreation(ctx, escrow, to, one, false); err != nil { return err } }