feat(transfer): use protobuf for ser/de.

cosmos · Jul 2, 2024 · b4c6a5e · b4c6a5e
1 parent 3456ac3
commit b4c6a5e
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 66 deletions.
diff --git a/modules/apps/callbacks/ibc_middleware_test.go b/modules/apps/callbacks/ibc_middleware_test.go
@@ -1,9 +1,10 @@
 package ibccallbacks_test
 
 import (
-	"encoding/json"
 	"fmt"
 
+	"github.com/cosmos/gogoproto/proto"
+
 	errorsmod "cosmossdk.io/errors"
 	storetypes "cosmossdk.io/store/types"
 
@@ -507,7 +508,7 @@ func (s *CallbacksTestSuite) TestOnTimeoutPacket() {
 			s.Require().NoError(err)
 			s.Require().NotNil(packet)
 
-			err = json.Unmarshal(packet.Data, &packetData)
+			err = proto.Unmarshal(packet.Data, &packetData)
 			s.Require().NoError(err)
 
 			ctx = s.chainA.GetContext()

diff --git a/modules/apps/transfer/ibc_module_test.go b/modules/apps/transfer/ibc_module_test.go
@@ -332,11 +332,11 @@ func (suite *TransferTestSuite) TestOnRecvPacket() {
 					sdk.NewAttribute(types.AttributeKeyMemo, ""),
 					sdk.NewAttribute(types.AttributeKeyForwardingHops, "null"),
 					sdk.NewAttribute(types.AttributeKeyAckSuccess, "false"),
-					sdk.NewAttribute(types.AttributeKeyAckError, "cannot unmarshal ICS20-V2 transfer packet data: invalid character 'i' looking for beginning of value: invalid type: invalid type"),
+					sdk.NewAttribute(types.AttributeKeyAckError, "cannot unmarshal ICS20-V2 transfer packet data: unexpected EOF: invalid type: invalid type"),
 				}
 			},
 			channeltypes.NewErrorAcknowledgement(ibcerrors.ErrInvalidType),
-			"cannot unmarshal ICS20-V2 transfer packet data: invalid character 'i' looking for beginning of value: invalid type: invalid type",
+			"cannot unmarshal ICS20-V2 transfer packet data: unexpected EOF: invalid type: invalid type",
 		},
 		{
 			"failure: receive disabled",

diff --git a/modules/apps/transfer/internal/packet.go b/modules/apps/transfer/internal/packet.go
@@ -3,6 +3,8 @@ package internal
 import (
 	"encoding/json"
 
+	"github.com/cosmos/gogoproto/proto"
+
 	errorsmod "cosmossdk.io/errors"
 
 	"github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
@@ -22,7 +24,7 @@ func UnmarshalPacketData(bz []byte, ics20Version string) (types.FungibleTokenPac
 		return packetDataV1ToV2(datav1)
 	case types.V2:
 		var datav2 types.FungibleTokenPacketDataV2
-		if err := json.Unmarshal(bz, &datav2); err != nil {
+		if err := proto.Unmarshal(bz, &datav2); err != nil {
 			return types.FungibleTokenPacketDataV2{}, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot unmarshal ICS20-V2 transfer packet data: %s", err.Error())
 		}
 

diff --git a/modules/apps/transfer/keeper/relay_forwarding_test.go b/modules/apps/transfer/keeper/relay_forwarding_test.go
@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"time"
 
+	"github.com/cosmos/gogoproto/proto"
+
 	sdkmath "cosmossdk.io/math"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -274,7 +276,7 @@ func (suite *KeeperTestSuite) TestSuccessfulForwardWithMemo() {
 
 	// Check that the memo is stored correctly in the packet sent from A
 	var tokenPacketOnA types.FungibleTokenPacketDataV2
-	err = suite.chainA.Codec.UnmarshalJSON(packetFromAtoB.Data, &tokenPacketOnA)
+	err = proto.Unmarshal(packetFromAtoB.Data, &tokenPacketOnA)
 	suite.Require().NoError(err)
 	suite.Require().Equal("", tokenPacketOnA.Memo)
 	suite.Require().Equal(testMemo, tokenPacketOnA.Forwarding.DestinationMemo)
@@ -296,7 +298,7 @@ func (suite *KeeperTestSuite) TestSuccessfulForwardWithMemo() {
 
 	// Check that the memo is stored correctly in the packet sent from B
 	var tokenPacketOnB types.FungibleTokenPacketDataV2
-	err = suite.chainB.Codec.UnmarshalJSON(packetFromBtoC.Data, &tokenPacketOnB)
+	err = proto.Unmarshal(packetFromBtoC.Data, &tokenPacketOnB)
 	suite.Require().NoError(err)
 	suite.Require().Equal(testMemo, tokenPacketOnB.Memo)
 	suite.Require().Equal("", tokenPacketOnB.Forwarding.DestinationMemo)
@@ -321,7 +323,7 @@ func (suite *KeeperTestSuite) TestSuccessfulForwardWithMemo() {
 
 	// Check that the memo is stored directly in the memo field on C
 	var tokenPacketOnC types.FungibleTokenPacketDataV2
-	err = suite.chainC.Codec.UnmarshalJSON(packetOnC.Data, &tokenPacketOnC)
+	err = proto.Unmarshal(packetOnC.Data, &tokenPacketOnC)
 	suite.Require().NoError(err)
 	suite.Require().Equal("", tokenPacketOnC.Forwarding.DestinationMemo)
 	suite.Require().Equal(testMemo, tokenPacketOnC.Memo)
@@ -409,7 +411,7 @@ func (suite *KeeperTestSuite) TestSuccessfulForwardWithNonCosmosAccAddress() {
 
 	// Check that the token sent from A has final receiver intact
 	var tokenPacketOnA types.FungibleTokenPacketDataV2
-	err = suite.chainA.Codec.UnmarshalJSON(packetFromAtoB.Data, &tokenPacketOnA)
+	err = proto.Unmarshal(packetFromAtoB.Data, &tokenPacketOnA)
 	suite.Require().NoError(err)
 	suite.Require().Equal(nonCosmosReceiver, tokenPacketOnA.Receiver)
 
@@ -429,7 +431,7 @@ func (suite *KeeperTestSuite) TestSuccessfulForwardWithNonCosmosAccAddress() {
 
 	// Check that the token sent from B has final receiver intact
 	var tokenPacketOnB types.FungibleTokenPacketDataV2
-	err = suite.chainB.Codec.UnmarshalJSON(packetFromBtoC.Data, &tokenPacketOnB)
+	err = proto.Unmarshal(packetFromBtoC.Data, &tokenPacketOnB)
 	suite.Require().NoError(err)
 	suite.Require().Equal(nonCosmosReceiver, tokenPacketOnB.Receiver)
 

diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go
@@ -1154,55 +1154,9 @@ func (suite *KeeperTestSuite) TestPacketForwardsCompatibility() {
 		expError    error
 		expAckError error
 	}{
-		{
-			"success: no new field with memo v2",
-			func() {
-				jsonString := fmt.Sprintf(`{"tokens":[{"denom": {"base": "atom", "trace": []},"amount":"100"}],"sender":"%s","receiver":"%s"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String())
-				packetData = []byte(jsonString)
-			},
-			nil,
-			nil,
-		},
-		{
-			"success: no new field without memo",
-			func() {
-				jsonString := fmt.Sprintf(`{"tokens":[{"denom": {"base": "atom", "trace": []},"amount":"100"}],"sender":"%s","receiver":"%s"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String())
-				packetData = []byte(jsonString)
-			},
-			nil,
-			nil,
-		},
-		{
-			"failure: invalid packet data v2",
-			func() {
-				packetData = []byte("invalid packet data")
-			},
-			ibcerrors.ErrInvalidType,
-			ibcerrors.ErrInvalidType,
-		},
-		{
-			"failure: new field v2",
-			func() {
-				jsonString := fmt.Sprintf(`{"tokens":[{"denom": {"base": "atom", "trace": []},"amount":"100"}],"sender":"%s","receiver":"%s", "new_field":"value"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String())
-				packetData = []byte(jsonString)
-			},
-			ibcerrors.ErrInvalidType,
-			ibcerrors.ErrInvalidType,
-		},
-		{
-			"failure: missing field v2",
-			func() {
-				jsonString := fmt.Sprintf(`{"tokens":[{"denom": {"trace": []},"amount":"100"}],"sender":"%s","receiver":"%s"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String())
-				packetData = []byte(jsonString)
-			},
-			types.ErrInvalidDenomForTransfer,
-			ibcerrors.ErrInvalidType,
-		},
 		{
 			"success: no new field with memo",
 			func() {
-				path.EndpointA.ChannelConfig.Version = types.V1
-				path.EndpointB.ChannelConfig.Version = types.V1
 				jsonString := fmt.Sprintf(`{"denom":"denom","amount":"100","sender":"%s","receiver":"%s","memo":"memo"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String())
 				packetData = []byte(jsonString)
 			},
@@ -1212,8 +1166,6 @@ func (suite *KeeperTestSuite) TestPacketForwardsCompatibility() {
 		{
 			"success: no new field without memo",
 			func() {
-				path.EndpointA.ChannelConfig.Version = types.V1
-				path.EndpointB.ChannelConfig.Version = types.V1
 				jsonString := fmt.Sprintf(`{"denom":"denom","amount":"100","sender":"%s","receiver":"%s"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String())
 				packetData = []byte(jsonString)
 			},
@@ -1223,8 +1175,6 @@ func (suite *KeeperTestSuite) TestPacketForwardsCompatibility() {
 		{
 			"failure: invalid packet data",
 			func() {
-				path.EndpointA.ChannelConfig.Version = types.V1
-				path.EndpointB.ChannelConfig.Version = types.V1
 				packetData = []byte("invalid packet data")
 			},
 			ibcerrors.ErrInvalidType,
@@ -1233,8 +1183,6 @@ func (suite *KeeperTestSuite) TestPacketForwardsCompatibility() {
 		{
 			"failure: new field",
 			func() {
-				path.EndpointA.ChannelConfig.Version = types.V1
-				path.EndpointB.ChannelConfig.Version = types.V1
 				jsonString := fmt.Sprintf(`{"denom":"denom","amount":"100","sender":"%s","receiver":"%s","memo":"memo","new_field":"value"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String())
 				packetData = []byte(jsonString)
 			},
@@ -1244,8 +1192,6 @@ func (suite *KeeperTestSuite) TestPacketForwardsCompatibility() {
 		{
 			"failure: missing field",
 			func() {
-				path.EndpointA.ChannelConfig.Version = types.V1
-				path.EndpointB.ChannelConfig.Version = types.V1
 				jsonString := fmt.Sprintf(`{"amount":"100","sender":%s","receiver":"%s"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String())
 				packetData = []byte(jsonString)
 			},
@@ -1261,6 +1207,8 @@ func (suite *KeeperTestSuite) TestPacketForwardsCompatibility() {
 			packetData = nil
 
 			path = ibctesting.NewTransferPath(suite.chainA, suite.chainB)
+			path.EndpointA.ChannelConfig.Version = types.V1
+			path.EndpointB.ChannelConfig.Version = types.V1
 
 			tc.malleate()
 

diff --git a/modules/apps/transfer/types/packet.go b/modules/apps/transfer/types/packet.go
@@ -5,6 +5,8 @@ import (
 	"errors"
 	"strings"
 
+	"github.com/cosmos/gogoproto/proto"
+
 	errorsmod "cosmossdk.io/errors"
 	sdkmath "cosmossdk.io/math"
 
@@ -152,9 +154,10 @@ func (ftpd FungibleTokenPacketDataV2) ValidateBasic() error {
 	return nil
 }
 
-// GetBytes is a helper for serialising
+// GetBytes is a helper for serialising a FungibleTokenPacketDataV2. It uses protobuf to serialise
+// the packet data and panics on failure.
 func (ftpd FungibleTokenPacketDataV2) GetBytes() []byte {
-	bz, err := json.Marshal(&ftpd)
+	bz, err := proto.Marshal(&ftpd)
 	if err != nil {
 		panic(errors.New("cannot marshal FungibleTokenPacketDataV2 into bytes"))
 	}