From 65cd29029b5af280dccda3eba408550e763ecdcf Mon Sep 17 00:00:00 2001
From: Michael Tsitrin <114929630+mtsitrin@users.noreply.github.com>
Date: Thu, 28 Nov 2024 09:44:11 +0200
Subject: [PATCH] fix(bridging_fee): adding cacheCtx for charging fees (#1563)

---
 app/keepers/keepers.go      |  1 -
 x/bridgingfee/ibc_module.go | 68 ++++++++++++++++---------------------
 2 files changed, 29 insertions(+), 40 deletions(-)

diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go
index bd20beff8..481456dc5 100644
--- a/app/keepers/keepers.go
+++ b/app/keepers/keepers.go
@@ -545,7 +545,6 @@ func (a *AppKeepers) InitTransferStack() {
 		a.DelayedAckKeeper,
 		a.TransferKeeper,
 		*a.TxFeesKeeper,
-		a.AccountKeeper.GetModuleAddress(txfeestypes.ModuleName),
 	)
 	a.TransferStack = packetforwardmiddleware.NewIBCMiddleware(
 		a.TransferStack,
diff --git a/x/bridgingfee/ibc_module.go b/x/bridgingfee/ibc_module.go
index c246d0db4..6a0236f04 100644
--- a/x/bridgingfee/ibc_module.go
+++ b/x/bridgingfee/ibc_module.go
@@ -11,6 +11,7 @@ import (
 	"github.com/cosmos/ibc-go/v7/modules/core/exported"
 	"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"
 
 	commontypes "github.com/dymensionxyz/dymension/v3/x/common/types"
@@ -32,7 +33,6 @@ type IBCModule struct {
 	delayedAckKeeper delayedackkeeper.Keeper
 	transferKeeper   transferkeeper.Keeper
 	txFeesKeeper     txfeeskeeper.Keeper
-	feeModuleAddr    sdk.AccAddress
 }
 
 func NewIBCModule(
@@ -41,7 +41,6 @@ func NewIBCModule(
 	delayedAckKeeper delayedackkeeper.Keeper,
 	transferKeeper transferkeeper.Keeper,
 	txFeesKeeper txfeeskeeper.Keeper,
-	feeModuleAddr sdk.AccAddress,
 ) *IBCModule {
 	return &IBCModule{
 		IBCModule:        next,
@@ -49,7 +48,6 @@ func NewIBCModule(
 		delayedAckKeeper: delayedAckKeeper,
 		transferKeeper:   transferKeeper,
 		txFeesKeeper:     txFeesKeeper,
-		feeModuleAddr:    feeModuleAddr,
 	}
 }
 
@@ -85,44 +83,36 @@ func (w *IBCModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, re
 		return w.IBCModule.OnRecvPacket(ctx, packet, relayer)
 	}
 
-	// Use the packet as a basis for a fee transfer
-	feeData := transfer
-	fee := w.delayedAckKeeper.BridgingFeeFromAmt(ctx, transfer.MustAmountInt())
-	feeData.Amount = fee.String()
-	feeData.Receiver = w.feeModuleAddr.String()
+	// handle the packet transfer
+	ack := w.IBCModule.OnRecvPacket(ctx, packet, relayer)
+	if !ack.Success() {
+		return ack
+	}
+	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)
 
-	// No event emitted, as we called the transfer keeper directly (vs the transfer middleware)
-	err = w.transferKeeper.OnRecvPacket(ctx, packet, feeData.FungibleTokenPacketData)
+	// charge the bridging fee in cache context
+	err = osmoutils.ApplyFuncIfNoError(ctx, func(ctx sdk.Context) error {
+		return w.txFeesKeeper.ChargeFeesFromPayer(ctx, receiver, feeCoin, nil)
+	})
 	if err != nil {
-		l.Error("Charge bridging fee.", "err", err)
-		// We continue as we don't want the fee charge to fail the transfer in any case
-		fee = sdk.ZeroInt()
-	} else {
-		// Charge the fee from the txfees module account: construct the IBC denom and use it for the fee coin.
-		denomTrace := uibc.GetForeignDenomTrace(packet.GetDestChannel(), feeData.Denom)
-		feeCoin := sdk.NewCoin(denomTrace.IBCDenom(), fee)
-
-		err = w.txFeesKeeper.ChargeFees(ctx, feeCoin, nil, transfer.Receiver)
-		if err != nil {
-			// We continue as we don't want the fee charge to fail the transfer in any case.
-			// Also, the fee was already successfully sent to x/txfees and charging will be retried at the epoch end.
-			w.logger(ctx, packet, "OnRecvPacket").Error("Charge bridging fee from x/txfees account.", "err", err)
-		}
-
-		ctx.EventManager().EmitEvent(
-			sdk.NewEvent(
-				EventTypeBridgingFee,
-				sdk.NewAttribute(AttributeKeyFee, fee.String()),
-				sdk.NewAttribute(sdk.AttributeKeySender, transfer.Sender),
-				sdk.NewAttribute(transfertypes.AttributeKeyReceiver, transfer.Receiver),
-				sdk.NewAttribute(transfertypes.AttributeKeyDenom, transfer.Denom),
-				sdk.NewAttribute(transfertypes.AttributeKeyAmount, transfer.Amount),
-			),
-		)
+		// We continue as we don't want the fee charge to fail the transfer in any case.
+		l.Error("Charge bridging fee from payer.", "receiver", receiver, "err", err)
 	}
 
-	// transfer the rest to the original recipient
-	transfer.Amount = transfer.MustAmountInt().Sub(fee).String()
-	packet.Data = transfer.GetBytes()
-	return w.IBCModule.OnRecvPacket(ctx, packet, relayer)
+	ctx.EventManager().EmitEvent(
+		sdk.NewEvent(
+			EventTypeBridgingFee,
+			sdk.NewAttribute(AttributeKeyFee, feeAmt.String()),
+			sdk.NewAttribute(sdk.AttributeKeySender, transfer.Sender),
+			sdk.NewAttribute(transfertypes.AttributeKeyReceiver, transfer.Receiver),
+			sdk.NewAttribute(transfertypes.AttributeKeyDenom, transfer.Denom),
+			sdk.NewAttribute(transfertypes.AttributeKeyAmount, transfer.Amount),
+		),
+	)
+
+	return ack
 }