Adding blockhash parameter to getrawtransaction - Issue #2077 #2421

pull silvioq wants to merge 3 commits into bitcoin:master from silvioq:master changing 4 files +28 −5
  1. silvioq commented at 1:48 am on March 29, 2013: none

    This optional parameter is used to specify the block which is used to look up the raw transaction.

    (https://github.com/bitcoin/bitcoin/issues/2077) Pantallazo

  2. Adding blockhash parameter to getrawtransaction RPC function. (Issue #2077) 303101f2da
  3. gmaxwell commented at 1:50 am on March 29, 2013: contributor

    You know if you set txindex=1 this works without the blockhash, right?

    I guess this isn’t so bad— it’s incompatible with pruning but so is txindex=1.

  4. silvioq commented at 1:58 am on March 29, 2013: none

    Ok. But txindex requires reindex and it’s bad for me (now).

    “If you need that functionality, you must run once with -txindex -reindex to rebuild block-chain indices (see below for more details)”

  5. sipa commented at 2:06 am on March 29, 2013: member

    I’m unsure about this.

    On the one hand, the blocks are available, and getblock() exists- it’s stupid that it cannot give you the transactions in it.

    On the other hand, I really don’t want to encouraging services that depend on the presense of (indexable) block chain data in the first place.

  6. silvioq commented at 2:27 am on March 29, 2013: none

    Well … Maybe you must check this addon with the user community and decide if this is useful/compatible/promotable or not.

    Meanwhile, as I say, the fork is fine for me now and I can use it.

    Regards (and sorry by my basic English!)

  7. gavinandresen commented at 6:56 pm on March 29, 2013: contributor
    I don’t feel strongly one way or the other; code looks fine, I haven’t tested (only question: do you get a reasonable error message if you specify an invalid block hash?)
  8. BitcoinPullTester commented at 9:04 am on March 30, 2013: none
    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/303101f2da0550ff3b51693b65171b2b02d7f2d5 for binaries and test log. This is an automated test script which runs test cases on each commit every time is updated. It, however, dies sometimes and fails to test properly, if you are waiting on a test, please check timestamps and if the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ and contact BlueMatt on freenode if something looks broken.
  9. getrawtransaction: add test for invalid block hash 602a1c484b
  10. Add a reasonable error message on invalid blockhash parameter at getrawtransaction API function.
    GetTransaction now assumes lookupHashBlock is zero or valid hash block.
    8b011a4871
  11. BitcoinPullTester commented at 3:10 am on April 3, 2013: none

    Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/8b011a4871fab786913489abf3b78121a8452396 for binaries and test log.

    This could happen for one of several reasons:

    1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (you can find the patches applied at test-time at http://jenkins.bluematt.me/pull-tester/files/patches/)
    2. It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
    3. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
    4. The test suite fails on either Linux i386 or Win32
    5. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

    If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

    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.

  12. runeksvendsen commented at 7:23 pm on May 21, 2013: contributor

    I’m interested in this feature as well. I like being able to parse transaction data, regardless of whether it has been spent or no.

    silvioq, do you know why it failed to build? I can’t access the build log.

  13. sipa commented at 2:59 pm on June 15, 2013: member

    So, some more thoughts. The generic problem this pullreq tries to solve is very worthwhile: right now, we have the full blocks stored and indexed, but retrieving anything more than the headers and the txids in them is not possible without txindex=1. This shouldn’t be necessary - just a sequential scan through the blocks is perfectly meaningful.

    I’m still unconvinced about the implementation though. Doing such a sequential scan using this code here results in every block being read from disk and parsed N times, with N the number of transactions in it - only to fetch a single one from it. I have some suggestions for alternatives, but neither is perfect imho either:

    • Add a mode to getblock to inline the full transactions (efficient, but huge RPC replies, and unable to filter transactions you don’t care about)
    • Add a mode to getblock which adds opaque transaction pointers to the output (e.g. block file number + position + size encoded in some form, or block hash + offset, or even just txids itself). The client isn’t supposed to interpret these transaction pointers in any way, but pass them on to getrawtransaction. Ugly, but efficient and flexible.
    • Cache the last block read through getblock, so getrawtransaction (which takes the block hash, as implemented here) can just scan through it. Nice solution, but what if access isn’t entirely sequential?
    • Cache the positions of transactions of the last few blocks read through getblock, so they can be read directly. Not a trivial change anymore though, …
  14. jgarzik commented at 2:54 pm on June 24, 2013: contributor

    Agree w/ @sipa. This functionality is OK to have, but in a different form.

    The first suggestion is the most powerful, and would seem to solve many common requests: ‘getblock’ simply returns a full block, including all transactions.

    Closing, as consensus seems to point elsewhere.

  15. jgarzik closed this on Jun 24, 2013

  16. DrahtBot locked this on Sep 8, 2021

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: 2024-10-04 22:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me