Merge branch 'main' into jim/5666-remove-legacy-gov-handlers

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/keeper_test.go

@@ -30,15 +30,13 @@ type KeeperTestSuite struct {
 	chainA *ibctesting.TestChain
 	chainB *ibctesting.TestChain
 	chainC *ibctesting.TestChain
-	chainD *ibctesting.TestChain
 }
 
 func (suite *KeeperTestSuite) SetupTest() {
-	suite.coordinator = ibctesting.NewCoordinator(suite.T(), 4)
+	suite.coordinator = ibctesting.NewCoordinator(suite.T(), 3)
 	suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1))
 	suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2))
 	suite.chainC = suite.coordinator.GetChain(ibctesting.GetChainID(3))
-	suite.chainD = suite.coordinator.GetChain(ibctesting.GetChainID(4))
 
 	queryHelper := baseapp.NewQueryServerTestHelper(suite.chainA.GetContext(), suite.chainA.GetSimApp().InterfaceRegistry())
 	types.RegisterQueryServer(queryHelper, suite.chainA.GetSimApp().TransferKeeper)

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

modules/apps/transfer/keeper/relay_forwarding_test.go

@@ -2,9 +2,11 @@ package keeper_test
 
 import (
 	"fmt"
+	"testing"
 	"time"
 
 	"github.com/cosmos/gogoproto/proto"
+	testifysuite "github.com/stretchr/testify/suite"
 
 	sdkmath "cosmossdk.io/math"
 
@@ -19,39 +21,52 @@ import (
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
 )
 
-func (suite *KeeperTestSuite) setupForwardingPaths() (pathAtoB, pathBtoC *ibctesting.Path) {
-	pathAtoB = ibctesting.NewTransferPath(suite.chainA, suite.chainB)
-	pathBtoC = ibctesting.NewTransferPath(suite.chainB, suite.chainC)
-	pathAtoB.Setup()
-	pathBtoC.Setup()
-	return pathAtoB, pathBtoC
-}
-
-type amountType int
-
 const (
 	escrow amountType = iota
 	balance
 )
 
-func (suite *KeeperTestSuite) assertAmountOnChain(chain *ibctesting.TestChain, balanceType amountType, amount sdkmath.Int, denom string) {
-	var total sdk.Coin
-	switch balanceType {
-	case escrow:
-		total = chain.GetSimApp().TransferKeeper.GetTotalEscrowForDenom(chain.GetContext(), denom)
-	case balance:
-		total = chain.GetSimApp().BankKeeper.GetBalance(chain.GetContext(), chain.SenderAccounts[0].SenderAccount.GetAddress(), denom)
-	default:
-		suite.Fail("invalid amountType %s", balanceType)
-	}
-	suite.Require().Equal(amount, total.Amount, fmt.Sprintf("Chain %s: got balance of %d, wanted %d", chain.Name(), total.Amount, amount))
+type ForwardingTestSuite struct {
+	testifysuite.Suite
+
+	coordinator *ibctesting.Coordinator
+
+	// testing chains used for convenience and readability
+	chainA *ibctesting.TestChain
+	chainB *ibctesting.TestChain
+	chainC *ibctesting.TestChain
+	chainD *ibctesting.TestChain
+}
+
+type amountType int
+
+func TestForwardingTestSuite(t *testing.T) {
+	testifysuite.Run(t, new(ForwardingTestSuite))
+}
+
+func (suite *ForwardingTestSuite) SetupTest() {
+	suite.coordinator = ibctesting.NewCoordinator(suite.T(), 4)
+
+	suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1))
+	suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2))
+	suite.chainC = suite.coordinator.GetChain(ibctesting.GetChainID(3))
+	suite.chainD = suite.coordinator.GetChain(ibctesting.GetChainID(4))
+}
+
+func (suite *ForwardingTestSuite) setupForwardingPaths() (pathAtoB, pathBtoC *ibctesting.Path) {
+	pathAtoB = ibctesting.NewTransferPath(suite.chainA, suite.chainB)
+	pathBtoC = ibctesting.NewTransferPath(suite.chainB, suite.chainC)
+	pathAtoB.Setup()
+	pathBtoC.Setup()
+
+	return pathAtoB, pathBtoC
 }
 
 // TestStoredForwardedPacketAndEscrowAfterFirstHop tests that the forwarded packet
 // from chain A to chain B is stored after when the packet is received on chain B
 // and then forwarded to chain C, and checks the balance of the escrow accounts
 // in chain A nad B.
-func (suite *KeeperTestSuite) TestStoredForwardedPacketAndEscrowAfterFirstHop() {
+func (suite *ForwardingTestSuite) TestStoredForwardedPacketAndEscrowAfterFirstHop() {
 	/*
 		Given the following topology:
 		chain A (channel 0) -> (channel-0) chain B (channel-1) -> (channel-0) chain A
@@ -112,7 +127,7 @@ func (suite *KeeperTestSuite) TestStoredForwardedPacketAndEscrowAfterFirstHop()
 }
 
 // TestSuccessfulForward tests a successful transfer from A to C through B.
-func (suite *KeeperTestSuite) TestSuccessfulForward() {
+func (suite *ForwardingTestSuite) TestSuccessfulForward() {
 	/*
 		Given the following topology:
 		chain A (channel 0) -> (channel-0) chain B (channel-1) -> (channel-0) chain C
@@ -225,7 +240,7 @@ func (suite *KeeperTestSuite) TestSuccessfulForward() {
 }
 
 // TestSuccessfulForwardWithMemo tests a successful transfer from A to C through B with a memo that should arrive at C.
-func (suite *KeeperTestSuite) TestSuccessfulForwardWithMemo() {
+func (suite *ForwardingTestSuite) TestSuccessfulForwardWithMemo() {
 	/*
 		Given the following topology:
 		chain A (channel 0) -> (channel-0) chain B (channel-1) -> (channel-0) chain C
@@ -366,7 +381,7 @@ func (suite *KeeperTestSuite) TestSuccessfulForwardWithMemo() {
 
 // TestSuccessfulForwardWithNonCosmosAccAddress tests that a packet is successfully forwarded with a non-Cosmos account address.
 // The test stops before verifying the final receive, because we don't have a non-cosmos chain to test with.
-func (suite *KeeperTestSuite) TestSuccessfulForwardWithNonCosmosAccAddress() {
+func (suite *ForwardingTestSuite) TestSuccessfulForwardWithNonCosmosAccAddress() {
 	/*
 		Given the following topology:
 		chain A (channel 0) -> (channel-0) chain B (channel-1) -> (channel-0) chain C
@@ -445,7 +460,7 @@ func (suite *KeeperTestSuite) TestSuccessfulForwardWithNonCosmosAccAddress() {
 
 // TestSuccessfulUnwind tests unwinding of tokens sent from A -> B -> C by
 // forwarding the tokens back from C -> B -> A.
-func (suite *KeeperTestSuite) TestSuccessfulUnwind() {
+func (suite *ForwardingTestSuite) TestSuccessfulUnwind() {
 	/*
 		Given the following topolgy:
 		chain A (channel 0) -> (channel-0) chain B (channel-1) -> (channel-0) chain C
@@ -590,7 +605,7 @@ func (suite *KeeperTestSuite) TestSuccessfulUnwind() {
 // middle chain is native source when receiving and sending the packet. In other words, the middle chain's native
 // token has been sent to chain C, and the multi-hop transfer from C -> B -> A has chain B being the source of
 // the token both when receiving and forwarding (sending).
-func (suite *KeeperTestSuite) TestAcknowledgementFailureWithMiddleChainAsNativeTokenSource() {
+func (suite *ForwardingTestSuite) TestAcknowledgementFailureWithMiddleChainAsNativeTokenSource() {
 	/*
 		Given the following topology:
 		chain A (channel 0) -> (channel-0) chain B (channel-1) -> (channel-0) chain C
@@ -758,7 +773,7 @@ func (suite *KeeperTestSuite) TestAcknowledgementFailureWithMiddleChainAsNativeT
 // TestAcknowledgementFailureWithMiddleChainAsNotBeingTokenSource tests a failure in the last hop where the middle chain
 // is not source of the token when receiving or sending the packet. In other words, the middle chain's is sent
 // (and forwarding) someone else's native token (in this case chain C).
-func (suite *KeeperTestSuite) TestAcknowledgementFailureWithMiddleChainAsNotBeingTokenSource() {
+func (suite *ForwardingTestSuite) TestAcknowledgementFailureWithMiddleChainAsNotBeingTokenSource() {
 	/*
 		Given the following topology:
 		chain A (channel 0) 												<- (channel-0) chain B (channel-1) <- (channel-0) chain C
@@ -878,7 +893,7 @@ func (suite *KeeperTestSuite) TestAcknowledgementFailureWithMiddleChainAsNotBein
 // TestOnTimeoutPacketForwarding tests the scenario in which a packet goes from
 // A to C, using B as a forwarding hop. The packet times out when going to C
 // from B and we verify that funds are properly returned to A.
-func (suite *KeeperTestSuite) TestOnTimeoutPacketForwarding() {
+func (suite *ForwardingTestSuite) TestOnTimeoutPacketForwarding() {
 	pathAtoB, pathBtoC := suite.setupForwardingPaths()
 
 	amount := sdkmath.NewInt(100)
@@ -1018,7 +1033,7 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacketForwarding() {
 
 // TestForwardingWithMoreThanOneHop tests the scenario in which we
 // forward with more than one forwarding hop.
-func (suite *KeeperTestSuite) TestForwardingWithMoreThanOneHop() {
+func (suite *ForwardingTestSuite) TestForwardingWithMoreThanOneHop() {
 	// Setup A->B->C->D
 	coinOnA := ibctesting.TestCoin
 
@@ -1131,7 +1146,7 @@ func (suite *KeeperTestSuite) TestForwardingWithMoreThanOneHop() {
 	suite.Require().NoError(err)
 }
 
-func (suite *KeeperTestSuite) TestMultihopForwardingErrorAcknowledgement() {
+func (suite *ForwardingTestSuite) TestMultihopForwardingErrorAcknowledgement() {
 	// Setup A->B->C->D
 	coinOnA := ibctesting.TestCoin
 
@@ -1267,3 +1282,16 @@ func parseAckFromTransferEvents(events []abci.Event) (string, error) {
 
 	return "", fmt.Errorf("acknowledgement event attribute not found")
 }
+
+func (suite *ForwardingTestSuite) assertAmountOnChain(chain *ibctesting.TestChain, balanceType amountType, amount sdkmath.Int, denom string) {
+	var total sdk.Coin
+	switch balanceType {
+	case escrow:
+		total = chain.GetSimApp().TransferKeeper.GetTotalEscrowForDenom(chain.GetContext(), denom)
+	case balance:
+		total = chain.GetSimApp().BankKeeper.GetBalance(chain.GetContext(), chain.SenderAccounts[0].SenderAccount.GetAddress(), denom)
+	default:
+		suite.Fail("invalid amountType %s", balanceType)
+	}
+	suite.Require().Equal(amount, total.Amount, fmt.Sprintf("Chain %s: got balance of %d, wanted %d", chain.Name(), total.Amount, amount))
+}