diff --git a/modules/apps/transfer/ibc_module.go b/modules/apps/transfer/ibc_module.go index 4cfd55ffdef..efdb32939ab 100644 --- a/modules/apps/transfer/ibc_module.go +++ b/modules/apps/transfer/ibc_module.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "math" + "slices" "strings" errorsmod "cosmossdk.io/errors" @@ -85,12 +86,13 @@ func (im IBCModule) OnChanOpenInit( return "", err } + // default to latest supported version if strings.TrimSpace(version) == "" { version = types.Version } - if version != types.Version { - return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, version) + if !slices.Contains(types.SupportedVersions, version) { + return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected one of %s, got %s", types.SupportedVersions, version) } // Claim channel capability passed back by IBC module @@ -116,8 +118,8 @@ func (im IBCModule) OnChanOpenTry( return "", err } - if counterpartyVersion != types.Version { - return "", errorsmod.Wrapf(types.ErrInvalidVersion, "invalid counterparty version: expected %s, got %s", types.Version, counterpartyVersion) + if !slices.Contains(types.SupportedVersions, counterpartyVersion) { + return types.Version, nil } // OpenTry must claim the channelCapability that IBC passes into the callback @@ -125,7 +127,7 @@ func (im IBCModule) OnChanOpenTry( return "", err } - return types.Version, nil + return counterpartyVersion, nil } // OnChanOpenAck implements the IBCModule interface @@ -136,9 +138,10 @@ func (IBCModule) OnChanOpenAck( _ string, counterpartyVersion string, ) error { - if counterpartyVersion != types.Version { - return errorsmod.Wrapf(types.ErrInvalidVersion, "invalid counterparty version: expected %s, got %s", types.Version, counterpartyVersion) + if !slices.Contains(types.SupportedVersions, counterpartyVersion) { + return errorsmod.Wrapf(types.ErrInvalidVersion, "invalid counterparty version: expected one of %s, got %s", types.SupportedVersions, counterpartyVersion) } + return nil } @@ -315,8 +318,8 @@ func (im IBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, return "", err } - if proposedVersion != types.Version { - return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, proposedVersion) + if !slices.Contains(types.SupportedVersions, proposedVersion) { + return "", errorsmod.Wrapf(types.ErrInvalidVersion, "invalid counterparty version: expected one of %s, got %s", types.SupportedVersions, proposedVersion) } return proposedVersion, nil @@ -328,8 +331,8 @@ func (im IBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, return "", err } - if counterpartyVersion != types.Version { - return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, counterpartyVersion) + if !slices.Contains(types.SupportedVersions, counterpartyVersion) { + return types.Version, nil } return counterpartyVersion, nil @@ -337,8 +340,8 @@ func (im IBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, // OnChanUpgradeAck implements the IBCModule interface func (IBCModule) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion string) error { - if counterpartyVersion != types.Version { - return errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, counterpartyVersion) + if !slices.Contains(types.SupportedVersions, counterpartyVersion) { + return errorsmod.Wrapf(types.ErrInvalidVersion, "invalid counterparty version: expected one of %s, got %s", types.SupportedVersions, counterpartyVersion) } return nil diff --git a/modules/apps/transfer/ibc_module_test.go b/modules/apps/transfer/ibc_module_test.go index ad4b0be3861..f27505c3a44 100644 --- a/modules/apps/transfer/ibc_module_test.go +++ b/modules/apps/transfer/ibc_module_test.go @@ -25,50 +25,56 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() { ) testCases := []struct { - name string - malleate func() - expPass bool + name string + malleate func() + expPass bool + expVersion string }{ { - "success", func() {}, true, + "success", func() {}, true, types.Version, }, { // connection hops is not used in the transfer application callback, // it is already validated in the core OnChanUpgradeInit. "success: invalid connection hops", func() { path.EndpointA.ConnectionID = "invalid-connection-id" - }, true, + }, true, types.Version, }, { - "empty version string", func() { + "success: empty version string", func() { channel.Version = "" - }, true, + }, true, types.Version, + }, + { + "success: ics20-1 legacy", func() { + channel.Version = types.Version1 + }, true, types.Version1, }, { "max channels reached", func() { path.EndpointA.ChannelID = channeltypes.FormatChannelIdentifier(math.MaxUint32 + 1) - }, false, + }, false, "", }, { "invalid order - ORDERED", func() { channel.Ordering = channeltypes.ORDERED - }, false, + }, false, "", }, { "invalid port ID", func() { path.EndpointA.ChannelConfig.PortID = ibctesting.MockPort - }, false, + }, false, "", }, { "invalid version", func() { channel.Version = "version" //nolint:goconst - }, false, + }, false, "", }, { "capability already claimed", func() { err := suite.chainA.GetSimApp().ScopedTransferKeeper.ClaimCapability(suite.chainA.GetContext(), chanCap, host.ChannelCapabilityPath(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)) suite.Require().NoError(err) - }, false, + }, false, "", }, } @@ -103,7 +109,7 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() { if tc.expPass { suite.Require().NoError(err) - suite.Require().Equal(types.Version, version) + suite.Require().Equal(tc.expVersion, version) } else { suite.Require().Error(err) suite.Require().Equal(version, "") @@ -122,38 +128,44 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() { ) testCases := []struct { - name string - malleate func() - expPass bool + name string + malleate func() + expPass bool + expVersion string }{ { - "success", func() {}, true, + "success", func() {}, true, types.Version, + }, + { + "success: counterparty version is legacy ics20-1", func() { + counterpartyVersion = types.Version1 + }, true, types.Version1, + }, + { + "success: invalid counterparty version, we use our proposed version", func() { + counterpartyVersion = "version" + }, true, types.Version, }, { "max channels reached", func() { path.EndpointA.ChannelID = channeltypes.FormatChannelIdentifier(math.MaxUint32 + 1) - }, false, + }, false, "", }, { "capability already claimed", func() { err := suite.chainA.GetSimApp().ScopedTransferKeeper.ClaimCapability(suite.chainA.GetContext(), chanCap, host.ChannelCapabilityPath(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)) suite.Require().NoError(err) - }, false, + }, false, "", }, { "invalid order - ORDERED", func() { channel.Ordering = channeltypes.ORDERED - }, false, + }, false, "", }, { "invalid port ID", func() { path.EndpointA.ChannelConfig.PortID = ibctesting.MockPort - }, false, - }, - { - "invalid counterparty version", func() { - counterpartyVersion = "version" - }, false, + }, false, "", }, } @@ -194,7 +206,7 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() { if tc.expPass { suite.Require().NoError(err) - suite.Require().Equal(types.Version, version) + suite.Require().Equal(tc.expVersion, version) } else { suite.Require().Error(err) suite.Require().Equal("", version) @@ -339,18 +351,18 @@ func (suite *TransferTestSuite) TestOnChanUpgradeTry() { nil, }, { - "invalid upgrade ordering", + "success: invalid upgrade version from counterparty, we use our proposed version", func() { - counterpartyUpgrade.Fields.Ordering = channeltypes.ORDERED + counterpartyUpgrade.Fields.Version = ibctesting.InvalidID }, - channeltypes.ErrInvalidChannelOrdering, + nil, }, { - "invalid upgrade version", + "invalid upgrade ordering", func() { - counterpartyUpgrade.Fields.Version = ibctesting.InvalidID + counterpartyUpgrade.Fields.Ordering = channeltypes.ORDERED }, - types.ErrInvalidVersion, + channeltypes.ErrInvalidChannelOrdering, }, } diff --git a/modules/apps/transfer/types/keys.go b/modules/apps/transfer/types/keys.go index 8db04bb87bb..d985438ab71 100644 --- a/modules/apps/transfer/types/keys.go +++ b/modules/apps/transfer/types/keys.go @@ -51,6 +51,9 @@ var ( PortKey = []byte{0x01} // DenomTraceKey defines the key to store the denomination trace info in store DenomTraceKey = []byte{0x02} + + // SupportedVersions defines all versions that are supported by the module + SupportedVersions = []string{Version, Version1} ) // GetEscrowAddress returns the escrow address for the specified channel.