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 22 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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/upgrade) [\#8743](https://github.com/cosmos/cosmos-sdk/pull/8743) `UpgradeHandler` includes a new argument `VersionMap` which helps facilitate in-place migrations.
* (x/auth) [\#8129](https://github.com/cosmos/cosmos-sdk/pull/8828) Updated `SigVerifiableTx.GetPubKeys` method signature to return error.
* [\#8682](https://github.com/cosmos/cosmos-sdk/pull/8682) `ante.NewAnteHandler` updated to receive all positional params as `ante.HandlerOptions` struct. If required fields aren't set, throws error accordingly.
* (client) [\#8926](https://github.com/cosmos/cosmos-sdk/pull/8926) `client/tx.PrepareFactory` has been converted to a private function, as it's only used internally.
* (auth/tx) [\#8926](https://github.com/cosmos/cosmos-sdk/pull/8926) The `ProtoTxProvider` interface used as a workaround for transaction simulation has been removed.


### State Machine Breaking
Expand Down Expand Up @@ -95,6 +97,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (server) [\#8399](https://github.com/cosmos/cosmos-sdk/pull/8399) fix gRPC-web flag default value
* (crypto) [\#8841](https://github.com/cosmos/cosmos-sdk/pull/8841) Fix legacy multisig amino marshaling, allowing migrations to work between v0.39 and v0.40+.

### Deprecated

* (grpc) [\#8926](https://github.com/cosmos/cosmos-sdk/pull/8926) The `tx` field in `SimulateRequest` has been deprecated, prefer to pass `tx_bytes` instead.

## [v0.42.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.42.0) - 2021-03-08

**IMPORTANT**: This release contains an important security fix for all non Cosmos Hub chains running Stargate version of the Cosmos SDK (>0.40). Non-hub chains should not be using any version of the SDK in the v0.40.x or v0.41.x release series. See [#8461](https://github.com/cosmos/cosmos-sdk/pull/8461) for more details.
Expand Down
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 @@ -7257,7 +7257,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
20 changes: 4 additions & 16 deletions docs/run-node/txs.md
Original file line number Diff line number Diff line change
Expand Up @@ -305,15 +305,13 @@ func simulateTx() error {
// Simulate the tx via gRPC. We create a new client for the Protobuf Tx
// service.
txClient := tx.NewServiceClient(grpcConn)
// We then call the BroadcastTx method on this client.
protoTx := txBuilderToProtoTx(txBuilder)
if err != nil {
return err
}
txBytes := /* Fill in with your signed transaction bytes. */

// We then call the Simulate method on this client.
grpcRes, err := txClient.Simulate(
context.Background(),
&tx.SimulateRequest{
Tx: protoTx,
TxBytes: txBytes,
},
)
if err != nil {
Expand All @@ -324,16 +322,6 @@ func simulateTx() error {

return nil
}

// txBuilderToProtoTx converts a txBuilder into a proto tx.Tx.
func txBuilderToProtoTx(txBuilder client.TxBuilder) (*tx.Tx, error) { // nolint
protoProvider, ok := txBuilder.(authtx.ProtoTxProvider)
if !ok {
return nil, fmt.Errorf("expected proto tx builder, got %T", txBuilder)
}

return protoProvider.GetProtoTx(), nil
}
```

## Using REST
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