Insufficient broadcast txs error info returned by POST /txs

[whunmr]

Summary of Bug

Not consistent return value structure between success and failure result of POST /txs API.

• when execution of tx failed with some error (e.g.: delegate without enough funds)
  the POST /txs API will return like:

  {
      "error": "[{\"msg_index\":0,\"success\":false,\"log\":\"    {\\\"codespace\\\":\\\"sdk\\\",\\\"code\\\":10,\\\"message\\\":\\\"insufficient account funds;     8999900000000cet \\u003c 9000000000000cet\\\"}\"}]"
  }
  

the problem is when we got this error result, can not know the txhash for further use

• But the success execution through POST /txs will return like:

  {
      "events": [
          {
              "attributes": [
                  {
                      "key": "validator",
                      "value": "coinexvaloper13hkgv5f9xm3v2cxxkkzvse8t6pcjqpk974pv83"
                  },
                  {
                      "key": "amount",
                      "value": "3000000000000"
                  }
              ],
              "type": "delegate"
          },
          {
              "attributes": [
                  {
                      "key": "action",
                      "value": "delegate"
                  },
                  {
                      "key": "module",
                      "value": "staking"
                  },
                  {
                      "key": "sender",
                      "value": "coinex13hkgv5f9xm3v2cxxkkzvse8t6pcjqpk996zyf9"
                  }
              ],
              "type": "message"
          }
      ],
      "gas_used": "99076",
      "gas_wanted": "200000",
      "height": "6",
      "logs": [
          {
              "log": "",
              "msg_index": 0,
              "success": true
          }
      ],
      "raw_log": "[{\"msg_index\":0,\"success\":true,\"log\":\"\"}]",
      "txhash": "12E644C8BC21DA5B5CA87AC912AF78E8E7F355C94A5DD2B355A12999802FA47F"
  }
  

Version

$ git rev-parse HEAD
07a0a1bda6647e9f859d8206b80413b372f8feeb
$ git describe
v0.36.0-rc1

Steps to Reproduce

• POST /txs a successful delegation tx
• POST /txs a failed delegation tx (failure caused by not enough account funds)
• compare two result structure
  • ACTUAL: structure not consistent (missing various fields in failed delegate scenario)
  • EXPECTED: with same structure, also contains fields(txhash, raw_logs, etc) in result of failed scenario

codes

broadcast.go:47

• Maybe we should also return res in the rest.WriteErrorResponse.
• Meanwhile, delegation without sufficient funds, are not feels like an InternalServerError, and maybe we should not return http code 500 here.

		res, err := cliCtx.BroadcastTx(txBytes)
		if err != nil {
			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
			return
		}
                rest.PostProcessResponseBare(w, cliCtx, res)

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[alexanderbez]

Can you summarize the issue/problem you're experiencing @whunmr? I'm having trouble understanding. Also note, block broadcasting mode is highly discouraged.

[whunmr]

Thanks for the reply.

we need the POST /txs api to return block hash and other info, when runtx failes.
so in our functional test which using block mode can get more context about the tx execution result.

[whunmr]

have rephrased the problem summary in first comment. hope it's more clearer.

Also thanks for the notice block broadcasting mode is highly discouraged,
but in our functional tests, block mode could make tests more stable by not using async and sleep waits.

[alexanderbez]

Also thanks for the notice block broadcasting mode is highly discouraged,
but in our functional tests, block mode could make tests more stable by not using async and sleep waits.

In functional tests sure, but this does not reflect a common live network situation.

But I see what you mean. The solution is to not return an error:

diff --git a/client/context/broadcast.go b/client/context/broadcast.go
index 8aa770ce1..5dbf530f3 100644
--- a/client/context/broadcast.go
+++ b/client/context/broadcast.go
@@ -47,11 +47,11 @@ func (ctx CLIContext) BroadcastTxCommit(txBytes []byte) (sdk.TxResponse, error)
        }

        if !res.CheckTx.IsOK() {
-               return sdk.NewResponseFormatBroadcastTxCommit(res), fmt.Errorf(res.CheckTx.Log)
+               return sdk.NewResponseFormatBroadcastTxCommit(res), nil
        }

        if !res.DeliverTx.IsOK() {
-               return sdk.NewResponseFormatBroadcastTxCommit(res), fmt.Errorf(res.DeliverTx.Log)
+               return sdk.NewResponseFormatBroadcastTxCommit(res), nil
        }

        return sdk.NewResponseFormatBroadcastTxCommit(res), nil