fix: compute ibc/xxx denom instead of base denom for comparison

utils/coins.go

@@ -5,6 +5,7 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+	ibctransfertypes "github.com/cosmos/ibc-go/v5/modules/apps/transfer/types"
 )
 
 func DenomFromRequestKey(query []byte, accAddr sdk.AccAddress) (string, error) {
@@ -24,3 +25,19 @@ func DenomFromRequestKey(query []byte, accAddr sdk.AccAddress) (string, error) {
 
 	return denom, nil
 }
+
+func DeriveIbcDenom(port, channel, denom string) string {
+	return DeriveIbcDenomTrace(port, channel, denom).IBCDenom()
+}
+
+func DeriveIbcDenomTrace(port, channel, denom string) ibctransfertypes.DenomTrace {
+	// generate denomination prefix
+	sourcePrefix := ibctransfertypes.GetDenomPrefix(port, channel)
+	// NOTE: sourcePrefix contains the trailing "/"
+	prefixedDenom := sourcePrefix + denom
+
+	// construct the denomination trace from the full raw denomination
+	denomTrace := ibctransfertypes.ParseDenomTrace(prefixedDenom)
+
+	return denomTrace
+}

x/interchainstaking/keeper/ibc_packet_handlers.go

@@ -315,7 +315,11 @@ func (k *Keeper) HandleMsgTransfer(ctx sdk.Context, msg sdk.Msg) error {
 
 	zone, found := k.GetZoneForWithdrawalAccount(ctx, sMsg.Sender)
 
-	if found && receivedCoin.Denom != zone.BaseDenom {
+	denomTrace := utils.DeriveIbcDenomTrace(sMsg.SourcePort, sMsg.SourceChannel, receivedCoin.Denom)
+	receivedCoin.Denom = denomTrace.IBCDenom()
+
+	if found && denomTrace.BaseDenom != zone.BaseDenom {
+		// k.Logger(ctx).Error("got withdrawal account and NOT staking denom", "rx", receivedCoin.Denom, "trace_base_denom", denomTrace.BaseDenom, "zone_base_denom", zone.BaseDenom)
 		feeAmount := sdk.NewDecFromInt(receivedCoin.Amount).Mul(k.GetCommissionRate(ctx)).TruncateInt()
 		rewardCoin := receivedCoin.SubAmount(feeAmount)
 		zoneAddress, err := addressutils.AccAddressFromBech32(zone.WithdrawalAddress.Address, "")
@@ -355,7 +359,7 @@ func (k *Keeper) HandleCompleteSend(ctx sdk.Context, msg sdk.Msg, memo string) e
 
 	// checks here are specific to ensure future extensibility;
 	switch {
-	case sMsg.FromAddress == zone.WithdrawalAddress.GetAddress():
+	case zone.IsWithdrawalAddress(sMsg.FromAddress):
 		// WithdrawalAddress (for rewards) only send to DelegationAddresses.
 		// Target here is the DelegationAddresses.
 		return k.handleRewardsDelegation(ctx, *zone, sMsg)
@@ -723,15 +727,43 @@ func (k *Keeper) HandleUndelegate(ctx sdk.Context, msg sdk.Msg, completion time.
 }
 
 func (k *Keeper) HandleFailedBankSend(ctx sdk.Context, msg sdk.Msg, memo string) error {
-	txHash, err := types.ParseTxMsgMemo(memo, types.MsgTypeUnbondSend)
+	sMsg, ok := msg.(*banktypes.MsgSend)
+	if !ok {
+		err := errors.New("unable to cast source message to MsgSend")
+		k.Logger(ctx).Error(err.Error())
+		return err
+	}
+
+	// get zone
+	zone, err := k.GetZoneFromContext(ctx)
 	if err != nil {
+		k.Logger(ctx).Error(err.Error())
 		return err
 	}
 
-	// valid msg type bank send
-	sendMsg, ok := msg.(*banktypes.MsgSend)
-	if !ok {
-		return errors.New("unable to unmarshal MsgSend")
+	// checks here are specific to ensure future extensibility;
+	switch {
+	case zone.IsWithdrawalAddress(sMsg.FromAddress):
+		// MsgSend from Withdrawal account to delegate account was not completed. We can ignore this.
+		k.Logger(ctx).Error("MsgSend from withdrawal account to delegate account failed")
+	case zone.IsDelegateAddress(sMsg.FromAddress):
+		return k.HandleFailedUnbondSend(ctx, sMsg, memo)
+	case zone.IsDelegateAddress(sMsg.ToAddress) && zone.DepositAddress.Address == sMsg.FromAddress:
+		// MsgSend from deposit account to delegate account for deposit.
+		k.Logger(ctx).Error("MsgSend from deposit account to delegate account failed")
+	default:
+		err = errors.New("unexpected completed send")
+		k.Logger(ctx).Error(err.Error())
+		return err
+	}
+
+	return nil
+}
+
+func (k *Keeper) HandleFailedUnbondSend(ctx sdk.Context, sendMsg *banktypes.MsgSend, memo string) error {
+	txHash, err := types.ParseTxMsgMemo(memo, types.MsgTypeUnbondSend)
+	if err != nil {
+		return err
 	}
 
 	// get chainID for the remote zone using msg addresses (ICA acc)
@@ -1174,6 +1206,10 @@ func DistributeRewardsFromWithdrawAccount(k *Keeper, ctx sdk.Context, args []byt
 }
 
 func (k *Keeper) prepareRewardsDistributionMsgs(zone types.Zone, rewards sdkmath.Int) sdk.Msg {
+	if !rewards.IsPositive() {
+		return &banktypes.MsgSend{}
+	}
+
 	return &banktypes.MsgSend{
 		FromAddress: zone.WithdrawalAddress.GetAddress(),
 		ToAddress:   zone.DelegationAddress.GetAddress(),

x/interchainstaking/keeper/ibc_packet_handlers_test.go

@@ -18,6 +18,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/ingenuity-build/quicksilver/app"
+	"github.com/ingenuity-build/quicksilver/utils"
 	"github.com/ingenuity-build/quicksilver/utils/addressutils"
 	"github.com/ingenuity-build/quicksilver/utils/randomutils"
 	icstypes "github.com/ingenuity-build/quicksilver/x/interchainstaking/types"
@@ -29,27 +30,27 @@ func (suite *KeeperTestSuite) TestHandleMsgTransferGood() {
 	tcs := []struct {
 		name             string
 		amount           sdk.Coin
-		fcAmount         sdk.Coin
-		withdrawalAmount sdk.Coin
+		fcAmount         math.Int
+		withdrawalAmount math.Int
 		feeAmount        *sdk.Dec
 	}{
 		{
 			name:             "staking denom - all goes to fc",
 			amount:           sdk.NewCoin("uatom", math.NewInt(100)),
-			fcAmount:         sdk.NewCoin("uatom", math.NewInt(100)),
-			withdrawalAmount: sdk.Coin{},
+			fcAmount:         math.NewInt(100),
+			withdrawalAmount: math.ZeroInt(),
 		},
 		{
 			name:             "non staking denom - default (2.5%) to fc, remainder to withdrawal",
 			amount:           sdk.NewCoin("ujuno", math.NewInt(100)),
-			fcAmount:         sdk.NewCoin("ujuno", math.NewInt(2)),
-			withdrawalAmount: sdk.NewCoin("ujuno", math.NewInt(98)),
+			fcAmount:         math.NewInt(2),
+			withdrawalAmount: math.NewInt(98),
 		},
 		{
 			name:             "non staking denom - non-default (9%) to fc, remainder to withdrawal",
-			amount:           sdk.NewCoin("ibc/01234567890123456789012345678901", math.NewInt(100)),
-			fcAmount:         sdk.NewCoin("ibc/01234567890123456789012345678901", math.NewInt(9)),
-			withdrawalAmount: sdk.NewCoin("ibc/01234567890123456789012345678901", math.NewInt(91)),
+			amount:           sdk.NewCoin("uakt", math.NewInt(100)),
+			fcAmount:         math.NewInt(9),
+			withdrawalAmount: math.NewInt(91),
 			feeAmount:        &nineDec, // 0.09 = 9%
 		},
 	}
@@ -61,7 +62,9 @@ func (suite *KeeperTestSuite) TestHandleMsgTransferGood() {
 			quicksilver := suite.GetQuicksilverApp(suite.chainA)
 			ctx := suite.chainA.GetContext()
 
-			err := quicksilver.BankKeeper.MintCoins(ctx, icstypes.ModuleName, sdk.NewCoins(tc.amount))
+			ibcDenom := utils.DeriveIbcDenom("transfer", "channel-0", tc.amount.Denom)
+
+			err := quicksilver.BankKeeper.MintCoins(ctx, icstypes.ModuleName, sdk.NewCoins(sdk.NewCoin(ibcDenom, tc.amount.Amount)))
 			suite.Require().NoError(err)
 
 			if tc.feeAmount != nil {
@@ -99,12 +102,10 @@ func (suite *KeeperTestSuite) TestHandleMsgTransferGood() {
 			suite.Require().Equal(sdk.Coins{}, txMaccBalance)
 
 			// assert that fee collector module balance is the expected value
-			suite.Require().Contains(feeMaccBalance, tc.fcAmount)
+			suite.Require().Equal(feeMaccBalance.AmountOf(ibcDenom), tc.fcAmount)
 
-			if !tc.withdrawalAmount.IsNil() {
-				// assert that zone withdrawal address balance (local chain) is the expected value
-				suite.Require().Contains(wdAccountBalance, tc.withdrawalAmount)
-			}
+			// assert that zone withdrawal address balance (local chain) is the expected value
+			suite.Require().Equal(wdAccountBalance.AmountOf(ibcDenom), tc.withdrawalAmount)
 		})
 	}
 }
@@ -1451,7 +1452,8 @@ func (suite *KeeperTestSuite) Test_v045Callback() {
 		{
 			name: "msg response with some data",
 			setStatements: func(ctx sdk.Context, quicksilver *app.Quicksilver) ([]sdk.Msg, []byte) {
-				err := quicksilver.BankKeeper.MintCoins(ctx, icstypes.ModuleName, sdk.NewCoins(sdk.NewCoin("denom", sdk.NewInt(100))))
+				ibcDenom := utils.DeriveIbcDenom("transfer", "channel-0", "denom")
+				err := quicksilver.BankKeeper.MintCoins(ctx, icstypes.ModuleName, sdk.NewCoins(sdk.NewCoin(ibcDenom, sdk.NewInt(100))))
 				suite.Require().NoError(err)
 				sender := addressutils.GenerateAccAddressForTest()
 				senderAddr, err := sdk.Bech32ifyAddressBytes("cosmos", sender)
@@ -1478,8 +1480,8 @@ func (suite *KeeperTestSuite) Test_v045Callback() {
 				feeMaccBalance2 := quicksilver.BankKeeper.GetAllBalances(ctx, feeMacc)
 
 				// assert that ics module balance is now 100denom less than before HandleMsgTransfer()
-
-				if txMaccBalance2.AmountOf("denom").Equal(sdk.ZeroInt()) && feeMaccBalance2.AmountOf("denom").Equal(sdk.NewInt(100)) {
+				ibcDenom := utils.DeriveIbcDenom("transfer", "channel-0", "denom")
+				if txMaccBalance2.AmountOf(ibcDenom).Equal(sdk.ZeroInt()) && feeMaccBalance2.AmountOf(ibcDenom).Equal(sdk.NewInt(100)) {
 					return true
 				}
 				return false
@@ -1571,7 +1573,9 @@ func (suite *KeeperTestSuite) Test_v046Callback() {
 		{
 			name: "msg response with some data",
 			setStatements: func(ctx sdk.Context, quicksilver *app.Quicksilver) ([]sdk.Msg, *codectypes.Any) {
-				err := quicksilver.BankKeeper.MintCoins(ctx, icstypes.ModuleName, sdk.NewCoins(sdk.NewCoin("denom", sdk.NewInt(100))))
+				// since SendPacket did not prefix the denomination, we must prefix denomination here
+				ibcDenom := utils.DeriveIbcDenom("transfer", "channel-0", "denom")
+				err := quicksilver.BankKeeper.MintCoins(ctx, icstypes.ModuleName, sdk.NewCoins(sdk.NewCoin(ibcDenom, sdk.NewInt(100))))
 				suite.Require().NoError(err)
 				sender := addressutils.GenerateAccAddressForTest()
 				senderAddr, err := sdk.Bech32ifyAddressBytes("cosmos", sender)
@@ -1599,8 +1603,8 @@ func (suite *KeeperTestSuite) Test_v046Callback() {
 				feeMaccBalance2 := quicksilver.BankKeeper.GetAllBalances(ctx, feeMacc)
 
 				// assert that ics module balance is now 100denom less than before HandleMsgTransfer()
-
-				if txMaccBalance2.AmountOf("denom").Equal(sdk.ZeroInt()) && feeMaccBalance2.AmountOf("denom").Equal(sdk.NewInt(100)) {
+				ibcDenom := utils.DeriveIbcDenom("transfer", "channel-0", "denom")
+				if txMaccBalance2.AmountOf(ibcDenom).Equal(sdk.ZeroInt()) && feeMaccBalance2.AmountOf(ibcDenom).Equal(sdk.NewInt(100)) {
 					return true
 				}
 				return false

x/interchainstaking/types/zones.go

@@ -26,6 +26,10 @@ func (z Zone) IsDelegateAddress(addr string) bool {
 	return z.DelegationAddress != nil && z.DelegationAddress.Address == addr
 }
 
+func (z Zone) IsWithdrawalAddress(addr string) bool {
+	return z.WithdrawalAddress != nil && z.WithdrawalAddress.Address == addr
+}
+
 func (z *Zone) GetDelegationAccount() (*ICAAccount, error) {
 	if z.DelegationAddress == nil {
 		return nil, fmt.Errorf("no delegation account set: %v", z)