Skip to content

Commit 6cb4a38

Browse files
refactor: ics29 json encoded version metadata (#883)
* adding metadata type to ics29 protos * updating ics29 handshake handlers to support json encoded metadata * updating tests to support json encoded metadata * Update modules/apps/29-fee/ibc_module.go Co-authored-by: colin axnér <[email protected]> * Update modules/apps/29-fee/ibc_module.go Co-authored-by: colin axnér <[email protected]> * renaming metadata version to fee_version Co-authored-by: colin axnér <[email protected]>
1 parent 13f77de commit 6cb4a38

File tree

10 files changed

+507
-70
lines changed

10 files changed

+507
-70
lines changed

docs/ibc/proto-docs.md

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@
3838
- [GenesisState](#ibc.applications.fee.v1.GenesisState)
3939
- [RegisteredRelayerAddress](#ibc.applications.fee.v1.RegisteredRelayerAddress)
4040

41+
- [ibc/applications/fee/v1/metadata.proto](#ibc/applications/fee/v1/metadata.proto)
42+
- [Metadata](#ibc.applications.fee.v1.Metadata)
43+
4144
- [ibc/applications/fee/v1/query.proto](#ibc/applications/fee/v1/query.proto)
4245
- [QueryIncentivizedPacketRequest](#ibc.applications.fee.v1.QueryIncentivizedPacketRequest)
4346
- [QueryIncentivizedPacketResponse](#ibc.applications.fee.v1.QueryIncentivizedPacketResponse)
@@ -812,6 +815,39 @@ RegisteredRelayerAddress contains the address and counterparty address for a spe
812815

813816

814817

818+
<!-- end messages -->
819+
820+
<!-- end enums -->
821+
822+
<!-- end HasExtensions -->
823+
824+
<!-- end services -->
825+
826+
827+
828+
<a name="ibc/applications/fee/v1/metadata.proto"></a>
829+
<p align="right"><a href="#top">Top</a></p>
830+
831+
## ibc/applications/fee/v1/metadata.proto
832+
833+
834+
835+
<a name="ibc.applications.fee.v1.Metadata"></a>
836+
837+
### Metadata
838+
Metadata defines the ICS29 channel specific metadata encoded into the channel version bytestring
839+
See ICS004: https://github.com/cosmos/ibc/tree/master/spec/core/ics-004-channel-and-packet-semantics#Versioning
840+
841+
842+
| Field | Type | Label | Description |
843+
| ----- | ---- | ----- | ----------- |
844+
| `fee_version` | [string](#string) | | fee_version defines the ICS29 fee version |
845+
| `app_version` | [string](#string) | | app_version defines the underlying application version, which may or may not be a JSON encoded bytestring |
846+
847+
848+
849+
850+
815851
<!-- end messages -->
816852

817853
<!-- end enums -->

modules/apps/29-fee/fee_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func (suite *FeeTestSuite) SetupTest() {
3030
suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(1))
3131

3232
path := ibctesting.NewPath(suite.chainA, suite.chainB)
33-
feeTransferVersion := channeltypes.MergeChannelVersions(types.Version, transfertypes.Version)
33+
feeTransferVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.Version}))
3434
path.EndpointA.ChannelConfig.Version = feeTransferVersion
3535
path.EndpointB.ChannelConfig.Version = feeTransferVersion
3636
path.EndpointA.ChannelConfig.PortID = transfertypes.PortID

modules/apps/29-fee/ibc_module.go

Lines changed: 43 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -37,22 +37,24 @@ func (im IBCModule) OnChanOpenInit(
3737
counterparty channeltypes.Counterparty,
3838
version string,
3939
) error {
40-
mwVersion, appVersion := channeltypes.SplitChannelVersion(version)
41-
// Since it is valid for fee version to not be specified, the above middleware version may be for a middleware
42-
// lower down in the stack. Thus, if it is not a fee version we pass the entire version string onto the underlying
43-
// application.
44-
// If an invalid fee version was passed, we expect the underlying application to fail on its version negotiation.
45-
if mwVersion == types.Version {
46-
im.keeper.SetFeeEnabled(ctx, portID, channelID)
47-
} else {
48-
// middleware version is not the expected version for this midddleware. Pass the full version string along,
49-
// if it not valid version for any other lower middleware, an error will be returned by base application.
50-
appVersion = version
40+
var versionMetadata types.Metadata
41+
if err := types.ModuleCdc.UnmarshalJSON([]byte(version), &versionMetadata); err != nil {
42+
// Since it is valid for fee version to not be specified, the above middleware version may be for a middleware
43+
// lower down in the stack. Thus, if it is not a fee version we pass the entire version string onto the underlying
44+
// application.
45+
return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID,
46+
chanCap, counterparty, version)
5147
}
5248

49+
if versionMetadata.FeeVersion != types.Version {
50+
return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, versionMetadata.FeeVersion)
51+
}
52+
53+
im.keeper.SetFeeEnabled(ctx, portID, channelID)
54+
5355
// call underlying app's OnChanOpenInit callback with the appVersion
5456
return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID,
55-
chanCap, counterparty, appVersion)
57+
chanCap, counterparty, versionMetadata.AppVersion)
5658
}
5759

5860
// OnChanOpenTry implements the IBCModule interface
@@ -68,31 +70,34 @@ func (im IBCModule) OnChanOpenTry(
6870
counterparty channeltypes.Counterparty,
6971
counterpartyVersion string,
7072
) (string, error) {
71-
cpMwVersion, cpAppVersion := channeltypes.SplitChannelVersion(counterpartyVersion)
73+
var versionMetadata types.Metadata
74+
if err := types.ModuleCdc.UnmarshalJSON([]byte(counterpartyVersion), &versionMetadata); err != nil {
75+
// Since it is valid for fee version to not be specified, the above middleware version may be for a middleware
76+
// lower down in the stack. Thus, if it is not a fee version we pass the entire version string onto the underlying
77+
// application.
78+
return im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, counterpartyVersion)
79+
}
7280

73-
// Since it is valid for fee version to not be specified, the above middleware version may be for a middleware
74-
// lower down in the stack. Thus, if it is not a fee version we pass the entire version string onto the underlying
75-
// application.
76-
// If an invalid fee version was passed, we expect the underlying application to fail on its version negotiation.
77-
if cpMwVersion == types.Version {
78-
im.keeper.SetFeeEnabled(ctx, portID, channelID)
79-
} else {
80-
// middleware versions are not the expected version for this midddleware. Pass the full version strings along,
81-
// if it not valid version for any other lower middleware, an error will be returned by base application.
82-
cpAppVersion = counterpartyVersion
81+
if versionMetadata.FeeVersion != types.Version {
82+
return "", sdkerrors.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, versionMetadata.FeeVersion)
8383
}
8484

85+
im.keeper.SetFeeEnabled(ctx, portID, channelID)
86+
8587
// call underlying app's OnChanOpenTry callback with the app versions
86-
appVersion, err := im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, cpAppVersion)
88+
appVersion, err := im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, versionMetadata.AppVersion)
8789
if err != nil {
8890
return "", err
8991
}
9092

91-
if !im.keeper.IsFeeEnabled(ctx, portID, channelID) {
92-
return appVersion, nil
93+
versionMetadata.AppVersion = appVersion
94+
95+
versionBytes, err := types.ModuleCdc.MarshalJSON(&versionMetadata)
96+
if err != nil {
97+
return "", err
9398
}
9499

95-
return channeltypes.MergeChannelVersions(types.Version, appVersion), nil
100+
return string(versionBytes), nil
96101
}
97102

98103
// OnChanOpenAck implements the IBCModule interface
@@ -104,17 +109,22 @@ func (im IBCModule) OnChanOpenAck(
104109
) error {
105110
// If handshake was initialized with fee enabled it must complete with fee enabled.
106111
// If handshake was initialized with fee disabled it must complete with fee disabled.
107-
cpAppVersion := counterpartyVersion
108112
if im.keeper.IsFeeEnabled(ctx, portID, channelID) {
109-
var cpFeeVersion string
110-
cpFeeVersion, cpAppVersion = channeltypes.SplitChannelVersion(counterpartyVersion)
113+
var versionMetadata types.Metadata
114+
if err := types.ModuleCdc.UnmarshalJSON([]byte(counterpartyVersion), &versionMetadata); err != nil {
115+
return sdkerrors.Wrap(types.ErrInvalidVersion, "failed to unmarshal ICS29 counterparty version metadata")
116+
}
111117

112-
if cpFeeVersion != types.Version {
113-
return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty version: %s, got: %s", types.Version, cpFeeVersion)
118+
if versionMetadata.FeeVersion != types.Version {
119+
return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty version: %s, got: %s", types.Version, versionMetadata.FeeVersion)
114120
}
121+
122+
// call underlying app's OnChanOpenAck callback with the counterparty app version.
123+
return im.app.OnChanOpenAck(ctx, portID, channelID, versionMetadata.AppVersion)
115124
}
125+
116126
// call underlying app's OnChanOpenAck callback with the counterparty app version.
117-
return im.app.OnChanOpenAck(ctx, portID, channelID, cpAppVersion)
127+
return im.app.OnChanOpenAck(ctx, portID, channelID, counterpartyVersion)
118128
}
119129

120130
// OnChanOpenConfirm implements the IBCModule interface

modules/apps/29-fee/ibc_module_test.go

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package fee_test
22

33
import (
4-
"fmt"
5-
64
sdk "github.com/cosmos/cosmos-sdk/types"
75
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
86

@@ -28,40 +26,30 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() {
2826
expPass bool
2927
}{
3028
{
31-
"valid fee middleware and transfer version",
32-
channeltypes.MergeChannelVersions(types.Version, transfertypes.Version),
29+
"success - valid fee middleware and transfer version",
30+
string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.Version})),
3331
true,
3432
},
3533
{
36-
"fee version not included, only perform transfer logic",
34+
"success - fee version not included, only perform transfer logic",
3735
transfertypes.Version,
3836
true,
3937
},
4038
{
4139
"invalid fee middleware version",
42-
channeltypes.MergeChannelVersions("otherfee28-1", transfertypes.Version),
40+
string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: "invalid-ics29-1", AppVersion: transfertypes.Version})),
4341
false,
4442
},
4543
{
4644
"invalid transfer version",
47-
channeltypes.MergeChannelVersions(types.Version, "wrongics20-1"),
48-
false,
49-
},
50-
{
51-
"incorrect wrapping delimiter",
52-
fmt.Sprintf("%s//%s", types.Version, transfertypes.Version),
45+
string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: "invalid-ics20-1"})),
5346
false,
5447
},
5548
{
5649
"transfer version not wrapped",
5750
types.Version,
5851
false,
5952
},
60-
{
61-
"hanging delimiter",
62-
fmt.Sprintf("%s:%s:", types.Version, transfertypes.Version),
63-
false,
64-
},
6553
}
6654

6755
for _, tc := range testCases {
@@ -112,32 +100,32 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() {
112100
expPass bool
113101
}{
114102
{
115-
"valid fee middleware version",
116-
channeltypes.MergeChannelVersions(types.Version, transfertypes.Version),
103+
"success - valid fee middleware version",
104+
string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.Version})),
117105
false,
118106
true,
119107
},
120108
{
121-
"valid transfer version",
109+
"success - valid transfer version",
122110
transfertypes.Version,
123111
false,
124112
true,
125113
},
126114
{
127-
"crossing hellos: valid fee middleware",
128-
channeltypes.MergeChannelVersions(types.Version, transfertypes.Version),
115+
"success - crossing hellos: valid fee middleware",
116+
string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.Version})),
129117
true,
130118
true,
131119
},
132120
{
133121
"invalid fee middleware version",
134-
channeltypes.MergeChannelVersions("wrongfee29-1", transfertypes.Version),
122+
string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: "invalid-ics29-1", AppVersion: transfertypes.Version})),
135123
false,
136124
false,
137125
},
138126
{
139127
"invalid transfer version",
140-
channeltypes.MergeChannelVersions(types.Version, "wrongics20-1"),
128+
string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: "invalid-ics20-1"})),
141129
false,
142130
false,
143131
},
@@ -205,25 +193,31 @@ func (suite *FeeTestSuite) TestOnChanOpenAck() {
205193
}{
206194
{
207195
"success",
208-
channeltypes.MergeChannelVersions(types.Version, transfertypes.Version),
196+
string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.Version})),
209197
func(suite *FeeTestSuite) {},
210198
true,
211199
},
212200
{
213201
"invalid fee version",
214-
channeltypes.MergeChannelVersions("fee29-A", transfertypes.Version),
202+
string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: "invalid-ics29-1", AppVersion: transfertypes.Version})),
215203
func(suite *FeeTestSuite) {},
216204
false,
217205
},
218206
{
219207
"invalid transfer version",
220-
channeltypes.MergeChannelVersions(types.Version, "ics20-4"),
208+
string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: "invalid-ics20-1"})),
209+
func(suite *FeeTestSuite) {},
210+
false,
211+
},
212+
{
213+
"invalid version fails to unmarshal metadata",
214+
"invalid-version",
221215
func(suite *FeeTestSuite) {},
222216
false,
223217
},
224218
{
225219
"previous INIT set without fee, however counterparty set fee version", // note this can only happen with incompetent or malicious counterparty chain
226-
channeltypes.MergeChannelVersions(types.Version, transfertypes.Version),
220+
string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.Version})),
227221
func(suite *FeeTestSuite) {
228222
// do the first steps without fee version, then pass the fee version as counterparty version in ChanOpenACK
229223
suite.path.EndpointA.ChannelConfig.Version = transfertypes.Version

modules/apps/29-fee/keeper/keeper_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func (suite *KeeperTestSuite) SetupTest() {
4040
suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(1))
4141

4242
path := ibctesting.NewPath(suite.chainA, suite.chainB)
43-
feeTransferVersion := channeltypes.MergeChannelVersions(types.Version, transfertypes.Version)
43+
feeTransferVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.Version}))
4444
path.EndpointA.ChannelConfig.Version = feeTransferVersion
4545
path.EndpointB.ChannelConfig.Version = feeTransferVersion
4646
path.EndpointA.ChannelConfig.PortID = transfertypes.PortID

0 commit comments

Comments
 (0)