Merge #13946: p2p: Clarify control flow in ProcessMessage

fa6c3dea42 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
This commit is contained in:
Wladimir J. van der Laan 2018-08-25 17:57:05 +02:00
commit 021dce935a
No known key found for this signature in database
GPG Key ID: 1E4AED62986CD25D

View File

@ -1618,8 +1618,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
return true;
}
else if (strCommand == NetMsgType::VERSION)
{
if (strCommand == NetMsgType::VERSION) {
// Each connection can only send one version message
if (pfrom->nVersion != 0)
{
@ -1793,9 +1792,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
return true;
}
else if (pfrom->nVersion == 0)
{
if (pfrom->nVersion == 0) {
// Must have a version message before anything else
LOCK(cs_main);
Misbehaving(pfrom->GetId(), 1);
@ -1839,18 +1836,17 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion));
}
pfrom->fSuccessfullyConnected = true;
return true;
}
else if (!pfrom->fSuccessfullyConnected)
{
if (!pfrom->fSuccessfullyConnected) {
// Must have a verack message before anything else
LOCK(cs_main);
Misbehaving(pfrom->GetId(), 1);
return false;
}
else if (strCommand == NetMsgType::ADDR)
{
if (strCommand == NetMsgType::ADDR) {
std::vector<CAddress> vAddr;
vRecv >> vAddr;
@ -1897,16 +1893,16 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
pfrom->fGetAddr = false;
if (pfrom->fOneShot)
pfrom->fDisconnect = true;
return true;
}
else if (strCommand == NetMsgType::SENDHEADERS)
{
if (strCommand == NetMsgType::SENDHEADERS) {
LOCK(cs_main);
State(pfrom->GetId())->fPreferHeaders = true;
return true;
}
else if (strCommand == NetMsgType::SENDCMPCT)
{
if (strCommand == NetMsgType::SENDCMPCT) {
bool fAnnounceUsingCMPCTBLOCK = false;
uint64_t nCMPCTBLOCKVersion = 0;
vRecv >> fAnnounceUsingCMPCTBLOCK >> nCMPCTBLOCKVersion;
@ -1926,11 +1922,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
State(pfrom->GetId())->fSupportsDesiredCmpctVersion = (nCMPCTBLOCKVersion == 1);
}
}
return true;
}
else if (strCommand == NetMsgType::INV)
{
if (strCommand == NetMsgType::INV) {
std::vector<CInv> vInv;
vRecv >> vInv;
if (vInv.size() > MAX_INV_SZ)
@ -1984,11 +1979,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
}
}
}
return true;
}
else if (strCommand == NetMsgType::GETDATA)
{
if (strCommand == NetMsgType::GETDATA) {
std::vector<CInv> vInv;
vRecv >> vInv;
if (vInv.size() > MAX_INV_SZ)
@ -2006,11 +2000,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
pfrom->vRecvGetData.insert(pfrom->vRecvGetData.end(), vInv.begin(), vInv.end());
ProcessGetData(pfrom, chainparams, connman, interruptMsgProc);
return true;
}
else if (strCommand == NetMsgType::GETBLOCKS)
{
if (strCommand == NetMsgType::GETBLOCKS) {
CBlockLocator locator;
uint256 hashStop;
vRecv >> locator >> hashStop;
@ -2075,11 +2068,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
break;
}
}
return true;
}
else if (strCommand == NetMsgType::GETBLOCKTXN)
{
if (strCommand == NetMsgType::GETBLOCKTXN) {
BlockTransactionsRequest req;
vRecv >> req;
@ -2125,11 +2117,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
assert(ret);
SendBlockTransactions(block, req, pfrom, connman);
return true;
}
else if (strCommand == NetMsgType::GETHEADERS)
{
if (strCommand == NetMsgType::GETHEADERS) {
CBlockLocator locator;
uint256 hashStop;
vRecv >> locator >> hashStop;
@ -2193,11 +2184,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
// in the SendMessages logic.
nodestate->pindexBestHeaderSent = pindex ? pindex : chainActive.Tip();
connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::HEADERS, vHeaders));
return true;
}
else if (strCommand == NetMsgType::TX)
{
if (strCommand == NetMsgType::TX) {
// Stop processing the transaction early if
// We are in blocks only mode and peer is either not whitelisted or whitelistrelay is off
if (!fRelayTxes && (!pfrom->fWhitelisted || !gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY)))
@ -2381,10 +2371,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
Misbehaving(pfrom->GetId(), nDoS);
}
}
return true;
}
else if (strCommand == NetMsgType::CMPCTBLOCK && !fImporting && !fReindex) // Ignore blocks received while importing
if (strCommand == NetMsgType::CMPCTBLOCK && !fImporting && !fReindex) // Ignore blocks received while importing
{
CBlockHeaderAndShortTxIDs cmpctblock;
vRecv >> cmpctblock;
@ -2602,10 +2592,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
MarkBlockAsReceived(pblock->GetHash());
}
}
return true;
}
else if (strCommand == NetMsgType::BLOCKTXN && !fImporting && !fReindex) // Ignore blocks received while importing
if (strCommand == NetMsgType::BLOCKTXN && !fImporting && !fReindex) // Ignore blocks received while importing
{
BlockTransactions resp;
vRecv >> resp;
@ -2677,10 +2667,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
mapBlockSource.erase(pblock->GetHash());
}
}
return true;
}
else if (strCommand == NetMsgType::HEADERS && !fImporting && !fReindex) // Ignore headers received while importing
if (strCommand == NetMsgType::HEADERS && !fImporting && !fReindex) // Ignore headers received while importing
{
std::vector<CBlockHeader> headers;
@ -2705,7 +2695,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
return ProcessHeadersMessage(pfrom, connman, headers, chainparams, should_punish);
}
else if (strCommand == NetMsgType::BLOCK && !fImporting && !fReindex) // Ignore blocks received while importing
if (strCommand == NetMsgType::BLOCK && !fImporting && !fReindex) // Ignore blocks received while importing
{
std::shared_ptr<CBlock> pblock = std::make_shared<CBlock>();
vRecv >> *pblock;
@ -2731,11 +2721,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
LOCK(cs_main);
mapBlockSource.erase(pblock->GetHash());
}
return true;
}
else if (strCommand == NetMsgType::GETADDR)
{
if (strCommand == NetMsgType::GETADDR) {
// This asymmetric behavior for inbound and outbound connections was introduced
// to prevent a fingerprinting attack: an attacker can send specific fake addresses
// to users' AddrMan and later request them by sending getaddr messages.
@ -2759,11 +2748,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
FastRandomContext insecure_rand;
for (const CAddress &addr : vAddr)
pfrom->PushAddress(addr, insecure_rand);
return true;
}
else if (strCommand == NetMsgType::MEMPOOL)
{
if (strCommand == NetMsgType::MEMPOOL) {
if (!(pfrom->GetLocalServices() & NODE_BLOOM) && !pfrom->fWhitelisted)
{
LogPrint(BCLog::NET, "mempool request with bloom filters disabled, disconnect peer=%d\n", pfrom->GetId());
@ -2780,11 +2768,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
LOCK(pfrom->cs_inventory);
pfrom->fSendMempool = true;
return true;
}
else if (strCommand == NetMsgType::PING)
{
if (strCommand == NetMsgType::PING) {
if (pfrom->nVersion > BIP0031_VERSION)
{
uint64_t nonce = 0;
@ -2802,11 +2789,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
// return very quickly.
connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::PONG, nonce));
}
return true;
}
else if (strCommand == NetMsgType::PONG)
{
if (strCommand == NetMsgType::PONG) {
int64_t pingUsecEnd = nTimeReceived;
uint64_t nonce = 0;
size_t nAvail = vRecv.in_avail();
@ -2859,11 +2845,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
if (bPingFinished) {
pfrom->nPingNonceSent = 0;
}
return true;
}
else if (strCommand == NetMsgType::FILTERLOAD)
{
if (strCommand == NetMsgType::FILTERLOAD) {
CBloomFilter filter;
vRecv >> filter;
@ -2880,11 +2865,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
pfrom->pfilter->UpdateEmptyFull();
pfrom->fRelayTxes = true;
}
return true;
}
else if (strCommand == NetMsgType::FILTERADD)
{
if (strCommand == NetMsgType::FILTERADD) {
std::vector<unsigned char> vData;
vRecv >> vData;
@ -2905,19 +2889,19 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
LOCK(cs_main);
Misbehaving(pfrom->GetId(), 100);
}
return true;
}
else if (strCommand == NetMsgType::FILTERCLEAR)
{
if (strCommand == NetMsgType::FILTERCLEAR) {
LOCK(pfrom->cs_filter);
if (pfrom->GetLocalServices() & NODE_BLOOM) {
pfrom->pfilter.reset(new CBloomFilter());
}
pfrom->fRelayTxes = true;
return true;
}
else if (strCommand == NetMsgType::FEEFILTER) {
if (strCommand == NetMsgType::FEEFILTER) {
CAmount newFeeFilter = 0;
vRecv >> newFeeFilter;
if (MoneyRange(newFeeFilter)) {
@ -2927,20 +2911,17 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
}
LogPrint(BCLog::NET, "received: feefilter of %s from peer=%d\n", CFeeRate(newFeeFilter).ToString(), pfrom->GetId());
}
return true;
}
else if (strCommand == NetMsgType::NOTFOUND) {
if (strCommand == NetMsgType::NOTFOUND) {
// We do not care about the NOTFOUND message, but logging an Unknown Command
// message would be undesirable as we transmit it ourselves.
return true;
}
else {
// Ignore unknown commands for extensibility
LogPrint(BCLog::NET, "Unknown command \"%s\" from peer=%d\n", SanitizeString(strCommand), pfrom->GetId());
}
// Ignore unknown commands for extensibility
LogPrint(BCLog::NET, "Unknown command \"%s\" from peer=%d\n", SanitizeString(strCommand), pfrom->GetId());
return true;
}