Pull request #948 introduced a fix for nodes stuck on a long side branch
of the main chain. The fix was non-functional however, as the additional
getdata request was created in a first step of processing, but dropped
in a second step as it was considered redundant. This commits fixes it
by sending the request directly.
Conflict:
* cs_main in ProcessMessages() (before calling ProcessMessages)
* cs_vSend in CNode::BeginMessage
versus:
* cs_vSend in ThreadMessageHandler2 (before calling SendMessages)
* cs_main in SendMessages
Even though cs_vSend is a try_lock, if it succeeds simultaneously with
the locking of cs_main in ProcessMessages(), it could cause a deadlock.
Doing so would allow an attack on old nodes, which would relay a
standard transaction spending a BIP16 output in an invalid way,
until reaching a new node, which will disconnect their peer.
Reported by makomk on IRC.
In cases of very large reorganisations (hundreds of blocks), a situation
may appear where an 'inv' is sent as response to a 'getblocks', but the
last block mentioned in the inv is already known to the receiver node.
However, the supplying node uses a request for this last block as a
trigger to send the rest of the inv blocks. If it never comes, the block
chain download is stuck.
This commit makes the receiver node always request the last inv'ed block,
even if it is already known, to prevent this problem.
Sometimes a new block arrives in a new chain that was already the
best valid one, but wasn't marked that way. This happens for example
when network rules change to recover after a fork.
In this case, it is not necessary to do the entire reorganisation
inside a single db commit. These can become huge, and exceed the
objects/lockers limits in bdb. This patch limits the blocks the
actual reorganisation is applied to, and adds the next blocks
afterwards in separate db transactions.
Introduce the following network rule:
* a block is not valid if it contains a transaction whose hash
already exists in the block chain, unless all that transaction's
outputs were already spent before said block.
Warning: this is effectively a network rule change, with potential
risk for forking the block chain. Leaving this unfixed carries the
same risk however, for attackers that can cause a reorganisation
in part of the network.
Thanks to Russell O'Connor and Ben Reeves.
Introduce the following network rule:
* a block is not valid if it contains a transaction whose hash
already exists in the block chain, unless all that transaction's
outputs were already spent before said block.
Warning: this is effectively a network rule change, with potential
risk for forking the block chain. Leaving this unfixed carries the
same risk however, for attackers that can cause a reorganisation
in part of the network.
Thanks to Russell O'Connor and Ben Reeves.
