Skip to content

Commit

Permalink
Allow balance to go negative in SendCoinsAndWei (#417)
Browse files Browse the repository at this point in the history 
## 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
  • Loading branch information
@codchen @udpatil
codchen authored and udpatil committed Apr 16, 2024
1 parent 60c79a1 commit 5fd8012
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 32 deletions.
8 changes: 8 additions & 0 deletions types/coin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
Expand Down
16 changes: 8 additions & 8 deletions x/bank/keeper/keeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
53 changes: 29 additions & 24 deletions x/bank/keeper/send.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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())
}
Expand All @@ -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
}
Expand All @@ -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())
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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())
}

Expand Down Expand Up @@ -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
}
}
Expand All @@ -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
}
}
Expand Down

0 comments on commit 5fd8012

Please sign in to comment.