Adding blockhash parameter to getrawtransaction - Issue #2077 #2421

Closed
wants to merge 3 commits into
from

Projects

None yet

7 participants

silvioq commented Mar 29, 2013

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

(#2077)
Pantallazo

Member

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.

silvioq commented Mar 29, 2013

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)"

Owner
sipa commented Mar 29, 2013

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.

silvioq commented Mar 29, 2013

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!)

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?)

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.

silvioq added some commits Apr 1, 2013
@silvioq silvioq getrawtransaction: add test for invalid block hash 602a1c4
@silvioq silvioq Add a reasonable error message on invalid blockhash parameter at getr…
…awtransaction API function.

GetTransaction now assumes lookupHashBlock is zero or valid hash block.
8b011a4

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.

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.

Owner
sipa commented Jun 15, 2013

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, ...
Member
jgarzik commented Jun 24, 2013

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.

@jgarzik jgarzik closed this Jun 24, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment