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: use gRPC for queries #10997

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Features

* [\#10997](https://github.com/cosmos/cosmos-sdk/pull/10997) Use gRPC for queries if `clientCtx.WithClientConn` is set.
* [\#10710](https://github.com/cosmos/cosmos-sdk/pull/10710) Chain-id shouldn't be required for creating a transaction with both --generate-only and --offline flags.
* [\#10703](https://github.com/cosmos/cosmos-sdk/pull/10703) Create a new grantee account, if the grantee of an authorization does not exist.
* [\#10592](https://github.com/cosmos/cosmos-sdk/pull/10592) Add a `DecApproxEq` function that checks to see if `|d1 - d2| < tol` for some Dec `d1, d2, tol`.
Expand All @@ -60,6 +61,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* [\#10997](https://github.com/cosmos/cosmos-sdk/pull/10997) Add `ctx` parameter to `ReadPersistentCommandFlags`
* [\#10950](https://github.com/cosmos/cosmos-sdk/pull/10950) Add `envPrefix` parameter to `cmd.Execute`.
* (x/mint) [\#10441](https://github.com/cosmos/cosmos-sdk/pull/10441) The `NewAppModule` function now accepts an inflation calculation function as an argument.
* [\#10295](https://github.com/cosmos/cosmos-sdk/pull/10295) Remove store type aliases from /types
Expand Down
38 changes: 29 additions & 9 deletions client/cmd.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package client

import (
"context"
"fmt"
"strings"

"github.com/pkg/errors"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"github.com/tendermint/tendermint/libs/cli"
"google.golang.org/grpc"
"google.golang.org/grpc/keepalive"

"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/crypto/keyring"
Expand All @@ -21,7 +24,7 @@ const ClientContextKey = sdk.ContextKey("client.context")
// SetCmdClientContextHandler is to be used in a command pre-hook execution to
// read flags that populate a Context and sets that to the command's Context.
func SetCmdClientContextHandler(clientCtx Context, cmd *cobra.Command) (err error) {
clientCtx, err = ReadPersistentCommandFlags(clientCtx, cmd.Flags())
clientCtx, err = ReadPersistentCommandFlags(cmd.Context(), clientCtx, cmd.Flags())
if err != nil {
return err
}
Expand Down Expand Up @@ -87,7 +90,7 @@ func ValidateCmd(cmd *cobra.Command, args []string) error {
// - client.Context field not pre-populated & flag set: uses set flag value
// - client.Context field pre-populated & flag not set: uses pre-populated value
// - client.Context field pre-populated & flag set: uses set flag value
func ReadPersistentCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, error) {
func ReadPersistentCommandFlags(ctx context.Context, clientCtx Context, flagSet *pflag.FlagSet) (Context, error) {
troian marked this conversation as resolved.
Show resolved Hide resolved
if clientCtx.OutputFormat == "" || flagSet.Changed(cli.OutputFlag) {
output, _ := flagSet.GetString(cli.OutputFlag)
clientCtx = clientCtx.WithOutputFormat(output)
Expand Down Expand Up @@ -133,8 +136,8 @@ func ReadPersistentCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Cont
}
}

rpcURI, _ := flagSet.GetString(flags.FlagNode)
if clientCtx.Client == nil || flagSet.Changed(flags.FlagNode) {
rpcURI, _ := flagSet.GetString(flags.FlagNode)
if rpcURI != "" {
clientCtx = clientCtx.WithNodeURI(rpcURI)

Expand All @@ -147,6 +150,23 @@ func ReadPersistentCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Cont
}
}

if clientCtx.ClientConn == nil || flagSet.Changed(flags.FlagGRPCNode) {
grpcURI, _ := flagSet.GetString(flags.FlagGRPCNode)

Choose a reason for hiding this comment

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

Check the error.


if grpcURI != "" {
grpcKeepAlive, _ := flagSet.GetDuration(flags.FlagGRPCKeepAlive)

Choose a reason for hiding this comment

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

Check the error.

gcl, err := grpc.DialContext(ctx, grpcURI,
grpc.WithInsecure(),
grpc.WithKeepaliveParams(keepalive.ClientParameters{
Time: grpcKeepAlive,
}),
)
if err == nil {
clientCtx = clientCtx.WithClientConn(gcl)
}
Comment on lines +164 to +166
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if err == nil {
clientCtx = clientCtx.WithClientConn(gcl)
}
if err != nil {
return clientCtx, err
}
clientCtx = clientCtx.WithClientConn(gcl)

Choose a reason for hiding this comment

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

Prefer returning a zero-value Context when the error is non-nil.

}
}

return clientCtx, nil
}

Expand All @@ -160,7 +180,7 @@ func ReadPersistentCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Cont
// - client.Context field not pre-populated & flag set: uses set flag value
// - client.Context field pre-populated & flag not set: uses pre-populated value
// - client.Context field pre-populated & flag set: uses set flag value
func readQueryCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, error) {
func readQueryCommandFlags(ctx context.Context, clientCtx Context, flagSet *pflag.FlagSet) (Context, error) {
if clientCtx.Height == 0 || flagSet.Changed(flags.FlagHeight) {
height, _ := flagSet.GetInt64(flags.FlagHeight)
clientCtx = clientCtx.WithHeight(height)
Expand All @@ -171,7 +191,7 @@ func readQueryCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context,
clientCtx = clientCtx.WithUseLedger(useLedger)
}

return ReadPersistentCommandFlags(clientCtx, flagSet)
return ReadPersistentCommandFlags(ctx, clientCtx, flagSet)
}

// readTxCommandFlags returns an updated Context with fields set based on flags
Expand All @@ -184,8 +204,8 @@ func readQueryCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context,
// - client.Context field not pre-populated & flag set: uses set flag value
// - client.Context field pre-populated & flag not set: uses pre-populated value
// - client.Context field pre-populated & flag set: uses set flag value
func readTxCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, error) {
clientCtx, err := ReadPersistentCommandFlags(clientCtx, flagSet)
func readTxCommandFlags(ctx context.Context, clientCtx Context, flagSet *pflag.FlagSet) (Context, error) {
clientCtx, err := ReadPersistentCommandFlags(ctx, clientCtx, flagSet)
if err != nil {
return clientCtx, err
}
Expand Down Expand Up @@ -294,7 +314,7 @@ func readTxCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, err
// - client.Context field pre-populated & flag set: uses set flag value
func GetClientQueryContext(cmd *cobra.Command) (Context, error) {
ctx := GetClientContextFromCmd(cmd)
return readQueryCommandFlags(ctx, cmd.Flags())
return readQueryCommandFlags(cmd.Context(), ctx, cmd.Flags())
}

// GetClientTxContext returns a Context from a command with fields set based on flags
Expand All @@ -306,7 +326,7 @@ func GetClientQueryContext(cmd *cobra.Command) (Context, error) {
// - client.Context field pre-populated & flag set: uses set flag value
func GetClientTxContext(cmd *cobra.Command) (Context, error) {
ctx := GetClientContextFromCmd(cmd)
return readTxCommandFlags(ctx, cmd.Flags())
return readTxCommandFlags(cmd.Context(), ctx, cmd.Flags())
}

// GetClientContextFromCmd returns a Context from a command or an empty Context
Expand Down
17 changes: 12 additions & 5 deletions client/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import (
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/config"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/codec"
clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
"github.com/cosmos/cosmos-sdk/x/staking/client/cli"
)

Expand All @@ -25,9 +27,12 @@ const (
// initClientContext initiates client Context for tests
func initClientContext(t *testing.T, envVar string) (client.Context, func()) {
home := t.TempDir()
registry := testdata.NewTestInterfaceRegistry()

clientCtx := client.Context{}.
WithHomeDir(home).
WithViper("")
WithViper("").
WithCodec(codec.NewProtoCodec(registry))

clientCtx.Viper.BindEnv(nodeEnv)
if envVar != "" {
Expand All @@ -53,7 +58,7 @@ func TestConfigCmd(t *testing.T) {
_, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
require.NoError(t, err)

//./build/simd config node //http://localhost:1
// ./build/simd config node //http://localhost:1
b := bytes.NewBufferString("")
cmd.SetOut(b)
cmd.SetArgs([]string{"node"})
Expand All @@ -68,16 +73,18 @@ func TestConfigCmdEnvFlag(t *testing.T) {
defaultNode = "http://localhost:26657"
)

nogrpc := fmt.Sprintf("--%s=", flags.FlagGRPCNode)

tt := []struct {
name string
envVar string
args []string
expNode string
}{
{"env var is set with no flag", testNode1, []string{"validators"}, testNode1},
{"env var is set with a flag", testNode1, []string{"validators", fmt.Sprintf("--%s=%s", flags.FlagNode, testNode2)}, testNode2},
{"env var is not set with no flag", "", []string{"validators"}, defaultNode},
{"env var is not set with a flag", "", []string{"validators", fmt.Sprintf("--%s=%s", flags.FlagNode, testNode2)}, testNode2},
{"env var is set with a flag", testNode1, []string{"validators", nogrpc, fmt.Sprintf("--%s=%s", flags.FlagNode, testNode2)}, testNode2},
{"env var is not set with no flag", "", []string{"validators", nogrpc}, defaultNode},
{"env var is not set with a flag", "", []string{"validators", nogrpc, fmt.Sprintf("--%s=%s", flags.FlagNode, testNode2)}, testNode2},
}

for _, tc := range tt {
Expand Down
14 changes: 12 additions & 2 deletions client/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"

"github.com/spf13/viper"
"google.golang.org/grpc"

"sigs.k8s.io/yaml"

Expand Down Expand Up @@ -49,9 +50,10 @@ type Context struct {
FeePayer sdk.AccAddress
FeeGranter sdk.AccAddress
Viper *viper.Viper

ClientConn grpc.ClientConnInterface

// IsAux is true when the signer is an auxiliary signer (e.g. the tipper).
IsAux bool
IsAux bool

// TODO: Deprecated (remove).
LegacyAmino *codec.LegacyAmino
Expand Down Expand Up @@ -128,6 +130,14 @@ func (ctx Context) WithClient(client rpcclient.Client) Context {
return ctx
}

// WithClientConn returns a copy of the context with an updated gRPC client
// instance. This is optional, if not set, then the client.Context will default
// to using Tendermint's RPC `abci_query` for making queries.
func (ctx Context) WithClientConn(rpc grpc.ClientConnInterface) Context {
ctx.ClientConn = rpc
return ctx
}

// WithUseLedger returns a copy of the context with an updated UseLedger flag.
func (ctx Context) WithUseLedger(useLedger bool) Context {
ctx.UseLedger = useLedger
Expand Down
7 changes: 7 additions & 0 deletions client/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package flags
import (
"fmt"
"strconv"
"time"

"github.com/spf13/cobra"
tmcli "github.com/tendermint/tendermint/libs/cli"
Expand Down Expand Up @@ -46,6 +47,8 @@ const (
FlagUseLedger = "ledger"
FlagChainID = "chain-id"
FlagNode = "node"
FlagGRPCNode = "grpc-node"
FlagGRPCKeepAlive = "grpc-keepalive"
FlagHeight = "height"
FlagGasAdjustment = "gas-adjustment"
FlagFrom = "from"
Expand Down Expand Up @@ -91,6 +94,8 @@ var LineBreak = &cobra.Command{Run: func(*cobra.Command, []string) {}}
// AddQueryFlagsToCmd adds common flags to a module query command.
func AddQueryFlagsToCmd(cmd *cobra.Command) {
cmd.Flags().String(FlagNode, "tcp://localhost:26657", "<host>:<port> to Tendermint RPC interface for this chain")
cmd.Flags().String(FlagGRPCNode, "", "<host>:<port> to GRPC interface for this chain")
Copy link
Contributor

@amaury1093 amaury1093 Jan 27, 2022

Choose a reason for hiding this comment

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

@troian I understand your point about grpc URI deriving, but imo it's a bit confusing. What do you think about this:

Suggested change
cmd.Flags().String(FlagGRPCNode, "", "<host>:<port> to GRPC interface for this chain")
cmd.Flags().String(FlagGRPCNode, "localhost:9090", "<host>:<port> to GRPC interface for this chain")

The UX is:

  • by default, the client.Context pings localhost:9090
  • if the user wants to change gRPC uri, then --grpc-node=<...> or _GRPC_NODE=<...> (both are equivalent) (note: if the grpc-node is set, which is the case by default, we should never ping the TM rpc node anymore).
  • if the user explicitly wants to use Tendermint rpc, then --grpc-node="" (and optionally --node=<...>)

cmd.Flags().Duration(FlagGRPCKeepAlive, 20*time.Second, "set GRPC keepalive. defaults to 20s")

Choose a reason for hiding this comment

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

No need to re-iterate the default value in the help text, as it is printed anyway.

cmd.Flags().Int64(FlagHeight, 0, "Use a specific height to query state at (this can error if the node is pruning state)")
cmd.Flags().StringP(tmcli.OutputFlag, "o", "text", "Output format (text|json)")

Expand All @@ -108,6 +113,8 @@ func AddTxFlagsToCmd(cmd *cobra.Command) {
cmd.Flags().String(FlagFees, "", "Fees to pay along with transaction; eg: 10uatom")
cmd.Flags().String(FlagGasPrices, "", "Gas prices in decimal format to determine the transaction fee (e.g. 0.1uatom)")
cmd.Flags().String(FlagNode, "tcp://localhost:26657", "<host>:<port> to tendermint rpc interface for this chain")
cmd.Flags().String(FlagGRPCNode, "", "<host>:<port> to GRPC interface for this chain")
cmd.Flags().Duration(FlagGRPCKeepAlive, 20*time.Second, "set GRPC keepalive. defaults to 20s")
cmd.Flags().Bool(FlagUseLedger, false, "Use a connected Ledger device")
cmd.Flags().Float64(FlagGasAdjustment, DefaultGasAdjustment, "adjustment factor to be multiplied against the estimate returned by the tx simulation; if the gas limit is set manually this flag is ignored ")
cmd.Flags().StringP(FlagBroadcastMode, "b", BroadcastSync, "Transaction broadcasting mode (sync|async|block)")
Expand Down
68 changes: 40 additions & 28 deletions client/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,6 @@ func (ctx Context) Invoke(grpcCtx gocontext.Context, method string, req, reply i
}

// Case 2. Querying state.
reqBz, err := protoCodec.Marshal(req)
if err != nil {
return err
}

// parse height header
md, _ := metadata.FromOutgoingContext(grpcCtx)
Expand All @@ -72,35 +68,51 @@ func (ctx Context) Invoke(grpcCtx gocontext.Context, method string, req, reply i
ctx = ctx.WithHeight(height)
}

abciReq := abci.RequestQuery{
Path: method,
Data: reqBz,
Height: ctx.Height,
}
if ctx.ClientConn != nil {
if ctx.Height > 0 {
grpcCtx = metadata.AppendToOutgoingContext(grpcCtx, grpctypes.GRPCBlockHeightHeader, strconv.FormatUint(uint64(ctx.Height), 10))
}

res, err := ctx.QueryABCI(abciReq)
if err != nil {
return err
}
err = ctx.ClientConn.Invoke(grpcCtx, method, req, reply, opts...)
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}
} else {
reqBz, err := protoCodec.Marshal(req)
if err != nil {
return err
}

err = protoCodec.Unmarshal(res.Value, reply)
if err != nil {
return err
}
abciReq := abci.RequestQuery{
Path: method,
Data: reqBz,
Height: ctx.Height,
}

// Create header metadata. For now the headers contain:
// - block height
// We then parse all the call options, if the call option is a
// HeaderCallOption, then we manually set the value of that header to the
// metadata.
md = metadata.Pairs(grpctypes.GRPCBlockHeightHeader, strconv.FormatInt(res.Height, 10))
for _, callOpt := range opts {
header, ok := callOpt.(grpc.HeaderCallOption)
if !ok {
continue
res, err := ctx.QueryABCI(abciReq)
if err != nil {
return err
}

*header.HeaderAddr = md
err = protoCodec.Unmarshal(res.Value, reply)
if err != nil {
return err
}

// Create header metadata. For now the headers contain:
// - block height
// We then parse all the call options, if the call option is a
// HeaderCallOption, then we manually set the value of that header to the
// metadata.
md = metadata.Pairs(grpctypes.GRPCBlockHeightHeader, strconv.FormatInt(res.Height, 10))
for _, callOpt := range opts {
header, ok := callOpt.(grpc.HeaderCallOption)
if !ok {
continue
}

*header.HeaderAddr = md
}
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
}

if ctx.InterfaceRegistry != nil {
Expand Down
1 change: 1 addition & 0 deletions client/grpc_query_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build norace
// +build norace

package client_test
Expand Down
2 changes: 1 addition & 1 deletion contrib/rosetta/configuration/send_funds.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@

set -e
addr=$(simd keys show fd -a --keyring-backend=test)
echo "12345678" | simd tx bank send "$addr" "$1" 100stake --chain-id="testing" --node tcp://cosmos:26657 --yes --keyring-backend=test
echo "12345678" | simd tx bank send "$addr" "$1" 100stake --chain-id="testing" --node tcp://cosmos:26657 --yes --keyring-backend=test
2 changes: 1 addition & 1 deletion contrib/rosetta/docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ services:
working_dir: /rosetta
command: ["python3", "faucet.py"]
expose:
- 8080
- 8000
Copy link
Contributor

Choose a reason for hiding this comment

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

are you sure about this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

faucet.py listens on 8000

but i think it should be separate pr


test_rosetta:
image: tendermintdev/rosetta-cli:v0.6.7
Expand Down
2 changes: 1 addition & 1 deletion simapp/simd/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func NewRootCmd() (*cobra.Command, params.EncodingConfig) {
cmd.SetOut(cmd.OutOrStdout())
cmd.SetErr(cmd.ErrOrStderr())

initClientCtx, err := client.ReadPersistentCommandFlags(initClientCtx, cmd.Flags())
initClientCtx, err := client.ReadPersistentCommandFlags(cmd.Context(), initClientCtx, cmd.Flags())
if err != nil {
return err
}
Expand Down
Loading