Skip to content

Commit 19ecea8

Browse files
srdtrkmergify[bot]
authored andcommitted
imp: add extra validation to ica msgs (#8734)
* imp: add extra validation * chore: lint * imp: error msg * test: fix tests * docs: document the validation change --------- Co-authored-by: Alex | Cosmos Labs <alex@cosmoslabs.io> (cherry picked from commit 7574333) # Conflicts: # docs/docs/02-apps/02-interchain-accounts/07-tx-encoding.md # modules/apps/27-interchain-accounts/host/keeper/relay_test.go
1 parent 4872019 commit 19ecea8

File tree

3 files changed

+233
-12
lines changed

3 files changed

+233
-12
lines changed
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
---
2+
title: Transaction Encoding
3+
sidebar_label: Transaction Encoding
4+
sidebar_position: 7
5+
slug: /apps/interchain-accounts/tx-encoding
6+
---
7+
8+
# Transaction Encoding
9+
10+
When orchestrating an interchain account transaction, which comprises multiple `sdk.Msg` objects represented as `Any` types, the transactions must be encoded as bytes within [`InterchainAccountPacketData`](https://github.com/cosmos/ibc-go/blob/v7.2.0/proto/ibc/applications/interchain_accounts/v1/packet.proto#L21-L26).
11+
12+
```protobuf
13+
// InterchainAccountPacketData is comprised of a raw transaction, type of transaction and optional memo field.
14+
message InterchainAccountPacketData {
15+
Type type = 1;
16+
bytes data = 2;
17+
string memo = 3;
18+
}
19+
```
20+
21+
The `data` field must be encoded as a [`CosmosTx`](https://github.com/cosmos/ibc-go/blob/v7.2.0/proto/ibc/applications/interchain_accounts/v1/packet.proto#L28-L31).
22+
23+
```protobuf
24+
// CosmosTx contains a list of sdk.Msg's. It should be used when sending transactions to an SDK host chain.
25+
message CosmosTx {
26+
repeated google.protobuf.Any messages = 1;
27+
}
28+
```
29+
30+
The encoding method for `CosmosTx` is determined during the channel handshake process. If the channel version [metadata's `encoding` field](https://github.com/cosmos/ibc-go/blob/v7.2.0/proto/ibc/applications/interchain_accounts/v1/metadata.proto#L22) is marked as `proto3`, then `CosmosTx` undergoes protobuf encoding. Conversely, if the field is set to `proto3json`, then [proto3 json](https://protobuf.dev/programming-guides/proto3/#json) encoding takes place, which generates a JSON representation of the protobuf message.
31+
32+
## Protobuf Encoding
33+
34+
Protobuf encoding serves as the standard encoding process for `CosmosTx`. This occurs if the channel handshake initiates with an empty channel version metadata or if the `encoding` field explicitly denotes `proto3`. In Golang, the protobuf encoding procedure utilizes the `proto.Marshal` function. Every protobuf autogenerated Golang type comes equipped with a `Marshal` method that can be employed to encode the message.
35+
36+
## (Protobuf) JSON Encoding
37+
38+
The proto3 JSON encoding presents an alternative encoding technique for `CosmosTx`. It is selected if the channel handshake begins with the channel version metadata `encoding` field labeled as `proto3json`. In Golang, the Proto3 canonical encoding in JSON is implemented by the `"github.com/cosmos/gogoproto/jsonpb"` package. Within Cosmos SDK, the `ProtoCodec` structure implements the `JSONCodec` interface, leveraging the `jsonpb` package. This method generates a JSON format as follows:
39+
40+
```json
41+
{
42+
"messages": [
43+
{
44+
"@type": "/cosmos.bank.v1beta1.MsgSend",
45+
"from_address": "cosmos1...",
46+
"to_address": "cosmos1...",
47+
"amount": [
48+
{
49+
"denom": "uatom",
50+
"amount": "1000000"
51+
}
52+
]
53+
}
54+
]
55+
}
56+
```
57+
58+
Here, the `"messages"` array is populated with transactions. Each transaction is represented as a JSON object with the `@type` field denoting the transaction type and the remaining fields representing the transaction's attributes.
59+
60+
:::warning
61+
When utilizing proto3 JSON encoding, we have extra validations to ensure the integrity of the encoded data. Specifically, after decoding the `CosmosTx` from JSON, we re-encode it back to JSON and compare it with the original input. If non-formatting discrepancies arise, an error is returned. This validation step ensures that the JSON representation remains consistent throughout the encoding and decoding processes. This additional check means that all optional fields must be explicitly set in the JSON input, all enums and integers must be represented as strings.
62+
:::

modules/apps/27-interchain-accounts/host/keeper/relay_test.go

Lines changed: 148 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
555555
expPass bool
556556
}{
557557
{
558-
"interchain account successfully executes an arbitrary message type using the * (allow all message types) param",
558+
"success: interchain account successfully executes an arbitrary message type using the * (allow all message types) param",
559559
func(icaAddress string) {
560560
// Populate the gov keeper in advance with an active proposal
561561
testProposal := &govtypes.TextProposal{
@@ -579,8 +579,9 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
579579
{
580580
"@type": "/cosmos.gov.v1beta1.MsgVote",
581581
"voter": "` + icaAddress + `",
582-
"proposal_id": 1,
583-
"option": 1
582+
"proposal_id": "1",
583+
"option": "VOTE_OPTION_YES",
584+
"metadata": ""
584585
}
585586
]
586587
}`)
@@ -599,7 +600,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
599600
true,
600601
},
601602
{
602-
"interchain account successfully executes banktypes.MsgSend",
603+
"success: interchain account successfully executes banktypes.MsgSend",
603604
func(icaAddress string) {
604605
msgBytes := []byte(`{
605606
"messages": [
@@ -624,7 +625,11 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
624625
true,
625626
},
626627
{
628+
<<<<<<< HEAD
627629
"interchain account successfully executes govtypes.MsgSubmitProposal",
630+
=======
631+
"success: interchain account successfully executes govtypesv1.MsgSubmitProposal",
632+
>>>>>>> 7574333c (imp: add extra validation to ica msgs (#8734))
628633
func(icaAddress string) {
629634
msgBytes := []byte(`{
630635
"messages": [
@@ -653,7 +658,11 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
653658
true,
654659
},
655660
{
661+
<<<<<<< HEAD
656662
"interchain account successfully executes govtypes.MsgVote",
663+
=======
664+
"success: interchain account successfully executes govtypesv1.MsgVote",
665+
>>>>>>> 7574333c (imp: add extra validation to ica msgs (#8734))
657666
func(icaAddress string) {
658667
// Populate the gov keeper in advance with an active proposal
659668
testProposal := &govtypes.TextProposal{
@@ -677,8 +686,9 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
677686
{
678687
"@type": "/cosmos.gov.v1beta1.MsgVote",
679688
"voter": "` + icaAddress + `",
680-
"proposal_id": 1,
681-
"option": 1
689+
"proposal_id": "1",
690+
"option": "VOTE_OPTION_YES",
691+
"metadata": ""
682692
}
683693
]
684694
}`)
@@ -695,7 +705,11 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
695705
true,
696706
},
697707
{
708+
<<<<<<< HEAD
698709
"interchain account successfully executes govtypes.MsgSubmitProposal, govtypes.MsgDeposit, and then govtypes.MsgVote sequentially",
710+
=======
711+
"success: interchain account successfully executes govtypesv1.MsgSubmitProposal, govtypesv1.MsgDeposit, and then govtypesv1.MsgVote sequentially",
712+
>>>>>>> 7574333c (imp: add extra validation to ica msgs (#8734))
699713
func(icaAddress string) {
700714
msgBytes := []byte(`{
701715
"messages": [
@@ -710,16 +724,22 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
710724
"proposer": "` + icaAddress + `"
711725
},
712726
{
727+
<<<<<<< HEAD
713728
"@type": "/cosmos.gov.v1beta1.MsgDeposit",
714729
"proposal_id": 1,
730+
=======
731+
"@type": "/cosmos.gov.v1.MsgDeposit",
732+
"proposal_id": "1",
733+
>>>>>>> 7574333c (imp: add extra validation to ica msgs (#8734))
715734
"depositor": "` + icaAddress + `",
716735
"amount": [{ "denom": "stake", "amount": "10000000" }]
717736
},
718737
{
719738
"@type": "/cosmos.gov.v1beta1.MsgVote",
720739
"voter": "` + icaAddress + `",
721-
"proposal_id": 1,
722-
"option": 1
740+
"proposal_id": "1",
741+
"option": "VOTE_OPTION_YES",
742+
"metadata": ""
723743
}
724744
]
725745
}`)
@@ -736,7 +756,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
736756
true,
737757
},
738758
{
739-
"interchain account successfully executes transfertypes.MsgTransfer",
759+
"success: interchain account successfully executes transfertypes.MsgTransfer",
740760
func(icaAddress string) {
741761
transferPath := ibctesting.NewTransferPath(suite.chainB, suite.chainC)
742762

@@ -751,8 +771,17 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
751771
"token": { "denom": "stake", "amount": "100" },
752772
"sender": "` + icaAddress + `",
753773
"receiver": "cosmos15ulrf36d4wdtrtqzkgaan9ylwuhs7k7qz753uk",
774+
<<<<<<< HEAD
754775
"timeout_height": { "revision_number": 1, "revision_height": 100 },
755776
"timeout_timestamp": 0
777+
=======
778+
"timeout_height": { "revision_number": "1", "revision_height": "100" },
779+
"timeout_timestamp": "0",
780+
"memo": "",
781+
"encoding": "",
782+
"use_aliasing": false
783+
784+
>>>>>>> 7574333c (imp: add extra validation to ica msgs (#8734))
756785
}
757786
]
758787
}`)
@@ -769,7 +798,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
769798
true,
770799
},
771800
{
772-
"unregistered sdk.Msg",
801+
"failure: unregistered sdk.Msg",
773802
func(icaAddress string) {
774803
msgBytes := []byte(`{"messages":[{}]}`)
775804
byteArrayString := strings.Join(strings.Fields(fmt.Sprint(msgBytes)), ",")
@@ -785,7 +814,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
785814
false,
786815
},
787816
{
788-
"message type not allowed banktypes.MsgSend",
817+
"failure: message type not allowed banktypes.MsgSend",
789818
func(icaAddress string) {
790819
msgBytes := []byte(`{
791820
"messages": [
@@ -810,7 +839,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
810839
false,
811840
},
812841
{
813-
"unauthorised: signer address is not the interchain account associated with the controller portID",
842+
"failure: signer address is not the interchain account associated with the controller portID",
814843
func(icaAddress string) {
815844
msgBytes := []byte(`{
816845
"messages": [
@@ -834,6 +863,113 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
834863
},
835864
false,
836865
},
866+
{
867+
"failure: missing optional metadata field",
868+
func(icaAddress string) {
869+
proposal, err := govtypesv1.NewProposal([]sdk.Msg{getTestProposalMessage()}, govtypesv1.DefaultStartingProposalID, s.chainA.GetContext().BlockTime(), s.chainA.GetContext().BlockTime(), "test proposal", "title", "Description", sdk.AccAddress(interchainAccountAddr), false)
870+
s.Require().NoError(err)
871+
872+
err = s.chainB.GetSimApp().GovKeeper.SetProposal(s.chainB.GetContext(), proposal)
873+
s.Require().NoError(err)
874+
err = s.chainB.GetSimApp().GovKeeper.ActivateVotingPeriod(s.chainB.GetContext(), proposal)
875+
s.Require().NoError(err)
876+
877+
msgBytes := []byte(`{
878+
"messages": [
879+
{
880+
"@type": "/cosmos.gov.v1.MsgVote",
881+
"voter": "` + icaAddress + `",
882+
"proposal_id": "1",
883+
"option": "VOTE_OPTION_YES"
884+
}
885+
]
886+
}`)
887+
// this is the way cosmwasm encodes byte arrays by default
888+
// golang doesn't use this encoding by default, but it can still deserialize:
889+
byteArrayString := strings.Join(strings.Fields(fmt.Sprint(msgBytes)), ",") //nolint:staticcheck
890+
891+
packetData = []byte(`{
892+
"type": 1,
893+
"data":` + byteArrayString + `
894+
}`)
895+
896+
params := types.NewParams(true, []string{"*"})
897+
s.chainB.GetSimApp().ICAHostKeeper.SetParams(s.chainB.GetContext(), params)
898+
},
899+
ibcerrors.ErrInvalidType,
900+
},
901+
{
902+
"failure: alternative int representation of enum field",
903+
func(icaAddress string) {
904+
proposal, err := govtypesv1.NewProposal([]sdk.Msg{getTestProposalMessage()}, govtypesv1.DefaultStartingProposalID, s.chainA.GetContext().BlockTime(), s.chainA.GetContext().BlockTime(), "test proposal", "title", "Description", sdk.AccAddress(interchainAccountAddr), false)
905+
s.Require().NoError(err)
906+
907+
err = s.chainB.GetSimApp().GovKeeper.SetProposal(s.chainB.GetContext(), proposal)
908+
s.Require().NoError(err)
909+
err = s.chainB.GetSimApp().GovKeeper.ActivateVotingPeriod(s.chainB.GetContext(), proposal)
910+
s.Require().NoError(err)
911+
912+
msgBytes := []byte(`{
913+
"messages": [
914+
{
915+
"@type": "/cosmos.gov.v1.MsgVote",
916+
"voter": "` + icaAddress + `",
917+
"proposal_id": "1",
918+
"option": 1,
919+
"metadata": ""
920+
}
921+
]
922+
}`)
923+
// this is the way cosmwasm encodes byte arrays by default
924+
// golang doesn't use this encoding by default, but it can still deserialize:
925+
byteArrayString := strings.Join(strings.Fields(fmt.Sprint(msgBytes)), ",") //nolint:staticcheck
926+
927+
packetData = []byte(`{
928+
"type": 1,
929+
"data":` + byteArrayString + `
930+
}`)
931+
932+
params := types.NewParams(true, []string{"*"})
933+
s.chainB.GetSimApp().ICAHostKeeper.SetParams(s.chainB.GetContext(), params)
934+
},
935+
ibcerrors.ErrInvalidType,
936+
},
937+
{
938+
"failure: alternative integer field representation",
939+
func(icaAddress string) {
940+
proposal, err := govtypesv1.NewProposal([]sdk.Msg{getTestProposalMessage()}, govtypesv1.DefaultStartingProposalID, s.chainA.GetContext().BlockTime(), s.chainA.GetContext().BlockTime(), "test proposal", "title", "Description", sdk.AccAddress(interchainAccountAddr), false)
941+
s.Require().NoError(err)
942+
943+
err = s.chainB.GetSimApp().GovKeeper.SetProposal(s.chainB.GetContext(), proposal)
944+
s.Require().NoError(err)
945+
err = s.chainB.GetSimApp().GovKeeper.ActivateVotingPeriod(s.chainB.GetContext(), proposal)
946+
s.Require().NoError(err)
947+
948+
msgBytes := []byte(`{
949+
"messages": [
950+
{
951+
"@type": "/cosmos.gov.v1.MsgVote",
952+
"voter": "` + icaAddress + `",
953+
"proposal_id": 1,
954+
"option": "VOTE_OPTION_YES",
955+
"metadata": ""
956+
}
957+
]
958+
}`)
959+
// this is the way cosmwasm encodes byte arrays by default
960+
// golang doesn't use this encoding by default, but it can still deserialize:
961+
byteArrayString := strings.Join(strings.Fields(fmt.Sprint(msgBytes)), ",") //nolint:staticcheck
962+
963+
packetData = []byte(`{
964+
"type": 1,
965+
"data":` + byteArrayString + `
966+
}`)
967+
968+
params := types.NewParams(true, []string{"*"})
969+
s.chainB.GetSimApp().ICAHostKeeper.SetParams(s.chainB.GetContext(), params)
970+
},
971+
ibcerrors.ErrInvalidType,
972+
},
837973
}
838974

839975
for _, tc := range testCases {

modules/apps/27-interchain-accounts/types/codec.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package types
22

33
import (
4+
"encoding/json"
5+
"reflect"
6+
47
"github.com/cosmos/gogoproto/proto"
58

69
errorsmod "cosmossdk.io/errors"
@@ -90,6 +93,15 @@ func DeserializeCosmosTx(cdc codec.Codec, data []byte, encoding string) ([]sdk.M
9093
if err := cdc.UnmarshalJSON(data, &cosmosTx); err != nil {
9194
return nil, errorsmod.Wrapf(ErrUnknownDataType, "cannot unmarshal CosmosTx with proto3 json")
9295
}
96+
reconstructedData, err := cdc.MarshalJSON(&cosmosTx)
97+
if err != nil {
98+
return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot remarshal CosmosTx with proto3 json: %v", err)
99+
}
100+
if isEqual, err := equalJSON(data, reconstructedData); err != nil {
101+
return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot compare original and reconstructed JSON: %v", err)
102+
} else if !isEqual {
103+
return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "original and reconstructed JSON objects do not match, original: %s, reconstructed: %s", string(data), string(reconstructedData))
104+
}
93105
default:
94106
return nil, errorsmod.Wrapf(ErrInvalidCodec, "unsupported encoding format %s", encoding)
95107
}
@@ -107,3 +119,14 @@ func DeserializeCosmosTx(cdc codec.Codec, data []byte, encoding string) ([]sdk.M
107119

108120
return msgs, nil
109121
}
122+
123+
func equalJSON(a, b []byte) (bool, error) {
124+
var x, y any
125+
if err := json.Unmarshal(a, &x); err != nil {
126+
return false, err
127+
}
128+
if err := json.Unmarshal(b, &y); err != nil {
129+
return false, err
130+
}
131+
return reflect.DeepEqual(x, y), nil
132+
}

0 commit comments

Comments
 (0)