Merge branch 'main' into cian/enable-per-suite-tests-for-other-tests

CHANGELOG.md

@@ -62,6 +62,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (23-commmitment) [\#6644](https://github.com/cosmos/ibc-go/pull/6644) Introduce commitment/v2 `MerklePath` to include `repeated bytes` in favour of `repeated string`. This supports using merkle path keys which include non UTF-8 encoded runes.
 * (23-commmitment) [\#6633](https://github.com/cosmos/ibc-go/pull/6633) MerklePath has been changed to use `repeated bytes` in favour of `repeated strings`.
 * (apps/27-interchain-accounts) [\#6749](https://github.com/cosmos/ibc-go/pull/6749) The ICA controller `NewIBCMiddleware` constructor function sets by default the auth module to nil.
+* (core/02-client) [\#6777](https://github.com/cosmos/ibc-go/pull/6777) The `NewClientProposalHandler` of `02-client` has been removed.
 
 ### State Machine Breaking
 

README.md

@@ -23,7 +23,7 @@
 </div>
 <div align="center">
   <a href="https://github.com/cosmos/ibc-go">
-    <img alt="Lines Of Code" src="https://tokei.rs/b1/github/cosmos/ibc-go" />
+    <img alt="Lines Of Code" src="https://sonarcloud.io/api/project_badges/measure?project=cosmos_ibc-go&metric=ncloc" />
   </a>
   <a href="https://discord.com/invite/interchain">
     <img alt="Discord" src="https://img.shields.io/discord/669268347736686612.svg" />

docs/docs/05-migrations/13-v8-to-v9.md

@@ -20,7 +20,15 @@ There are four sections based on the four potential user groups of this document
 
 ## Chains
 
-- No relevant changes were made in this release.
+Chains will need to remove the route for the legacy proposal handler for `02-client` from their `app/app.go`:
+
+```diff
+// app.go
+govRouter.AddRoute(govtypes.RouterKey, govv1beta1.ProposalHandler).
+-       AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper)).
+-       AddRoute(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper))
++       AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper))
+```
 
 ## IBC Apps
 
@@ -114,6 +122,7 @@ The base application is then set by default to nil and thus authentication is as
 - `UnmarshalPacketData` now takes in the context, portID, and channelID. This allows the packet data to be unmarshaled based on the channel version.
 - `Router` reference has been removed from IBC core keeper: [#6138](https://github.com/cosmos/ibc-go/pull/6138). Please use `PortKeeper.Router` instead.
 - The function `CreateLocalhostClient` has been removed. The localhost client is now stateless. 
+- The function `NewClientProposalHandler` has been removed. [#6777](https://github.com/cosmos/ibc-go/pull/6777).
 
 ### 02-client
 

e2e/tests/core/02-client/client_test.go

@@ -160,86 +160,6 @@ func (s *ClientTestSuite) TestScheduleIBCUpgrade_Succeeds() {
 	})
 }
 
-func (s *ClientTestSuite) TestClientUpdateProposal_Succeeds() {
-	t := s.T()
-	ctx := context.TODO()
-
-	var (
-		pathName           string
-		subjectClientID    string
-		substituteClientID string
-		// set the trusting period to a value which will still be valid upon client creation, but invalid before the first update
-		badTrustingPeriod = time.Second * 10
-	)
-
-	testName := t.Name()
-	relayer := s.CreateDefaultPaths(testName)
-
-	t.Run("create substitute client with correct trusting period", func(t *testing.T) {
-		// TODO: update when client identifier created is accessible
-		// currently assumes first client is 07-tendermint-0
-		substituteClientID = clienttypes.FormatClientIdentifier(ibcexported.Tendermint, 0)
-
-		pathName = s.GetPaths(testName)[0]
-	})
-
-	chainA, chainB := s.GetChains()
-	chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)
-
-	t.Run("create subject client with bad trusting period", func(t *testing.T) {
-		createClientOptions := ibc.CreateClientOptions{
-			TrustingPeriod: badTrustingPeriod.String(),
-		}
-
-		s.SetupClients(ctx, relayer, createClientOptions)
-
-		// TODO: update when client identifier created is accessible
-		// currently assumes second client is 07-tendermint-1
-		subjectClientID = clienttypes.FormatClientIdentifier(ibcexported.Tendermint, 1)
-	})
-
-	time.Sleep(badTrustingPeriod)
-
-	t.Run("update substitute client", func(t *testing.T) {
-		s.UpdateClients(ctx, relayer, pathName)
-	})
-
-	s.Require().NoError(test.WaitForBlocks(ctx, 1, chainA, chainB), "failed to wait for blocks")
-
-	t.Run("check status of each client", func(t *testing.T) {
-		t.Run("substitute should be active", func(t *testing.T) {
-			status, err := query.ClientStatus(ctx, chainA, substituteClientID)
-			s.Require().NoError(err)
-			s.Require().Equal(ibcexported.Active.String(), status)
-		})
-
-		t.Run("subject should be expired", func(t *testing.T) {
-			status, err := query.ClientStatus(ctx, chainA, subjectClientID)
-			s.Require().NoError(err)
-			s.Require().Equal(ibcexported.Expired.String(), status)
-		})
-	})
-
-	t.Run("pass client update proposal", func(t *testing.T) {
-		proposal := clienttypes.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subjectClientID, substituteClientID)
-		s.ExecuteAndPassGovV1Beta1Proposal(ctx, chainA, chainAWallet, proposal)
-	})
-
-	t.Run("check status of each client", func(t *testing.T) {
-		t.Run("substitute should be active", func(t *testing.T) {
-			status, err := query.ClientStatus(ctx, chainA, substituteClientID)
-			s.Require().NoError(err)
-			s.Require().Equal(ibcexported.Active.String(), status)
-		})
-
-		t.Run("subject should be active", func(t *testing.T) {
-			status, err := query.ClientStatus(ctx, chainA, subjectClientID)
-			s.Require().NoError(err)
-			s.Require().Equal(ibcexported.Active.String(), status)
-		})
-	})
-}
-
 // TestRecoverClient_Succeeds tests that a governance proposal to recover a client using a MsgRecoverClient is successful.
 func (s *ClientTestSuite) TestRecoverClient_Succeeds() {
 	t := s.T()

e2e/tests/transfer/forwarding_test.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/strangelove-ventures/interchaintest/v8/ibc"
+	test "github.com/strangelove-ventures/interchaintest/v8/testutil"
 	testifysuite "github.com/stretchr/testify/suite"
 
 	"github.com/cosmos/ibc-go/e2e/testsuite"
@@ -117,3 +118,105 @@ func (s *TransferForwardingTestSuite) testForwardingThreeChains(lastChainVersion
 		s.Require().Equal(expected, actualBalance.Int64())
 	})
 }
+
+// TestForwardingWithUnwindSucceeds tests the forwarding scenario in which
+// a packet is sent from A to B, then unwound back to A and forwarded to C
+// The overall flow of the packet is:
+// A ---> B
+// B --(unwind)-->A --(forward)-->B --(forward)--> C
+func (s *TransferForwardingTestSuite) TestForwardingWithUnwindSucceeds() {
+	t := s.T()
+	ctx := context.TODO()
+	t.Parallel()
+	testName := t.Name()
+	relayer := s.CreateDefaultPaths(testName)
+
+	chains := s.GetAllChains()
+
+	chainA, chainB, chainC := chains[0], chains[1], chains[2]
+
+	channelAtoB := s.GetChainAChannelForTest(testName)
+	channelBtoC := s.GetChannelsForTest(chainB, testName)[1]
+
+	chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)
+	chainAAddress := chainAWallet.FormattedAddress()
+	chainADenom := chainA.Config().Denom
+
+	chainBWallet := s.CreateUserOnChainB(ctx, testvalues.StartingTokenAmount)
+	chainBAddress := chainBWallet.FormattedAddress()
+
+	chainCWallet := s.CreateUserOnChainC(ctx, testvalues.StartingTokenAmount)
+	chainCAddress := chainCWallet.FormattedAddress()
+
+	t.Run("IBC transfer from A to B", func(t *testing.T) {
+		inFiveMinutes := time.Now().Add(5 * time.Minute).UnixNano()
+
+		msgTransfer := testsuite.GetMsgTransfer(
+			channelAtoB.PortID,
+			channelAtoB.ChannelID,
+			transfertypes.V2,
+			testvalues.DefaultTransferCoins(chainADenom),
+			chainAAddress,
+			chainBAddress,
+			clienttypes.ZeroHeight(),
+			uint64(inFiveMinutes),
+			"",
+			nil)
+		resp := s.BroadcastMessages(ctx, chainA, chainAWallet, msgTransfer)
+		s.AssertTxSuccess(resp)
+	})
+
+	t.Run("start relayer", func(t *testing.T) {
+		s.StartRelayer(relayer, testName)
+	})
+
+	chainBDenom := transfertypes.NewDenom(chainADenom, transfertypes.NewHop(channelAtoB.Counterparty.PortID, channelAtoB.Counterparty.ChannelID))
+	t.Run("packet has reached B", func(t *testing.T) {
+		s.AssertPacketRelayed(ctx, chainA, channelAtoB.PortID, channelAtoB.ChannelID, 1)
+
+		balance, err := query.Balance(ctx, chainB, chainBAddress, chainBDenom.IBCDenom())
+		s.Require().NoError(err)
+
+		s.Require().Equal(testvalues.IBCTransferAmount, balance.Int64())
+	})
+
+	t.Run("IBC transfer from B (unwind) to A and forwarded to C through B", func(t *testing.T) {
+		inFiveMinutes := time.Now().Add(5 * time.Minute).UnixNano()
+
+		forwarding := transfertypes.NewForwarding(
+			true,
+			transfertypes.NewHop(channelAtoB.PortID, channelAtoB.ChannelID),
+			transfertypes.NewHop(channelBtoC.PortID, channelBtoC.ChannelID),
+		)
+		msgTransfer := testsuite.GetMsgTransfer(
+			"",
+			"",
+			transfertypes.V2,
+			testvalues.DefaultTransferCoins(chainBDenom.IBCDenom()),
+			chainBAddress,
+			chainCAddress,
+			clienttypes.ZeroHeight(),
+			uint64(inFiveMinutes),
+			"",
+			forwarding)
+		resp := s.BroadcastMessages(ctx, chainB, chainBWallet, msgTransfer)
+		s.AssertTxSuccess(resp)
+	})
+
+	t.Run("packet has reached C", func(t *testing.T) {
+		chainCDenom := transfertypes.NewDenom(chainADenom,
+			transfertypes.NewHop(channelAtoB.Counterparty.PortID, channelAtoB.Counterparty.ChannelID),
+			transfertypes.NewHop(channelBtoC.Counterparty.PortID, channelBtoC.Counterparty.ChannelID),
+		)
+
+		err := test.WaitForCondition(time.Minute*10, time.Second*30, func() (bool, error) {
+			balance, err := query.Balance(ctx, chainC, chainCAddress, chainCDenom.IBCDenom())
+			if err != nil {
+				return false, err
+			}
+			return balance.Int64() == testvalues.IBCTransferAmount, nil
+		})
+		s.Require().NoError(err)
+		s.AssertPacketRelayed(ctx, chainB, channelBtoC.PortID, channelBtoC.ChannelID, 1)
+	})
+}

modules/apps/transfer/keeper/export_test.go

@@ -34,12 +34,12 @@ func (k Keeper) TokenFromCoin(ctx sdk.Context, coin sdk.Coin) (types.Token, erro
 	return k.tokenFromCoin(ctx, coin)
 }
 
-// UnwindHops is a wrapper around unwindToken for testing purposes.
+// UnwindHops is a wrapper around unwindHops for testing purposes.
 func (k Keeper) UnwindHops(ctx sdk.Context, msg *types.MsgTransfer) (*types.MsgTransfer, error) {
 	return k.unwindHops(ctx, msg)
 }
 
-// UnwindHops is a wrapper around unwindToken for testing purposes.
+// GetForwardedPacket is a wrapper around getForwardedPacket for testing purposes.
 func (k Keeper) GetForwardedPacket(ctx sdk.Context, portID, channelID string, sequence uint64) (channeltypes.Packet, bool) {
 	return k.getForwardedPacket(ctx, portID, channelID, sequence)
 }

modules/apps/transfer/keeper/forwarding.go

@@ -62,7 +62,7 @@ func (k Keeper) acknowledgeForwardedPacket(ctx sdk.Context, packet, forwardedPac
 // revertForwardedPacket reverts the logic of receive packet that occurs in the middle chains during a packet forwarding.
 // If the packet fails to be forwarded all the way to the final destination, the state changes on this chain must be reverted
 // before sending back the error acknowledgement to ensure atomic packet forwarding.
-func (k Keeper) revertForwardedPacket(ctx sdk.Context, prevPacket channeltypes.Packet, failedPacketData types.FungibleTokenPacketDataV2) error {
+func (k Keeper) revertForwardedPacket(ctx sdk.Context, forwardedPacket channeltypes.Packet, failedPacketData types.FungibleTokenPacketDataV2) error {
 	/*
 		Recall that RecvPacket handles an incoming packet depending on the denom of the received funds:
 			1. If the funds are native, then the amount is sent to the receiver from the escrow.
@@ -73,9 +73,9 @@ func (k Keeper) revertForwardedPacket(ctx sdk.Context, prevPacket channeltypes.P
 	*/
 
 	forwardingAddr := k.authKeeper.GetModuleAddress(types.ModuleName)
-	escrow := types.GetEscrowAddress(prevPacket.DestinationPort, prevPacket.DestinationChannel)
+	escrow := types.GetEscrowAddress(forwardedPacket.DestinationPort, forwardedPacket.DestinationChannel)
 
-	// we can iterate over the received tokens of prevPacket by iterating over the sent tokens of failedPacketData
+	// we can iterate over the received tokens of forwardedPacket by iterating over the sent tokens of failedPacketData
 	for _, token := range failedPacketData.Tokens {
 		// parse the transfer amount
 		coin, err := token.ToCoin()
@@ -85,8 +85,8 @@ func (k Keeper) revertForwardedPacket(ctx sdk.Context, prevPacket channeltypes.P
 
 		// check if the token we received originated on the sender
 		// given that the packet is being reversed, we check the DestinationChannel and DestinationPort
-		// of the prevPacket to see if a hop was added to the trace during the receive step
-		if token.Denom.HasPrefix(prevPacket.DestinationPort, prevPacket.DestinationChannel) {
+		// of the forwardedPacket to see if a hop was added to the trace during the receive step
+		if token.Denom.HasPrefix(forwardedPacket.DestinationPort, forwardedPacket.DestinationChannel) {
 			if err := k.bankKeeper.BurnCoins(ctx, types.ModuleName, sdk.NewCoins(coin)); err != nil {
 				return err
 			}

modules/apps/transfer/keeper/relay.go

@@ -272,14 +272,14 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t
 // acknowledgement was a success then nothing occurs. If the acknowledgement failed,
 // then the sender is refunded their tokens using the refundPacketToken function.
 func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketDataV2, ack channeltypes.Acknowledgement) error {
-	prevPacket, isForwarded := k.getForwardedPacket(ctx, packet.SourcePort, packet.SourceChannel, packet.Sequence)
+	forwardedPacket, isForwarded := k.getForwardedPacket(ctx, packet.SourcePort, packet.SourceChannel, packet.Sequence)
 
 	switch ack.Response.(type) {
 	case *channeltypes.Acknowledgement_Result:
 		if isForwarded {
-			// Write a successful async ack for the prevPacket
+			// Write a successful async ack for the forwardedPacket
 			forwardAck := channeltypes.NewResultAcknowledgement([]byte{byte(1)})
-			return k.acknowledgeForwardedPacket(ctx, prevPacket, packet, forwardAck)
+			return k.acknowledgeForwardedPacket(ctx, forwardedPacket, packet, forwardAck)
 		}
 
 		// the acknowledgement succeeded on the receiving chain so nothing
@@ -294,12 +294,12 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Pac
 			// the forwarded packet has failed, thus the funds have been refunded to the intermediate address.
 			// we must revert the changes that came from successfully receiving the tokens on our chain
 			// before propagating the error acknowledgement back to original sender chain
-			if err := k.revertForwardedPacket(ctx, prevPacket, data); err != nil {
+			if err := k.revertForwardedPacket(ctx, forwardedPacket, data); err != nil {
 				return err
 			}
 
 			forwardAck := internaltypes.NewForwardErrorAcknowledgement(packet, ack)
-			return k.acknowledgeForwardedPacket(ctx, prevPacket, packet, forwardAck)
+			return k.acknowledgeForwardedPacket(ctx, forwardedPacket, packet, forwardAck)
 		}
 
 		return nil
@@ -316,14 +316,14 @@ func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, dat
 		return err
 	}
 
-	prevPacket, isForwarded := k.getForwardedPacket(ctx, packet.SourcePort, packet.SourceChannel, packet.Sequence)
+	forwardedPacket, isForwarded := k.getForwardedPacket(ctx, packet.SourcePort, packet.SourceChannel, packet.Sequence)
 	if isForwarded {
-		if err := k.revertForwardedPacket(ctx, prevPacket, data); err != nil {
+		if err := k.revertForwardedPacket(ctx, forwardedPacket, data); err != nil {
 			return err
 		}
 
 		forwardAck := internaltypes.NewForwardTimeoutAcknowledgement(packet)
-		return k.acknowledgeForwardedPacket(ctx, prevPacket, packet, forwardAck)
+		return k.acknowledgeForwardedPacket(ctx, forwardedPacket, packet, forwardAck)
 	}
 
 	return nil