From 3e6454d029a7bfe38c197689a9a1e460e7888a4d Mon Sep 17 00:00:00 2001
From: fx0x55 <dennis@wokoworks.com>
Date: Fri, 16 Dec 2022 11:26:40 +0800
Subject: [PATCH] fix: can't parse OnAck and OnTimeout packetData Receiver

---
 x/ibc/applications/transfer/keeper/parse.go   | 18 ++++++
 .../transfer/keeper/parse_test.go             | 63 +++++++++++++++++++
 x/ibc/applications/transfer/keeper/relay.go   |  4 +-
 3 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/x/ibc/applications/transfer/keeper/parse.go b/x/ibc/applications/transfer/keeper/parse.go
index e7a8d81..5e35696 100644
--- a/x/ibc/applications/transfer/keeper/parse.go
+++ b/x/ibc/applications/transfer/keeper/parse.go
@@ -81,6 +81,24 @@ func parseReceiveAndAmountByPacket(data types.FungibleTokenPacketData) (sdk.AccA
 	return receiverAddr, transferAmount, sdk.ZeroInt(), nil
 }
 
+func parseAmountAndFeeByPacket(data types.FungibleTokenPacketData) (sdk.Int, sdk.Int, error) {
+	// parse the transfer amount
+	transferAmount, ok := sdk.NewIntFromString(data.Amount)
+	if !ok {
+		return sdk.Int{}, sdk.Int{}, sdkerrors.Wrapf(transfertypes.ErrInvalidAmount, "unable to parse transfer amount (%s) into sdk.Int", data.Amount)
+	}
+
+	feeAmount := sdk.ZeroInt()
+	if data.Router != "" {
+		fee, ok := sdk.NewIntFromString(data.Fee)
+		if !ok || fee.IsNegative() {
+			return sdk.Int{}, sdk.Int{}, sdkerrors.Wrapf(transfertypes.ErrInvalidAmount, "fee amount is invalid:%s", data.Fee)
+		}
+		feeAmount = fee
+	}
+	return transferAmount, feeAmount, nil
+}
+
 func parsePacketAddress(ibcSender string) (sdk.AccAddress, error) {
 	_, addBytes, err := bech32.DecodeAndConvert(ibcSender)
 	return addBytes, err
diff --git a/x/ibc/applications/transfer/keeper/parse_test.go b/x/ibc/applications/transfer/keeper/parse_test.go
index 3b64f3e..2e8605b 100644
--- a/x/ibc/applications/transfer/keeper/parse_test.go
+++ b/x/ibc/applications/transfer/keeper/parse_test.go
@@ -66,6 +66,69 @@ func TestParseReceiveAndAmountByPacket(t *testing.T) {
 	}
 }
 
+func TestParseAmountAndFeeByPacket(t *testing.T) {
+	type expect struct {
+		amount sdk.Int
+		fee    sdk.Int
+	}
+	testCases := []struct {
+		name    string
+		packet  types.FungibleTokenPacketData
+		expPass bool
+		errStr  string
+		expect  expect
+	}{
+		{
+			"pass - no router only amount ",
+			types.FungibleTokenPacketData{Amount: "1"},
+			true, "",
+			expect{amount: sdk.NewInt(1), fee: sdk.ZeroInt()},
+		},
+		{
+			"error - amount is empty",
+			types.FungibleTokenPacketData{Amount: ""},
+			false,
+			"unable to parse transfer amount () into sdk.Int: invalid token amount",
+			expect{amount: sdk.Int{}, fee: sdk.Int{}},
+		},
+		{
+			"error - fee is empty",
+			types.FungibleTokenPacketData{Amount: "1", Fee: "", Router: "aaa"},
+			false,
+			"fee amount is invalid:: invalid token amount",
+			expect{amount: sdk.Int{}, fee: sdk.Int{}},
+		},
+		{
+			"error - fee is negative",
+			types.FungibleTokenPacketData{Amount: "1", Fee: "-1", Router: "aaa"},
+			false,
+			"fee amount is invalid:-1: invalid token amount",
+			expect{amount: sdk.Int{}, fee: sdk.Int{}},
+		},
+		{
+			"pass - fee is zero",
+			types.FungibleTokenPacketData{Amount: "1", Fee: "0", Router: "aaa"},
+			true,
+			"",
+			expect{amount: sdk.NewInt(1), fee: sdk.ZeroInt()},
+		},
+	}
+
+	for i, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			actualAmount, actualFee, err := parseAmountAndFeeByPacket(tc.packet)
+			if tc.expPass {
+				require.NoError(t, err, "valid test case %d failed: %v", i, err)
+			} else {
+				require.Error(t, err)
+				require.EqualValues(t, tc.errStr, err.Error())
+			}
+			require.EqualValues(t, tc.expect.amount.String(), actualAmount.String())
+			require.EqualValues(t, tc.expect.fee.String(), actualFee.String())
+		})
+	}
+}
+
 func TestParsePacketAddress(t *testing.T) {
 	testCases := []struct {
 		name    string
diff --git a/x/ibc/applications/transfer/keeper/relay.go b/x/ibc/applications/transfer/keeper/relay.go
index 0c8f6bb..a8b565c 100644
--- a/x/ibc/applications/transfer/keeper/relay.go
+++ b/x/ibc/applications/transfer/keeper/relay.go
@@ -241,7 +241,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t
 func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketData, ack channeltypes.Acknowledgement) error {
 	switch ack.Response.(type) {
 	case *channeltypes.Acknowledgement_Error:
-		_, amount, fee, err := parseReceiveAndAmountByPacket(data)
+		amount, fee, err := parseAmountAndFeeByPacket(data)
 		if err != nil {
 			return err
 		}
@@ -258,7 +258,7 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Pac
 // OnTimeoutPacket refunds the sender since the original packet sent was
 // never received and has been timed out.
 func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketData) error {
-	_, amount, fee, err := parseReceiveAndAmountByPacket(data)
+	amount, fee, err := parseAmountAndFeeByPacket(data)
 	if err != nil {
 		return err
 	}