[rpc] Verbose flags for chaining and scripting

kallewoof commented at 4:01 am on July 19, 2017: member

[Reviewer hint: use ?w=1 to avoid seeing a bunch of indentation changes.]

Right now, some RPC commands return JSON data along with transaction hex strings, which means scripting and/or chaining needs to do JSON processing before being able to use the resulting transaction data. For use cases where you don’t need the extra info, this PR adds a verbose=false mode (default true) which returns only the transaction hex string.

This means you can now do things like this:

0./bitcoin-cli -regtest -named signrawtransactionwithwallet hexstring=$(
1    ./bitcoin-cli -regtest -named fundrawtransaction hexstring=$(
2        ./bitcoin-cli -regtest createrawtransaction "[]" "{\"$(
3            ./bitcoin-cli -regtest getnewaddress)\": 10.0}") verbose=false) verbose=false

This is also useful in scripts, where you perform some operation on a transaction and store the tx itself in a variable. Before you would need e.g. jq or some other tool to do this.

Edit: signrawtransaction has been deprecated; I chose to add verbose to the new commands withkey/withwallet but not to signrawtransaction.

jonasschnelli commented at 8:41 am on July 19, 2017: contributor

Code wise, this is more of a refactor (much more lines are touch for refactoring then the intended feature).

Wouldn’t’ the non verbose mode (== directly return hex) include some risks because one my miss important errors (sent back through the “error” json array/object)?

jonasschnelli added the label RPC/REST/ZMQ on Jul 19, 2017

kallewoof commented at 9:15 am on July 19, 2017: member

@jonasschnelli For fundrawtransaction, you will no longer see the resulting fee, which is a pity, but if you don’t need to know it, it’s fine. For signrawtransaction, you will not see if it was only partially signed, which may trip you in certain cases, but I don’t think it would end up in lost money or be dangerous (you just have a transaction that the network will reject).

jonasschnelli commented at 9:44 am on July 19, 2017: contributor

@kallewoof your right about fundrawtransaction (I mixed it up with bumpfee). But what about signrawtransaction Errors like TxInErrorToJSON(txin, vErrors, "Input not found or already spent"); or the possible script sig errors?

I think its probably acceptable to suppress those errors, but maybe we should mention that in the help description.

kallewoof commented at 12:12 pm on July 19, 2017: member

I’m actually confused why the sign code doesn’t throw an error in the case you mentioned. Why would it continue when the tx appears to be broken? Hm..

promag commented at 3:09 pm on July 19, 2017: member

Does this sounds more like a bash mode for cli than verbose option?

Edit: Do we want this option in the RPC interface?

promag commented at 8:51 pm on July 19, 2017: member

Another suggestion to ease chaining and eventually reduce the transmitted data is to specify a json path to filter the output. For instance:

0bitcoin-cli -rpcfilter '*.amount' listunspent

Compared to jq, it has the advantage of server side filtering. But I guess it is too much for core.

kallewoof commented at 11:27 pm on July 19, 2017: member

Does this sounds more like a bash mode for cli than verbose option?

I chose verbose because if you look at e.g. createrawtransaction it responds with the hex only (the same style), whereas the ones here will respond with a JSON object with extra information. That to me was actually a bit unexpected, and it still trips me up on occasion when I’m trying to do something (I somehow think the results will be the tx hex when in fact they’re a json object).

promag commented at 0:27 am on July 20, 2017: member

IMO verbose is unclear and usually means more detailed logs which is unrelated.

Also, I think we should discourage chaining these calls because the middle results/errors are important and should be acknowledge/logged/handled/whatever.

As you said, it’s still doable using other tools.

Not sure about others, but for me it’s a NACK.

kallewoof commented at 1:25 am on July 20, 2017: member

Verbose is not restricted to logs, in my world. The word itself simply means “using or expressed in more words than are needed” which I think fits very nicely with the use case here (you don’t need the complete flag for signing, for instance; it’s just a helpful thing to have).

Yes, it’s doable using other tools, but this dependency is rather superfluous considering the relatively small addition (style fixes outweighs the code itself) means less work for the node (a string rather than a JSON object).

It’s ultimately ~2-3 lines per RPC call to remove a potential dependency on external tools. I think that’s worth something personally.

jnewbery commented at 8:30 pm on September 1, 2017: member

Concept ACK. I think chaining commands together without intermediate error checking like this is definitely dangerous and poor practice, but there are probably legitimate use-cases for testing, etc.

Thinking out loud: verbosity is a very common option for RPCs. Perhaps we should consider making it a query string in the URL rather than adding parameters to every RPC that requires it. Maybe that’s not practical, but worth considering.

kallewoof force-pushed on Sep 4, 2017

kallewoof force-pushed on Oct 11, 2017

kallewoof force-pushed on Dec 28, 2017

kallewoof force-pushed on Feb 20, 2018

[style] Fix fundrawtransaction styling. cb2ca03a86

[rpc] [wallet] Add verbose flag (default=true) to fundrawtransaction e5c9ecb2fe

[rpc] [wallet] Add verbose flag (default=true) to SignTransaction helper fc7397ba6b

[test] Add verbose=false tests for fundrawtransaction. fe131b8f41

[rpc] [wallet] Add verbose mode to signrawtransactionwithwallet 2235913e0e

[rpc] [wallet] Add verbose mode to signrawtransactionwithkey bf7080cbaa

[test] Add case with verbose=false to signrawtransaction tests 317f1d2a60

kallewoof force-pushed on Feb 22, 2018

kallewoof commented at 8:01 am on February 22, 2018: member

Rebased, and dropped signrawtransaction patch and added signrawtransactionwith* support instead.

kallewoof commented at 10:29 am on March 15, 2018: member

One NACK, one concept ACK. I think it’s not worth keeping this around.

kallewoof closed this on Mar 15, 2018

kallewoof deleted the branch on Oct 17, 2019

DrahtBot locked this on Dec 16, 2021

[rpc] Verbose flags for chaining and scripting #10877