Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: send entire balance not working as expected #7871

Open
wants to merge 1 commit into
base: feat/ibc-eureka
Choose a base branch
from
Open

fix: send entire balance not working as expected #7871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions e2e/tests/transfer/base_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,6 @@ func (s *TransferTestSuite) TestMsgTransfer_EntireBalance() {
})

chainBIBCToken := testsuite.GetIBCToken(chainADenom, channelA.Counterparty.PortID, channelA.Counterparty.ChannelID)

t.Run("packets relayed", func(t *testing.T) {
s.AssertPacketRelayed(ctx, chainA, channelA.PortID, channelA.ChannelID, 1)
actualBalance, err := query.Balance(ctx, chainB, chainBAddress, chainBIBCToken.IBCDenom())
Expand All @@ -619,7 +618,7 @@ func (s *TransferTestSuite) TestMsgTransfer_EntireBalance() {
chainAIBCToken := testsuite.GetIBCToken(chainB.Config().Denom, channelA.PortID, channelA.ChannelID)
t.Run("packets relayed", func(t *testing.T) {
// test that chainA has the entire balance back of its native token.
s.AssertPacketRelayed(ctx, chainA, channelA.PortID, channelA.ChannelID, 1)
s.AssertPacketRelayed(ctx, chainB, channelA.Counterparty.PortID, channelA.Counterparty.ChannelID, 1)
actualBalance, err := query.Balance(ctx, chainA, chainAAddress, chainADenom)

s.Require().NoError(err)
Expand Down
8 changes: 8 additions & 0 deletions modules/apps/transfer/keeper/msg_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,14 @@ func (k Keeper) Transfer(ctx context.Context, msg *types.MsgTransfer) (*types.Ms
tokens := make([]types.Token, len(coins))

for i, coin := range coins {
// Using types.UnboundedSpendLimit allows us to send the entire balance of a given denom.
if coin.Amount.Equal(types.UnboundedSpendLimit()) {
coin.Amount = k.BankKeeper.SpendableCoin(ctx, sender, coin.Denom).Amount
if coin.Amount.IsZero() {
return nil, errorsmod.Wrapf(types.ErrInvalidAmount, "empty spendable balance for %s", coin.Denom)
}
}

tokens[i], err = k.tokenFromCoin(ctx, coin)
if err != nil {
return nil, err
Expand Down
8 changes: 0 additions & 8 deletions modules/apps/transfer/keeper/relay.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,6 @@ func (k Keeper) SendTransfer(
return errorsmod.Wrap(types.ErrSendDisabled, err.Error())
}

// Using types.UnboundedSpendLimit allows us to send the entire balance of a given denom.
if coin.Amount.Equal(types.UnboundedSpendLimit()) {
coin.Amount = k.BankKeeper.SpendableCoin(ctx, sender, coin.Denom).Amount
if coin.Amount.IsZero() {
return errorsmod.Wrapf(types.ErrInvalidAmount, "empty spendable balance for %s", coin.Denom)
}
}

// NOTE: SendTransfer simply sends the denomination as it exists on its own
// chain inside the packet data. The receiving chain will perform denom
// prefixing as necessary.
Expand Down
60 changes: 45 additions & 15 deletions modules/apps/transfer/transfer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,38 @@ func (suite *TransferTestSuite) SetupTest() {
// 2 - from chainB to chainC
// 3 - from chainC to chainB
func (suite *TransferTestSuite) TestHandleMsgTransfer() {
testCases := []struct {
name string
var (
sourceDenomsToTransfer []string
msgAmount sdkmath.Int
)

testCases := []struct {
name string
malleate func()
}{
{
"transfer single denom",
[]string{sdk.DefaultBondDenom},
func() {},
},
{
"transfer amount larger than int64",
func() {
var ok bool
msgAmount, ok = sdkmath.NewIntFromString("9223372036854775808") // 2^63 (one above int64)
suite.Require().True(ok)
},
},
{
"transfer multiple denoms",
[]string{sdk.DefaultBondDenom, ibctesting.SecondaryDenom},
func() {
sourceDenomsToTransfer = []string{sdk.DefaultBondDenom, ibctesting.SecondaryDenom}
},
},
{
"transfer entire balance",
func() {
msgAmount = types.UnboundedSpendLimit()
},
},
}

Expand All @@ -63,19 +84,22 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() {
pathAToB.Setup()
traceAToB := types.NewHop(pathAToB.EndpointB.ChannelConfig.PortID, pathAToB.EndpointB.ChannelID)

sourceDenomsToTransfer = []string{sdk.DefaultBondDenom}
msgAmount = ibctesting.DefaultCoinAmount

tc.malleate()

originalBalances := sdk.NewCoins()
for _, denom := range tc.sourceDenomsToTransfer {
for _, denom := range sourceDenomsToTransfer {
originalBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), denom)
originalBalances = originalBalances.Add(originalBalance)
}

timeoutHeight := clienttypes.NewHeight(1, 110)

amount, ok := sdkmath.NewIntFromString("9223372036854775808") // 2^63 (one above int64)
suite.Require().True(ok)
originalCoins := sdk.NewCoins()
for _, denom := range tc.sourceDenomsToTransfer {
coinToSendToB := sdk.NewCoin(denom, amount)
for _, denom := range sourceDenomsToTransfer {
coinToSendToB := sdk.NewCoin(denom, msgAmount)
originalCoins = originalCoins.Add(coinToSendToB)
}

Expand All @@ -87,6 +111,12 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() {
packet, err := ibctesting.ParsePacketFromEvents(res.Events)
suite.Require().NoError(err)

// Get the packet data to determine the amount of tokens being transferred (needed for sending entire balance)
packetData, err := types.UnmarshalPacketData(packet.GetData(), pathAToB.EndpointA.GetChannel().Version, "")
suite.Require().NoError(err)
transferAmount, ok := sdkmath.NewIntFromString(packetData.Tokens[0].Amount)
suite.Require().True(ok)

// relay send
err = pathAToB.RelayPacket(packet)
suite.Require().NoError(err) // relay committed
Expand All @@ -96,16 +126,16 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() {
for _, coin := range originalCoins {
// check that the balance for chainA is updated
chainABalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), coin.Denom)
suite.Require().Equal(originalBalances.AmountOf(coin.Denom).Sub(amount).Int64(), chainABalance.Amount.Int64())
suite.Require().True(originalBalances.AmountOf(coin.Denom).Sub(transferAmount).Equal(chainABalance.Amount))

// check that module account escrow address has locked the tokens
chainAEscrowBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), escrowAddress, coin.Denom)
suite.Require().Equal(coin, chainAEscrowBalance)
suite.Require().True(transferAmount.Equal(chainAEscrowBalance.Amount))

// check that voucher exists on chain B
chainBDenom := types.NewDenom(coin.Denom, traceAToB)
chainBBalance := suite.chainB.GetSimApp().BankKeeper.GetBalance(suite.chainB.GetContext(), suite.chainB.SenderAccount.GetAddress(), chainBDenom.IBCDenom())
coinSentFromAToB := sdk.NewCoin(chainBDenom.IBCDenom(), amount)
coinSentFromAToB := sdk.NewCoin(chainBDenom.IBCDenom(), transferAmount)
suite.Require().Equal(coinSentFromAToB, chainBBalance)

coinsSentFromAToB = coinsSentFromAToB.Add(coinSentFromAToB)
Expand Down Expand Up @@ -137,7 +167,7 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() {
chainCDenom := types.NewDenom(coin.Denom, traceBToC, traceAToB)

// check that the balance is updated on chainC
coinSentFromBToC := sdk.NewCoin(chainCDenom.IBCDenom(), amount)
coinSentFromBToC := sdk.NewCoin(chainCDenom.IBCDenom(), transferAmount)
chainCBalance := suite.chainC.GetSimApp().BankKeeper.GetBalance(suite.chainC.GetContext(), suite.chainC.SenderAccount.GetAddress(), coinSentFromBToC.Denom)
suite.Require().Equal(coinSentFromBToC, chainCBalance)

Expand Down Expand Up @@ -182,12 +212,12 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() {
for _, coin := range originalCoins {
// check that the balance is unchanged
chainABalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), coin.Denom)
suite.Require().Equal(originalBalances.AmountOf(coin.Denom).Sub(amount).Int64(), chainABalance.Amount.Int64())
suite.Require().True(originalBalances.AmountOf(coin.Denom).Sub(transferAmount).Equal(chainABalance.Amount))

// check that module account escrow address is unchanged
escrowAddress = types.GetEscrowAddress(pathAToB.EndpointA.ChannelConfig.PortID, pathAToB.EndpointA.ChannelID)
chainAEscrowBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), escrowAddress, coin.Denom)
suite.Require().Equal(coin, chainAEscrowBalance)
suite.Require().True(transferAmount.Equal(chainAEscrowBalance.Amount))
}
})
}
Expand Down
Loading