We could be reading multiple messages from a socket buffer at once _without actually processing them yet_ which means that `fSuccessfullyConnected` might not be switched to `true` at the time we already parsed `VERACK` message and started to parse the next one. This is basically a false positive and we drop a legit node as a result even though the order of messages sent by this node was completely fine. To fix this I partially reverted #2790 (where the issue was initially introduced) and moved the logic for tracking the first message into ProcessMessage instead.
fad63eb [logging] Don't incorrectly log that REJECT messages are unknown. (John Newbery)
Pull request description:
Reject messages are logged to debug.log if NET debug logging is enabled.
Because of the way the `ProcessMessages()` function is structured,
processing for REJECT messages will also drop through to the default
branch and incorrectly log `Unknown command "reject" from peer-?`. Fix
that by exiting from `ProcessMessages()` early.
without this PR:
```
2018-05-03T17:37:00.930600Z received: reject (21 bytes) peer=0
2018-05-03T17:37:00.930620Z Reject message code 16: spammy spam
2018-05-03T17:37:00.930656Z Unknown command "reject" from peer=0
```
with this PR:
```
2018-05-03T17:35:04.751246Z received: reject (21 bytes) peer=0
2018-05-03T17:35:04.751274Z Reject message code 16: spammy spam
```
Tree-SHA512: 5c84c98433ab99e0db2dd481f9c2db6f87ff0d39022ff317a791737e918714bbcb4a23e81118212ed8e594ebcf098ab7f52f7fd5e21ebc3f07b1efb279b9b30b
fa6c3dea420b6c50c164ccc34f4e9e8a7d9a8022 p2p: Clarify control flow in ProcessMessage() (MarcoFalke)
Pull request description:
`ProcessMessage` is effectively a massive switch case construct. In the past there were attempts to clarify the control flow in `ProcessMessage()` by moving each case into a separate static function (see #9608). It was closed because it wasn't clear if moving each case into a function was the right approach.
Though, we can quasi treat each case as a function by adding a return statement to each case. (Can be seen as a continuation of bugfix #13162)
This patch does exactly that.
Also note that this patch is a subset of previous approaches such as #9608 and #10145.
Review suggestion: `git diff HEAD~ --function-context`
Tree-SHA512: 91f6106840de2f29bb4f10d27bae0616b03a91126e6c6013479e1dd79bee53f22a78902b631fe85517dd5dc0fa7239939b4fefc231851a13c819458559f6c201
Sometimes the node we ask for mnlistdiff is so fast to reply that we receive the message back before we reset `last_mnlistdiff`. To fix this we should reset it before sending the message, not after.
* Check MNs up to 24 blocks deep when verifying `dstx`
* Handle DSTX-es more like regular txes and not like "other" invs
* Try asking for a DSTX too when trying to find missing tx parents
* Check DSTX-es when chainlock arrives
`HasChainLock` was always `false` in `IsExpired` because tip is updated before the corresponding chainlock is received
* Apply `Handle DSTX-es more like regular txes` idea to `AlreadyHave()`
* Alternative handling of DSTX+recentRejects
Co-authored-by: Alexander Block <ablock84@gmail.com>
