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: scalable version queries #384

Merged
merged 14 commits into from
Sep 13, 2021
Merged
Show file tree
Hide file tree
Changes from 9 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 @@ -49,6 +49,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Features
* [\#372](https://github.com/cosmos/ibc-go/pull/372) New CLI command `query ibc client status <client id>` to get the current activity status of a client
* [\#384](https://github.com/cosmos/ibc-go/pull/384) Added `NegotiateAppVersion` method to `IBCModule` interface supported by a gRPC query service in `05-port`. This provides routing of requests to the desired application module callback, which in turn performs application version negotiation.

## [v1.0.0](https://github.com/cosmos/ibc-go/releases/tag/v1.0.0) - 2021-08-10

Expand Down
65 changes: 65 additions & 0 deletions docs/ibc/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,12 @@

- [Msg](#ibc.core.connection.v1.Msg)

- [ibc/core/port/v1/query.proto](#ibc/core/port/v1/query.proto)
- [NegotiateAppVersionRequest](#ibc.core.port.v1.NegotiateAppVersionRequest)
- [NegotiateAppVersionResponse](#ibc.core.port.v1.NegotiateAppVersionResponse)

- [Query](#ibc.core.port.v1.Query)

- [ibc/core/types/v1/genesis.proto](#ibc/core/types/v1/genesis.proto)
- [GenesisState](#ibc.core.types.v1.GenesisState)

Expand Down Expand Up @@ -2874,6 +2880,65 @@ Msg defines the ibc/connection Msg service.



<a name="ibc/core/port/v1/query.proto"></a>
<p align="right"><a href="#top">Top</a></p>

## ibc/core/port/v1/query.proto



<a name="ibc.core.port.v1.NegotiateAppVersionRequest"></a>

### NegotiateAppVersionRequest
NegotiateAppVersionRequest is the request type for the Query/NegotiateAppVersion RPC method


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `port_id` | [string](#string) | | port unique identifier |
| `counterparty` | [ibc.core.channel.v1.Counterparty](#ibc.core.channel.v1.Counterparty) | | counterparty channel end |
| `proposed_version` | [string](#string) | | proposed version |






<a name="ibc.core.port.v1.NegotiateAppVersionResponse"></a>

### NegotiateAppVersionResponse
NegotiateAppVersionResponse is the response type for the Query/NegotiateAppVersion RPC method.


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `port_id` | [string](#string) | | port id associated with the request identifiers |
| `version` | [string](#string) | | supported app version |





<!-- end messages -->

<!-- end enums -->

<!-- end HasExtensions -->


<a name="ibc.core.port.v1.Query"></a>

### Query
Query defines the gRPC querier service

| Method Name | Request Type | Response Type | Description | HTTP Verb | Endpoint |
| ----------- | ------------ | ------------- | ------------| ------- | -------- |
| `NegotiateAppVersion` | [NegotiateAppVersionRequest](#ibc.core.port.v1.NegotiateAppVersionRequest) | [NegotiateAppVersionResponse](#ibc.core.port.v1.NegotiateAppVersionResponse) | NegotiateAppVersion queries an IBC Port and determines the appropriate application version to be used | |

<!-- end services -->



<a name="ibc/core/types/v1/genesis.proto"></a>
<p align="right"><a href="#top">Top</a></p>

Expand Down
14 changes: 14 additions & 0 deletions modules/apps/transfer/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -434,3 +434,17 @@ func (am AppModule) OnTimeoutPacket(

return nil
}

// NegotiateAppVersion implements the IBCModule interface
func (am AppModule) NegotiateAppVersion(
ctx sdk.Context,
portID string,
counterparty channeltypes.Counterparty,
proposedVersion string,
) (version string, err error) {
if proposedVersion != types.Version {
return "", sdkerrors.Wrapf(types.ErrInvalidVersion, "failed to negotiate app version: got: %s, expected %s", proposedVersion, types.Version)
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
}

return types.Version, nil
}
2 changes: 1 addition & 1 deletion modules/apps/transfer/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

// IBC channel sentinel errors
// IBC transfer sentinel errors
var (
ErrInvalidPacketTimeout = sdkerrors.Register(ModuleName, 2, "invalid packet timeout")
ErrInvalidDenomForTransfer = sdkerrors.Register(ModuleName, 3, "invalid denomination for cross-chain transfer")
Expand Down
24 changes: 24 additions & 0 deletions modules/core/05-port/client/cli/cli.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package cli

import (
"github.com/spf13/cobra"

"github.com/cosmos/cosmos-sdk/client"

"github.com/cosmos/ibc-go/modules/core/05-port/types"
)

// GetQueryCmd returns the query commands for IBC ports
func GetQueryCmd() *cobra.Command {
queryCmd := &cobra.Command{
Use: types.SubModuleName,
Short: "IBC port query subcommands",
DisableFlagParsing: true,
SuggestionsMinimumDistance: 2,
RunE: client.ValidateCmd,
}

queryCmd.AddCommand()

return queryCmd
}
52 changes: 52 additions & 0 deletions modules/core/05-port/keeper/grpc_query.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package keeper

import (
"context"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"github.com/cosmos/ibc-go/modules/core/05-port/types"
host "github.com/cosmos/ibc-go/modules/core/24-host"
)

var _ types.QueryServer = (*Keeper)(nil)

// NegotiateAppVersion implements the Query/NegotiateAppVersion gRPC method
func (q Keeper) NegotiateAppVersion(c context.Context, req *types.NegotiateAppVersionRequest) (*types.NegotiateAppVersionResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "empty request")
}

if err := validategRPCRequest(req.PortId); err != nil {
return nil, err
}

ctx := sdk.UnwrapSDKContext(c)
module, _, err := q.LookupModuleByPort(ctx, req.PortId)
if err != nil {
return nil, status.Errorf(codes.NotFound, sdkerrors.Wrap(err, "could not retrieve module from port-id").Error())
}

ibcModule, found := q.Router.GetRoute(module)
if !found {
return nil, status.Errorf(codes.NotFound, sdkerrors.Wrapf(types.ErrInvalidRoute, "route not found to module: %s", module).Error())
}

version, err := ibcModule.NegotiateAppVersion(ctx, req.PortId, *req.Counterparty, req.ProposedVersion)
if err != nil {
return nil, status.Errorf(codes.InvalidArgument, sdkerrors.Wrap(err, "version negotation failed").Error())
}

return types.NewNegotiateAppVersionResponse(req.PortId, version), nil
}

func validategRPCRequest(portID string) error {
if err := host.PortIdentifierValidator(portID); err != nil {
return status.Error(codes.InvalidArgument, err.Error())
}

return nil
}
103 changes: 103 additions & 0 deletions modules/core/05-port/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package keeper_test

import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"

channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types"
"github.com/cosmos/ibc-go/modules/core/05-port/types"
"github.com/cosmos/ibc-go/testing/mock"
)

func (suite *KeeperTestSuite) TestNegotiateAppVersion() {
var (
req *types.NegotiateAppVersionRequest
expVersion string
)

testCases := []struct {
msg string
malleate func()
expPass bool
}{
{
"empty request",
func() {
req = nil
},
false,
},
{
"invalid port ID",
func() {
req = &types.NegotiateAppVersionRequest{
PortId: "",
}
},
false,
},
{
"module not found",
func() {
req = &types.NegotiateAppVersionRequest{
PortId: "mock-port-id",
}
},
false,
},
{
"version negotiation failure",
func() {

expVersion = mock.Version

req = &types.NegotiateAppVersionRequest{
PortId: "mock", // retrieves the mock testing module
Counterparty: &channeltypes.Counterparty{
PortId: "mock-port-id",
ChannelId: "mock-channel-id",
},
ProposedVersion: "invalid-proposed-version",
}
},
false,
},
{
"success",
func() {

expVersion = mock.Version

req = &types.NegotiateAppVersionRequest{
PortId: "mock", // retrieves the mock testing module
Counterparty: &channeltypes.Counterparty{
PortId: "mock-port-id",
ChannelId: "mock-channel-id",
},
ProposedVersion: mock.Version,
}
},
true,
},
}

for _, tc := range testCases {
suite.Run(fmt.Sprintf("Case %s", tc.msg), func() {
suite.SetupTest() // reset

tc.malleate()

ctx := sdk.WrapSDKContext(suite.ctx)
res, err := suite.keeper.NegotiateAppVersion(ctx, req)

if tc.expPass {
suite.Require().NoError(err)
suite.Require().NotNil(res)
suite.Require().Equal(expVersion, res.Version)
} else {
suite.Require().Error(err)
}
})
}
}
2 changes: 2 additions & 0 deletions modules/core/05-port/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (

// Keeper defines the IBC connection keeper
type Keeper struct {
Router *types.Router

scopedKeeper capabilitykeeper.ScopedKeeper
}

Expand Down
24 changes: 24 additions & 0 deletions modules/core/05-port/module.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package port

import (
"github.com/gogo/protobuf/grpc"
"github.com/spf13/cobra"

"github.com/cosmos/ibc-go/modules/core/05-port/types"
"github.com/cosmos/ibc-go/modules/core/client/cli"
)

// Name returns the IBC port ICS name.
func Name() string {
return types.SubModuleName
}

// GetQueryCmd returns the root query command for IBC ports.
func GetQueryCmd() *cobra.Command {
return cli.GetQueryCmd()
}

// RegisterQueryService registers the gRPC query service for IBC ports.
func RegisterQueryService(server grpc.Server, queryServer types.QueryServer) {
types.RegisterQueryServer(server, queryServer)
}
10 changes: 10 additions & 0 deletions modules/core/05-port/types/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,14 @@ type IBCModule interface {
packet channeltypes.Packet,
relayer sdk.AccAddress,
) error

// NegotiateAppVersion performs application version negotiation given the provided port ID, counterparty and proposed version.
// An error is returned if version negotiation cannot be performed. For example, an application module implementing this interface
// may decide to return an error in the event of the proposed version being incompatible with it's own
NegotiateAppVersion(
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
ctx sdk.Context,
portID string,
counterparty channeltypes.Counterparty,
proposedVersion string,
) (version string, err error)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this still needs to take in the channel order and connection id

Copy link
Member Author

@damiannolan damiannolan Sep 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll add these fields to QueryAppVersionRequest also 👍

Edit: @colin-axner connection id or connection hops as []string?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of connectionID with string. Connection hops can be confusing and we aren't even sure if multi hop will ever be baked into core IBC. If it is, it'll probably be API breaking anyways so better to force app developers to realize the change has been made

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thank you ❤️

}
9 changes: 9 additions & 0 deletions modules/core/05-port/types/query.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package types

// NewNegotiateAppVersionResponse creates a new NegotiateAppVersionResponse instance
func NewNegotiateAppVersionResponse(portID, version string) *NegotiateAppVersionResponse {
return &NegotiateAppVersionResponse{
PortId: portID,
Version: version,
}
}
Loading