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

R4R: New broadcast command #2240

Merged
merged 3 commits into from
Sep 8, 2018
Merged

R4R: New broadcast command #2240

merged 3 commits into from
Sep 8, 2018

Conversation

alessio
Copy link
Contributor

@alessio alessio commented Sep 5, 2018

Implement broadcast command/REST endpoint to submit transactions generated offline with --generated-only and the sign command.

Closes: #1954

  • Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
  • Wrote tests
  • Updated relevant documentation (docs/)
  • Added entries in PENDING.md with issue #
  • 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
Copy link

codecov bot commented Sep 5, 2018

Codecov Report

Merging #2240 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #2240   +/-   ##
========================================
  Coverage    63.74%   63.74%           
========================================
  Files          140      140           
  Lines         8645     8645           
========================================
  Hits          5511     5511           
  Misses        2755     2755           
  Partials       379      379

@alessio alessio force-pushed the alessio/1954-submit-signed-tx branch 8 times, most recently from 2f85aaf to 1cf2d5f Compare September 7, 2018 15:06
Copy link
Collaborator

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

Left some comments. Mostly naming, tests and docs. Logic seems fine.

PENDING.md Outdated Show resolved Hide resolved
cmd/gaia/cli_test/cli_test.go Show resolved Hide resolved
cmd/gaia/cli_test/cli_test.go Outdated Show resolved Hide resolved
x/bank/client/cli/broadcast.go Show resolved Hide resolved
x/bank/client/rest/sendtx.go Outdated Show resolved Hide resolved
x/bank/client/cli/broadcast.go Show resolved Hide resolved
x/bank/client/rest/sendtx.go Outdated Show resolved Hide resolved
@jackzampolin
Copy link
Member

Lets vote:

  • 😄/tx/broadcast
  • 🎉/broadcast
  • ❤️/broadcast-tx

@alessio alessio force-pushed the alessio/1954-submit-signed-tx branch 2 times, most recently from f4c46a7 to f0c27ed Compare September 8, 2018 01:14
@alessio alessio changed the title WIP: Alessio/1954 submit signed tx R4R: New broadcast command Sep 8, 2018
@alessio alessio added prelaunch and removed wip labels Sep 8, 2018
@alessio alessio force-pushed the alessio/1954-submit-signed-tx branch from 28afbda to 8ee7a52 Compare September 8, 2018 06:58
@alessio alessio requested a review from ValarDragon September 8, 2018 08:21
@cwgoes
Copy link
Contributor

cwgoes commented Sep 8, 2018

@alessio FYI, you don't need to squash while writing the PR, that can be done by the reviewer at the end.

Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

Quick comments.

cmd/gaia/cli_test/cli_test.go Show resolved Hide resolved
x/bank/client/cli/broadcast.go Show resolved Hide resolved
Implement broadcast command/REST endpoint to submit transactions
generated offline with --generated-only and the sign command.

Closes: #1954
@alessio alessio force-pushed the alessio/1954-submit-signed-tx branch from 8708c44 to 4c8cd38 Compare September 8, 2018 08:58
@cwgoes cwgoes dismissed fedekunze’s stale review September 8, 2018 09:19

Comments addressed by @alessio

@cwgoes cwgoes merged commit 4448d17 into develop Sep 8, 2018
@cwgoes cwgoes deleted the alessio/1954-submit-signed-tx branch September 8, 2018 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants