Technically cs_sendProcessing is entirely useless now because it
is only ever taken on the one MessageHandler thread, but because
there may be multiple of those in the future, it is left in place
cs_vSend is used for two purposes - to lock the datastructures used
to queue messages to place on the wire and to only call
SendMessages once at a time per-node. I believe SendMessages used
to access some of the vSendMsg stuff, but it doesn't anymore, so
these locks do not need to be on the same mutex, and also make
deadlocking much more likely.
This disentangles the script validation skipping from checkpoints.
A new option is introduced "assumevalid" which specifies a block whos
ancestors we assume all have valid scriptsigs and so we do not check
them when they are also burried under the best header by two weeks
worth of work.
Unlike checkpoints this has no influence on consensus unless you set
it to a block with an invalid history. Because of this it can be
easily be updated without risk of influencing the network consensus.
This results in a massive IBD speedup.
This approach was independently recommended by Peter Todd and Luke-Jr
since POW based signature skipping (see PR#9180) does not have the
verifiable properties of a specific hash and may create bad incentives.
The downside is that, like checkpoints, the defaults bitrot and older
releases will sync slower. On the plus side users can provide their
own value here, and if they set it to something crazy all that will
happen is more time will be spend validating signatures.
Checkblocks and checklevel are also moved to the hidden debug options:
Especially now that checkblocks has a low default there is little need
to change these settings, and users frequently misunderstand them as
influencing security or IBD speed. By hiding them we offset the
space added by this new option.
vRecvMsg is now only touched by the socket handler thread.
The accounting vars (nRecvBytes/nLastRecv/mapRecvBytesPerMsgCmd) are also
only used by the socket handler thread, with the exception of queries from
rpc/gui. These accesses are not threadsafe, but they never were. This needs to
be addressed separately.
Also, update comment describing data flow
Similar to the recv flag, but this one indicates whether or not the net's send
buffer is full.
The socket handler checks the send queue when a new message is added and pauses
if necessary, and possibly unpauses after each message is drained from its buffer.
Messages are dumped very quickly from the socket handler to the processor, so
it's the depth of the processing queue that's interesting.
The socket handler checks the process queue's size during the brief message
hand-off and pauses if necessary, and the processor possibly unpauses each time
a message is popped off of its queue.
In order to sleep accurately, the message handler needs to know if _any_ node
has more processing that it should do before the entire thread sleeps.
Rather than returning a value that represents whether ProcessMessages
encountered a message that should trigger a disconnnect, interpret the return
value as whether or not that node has more work to do.
Also, use a global fProcessWake value that can be set by other threads,
which takes precedence (for one cycle) over the messagehandler's decision.
Note that the previous behavior was to only process one message per loop
(except in the case of a bad checksum or invalid header). That was changed in
PR #3180.
The only change here in that regard is that the current node now falls to the
back of the processing queue for the bad checksum/invalid header cases.
In spite of the name FindLatestBefore used std::lower_bound to try
to find the earliest block with a nTime greater or equal to the
the requested value. But lower_bound uses bisection and requires
the input to be ordered with respect to the comparison operation.
Block times are not well ordered.
I don't know what lower_bound is permitted to do when the data
is not sufficiently ordered, but it's probably not good.
(I could construct an implementation which would infinite loop...)
To resolve the issue this commit introduces a maximum-so-far to the
block indexes and searches that.
For clarity the function is renamed to reflect what it actually does.
An issue that remains is that there is no grace period in importmulti:
If a address is created at time T and a send is immediately broadcast
and included by a miner with a slow clock there may not yet have been
any block with at least time T.
The normal rescan has a grace period of 7200 seconds, but importmulti
does not.
There is still a call to ActivateBestChain with cs_main if a peer
requests the block prior to it being validated, but this one is
more specifically-gated, so should be less of an issue.
AcceptToMemoryPool has several classes of return false statements.
- return state.Invalid or state.DoS directly itself
- return false and set fMissingInputs (state is valid)
- return false and state is set by failed CheckTransaction
- return false and state is set by failed CheckInputs.
This commit patches the last case where the state variable was reused for additional calls to CheckInputs to identify witness stripping as cause of validation failure. After this commit, it should be the case that if !fMissingInputs, state is always Invalid if AcceptToMemoryPool returns false.
practicalswift