R4R: Implement Tx Decode in REST/CLI Clients #4213

yangyanqing · 2019-04-27T08:33:14Z

Targeted PR against correct branch (see CONTRIBUTING.md)

close: #3872
replace: #4026

Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
Wrote tests
Updated relevant documentation (docs/)
Added a relevant changelog entry: sdkch add [section] [stanza] [message]
rereviewed Files changed in the github PR explorer

For Admin Use:

Added appropriate labels to PR (ex. wip, ready-for-review, docs)
Reviewers Assigned
Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

codecov · 2019-04-27T09:03:14Z

Codecov Report

Merging #4213 into master will decrease coverage by 0.02%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #4213      +/-   ##
==========================================
- Coverage   60.16%   60.13%   -0.03%     
==========================================
  Files         212      212              
  Lines       15188    15192       +4     
==========================================
- Hits         9138     9136       -2     
- Misses       5421     5427       +6     
  Partials      629      629

codecov · 2019-04-27T09:03:14Z

Codecov Report

Merging #4213 into master will increase coverage by 0.62%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #4213      +/-   ##
==========================================
+ Coverage   54.15%   54.78%   +0.62%     
==========================================
  Files         272      269       -3     
  Lines       17349    16911     -438     
==========================================
- Hits         9395     9264     -131     
+ Misses       7271     6965     -306     
+ Partials      683      682       -1

alexanderbez

ACK

yangyanqing · 2019-05-10T10:01:08Z

Do you mind review this PR ? @jackzampolin @alessio

rigelrozanski

looks good

alessio

Hold off please - this is blocked by the gaia split migration

yangyanqing · 2019-05-17T06:46:53Z

Hold off please - this is blocked by the gaia split migration

Are you referring to #4347 ? @alessio

jackzampolin · 2019-06-16T11:23:39Z

@yangyanqing if you could bring this PR back up to the latest we will merge it!

yangyanqing · 2019-06-25T12:53:21Z

I have no idea about the cycle import error caused by moving decode.go from client/tx to x/auth/client/rest/.
@jackzampolin

➜  cosmos-sdk git:(frank/3872-gaiacli-tx-decode-master) ✗ make build
import cycle not allowed
package github.com/cosmos/cosmos-sdk/simapp
        imports github.com/cosmos/cosmos-sdk/x/auth
        imports github.com/cosmos/cosmos-sdk/x/auth/client/rest
        imports github.com/cosmos/cosmos-sdk/x/auth
can't load package: import cycle not allowed
package github.com/cosmos/cosmos-sdk/simapp
        imports github.com/cosmos/cosmos-sdk/x/auth
        imports github.com/cosmos/cosmos-sdk/x/auth/client/rest
        imports github.com/cosmos/cosmos-sdk/x/auth
Makefile:28: recipe for target 'build' failed
make: *** [build] Error 1

client/tx/decode.go

x/auth/client/rest/decode.go

yangyanqing · 2019-07-01T05:45:59Z

Can this PR be merged ? @jackzampolin
client/lcd/statik/statik.go would be conflict if any one update swagger doc.

alexanderbez

Looks good @yangyanqing. Left some further feedback. Also statik.go needs to be regenerated.

x/auth/client/utils/tx.go

yangyanqing · 2019-07-02T15:38:15Z

Looks good @yangyanqing. Left some further feedback. Also statik.go needs to be regenerated.

statik.go is too easy to be conflicted. Maybe it can be generate at the first step of compile.

alexanderbez · 2019-07-02T16:07:48Z

@yangyanqing not true, it's very easy:

$ rm -fr client/lcd/statik/statik.go; make update-swagger-docs

Simply take preference over the new statik.go file. It's that simple. No need to resolve any conflicts manually (assuming you're rebased off of the latest master).

yangyanqing · 2019-07-05T07:32:37Z

@yangyanqing not true, it's very easy:
$ rm -fr client/lcd/statik/statik.go; make update-swagger-docs
Simply take preference over the new statik.go file. It's that simple. No need to resolve any conflicts manually (assuming you're rebased off of the latest master).

Yes, I had repeated the steps three times.
Because someone else also modified statik.go and be merged before my PR, then my PR was conflicted.
@alexanderbez

client/lcd/swagger-ui/swagger.yaml

alexanderbez · 2019-07-05T14:11:34Z

@yangyanqing so long as your PR has the latest master, you should always overwrite. Conflicts can be annoying yes, but it's trivial to run that command.

Co-Authored-By: yangyanqing <f.yang@hashgard.com>

➜ cosmos-sdk git:(frank/3872-gaiacli-tx-decode-master) ✗ make build import cycle not allowed package github.com/cosmos/cosmos-sdk/simapp imports github.com/cosmos/cosmos-sdk/x/auth imports github.com/cosmos/cosmos-sdk/x/auth/client/rest imports github.com/cosmos/cosmos-sdk/x/auth can't load package: import cycle not allowed package github.com/cosmos/cosmos-sdk/simapp imports github.com/cosmos/cosmos-sdk/x/auth imports github.com/cosmos/cosmos-sdk/x/auth/client/rest imports github.com/cosmos/cosmos-sdk/x/auth Makefile:28: recipe for target 'build' failed make: *** [build] Error 1

Co-Authored-By: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>

fedekunze

1 comment and pending CI fix

fedekunze · 2019-07-31T19:03:22Z

x/auth/client/rest/decode.go

+
+// GetDecodeCommand returns the decode command to take Amino-serialized bytes and turn it into
+// a JSONified transaction
+func GetDecodeCommand(codec *amino.Codec) *cobra.Command {


this should live in auth/client/CLI

fedekunze · 2019-08-26T13:29:15Z

closing this PR as it hasn't been updated in a while. I'll reopen a new PR with these changes later today

yangyanqing requested review from alessio, alexanderbez, jackzampolin and rigelrozanski as code owners April 27, 2019 08:33

alexanderbez approved these changes Apr 27, 2019

View reviewed changes

alexanderbez added C:CLI ready-for-review labels Apr 27, 2019

rigelrozanski approved these changes May 16, 2019

View reviewed changes

alessio suggested changes May 16, 2019

View reviewed changes

Onefox mentioned this pull request May 22, 2019

Decode can not decodeSignedVarint invalid input cybercongress/js-amino#44

Open

yangyanqing changed the title ~~R4R: Implement Tx Decode in REST/CLI Clients~~ WIP: Implement Tx Decode in REST/CLI Clients Jun 25, 2019

alexanderbez reviewed Jun 25, 2019

View reviewed changes

client/tx/decode.go Outdated Show resolved Hide resolved

client/tx/decode.go Outdated Show resolved Hide resolved

x/auth/client/rest/decode.go Outdated Show resolved Hide resolved

x/auth/client/rest/decode.go Show resolved Hide resolved

yangyanqing changed the title ~~WIP: Implement Tx Decode in REST/CLI Clients~~ R4R: Implement Tx Decode in REST/CLI Clients Jun 27, 2019

alexanderbez suggested changes Jul 1, 2019

View reviewed changes

x/auth/client/utils/tx.go Outdated Show resolved Hide resolved

tac0turtle reviewed Jul 5, 2019

View reviewed changes

client/lcd/swagger-ui/swagger.yaml Outdated Show resolved Hide resolved

Frank Yang added 3 commits July 11, 2019 20:02

gaiacli tx decode #3872

fdf02a8

Modify Short usage and delete Long.

6c9b762

add pendings

a05f423

Frank Yang and others added 10 commits July 11, 2019 20:02

Change response to auth.StdTx.

f9e8676

Update .pending/features/gaiarest/3872-Add-POST-txs-de

30c5bbd

Co-Authored-By: yangyanqing <f.yang@hashgard.com>

Update .pending/features/gaiacli/3872-Add-gaiacli-tx-

2ddcd4d

Co-Authored-By: yangyanqing <f.yang@hashgard.com>

Update x/auth/client/rest/decode.go

e964938

Co-Authored-By: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>

Fix build error.

40015bb

correct err process

8d2897c

update swagger docs

b19c244

Fix swagger

1a78795

Fix swagger conflict

c3b914c

fedekunze suggested changes Jul 31, 2019

View reviewed changes

fedekunze changed the base branch from master to fedekunze/3872-tx-decode August 6, 2019 16:54

fedekunze closed this Aug 26, 2019

tnachen mentioned this pull request Aug 30, 2019

gaiacli tx decode #3872

Closed

tnachen mentioned this pull request Sep 10, 2019

Implement Gaia Tx Decode #5023

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

R4R: Implement Tx Decode in REST/CLI Clients #4213

R4R: Implement Tx Decode in REST/CLI Clients #4213

yangyanqing commented Apr 27, 2019 •

edited by fedekunze

Loading

codecov bot commented Apr 27, 2019

codecov bot commented Apr 27, 2019 •

edited

Loading

alexanderbez left a comment

yangyanqing commented May 10, 2019

rigelrozanski left a comment

alessio left a comment

yangyanqing commented May 17, 2019

jackzampolin commented Jun 16, 2019

yangyanqing commented Jun 25, 2019 •

edited

Loading

yangyanqing commented Jul 1, 2019

alexanderbez left a comment

yangyanqing commented Jul 2, 2019

alexanderbez commented Jul 2, 2019 •

edited

Loading

yangyanqing commented Jul 5, 2019

alexanderbez commented Jul 5, 2019

fedekunze left a comment

fedekunze Jul 31, 2019

fedekunze commented Aug 26, 2019

R4R: Implement Tx Decode in REST/CLI Clients #4213

R4R: Implement Tx Decode in REST/CLI Clients #4213

Conversation

yangyanqing commented Apr 27, 2019 • edited by fedekunze Loading

codecov bot commented Apr 27, 2019

Codecov Report

codecov bot commented Apr 27, 2019 • edited Loading

Codecov Report

alexanderbez left a comment

Choose a reason for hiding this comment

yangyanqing commented May 10, 2019

rigelrozanski left a comment

Choose a reason for hiding this comment

alessio left a comment

Choose a reason for hiding this comment

yangyanqing commented May 17, 2019

jackzampolin commented Jun 16, 2019

yangyanqing commented Jun 25, 2019 • edited Loading

yangyanqing commented Jul 1, 2019

alexanderbez left a comment

Choose a reason for hiding this comment

yangyanqing commented Jul 2, 2019

alexanderbez commented Jul 2, 2019 • edited Loading

yangyanqing commented Jul 5, 2019

alexanderbez commented Jul 5, 2019

fedekunze left a comment

Choose a reason for hiding this comment

fedekunze Jul 31, 2019

Choose a reason for hiding this comment

fedekunze commented Aug 26, 2019

yangyanqing commented Apr 27, 2019 •

edited by fedekunze

Loading

codecov bot commented Apr 27, 2019 •

edited

Loading

yangyanqing commented Jun 25, 2019 •

edited

Loading

alexanderbez commented Jul 2, 2019 •

edited

Loading