pmlyon (OP)
Member
Offline
Activity: 72
Merit: 10
|
|
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#headersHowever, 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_HeadersI 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.
|
|
|
|
kjj
Legendary
Offline
Activity: 1302
Merit: 1026
|
|
July 25, 2013, 01:19:56 AM |
|
Look in main.cpp, function ProcessMessage. Search for "getheaders" because ProcessMessage is a long function.
|
17Np17BSrpnHCZ2pgtiMNnhjnsWJ2TMqq8 I routinely ignore posters with paid advertising in their sigs. You should too.
|
|
|
pmlyon (OP)
Member
Offline
Activity: 72
Merit: 10
|
|
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?
|
|
|
|
kjj
Legendary
Offline
Activity: 1302
Merit: 1026
|
|
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.
|
17Np17BSrpnHCZ2pgtiMNnhjnsWJ2TMqq8 I routinely ignore posters with paid advertising in their sigs. You should too.
|
|
|
TierNolan
Legendary
Offline
Activity: 1232
Merit: 1104
|
|
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.
|
1LxbG5cKXzTwZg9mjL3gaRE835uNQEteWF
|
|
|
pmlyon (OP)
Member
Offline
Activity: 72
Merit: 10
|
|
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.
|
|
|
|
Mike Hearn
Legendary
Offline
Activity: 1526
Merit: 1134
|
|
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.
|
|
|
|
pmlyon (OP)
Member
Offline
Activity: 72
Merit: 10
|
|
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.
|
|
|
|
TierNolan
Legendary
Offline
Activity: 1232
Merit: 1104
|
|
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.
|
1LxbG5cKXzTwZg9mjL3gaRE835uNQEteWF
|
|
|
pmlyon (OP)
Member
Offline
Activity: 72
Merit: 10
|
|
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.
|
|
|
|
kjj
Legendary
Offline
Activity: 1302
Merit: 1026
|
|
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 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.
|
17Np17BSrpnHCZ2pgtiMNnhjnsWJ2TMqq8 I routinely ignore posters with paid advertising in their sigs. You should too.
|
|
|
pmlyon (OP)
Member
Offline
Activity: 72
Merit: 10
|
|
July 31, 2013, 07:27:50 PM |
|
/sigh You should endeavor to follow Postel's law. In this case, that means (using RFC2119 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.
|
|
|
|
jgarzik
Legendary
Offline
Activity: 1596
Merit: 1099
|
|
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 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.
|
Jeff Garzik, Bloq CEO, former bitcoin core dev team; opinions are my own. Visit bloq.com / metronome.io Donations / tip jar: 1BrufViLKnSWtuWGkryPsKsxonV2NQ7Tcj
|
|
|
pmlyon (OP)
Member
Offline
Activity: 72
Merit: 10
|
|
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.
|
|
|
|
TierNolan
Legendary
Offline
Activity: 1232
Merit: 1104
|
|
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 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.
|
1LxbG5cKXzTwZg9mjL3gaRE835uNQEteWF
|
|
|
pmlyon (OP)
Member
Offline
Activity: 72
Merit: 10
|
|
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 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!
|
|
|
|
|
jgarzik
Legendary
Offline
Activity: 1596
Merit: 1099
|
|
August 02, 2013, 04:41:05 AM Last edit: August 02, 2013, 01:35:45 PM by jgarzik |
|
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.
|
Jeff Garzik, Bloq CEO, former bitcoin core dev team; opinions are my own. Visit bloq.com / metronome.io Donations / tip jar: 1BrufViLKnSWtuWGkryPsKsxonV2NQ7Tcj
|
|
|
pmlyon (OP)
Member
Offline
Activity: 72
Merit: 10
|
|
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?
|
|
|
|
jgarzik
Legendary
Offline
Activity: 1596
Merit: 1099
|
|
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.
|
Jeff Garzik, Bloq CEO, former bitcoin core dev team; opinions are my own. Visit bloq.com / metronome.io Donations / tip jar: 1BrufViLKnSWtuWGkryPsKsxonV2NQ7Tcj
|
|
|
|