Migrate {x/auth, x/gov, x/staking} missing CLI queries to proto #6994

sahith-narahari · 2020-08-09T17:01:22Z

Description

This PR updates queries using QueryByTxs to use proto.
Also:

tx.Tx (the proto Message) now implements sdk.Tx

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer
Review Codecov Report in the comment section below once CI passes

codecov · 2020-08-09T17:06:31Z

Codecov Report

Merging #6994 into master will decrease coverage by 0.02%.
The diff coverage is 35.84%.

@@            Coverage Diff             @@
##           master    #6994      +/-   ##
==========================================
- Coverage   55.17%   55.15%   -0.03%     
==========================================
  Files         584      584              
  Lines       40538    40504      -34     
==========================================
- Hits        22367    22340      -27     
+ Misses      16301    16290      -11     
- Partials     1870     1874       +4

…ate-query-cli

sahith-narahari · 2020-08-17T15:51:46Z

Blocked on #7067, will update once I add tests.

…ate-query-cli

…k into sahith/update-query-cli

alexanderbez

tested and not working.

create a network ($ make clean localnet-start)
send a tx
query for that tx by events

$ simd q txs --events="message.sender=cosmos13dtdcpyv0z2tda9vcv6e8unamkkah3zhxyafn8"
Error: cannot protobuf JSON encode unsupported type: *types.SearchTxsResult
Usage:
  simd query txs [flags]

Flags:
      --events string            list of transaction events in the form of {eventType}.{eventAttribute}={value}
  -h, --help                     help for txs
      --keyring-backend string   Select keyring's backend (os|file|kwallet|pass|test) (default "os")
      --limit int                Query number of transactions results per page returned (default 30)
  -n, --node string              Node to connect to (default "tcp://localhost:26657")
      --page int                 Query a specific page of paginated results (default 1)

Global Flags:
      --chain-id string   The network chain ID
      --home string       directory for config and data (default "/Users/aleksbez/.simapp")
      --trace             print out full stack trace on errors

alexanderbez · 2020-08-25T13:19:56Z

Also, querying by hash fails too:

$ simd q tx 5A07FC41325447C33D856A9122C93DEA83479C5F1596C9375427493B10C7D9A0
Error: unknown message type ""
Usage:
  simd query tx [hash] [flags]

Flags:
  -h, --help                     help for tx
      --keyring-backend string   Select keyring's backend (os|file|kwallet|pass|test) (default "os")
  -n, --node string              Node to connect to (default "tcp://localhost:26657")

Global Flags:
      --chain-id string   The network chain ID
      --home string       directory for config and data (default "/Users/aleksbez/.simapp")
      --trace             print out full stack trace on errors

anilcse · 2020-08-25T18:10:16Z

$ simd q txs --events="message.sender=cosmos13dtdcpyv0z2tda9vcv6e8unamkkah3zhxyafn8"
Error: cannot protobuf JSON encode unsupported type: *types.SearchTxsResult

Migrated SearchTxsResult to proto but still getting empty response from node.TxSearch.

…ate-query-cli

…k into sahith/update-query-cli

x/ibc/04-channel/client/cli/query.go

…mos-sdk into am-update-query-cli

amaury1093 · 2020-09-07T16:56:43Z

client/grpc/simulate/simulate_test.go

+	any, ok := txBuilder.(codectypes.IntoAny)
+	s.Require().True(ok)
+	cached := any.AsAny().GetCachedValue()
+	txTx, ok := cached.(*txtypes.Tx)
+	s.Require().True(ok)
 	res, err := s.queryClient.Simulate(
 		context.Background(),
-		&simulate.SimulateRequest{Tx: txBuilder.GetProtoTx()},
+		&simulate.SimulateRequest{Tx: txTx},


GetProtoTx() is a little bit more verbose, but at least it makes sense in the StdTx case too.

amaury1093 · 2020-09-07T17:02:04Z

types/tx/types.go

 var _, _ codectypes.UnpackInterfacesMessage = &Tx{}, &TxBody{}
+var _ sdk.Tx = &Tx{}


We can remove WrapTx now that tx.Tx implements sdk.Tx.

However, in this PR, I just modified the minimal code on Tx to make it work. For clarity, I propose to leave it to another PR (I can do it) to remove WrapTx, and also add GetGas on tx.Tx to make simulation work.

alexanderbez

Looks good! I think some of the tx indirection between types can get a bit confusing.

Also, all the tx wrapper stuff is being removed, correct?

client/tx/tx.go

alexanderbez · 2020-09-07T19:27:52Z

codec/types/compat.go

@@ -50,7 +50,7 @@ func (any *Any) UnmarshalAmino(bz []byte) error {
 	return nil
 }

-func (any Any) MarshalJSON() ([]byte, error) {
+func (any *Any) MarshalJSON() ([]byte, error) {


Why do we need a pointer reference when marshaling?

Using Any, any.compat (just below) is nil. Using *Any, any.compat is correct. I'm not 100% I understand why; it's set on this line btw.

I didn't dig too much into it, but anyways, without the reference, tests fail.

@amaurymartiny , this could be some weird concurrency issue.

If you use Any then a method receiver works on an object copy. Sometimes it's a source of errors in methods which modifies the object. Here all methods which modifies the object are using pointer receivers. So that's why my guess is that tests are doing something weird in parallel.

alexanderbez · 2020-09-07T19:31:07Z

types/tx/types.go

 var _, _ codectypes.UnpackInterfacesMessage = &Tx{}, &TxBody{}
+var _ sdk.Tx = &Tx{}


x/auth/client/cli/cli_test.go

amaury1093

I worked a bit on this PR, so approving.

Fix error code

e353cd0

anilcse changed the title ~~Migrate pending cli queries~~ Migrate pending cli queries to proto Aug 10, 2020

sahith-narahari added 2 commits August 13, 2020 13:04

Merge branch 'master' of github.com:cosmos/cosmos-sdk into sahith/upd…

93fab54

…ate-query-cli

Merge branch 'master' of github.com:cosmos/cosmos-sdk into sahith/upd…

237c08f

…ate-query-cli

anilcse added 9 commits August 18, 2020 00:30

Merge branch 'master' into sahith/update-query-cli

a8e1550

Merge branch 'master' of github.com:cosmos/cosmos-sdk into sahith/upd…

ce925c3

…ate-query-cli

Merge branch 'master' into sahith/update-query-cli

97e1d87

Merge branch 'sahith/update-query-cli' of github.com:cosmos/cosmos-sd…

647d146

…k into sahith/update-query-cli

Fix decoder

bc70575

Fix typo

e564ed1

Fix decode

f34a03b

refactor

810fcac

Merge branch 'master' into sahith/update-query-cli

176592c

anilcse marked this pull request as ready for review August 24, 2020 19:19

anilcse requested review from aaronc, alessio and alexanderbez as code owners August 24, 2020 19:19

anilcse requested review from amaury1093, atheeshp and blushi August 24, 2020 19:20

anilcse self-assigned this Aug 24, 2020

anilcse mentioned this pull request Aug 24, 2020

Migrate remaining CLI cmd's to proto #6987

Closed

8 tasks

alexanderbez suggested changes Aug 25, 2020

View reviewed changes

Migrate SearchTxsResult to proto

d8d3264

alexanderbez and others added 2 commits August 25, 2020 14:16

Merge branch 'master' into sahith/update-query-cli

d899405

fix MarkEventsToIndex

706bacc

anilcse changed the title ~~Migrate {x/auth, x/gov, x/staking} missing CLI queries to use proto~~ Migrate {x/auth, x/gov, x/staking} missing CLI queries to proto Sep 5, 2020

anilcse added 3 commits September 6, 2020 01:37

Merge branch 'master' of github.com:cosmos/cosmos-sdk into sahith/upd…

c8815e7

…ate-query-cli

Fix ibc CLI to use proto

e08c852

Merge branch 'sahith/update-query-cli' of github.com:cosmos/cosmos-sd…

1aa75c4

…k into sahith/update-query-cli

anilcse reviewed Sep 5, 2020

View reviewed changes

x/ibc/04-channel/client/cli/query.go Show resolved Hide resolved

amaury1093 added 6 commits September 7, 2020 11:46

Merge branch 'sahith/update-query-cli' of ssh://github.com/cosmos/cos…

bed75a8

…mos-sdk into am-update-query-cli

Fix any MarshalJSON

aa9b78f

Fix test

1ea476f

Make tx.Tx implement sdk.Tx

cf17bcd

Register sdk.Tx

8fb572d

Fix lint

85a92e9

amaury1093 reviewed Sep 7, 2020

View reviewed changes

Allow DefaultJSONTxEncoder to take tx.Tx

8b96e49

alexanderbez suggested changes Sep 7, 2020

View reviewed changes

anilcse and others added 3 commits September 8, 2020 01:43

refactor

c2e6f7f

Merge branch 'master' into sahith/update-query-cli

97269db

Rename variable

afca789

alexanderbez approved these changes Sep 8, 2020

View reviewed changes

Merge branch 'master' into sahith/update-query-cli

598be43

clevinson mentioned this pull request Sep 8, 2020

Tracking Issue: Stargate Release Candidate #7152

Closed

25 tasks

Merge branch 'master' into sahith/update-query-cli

1a406c9

anilcse added the C:CLI label Sep 8, 2020

anilcse added 2 commits September 10, 2020 14:35

Merge branch 'master' into sahith/update-query-cli

3ec3d52

remove fmt

ff49992

anilcse approved these changes Sep 10, 2020

View reviewed changes

anilcse requested a review from blushi September 10, 2020 09:44

amaury1093 approved these changes Sep 10, 2020

View reviewed changes

amaury1093 added the A:automerge Automatically merge PR once all prerequisites pass. label Sep 10, 2020

mergify bot merged commit b234818 into master Sep 10, 2020

mergify bot deleted the sahith/update-query-cli branch September 10, 2020 18:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate {x/auth, x/gov, x/staking} missing CLI queries to proto #6994

Migrate {x/auth, x/gov, x/staking} missing CLI queries to proto #6994

sahith-narahari commented Aug 9, 2020 •

edited by amaury1093

Loading

codecov bot commented Aug 9, 2020 •

edited

Loading

sahith-narahari commented Aug 17, 2020

alexanderbez left a comment

alexanderbez commented Aug 25, 2020

anilcse commented Aug 25, 2020

amaury1093 Sep 7, 2020

amaury1093 Sep 7, 2020

alexanderbez Sep 7, 2020

alexanderbez left a comment

alexanderbez Sep 7, 2020

amaury1093 Sep 8, 2020

robert-zaremba Sep 8, 2020

alexanderbez Sep 7, 2020

amaury1093 left a comment

		var _, _ codectypes.UnpackInterfacesMessage = &Tx{}, &TxBody{}
		var _ sdk.Tx = &Tx{}

Migrate {x/auth, x/gov, x/staking} missing CLI queries to proto #6994

Migrate {x/auth, x/gov, x/staking} missing CLI queries to proto #6994

Conversation

sahith-narahari commented Aug 9, 2020 • edited by amaury1093 Loading

Description

codecov bot commented Aug 9, 2020 • edited Loading

Codecov Report

sahith-narahari commented Aug 17, 2020

alexanderbez left a comment

Choose a reason for hiding this comment

alexanderbez commented Aug 25, 2020

anilcse commented Aug 25, 2020

amaury1093 Sep 7, 2020

Choose a reason for hiding this comment

amaury1093 Sep 7, 2020

Choose a reason for hiding this comment

alexanderbez Sep 7, 2020

Choose a reason for hiding this comment

alexanderbez left a comment

Choose a reason for hiding this comment

alexanderbez Sep 7, 2020

Choose a reason for hiding this comment

amaury1093 Sep 8, 2020

Choose a reason for hiding this comment

robert-zaremba Sep 8, 2020

Choose a reason for hiding this comment

alexanderbez Sep 7, 2020

Choose a reason for hiding this comment

amaury1093 left a comment

Choose a reason for hiding this comment

sahith-narahari commented Aug 9, 2020 •

edited by amaury1093

Loading

codecov bot commented Aug 9, 2020 •

edited

Loading