Bitcoin Forum

Bitcoin => Development & Technical Discussion => Topic started by: pmlyon on July 25, 2013, 01:05:59 AM



Title: Wiki clarification on headers payload
Post by: pmlyon on July 25, 2013, 01:05:59 AM
I noticed tonight that the protocol specification on the bitcoin wiki says that block headers have an additional varint transaction count at the end in the headers payload: "Note that the block headers in this packet include a transaction count (a var_int, so there can be more than 81 bytes per header) as opposed to the block headers which are sent to miners."

https://en.bitcoin.it/wiki/Protocol_specification#headers

However, under the table listing for the block header format it says: "txn_count, uint8_t, Number of transaction entries, this value is always 0".

https://en.bitcoin.it/wiki/Protocol_specification#Block_Headers

I understand that if the count is always zero a uint8_t and a varint will come out to the same thing, but I'm wondering what the software actually interprets this field as. I'd like to submit an update to the wiki to correct it, but I couldn't tell what the correct format is from the source code as I'm not familiar with it at all.

Does anyone familiar with the source code know whether it should be a uint8_t or a varint? Thanks.


Title: Re: Wiki clarification on headers payload
Post by: kjj on July 25, 2013, 01:19:56 AM
Look in main.cpp, function ProcessMessage.  Search for "getheaders" because ProcessMessage is a long function.


Title: Re: Wiki clarification on headers payload
Post by: pmlyon on July 25, 2013, 01:29:39 AM
I see the comment there that says "// we must use CBlocks, as CBlockHeaders won't include the 0x00 nTx count at the end". I'm not sure what CBlock actually uses as its logic for nTx though. Even if it only ever wrote out 0, if someone put a 3 byte varint there, would it read that correctly?


Title: Re: Wiki clarification on headers payload
Post by: kjj on July 25, 2013, 04:31:44 AM
Ok, so it iterates starting with CBlockIndex* pindex and pushes the result of pindex->GetBlockHeader() into a vector of CBlock.

CBlock includes a vector of CTransaction, vtx.

CBlock::IMPLEMENT_SERIALIZE does the block proper, and then the transaction vector.

Because this is done in a way that does not ever add any transactions, the transaction list here is always serialized as 0x00, which is the proper coding for a count of zero followed by zero records.

To be perfectly honest, I have no idea where this message is processed when received.  ProcessMessage has a giant if/elseif/else structure that looks for the ASCII string of each message type.  None of the ifs seem to care about the string "headers", and I can't find it anywhere else either.


Title: Re: Wiki clarification on headers payload
Post by: TierNolan on July 25, 2013, 09:36:56 AM
To be perfectly honest, I have no idea where this message is processed when received.  ProcessMessage has a giant if/elseif/else structure that looks for the ASCII string of each message type.  None of the ifs seem to care about the string "headers", and I can't find it anywhere else either.

Yeah, it looks like the message is purely for SPV clients.


Title: Re: Wiki clarification on headers payload
Post by: pmlyon on July 31, 2013, 02:14:21 PM
Thanks for the info. My guess would be that SPV clients should read it as a varint, I can't imagine it'd be useful to only ever allow a single byte for a transaction count.


Title: Re: Wiki clarification on headers payload
Post by: Mike Hearn on July 31, 2013, 04:38:50 PM
The only codebase that uses headers is bitcoinj and it requires the final byte be a single null.


Title: Re: Wiki clarification on headers payload
Post by: pmlyon on July 31, 2013, 05:12:18 PM
Thanks, I'm using it in my node as well to be able to chain blocks very quickly before I download them. Based on what you're saying, it sounds like the wiki should indicate that the transaction count is a single byte then, not a varint?

Also, thanks for the awesome work you are doing with bitcoinj! I was able to integrate with the bitcoind comparison tool and that is going to be immensely helpful to me down the line.


Title: Re: Wiki clarification on headers payload
Post by: TierNolan on July 31, 2013, 05:42:14 PM
Based on what you're saying, it sounds like the wiki should indicate that the transaction count is a single byte then, not a varint?

It is a varint.  The varint for zero is a single 0 byte.

When the header is part of the block, it gives the transaction count.


Title: Re: Wiki clarification on headers payload
Post by: pmlyon on July 31, 2013, 06:07:17 PM
Based on what you're saying, it sounds like the wiki should indicate that the transaction count is a single byte then, not a varint?

It is a varint.  The varint for zero is a single 0 byte.

When the header is part of the block, it gives the transaction count.

Based on Mike's reply and in the context of the headers message, though, it sounds like there shouldn't be a varint here, just a single zero byte. For zero that comes out to the same thing but I'm trying to understand the correct parsing behavior. If I ever see a non-zero byte in that position what would the correct behavior be? Consider the entire message invalid? Alternatively, if I ignore the difference I'd need to know if I should read just that last byte or if I should try to read it as a varint.


Title: Re: Wiki clarification on headers payload
Post by: kjj on July 31, 2013, 06:52:32 PM
Based on what you're saying, it sounds like the wiki should indicate that the transaction count is a single byte then, not a varint?

It is a varint.  The varint for zero is a single 0 byte.

When the header is part of the block, it gives the transaction count.

Based on Mike's reply and in the context of the headers message, though, it sounds like there shouldn't be a varint here, just a single zero byte. For zero that comes out to the same thing but I'm trying to understand the correct parsing behavior. If I ever see a non-zero byte in that position what would the correct behavior be? Consider the entire message invalid? Alternatively, if I ignore the difference I'd need to know if I should read just that last byte or if I should try to read it as a varint.

/sigh

You should endeavor to follow Postel's law.  In this case, that means (using RFC2119 (http://www.ietf.org/rfc/rfc2119.txt) notation):

If you are creating a "headers" packet, you must include a properly coded varint representing the number of transactions that are included in the packet.  The number of transactions included must be zero.  (The only valid encoding for zero is a single octet: 0x00.  0x05 is right out.)

If you are reading a "headers" packet, you must parse that field as a varint.  You should reject any packets that include a non-zero number of transactions.  You may disconnect the peer that sent it, and attempt to publicly shame the authors of that implementation.


Title: Re: Wiki clarification on headers payload
Post by: pmlyon on July 31, 2013, 07:27:50 PM
/sigh

You should endeavor to follow Postel's law.  In this case, that means (using RFC2119 (http://www.ietf.org/rfc/rfc2119.txt) notation):

If you are creating a "headers" packet, you must include a properly coded varint representing the number of transactions that are included in the packet.  The number of transactions included must be zero.  (The only valid encoding for zero is a single octet: 0x00.  0x05 is right out.)

If you are reading a "headers" packet, you must parse that field as a varint.  You should reject any packets that include a non-zero number of transactions.  You may disconnect the peer that sent it, and attempt to publicly shame the authors of that implementation.

Thanks, that matches how I would have expected it to be implemented myself. I'll submit an update to the wiki tonight to drop the uint8_t in favor of varint.


Title: Re: Wiki clarification on headers payload
Post by: jgarzik on July 31, 2013, 08:49:43 PM
Based on what you're saying, it sounds like the wiki should indicate that the transaction count is a single byte then, not a varint?

It is a varint.  The varint for zero is a single 0 byte.

When the header is part of the block, it gives the transaction count.

Based on Mike's reply and in the context of the headers message, though, it sounds like there shouldn't be a varint here, just a single zero byte. For zero that comes out to the same thing but I'm trying to understand the correct parsing behavior. If I ever see a non-zero byte in that position what would the correct behavior be? Consider the entire message invalid? Alternatively, if I ignore the difference I'd need to know if I should read just that last byte or if I should try to read it as a varint.

/sigh

You should endeavor to follow Postel's law.  In this case, that means (using RFC2119 (http://www.ietf.org/rfc/rfc2119.txt) notation):

If you are creating a "headers" packet, you must include a properly coded varint representing the number of transactions that are included in the packet.  The number of transactions included must be zero.  (The only valid encoding for zero is a single octet: 0x00.  0x05 is right out.)

If you are reading a "headers" packet, you must parse that field as a varint.  You should reject any packets that include a non-zero number of transactions.  You may disconnect the peer that sent it, and attempt to publicly shame the authors of that implementation.

Sadly this is one of those cases where it can be easy to get it wrong.  It does seem worth investigating whether there was any breakage or behavior changes since 0.3.x days.  We have changed CBlock to CBlockHeader and it is entirely conceivable that one might have output the "number of transactions" variable, and another did not.  Worth checking.



Title: Re: Wiki clarification on headers payload
Post by: pmlyon on August 01, 2013, 12:23:50 PM
Sadly this is one of those cases where it can be easy to get it wrong.  It does seem worth investigating whether there was any breakage or behavior changes since 0.3.x days.  We have changed CBlock to CBlockHeader and it is entirely conceivable that one might have output the "number of transactions" variable, and another did not.  Worth checking.



Thanks Jeff. Do you happen to have access to the wiki? I didn't see a place to submit corrections. I was hoping I'd be able to submit something on the discussion page but there's no access to that either. I know it's a minor thing, but I noticed it and figured it'd be worth fixing up. :)


Title: Re: Wiki clarification on headers payload
Post by: TierNolan on August 01, 2013, 12:51:27 PM
Thanks Jeff. Do you happen to have access to the wiki? I didn't see a place to submit corrections. I was hoping I'd be able to submit something on the discussion page but there's no access to that either. I know it's a minor thing, but I noticed it and figured it'd be worth fixing up. :)

You have to make a (0.01BTC) donation (https://en.bitcoin.it/wiki/Special:BitcoinPayment) to the wiki to get write access.

It is supposed to tell you when you create an account, but apparently, it doesn't work very well.

I think the donation link gives a different address for each person.


Title: Re: Wiki clarification on headers payload
Post by: pmlyon on August 01, 2013, 01:07:48 PM
Thanks Jeff. Do you happen to have access to the wiki? I didn't see a place to submit corrections. I was hoping I'd be able to submit something on the discussion page but there's no access to that either. I know it's a minor thing, but I noticed it and figured it'd be worth fixing up. :)

You have to make a (0.01BTC) donation (https://en.bitcoin.it/wiki/Special:BitcoinPayment) to the wiki to get write access.

It is supposed to tell you when you create an account, but apparently, it doesn't work very well.

I think the donation link gives a different address for each person.

That was almost certainly me not reading the create account page very well. ;) I've sent in my donation, my first bitcoin purchase, thanks!


Title: Re: Wiki clarification on headers payload
Post by: pmlyon on August 01, 2013, 03:12:48 PM
I've submitted the change to the wiki: https://en.bitcoin.it/wiki/Protocol_specification#Block_Headers

I'm not sure if there's any kind of review process on these edits, so hopefully that's ok. :)


Title: Re: Wiki clarification on headers payload
Post by: jgarzik on August 02, 2013, 04:41:05 AM
Sadly this is one of those cases where it can be easy to get it wrong.  It does seem worth investigating whether there was any breakage or behavior changes since 0.3.x days.  We have changed CBlock to CBlockHeader and it is entirely conceivable that one might have output the "number of transactions" variable, and another did not.  Worth checking.

This may be the case.

Untested thesis, based on code read:  "headers" message format changed when https://github.com/bitcoin/bitcoin/pull/2013 was merged in Nov 2012.

You may have uncovered a protocol-related bitcoind bug.

Update: Incorrect.  I was misreading some code.



Title: Re: Wiki clarification on headers payload
Post by: pmlyon on August 02, 2013, 01:41:16 PM
Sadly this is one of those cases where it can be easy to get it wrong.  It does seem worth investigating whether there was any breakage or behavior changes since 0.3.x days.  We have changed CBlock to CBlockHeader and it is entirely conceivable that one might have output the "number of transactions" variable, and another did not.  Worth checking.

This may be the case.

Untested thesis, based on code read:  "headers" message format changed when https://github.com/bitcoin/bitcoin/pull/2013 was merged in Nov 2012.

You may have uncovered a protocol-related bitcoind bug.

Update: Incorrect.  I was misreading some code.



Thanks for checking into this! Was I correct to update the block headers table on the wiki to indicate that the value is a var_int that is always 0?


Title: Re: Wiki clarification on headers payload
Post by: jgarzik on August 02, 2013, 02:14:24 PM
Thanks for checking into this! Was I correct to update the block headers table on the wiki to indicate that the value is a var_int that is always 0?

For the "headers" response message, that is correct.