refactor: use typed Trace (#6432)

* refactor: initial introduction of typed trace * fix: build * fix: types test * fix: callbacks test * Update modules/apps/transfer/ibc_module_test.go Co-authored-by: DimitrisJim <[email protected]> * review: apply suggestion * lint * tiny nit * fix: tests from merging main --------- Co-authored-by: DimitrisJim <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]>
cosmos · May 30, 2024 · 0beaba6 · 0beaba6
1 parent 80b0c9a
commit 0beaba6
Show file tree

Hide file tree

Showing 21 changed files with 289 additions and 326 deletions.
diff --git a/modules/apps/callbacks/ibc_middleware_test.go b/modules/apps/callbacks/ibc_middleware_test.go
@@ -169,7 +169,7 @@ func (s *CallbacksTestSuite) TestSendPacket() {
 					{
 						Denom: transfertypes.Denom{
 							Base:  ibctesting.TestCoin.GetDenom(),
-							Trace: []string{},
+							Trace: []transfertypes.Trace{},
 						},
 						Amount: ibctesting.TestCoin.Amount.String(),
 					},
@@ -313,7 +313,7 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() {
 					{
 						Denom: transfertypes.Denom{
 							Base:  ibctesting.TestCoin.GetDenom(),
-							Trace: []string{},
+							Trace: []transfertypes.Trace{},
 						},
 						Amount: ibctesting.TestCoin.Amount.String(),
 					},
@@ -648,7 +648,7 @@ func (s *CallbacksTestSuite) TestOnRecvPacket() {
 					{
 						Denom: transfertypes.Denom{
 							Base:  ibctesting.TestCoin.GetDenom(),
-							Trace: []string{},
+							Trace: []transfertypes.Trace{},
 						},
 						Amount: ibctesting.TestCoin.Amount.String(),
 					},
@@ -782,7 +782,7 @@ func (s *CallbacksTestSuite) TestWriteAcknowledgement() {
 					{
 						Denom: transfertypes.Denom{
 							Base:  ibctesting.TestCoin.GetDenom(),
-							Trace: []string{},
+							Trace: []transfertypes.Trace{},
 						},
 						Amount: ibctesting.TestCoin.Amount.String(),
 					},

diff --git a/modules/apps/transfer/ibc_module_test.go b/modules/apps/transfer/ibc_module_test.go
@@ -556,8 +556,10 @@ func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() {
 					Tokens: []types.Token{
 						{
 							Denom: types.Denom{
-								Base:  "atom",
-								Trace: []string{"transfer/channel-0"},
+								Base: "atom",
+								Trace: []types.Trace{
+									types.NewTrace("transfer", "channel-0"),
+								},
 							},
 							Amount: ibctesting.TestCoin.Amount.String(),
 						},
@@ -603,7 +605,7 @@ func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() {
 						{
 							Denom: types.Denom{
 								Base:  ibctesting.TestCoin.Denom,
-								Trace: []string{""},
+								Trace: []types.Trace{{}},
 							},
 							Amount: ibctesting.TestCoin.Amount.String(),
 						},
@@ -615,7 +617,7 @@ func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() {
 
 				data = initialPacketData.(types.FungibleTokenPacketDataV2).GetBytes()
 			},
-			errors.New("trace info must come in pairs of port and channel identifiers"),
+			errors.New("invalid token denom: invalid trace: invalid portID: identifier cannot be blank: invalid identifier"),
 		},
 		{
 			"failure: invalid packet data",

diff --git a/modules/apps/transfer/internal/convert/convert_test.go b/modules/apps/transfer/internal/convert/convert_test.go
@@ -29,8 +29,10 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) {
 				[]types.Token{
 					{
 						Denom: types.Denom{
-							Base:  "atom",
-							Trace: []string{"transfer/channel-0"},
+							Base: "atom",
+							Trace: []types.Trace{
+								types.NewTrace("transfer", "channel-0"),
+							},
 						},
 						Amount: "1000",
 					},
@@ -59,8 +61,10 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) {
 				[]types.Token{
 					{
 						Denom: types.Denom{
-							Base:  "atom/withslash",
-							Trace: []string{"transfer/channel-0"},
+							Base: "atom/withslash",
+							Trace: []types.Trace{
+								types.NewTrace("transfer", "channel-0"),
+							},
 						},
 						Amount: "1000",
 					},
@@ -74,8 +78,10 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) {
 				[]types.Token{
 					{
 						Denom: types.Denom{
-							Base:  "atom/",
-							Trace: []string{"transfer/channel-0"},
+							Base: "atom/",
+							Trace: []types.Trace{
+								types.NewTrace("transfer", "channel-0"),
+							},
 						},
 						Amount: "1000",
 					},
@@ -89,8 +95,11 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) {
 				[]types.Token{
 					{
 						Denom: types.Denom{
-							Base:  "atom/pool",
-							Trace: []string{"transfer/channel-0", "transfer/channel-1"},
+							Base: "atom/pool",
+							Trace: []types.Trace{
+								types.NewTrace("transfer", "channel-0"),
+								types.NewTrace("transfer", "channel-1"),
+							},
 						},
 						Amount: "1000",
 					},
@@ -104,8 +113,12 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) {
 				[]types.Token{
 					{
 						Denom: types.Denom{
-							Base:  "atom",
-							Trace: []string{"transfer/channel-0", "transfer/channel-1", "transfer-custom/channel-2"},
+							Base: "atom",
+							Trace: []types.Trace{
+								types.NewTrace("transfer", "channel-0"),
+								types.NewTrace("transfer", "channel-1"),
+								types.NewTrace("transfer-custom", "channel-2"),
+							},
 						},
 						Amount: "1000",
 					},
@@ -119,8 +132,12 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) {
 				[]types.Token{
 					{
 						Denom: types.Denom{
-							Base:  "atom/pool",
-							Trace: []string{"transfer/channel-0", "transfer/channel-1", "transfer-custom/channel-2"},
+							Base: "atom/pool",
+							Trace: []types.Trace{
+								types.NewTrace("transfer", "channel-0"),
+								types.NewTrace("transfer", "channel-1"),
+								types.NewTrace("transfer-custom", "channel-2"),
+							},
 						},
 						Amount: "1000",
 					},

diff --git a/modules/apps/transfer/keeper/genesis_test.go b/modules/apps/transfer/keeper/genesis_test.go
@@ -11,22 +11,22 @@ import (
 )
 
 func (suite *KeeperTestSuite) TestGenesis() {
-	getTrace := func(index uint) string {
-		return fmt.Sprintf("transfer/channelToChain%d", index)
+	getTrace := func(index uint) types.Trace {
+		return types.NewTrace("transfer", fmt.Sprintf("channelToChain%d", index))
 	}
 
 	var (
 		denoms                types.Denoms
 		escrows               sdk.Coins
 		traceAndEscrowAmounts = []struct {
-			trace  []string
+			trace  []types.Trace
 			escrow string
 		}{
-			{[]string{getTrace(0)}, "10"},
-			{[]string{getTrace(1), getTrace(0)}, "100000"},
-			{[]string{getTrace(2), getTrace(1), getTrace(0)}, "10000000000"},
-			{[]string{getTrace(3), getTrace(2), getTrace(1), getTrace(0)}, "1000000000000000"},
-			{[]string{getTrace(4), getTrace(3), getTrace(2), getTrace(1), getTrace(0)}, "100000000000000000000"},
+			{[]types.Trace{getTrace(0)}, "10"},
+			{[]types.Trace{getTrace(1), getTrace(0)}, "100000"},
+			{[]types.Trace{getTrace(2), getTrace(1), getTrace(0)}, "10000000000"},
+			{[]types.Trace{getTrace(3), getTrace(2), getTrace(1), getTrace(0)}, "1000000000000000"},
+			{[]types.Trace{getTrace(4), getTrace(3), getTrace(2), getTrace(1), getTrace(0)}, "100000000000000000000"},
 		}
 	)
 

diff --git a/modules/apps/transfer/keeper/grpc_query_test.go b/modules/apps/transfer/keeper/grpc_query_test.go
@@ -27,8 +27,11 @@ func (suite *KeeperTestSuite) TestQueryDenom() {
 			"success: correct ibc denom",
 			func() {
 				expDenom = types.Denom{
-					Base:  "uatom",                                                //nolint:goconst
-					Trace: []string{"transfer/channelToA", "transfer/channelToB"}, //nolint:goconst
+					Base: "uatom", //nolint:goconst
+					Trace: []types.Trace{
+						types.NewTrace("transfer", "channelToA"), //nolint:goconst
+						types.NewTrace("transfer", "channelToB"), //nolint:goconst
+					},
 				}
 				suite.chainA.GetSimApp().TransferKeeper.SetDenom(suite.chainA.GetContext(), expDenom)
 
@@ -42,10 +45,12 @@ func (suite *KeeperTestSuite) TestQueryDenom() {
 			"success: correct hex hash",
 			func() {
 				expDenom = types.Denom{
-					Base:  "uatom",                                                //nolint:goconst
-					Trace: []string{"transfer/channelToA", "transfer/channelToB"}, //nolint:goconst
+					Base: "uatom", //nolint:goconst
+					Trace: []types.Trace{
+						types.NewTrace("transfer", "channelToA"), //nolint:goconst
+						types.NewTrace("transfer", "channelToB"), //nolint:goconst
+					},
 				}
-
 				suite.chainA.GetSimApp().TransferKeeper.SetDenom(suite.chainA.GetContext(), expDenom)
 
 				req = &types.QueryDenomRequest{
@@ -67,8 +72,11 @@ func (suite *KeeperTestSuite) TestQueryDenom() {
 			"failure: not found denom trace",
 			func() {
 				expDenom = types.Denom{
-					Base:  "uatom",                                                //nolint:goconst
-					Trace: []string{"transfer/channelToA", "transfer/channelToB"}, //nolint:goconst
+					Base: "uatom", //nolint:goconst
+					Trace: []types.Trace{
+						types.NewTrace("transfer", "channelToA"), //nolint:goconst
+						types.NewTrace("transfer", "channelToB"), //nolint:goconst
+					},
 				}
 
 				req = &types.QueryDenomRequest{
@@ -122,8 +130,8 @@ func (suite *KeeperTestSuite) TestQueryDenoms() {
 			"success",
 			func() {
 				expDenoms = append(expDenoms, types.Denom{Base: "uatom"})
-				expDenoms = append(expDenoms, types.Denom{Base: "uatom", Trace: []string{"transfer/channelToB"}})
-				expDenoms = append(expDenoms, types.Denom{Base: "uatom", Trace: []string{"transfer/channelToA", "transfer/channelToB"}})
+				expDenoms = append(expDenoms, types.Denom{Base: "uatom", Trace: []types.Trace{types.NewTrace("transfer", "channelToB")}})
+				expDenoms = append(expDenoms, types.Denom{Base: "uatom", Trace: []types.Trace{types.NewTrace("transfer", "channelToA"), types.NewTrace("transfer", "channelToB")}})
 
 				for _, trace := range expDenoms {
 					suite.chainA.GetSimApp().TransferKeeper.SetDenom(suite.chainA.GetContext(), trace)
@@ -170,8 +178,11 @@ func (suite *KeeperTestSuite) TestQueryParams() {
 
 func (suite *KeeperTestSuite) TestQueryDenomHash() {
 	reqDenom := types.Denom{
-		Base:  "uatom",
-		Trace: []string{"transfer/channelToA", "transfer/channelToB"},
+		Base: "uatom",
+		Trace: []types.Trace{
+			types.NewTrace("transfer", "channelToA"),
+			types.NewTrace("transfer", "channelToB"),
+		},
 	}
 
 	var (
@@ -336,7 +347,7 @@ func (suite *KeeperTestSuite) TestTotalEscrowForDenom() {
 			func() {
 				denomTrace := types.Denom{
 					Base:  sdk.DefaultBondDenom,
-					Trace: []string{"transfer/channel-0"},
+					Trace: []types.Trace{types.NewTrace("transfer", "channel-0")},
 				}
 
 				suite.chainA.GetSimApp().TransferKeeper.SetDenom(suite.chainA.GetContext(), denomTrace)
@@ -355,7 +366,7 @@ func (suite *KeeperTestSuite) TestTotalEscrowForDenom() {
 			func() {
 				denomTrace := types.Denom{
 					Base:  sdk.DefaultBondDenom,
-					Trace: []string{"transfer/channel-0"},
+					Trace: []types.Trace{types.NewTrace("transfer", "channel-0")},
 				}
 
 				req = &types.QueryTotalEscrowForDenomRequest{

diff --git a/modules/apps/transfer/keeper/mbt_relay_test.go b/modules/apps/transfer/keeper/mbt_relay_test.go
@@ -341,8 +341,7 @@ func (suite *KeeperTestSuite) TestModelBasedRelay() {
 						panic(errors.New("MBT failed to convert sender address"))
 					}
 					registerDenomFn()
-					denomTrace := types.ParseDenomTrace(tc.packet.Data.Tokens[0].GetFullDenomPath())
-					denom := denomTrace.IBCDenom()
+					denom := tc.packet.Data.Tokens[0].Denom.IBCDenom()
 					err = sdk.ValidateDenom(denom)
 					if err == nil {
 						amount, ok := sdkmath.NewIntFromString(tc.packet.Data.Tokens[0].Amount)

diff --git a/modules/apps/transfer/keeper/relay.go b/modules/apps/transfer/keeper/relay.go
@@ -141,7 +141,7 @@ func (k Keeper) sendTransfer(
 				telemetry.SetGaugeWithLabels(
 					[]string{"tx", "msg", "ibc", "transfer"},
 					float32(amount.Int64()),
-					[]metrics.Label{telemetry.NewLabel(coretypes.LabelDenom, token.GetFullDenomPath())},
+					[]metrics.Label{telemetry.NewLabel(coretypes.LabelDenom, token.Denom.FullPath())},
 				)
 			}
 		}
@@ -239,7 +239,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t
 		// sender chain is the source, mint vouchers
 
 		// since SendPacket did not prefix the denomination, we must add the destination port and channel to the trace
-		trace := []string{fmt.Sprintf("%s/%s", packet.DestinationPort, packet.DestinationChannel)}
+		trace := []types.Trace{types.NewTrace(packet.DestinationPort, packet.DestinationChannel)}
 		token.Denom.Trace = append(trace, token.Denom.Trace...)
 
 		if !k.HasDenom(ctx, token.Denom.Hash()) {

diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go
@@ -56,8 +56,10 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
 			func() {
 				// send IBC token back to chainB
 				denom := types.Denom{
-					Base:  coin.Denom,
-					Trace: []string{path.EndpointA.ChannelConfig.PortID + "/" + path.EndpointA.ChannelID},
+					Base: coin.Denom,
+					Trace: []types.Trace{
+						types.NewTrace(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID),
+					},
 				}
 				coin = sdk.NewCoin(denom.IBCDenom(), coin.Amount)
 			},
@@ -68,8 +70,10 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
 			func() {
 				// send IBC token back to chainB
 				denom := types.Denom{
-					Base:  coin.Denom,
-					Trace: []string{path.EndpointA.ChannelConfig.PortID + "/" + path.EndpointA.ChannelID},
+					Base: coin.Denom,
+					Trace: []types.Trace{
+						types.NewTrace(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID),
+					},
 				}
 				coin = sdk.NewCoin(denom.IBCDenom(), coin.Amount)
 				memo = "memo"
@@ -102,8 +106,10 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
 			"failure: denom trace not found",
 			func() {
 				denom := types.Denom{
-					Base:  "randomdenom",
-					Trace: []string{path.EndpointA.ChannelConfig.PortID + "/" + path.EndpointA.ChannelID},
+					Base: "randomdenom",
+					Trace: []types.Trace{
+						types.NewTrace(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID),
+					},
 				}
 				coin = sdk.NewCoin(denom.IBCDenom(), coin.Amount)
 			},
@@ -113,8 +119,10 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
 			"failure: bank send from module account failed, insufficient balance",
 			func() {
 				denom := types.Denom{
-					Base:  coin.Denom,
-					Trace: []string{path.EndpointA.ChannelConfig.PortID + "/" + path.EndpointA.ChannelID},
+					Base: coin.Denom,
+					Trace: []types.Trace{
+						types.NewTrace(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID),
+					},
 				}
 				coin = sdk.NewCoin(denom.IBCDenom(), coin.Amount.Add(sdkmath.NewInt(1)))
 			},
@@ -374,7 +382,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket_ReceiverIsNotSource() {
 					{
 						Denom: types.Denom{
 							Base:  sdk.DefaultBondDenom,
-							Trace: []string{},
+							Trace: []types.Trace{},
 						},
 						Amount: amount.String(),
 					},

diff --git a/modules/apps/transfer/transfer_test.go b/modules/apps/transfer/transfer_test.go
@@ -1,7 +1,6 @@
 package transfer_test
 
 import (
-	"fmt"
 	"testing"
 
 	testifysuite "github.com/stretchr/testify/suite"
@@ -71,8 +70,10 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() {
 
 	// check that voucher exists on chain B
 	denom := types.Denom{
-		Base:  sdk.DefaultBondDenom,
-		Trace: []string{fmt.Sprintf("%s/%s", packet.DestinationPort, packet.DestinationChannel)},
+		Base: sdk.DefaultBondDenom,
+		Trace: []types.Trace{
+			types.NewTrace(packet.DestinationPort, packet.DestinationChannel),
+		},
 	}
 	balance = suite.chainB.GetSimApp().BankKeeper.GetBalance(suite.chainB.GetContext(), suite.chainB.SenderAccount.GetAddress(), denom.IBCDenom())
 	coinSentFromAToB := sdk.NewCoin(denom.IBCDenom(), amount)
@@ -97,7 +98,7 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() {
 	suite.Require().NoError(err) // relay committed
 
 	// NOTE: fungible token is prefixed with the full trace in order to verify the packet commitment
-	trace := []string{fmt.Sprintf("%s/%s", pathBtoC.EndpointB.ChannelConfig.PortID, pathBtoC.EndpointB.ChannelID)}
+	trace := []types.Trace{types.NewTrace(pathBtoC.EndpointB.ChannelConfig.PortID, pathBtoC.EndpointB.ChannelID)}
 	denom.Trace = append(trace, denom.Trace...)
 
 	// check that the balance is updated on chainC