feat: added logic to refundGasFee in failure outbound txs as well

Author: 0xNilesh

test/integration/uexecutor/gas_fee_refund_test.go

@@ -13,28 +13,28 @@ import (
 
 func TestGasFeeRefund(t *testing.T) {
 
-	t.Run("success vote with empty gasFeeUsed is rejected", func(t *testing.T) {
-		app, ctx, vals, utxId, outbound, coreVals :=
-			setupOutboundVotingTest(t, 4)
+	t.Run("vote with empty gasFeeUsed is always rejected", func(t *testing.T) {
+		// gas_fee_used is mandatory for both success and failure votes.
+		for _, success := range []bool{true, false} {
+			app, ctx, vals, utxId, outbound, coreVals :=
+				setupOutboundVotingTest(t, 4)
 
-		valAddr, err := sdk.ValAddressFromBech32(coreVals[0].OperatorAddress)
-		require.NoError(t, err)
-		coreAcc := sdk.AccAddress(valAddr).String()
-
-		err = utils.ExecVoteOutbound(
-			t,
-			ctx,
-			app,
-			vals[0],
-			coreAcc,
-			utxId,
-			outbound,
-			true,
-			"",
-			"", // gas_fee_used required when success=true → must be rejected
-		)
-		require.Error(t, err)
-		require.Contains(t, err.Error(), "gas_fee_used required when success=true")
+			valAddr, err := sdk.ValAddressFromBech32(coreVals[0].OperatorAddress)
+			require.NoError(t, err)
+			coreAcc := sdk.AccAddress(valAddr).String()
+
+			revertReason := ""
+			if !success {
+				revertReason = "execution failed"
+			}
+
+			err = utils.ExecVoteOutbound(
+				t, ctx, app, vals[0], coreAcc, utxId, outbound,
+				success, revertReason, "", // empty gas_fee_used → must be rejected
+			)
+			require.Error(t, err)
+			require.Contains(t, err.Error(), "observed_tx.gas_fee_used is required")
+		}
 	})
 
 	t.Run("no refund when gasFeeUsed equals gasFee", func(t *testing.T) {
@@ -196,11 +196,12 @@ func TestGasFeeRefund(t *testing.T) {
 		require.Equal(t, uexecutortypes.Status_OBSERVED, ob.OutboundStatus)
 	})
 
-	t.Run("failed outbound still performs revert not refund", func(t *testing.T) {
+	t.Run("failed outbound performs both revert and gas refund", func(t *testing.T) {
 		app, ctx, vals, utxId, outbound, coreVals :=
 			setupOutboundVotingTest(t, 4)
 
-		// Vote as FAILED with gasFeeUsed set — refund should NOT run for failed outbounds
+		// gasFee = 111 (mock), gasFeeUsed = 50 → 61 excess to refund.
+		// Both the bridged funds revert AND the excess gas refund must happen.
 		for i := 0; i < 3; i++ {
 			valAddr, err := sdk.ValAddressFromBech32(coreVals[i].OperatorAddress)
 			require.NoError(t, err)
@@ -216,7 +217,7 @@ func TestGasFeeRefund(t *testing.T) {
 				outbound,
 				false,
 				"execution failed",
-				"50", // gasFeeUsed provided but shouldn't trigger refund on failure
+				"50", // gasFeeUsed=50 < gasFee=111 → 61 excess
 			)
 			require.NoError(t, err)
 		}
@@ -227,12 +228,13 @@ func TestGasFeeRefund(t *testing.T) {
 		ob := utx.OutboundTx[0]
 		require.Equal(t, uexecutortypes.Status_REVERTED, ob.OutboundStatus)
 
-		// Revert was executed (funds minted back)
+		// Revert: bridged funds minted back
 		require.NotNil(t, ob.PcRevertExecution)
 		require.Equal(t, "SUCCESS", ob.PcRevertExecution.Status)
 
-		// Gas refund must NOT run for failed outbounds
-		require.Nil(t, ob.PcRefundExecution,
-			"gas refund must not run when outbound failed")
+		// Gas refund: excess gas must also be returned on failure
+		require.NotNil(t, ob.PcRefundExecution,
+			"excess gas must be refunded even when outbound failed")
+		require.NotEmpty(t, ob.PcRefundExecution.Status)
 	})
 }

test/integration/uexecutor/vote_outbound_test.go

@@ -240,7 +240,7 @@ func TestOutboundVoting(t *testing.T) {
 				outbound,
 				false,
 				"execution reverted", // revert reason
-				"",
+				outbound.GasFee,      // gas_fee_used required; use full fee → no excess refund
 			)
 			require.NoError(t, err)
 		}
@@ -281,7 +281,7 @@ func TestOutboundVoting(t *testing.T) {
 				outbound,
 				false,
 				"failed",
-				"",
+				outbound.GasFee, // gas_fee_used required; use full fee → no excess refund
 			)
 			require.NoError(t, err)
 		}

x/uexecutor/keeper/outbound.go

@@ -51,79 +51,86 @@ func (k Keeper) FinalizeOutbound(ctx context.Context, utxId string, outbound typ
 	sdkCtx := sdk.UnwrapSDKContext(ctx)
 
 	if !obs.Success {
-		return k.handleFailedOutbound(sdkCtx, utxId, outbound)
+		return k.handleFailedOutbound(sdkCtx, utxId, outbound, obs)
 	}
 
 	return k.handleSuccessfulOutbound(sdkCtx, utxId, outbound, obs)
 }
 
-// handleFailedOutbound mints back the bridged tokens to the revert recipient.
-func (k Keeper) handleFailedOutbound(ctx sdk.Context, utxId string, outbound types.OutboundTx) error {
-	// Only refund for funds-related tx types
-	if outbound.TxType != types.TxType_FUNDS && outbound.TxType != types.TxType_GAS_AND_PAYLOAD &&
-		outbound.TxType != types.TxType_FUNDS_AND_PAYLOAD {
-		return nil
-	}
+// handleFailedOutbound mints back the bridged tokens to the revert recipient,
+// then attempts to refund any excess gas (gasFee - gasFeeUsed) just like a
+// successful outbound would. Both operations are recorded on the outbound.
+func (k Keeper) handleFailedOutbound(ctx sdk.Context, utxId string, outbound types.OutboundTx, obs *types.OutboundObservation) error {
+	// Only revert bridged funds for funds-related tx types
+	if outbound.TxType == types.TxType_FUNDS || outbound.TxType == types.TxType_GAS_AND_PAYLOAD ||
+		outbound.TxType == types.TxType_FUNDS_AND_PAYLOAD {
+
+		// Decide revert recipient safely
+		recipient := outbound.Sender
+		if outbound.RevertInstructions != nil &&
+			outbound.RevertInstructions.FundRecipient != "" {
+			recipient = outbound.RevertInstructions.FundRecipient
+		}
 
-	// Decide refund recipient safely
-	recipient := outbound.Sender
-	if outbound.RevertInstructions != nil &&
-		outbound.RevertInstructions.FundRecipient != "" {
-		recipient = outbound.RevertInstructions.FundRecipient
-	}
+		amount := new(big.Int)
+		amount, ok := amount.SetString(outbound.Amount, 10)
+		if !ok {
+			return fmt.Errorf("invalid amount: %s", outbound.Amount)
+		}
+		receipt, err := k.CallPRC20Deposit(ctx, common.HexToAddress(outbound.Prc20AssetAddr), common.HexToAddress(recipient), amount)
 
-	// Mint tokens back
-	amount := new(big.Int)
-	amount, ok := amount.SetString(outbound.Amount, 10)
-	if !ok {
-		return fmt.Errorf("invalid amount: %s", outbound.Amount)
+		pcTx := types.PCTx{
+			Sender:      outbound.Sender,
+			BlockHeight: uint64(ctx.BlockHeight()),
+		}
+		if err != nil {
+			pcTx.Status = "FAILED"
+			pcTx.ErrorMsg = err.Error()
+		} else {
+			pcTx.TxHash = receipt.Hash
+			pcTx.GasUsed = receipt.GasUsed
+			pcTx.Status = "SUCCESS"
+		}
+		outbound.PcRevertExecution = &pcTx
 	}
-	receipt, err := k.CallPRC20Deposit(ctx, common.HexToAddress(outbound.Prc20AssetAddr), common.HexToAddress(recipient), amount)
 
-	// Update outbound status
 	outbound.OutboundStatus = types.Status_REVERTED
 
-	pcTx := types.PCTx{
-		TxHash:      "",
-		Sender:      outbound.Sender,
-		GasUsed:     0,
-		BlockHeight: uint64(ctx.BlockHeight()),
-	}
-
-	if err != nil {
-		pcTx.Status = "FAILED"
-		pcTx.ErrorMsg = err.Error()
-	} else {
-		pcTx.TxHash = receipt.Hash
-		pcTx.GasUsed = receipt.GasUsed
-		pcTx.Status = "SUCCESS"
-	}
-
-	outbound.PcRevertExecution = &pcTx
+	// Refund excess gas regardless of tx type — gas was consumed on the external
+	// chain whether the execution succeeded or failed.
+	k.applyGasRefund(ctx, &outbound, obs)
 
 	return k.UpdateOutbound(ctx, utxId, outbound)
 }
 
 // handleSuccessfulOutbound refunds unused gas fee when gasFee > gasFeeUsed.
 func (k Keeper) handleSuccessfulOutbound(ctx sdk.Context, utxId string, outbound types.OutboundTx, obs *types.OutboundObservation) error {
-	// Only attempt refund if gas_fee_used is populated and gas token is known
+	k.applyGasRefund(ctx, &outbound, obs)
+	return k.UpdateOutbound(ctx, utxId, outbound)
+}
+
+// applyGasRefund computes the excess gas (gasFee - gasFeeUsed) and, if positive,
+// calls UniversalCore refundUnusedGas. The result is recorded in outbound.PcRefundExecution.
+// It is called for both successful and failed outbounds — gas is consumed on the
+// external chain regardless of execution outcome.
+func (k Keeper) applyGasRefund(ctx sdk.Context, outbound *types.OutboundTx, obs *types.OutboundObservation) {
 	if obs.GasFeeUsed == "" || outbound.GasFee == "" || outbound.GasToken == "" {
-		return k.UpdateOutbound(ctx, utxId, outbound)
+		return
 	}
 
 	gasFee := new(big.Int)
 	if _, ok := gasFee.SetString(outbound.GasFee, 10); !ok {
-		return k.UpdateOutbound(ctx, utxId, outbound)
+		return
 	}
 
 	gasFeeUsed := new(big.Int)
 	if _, ok := gasFeeUsed.SetString(obs.GasFeeUsed, 10); !ok {
-		return k.UpdateOutbound(ctx, utxId, outbound)
+		return
 	}
 
-	// No refund needed
+	// No excess gas to refund
 	if gasFee.Cmp(gasFeeUsed) <= 0 {
-		return k.UpdateOutbound(ctx, utxId, outbound)
+		return
 	}
 
 	refundAmount := new(big.Int).Sub(gasFee, gasFeeUsed)
@@ -144,7 +151,6 @@ func (k Keeper) handleSuccessfulOutbound(ctx sdk.Context, utxId string, outbound
 	// Step 1: try refund with swap (gasToken → PC native)
 	fee, swapErr := k.GetDefaultFeeTierForToken(ctx, gasToken)
 	var swapFallbackReason string
-	var receipt interface{ GetHash() string }
 
 	if swapErr == nil {
 		quote, quoteErr := k.getSwapQuoteForRefund(ctx, gasToken, fee, refundAmount)
@@ -158,7 +164,7 @@ func (k Keeper) handleSuccessfulOutbound(ctx sdk.Context, utxId string, outbound
 				refundPcTx.GasUsed = resp.GasUsed
 				refundPcTx.Status = "SUCCESS"
 				outbound.PcRefundExecution = refundPcTx
-				return k.UpdateOutbound(ctx, utxId, outbound)
+				return
 			}
 			swapFallbackReason = fmt.Sprintf("swap refund failed: %s", err.Error())
 		} else {
@@ -168,10 +174,8 @@ func (k Keeper) handleSuccessfulOutbound(ctx sdk.Context, utxId string, outbound
 		swapFallbackReason = fmt.Sprintf("fee tier fetch failed: %s", swapErr.Error())
 	}
 
-	_ = receipt
-
 	// Step 2: fallback — refund without swap (deposit PRC20 directly to recipient)
-	ctx.Logger().Error("FinalizeOutbound: swap refund failed, falling back to no-swap",
+	ctx.Logger().Error("applyGasRefund: swap refund failed, falling back to no-swap",
 		"outbound_id", outbound.Id,
 		"reason", swapFallbackReason,
 	)
@@ -188,8 +192,6 @@ func (k Keeper) handleSuccessfulOutbound(ctx sdk.Context, utxId string, outbound
 
 	outbound.PcRefundExecution = refundPcTx
 	outbound.RefundSwapError = swapFallbackReason
-
-	return k.UpdateOutbound(ctx, utxId, outbound)
 }
 
 // getSwapQuoteForRefund fetches a Uniswap quote for the gas token refund swap.

x/uexecutor/types/msg_vote_outbound.go

@@ -68,8 +68,15 @@ func (msg *MsgVoteOutbound) ValidateBasic() error {
 	// Validate observed_tx content
 	obs := msg.ObservedTx
 
+	// gas_fee_used is always required — the external chain consumes gas regardless
+	// of success or failure, and excess gas must be refundable in both cases.
+	if strings.TrimSpace(obs.GasFeeUsed) == "" {
+		return errors.Wrap(sdkerrors.ErrInvalidRequest,
+			"observed_tx.gas_fee_used is required")
+	}
+
 	if obs.Success {
-		// Success requires tx_hash AND block_height > 0 AND gas_fee_used
+		// Success additionally requires tx_hash and block_height.
 		if strings.TrimSpace(obs.TxHash) == "" {
 			return errors.Wrap(sdkerrors.ErrInvalidRequest,
 				"observed_tx.tx_hash required when success=true")
@@ -78,14 +85,8 @@ func (msg *MsgVoteOutbound) ValidateBasic() error {
 			return errors.Wrap(sdkerrors.ErrInvalidRequest,
 				"observed_tx.block_height must be > 0 when success=true")
 		}
-		if strings.TrimSpace(obs.GasFeeUsed) == "" {
-			return errors.Wrap(sdkerrors.ErrInvalidRequest,
-				"observed_tx.gas_fee_used required when success=true")
-		}
-
 	} else {
-		// Failure case:
-		// tx_hash MAY be empty.
+		// Failure case: tx_hash MAY be empty.
 		// BUT if tx_hash is present, block_height must be > 0.
 		if strings.TrimSpace(obs.TxHash) != "" && obs.BlockHeight == 0 {
 			return errors.Wrap(sdkerrors.ErrInvalidRequest,