The current getblocktxn implementation drops and ignores requests for old
blocks, which causes occasional sync_block timeouts during the
p2p-compactblocks.py test as reported in
https://github.com/bitcoin/bitcoin/issues/8842.
The p2p-compactblocks.py test setup creates many new blocks in a short
period of time, which can lead to getblocktxn requests for blocks below the
hardcoded depth limit of 10 blocks. This commit changes the getblocktxn
handler not to ignore these requests, so the peer nodes in the test setup
will reliably be able to sync.
The protocol change is documented in BIP-152 update "Allow block responses
to getblocktxn requests" at https://github.com/bitcoin/bips/pull/469.
The protocol change is not expected to affect nodes running outside the test
environment, because there shouldn't normally be lots of new blocks being
rapidly added that need to be synced.
Clear test_node.last_block before requesting blocks in the
compactblocks_not_at_tip test so comparisons won't fail if a blocks were received
before the test started.
The bug doesn't currently cause any problems due to the order tests run, but
this will change in the next commit.
Bug caused the wait_for_block_announcement to be called on the wrong node,
leading to nondeterminism and occasional test failures. Bug was introduced in
merge commit:
d075479 Merge #8882: [qa] Fix race conditions in p2p-compactblocks.py and sendheaders.py
Underlying commits which conflicted were:
27acfc1 [qa] Update p2p-compactblocks.py for compactblocks v2
6976db2 [qa] Another attempt to fix race condition in p2p-compactblocks.py
The first commit changed the test_compactblock_construction function signature
and second commit added code which wasn't updated during the merge to use the
new arguments.
Suhas Daftuar <sdaftuar@chaincode.com> noticed the bug and suggested the fix.
0334430 Add some missing includes (Pieter Wuille)
4100499 Return shared_ptr<CTransaction> from mempool removes (Pieter Wuille)
51f2783 Make removed and conflicted arguments optional to remove (Pieter Wuille)
f48211b Bypass removeRecursive in removeForReorg (Pieter Wuille)
nMaxInbound might very well be 0 or -1, if the user prefers to keep
a small number of maxconnections.
Note: nMaxInbound of -1 means that the user set maxconnections
to 8 or less, but we still want to keep an additional slot for
the feeler connection.
> This new feature is enabled by default if Bitcoin Core is listening, and a connection to Tor can be made. It can be configured with the -listenonion, -torcontrol and -torpassword settings. To show verbose debugging information, pass -debug=tor.
But it is correct to say that the feature is enabled *regardless* of whether a connection to Tor can be made.
I propose to clarify that so that users can eliminate these in their logs (when `listen=1` and no Tor).
And I think it's okay to clarify about the `listen` option, because on several occasions when I read this before I always assumed `listening` meant `server=1` which cost me a lot of time in troubleshooting.
```
2016-10-24 06:19:22.551029 tor: Error connecting to Tor control socket
2016-10-24 06:19:22.551700 tor: Not connected to Tor control port 127.0.0.1:9051, trying to reconnect
```
### What version of bitcoin-core are you using?
0.12.1
Note that this is not a major issue as, in order for the missing
lock to cause issues, you have to receive a GETBLOCKTXN message
while reindexing, adding a block header via RPC, etc, which results
in either a table rehash or an insert into the bucket which you are
currently looking at.
3154d6e [Qt] use NotifyHeaderTip's height and date for the progress update (Jonas Schnelli)
0a261b6 Use pindexBestHeader instead of setBlockIndexCandidates for NotifyHeaderTip() (Jonas Schnelli)
There were discrepancies between usage of "block chain" and "blockchain", I've changed them to the latter. The reason for this was that Wikipedia when describing this data structure writes "A blockchain — *originally block chain*", so it seemed the more appropriate term.
Russell Yanofsky