Fix missing gettransaction entries in rpcclient #4863

pull bencxr wants to merge 2 commits into bitcoin:master from bencxr:fix-gettransaction-includewatchonly changing 2 files +3 −1

bencxr commented at 12:46 PM on September 7, 2014: contributor

When getting a transaction to a watch-only wallet, the gettransaction RPC call should show transaction details (amount, address, received). This won't happen unless the true "includewatchonly" param is true.

But, when doing bitcoin-cli gettransaction "7b38c67e8f32678bd19ad3537b384f7a619ba1e5bdacac983c9a4cc2c16b657e" true

we currently get an error: error: {"code":-1,"message":"value is type str, expected bool"}

This is the result of the "true" value method not being serialized to json properly in the rpc client due to the gettransaction entry being missing in rpcclient vRPCConvertParams. I believe this was a bug here on at src/rpcclient.cpp:68 https://github.com/bitcoin/bitcoin/commit/0fa2f8899adf8f9f0ead29ba5d708ead6c5d4eaf

I added the entries correctly (for both 1 and 2 arguments) and also updated the rpc help message for gettransaction to let users know about the includeWatchonly param, following the conventions for the other rpc calls.
sipa commented at 12:49 PM on September 7, 2014: member

Untested ACK
in src/rpcclient.cpp:None in da30d208ed outdated
```
  66 | @@ -67,6 +67,8 @@ static const CRPCConvertParam vRPCConvertParams[] =
  67 |      { "listunspent", 1 },
  68 |      { "listunspent", 2 },
  69 |      { "getblock", 1 },
  70 | +    { "gettransaction", 1 },
  71 | +    { "gettransaction", 2 },
```
laanwj commented at 7:17 AM on September 8, 2014:

This should only add parameter 1. There is no parameter 2 (remember, zero-based) for gettransaction.

bencxr commented at 7:58 AM on September 8, 2014:

you're right, I mistook the command to be 0. fixed.
jgarzik commented at 1:39 AM on September 15, 2014: contributor

ut ACK
laanwj commented at 9:12 AM on September 16, 2014: member

ACK after squashing the "fix number of params for gettransaction" commit
update rpc help message for gettransaction to add includeWatchonly param 57e1716de6
fix missing gettransaction entries in rpcclient 1a613963e1
bencxr force-pushed on Sep 16, 2014
BitcoinPullTester commented at 11:27 AM on September 16, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4863_1a613963e1567fd6102e783422cc6cad672dd1a5/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
laanwj merged this on Sep 17, 2014
laanwj closed this on Sep 17, 2014
laanwj referenced this in commit 5b9d1e10a8 on Sep 17, 2014
MarcoFalke locked this on Sep 8, 2021

Contributors