Remove Tx Broadcast and Commit Support #3954

alexanderbez · 2019-03-20T23:25:57Z

Replace --async flag with --broadcast-mode flag. This flag defaults to sync which returns once CheckTx execution is finished. This is inline with the way nearly any other blockchain operates (e.g. broadcast and keep checking when it's included in a block).
- The flag still supports block (commit) mode but this is mainly due to unit/integration tests. A doc entry has been added to stipulate this mode should not be used.
Add a Raw Log entry to the TxResponse. This is not JSON-encoded, but will allow getting a log entry when a tx fails pre message execution.

closes: #3875
closes: #3941

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.
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-03-21T13:57:27Z

Codecov Report

Merging #3954 into develop will decrease coverage by 0.12%.
The diff coverage is 20%.

@@             Coverage Diff             @@
##           develop    #3954      +/-   ##
===========================================
- Coverage    60.33%   60.21%   -0.13%     
===========================================
  Files          196      196              
  Lines        14533    14557      +24     
===========================================
- Hits          8768     8765       -3     
- Misses        5181     5208      +27     
  Partials       584      584

fedekunze

utACK

docs/gaia/gaiacli.md

jackzampolin · 2019-03-25T19:06:04Z

types/result.go

@@ -68,6 +68,7 @@ type TxResponse struct {
 	TxHash    string          `json:"txhash"`
 	Code      uint32          `json:"code,omitempty"`
 	Data      []byte          `json:"data,omitempty"`
+	RawLog    string          `json:"raw_log,omitempty"`


jackzampolin · 2019-03-25T19:18:11Z

types/result.go

 func NewResponseFormatBroadcastTxCommit(res *ctypes.ResultBroadcastTxCommit) TxResponse {
+	if !res.CheckTx.IsOK() {


++ much better here.

cmd/gaia/cli_test/test_helpers.go

jackzampolin · 2019-03-25T19:20:11Z

client/context/broadcast.go

-		}
-		return
-	}
+	switch ctx.BroadcastMode {


jackzampolin · 2019-03-25T19:21:12Z

Left a few comments. Would really like to see the ability to set this via gaiacli config but not stuck on it (could be another PR). Otherwise LGTM!

jackzampolin · 2019-03-25T19:43:58Z

Another item bordering on this that might want to be a separate PR as well is short flags for --account-number --sequence- and --chain-id

alexanderbez · 2019-03-25T21:05:53Z

Another item bordering on this that might want to be a separate PR as well is short flags for --account-number --sequence- and --chain-id

yup, that's addressed in #3970.

Onefox · 2019-04-12T13:13:35Z

The swagger doc is not up to date here:
https://cosmos.network/rpc/#/ICS0/post_txs it should be mode now as param instead of return

alexanderbez · 2019-04-12T23:15:53Z

@Onefox

The Swagger doc is updated:

cosmos-sdk/client/lcd/swagger-ui/swagger.yaml

Lines 227 to 291 in 8550d87

    
           /txs: 
        
             get: 
        
               tags: 
        
                 - ICS0 
        
               summary: Search transactions 
        
               description: Search transactions by tag(s). 
        
               produces: 
        
                 - application/json 
        
               parameters: 
        
                 - in: query 
        
                   name: tag 
        
                   type: string 
        
                   description: "transaction tags such as 'action=submit-proposal' and 'sender=cosmos1g9ahr6xhht5rmqven628nklxluzyv8z9jqjcmc' which results in the following endpoint: 'GET /txs?action=submit-proposal&sender=cosmos1g9ahr6xhht5rmqven628nklxluzyv8z9jqjcmc'" 
        
                   required: true 
        
                   x-example: 'TODO' 
        
                 - in: query 
        
                   name: page 
        
                   description: Page number 
        
                   type: integer 
        
                   x-example: 1 
        
                 - in: query 
        
                   name: limit 
        
                   description: Maximum number of items per page 
        
                   type: integer 
        
                   x-example: 1 
        
               responses: 
        
                 200: 
        
                   description: All txs matching the provided tags 
        
                   schema: 
        
                     type: array 
        
                     items: 
        
                       $ref: "#/definitions/TxQuery" 
        
                 400: 
        
                   description: Invalid search tags 
        
                 500: 
        
                   description: Internal Server Error 
        
             post: 
        
               tags: 
        
                 - ICS0 
        
               summary: Broadcast a signed tx 
        
               description: Broadcast a signed tx to a full node 
        
               consumes: 
        
                 - application/json 
        
               produces: 
        
                 - application/json 
        
               parameters: 
        
                 - in: body 
        
                   name: txBroadcast 
        
                   description: The tx must be a signed StdTx. The supported broadcast modes include `"block"`(return after tx commit), `"sync"`(return afer CheckTx) and `"async"`(return right away). 
        
                   required: true 
        
                   schema: 
        
                     type: object 
        
                     properties: 
        
                       tx: 
        
                         $ref: "#/definitions/StdTx" 
        
                       mode: 
        
                         type: string 
        
                         example: block 
        
               responses: 
        
                 200: 
        
                   description: Tx broadcasting result 
        
                   schema: 
        
                     $ref: "#/definitions/BroadcastTxCommitResult" 
        
                 500: 
        
                   description: Internal Server Error

I don't know where that site pulls its docs from though. @fedekunze?

fedekunze · 2019-04-13T05:28:37Z

it’s pulled from master

alexanderbez · 2019-04-13T16:45:00Z

Then it looks like it'll be resolved once 0.34 is released!

alexanderbez added 4 commits March 20, 2019 19:18

BaseApp cleanup

0750e5d

Replace async CLI flag with sync flag

be54368

Remove tx broadcast commit support

2ab7e77

Update TxResponse to include a raw log

14f5bd3

alexanderbez added WIP T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). labels Mar 20, 2019

alexanderbez added 11 commits March 20, 2019 19:34

Add a section about tx broadcasting to the gaiacli docs

0e3eacb

Add back NewResponseFormatBroadcastTxCommit

d380760

Use new broadcast-mode flag instead of sync flag

1fede61

Update BroadcastTx to include commit mode

9cef5d3

Update CLI context

d66ae9b

Update gaiacli doc section

87288aa

Update BroadcastTxAndAwaitCommit godoc

717b175

Update REST broadcasting to use mode instead of return

d6e2dc8

Update BroadcastTxAndAwaitCommit to return an error

91ab6b5

Rename BroadcastTxAndAwaitCommit

9b2d517

Add a pending log entry

c70980f

Fix CLI tests

751abe1

alexanderbez marked this pull request as ready for review March 21, 2019 14:42

alexanderbez requested review from cwgoes, ebuchman and rigelrozanski as code owners March 21, 2019 14:42

alexanderbez added ready-for-review and removed WIP labels Mar 21, 2019

fedekunze approved these changes Mar 21, 2019

View reviewed changes

alexanderbez requested review from jackzampolin, alessio and mossid March 25, 2019 17:40

jackzampolin reviewed Mar 25, 2019

View reviewed changes

docs/gaia/gaiacli.md Show resolved Hide resolved

jackzampolin reviewed Mar 25, 2019

View reviewed changes

cmd/gaia/cli_test/test_helpers.go Outdated Show resolved Hide resolved

jackzampolin reviewed Mar 25, 2019

View reviewed changes

client/context/broadcast.go

}

return

}

switch ctx.BroadcastMode {

Copy link

Member

jackzampolin Mar 25, 2019

Choose a reason for hiding this comment

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

++

jackzampolin approved these changes Mar 25, 2019

View reviewed changes

alexanderbez added 3 commits March 25, 2019 17:09

Add shorthand flag for broadcast mode

2e18c7a

Add support to set broadcasting mode in CLI config

daf4323

Merge branch 'develop' into bez/3875-remove-tx-broadcast-commit

e48dd60

alexanderbez merged commit 2b43e25 into develop Mar 26, 2019

alexanderbez deleted the bez/3875-remove-tx-broadcast-commit branch March 26, 2019 00:54

alexanderbez mentioned this pull request Mar 28, 2019

Post /txs sync returns height 0 #4000

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove Tx Broadcast and Commit Support #3954

Remove Tx Broadcast and Commit Support #3954

alexanderbez commented Mar 20, 2019 •

edited

Loading

codecov bot commented Mar 21, 2019 •

edited

Loading

fedekunze left a comment

jackzampolin Mar 25, 2019

jackzampolin Mar 25, 2019

jackzampolin Mar 25, 2019

jackzampolin commented Mar 25, 2019

jackzampolin commented Mar 25, 2019 •

edited

Loading

alexanderbez commented Mar 25, 2019

Onefox commented Apr 12, 2019

alexanderbez commented Apr 12, 2019

fedekunze commented Apr 13, 2019

alexanderbez commented Apr 13, 2019

		func NewResponseFormatBroadcastTxCommit(res *ctypes.ResultBroadcastTxCommit) TxResponse {
		if !res.CheckTx.IsOK() {

Remove Tx Broadcast and Commit Support #3954

Remove Tx Broadcast and Commit Support #3954

Conversation

alexanderbez commented Mar 20, 2019 • edited Loading

codecov bot commented Mar 21, 2019 • edited Loading

Codecov Report

fedekunze left a comment

Choose a reason for hiding this comment

jackzampolin Mar 25, 2019

Choose a reason for hiding this comment

jackzampolin Mar 25, 2019

Choose a reason for hiding this comment

jackzampolin Mar 25, 2019

Choose a reason for hiding this comment

jackzampolin commented Mar 25, 2019

jackzampolin commented Mar 25, 2019 • edited Loading

alexanderbez commented Mar 25, 2019

Onefox commented Apr 12, 2019

alexanderbez commented Apr 12, 2019

fedekunze commented Apr 13, 2019

alexanderbez commented Apr 13, 2019

alexanderbez commented Mar 20, 2019 •

edited

Loading

codecov bot commented Mar 21, 2019 •

edited

Loading

jackzampolin commented Mar 25, 2019 •

edited

Loading