Topic: [PATCH] getblockbycount, getblockbyhash RPCs - page 2. (Read 30500 times)

Patch updated to latest SVN + theymos's fix.  See top of thread for URL.

An orphan block was created at 91405, and I was able to verify that my change to getblock fixes the problem. I suggest that it be included in the patch.

I think that this can be fixed by changing this:


to this:


jgarzik: I don't have any orphan blocks right now, so can you test this on your block 90539 and compare it to the real one?

ArtForz figured out that this is caused because getblock is showing an orphan block at that position instead of the real block. Deleting the block database files fixes it because you won't re-download orphan blocks.

hmmmm.  That's quite strange, considering the code simply loops over all 'vin' and 'vout', duplicating the code pattern used elsewhere to dump a block to the debug log.

I noticed a bug in getblock (and bitcointools). Block 67300 is missing several transactions when compared to the output of Bitcoin's printblock, and the hash is also wrong.

I ran into the bug while parsing the entire chain: transaction e7a995 is spent in 67301, but according to getblock data that transaction was never made.

Patch updated to latest SVN (v0.3.14).

patch is not meant to be used blindly. You're supposed to look at the reject file manually and figure it out.

The problem is caused when trying to patch the section of rpc.cpp around line 1330. Both patches try to apply their changes to the bottom of the list, with context looking up. They modify each other's context. To fix this, they should be applied to the top looking only up or the bottom looking only down. (Not sure if you can actually do this, however.)

Hummm...  I was able to use one patch or the other.  When trying to use both patches, I couldn't apply the patches.  Perhaps I did something wrong.  I will try again.

When applying gavin's monitor patch first and your getblock patch second, the following error occurs: http://pastebin.com/CUr1cvgS
When applying your getblock patch first and gavin's monitor patch second, the following error occurs: http://pastebin.com/6UY4dscY

That is incorrect.

The content in gavin's patch makes my getblock largely redundant -- but they are not incompatible.

Just a quick note that this patch is not compatible with https://bitcointalksearch.org/topic/m.15065

Patch updated to latest SVN, with a few of the suggested gavinchanges:

• 'getblock' renamed to 'getblockbycount'
• s/ver/version/
• vin_sz and vout_sz removed, redundant

blkindex.dat contains all the transactions, in records that look like:
  key:  "tx" + HASH_OF_TRANSACTION
  value: serialized CTxIndex

The CTxIndex points to a record in one of the blk000n.dat files.

That does bring up an interesting point, though:  if lightweight clients are implemented, they wouldn't be able to support an arbitrary 'gettransaction'.

Is there an index that indexes all transaction ids for all blocks?

Or should 'gettransaction' only work for wallet transactions?

I've been working on a "monitoraddress" / "monitorblocks" patch, so bitcoind will POST transaction/block information to an arbitrary URL.

And it seems to me "getblock" should be split into "getblockbycount", "getblockbyhash" and "gettransaction".  I also have some suggestions for making the naming more consistent:

I'd suggest getblockby* return:

Reasoning:
  blockcount instead of 'height' or 'depth' because getinfo returns "blockcount"
  getblockbyhash might return a block not on the "best" chain; return blockcount=-1 in that case?
  version instead of 'ver' because abbrvs shd b avoided (thy cn b hrd to rd/rmbr)
  only return transaction hashes; if you need more transaction details, call gettransaction.
  No n_tx (or n_anything) because it is implicit in length of returned array

(stuff I'm not sure about:  what is 'bits' in the implemented getblock?  Is that difficulty?  Does anybody really need all the detailed merkle tree info?)

I'd suggest gettransaction return:

Reasoning:
  Returning the CScript string seems like it is both too much detail AND not very useful (I want the bitcoin addresses, NOT OP_DUP 0xblahblahblah ...)
  "from" is just a list of pointers to previous transaction "to"s
  confirmations is duplicate info (you could getblockbyhash and then look at its blockcount), but is so useful I think it belongs here.

Patch updated to latest SVN.

Patch updated for current SVN, r146, aka the "break patches with a CR/LF policy change" revision.

Same patch URL (see top of this thread), no change in implementation or operation.

Patch updated for current SVN, r130.  Same patch URL, no change in implementation or operation.

Oh! You have already implemented the feature I need!

Great!

Updated patch to v2, with a simple cast fix for 32-bit machines.

Same URL: http://gtf.org/garzik/bitcoin/patch.bitcoin-getblock