[RPC] Transaction details in getblock #8704

pull achow101 wants to merge 2 commits into bitcoin:master from achow101:getblock-extraverbose changing 3 files +39 −16
  1. achow101 commented at 2:08 AM on September 12, 2016: member

    Adds an optional parameter extraVerbose to getblock to have transaction details displayed in a getblock call.

  2. fanquake added the label RPC/REST/ZMQ on Sep 12, 2016
  3. dcousens commented at 3:10 AM on September 12, 2016: contributor

    concept NACK, is two RPC requests really so bad?

    edit: weak concept NACK

  4. achow101 commented at 3:25 AM on September 12, 2016: member

    @dcousens yes, two RPC calls is really bad, especially when you want the details of all the transactions in a block. Then you end up running getrawtransaction thousands of times. Also, AFAICT, this doesn't require the txindex in order to get the transactions, unlike getrawtransaction.

  5. dcousens commented at 3:50 AM on September 12, 2016: contributor

    @dcousens yes, two RPC calls is really bad

    Why? If you're following best practices (RPC is over localhost only) in terms of access, the latency should be irrelevant.

    In a batched RPC call, the overhead is literally just 1RTT more. Not N (aka, thousands), 1.

    AFAICT, this doesn't require the txindex in order to get the transactions, unlike getrawtransaction.

    Perhaps getrawtransaction should have an optional block parameter?

    edit: In fact, adding that option would be significantly more useful than this RTT optimisation IMHO.

  6. achow101 commented at 4:05 AM on September 12, 2016: member

    In a batched RPC call, the overhead is literally just 1RTT more. Not N (aka, thousands), 1.

    How?

    edit: In fact, adding that option would be significantly more useful than this RTT optimisation IMHO.

    Well the functionality to this was already in the code. The BlockToJson method took a parameter for txDetails which was by default false. This just lets you set that to true.

  7. dcousens commented at 4:11 AM on September 12, 2016: contributor

    http://www.jsonrpc.org/specification#batch

    What language are you using? Your RPC library should be able to handle this quite easily.

  8. achow101 commented at 4:27 AM on September 12, 2016: member

    Huh. Didn't know that. I've been using bash with either curl or bitcoin-cli and python.

  9. laanwj commented at 3:33 PM on September 13, 2016: member

    Saving roundtrip time is not a valid reason to add a RPC API (see discussion in #8457).

    Also, AFAICT, this doesn't require the txindex in order to get the transactions, unlike getrawtransaction.

    This is the only important fact here: you cannot get this information any other way. getrawtransaction won't get you transactions in blocks, at least without tx index.

    The only way to get this information right now is to getblock raw then parse the block locally.

    So concept ACK because of that.

  10. dcousens commented at 9:54 PM on September 13, 2016: contributor

    @laanwj would it maybe be better to add functionality via getrawtransaction w/ a block id? (perhaps in addition to this)

  11. laanwj commented at 9:24 AM on September 14, 2016: member

    would it maybe be better to add functionality via getrawtransaction w/ a block id? (perhaps in addition to this)

    Well yes a "gather transaction [X,...] from prespecified block Y" RPC call could be useful in some rare cases. But on the other hand it'll still have to read the entire block from disk and deserialize it. It's terribly inefficient already. So if it decoded the entire block why not report details for the entire block as done here?

  12. dcousens commented at 12:06 PM on September 14, 2016: contributor

    @laanwj indeed! Forgot about the resulting deserialization.

  13. luke-jr commented at 4:14 AM on September 22, 2016: member

    Concept ACK for all the reasons @laanwj already discussed.

  14. in src/rpc/blockchain.cpp:None in 0115d18b82 outdated 
     688 | @@ -689,11 +689,12 @@ UniValue getblockheader(const UniValue& params, bool fHelp)
 689 |  
 690 |  UniValue getblock(const UniValue& params, bool fHelp)
 691 |  {
 692 | -    if (fHelp || params.size() < 1 || params.size() > 2)
 693 | +    if (fHelp || params.size() < 1 || params.size() > 3)
 694 |          throw runtime_error(
 695 | -            "getblock \"hash\" ( verbose )\n"
 696 | +            "getblock \"hash\" ( verbose ) ( extraVerbose )\n"
    luke-jr commented at 8:42 AM on October 18, 2016:

    Double boolean here seems ugly. Maybe allow verbose to be boolean or a number 0-2 (with 2 being extraVerbose)?

    laanwj commented at 9:10 AM on October 18, 2016:

    It is ugly, but overloading on type in JSON which is essentially dynamically typed is also ugly.

    Named parameters as implemented in #8811 would make this more bearable.

    laanwj commented at 9:15 AM on October 18, 2016:

    I do agree that for a new interface, a scale for verbosity would have made more sense instead of a boolean. Maybe that's a better choice I'm just not sure. Changing the meaning of existing arguments is always annoying and means extra testing for backwards compatibility.

  15. laanwj commented at 11:33 AM on October 26, 2016: member

    I'm starting to realize that @luke-jr's idea to accept 0-2 isn't such a bad idea after all. Just rename the argument 'verbosityLevel'. It is easier to use and understand from a user perspective than two booleans (we regularly get confused there in the tests ourselves). Also it'd remove having to deal with the redundant and nonsensical combo verbose=false extraVerbose=true.

    It should still accept false and true too. For consistency we should do the same in getrawtransaction. This method currently accepts a verbose argument that can be 0 or 1 but not false or true. The mapping false:0,true:1 should be added there (not in this pull though).

    Also: needs rebase.

  16. laanwj cross-referenced this on Oct 26, 2016 from issue getrawtransaction should take a bool for verbose by jnewbery
  17. achow101 commented at 3:03 PM on October 26, 2016: member

    How should it document in the help message that the old behavior is still accepted?

  18. achow101 force-pushed on Oct 26, 2016
  19. achow101 commented at 3:29 PM on October 26, 2016: member

    Rebased and made verbose an int from 0-2

  20. luke-jr commented at 5:32 AM on November 24, 2016: member

    @achow101 The old behaviour should be considered deprecated, and therefore not documented.

  21. in src/rpc/blockchain.cpp:None in ad8fc81cde outdated 
     701 | +            "The previous behavior of verbose being a boolean is still maintained. False will have the behavior of 0 and True will have the behavior of 1.\n"
 702 |              "\nArguments:\n"
 703 |              "1. \"hash\"          (string, required) The block hash\n"
 704 | -            "2. verbose           (boolean, optional, default=true) true for a json object, false for the hex encoded data\n"
 705 | -            "\nResult (for verbose = true):\n"
 706 | +            "2. verbose           (boolean, optional, default=1) 0 for hex encoded data, 1 for a json object, and 2 for json object with transaction data\n"
    luke-jr commented at 5:33 AM on November 24, 2016:

    Should be renamed to verbosity.

  22. achow101 force-pushed on Nov 24, 2016
  23. achow101 commented at 5:47 AM on November 24, 2016: member

    @luke-jr done

  24. luke-jr referenced this in commit 645b21cff4 on Dec 21, 2016
  25. achow101 commented at 12:50 AM on January 10, 2017: member

    merge please?

  26. luke-jr approved
  27. luke-jr commented at 4:00 PM on February 3, 2017: member

    utACK

  28. luke-jr commented at 4:00 PM on February 3, 2017: member

    (Needs rebase)

  29. achow101 force-pushed on Feb 3, 2017
  30. achow101 commented at 4:26 PM on February 3, 2017: member

    rebased

  31. luke-jr approved
  32. achow101 cross-referenced this on Feb 14, 2017 from issue Add txdetails parameter to getblock. by pstratem
  33. pstratem commented at 12:06 AM on February 15, 2017: contributor

    Is this compatible with callers of getblock which send true instead of 0/1 ?

  34. achow101 commented at 12:39 AM on February 15, 2017: member

    @pstratem Yes. The old style of calling is maintained for compatibility.

  35. in src/rpc/blockchain.cpp:None in 9ada7f04c8 outdated 
     735 | +            "  \"weight\" : n           (numeric) The block weight (BIP 141)\n"
 736 | +            "  \"height\" : n,          (numeric) The block height or index\n"
 737 | +            "  \"version\" : n,         (numeric) The block version\n"
 738 | +            "  \"versionHex\" : \"00000000\", (string) The block version formatted in hexadecimal\n"
 739 | +            "  \"merkleroot\" : \"xxxx\", (string) The merkle root\n"
 740 | +            "  \"tx\" : [               (array of Objects) The transactions\n"
    jnewbery commented at 11:04 PM on February 15, 2017:

    I'd suggest not including this in the help output, and just having something like:

               "  \"tx\" : [               (array of Objects) The transactions. Format is the same as for getrawtransaction call\n"
           "         ,...\n"
           "  ],\n"

    If the output format of TxToJSON() ever changes, I'm sure that changing this help text will be forgotten!

  36. sipa commented at 3:29 PM on April 9, 2017: member

    Needs rebase.

  37. RPC: Allow multiple names for parameters c99ab3ca4b
  38. achow101 force-pushed on Apr 10, 2017
  39. achow101 commented at 1:30 PM on April 10, 2017: member

    rebased. also added @jnewbery's suggestion with the help text.

  40. luke-jr referenced this in commit ae106ec2b8 on Apr 21, 2017
  41. sdaftuar cross-referenced this on May 11, 2017 from issue RPC: getblock returns coinbase scriptsig by pinheadmz
  42. kallewoof commented at 1:09 AM on May 12, 2017: member

    utACK b779f3093d7b68ee09305a5294c48825dce8ae1f

  43. Use a verbosity instead of two verbose parameters
    Verbose is changed to an int. This can have values from 0-2 for each level of verbosity.
Verbosity level 2 has transaction details displayed in the results.
    e3c9f2ddb1
  44. in src/rpc/blockchain.cpp:722 in b779f3093d outdated 
     718 | @@ -716,8 +719,29 @@ UniValue getblock(const JSONRPCRequest& request)
 719 |              "  \"previousblockhash\" : \"hash\",  (string) The hash of the previous block\n"
 720 |              "  \"nextblockhash\" : \"hash\"       (string) The hash of the next block\n"
 721 |              "}\n"
 722 | -            "\nResult (for verbose=false):\n"
 723 | -            "\"data\"             (string) A string that is serialized, hex-encoded data for block 'hash'.\n"
 724 | +            "\nResult (for verbosity = 2):\n"
    paveljanik commented at 6:39 AM on May 12, 2017:

    What about mentioning only the differences to verbosity=1 instead?

    achow101 commented at 3:47 PM on May 12, 2017:

    something like this?

    {
..., Same as for verbosity=1
"tx" : [           (array of Objects) The transactions in the format of the getrawtransaction RPC.
         , ...
],
... Same as for verbosity=1
}
    paveljanik commented at 3:50 PM on May 12, 2017:

    Yup, or maybe even better:

    Additional output (for verbosity=1):...
  45. achow101 force-pushed on May 12, 2017
  46. achow101 commented at 3:59 PM on May 12, 2017: member

    @paveljanik I made the change.

  47. laanwj commented at 3:20 PM on May 15, 2017: member

    Tested ACK e3c9f2d

  48. laanwj merged this on May 15, 2017
  49. laanwj closed this on May 15, 2017
  50. laanwj referenced this in commit 96c850c209 on May 15, 2017
  51. sipa cross-referenced this on May 15, 2017 from issue [rpc] Allow fetching tx directly from specified block in getrawtransaction by kallewoof
  52. achow101 deleted the branch on May 17, 2017
  53. luke-jr referenced this in commit ad10ed859a on Jun 15, 2017
  54. jnewbery cross-referenced this on Jul 5, 2017 from issue Be consistent in calling transactions "replaceable" for Opt-In RBF by TheBlueMatt
  55. jnewbery cross-referenced this on Jul 5, 2017 from issue [rpc] fix verbose argument for getblock in bitcoin-cli by jnewbery
  56. jnewbery cross-referenced this on Jul 7, 2017 from issue RPC: Introduce getblockstats to plot things by jtimon
  57. wirwolf referenced this in commit 229a2d8e43 on Sep 28, 2017
  58. wirwolf cross-referenced this on Sep 28, 2017 from issue Merge #9025: [RPC] Transaction details in getblock by wirwolf
  59. bitcartel cross-referenced this on Apr 23, 2018 from issue Add improvements to getblock RPC output by bitcartel
  60. zkbot referenced this in commit e57c0b0d1c on May 1, 2018
  61. zkbot referenced this in commit 53fa6f1315 on May 1, 2018
  62. achow101 restored the branch on Aug 17, 2018
  63. achow101 deleted the branch on Aug 17, 2018
  64. salisbury-espinosa cross-referenced this on Nov 28, 2018 from issue [RPC] Transaction details in getblock by salisbury-espinosa
  65. thephez referenced this in commit b652fe17d3 on Dec 17, 2018
  66. thephez referenced this in commit 28514b62d7 on Dec 26, 2018
  67. hebasto cross-referenced this on Jan 16, 2019 from issue qt: Improve "help-console" message by hebasto
  68. laanwj referenced this in commit 3b59fa2ce8 on Jan 19, 2019
  69. IntegralTeam cross-referenced this on May 31, 2019 from issue [RPC] Transaction details in getblock by IntegralTeam
  70. PastaPastaPasta referenced this in commit 4e6eba8208 on Jun 26, 2021
  71. PastaPastaPasta referenced this in commit 7269286cd4 on Jun 26, 2021
  72. PastaPastaPasta referenced this in commit 4dcae64501 on Jun 27, 2021
  73. PastaPastaPasta referenced this in commit 29645d5832 on Jun 28, 2021
  74. bitcoin locked this on Sep 8, 2021
Contributors
achow101dcousenslaanwjluke-jrpstratemjnewberysipakallewoofpaveljanik
Labels
RPC/REST/ZMQ
Linked (view graph)
#8457 Add block height support in rpc call getblock#8811 rpc: Add support for JSON-RPC named arguments#9025 getrawtransaction should take a bool for verbose#9762 Add txdetails parameter to getblock.#10275 [rpc] Allow fetching tx directly from specified block in getrawtransaction#10385 RPC: getblock returns coinbase scriptsig#10698 Be consistent in calling transactions "replaceable" for Opt-In RBF#10747 [rpc] fix verbose argument for getblock in bitcoin-cli#10757 RPC: Introduce getblockstats to plot things#15178 qt: Improve "help-console" message
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:55 UTC