Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add GetAppVersion to ICS4Wrapper #1022

Merged
merged 18 commits into from
Apr 20, 2022
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

* (middleware) [\#1022](https://github.com/cosmos/ibc-go/pull/1022) Add `GetAppVersion` to the ICS4Wrapper interface. This function should be used by IBC applications to obtain their own version since the version set in the channel structure may be wrapped many times by middleware.
* (modules/core/04-channel) [\#1160](https://github.com/cosmos/ibc-go/pull/1160) Improve `uint64 -> string` performance in `Logger`.
* (modules/core/04-channel) [\#1232](https://github.com/cosmos/ibc-go/pull/1232) Updating params on `NewPacketId` and moving to bottom of file.

Expand Down
21 changes: 21 additions & 0 deletions docs/ibc/middleware/develop.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type Middleware interface {
type ICS4Wrapper interface {
SendPacket(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet exported.Packet) error
WriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet exported.Packet, ack []byte) error
GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool)
}
```

Expand Down Expand Up @@ -239,4 +240,24 @@ func SendPacket(appPacket channeltypes.Packet) {

return ics4Keeper.SendPacket(packet)
}

// middleware must return the underlying application version
func GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
version, found := ics4Keeper.GetAppVersion(ctx, portID, channelID)
if !found {
return "", false
}

if !MiddlewareEnabled {
return version, true
}

// unwrap channel version
metadata, err := Unmarshal(version)
if err != nil {
panic(fmt.Errof("unable to unmarshal version: %w", err))
}

return metadata.AppVersion, true
}
```
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,8 @@ func (im IBCModule) OnTimeoutPacket(

return im.app.OnTimeoutPacket(ctx, packet, relayer)
}

// GetAppVersion returns the interchain accounts metadata.
func (im IBCModule) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
return im.keeper.GetAppVersion(ctx, portID, channelID)
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/stretchr/testify/suite"
"github.com/tendermint/tendermint/crypto"

icacontroller "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/controller"
"github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/controller/types"
icatypes "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/types"
clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
Expand Down Expand Up @@ -702,3 +703,23 @@ func (suite *InterchainAccountsTestSuite) TestSingleHostMultipleControllers() {
})
}
}

func (suite *InterchainAccountsTestSuite) TestGetAppVersion() {
path := NewICAPath(suite.chainA, suite.chainB)
suite.coordinator.SetupConnections(path)

err := SetupICAPath(path, TestOwnerAddress)
suite.Require().NoError(err)

module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID)
suite.Require().NoError(err)

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

controllerModule := cbs.(icacontroller.IBCModule)

appVersion, found := controllerModule.GetAppVersion(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
suite.Require().True(found)
suite.Require().Equal(path.EndpointA.ChannelConfig.Version, appVersion)
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ func (k Keeper) ClaimCapability(ctx sdk.Context, cap *capabilitytypes.Capability
return k.scopedKeeper.ClaimCapability(ctx, cap, name)
}

// GetAppVersion calls the ICS4Wrapper GetAppVersion function.
func (k Keeper) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
return k.ics4Wrapper.GetAppVersion(ctx, portID, channelID)
}

// GetActiveChannelID retrieves the active channelID from the store, keyed by the provided connectionID and portID
func (k Keeper) GetActiveChannelID(ctx sdk.Context, connectionID, portID string) (string, bool) {
store := ctx.KVStore(k.storeKey)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type AccountKeeper interface {
// ICS4Wrapper defines the expected ICS4Wrapper for middleware
type ICS4Wrapper interface {
SendPacket(ctx sdk.Context, channelCap *capabilitytypes.Capability, packet ibcexported.PacketI) error
GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool)
}

// ChannelKeeper defines the expected IBC channel keeper
Expand Down
5 changes: 5 additions & 0 deletions modules/apps/29-fee/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,3 +267,8 @@ func (im IBCModule) OnTimeoutPacket(
// call underlying callback
return im.app.OnTimeoutPacket(ctx, packet, relayer)
}

// GetAppVersion returns the application version of the underlying application
func (im IBCModule) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
return im.keeper.GetAppVersion(ctx, portID, channelID)
}
77 changes: 77 additions & 0 deletions modules/apps/29-fee/ibc_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"

fee "github.com/cosmos/ibc-go/v3/modules/apps/29-fee"
"github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types"
transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types"
channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
Expand Down Expand Up @@ -825,3 +826,79 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
})
}
}

func (suite *FeeTestSuite) TestGetAppVersion() {
var (
portID string
channelID string
expAppVersion string
)
testCases := []struct {
name string
malleate func()
expFound bool
}{
{
"success for fee enabled channel",
func() {
expAppVersion = ibcmock.Version
},
true,
},
{
"success for non fee enabled channel",
func() {
path := ibctesting.NewPath(suite.chainA, suite.chainB)
path.EndpointA.ChannelConfig.PortID = ibctesting.MockFeePort
path.EndpointB.ChannelConfig.PortID = ibctesting.MockFeePort
// by default a new path uses a non fee channel

colin-axner marked this conversation as resolved.
Show resolved Hide resolved
suite.coordinator.Setup(path)
portID = path.EndpointA.ChannelConfig.PortID
channelID = path.EndpointA.ChannelID

expAppVersion = ibcmock.Version
},
true,
},
{
"channel does not exist",
func() {
portID = "does not exist"
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
},
false,
},
}

for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
suite.SetupTest()
suite.coordinator.Setup(suite.path)

portID = suite.path.EndpointA.ChannelConfig.PortID
channelID = suite.path.EndpointA.ChannelID

// malleate test case
tc.malleate()

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

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

feeModule := cbs.(fee.IBCModule)

appVersion, found := feeModule.GetAppVersion(suite.chainA.GetContext(), portID, channelID)

if tc.expFound {
suite.Require().True(found)
suite.Require().Equal(expAppVersion, appVersion)
} else {
suite.Require().False(found)
suite.Require().Empty(appVersion)
}
})
}
}
21 changes: 21 additions & 0 deletions modules/apps/29-fee/keeper/relay.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package keeper

import (
"fmt"

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 @@ -42,3 +44,22 @@ func (k Keeper) WriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.C
// ics4Wrapper may be core IBC or higher-level middleware
return k.ics4Wrapper.WriteAcknowledgement(ctx, chanCap, packet, ack)
}

// GetAppVersion returns the underlying application version.
func (k Keeper) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
version, found := k.ics4Wrapper.GetAppVersion(ctx, portID, channelID)
if !found {
return "", false
}

if !k.IsFeeEnabled(ctx, portID, channelID) {
return version, true
}

var metadata types.Metadata
if err := types.ModuleCdc.UnmarshalJSON([]byte(version), &metadata); err != nil {
panic(fmt.Errorf("unable to unmarshal metadata for fee enabled channel: %w", err))
}

return metadata.AppVersion, true
}
70 changes: 70 additions & 0 deletions modules/apps/29-fee/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types"
clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
ibctesting "github.com/cosmos/ibc-go/v3/testing"
ibcmock "github.com/cosmos/ibc-go/v3/testing/mock"
)

func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() {
Expand Down Expand Up @@ -101,3 +103,71 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsyncFeeDisabled() {
packetAck, _ := suite.chainB.GetSimApp().GetIBCKeeper().ChannelKeeper.GetPacketAcknowledgement(suite.chainB.GetContext(), packet.DestinationPort, packet.DestinationChannel, 1)
suite.Require().Equal(packetAck, channeltypes.CommitAcknowledgement(ack.Acknowledgement()))
}

func (suite *KeeperTestSuite) TestGetAppVersion() {
var (
portID string
channelID string
expAppVersion string
)
testCases := []struct {
name string
malleate func()
expFound bool
}{
{
"success for fee enabled channel",
func() {
expAppVersion = ibcmock.Version
},
true,
},
{
"success for non fee enabled channel",
func() {
path := ibctesting.NewPath(suite.chainA, suite.chainB)
path.EndpointA.ChannelConfig.PortID = ibctesting.MockFeePort
path.EndpointB.ChannelConfig.PortID = ibctesting.MockFeePort
// by default a new path uses a non fee channel

colin-axner marked this conversation as resolved.
Show resolved Hide resolved
suite.coordinator.Setup(path)
portID = path.EndpointA.ChannelConfig.PortID
channelID = path.EndpointA.ChannelID

expAppVersion = ibcmock.Version
},
true,
},
{
"channel does not exist",
func() {
portID = "does not exist"
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
},
false,
},
}

for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
suite.SetupTest()
suite.coordinator.Setup(suite.path)

portID = suite.path.EndpointA.ChannelConfig.PortID
channelID = suite.path.EndpointA.ChannelID

// malleate test case
tc.malleate()

appVersion, found := suite.chainA.GetSimApp().IBCFeeKeeper.GetAppVersion(suite.chainA.GetContext(), portID, channelID)

if tc.expFound {
suite.Require().True(found)
suite.Require().Equal(expAppVersion, appVersion)
} else {
suite.Require().False(found)
suite.Require().Empty(appVersion)
}
})
}
}
1 change: 1 addition & 0 deletions modules/apps/29-fee/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type AccountKeeper interface {
type ICS4Wrapper interface {
WriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI, acknowledgement ibcexported.Acknowledgement) error
SendPacket(ctx sdk.Context, channelCap *capabilitytypes.Capability, packet ibcexported.PacketI) error
GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool)
}

// ChannelKeeper defines the expected IBC channel keeper
Expand Down
10 changes: 10 additions & 0 deletions modules/core/04-channel/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,16 @@ func (k Keeper) SetChannel(ctx sdk.Context, portID, channelID string, channel ty
store.Set(host.ChannelKey(portID, channelID), bz)
}

// GetAppVersion gets the version for the specified channel.
func (k Keeper) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
channel, found := k.GetChannel(ctx, portID, channelID)
if !found {
return "", false
}

return channel.Version, true
}

// GetNextChannelSequence gets the next channel sequence from the store.
func (k Keeper) GetNextChannelSequence(ctx sdk.Context) uint64 {
store := ctx.KVStore(k.storeKey)
Expand Down
19 changes: 19 additions & 0 deletions modules/core/04-channel/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
ibctesting "github.com/cosmos/ibc-go/v3/testing"
ibcmock "github.com/cosmos/ibc-go/v3/testing/mock"
)

// KeeperTestSuite is a testing suite to test keeper functions.
Expand Down Expand Up @@ -62,6 +63,24 @@ func (suite *KeeperTestSuite) TestSetChannel() {
suite.Equal(expectedCounterparty, storedChannel.Counterparty)
}

func (suite *KeeperTestSuite) TestGetAppVersion() {
// create client and connections on both chains
path := ibctesting.NewPath(suite.chainA, suite.chainB)
suite.coordinator.SetupConnections(path)

version, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetAppVersion(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
suite.Require().False(found)
suite.Require().Empty(version)

// init channel
err := path.EndpointA.ChanOpenInit()
suite.NoError(err)

channelVersion, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetAppVersion(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
suite.Require().True(found)
suite.Require().Equal(ibcmock.Version, channelVersion)
}

// TestGetAllChannels creates multiple channels on chain A through various connections
// and tests their retrieval. 2 channels are on connA0 and 1 channel is on connA1
func (suite KeeperTestSuite) TestGetAllChannels() {
Expand Down
6 changes: 6 additions & 0 deletions modules/core/05-port/types/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,12 @@ type ICS4Wrapper interface {
packet exported.PacketI,
ack exported.Acknowledgement,
) error

GetAppVersion(
ctx sdk.Context,
portID,
channelID string,
) (string, bool)
}

// Middleware must implement IBCModule to wrap communication from core IBC to underlying application
Expand Down
9 changes: 8 additions & 1 deletion testing/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,14 @@ func (endpoint *Endpoint) ChanOpenAck() error {
proof, height,
endpoint.Chain.SenderAccount.GetAddress().String(),
)
return endpoint.Chain.sendMsgs(msg)

if err = endpoint.Chain.sendMsgs(msg); err != nil {
return err
}

endpoint.ChannelConfig.Version = endpoint.GetChannel().Version

return nil
}

// ChanOpenConfirm will construct and execute a MsgChannelOpenConfirm on the associated endpoint.
Expand Down