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

Prefer sending tx_bytes to Simulate gRPC endpoint #8926

Merged
merged 29 commits into from
Mar 25, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
7149841
First run
amaury1093 Mar 18, 2021
0a6b14a
Remove dead code
amaury1093 Mar 18, 2021
981f68d
Make test pass
amaury1093 Mar 18, 2021
2858900
Proto gen
amaury1093 Mar 18, 2021
a716ec6
Fix lint
amaury1093 Mar 18, 2021
8ee3a37
Merge branch 'master' into am/8425-fix-simulation
amaury1093 Mar 18, 2021
4165d78
Add changelog
amaury1093 Mar 18, 2021
76d16d4
Fix tests
amaury1093 Mar 18, 2021
806c3b5
Fix test
amaury1093 Mar 19, 2021
67a4d77
Merge branch 'master' of ssh://github.com/cosmos/cosmos-sdk into am/8…
amaury1093 Mar 22, 2021
7a9cd64
Update x/auth/tx/service.go
amaury1093 Mar 22, 2021
ca088c1
Remove protoTxProvider
amaury1093 Mar 22, 2021
5151a92
Add grpc-gateway test
amaury1093 Mar 22, 2021
693ea28
Add comment
amaury1093 Mar 22, 2021
573e5bd
move to api breaking
amaury1093 Mar 22, 2021
b8f7ccb
lesser diff
amaury1093 Mar 22, 2021
d58dd5f
Merge branch 'master' into am/8425-fix-simulation
amaury1093 Mar 22, 2021
781d389
Merge branch 'master' into am/8425-fix-simulation
amaury1093 Mar 22, 2021
aca386f
Merge branch 'master' into am/8425-fix-simulation
Mar 22, 2021
8e6c7aa
Merge master
amaury1093 Mar 23, 2021
a38d91c
Merge branch 'am/8425-fix-simulation' of ssh://github.com/cosmos/cosm…
amaury1093 Mar 23, 2021
c557f76
remove conflict
amaury1093 Mar 24, 2021
6412b30
Merge branch 'master' into am/8425-fix-simulation
Mar 24, 2021
519f643
Merge branch 'master' into am/8425-fix-simulation
mergify[bot] Mar 24, 2021
40cebc3
Merge branch 'master' into am/8425-fix-simulation
amaury1093 Mar 24, 2021
6d9e49d
Merge branch 'master' into am/8425-fix-simulation
amaury1093 Mar 25, 2021
1dae35e
empty commit to rerun CI
amaury1093 Mar 25, 2021
5c2e4a4
Merge branch 'master' into am/8425-fix-simulation
amaury1093 Mar 25, 2021
375b439
empty commit to rerun CI
amaury1093 Mar 25, 2021
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
55 changes: 23 additions & 32 deletions client/tx/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ package tx

import (
"bufio"
"context"
"errors"
"fmt"
"net/http"
"os"

gogogrpc "github.com/gogo/protobuf/grpc"
Copy link
Collaborator

Choose a reason for hiding this comment

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

typealias is not needed.

"github.com/spf13/pflag"

"github.com/cosmos/cosmos-sdk/client"
Expand All @@ -20,7 +22,6 @@ import (
"github.com/cosmos/cosmos-sdk/types/tx"
"github.com/cosmos/cosmos-sdk/types/tx/signing"
authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
)

// GenerateOrBroadcastTxCLI will either generate and print and unsigned transaction
Expand Down Expand Up @@ -50,7 +51,7 @@ func GenerateTx(clientCtx client.Context, txf Factory, msgs ...sdk.Msg) error {
return errors.New("cannot estimate gas in offline mode")
}

_, adjusted, err := CalculateGas(clientCtx.QueryWithData, txf, msgs...)
_, adjusted, err := CalculateGas(clientCtx, txf, msgs...)
if err != nil {
return err
}
Expand All @@ -76,13 +77,13 @@ func GenerateTx(clientCtx client.Context, txf Factory, msgs ...sdk.Msg) error {
// given set of messages. It will also simulate gas requirements if necessary.
// It will return an error upon failure.
func BroadcastTx(clientCtx client.Context, txf Factory, msgs ...sdk.Msg) error {
txf, err := PrepareFactory(clientCtx, txf)
txf, err := prepareFactory(clientCtx, txf)
if err != nil {
return err
}

if txf.SimulateAndExecute() || clientCtx.Simulate {
_, adjusted, err := CalculateGas(clientCtx.QueryWithData, txf, msgs...)
_, adjusted, err := CalculateGas(clientCtx, txf, msgs...)
if err != nil {
return err
}
Expand Down Expand Up @@ -142,8 +143,9 @@ func BroadcastTx(clientCtx client.Context, txf Factory, msgs ...sdk.Msg) error {
// BaseReq. Upon any error, the error will be written to the http.ResponseWriter.
// Note that this function returns the legacy StdTx Amino JSON format for compatibility
// with legacy clients.
// Deprecated: We are removing Amino soon.
func WriteGeneratedTxResponse(
ctx client.Context, w http.ResponseWriter, br rest.BaseReq, msgs ...sdk.Msg,
clientCtx client.Context, w http.ResponseWriter, br rest.BaseReq, msgs ...sdk.Msg,
Copy link
Collaborator

Choose a reason for hiding this comment

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

usually we don't need to repeat a typename in the variable name.

) {
gasAdj, ok := rest.ParseFloat64OrReturnBadRequest(w, br.GasAdjustment, flags.DefaultGasAdjustment)
if !ok {
Expand All @@ -163,7 +165,7 @@ func WriteGeneratedTxResponse(
WithMemo(br.Memo).
WithChainID(br.ChainID).
WithSimulateAndExecute(br.Simulate).
WithTxConfig(ctx.TxConfig).
WithTxConfig(clientCtx.TxConfig).
WithTimeoutHeight(br.TimeoutHeight)

if br.Simulate || gasSetting.Simulate {
Expand All @@ -172,15 +174,15 @@ func WriteGeneratedTxResponse(
return
}

_, adjusted, err := CalculateGas(ctx.QueryWithData, txf, msgs...)
_, adjusted, err := CalculateGas(clientCtx, txf, msgs...)
if rest.CheckInternalServerError(w, err) {
return
}

txf = txf.WithGas(adjusted)

if br.Simulate {
rest.WriteSimulationResponse(w, ctx.LegacyAmino, txf.Gas())
rest.WriteSimulationResponse(w, clientCtx.LegacyAmino, txf.Gas())
return
}
}
Expand All @@ -190,12 +192,12 @@ func WriteGeneratedTxResponse(
return
}

stdTx, err := ConvertTxToStdTx(ctx.LegacyAmino, tx.GetTx())
stdTx, err := ConvertTxToStdTx(clientCtx.LegacyAmino, tx.GetTx())
if rest.CheckInternalServerError(w, err) {
return
}

output, err := ctx.LegacyAmino.MarshalJSON(stdTx)
output, err := clientCtx.LegacyAmino.MarshalJSON(stdTx)
if rest.CheckInternalServerError(w, err) {
return
}
Expand Down Expand Up @@ -268,46 +270,35 @@ func BuildSimTx(txf Factory, msgs ...sdk.Msg) ([]byte, error) {
return nil, err
}

protoProvider, ok := txb.(authtx.ProtoTxProvider)
if !ok {
return nil, fmt.Errorf("cannot simulate amino tx")
}
simReq := tx.SimulateRequest{Tx: protoProvider.GetProtoTx()}

return simReq.Marshal()
return txf.txConfig.TxEncoder()(txb.GetTx())
}

// CalculateGas simulates the execution of a transaction and returns the
// simulation response obtained by the query and the adjusted gas amount.
func CalculateGas(
queryFunc func(string, []byte) ([]byte, int64, error), txf Factory, msgs ...sdk.Msg,
) (tx.SimulateResponse, uint64, error) {
clientCtx gogogrpc.ClientConn, txf Factory, msgs ...sdk.Msg,
) (*tx.SimulateResponse, uint64, error) {
txBytes, err := BuildSimTx(txf, msgs...)
if err != nil {
return tx.SimulateResponse{}, 0, err
return nil, 0, err
}

// TODO This should use the generated tx service Client.
// https://github.com/cosmos/cosmos-sdk/issues/7726
bz, _, err := queryFunc("/cosmos.tx.v1beta1.Service/Simulate", txBytes)
txSvcClient := tx.NewServiceClient(clientCtx)
simRes, err := txSvcClient.Simulate(context.Background(), &tx.SimulateRequest{
TxBytes: txBytes,
})
if err != nil {
return tx.SimulateResponse{}, 0, err
}

var simRes tx.SimulateResponse

if err := simRes.Unmarshal(bz); err != nil {
return tx.SimulateResponse{}, 0, err
return nil, 0, err
}

return simRes, uint64(txf.GasAdjustment() * float64(simRes.GasInfo.GasUsed)), nil
}

// PrepareFactory ensures the account defined by ctx.GetFromAddress() exists and
// prepareFactory ensures the account defined by ctx.GetFromAddress() exists and
// if the account number and/or the account sequence number are zero (not set),
// they will be queried for and set on the provided Factory. A new Factory with
// the updated fields will be returned.
func PrepareFactory(clientCtx client.Context, txf Factory) (Factory, error) {
func prepareFactory(clientCtx client.Context, txf Factory) (Factory, error) {
from := clientCtx.GetFromAddress()

if err := txf.accountRetriever.EnsureExists(clientCtx, from); err != nil {
Expand Down
55 changes: 32 additions & 23 deletions client/tx/tx_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package tx_test

import (
"errors"
gocontext "context"
"fmt"
"testing"

"github.com/stretchr/testify/require"
"google.golang.org/grpc"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/tx"
Expand All @@ -24,30 +26,34 @@ func NewTestTxConfig() client.TxConfig {
return cfg.TxConfig
}

func TestCalculateGas(t *testing.T) {
makeQueryFunc := func(gasUsed uint64, wantErr bool) func(string, []byte) ([]byte, int64, error) {
return func(string, []byte) ([]byte, int64, error) {
if wantErr {
return nil, 0, errors.New("query failed")
}
simRes := &txtypes.SimulateResponse{
GasInfo: &sdk.GasInfo{GasUsed: gasUsed, GasWanted: gasUsed},
Result: &sdk.Result{Data: []byte("tx data"), Log: "log"},
}
// mockContext is a mock client.Context to return abitrary simulation response, used to
// unit test CalculateGas.
type mockContext struct {
gasUsed uint64
wantErr bool
}

bz, err := simRes.Marshal()
if err != nil {
return nil, 0, err
}
func (m mockContext) Invoke(grpcCtx gocontext.Context, method string, req, reply interface{}, opts ...grpc.CallOption) (err error) {
if m.wantErr {
return fmt.Errorf("mock err")
}

return bz, 0, nil
}
*(reply.(*txtypes.SimulateResponse)) = txtypes.SimulateResponse{
GasInfo: &sdk.GasInfo{GasUsed: m.gasUsed, GasWanted: m.gasUsed},
Result: &sdk.Result{Data: []byte("tx data"), Log: "log"},
}

return nil
}
func (mockContext) NewStream(gocontext.Context, *grpc.StreamDesc, string, ...grpc.CallOption) (grpc.ClientStream, error) {
panic("not implemented")
}

func TestCalculateGas(t *testing.T) {
type args struct {
queryFuncGasUsed uint64
queryFuncWantErr bool
adjustment float64
mockGasUsed uint64
mockWantErr bool
adjustment float64
}

testCases := []struct {
Expand All @@ -70,16 +76,19 @@ func TestCalculateGas(t *testing.T) {
WithTxConfig(txCfg).WithSignMode(txCfg.SignModeHandler().DefaultMode())

t.Run(stc.name, func(t *testing.T) {
queryFunc := makeQueryFunc(stc.args.queryFuncGasUsed, stc.args.queryFuncWantErr)
simRes, gotAdjusted, err := tx.CalculateGas(queryFunc, txf.WithGasAdjustment(stc.args.adjustment))
mockClientCtx := mockContext{
gasUsed: tc.args.mockGasUsed,
wantErr: tc.args.mockWantErr,
}
simRes, gotAdjusted, err := tx.CalculateGas(mockClientCtx, txf.WithGasAdjustment(stc.args.adjustment))
if stc.expPass {
require.NoError(t, err)
require.Equal(t, simRes.GasInfo.GasUsed, stc.wantEstimate)
require.Equal(t, gotAdjusted, stc.wantAdjusted)
require.NotNil(t, simRes.Result)
} else {
require.Error(t, err)
require.Nil(t, simRes.Result)
require.Nil(t, simRes)
}
})
}
Expand Down
3 changes: 2 additions & 1 deletion docs/core/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -7256,7 +7256,8 @@ RPC method.

| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `tx` | [Tx](#cosmos.tx.v1beta1.Tx) | | tx is the transaction to simulate. |
| `tx` | [Tx](#cosmos.tx.v1beta1.Tx) | | **Deprecated.** tx is the transaction to simulate. Deprecated. Send raw tx bytes instead. |
| `tx_bytes` | [bytes](#bytes) | | tx_bytes is the raw transaction. |



Expand Down
5 changes: 4 additions & 1 deletion proto/cosmos/tx/v1beta1/service.proto
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,10 @@ message BroadcastTxResponse {
// RPC method.
message SimulateRequest {
// tx is the transaction to simulate.
cosmos.tx.v1beta1.Tx tx = 1;
// Deprecated. Send raw tx bytes instead.
cosmos.tx.v1beta1.Tx tx = 1 [deprecated=true];
// tx_bytes is the raw transaction.
bytes tx_bytes = 2;
}

// SimulateResponse is the response type for the
Expand Down
3 changes: 0 additions & 3 deletions simapp/simd/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/cosmos/cosmos-sdk/snapshots"
"github.com/cosmos/cosmos-sdk/store"
sdk "github.com/cosmos/cosmos-sdk/types"
authclient "github.com/cosmos/cosmos-sdk/x/auth/client"
authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli"
"github.com/cosmos/cosmos-sdk/x/auth/types"
vestingcli "github.com/cosmos/cosmos-sdk/x/auth/vesting/client/cli"
Expand Down Expand Up @@ -66,8 +65,6 @@ func NewRootCmd() (*cobra.Command, params.EncodingConfig) {
}

func initRootCmd(rootCmd *cobra.Command, encodingConfig params.EncodingConfig) {
authclient.Codec = encodingConfig.Marshaler

rootCmd.AddCommand(
genutilcli.InitCmd(simapp.ModuleBasics, simapp.DefaultNodeHome),
genutilcli.CollectGenTxsCmd(banktypes.GenesisBalancesIterator{}, simapp.DefaultNodeHome),
Expand Down
Loading