Upgrade x/ibcswap to be compatible with PFM v7.0.2

- Fix issues relating to PFMs new overrideReceiver address
- Small cleanup up OnRecvPacket for greater clarity
- Get rid of NonRefundable swap param and just use presence NeutronRefundAddress
  to determine refund behavior.
- Improve refund behavior to allow for IBC refund after failed forward

go.mod

@@ -17,7 +17,7 @@ require (
 	github.com/cosmos/cosmos-sdk v0.47.5
 	github.com/cosmos/gaia/v11 v11.0.0-00010101000000-000000000000
 	github.com/cosmos/gogoproto v1.4.10
-	github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v7 v7.0.1
+	github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v7 v7.0.2-0.20231103132047-e5a274cf6fc2
 	github.com/cosmos/ibc-go/v7 v7.3.1
 	github.com/cosmos/ics23/go v0.10.0
 	github.com/cosmos/interchain-security/v3 v3.1.0
@@ -169,7 +169,6 @@ require (
 	go.etcd.io/bbolt v1.3.7 // indirect
 	go.opencensus.io v0.24.0 // indirect
 	golang.org/x/crypto v0.13.0 // indirect
-	golang.org/x/exp v0.0.0-20230711153332-06a737ee72cb // indirect
 	golang.org/x/net v0.15.0 // indirect
 	golang.org/x/oauth2 v0.12.0 // indirect
 	golang.org/x/sync v0.3.0 // indirect

go.sum

@@ -399,8 +399,8 @@ github.com/cosmos/gogoproto v1.4.10 h1:QH/yT8X+c0F4ZDacDv3z+xE3WU1P1Z3wQoLMBRJoK
 github.com/cosmos/gogoproto v1.4.10/go.mod h1:3aAZzeRWpAwr+SS/LLkICX2/kDFyaYVzckBDzygIxek=
 github.com/cosmos/iavl v0.20.1 h1:rM1kqeG3/HBT85vsZdoSNsehciqUQPWrR4BYmqE2+zg=
 github.com/cosmos/iavl v0.20.1/go.mod h1:WO7FyvaZJoH65+HFOsDir7xU9FWk2w9cHXNW1XHcl7A=
-github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v7 v7.0.1 h1:8mK4Ha/56P6jkRcLhIYhg/ipWhEuXBtj5O4I6fAi6vg=
-github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v7 v7.0.1/go.mod h1:GGUJN4LnB3J1Luu4kxTklQLbW69So3QXUndSalB003s=
+github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v7 v7.0.2-0.20231103132047-e5a274cf6fc2 h1:FM6+3eZI9j9/Gud4TM6t+v1WgHpvyu95PU3Hwk3pPmk=
+github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v7 v7.0.2-0.20231103132047-e5a274cf6fc2/go.mod h1:GGUJN4LnB3J1Luu4kxTklQLbW69So3QXUndSalB003s=
 github.com/cosmos/ibc-go/v7 v7.3.1 h1:bil1IjnHdyWDASFYKfwdRiNtFP6WK3osW7QFEAgU4I8=
 github.com/cosmos/ibc-go/v7 v7.3.1/go.mod h1:wvx4pPBofe5ZdMNV3OFRxSI4auEP5Qfqf8JXLLNV04g=
 github.com/cosmos/ics23/go v0.10.0 h1:iXqLLgp2Lp+EdpIuwXTYIQU+AiHj9mOC2X9ab++bZDM=

tests/ibc/ibc_setup_test.go

@@ -6,6 +6,7 @@ import (
 
 	"cosmossdk.io/math"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v7/packetforward"
 	transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
 	clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
 	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
@@ -325,6 +326,14 @@ func (s *IBCTestSuite) assertChainCBalance(addr sdk.AccAddress, denom string, ex
 	s.assertBalance(s.bundleC.App.GetTestBankKeeper(), s.bundleC.Chain, addr, denom, expectedAmt)
 }
 
+func (s *IBCTestSuite) ReceiverOverrideAddr(channel, sender string) sdk.AccAddress {
+	addr, err := packetforward.GetReceiver(channel, sender)
+	if err != nil {
+		panic("Cannot calc receiver override: " + err.Error())
+	}
+	return sdk.MustAccAddressFromBech32(addr)
+}
+
 //nolint:unparam // keep this flexible even if we aren't currently using all the params
 func (s *IBCTestSuite) neutronDeposit(
 	token0 string,
@@ -355,11 +364,18 @@ func (s *IBCTestSuite) neutronDeposit(
 
 func (s *IBCTestSuite) RelayAllPacketsAToB(path *icsibctesting.Path) error {
 	sentPackets := path.EndpointA.Chain.SentPackets
+	chainB := path.EndpointB.Chain
 	if len(sentPackets) == 0 {
 		return fmt.Errorf("No packets to send")
 	}
 
 	for _, packet := range sentPackets {
+		// Skip if packet has already been sent
+		ack, _ := chainB.App.GetIBCKeeper().ChannelKeeper.
+			GetPacketAcknowledgement(chainB.GetContext(), packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
+		if ack != nil {
+			continue
+		}
 		err := path.RelayPacket(packet)
 		if err != nil {
 			return err

tests/ibc/swap_forward_test.go

@@ -333,11 +333,29 @@ func (s *IBCTestSuite) TestSwapAndForward_UnwindIBCDenomSuccess() {
 	metadataBz, err := json.Marshal(metadata)
 	s.Require().NoError(err)
 
+	// Transfer native denom from neutron to provider
+	s.IBCTransfer(
+		s.neutronTransferPath,
+		s.neutronTransferPath.EndpointA,
+		s.neutronAddr,
+		s.providerAddr,
+		nativeDenom,
+		ibcTransferAmount,
+		"",
+	)
+	transferDenomPath := transfertypes.GetPrefixedDenom(
+		transfertypes.PortID,
+		s.neutronTransferPath.EndpointB.ChannelID,
+		nativeDenom,
+	)
+	transferDenomNeutronProvider := transfertypes.ParseDenomTrace(transferDenomPath).IBCDenom()
+	s.assertProviderBalance(s.providerAddr, transferDenomNeutronProvider, ibcTransferAmount)
+
 	// Send an IBC transfer from provider to neutron with packet memo containing the swap metadata
 	s.IBCTransferProviderToNeutron(
 		s.providerAddr,
 		s.neutronAddr,
-		nativeDenom,
+		transferDenomNeutronProvider,
 		ibcTransferAmount,
 		string(metadataBz),
 	)
@@ -347,23 +365,18 @@ func (s *IBCTestSuite) TestSwapAndForward_UnwindIBCDenomSuccess() {
 	s.Assert().NoError(err)
 	s.coordinator.CommitBlock(s.neutronChain)
 
-	// Check that the amountIn is deduced from the neutron account
-	s.assertNeutronBalance(s.neutronAddr, nativeDenom, postDepositNeutronBalNative.Sub(swapAmount))
-	// Check that the amountIn has been deducted from the neutron chain
-	s.assertNeutronBalance(s.neutronAddr, nativeDenom, postDepositNeutronBalNative.Sub(swapAmount))
 	// Check that the funds are now present on the provider chainer
 	s.assertProviderBalance(
 		s.providerAddr,
 		nativeDenom,
-		newProviderBalNative.Sub(ibcTransferAmount).Add(expectedAmountOut),
+		newProviderBalNative.Add(expectedAmountOut),
 	)
-
-	s.Assert().NoError(err)
 }
 
-// TestSwapAndForward_ForwardFails asserts that the swap and forward middleware stack works as intended in the case
-// that an incoming IBC swap succeeds but the forward fails.
-func (s *IBCTestSuite) TestSwapAndForward_ForwardFails() {
+// TestSwapAndForward_ForwardFailsRefundAddr asserts that the swap and forward middleware stack works as intended in the case
+// that an incoming IBC swap succeeds but the forward fails when a NeutronRefundAddress is provided.
+// The swap will be reverted and the transferred amount will be credited to the refundAddr
+func (s *IBCTestSuite) TestSwapAndForward_ForwardFailsRefundAddr() {
 	// Send an IBC transfer from provider chain to neutron, so we can initialize a pool with the IBC denom token + native Neutron token
 	s.IBCTransferProviderToNeutron(
 		s.providerAddr,
@@ -397,7 +410,6 @@ func (s *IBCTestSuite) TestSwapAndForward_ForwardFails() {
 
 	// Compose the IBC transfer memo metadata to be used in the swap and forward
 	swapAmount := math.NewInt(100000)
-	expectedAmountOut := math.NewInt(99990)
 	chainBAddr := s.bundleB.Chain.SenderAccount.GetAddress()
 
 	retries := uint8(0)
@@ -420,6 +432,7 @@ func (s *IBCTestSuite) TestSwapAndForward_ForwardFails() {
 	err = json.Unmarshal(bz, nextJSON)
 	s.Assert().NoError(err)
 
+	refundAddr := s.neutronChain.SenderAccounts[1].SenderAccount.GetAddress()
 	metadata := swaptypes.PacketMetadata{
 		Swap: &swaptypes.SwapMetadata{
 			MsgPlaceLimitOrder: &types.MsgPlaceLimitOrder{
@@ -431,7 +444,8 @@ func (s *IBCTestSuite) TestSwapAndForward_ForwardFails() {
 				TickIndexInToOut: 2,
 				OrderType:        types.LimitOrderType_FILL_OR_KILL,
 			},
-			Next: nextJSON,
+			NeutronRefundAddress: refundAddr.String(),
+			Next:                 nextJSON,
 		},
 	}
 
@@ -459,14 +473,13 @@ func (s *IBCTestSuite) TestSwapAndForward_ForwardFails() {
 		newProviderBalNative.Sub(ibcTransferAmount),
 	)
 
-	// Check that the amountIn is deduced from the neutron account
-	s.assertNeutronBalance(s.neutronAddr, s.providerToNeutronDenom, math.ZeroInt())
-	// Check that the amountOut stays on the neutronchain
-	s.assertNeutronBalance(
-		s.neutronAddr,
-		nativeDenom,
-		postDepositNeutronBalNative.Add(expectedAmountOut),
-	)
+	// Check that nothing remains in the overrideReceiver account
+	overrideAddr := s.ReceiverOverrideAddr(s.neutronTransferPath.EndpointA.ChannelID, s.providerAddr.String())
+	s.assertNeutronBalance(overrideAddr, s.providerToNeutronDenom, math.ZeroInt())
+	s.assertNeutronBalance(overrideAddr, nativeDenom, math.ZeroInt())
+
+	// Check that the swap was reverted and the transfer amount is in the refundAddr
+	s.assertNeutronBalance(refundAddr, s.providerToNeutronDenom, ibcTransferAmount)
 
 	// Check that nothing made it to chainB
 	transferDenomPath := transfertypes.GetPrefixedDenom(
@@ -478,3 +491,103 @@ func (s *IBCTestSuite) TestSwapAndForward_ForwardFails() {
 
 	s.assertChainBBalance(chainBAddr, transferDenomNeutronChainB, math.ZeroInt())
 }
+
+// TestSwapAndForward_ForwardFailsRefundAddr asserts that the swap and forward middleware stack works as intended in the case
+// that an incoming IBC swap succeeds but the forward fails when no NeutronRefundAddress is provided.
+// The swap will be reverted and a refund to the src chain will take place.
+func (s *IBCTestSuite) TestSwapAndForward_ForwardFails() {
+	// Send an IBC transfer from provider chain to neutron, so we can initialize a pool with the IBC denom token + native Neutron token
+	s.IBCTransferProviderToNeutron(
+		s.providerAddr,
+		s.neutronAddr,
+		nativeDenom,
+		ibcTransferAmount,
+		"",
+	)
+
+	// Assert that the funds are gone from the acc on provider and present in the acc on Neutron
+	newProviderBalNative := genesisWalletAmount.Sub(ibcTransferAmount)
+	s.assertProviderBalance(s.providerAddr, nativeDenom, newProviderBalNative)
+
+	s.assertNeutronBalance(s.neutronAddr, s.providerToNeutronDenom, ibcTransferAmount)
+
+	// deposit stake<>ibcTransferToken to initialize the pool on Neutron
+	depositAmount := math.NewInt(100_000)
+	s.neutronDeposit(
+		nativeDenom,
+		s.providerToNeutronDenom,
+		depositAmount,
+		depositAmount,
+		0,
+		1,
+		s.neutronAddr)
+
+	// Assert that the deposit was successful and the funds are moved out of the Neutron user acc
+	s.assertNeutronBalance(s.neutronAddr, s.providerToNeutronDenom, math.ZeroInt())
+	postDepositNeutronBalNative := genesisWalletAmount.Sub(depositAmount)
+	s.assertNeutronBalance(s.neutronAddr, nativeDenom, postDepositNeutronBalNative)
+
+	// Compose the IBC transfer memo metadata to be used in the swap and forward
+	swapAmount := math.NewInt(100000)
+	chainBAddr := s.bundleB.Chain.SenderAccount.GetAddress()
+
+	retries := uint8(0)
+
+	forwardMetadata := pfmtypes.PacketMetadata{
+		Forward: &pfmtypes.ForwardMetadata{
+			Receiver: chainBAddr.String(),
+			Port:     s.neutronChainBPath.EndpointA.ChannelConfig.PortID,
+			Channel:  "invalid-channel", // add an invalid channel identifier so the forward fails
+			Timeout:  pfmtypes.Duration(5 * time.Minute),
+			Retries:  &retries,
+			Next:     nil,
+		},
+	}
+
+	bz, err := json.Marshal(forwardMetadata)
+	s.Assert().NoError(err)
+
+	nextJSON := new(swaptypes.JSONObject)
+	err = json.Unmarshal(bz, nextJSON)
+	s.Assert().NoError(err)
+
+	metadata := swaptypes.PacketMetadata{
+		Swap: &swaptypes.SwapMetadata{
+			MsgPlaceLimitOrder: &types.MsgPlaceLimitOrder{
+				Creator:          s.neutronAddr.String(),
+				Receiver:         s.neutronAddr.String(),
+				TokenIn:          s.providerToNeutronDenom,
+				TokenOut:         nativeDenom,
+				AmountIn:         swapAmount,
+				TickIndexInToOut: 2,
+				OrderType:        types.LimitOrderType_FILL_OR_KILL,
+			},
+			Next: nextJSON,
+		},
+	}
+
+	metadataBz, err := json.Marshal(metadata)
+	s.Require().NoError(err)
+
+	// Send an IBC transfer from provider to neutron with packet memo containing the swap metadata
+	s.IBCTransferProviderToNeutron(
+		s.providerAddr,
+		s.neutronAddr,
+		nativeDenom,
+		ibcTransferAmount,
+		string(metadataBz),
+	)
+
+	// Relay the packets from neutron => ChainB
+	err = s.RelayAllPacketsAToB(s.neutronChainBPath)
+	// Relay Fails
+	s.Assert().Error(err)
+
+	// Check that nothing remains in the overrideReceiver account
+	overrideAddr := s.ReceiverOverrideAddr(s.neutronTransferPath.EndpointA.ChannelID, s.providerAddr.String())
+	s.assertNeutronBalance(overrideAddr, s.providerToNeutronDenom, math.ZeroInt())
+	s.assertNeutronBalance(overrideAddr, nativeDenom, math.ZeroInt())
+
+	// Check that the refund takes place and the funds are moved back to the account on Gaia
+	s.assertProviderBalance(s.providerAddr, nativeDenom, genesisWalletAmount.Sub(depositAmount))
+}

tests/ibc/swap_test.go

@@ -102,8 +102,7 @@ func (s *IBCTestSuite) TestIBCSwapMiddleware_FailRefund() {
 				TickIndexInToOut: 1,
 				OrderType:        dextypes.LimitOrderType_FILL_OR_KILL,
 			},
-			NonRefundable: false,
-			Next:          nil,
+			Next: nil,
 		},
 	}
 
@@ -127,51 +126,6 @@ func (s *IBCTestSuite) TestIBCSwapMiddleware_FailRefund() {
 	s.assertProviderBalance(s.providerAddr, nativeDenom, genesisWalletAmount)
 }
 
-// TestIBCSwapMiddleware_FailNoRefund asserts that the IBC swap middleware works as intended with Neutron running as a
-// consumer chain connected to the Cosmos Hub. The swap should fail and funds should remain on Neutron.
-func (s *IBCTestSuite) TestIBCSwapMiddleware_FailNoRefund() {
-	// Compose the swap metadata, this swap will fail because there is no pool initialized for this pair
-	swapAmount := math.NewInt(100000)
-	metadata := swaptypes.PacketMetadata{
-		Swap: &swaptypes.SwapMetadata{
-			MsgPlaceLimitOrder: &dextypes.MsgPlaceLimitOrder{
-				Creator:          s.neutronAddr.String(),
-				Receiver:         s.neutronAddr.String(),
-				TokenIn:          s.providerToNeutronDenom,
-				TokenOut:         nativeDenom,
-				AmountIn:         swapAmount,
-				TickIndexInToOut: 1,
-				OrderType:        dextypes.LimitOrderType_FILL_OR_KILL,
-			},
-			NonRefundable: true,
-			Next:          nil,
-		},
-	}
-
-	metadataBz, err := json.Marshal(metadata)
-	s.Require().NoError(err)
-
-	// Send (failing) IBC transfer with swap metadata
-	s.IBCTransferProviderToNeutron(
-		s.providerAddr,
-		s.neutronAddr,
-		nativeDenom,
-		ibcTransferAmount,
-		string(metadataBz),
-	)
-
-	// Check that the funds are present in the account on Neutron
-	s.assertNeutronBalance(s.neutronAddr, nativeDenom, genesisWalletAmount)
-	s.assertNeutronBalance(s.neutronAddr, s.providerToNeutronDenom, ibcTransferAmount)
-
-	// Check that no refund takes place and the funds are not in the account on provider
-	s.assertProviderBalance(s.providerAddr, nativeDenom, genesisWalletAmount.Sub(ibcTransferAmount))
-}
-
-// TestIBCSwapMiddleware_FailWithRefundAddr asserts that the IBC swap middleware works as intended with Neutron running as a
-// consumer chain connected to the Cosmos Hub. The swap should fail and funds should remain on Neutron but be moved
-// to the refund address.
-
 func (s *IBCTestSuite) TestIBCSwapMiddleware_FailWithRefundAddr() {
 	// Compose the swap metadata, this swap will fail because there is no pool initialized for this pair
 	refundAddr := s.neutronChain.SenderAccounts[1].SenderAccount.GetAddress()
@@ -187,9 +141,8 @@ func (s *IBCTestSuite) TestIBCSwapMiddleware_FailWithRefundAddr() {
 				TickIndexInToOut: 1,
 				OrderType:        dextypes.LimitOrderType_FILL_OR_KILL,
 			},
-			RefundAddress: refundAddr.String(),
-			NonRefundable: true,
-			Next:          nil,
+			NeutronRefundAddress: refundAddr.String(),
+			Next:                 nil,
 		},
 	}