Skip to content

Commit

Permalink
ensure we dont attempt to send funds from deposit to delegate after w…
Browse files Browse the repository at this point in the history 
…e refund user; fixes #1761
  • Loading branch information
@joe-bowman
joe-bowman committed Dec 2, 2024
1 parent 1bd45ff commit cd581bc
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 21 deletions.
36 changes: 20 additions & 16 deletions x/interchainstaking/keeper/receipt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,21 +122,25 @@ func (k *Keeper) HandleReceiptTransaction(ctx sdk.Context, txn *tx.Tx, hash stri
k.Logger(ctx).Error("unable to update intent. Ignoring.", "senderAddress", senderAddress, "zone", zone.ChainId, "err", err.Error())
return fmt.Errorf("unable to update intent. Ignoring. senderAddress=%q zone=%q err: %w", senderAddress, zone.ChainId, err)
}
if err := k.MintAndSendQAsset(ctx, senderAccAddress, senderAddress, &zone, assets, memoRTS, mappedAddress); err != nil {

success, err := k.MintAndSendQAsset(ctx, senderAccAddress, senderAddress, &zone, assets, memoRTS, mappedAddress)
if err != nil {
k.Logger(ctx).Error("unable to mint QAsset. Ignoring.", "senderAddress", senderAddress, "zone", zone.ChainId, "err", err)
return fmt.Errorf("unable to mint QAsset. Ignoring. senderAddress=%q zone=%q err: %w", senderAddress, zone.ChainId, err)
}
if err := k.TransferToDelegate(ctx, &zone, assets, hash); err != nil {
k.Logger(ctx).Error("unable to transfer to delegate. Ignoring.", "senderAddress", senderAddress, "zone", zone.ChainId, "err", err)
return fmt.Errorf("unable to transfer to delegate. Ignoring. senderAddress=%q zone=%q err: %w", senderAddress, zone.ChainId, err)
}
if memoAutoClaim {
if err := k.HandleAutoClaim(ctx, senderAccAddress); err != nil {
k.Logger(ctx).Error("unable to handle auto claim. Ignoring.", "senderAddress", senderAddress, "zone", zone.ChainId, "err", err)
return fmt.Errorf("unable to handle auto claim. Ignoring. senderAddress=%q zone=%q err: %w", senderAddress, zone.ChainId, err)

if success {
if err := k.TransferToDelegate(ctx, &zone, assets, hash); err != nil {
k.Logger(ctx).Error("unable to transfer to delegate. Ignoring.", "senderAddress", senderAddress, "zone", zone.ChainId, "err", err)
return fmt.Errorf("unable to transfer to delegate. Ignoring. senderAddress=%q zone=%q err: %w", senderAddress, zone.ChainId, err)
}
if memoAutoClaim {
if err := k.HandleAutoClaim(ctx, senderAccAddress); err != nil {
k.Logger(ctx).Error("unable to handle auto claim. Ignoring.", "senderAddress", senderAddress, "zone", zone.ChainId, "err", err)
return fmt.Errorf("unable to handle auto claim. Ignoring. senderAddress=%q zone=%q err: %w", senderAddress, zone.ChainId, err)
}
}
}

// create receipt
receipt := k.NewReceipt(ctx, &zone, senderAddress, hash, assets)
k.SetReceipt(ctx, *receipt)
Expand Down Expand Up @@ -206,9 +210,9 @@ func (k *Keeper) HandleAutoClaim(ctx sdk.Context, senderAddress sdk.AccAddress)
// - Mint QAssets, set new mapping for the mapped account in the keeper, and send to corresponding mapped account.
// 5. If the zone is 118 and no other flags are set:
// - Mint QAssets and transfer to send to msg creator.
func (k *Keeper) MintAndSendQAsset(ctx sdk.Context, sender sdk.AccAddress, senderAddress string, zone *types.Zone, assets sdk.Coins, memoRTS bool, mappedAddress sdk.AccAddress) error {
func (k *Keeper) MintAndSendQAsset(ctx sdk.Context, sender sdk.AccAddress, senderAddress string, zone *types.Zone, assets sdk.Coins, memoRTS bool, mappedAddress sdk.AccAddress) (bool, error) {
if zone.RedemptionRate.IsZero() {
return errors.New("zero redemption rate")
return false, errors.New("zero redemption rate")
}

qAssets := sdk.Coins{}
Expand All @@ -225,7 +229,7 @@ func (k *Keeper) MintAndSendQAsset(ctx sdk.Context, sender sdk.AccAddress, sende
if !found {
// if not found, skip minting and refund assets
msg := &banktypes.MsgSend{FromAddress: zone.DepositAddress.GetAddress(), ToAddress: senderAddress, Amount: assets}
return k.SubmitTx(ctx, []sdk.Msg{msg}, zone.DepositAddress, "refund", zone.MessagesPerTx)
return false, k.SubmitTx(ctx, []sdk.Msg{msg}, zone.DepositAddress, "refund", zone.MessagesPerTx)
}
// do not set, since mapped address already exists
setMappedAddress = false
Expand All @@ -234,7 +238,7 @@ func (k *Keeper) MintAndSendQAsset(ctx sdk.Context, sender sdk.AccAddress, sende
k.Logger(ctx).Info("Minting qAssets for receipt", "assets", qAssets)
err := k.BankKeeper.MintCoins(ctx, types.ModuleName, qAssets)
if err != nil {
return err
return false, err
}

switch {
Expand All @@ -258,7 +262,7 @@ func (k *Keeper) MintAndSendQAsset(ctx sdk.Context, sender sdk.AccAddress, sende
}

if err != nil {
return fmt.Errorf("unable to transfer coins: %w", err)
return false, fmt.Errorf("unable to transfer coins: %w", err)
}

ctx.EventManager().EmitEvent(
Expand All @@ -267,7 +271,7 @@ func (k *Keeper) MintAndSendQAsset(ctx sdk.Context, sender sdk.AccAddress, sende
sdk.NewAttribute(sdk.AttributeKeyAmount, qAssets.String()),
),
)
return nil
return true, nil
}

// TransferToDelegate transfers tokens from the zone deposit account address to the zone delegate account address.
Expand Down
39 changes: 34 additions & 5 deletions x/interchainstaking/keeper/receipt_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,8 +397,9 @@ func (suite *KeeperTestSuite) TestMintAndSendQAsset1RR() {
amount := sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, sdk.NewInt(5000)))

// Test sending QAsset
err := quicksilver.InterchainstakingKeeper.MintAndSendQAsset(ctx, sender, senderAddress, &zone, amount, false, nil)
success, err := quicksilver.InterchainstakingKeeper.MintAndSendQAsset(ctx, sender, senderAddress, &zone, amount, false, nil)
suite.NoError(err)
suite.True(success)

// Verify balance of receiver
receiverBalance := quicksilver.BankKeeper.GetBalance(ctx, sender, zone.LocalDenom)
Expand All @@ -422,8 +423,9 @@ func (suite *KeeperTestSuite) TestMintAndSendQAssetNon1RR() {
amount := sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, sdk.NewInt(5000)))

// Test sending QAsset
err := quicksilver.InterchainstakingKeeper.MintAndSendQAsset(ctx, sender, senderAddress, &zone, amount, false, nil)
success, err := quicksilver.InterchainstakingKeeper.MintAndSendQAsset(ctx, sender, senderAddress, &zone, amount, false, nil)
suite.NoError(err)
suite.True(success)

// Verify balance of receiver
receiverBalance := quicksilver.BankKeeper.GetBalance(ctx, sender, zone.LocalDenom)
Expand All @@ -447,8 +449,9 @@ func (suite *KeeperTestSuite) TestMintAndSendQAssetSub1RR() {
amount := sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, sdk.NewInt(5000)))

// Test sending QAsset
err := quicksilver.InterchainstakingKeeper.MintAndSendQAsset(ctx, sender, senderAddress, &zone, amount, false, nil)
success, err := quicksilver.InterchainstakingKeeper.MintAndSendQAsset(ctx, sender, senderAddress, &zone, amount, false, nil)
suite.NoError(err)
suite.True(success)

// Verify balance of receiver
receiverBalance := quicksilver.BankKeeper.GetBalance(ctx, sender, zone.LocalDenom)
Expand All @@ -464,6 +467,7 @@ func (suite *KeeperTestSuite) TestMintAndSendQAssetNon1RRMappedAccount() {

zone, found := quicksilver.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID)
zone.RedemptionRate = sdk.NewDecWithPrec(110, 2)
zone.Is_118 = false
suite.True(found)

senderAddress := addressutils.GenerateAddressForTestWithPrefix("cosmos")
Expand All @@ -473,8 +477,9 @@ func (suite *KeeperTestSuite) TestMintAndSendQAssetNon1RRMappedAccount() {
amount := sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, sdk.NewInt(5000)))

// Test sending QAsset
err := quicksilver.InterchainstakingKeeper.MintAndSendQAsset(ctx, sender, senderAddress, &zone, amount, false, mappedAccount)
success, err := quicksilver.InterchainstakingKeeper.MintAndSendQAsset(ctx, sender, senderAddress, &zone, amount, false, mappedAccount)
suite.NoError(err)
suite.True(success)

// Verify balance of receiver
receiverBalance := quicksilver.BankKeeper.GetBalance(ctx, sender, zone.LocalDenom)
Expand All @@ -492,6 +497,29 @@ func (suite *KeeperTestSuite) TestMintAndSendQAssetNon1RRMappedAccount() {
suite.Equal(mappedAccount, localAddress)
}

func (suite *KeeperTestSuite) TestMintAndSendQAssetNon1RRNon118NoMappedAccount() {
suite.SetupTest()
suite.setupTestZones()

quicksilver := suite.GetQuicksilverApp(suite.chainA)
ctx := suite.chainA.GetContext()

zone, found := quicksilver.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID)
zone.RedemptionRate = sdk.NewDecWithPrec(110, 2)
zone.Is_118 = false
suite.True(found)

senderAddress := addressutils.GenerateAddressForTestWithPrefix("cosmos")
sender := addressutils.MustAccAddressFromBech32(senderAddress, "")

amount := sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, sdk.NewInt(5000)))

// Test sending QAsset
success, err := quicksilver.InterchainstakingKeeper.MintAndSendQAsset(ctx, sender, senderAddress, &zone, amount, false, nil)
suite.NoError(err)
suite.False(success)
}

func (suite *KeeperTestSuite) TestMintAndSendQAssetNon1RTS() {
suite.SetupTest()
// this is required because the ibc-go test suite CreateTransferChannels defaults to a value that causes executing a message to error.
Expand All @@ -514,8 +542,9 @@ func (suite *KeeperTestSuite) TestMintAndSendQAssetNon1RTS() {
amount := sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, sdk.NewInt(5000)))

// Test sending QAsset
err := quicksilver.InterchainstakingKeeper.MintAndSendQAsset(ctx, sender, senderAddress, &zone, amount, true, nil)
success, err := quicksilver.InterchainstakingKeeper.MintAndSendQAsset(ctx, sender, senderAddress, &zone, amount, true, nil)
suite.NoError(err)
suite.True(success)

// Verify balance of receiver
receiverBalance := quicksilver.BankKeeper.GetBalance(ctx, sender, zone.LocalDenom)
Expand Down

0 comments on commit cd581bc

Please sign in to comment.