Skip to content

Commit

Permalink
Merge branch 'main' into colin/1323-emit-events
Browse files Browse the repository at this point in the history
  • Loading branch information
colin-axner authored May 11, 2022
2 parents 197150f + dcd0681 commit 834e401
Show file tree
Hide file tree
Showing 15 changed files with 151 additions and 57 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### API Breaking

* (transfer) [\#1250](https://github.com/cosmos/ibc-go/pull/1250) Deprecate `GetTransferAccount` since the `transfer` module account is never used.
* (channel) [\#1283](https://github.com/cosmos/ibc-go/pull/1283) The `OnChanOpenInit` application callback now returns a version string in line with the latest [spec changes](https://github.com/cosmos/ibc/pull/629).
* (modules/29-fee)[\#1338](https://github.com/cosmos/ibc-go/pull/1338) Renaming `Result` field in `IncentivizedAcknowledgement` to `AppAcknowledgement`.
* (modules/29-fee)[\#1343](https://github.com/cosmos/ibc-go/pull/1343) Renaming `KeyForwardRelayerAddress` to `KeyRelayerAddressForAsyncAck`, and `ParseKeyForwardRelayerAddress` to `ParseKeyRelayerAddressForAsyncAck`.

Expand Down
6 changes: 5 additions & 1 deletion docs/migrations/v3-to-v4.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Migrating from ibc-go v2 to v3
# Migrating from ibc-go v3 to v4

This document is intended to highlight significant changes which may require more information than presented in the CHANGELOG.
Any changes that must be done by a user of ibc-go should be documented here.
Expand All @@ -23,4 +23,8 @@ No genesis or in-place migrations required when upgrading from v1 or v2 of ibc-g
The `WriteAcknowledgement` API now takes the `exported.Acknowledgement` type instead of passing in the acknowledgement byte array directly.
This is an API breaking change and as such IBC application developers will have to update any calls to `WriteAcknowledgement`.

The `OnChanOpenInit` application callback has been modified.
The return signature now includes the application version as detailed in the latest IBC [spec changes](https://github.com/cosmos/ibc/pull/629).



17 changes: 11 additions & 6 deletions modules/apps/27-interchain-accounts/controller/ibc_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,23 @@ func (im IBCMiddleware) OnChanOpenInit(
chanCap *capabilitytypes.Capability,
counterparty channeltypes.Counterparty,
version string,
) error {
) (string, error) {
if !im.keeper.IsControllerEnabled(ctx) {
return types.ErrControllerSubModuleDisabled
return "", types.ErrControllerSubModuleDisabled
}

if err := im.keeper.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version); err != nil {
return err
return "", err
}

// call underlying app's OnChanOpenInit callback with the passed in version
// the version returned is discarded as the ica-auth module does not have permission to edit the version string.
// ics27 will always return the version string containing the Metadata struct which is created during the `RegisterInterchainAccount` call.
if _, err := im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version); err != nil {
return "", err
}

// call underlying app's OnChanOpenInit callback with the appVersion
return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID,
chanCap, counterparty, version)
return version, nil
}

// OnChanOpenTry implements the IBCMiddleware interface
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() {
suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnChanOpenInit = func(ctx sdk.Context, order channeltypes.Order, connectionHops []string,
portID, channelID string, chanCap *capabilitytypes.Capability,
counterparty channeltypes.Counterparty, version string,
) error {
return fmt.Errorf("mock ica auth fails")
) (string, error) {
return "", fmt.Errorf("mock ica auth fails")
}
}, false,
},
Expand Down Expand Up @@ -197,11 +197,24 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() {
cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module)
suite.Require().True(ok)

err = cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(),
version, err := cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(),
path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, channel.Counterparty, channel.GetVersion(),
)

if tc.expPass {
expMetadata := icatypes.NewMetadata(
icatypes.Version,
path.EndpointA.ConnectionID,
path.EndpointB.ConnectionID,
"",
icatypes.EncodingProtobuf,
icatypes.TxTypeSDKMultiMsg,
)

expBytes, err := icatypes.ModuleCdc.MarshalJSON(&expMetadata)
suite.Require().NoError(err)

suite.Require().Equal(version, string(expBytes))
suite.Require().NoError(err)
} else {
suite.Require().Error(err)
Expand Down
4 changes: 2 additions & 2 deletions modules/apps/27-interchain-accounts/host/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ func (im IBCModule) OnChanOpenInit(
chanCap *capabilitytypes.Capability,
counterparty channeltypes.Counterparty,
version string,
) error {
return sdkerrors.Wrap(icatypes.ErrInvalidChannelFlow, "channel handshake must be initiated by controller chain")
) (string, error) {
return "", sdkerrors.Wrap(icatypes.ErrInvalidChannelFlow, "channel handshake must be initiated by controller chain")
}

// OnChanOpenTry implements the IBCModule interface
Expand Down
44 changes: 32 additions & 12 deletions modules/apps/29-fee/ibc_middleware.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package fee

import (
"strings"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
Expand Down Expand Up @@ -39,25 +41,44 @@ func (im IBCMiddleware) OnChanOpenInit(
chanCap *capabilitytypes.Capability,
counterparty channeltypes.Counterparty,
version string,
) error {
) (string, error) {
var versionMetadata types.Metadata
if err := types.ModuleCdc.UnmarshalJSON([]byte(version), &versionMetadata); err != nil {
// Since it is valid for fee version to not be specified, the above middleware version may be for a middleware
// lower down in the stack. Thus, if it is not a fee version we pass the entire version string onto the underlying
// application.
return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID,
chanCap, counterparty, version)

if strings.TrimSpace(version) == "" {
// default version
versionMetadata = types.Metadata{
FeeVersion: types.Version,
AppVersion: "",
}
} else {
if err := types.ModuleCdc.UnmarshalJSON([]byte(version), &versionMetadata); err != nil {
// Since it is valid for fee version to not be specified, the above middleware version may be for a middleware
// lower down in the stack. Thus, if it is not a fee version we pass the entire version string onto the underlying
// application.
return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID,
chanCap, counterparty, version)
}
}

if versionMetadata.FeeVersion != types.Version {
return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, versionMetadata.FeeVersion)
return "", sdkerrors.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, versionMetadata.FeeVersion)
}

appVersion, err := im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, versionMetadata.AppVersion)
if err != nil {
return "", err
}

versionMetadata.AppVersion = appVersion
versionBytes, err := types.ModuleCdc.MarshalJSON(&versionMetadata)
if err != nil {
return "", err
}

im.keeper.SetFeeEnabled(ctx, portID, channelID)

// call underlying app's OnChanOpenInit callback with the appVersion
return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID,
chanCap, counterparty, versionMetadata.AppVersion)
return string(versionBytes), nil
}

// OnChanOpenTry implements the IBCMiddleware interface
Expand Down Expand Up @@ -94,7 +115,6 @@ func (im IBCMiddleware) OnChanOpenTry(
}

versionMetadata.AppVersion = appVersion

versionBytes, err := types.ModuleCdc.MarshalJSON(&versionMetadata)
if err != nil {
return "", err
Expand All @@ -116,7 +136,7 @@ func (im IBCMiddleware) OnChanOpenAck(
if im.keeper.IsFeeEnabled(ctx, portID, channelID) {
var versionMetadata types.Metadata
if err := types.ModuleCdc.UnmarshalJSON([]byte(counterpartyVersion), &versionMetadata); err != nil {
return sdkerrors.Wrap(types.ErrInvalidVersion, "failed to unmarshal ICS29 counterparty version metadata")
return sdkerrors.Wrapf(err, "failed to unmarshal ICS29 counterparty version metadata: %s", counterpartyVersion)
}

if versionMetadata.FeeVersion != types.Version {
Expand Down
42 changes: 35 additions & 7 deletions modules/apps/29-fee/ibc_middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,34 +25,46 @@ var (
// Tests OnChanOpenInit on ChainA
func (suite *FeeTestSuite) TestOnChanOpenInit() {
testCases := []struct {
name string
version string
expPass bool
name string
version string
expPass bool
isFeeEnabled bool
}{
{
"success - valid fee middleware and mock version",
string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: ibcmock.Version})),
true,
true,
},
{
"success - fee version not included, only perform mock logic",
ibcmock.Version,
true,
false,
},
{
"invalid fee middleware version",
string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: "invalid-ics29-1", AppVersion: ibcmock.Version})),
false,
false,
},
{
"invalid mock version",
string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: "invalid-mock-version"})),
false,
false,
},
{
"mock version not wrapped",
types.Version,
false,
false,
},
{
"passing an empty string returns default version",
"",
true,
true,
},
}

Expand All @@ -68,11 +80,11 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() {
suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnChanOpenInit = func(ctx sdk.Context, order channeltypes.Order, connectionHops []string,
portID, channelID string, chanCap *capabilitytypes.Capability,
counterparty channeltypes.Counterparty, version string,
) error {
) (string, error) {
if version != ibcmock.Version {
return fmt.Errorf("incorrect mock version")
return "", fmt.Errorf("incorrect mock version")
}
return nil
return ibcmock.Version, nil
}

suite.path.EndpointA.ChannelID = ibctesting.FirstChannelID
Expand All @@ -95,13 +107,29 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() {
cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module)
suite.Require().True(ok)

err = cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(),
version, err := cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(),
suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, chanCap, counterparty, channel.Version)

if tc.expPass {
// check if the channel is fee enabled. If so version string should include metaData
if tc.isFeeEnabled {
versionMetadata := types.Metadata{
FeeVersion: types.Version,
AppVersion: ibcmock.Version,
}

versionBytes, err := types.ModuleCdc.MarshalJSON(&versionMetadata)
suite.Require().NoError(err)

suite.Require().Equal(version, string(versionBytes))
} else {
suite.Require().Equal(ibcmock.Version, version)
}

suite.Require().NoError(err, "unexpected error from version: %s", tc.version)
} else {
suite.Require().Error(err, "error not returned for version: %s", tc.version)
suite.Require().Equal("", version)
}
})
}
Expand Down
1 change: 0 additions & 1 deletion modules/apps/29-fee/transfer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,4 @@ func (suite *FeeTestSuite) TestFeeTransfer() {
fee.AckFee.Add(fee.TimeoutFee...), // ack fee paid, timeout fee refunded
sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)).Sub(originalChainASenderAccountBalance),
)

}
15 changes: 10 additions & 5 deletions modules/apps/transfer/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package transfer
import (
"fmt"
"math"
"strings"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
Expand Down Expand Up @@ -70,21 +71,25 @@ func (im IBCModule) OnChanOpenInit(
chanCap *capabilitytypes.Capability,
counterparty channeltypes.Counterparty,
version string,
) error {
) (string, error) {
if err := ValidateTransferChannelParams(ctx, im.keeper, order, portID, channelID); err != nil {
return err
return "", err
}

if strings.TrimSpace(version) == "" {
version = types.Version
}

if version != types.Version {
return sdkerrors.Wrapf(types.ErrInvalidVersion, "got %s, expected %s", version, types.Version)
return "", sdkerrors.Wrapf(types.ErrInvalidVersion, "got %s, expected %s", version, types.Version)
}

// Claim channel capability passed back by IBC module
if err := im.keeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil {
return err
return "", err
}

return nil
return version, nil
}

// OnChanOpenTry implements the IBCModule interface.
Expand Down
18 changes: 11 additions & 7 deletions modules/apps/transfer/ibc_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"

"github.com/cosmos/ibc-go/v3/modules/apps/transfer"
"github.com/cosmos/ibc-go/v3/modules/apps/transfer/types"
channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v3/modules/core/24-host"
Expand All @@ -28,6 +29,11 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() {
{
"success", func() {}, true,
},
{
"empty version string", func() {
channel.Version = ""
}, true,
},
{
"max channels reached", func() {
path.EndpointA.ChannelID = channeltypes.FormatChannelIdentifier(math.MaxUint32 + 1)
Expand Down Expand Up @@ -74,25 +80,23 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() {
Version: types.Version,
}

module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort)
suite.Require().NoError(err)

var err error
chanCap, err = suite.chainA.App.GetScopedIBCKeeper().NewCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(ibctesting.TransferPort, path.EndpointA.ChannelID))
suite.Require().NoError(err)

cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module)
suite.Require().True(ok)

tc.malleate() // explicitly change fields in channel and testChannel

err = cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(),
transferModule := transfer.NewIBCModule(suite.chainA.GetSimApp().TransferKeeper)
version, err := transferModule.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(),
path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, counterparty, channel.GetVersion(),
)

if tc.expPass {
suite.Require().NoError(err)
suite.Require().Equal(types.Version, version)
} else {
suite.Require().Error(err)
suite.Require().Equal(version, "")
}

})
Expand Down
Loading

0 comments on commit 834e401

Please sign in to comment.