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 1 commit
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
Prev Previous commit
Next Next commit
Remove protoTxProvider
  • Loading branch information
amaury1093 committed Mar 22, 2021
commit ca088c1b37a0cb78ab876422e3e2bc60d1efd060
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [\#8346](https://github.com/cosmos/cosmos-sdk/pull/8346) All CLI `tx` commands generate ServiceMsgs by default. Graceful Amino support has been added to ServiceMsgs to support signing legacy Msgs.
* (crypto/ed25519) [\#8690] Adopt zip1215 ed2559 verification rules.
* [\#8849](https://github.com/cosmos/cosmos-sdk/pull/8849) Upgrade module no longer supports time based upgrades.
* (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.
* [\#8880](https://github.com/cosmos/cosmos-sdk/pull/8880) The CLI `simd migrate v0.40 ...` command has been renamed to `simd migrate v0.42`.

* (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.

### API Breaking Changes

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
6 changes: 0 additions & 6 deletions x/auth/tx/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ var (
_ client.TxBuilder = &wrapper{}
_ ante.HasExtensionOptionsTx = &wrapper{}
_ ExtensionOptionsTxBuilder = &wrapper{}
_ ProtoTxProvider = &wrapper{}
)

// ExtensionOptionsTxBuilder defines a TxBuilder that can also set extensions.
Expand Down Expand Up @@ -356,8 +355,3 @@ func (w *wrapper) SetNonCriticalExtensionOptions(extOpts ...*codectypes.Any) {
w.tx.Body.NonCriticalExtensionOptions = extOpts
w.bodyBz = nil
}

// ProtoTxProvider is a type which can provide a proto transaction.
type ProtoTxProvider interface {
GetProtoTx() *tx.Tx
}
Comment on lines -360 to -363
Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @robert-zaremba, I removed this workaround, it's not needed anymore.

20 changes: 15 additions & 5 deletions x/auth/tx/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,21 @@ import (
"fmt"
"testing"

"github.com/cosmos/cosmos-sdk/testutil/testdata"

"github.com/stretchr/testify/suite"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
clienttx "github.com/cosmos/cosmos-sdk/client/tx"
"github.com/cosmos/cosmos-sdk/simapp"
"github.com/cosmos/cosmos-sdk/testutil/network"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/types/query"
"github.com/cosmos/cosmos-sdk/types/rest"
"github.com/cosmos/cosmos-sdk/types/tx"
"github.com/cosmos/cosmos-sdk/types/tx/signing"
authclient "github.com/cosmos/cosmos-sdk/x/auth/client"
authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
bankcli "github.com/cosmos/cosmos-sdk/x/bank/client/testutil"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
)
Expand Down Expand Up @@ -79,10 +78,14 @@ func (s *IntegrationTestSuite) TearDownSuite() {
}

func (s IntegrationTestSuite) TestSimulateTx_GRPC() {
encCfg := simapp.MakeTestEncodingConfig()
txBuilder := s.mkTxBuilder()
// Convert the txBuilder to a tx.Tx.
protoTx, err := txBuilderToProtoTx(txBuilder)
s.Require().NoError(err)
// Encode the txBuilder to txBytes.
txBytes, err := encCfg.TxConfig.TxEncoder()(txBuilder.GetTx())
s.Require().NoError(err)

testCases := []struct {
name string
Expand All @@ -92,7 +95,8 @@ func (s IntegrationTestSuite) TestSimulateTx_GRPC() {
}{
{"nil request", nil, true, "request cannot be nil"},
{"empty request", &tx.SimulateRequest{}, true, "empty txBytes is not allowed"},
{"valid request", &tx.SimulateRequest{Tx: protoTx}, false, ""},
{"valid request with proto tx (deprecated)", &tx.SimulateRequest{Tx: protoTx}, false, ""},
{"valid request with tx_bytes", &tx.SimulateRequest{TxBytes: txBytes}, false, ""},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -493,9 +497,15 @@ func (s IntegrationTestSuite) mkTxBuilder() client.TxBuilder {
return txBuilder
}

// protoTxProvider is a type which can provide a proto transaction. It is a
// workaround to get access to the wrapper TxBuilder's method GetProtoTx().
type protoTxProvider interface {
GetProtoTx() *tx.Tx
}

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