fix(bridging_fee): getting correct denom for incoming transfers (#1567)

ibctesting/bridging_fee_test.go

@@ -114,3 +114,95 @@ func (s *bridgingFeeSuite) TestBridgingFee() {
 	txFeesBalance := s.hubApp().BankKeeper.GetBalance(s.hubCtx(), addr.GetAddress(), denom)
 	s.True(txFeesBalance.IsZero())
 }
+
+func (s *bridgingFeeSuite) TestBridgingFeeReturnTokens() {
+	path := s.newTransferPath(s.hubChain(), s.rollappChain())
+	s.coordinator.Setup(path)
+	s.createRollappWithFinishedGenesis(path.EndpointA.ChannelID)
+	s.registerSequencer()
+	s.setRollappLightClientID(s.rollappCtx().ChainID(), path.EndpointA.ClientID)
+
+	hubEndpoint := path.EndpointA
+	rollappEndpoint := path.EndpointB
+	rollappIBCKeeper := s.rollappChain().App.GetIBCKeeper()
+
+	// Update rollapp state
+	currentRollappBlockHeight := uint64(s.rollappCtx().BlockHeight())
+	s.updateRollappState(currentRollappBlockHeight)
+
+	timeoutHeight := clienttypes.NewHeight(100, 110)
+	amount, ok := sdk.NewIntFromString("10000000000000000000") // 10DYM
+	s.Require().True(ok)
+	initialCoin := sdk.NewCoin(sdk.DefaultBondDenom, amount)
+
+	// First transfer: Hub -> Rollapp
+	msg := types.NewMsgTransfer(
+		hubEndpoint.ChannelConfig.PortID,
+		hubEndpoint.ChannelID,
+		initialCoin,
+		s.hubChain().SenderAccount.GetAddress().String(),
+		s.rollappChain().SenderAccount.GetAddress().String(),
+		timeoutHeight,
+		0,
+		"",
+	)
+	res, err := s.hubChain().SendMsgs(msg)
+	s.Require().NoError(err)
+	packet, err := ibctesting.ParsePacketFromEvents(res.GetEvents())
+	s.Require().NoError(err)
+	err = path.RelayPacket(packet)
+	s.NoError(err)
+
+	// Get the IBC denom on rollapp
+	rollappIBCDenom := s.getRollappToHubIBCDenomFromPacket(packet)
+	rollappReceivedCoin := sdk.NewCoin(rollappIBCDenom, initialCoin.Amount)
+	rollappBalance := s.rollappApp().BankKeeper.GetBalance(s.rollappCtx(), s.rollappChain().SenderAccount.GetAddress(), rollappIBCDenom)
+	s.Equal(rollappReceivedCoin, rollappBalance)
+
+	// Second transfer: Rollapp -> Hub (sending back the tokens)
+	msg = types.NewMsgTransfer(
+		rollappEndpoint.ChannelConfig.PortID,
+		rollappEndpoint.ChannelID,
+		rollappReceivedCoin,
+		s.rollappChain().SenderAccount.GetAddress().String(),
+		s.hubChain().SenderAccount.GetAddress().String(),
+		timeoutHeight,
+		0,
+		"",
+	)
+	res, err = s.rollappChain().SendMsgs(msg)
+	s.Require().NoError(err)
+	packet, err = ibctesting.ParsePacketFromEvents(res.GetEvents())
+	s.Require().NoError(err)
+	found := rollappIBCKeeper.ChannelKeeper.HasPacketCommitment(s.rollappCtx(), packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
+	s.Require().True(found)
+	err = path.RelayPacket(packet)
+	s.Require().Error(err) // expecting error as no AcknowledgePacket expected to return
+
+	// Get the final IBC denom on hub (should be the original denom)
+	hubDenom := s.getRollappToHubIBCDenomFromPacket(packet)
+	s.Equal(sdk.DefaultBondDenom, hubDenom) // Verify it's the original token returning
+
+	// Check balance before finalization
+	recipient := s.hubChain().SenderAccount.GetAddress()
+	initialHubBalance := s.hubApp().BankKeeper.SpendableCoins(s.hubCtx(), recipient)
+
+	// Finalize the rollapp state
+	currentRollappBlockHeight = uint64(s.rollappCtx().BlockHeight())
+	_, err = s.finalizeRollappState(1, currentRollappBlockHeight)
+	s.Require().NoError(err)
+
+	// Manually finalize packets through x/delayedack
+	s.finalizeRollappPacketsByAddress(s.hubChain().SenderAccount.GetAddress().String())
+
+	// Check balance after finalization
+	expectedFee := s.hubApp().DelayedAckKeeper.BridgingFeeFromAmt(s.hubCtx(), rollappReceivedCoin.Amount)
+	expectedBalance := initialHubBalance.Add(sdk.NewCoin(hubDenom, rollappReceivedCoin.Amount)).Sub(sdk.NewCoin(hubDenom, expectedFee))
+	finalBalance := s.hubApp().BankKeeper.SpendableCoins(s.hubCtx(), recipient)
+	s.Equal(expectedBalance, finalBalance)
+
+	// Check fees are burned
+	addr := s.hubApp().AccountKeeper.GetModuleAccount(s.hubCtx(), txfees.ModuleName)
+	txFeesBalance := s.hubApp().BankKeeper.GetBalance(s.hubCtx(), addr.GetAddress(), hubDenom)
+	s.True(txFeesBalance.IsZero())
+}

ibctesting/utils_test.go

@@ -28,6 +28,7 @@ import (
 
 	"github.com/dymensionxyz/dymension/v3/app"
 	"github.com/dymensionxyz/dymension/v3/app/apptesting"
+	denomutils "github.com/dymensionxyz/dymension/v3/utils/denom"
 	common "github.com/dymensionxyz/dymension/v3/x/common/types"
 	delayedackkeeper "github.com/dymensionxyz/dymension/v3/x/delayedack/keeper"
 	delayedacktypes "github.com/dymensionxyz/dymension/v3/x/delayedack/types"
@@ -287,17 +288,8 @@ func (s *utilSuite) getRollappToHubIBCDenomFromPacket(packet channeltypes.Packet
 	var data transfertypes.FungibleTokenPacketData
 	err := eibctypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &data)
 	s.Require().NoError(err)
-	return s.getIBCDenomForChannel(packet.GetDestChannel(), data.Denom)
-}
 
-func (s *utilSuite) getIBCDenomForChannel(channel string, denom string) string {
-	// since SendPacket did not prefix the denomination, we must prefix denomination here
-	sourcePrefix := types.GetDenomPrefix("transfer", channel)
-	// NOTE: sourcePrefix contains the trailing "/"
-	prefixedDenom := sourcePrefix + denom
-	// construct the denomination trace from the full raw denomination
-	denomTrace := types.ParseDenomTrace(prefixedDenom)
-	return denomTrace.IBCDenom()
+	return denomutils.GetIncomingTransferDenom(packet, data)
 }
 
 func (s *utilSuite) newTestChainWithSingleValidator(t *testing.T, coord *ibctesting.Coordinator, chainID string) *ibctesting.TestChain {

utils/denom/ibc.go

@@ -3,7 +3,9 @@ package denom
 import (
 	"strings"
 
-	transferTypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
+	transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
+	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
+	"github.com/dymensionxyz/sdk-utils/utils/uibc"
 )
 
 // ValidateIBCDenom validates that the given denomination is a valid fungible token representation (i.e 'ibc/{hash}')
@@ -13,7 +15,7 @@ import (
 func ValidateIBCDenom(denom string) (string, bool) {
 	denomSplit := strings.SplitN(denom, "/", 2)
 
-	if len(denomSplit) == 2 && denomSplit[0] == transferTypes.DenomPrefix && strings.TrimSpace(denomSplit[1]) != "" {
+	if len(denomSplit) == 2 && denomSplit[0] == transfertypes.DenomPrefix && strings.TrimSpace(denomSplit[1]) != "" {
 		return denomSplit[1], true
 	}
 
@@ -35,3 +37,27 @@ func SourcePortChanFromTracePath(tracePath string) (sourcePort, sourceChannel st
 	}
 	return sourcePort, sourceChannel, true
 }
+
+// GetIncomingTransferDenom returns the denom that will be credited on the hub.
+// The denom logic follows the transfer middleware's logic and is necessary in order to prefix/non-prefix the denom
+// based on the original chain it was sent from.
+func GetIncomingTransferDenom(packet channeltypes.Packet, fungibleTokenPacketData transfertypes.FungibleTokenPacketData) string {
+	var denom string
+
+	if transfertypes.ReceiverChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), fungibleTokenPacketData.Denom) {
+		// remove prefix added by sender chain
+		voucherPrefix := transfertypes.GetDenomPrefix(packet.GetSourcePort(), packet.GetSourceChannel())
+		unprefixedDenom := fungibleTokenPacketData.Denom[len(voucherPrefix):]
+		// coin denomination used in sending from the escrow address
+		denom = unprefixedDenom
+		// The denomination used to send the coins is either the native denom or the hash of the path
+		// if the denomination is not native.
+		denomTrace := transfertypes.ParseDenomTrace(unprefixedDenom)
+		if denomTrace.Path != "" {
+			denom = denomTrace.IBCDenom()
+		}
+	} else {
+		denom = uibc.GetForeignDenomTrace(packet.GetDestChannel(), fungibleTokenPacketData.Denom).IBCDenom()
+	}
+	return denom
+}

x/bridgingfee/ibc_module.go

@@ -9,8 +9,8 @@ import (
 	transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
 	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
 	"github.com/cosmos/ibc-go/v7/modules/core/exported"
+	denomutils "github.com/dymensionxyz/dymension/v3/utils/denom"
 	"github.com/dymensionxyz/sdk-utils/utils/uevent"
-	"github.com/dymensionxyz/sdk-utils/utils/uibc"
 	"github.com/osmosis-labs/osmosis/v15/osmoutils"
 	txfeeskeeper "github.com/osmosis-labs/osmosis/v15/x/txfees/keeper"
 
@@ -65,7 +65,12 @@ func (w IBCModule) logger(
 	)
 }
 
-func (w *IBCModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) exported.Acknowledgement {
+// OnRecvPacket implements the IBCModule interface. It processes IBC transfer packets,
+// charging bridging fees for transfers from rollapps to the hub. The fee is charged
+// in the denomination of the incoming tokens, which is determined by:
+// - For tokens originating from the hub: the original denomination
+// - For tokens originating on the rollapp: the IBC denomination on the hub
+func (w IBCModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) exported.Acknowledgement {
 	l := w.logger(ctx, packet, "OnRecvPacket")
 
 	if commontypes.SkipRollappMiddleware(ctx) {
@@ -91,8 +96,8 @@ func (w *IBCModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, re
 	receiver := sdk.MustAccAddressFromBech32(transfer.Receiver)
 
 	feeAmt := w.delayedAckKeeper.BridgingFeeFromAmt(ctx, transfer.MustAmountInt())
-	denomTrace := uibc.GetForeignDenomTrace(packet.GetDestChannel(), transfer.Denom)
-	feeCoin := sdk.NewCoin(denomTrace.IBCDenom(), feeAmt)
+	denom := denomutils.GetIncomingTransferDenom(packet, transfer.FungibleTokenPacketData)
+	feeCoin := sdk.NewCoin(denom, feeAmt)
 
 	// charge the bridging fee in cache context
 	err = osmoutils.ApplyFuncIfNoError(ctx, func(ctx sdk.Context) error {

x/eibc/keeper/handler.go

@@ -5,11 +5,10 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
-	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
 	"github.com/dymensionxyz/sdk-utils/utils/uevent"
-	"github.com/dymensionxyz/sdk-utils/utils/uibc"
 	"github.com/pkg/errors"
 
+	denomutils "github.com/dymensionxyz/dymension/v3/utils/denom"
 	commontypes "github.com/dymensionxyz/dymension/v3/x/common/types"
 	dacktypes "github.com/dymensionxyz/dymension/v3/x/delayedack/types"
 	"github.com/dymensionxyz/dymension/v3/x/eibc/types"
@@ -90,7 +89,7 @@ func (k *Keeper) CreateDemandOrderOnRecv(ctx sdk.Context, fungibleTokenPacketDat
 		return nil, err
 	}
 
-	demandOrderDenom := k.getEIBCTransferDenom(*rollappPacket.Packet, fungibleTokenPacketData)
+	demandOrderDenom := denomutils.GetIncomingTransferDenom(*rollappPacket.Packet, fungibleTokenPacketData)
 	demandOrderRecipient := fungibleTokenPacketData.Receiver // who we tried to send to
 	creationHeight := uint64(ctx.BlockHeight())
 
@@ -130,29 +129,6 @@ func (k Keeper) CreateDemandOrderOnErrAckOrTimeout(ctx sdk.Context, fungibleToke
 	return order, nil
 }
 
-// getEIBCTransferDenom returns the actual denom that will be credited to the eIBC fulfiller.
-// The denom logic follows the transfer middleware's logic and is necessary in order to prefix/non-prefix the denom
-// based on the original chain it was sent from.
-func (k *Keeper) getEIBCTransferDenom(packet channeltypes.Packet, fungibleTokenPacketData transfertypes.FungibleTokenPacketData) string {
-	var denom string
-	if transfertypes.ReceiverChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), fungibleTokenPacketData.Denom) {
-		// remove prefix added by sender chain
-		voucherPrefix := transfertypes.GetDenomPrefix(packet.GetSourcePort(), packet.GetSourceChannel())
-		unprefixedDenom := fungibleTokenPacketData.Denom[len(voucherPrefix):]
-		// coin denomination used in sending from the escrow address
-		denom = unprefixedDenom
-		// The denomination used to send the coins is either the native denom or the hash of the path
-		// if the denomination is not native.
-		denomTrace := transfertypes.ParseDenomTrace(unprefixedDenom)
-		if denomTrace.Path != "" {
-			denom = denomTrace.IBCDenom()
-		}
-	} else {
-		denom = uibc.GetForeignDenomTrace(packet.GetDestChannel(), fungibleTokenPacketData.Denom).IBCDenom()
-	}
-	return denom
-}
-
 func (k Keeper) BlockedAddr(addr string) bool {
 	account, err := sdk.AccAddressFromBech32(addr)
 	if err != nil {