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

RegisterInterfaces registers service Msg type_urls #7671

Merged
merged 31 commits into from
Oct 28, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
132e5eb
Add RegisterMsgServiceDesc
amaury1093 Oct 26, 2020
c986595
Refactor newSendTxMsgServiceCmd
amaury1093 Oct 26, 2020
3027b94
Add test
amaury1093 Oct 26, 2020
03eca89
Register in all modules
amaury1093 Oct 26, 2020
63562ef
Remove RegisterMsgServiceDesc from baseapp
amaury1093 Oct 26, 2020
c1f6923
Add explicit error message
amaury1093 Oct 26, 2020
44e8ac4
Add comment
amaury1093 Oct 26, 2020
80def81
Update comment
amaury1093 Oct 26, 2020
0f6facd
Add test
amaury1093 Oct 26, 2020
5564ad2
Merge branch 'master' of ssh://github.com/cosmos/cosmos-sdk into am-7…
amaury1093 Oct 26, 2020
5d8612d
Update comment
amaury1093 Oct 26, 2020
fb43263
Remove duplicate import
amaury1093 Oct 26, 2020
0edf85a
Fix lint
amaury1093 Oct 26, 2020
049bdfa
Forgot vesting
amaury1093 Oct 26, 2020
dd5b42f
Fix lint
amaury1093 Oct 26, 2020
8710259
Fix test
amaury1093 Oct 26, 2020
196414e
Put in types/module
amaury1093 Oct 26, 2020
295f625
Put in types/msgservice
amaury1093 Oct 26, 2020
8cd8466
Add comment about panic
amaury1093 Oct 26, 2020
9aa2f82
Update baseapp/msg_service_router.go
amaury1093 Oct 27, 2020
7b32d47
Update baseapp/msg_service_router.go
amaury1093 Oct 27, 2020
817c0ae
Update baseapp/msg_service_router.go
amaury1093 Oct 27, 2020
a2d09ea
Add comment
amaury1093 Oct 27, 2020
4ba80f4
Merge branch 'master' into am-7647-msgserver
amaury1093 Oct 27, 2020
4401ccf
Merge branch 'master' into am-7647-msgserver
amaury1093 Oct 27, 2020
5e5d453
Add better test
amaury1093 Oct 27, 2020
3a2f2d8
Merge branch 'am-7647-msgserver' of ssh://github.com/cosmos/cosmos-sd…
amaury1093 Oct 27, 2020
b442ee3
Merge branch 'master' of ssh://github.com/cosmos/cosmos-sdk into am-7…
amaury1093 Oct 27, 2020
d6d740c
Merge branch 'master' into am-7647-msgserver
amaury1093 Oct 27, 2020
5da8fb1
Update baseapp/msg_service_router.go
amaury1093 Oct 28, 2020
a57edcc
Merge branch 'master' into am-7647-msgserver
mergify[bot] Oct 28, 2020
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
21 changes: 1 addition & 20 deletions baseapp/msg_service_router.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,21 +45,7 @@ func (msr *MsgServiceRouter) RegisterService(sd *grpc.ServiceDesc, handler inter
fqMethod := fmt.Sprintf("/%s/%s", sd.ServiceName, method.MethodName)
methodHandler := method.Handler

// NOTE: This is how we pull the concrete request type for each handler for registering in the InterfaceRegistry.
// This approach is maybe a bit hacky, but less hacky than reflecting on the handler object itself.
// We use a no-op interceptor to avoid actually calling into the handler itself.
_, _ = methodHandler(nil, context.Background(), func(i interface{}) error {
msg, ok := i.(proto.Message)
if !ok {
// We panic here because there is no other alternative and the app cannot be initialized correctly
// this should only happen if there is a problem with code generation in which case the app won't
// work correctly anyway.
panic(fmt.Errorf("can't register request type %T for service method %s", i, fqMethod))
}

msr.interfaceRegistry.RegisterCustomTypeURL((*sdk.MsgRequest)(nil), fqMethod, msg)
return nil
}, noopInterceptor)
sdk.RegisterMsgServiceDesc(msr.interfaceRegistry, sd)

msr.routes[fqMethod] = func(ctx sdk.Context, req sdk.MsgRequest) (*sdk.Result, error) {
ctx = ctx.WithEventManager(sdk.NewEventManager())
Expand Down Expand Up @@ -89,9 +75,4 @@ func (msr *MsgServiceRouter) SetInterfaceRegistry(interfaceRegistry codectypes.I
msr.interfaceRegistry = interfaceRegistry
}

// gRPC NOOP interceptor
func noopInterceptor(_ context.Context, _ interface{}, _ *grpc.UnaryServerInfo, _ grpc.UnaryHandler) (interface{}, error) {
return nil, nil
}

func noopDecoder(_ interface{}) error { return nil }
43 changes: 43 additions & 0 deletions types/msg_service.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package types
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved

import (
context "context"
fmt "fmt"

proto "github.com/gogo/protobuf/proto"
grpc "google.golang.org/grpc"

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

// RegisterMsgServiceDesc registers all type_urls from Msg services described
// in `sd` inside the registry.
func RegisterMsgServiceDesc(registry types.InterfaceRegistry, sd *grpc.ServiceDesc) {
// Adds a top-level type_url based on the Msg service name.
for _, method := range sd.Methods {
fqMethod := fmt.Sprintf("/%s/%s", sd.ServiceName, method.MethodName)
methodHandler := method.Handler

// NOTE: This is how we pull the concrete request type for each handler for registering in the InterfaceRegistry.
// This approach is maybe a bit hacky, but less hacky than reflecting on the handler object itself.
// We use a no-op interceptor to avoid actually calling into the handler itself.
_, _ = methodHandler(nil, context.Background(), func(i interface{}) error {
msg, ok := i.(proto.Message)
if !ok {
// We panic here because there is no other alternative and the app cannot be initialized correctly
// this should only happen if there is a problem with code generation in which case the app won't
// work correctly anyway.
panic(fmt.Errorf("can't register request type %T for service method %s", i, fqMethod))
}

registry.RegisterCustomTypeURL((*MsgRequest)(nil), fqMethod, msg)
return nil
}, noopInterceptor)

}
}

// gRPC NOOP interceptor
func noopInterceptor(_ context.Context, _ interface{}, _ *grpc.UnaryServerInfo, _ grpc.UnaryHandler) (interface{}, error) {
return nil, nil
}
30 changes: 25 additions & 5 deletions x/auth/client/cli/cli_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// +build norace

package cli_test

import (
Expand Down Expand Up @@ -174,9 +172,11 @@ func (s *IntegrationTestSuite) TestCLIQueryTxCmd() {
account2, err := val.ClientCtx.Keyring.Key("newAccount2")
s.Require().NoError(err)

// Send coins.
sendTokens := sdk.NewInt64Coin(s.cfg.BondDenom, 10)
out, err := bankcli.MsgSendExec(

// Send coins, try both with legacy Msg and with Msg service.
// Legacy Msg.
legacyMsgOut, err := bankcli.MsgSendExec(
val.ClientCtx,
val.Address,
account2.GetAddress(),
Expand All @@ -187,7 +187,21 @@ func (s *IntegrationTestSuite) TestCLIQueryTxCmd() {
fmt.Sprintf("--gas=%d", flags.DefaultGasLimit),
)
s.Require().NoError(err)
var legacyMsgTxRes sdk.TxResponse
s.Require().NoError(val.ClientCtx.JSONMarshaler.UnmarshalJSON(legacyMsgOut.Bytes(), &legacyMsgTxRes))

// Service Msg.
out, err := bankcli.MsgSendExec(
val.ClientCtx,
val.Address,
account2.GetAddress(),
sdk.NewCoins(sendTokens),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
fmt.Sprintf("--gas=%d", flags.DefaultGasLimit),
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is an integration test. Not related to this PR, because other code in this test is doing a similar things. In the future we will need to mark this tests as such and skip them if we just want to do unit-tests.

s.Require().NoError(err)
var txRes sdk.TxResponse
s.Require().NoError(val.ClientCtx.JSONMarshaler.UnmarshalJSON(out.Bytes(), &txRes))

Expand All @@ -209,7 +223,12 @@ func (s *IntegrationTestSuite) TestCLIQueryTxCmd() {
true,
},
{
"happy case",
"happy case (legacy Msg)",
[]string{legacyMsgTxRes.TxHash, fmt.Sprintf("--%s=json", tmcli.OutputFlag)},
false,
},
{
"happy case (service Msg)",
[]string{txRes.TxHash, fmt.Sprintf("--%s=json", tmcli.OutputFlag)},
false,
},
Expand All @@ -229,6 +248,7 @@ func (s *IntegrationTestSuite) TestCLIQueryTxCmd() {
} else {
var result sdk.TxResponse
s.Require().NoError(val.ClientCtx.JSONMarshaler.UnmarshalJSON(out.Bytes(), &result))
fmt.Println(result)
s.Require().NotNil(result.Height)
}
})
Expand Down
88 changes: 1 addition & 87 deletions x/bank/client/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,6 @@ import (
"fmt"
"testing"

"github.com/spf13/cobra"

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

"github.com/gogo/protobuf/grpc"
grpc2 "google.golang.org/grpc"

"github.com/gogo/protobuf/proto"
"github.com/stretchr/testify/suite"
tmcli "github.com/tendermint/tendermint/libs/cli"
Expand Down Expand Up @@ -302,82 +295,6 @@ func (s *IntegrationTestSuite) TestNewSendTxCmd() {
}
}

// serviceMsgClientConn is an instance of grpc.ClientConn that is used to test building
// transactions with MsgClient's. It is intended to be replaced by the work in
// https://github.com/cosmos/cosmos-sdk/issues/7541 when that is ready.
type serviceMsgClientConn struct {
msgs []sdk.Msg
}

func (t *serviceMsgClientConn) Invoke(_ context.Context, method string, args, _ interface{}, _ ...grpc2.CallOption) error {
req, ok := args.(sdk.MsgRequest)
if !ok {
return fmt.Errorf("%T should implement %T", args, (*sdk.MsgRequest)(nil))
}

err := req.ValidateBasic()
if err != nil {
return err
}

t.msgs = append(t.msgs, sdk.ServiceMsg{
MethodName: method,
Request: req,
})

return nil
}

func (t *serviceMsgClientConn) NewStream(context.Context, *grpc2.StreamDesc, string, ...grpc2.CallOption) (grpc2.ClientStream, error) {
return nil, fmt.Errorf("not supported")
}

var _ grpc.ClientConn = &serviceMsgClientConn{}

// newSendTxMsgServiceCmd is just for the purpose of testing ServiceMsg's in an end-to-end case. It is effectively
// NewSendTxCmd but using MsgClient.
func newSendTxMsgServiceCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "send [from_key_or_address] [to_address] [amount]",
Short: `Send funds from one account to another. Note, the'--from' flag is
ignored as it is implied from [from_key_or_address].`,
Args: cobra.ExactArgs(3),
RunE: func(cmd *cobra.Command, args []string) error {
cmd.Flags().Set(flags.FlagFrom, args[0])

clientCtx := client.GetClientContextFromCmd(cmd)
clientCtx, err := client.ReadTxCommandFlags(clientCtx, cmd.Flags())
if err != nil {
return err
}

toAddr, err := sdk.AccAddressFromBech32(args[1])
if err != nil {
return err
}

coins, err := sdk.ParseCoins(args[2])
if err != nil {
return err
}

msg := types.NewMsgSend(clientCtx.GetFromAddress(), toAddr, coins)
svcMsgClientConn := &serviceMsgClientConn{}
bankMsgClient := types.NewMsgClient(svcMsgClientConn)
_, err = bankMsgClient.Send(context.Background(), msg)
if err != nil {
return err
}

return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), svcMsgClientConn.msgs...)
},
}

flags.AddTxFlagsToCmd(cmd)

return cmd
}

// TestBankMsgService does a basic test of whether or not service Msg's as defined
// in ADR 031 work in the most basic end-to-end case.
func (s *IntegrationTestSuite) TestBankMsgService() {
Expand Down Expand Up @@ -419,10 +336,7 @@ func (s *IntegrationTestSuite) TestBankMsgService() {
s.Run(tc.name, func() {
clientCtx := val.ClientCtx

args := []string{tc.from.String(), tc.to.String(), tc.amount.String()}
args = append(args, tc.args...)

bz, err := clitestutil.ExecTestCLICmd(clientCtx, newSendTxMsgServiceCmd(), args)
bz, err := banktestutil.ServiceMsgSendExec(clientCtx, tc.from, tc.to, tc.amount, tc.args...)
if tc.expectErr {
s.Require().Error(err)
} else {
Expand Down
95 changes: 95 additions & 0 deletions x/bank/client/testutil/cli_helpers.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
package testutil

import (
"context"
"fmt"

gogogrpc "github.com/gogo/protobuf/grpc"
"github.com/spf13/cobra"
"github.com/tendermint/tendermint/libs/cli"
grpc "google.golang.org/grpc"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/client/tx"
"github.com/cosmos/cosmos-sdk/testutil"
clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli"
sdk "github.com/cosmos/cosmos-sdk/types"
bankcli "github.com/cosmos/cosmos-sdk/x/bank/client/cli"
"github.com/cosmos/cosmos-sdk/x/bank/types"
)

func MsgSendExec(clientCtx client.Context, from, to, amount fmt.Stringer, extraArgs ...string) (testutil.BufferWriter, error) {
Expand All @@ -24,3 +32,90 @@ func QueryBalancesExec(clientCtx client.Context, address fmt.Stringer, extraArgs

return clitestutil.ExecTestCLICmd(clientCtx, bankcli.GetBalancesCmd(), args)
}

// serviceMsgClientConn is an instance of grpc.ClientConn that is used to test building
// transactions with MsgClient's. It is intended to be replaced by the work in
// https://github.com/cosmos/cosmos-sdk/issues/7541 when that is ready.
type serviceMsgClientConn struct {
msgs []sdk.Msg
}

func (t *serviceMsgClientConn) Invoke(_ context.Context, method string, args, _ interface{}, _ ...grpc.CallOption) error {
req, ok := args.(sdk.MsgRequest)
if !ok {
return fmt.Errorf("%T should implement %T", args, (*sdk.MsgRequest)(nil))
}

err := req.ValidateBasic()
if err != nil {
return err
}

t.msgs = append(t.msgs, sdk.ServiceMsg{
MethodName: method,
Request: req,
})

return nil
}

func (t *serviceMsgClientConn) NewStream(context.Context, *grpc.StreamDesc, string, ...grpc.CallOption) (grpc.ClientStream, error) {
return nil, fmt.Errorf("not supported")
}

var _ gogogrpc.ClientConn = &serviceMsgClientConn{}

// newSendTxMsgServiceCmd is just for the purpose of testing ServiceMsg's in an end-to-end case. It is effectively
// NewSendTxCmd but using MsgClient.
func newSendTxMsgServiceCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "send [from_key_or_address] [to_address] [amount]",
Short: `Send funds from one account to another. Note, the'--from' flag is
ignored as it is implied from [from_key_or_address].`,
Args: cobra.ExactArgs(3),
RunE: func(cmd *cobra.Command, args []string) error {
cmd.Flags().Set(flags.FlagFrom, args[0])

clientCtx := client.GetClientContextFromCmd(cmd)
clientCtx, err := client.ReadTxCommandFlags(clientCtx, cmd.Flags())
if err != nil {
return err
}

toAddr, err := sdk.AccAddressFromBech32(args[1])
if err != nil {
return err
}

coins, err := sdk.ParseCoins(args[2])
if err != nil {
return err
}

msg := types.NewMsgSend(clientCtx.GetFromAddress(), toAddr, coins)
svcMsgClientConn := &serviceMsgClientConn{}
bankMsgClient := types.NewMsgClient(svcMsgClientConn)
_, err = bankMsgClient.Send(context.Background(), msg)
if err != nil {
return err
}

return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), svcMsgClientConn.msgs...)
},
}

flags.AddTxFlagsToCmd(cmd)

return cmd
}

// ServiceMsgSendExec is a temporary method to test Msg services in CLI using
// x/bank's Msg/Send service. After https://github.com/cosmos/cosmos-sdk/issues/7541
// is merged, this method should be removed, and we should prefer MsgSendExec
// instead.
func ServiceMsgSendExec(clientCtx client.Context, from, to, amount fmt.Stringer, extraArgs ...string) (testutil.BufferWriter, error) {
args := []string{from.String(), to.String(), amount.String()}
args = append(args, extraArgs...)

return clitestutil.ExecTestCLICmd(clientCtx, newSendTxMsgServiceCmd(), args)
}
2 changes: 2 additions & 0 deletions x/bank/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {
(*exported.SupplyI)(nil),
&Supply{},
)

sdk.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)
}

var (
Expand Down
2 changes: 2 additions & 0 deletions x/crisis/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
registry.RegisterImplementations((*sdk.Msg)(nil),
&MsgVerifyInvariant{},
)

sdk.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)
}

var (
Expand Down