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 fb33573 commit 1a20583
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)
}

Check warning on line 136 in x/interchainstaking/keeper/receipt.go

View check run for this annotation

Codecov / codecov/patch

x/interchainstaking/keeper/receipt.go#L134-L136 

Added lines #L134 - L136 were not covered by tests
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)
}

Check warning on line 141 in x/interchainstaking/keeper/receipt.go

View check run for this annotation

Codecov / codecov/patch

x/interchainstaking/keeper/receipt.go#L138-L141 

Added lines #L138 - L141 were not covered by tests
}
}

// 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")

Check warning on line 215 in x/interchainstaking/keeper/receipt.go

View check run for this annotation

Codecov / codecov/patch

x/interchainstaking/keeper/receipt.go#L215 

Added line #L215 was not covered by tests
}

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

Check warning on line 241 in x/interchainstaking/keeper/receipt.go

View check run for this annotation

Codecov / codecov/patch

x/interchainstaking/keeper/receipt.go#L241 

Added line #L241 was not covered by tests
}

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)

Check warning on line 265 in x/interchainstaking/keeper/receipt.go

View check run for this annotation

Codecov / codecov/patch

x/interchainstaking/keeper/receipt.go#L265 

Added line #L265 was not covered by tests
}

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 1a20583

Please sign in to comment.