refactor: add and use NewHop constructor (#6727)

* refactor: use NewHop constructor * more NewHop
cosmos · Jun 28, 2024 · f7bdc75 · f7bdc75
1 parent b083e37
commit f7bdc75
Show file tree

Hide file tree

Showing 12 changed files with 101 additions and 170 deletions.
diff --git a/e2e/tests/transfer/forwarding_test.go b/e2e/tests/transfer/forwarding_test.go
@@ -57,10 +57,7 @@ func (s *TransferForwardingTestSuite) TestForwarding_WithLastChainBeingICS20v1_S
 
 	t.Run("IBC transfer from A to C with forwarding through B", func(t *testing.T) {
 		inFiveMinutes := time.Now().Add(5 * time.Minute).UnixNano()
-		forwarding := transfertypes.NewForwarding(false, transfertypes.Hop{
-			PortId:    channelBtoC.PortID,
-			ChannelId: channelBtoC.ChannelID,
-		})
+		forwarding := transfertypes.NewForwarding(false, transfertypes.NewHop(channelBtoC.PortID, channelBtoC.ChannelID))
 
 		msgTransfer := testsuite.GetMsgTransfer(
 			channelAtoB.PortID,

diff --git a/modules/apps/transfer/client/cli/tx.go b/modules/apps/transfer/client/cli/tx.go
@@ -161,7 +161,7 @@ func parseForwarding(cmd *cobra.Command) (types.Forwarding, error) {
 			return types.Forwarding{}, fmt.Errorf("expected a portID/channelID pair, found %s", pair)
 		}
 
-		hop := types.Hop{PortId: pairSplit[0], ChannelId: pairSplit[1]}
+		hop := types.NewHop(pairSplit[0], pairSplit[1])
 		hops = append(hops, hop)
 	}
 

diff --git a/modules/apps/transfer/ibc_module_test.go b/modules/apps/transfer/ibc_module_test.go
@@ -303,7 +303,7 @@ func (suite *TransferTestSuite) TestOnRecvPacket() {
 					suite.chainA.SenderAccount.GetAddress().String(),
 					suite.chainB.SenderAccount.GetAddress().String(),
 					"",
-					types.NewForwardingPacketData("", types.Hop{PortId: "transfer", ChannelId: "channel-0"}),
+					types.NewForwardingPacketData("", types.NewHop("transfer", "channel-0")),
 				)
 				packet.Data = packetData.GetBytes()
 

diff --git a/modules/apps/transfer/keeper/msg_server.go b/modules/apps/transfer/keeper/msg_server.go
@@ -83,7 +83,7 @@ func (k Keeper) unwindHops(ctx sdk.Context, msg *types.MsgTransfer) (*types.MsgT
 	var unwindHops []types.Hop
 	// remove the first hop in denom as it is the current port/channel on this chain
 	for _, trace := range token.Denom.Trace[1:] {
-		unwindHops = append(unwindHops, types.Hop{PortId: trace.PortId, ChannelId: trace.ChannelId}) //nolint: gosimple
+		unwindHops = append(unwindHops, types.NewHop(trace.PortId, trace.ChannelId)) //nolint: gosimple
 	}
 
 	// Update message fields.

diff --git a/modules/apps/transfer/keeper/msg_server_test.go b/modules/apps/transfer/keeper/msg_server_test.go
@@ -268,7 +268,7 @@ func (suite *KeeperTestSuite) TestUnwindHops() {
 				suite.Require().NoError(err, "got unexpected error from unwindHops")
 				msg.SourceChannel = denom.Trace[0].PortId
 				msg.SourcePort = denom.Trace[0].ChannelId
-				msg.Forwarding = types.NewForwarding(false, types.Hop{PortId: denom.Trace[1].PortId, ChannelId: denom.Trace[1].ChannelId})
+				msg.Forwarding = types.NewForwarding(false, types.NewHop(denom.Trace[1].PortId, denom.Trace[1].ChannelId))
 				suite.Require().Equal(*msg, *modified, "expected msg and modified msg are different")
 			},
 		},
@@ -285,9 +285,9 @@ func (suite *KeeperTestSuite) TestUnwindHops() {
 				msg.SourceChannel = denom.Trace[0].PortId
 				msg.SourcePort = denom.Trace[0].ChannelId
 				msg.Forwarding = types.NewForwarding(false,
-					types.Hop{PortId: denom.Trace[3].PortId, ChannelId: denom.Trace[3].ChannelId},
-					types.Hop{PortId: denom.Trace[2].PortId, ChannelId: denom.Trace[2].ChannelId},
-					types.Hop{PortId: denom.Trace[1].PortId, ChannelId: denom.Trace[1].ChannelId},
+					types.NewHop(denom.Trace[3].PortId, denom.Trace[3].ChannelId),
+					types.NewHop(denom.Trace[2].PortId, denom.Trace[2].ChannelId),
+					types.NewHop(denom.Trace[1].PortId, denom.Trace[1].ChannelId),
 				)
 				suite.Require().Equal(*msg, *modified, "expected msg and modified msg are different")
 			},
@@ -296,15 +296,15 @@ func (suite *KeeperTestSuite) TestUnwindHops() {
 			"success - unwind hops are added to existing hops",
 			func() {
 				suite.chainA.GetSimApp().TransferKeeper.SetDenom(suite.chainA.GetContext(), denom)
-				msg.Forwarding = types.NewForwarding(true, types.Hop{PortId: ibctesting.MockPort, ChannelId: "channel-2"})
+				msg.Forwarding = types.NewForwarding(true, types.NewHop(ibctesting.MockPort, "channel-2"))
 			},
 			func(modified *types.MsgTransfer, err error) {
 				suite.Require().NoError(err, "got unexpected error from unwindHops")
 				msg.SourceChannel = denom.Trace[0].PortId
 				msg.SourcePort = denom.Trace[0].ChannelId
 				msg.Forwarding = types.NewForwarding(false,
-					types.Hop{PortId: denom.Trace[1].PortId, ChannelId: denom.Trace[1].ChannelId},
-					types.Hop{PortId: ibctesting.MockPort, ChannelId: "channel-2"},
+					types.NewHop(denom.Trace[1].PortId, denom.Trace[1].ChannelId),
+					types.NewHop(ibctesting.MockPort, "channel-2"),
 				)
 				suite.Require().Equal(*msg, *modified, "expected msg and modified msg are different")
 			},

diff --git a/modules/apps/transfer/keeper/relay_forwarding_test.go b/modules/apps/transfer/keeper/relay_forwarding_test.go
@@ -65,10 +65,10 @@ func (suite *KeeperTestSuite) TestStoredForwardedPacketAndEscrowAfterFirstHop()
 	coin := ibctesting.TestCoin
 	sender := suite.chainA.SenderAccounts[0].SenderAccount
 	receiver := suite.chainC.SenderAccounts[0].SenderAccount
-	forwarding := types.NewForwarding(false, types.Hop{
-		PortId:    pathBtoC.EndpointA.ChannelConfig.PortID,
-		ChannelId: pathBtoC.EndpointA.ChannelID,
-	})
+	forwarding := types.NewForwarding(false, types.NewHop(
+		pathBtoC.EndpointA.ChannelConfig.PortID,
+		pathBtoC.EndpointA.ChannelID,
+	))
 
 	transferMsg := types.NewMsgTransfer(
 		pathAtoB.EndpointA.ChannelConfig.PortID,
@@ -130,10 +130,10 @@ func (suite *KeeperTestSuite) TestSuccessfulForward() {
 	coinOnA := ibctesting.TestCoin
 	sender := suite.chainA.SenderAccounts[0].SenderAccount
 	receiver := suite.chainC.SenderAccounts[0].SenderAccount
-	forwarding := types.NewForwarding(false, types.Hop{
-		PortId:    pathBtoC.EndpointA.ChannelConfig.PortID,
-		ChannelId: pathBtoC.EndpointA.ChannelID,
-	})
+	forwarding := types.NewForwarding(false, types.NewHop(
+		pathBtoC.EndpointA.ChannelConfig.PortID,
+		pathBtoC.EndpointA.ChannelID,
+	))
 
 	transferMsg := types.NewMsgTransfer(
 		pathAtoB.EndpointA.ChannelConfig.PortID,
@@ -244,10 +244,10 @@ func (suite *KeeperTestSuite) TestSuccessfulForwardWithMemo() {
 	coinOnA := ibctesting.TestCoin
 	sender := suite.chainA.SenderAccounts[0].SenderAccount
 	receiver := suite.chainC.SenderAccounts[0].SenderAccount
-	forwarding := types.NewForwarding(false, types.Hop{
-		PortId:    pathBtoC.EndpointA.ChannelConfig.PortID,
-		ChannelId: pathBtoC.EndpointA.ChannelID,
-	})
+	forwarding := types.NewForwarding(false, types.NewHop(
+		pathBtoC.EndpointA.ChannelConfig.PortID,
+		pathBtoC.EndpointA.ChannelID,
+	))
 
 	transferMsg := types.NewMsgTransfer(
 		pathAtoB.EndpointA.ChannelConfig.PortID,
@@ -383,10 +383,10 @@ func (suite *KeeperTestSuite) TestSuccessfulForwardWithNonCosmosAccAddress() {
 
 	sender := suite.chainA.SenderAccounts[0].SenderAccount
 	nonCosmosReceiver := "0x42069163Ac5919fD49e6A67e6c211E0C86397fa2"
-	forwarding := types.NewForwarding(false, types.Hop{
-		PortId:    pathBtoC.EndpointA.ChannelConfig.PortID,
-		ChannelId: pathBtoC.EndpointA.ChannelID,
-	})
+	forwarding := types.NewForwarding(false, types.NewHop(
+		pathBtoC.EndpointA.ChannelConfig.PortID,
+		pathBtoC.EndpointA.ChannelID,
+	))
 
 	transferMsg := types.NewMsgTransfer(
 		pathAtoB.EndpointA.ChannelConfig.PortID,
@@ -498,10 +498,10 @@ func (suite *KeeperTestSuite) TestSuccessfulUnwind() {
 
 	originalABalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), receiver.GetAddress(), coinOnA.Denom)
 
-	forwarding := types.NewForwarding(false, types.Hop{
-		PortId:    pathAtoB.EndpointB.ChannelConfig.PortID,
-		ChannelId: pathAtoB.EndpointB.ChannelID,
-	})
+	forwarding := types.NewForwarding(false, types.NewHop(
+		pathAtoB.EndpointB.ChannelConfig.PortID,
+		pathAtoB.EndpointB.ChannelID,
+	))
 
 	transferMsg := types.NewMsgTransfer(
 		pathBtoC.EndpointB.ChannelConfig.PortID,
@@ -647,10 +647,10 @@ func (suite *KeeperTestSuite) TestAcknowledgementFailureWithMiddleChainAsNativeT
 	sender := suite.chainC.SenderAccounts[0].SenderAccount
 	receiver := suite.chainA.SenderAccounts[0].SenderAccount
 
-	forwarding := types.NewForwarding(false, types.Hop{
-		PortId:    pathAtoB.EndpointB.ChannelConfig.PortID,
-		ChannelId: pathAtoB.EndpointB.ChannelID,
-	})
+	forwarding := types.NewForwarding(false, types.NewHop(
+		pathAtoB.EndpointB.ChannelConfig.PortID,
+		pathAtoB.EndpointB.ChannelID,
+	))
 
 	forwardTransfer := types.NewMsgTransfer(
 		pathBtoC.EndpointB.ChannelConfig.PortID,
@@ -780,10 +780,10 @@ func (suite *KeeperTestSuite) TestAcknowledgementFailureWithMiddleChainAsNotBein
 	sender := suite.chainC.SenderAccounts[0].SenderAccount
 	receiver := suite.chainA.SenderAccounts[0].SenderAccount
 
-	forwarding := types.NewForwarding(false, types.Hop{
-		PortId:    pathAtoB.EndpointB.ChannelConfig.PortID,
-		ChannelId: pathAtoB.EndpointB.ChannelID,
-	})
+	forwarding := types.NewForwarding(false, types.NewHop(
+		pathAtoB.EndpointB.ChannelConfig.PortID,
+		pathAtoB.EndpointB.ChannelID,
+	))
 
 	forwardTransfer := types.NewMsgTransfer(
 		pathBtoC.EndpointB.ChannelConfig.PortID,
@@ -900,12 +900,7 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacketForwarding() {
 	originalABalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), sender.GetAddress(), coin.Denom)
 
 	forwarding := types.Forwarding{
-		Hops: []types.Hop{
-			{
-				PortId:    pathBtoC.EndpointA.ChannelConfig.PortID,
-				ChannelId: pathBtoC.EndpointA.ChannelID,
-			},
-		},
+		Hops: []types.Hop{types.NewHop(pathBtoC.EndpointA.ChannelConfig.PortID, pathBtoC.EndpointA.ChannelID)},
 	}
 
 	transferMsg := types.NewMsgTransfer(
@@ -1049,8 +1044,8 @@ func (suite *KeeperTestSuite) TestForwardingWithMoreThanOneHop() {
 	receiver := suite.chainD.SenderAccounts[0].SenderAccount
 
 	forwarding := types.NewForwarding(false,
-		types.Hop{PortId: pathBtoC.EndpointA.ChannelConfig.PortID, ChannelId: pathBtoC.EndpointA.ChannelID},
-		types.Hop{PortId: pathCtoD.EndpointA.ChannelConfig.PortID, ChannelId: pathCtoD.EndpointA.ChannelID},
+		types.NewHop(pathBtoC.EndpointA.ChannelConfig.PortID, pathBtoC.EndpointA.ChannelID),
+		types.NewHop(pathCtoD.EndpointA.ChannelConfig.PortID, pathCtoD.EndpointA.ChannelID),
 	)
 
 	transferMsg := types.NewMsgTransfer(

diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go
@@ -97,10 +97,10 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
 			"successful transfer with non-empty forwarding hops and ics20-2",
 			func() {
 				expEscrowAmount = sdkmath.NewInt(100)
-				forwarding = types.NewForwarding(false, types.Hop{
-					PortId:    path.EndpointA.ChannelConfig.PortID,
-					ChannelId: path.EndpointA.ChannelID,
-				})
+				forwarding = types.NewForwarding(false, types.NewHop(
+					path.EndpointA.ChannelConfig.PortID,
+					path.EndpointA.ChannelID,
+				))
 			},
 			nil,
 		},
@@ -178,10 +178,10 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
 					channel.Version = types.V1
 				})
 
-				forwarding = types.NewForwarding(false, types.Hop{
-					PortId:    path.EndpointA.ChannelConfig.PortID,
-					ChannelId: path.EndpointA.ChannelID,
-				})
+				forwarding = types.NewForwarding(false, types.NewHop(
+					path.EndpointA.ChannelConfig.PortID,
+					path.EndpointA.ChannelID,
+				))
 			},
 			ibcerrors.ErrInvalidRequest,
 		},

diff --git a/modules/apps/transfer/types/forwarding.go b/modules/apps/transfer/types/forwarding.go
@@ -50,6 +50,11 @@ func (fpd ForwardingPacketData) Validate() error {
 	return nil
 }
 
+// NewHop creates a Hop with the given port ID and channel ID.
+func NewHop(portID, channelID string) Hop {
+	return Hop{portID, channelID}
+}
+
 // Validate performs a basic validation of the Hop fields.
 func (h Hop) Validate() error {
 	if err := host.PortIdentifierValidator(h.PortId); err != nil {