From 0905b911dcb3571f6d640779bce8f4c1469c1b1d Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Tue, 12 Mar 2019 08:03:52 +0100 Subject: [PATCH 01/11] Actually use cached most recent compact block This seems to be backported wrongly. In the Bitcoin code, there is a condition on requested witness data and we took the other branch which recreates the compact block. We should have taken the other branch because we always send with witness data (there is no Segwit in Dash). --- src/net_processing.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 8df501141116..0850f5c0e539 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3348,8 +3348,7 @@ bool SendMessages(CNode* pto, CConnman& connman, const std::atomic& interr { LOCK(cs_most_recent_block); if (most_recent_block_hash == pBestIndex->GetBlockHash()) { - CBlockHeaderAndShortTxIDs cmpctblock(*most_recent_block); - connman.PushMessage(pto, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); + connman.PushMessage(pto, msgMaker.Make(NetMsgType::CMPCTBLOCK, *most_recent_compact_block)); fGotBlockFromCache = true; } } From d1db98c67a9ae1d95d3fe065ee53016c08807d31 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 13 Apr 2017 17:20:31 +0200 Subject: [PATCH 02/11] Merge #9665: Use cached [compact] blocks to respond to getdata messages b49ad44 Add comment about cs_most_recent_block coverage (Matt Corallo) c47f5b7 Cache witness-enabled state with recent-compact-block-cache (Matt Corallo) efc135f Use cached [compact] blocks to respond to getdata messages (Matt Corallo) Tree-SHA512: ffc478bddbf14b8ed304a3041f47746520ce545bdeffa9652eff2ccb25c8b0d5194abe72568c10f9c1b246ee361176ba217767af834752a2ca7263d292005e87 --- src/net_processing.cpp | 51 +++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 0850f5c0e539..fdd34c20d297 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -794,6 +794,7 @@ void PeerLogicValidation::SyncTransaction(const CTransaction& tx, const CBlockIn } } +// All of the following cache a recent block, and are protected by cs_most_recent_block static CCriticalSection cs_most_recent_block; static std::shared_ptr most_recent_block; static std::shared_ptr most_recent_compact_block; @@ -1045,6 +1046,13 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam { bool send = false; BlockMap::iterator mi = mapBlockIndex.find(inv.hash); + std::shared_ptr a_recent_block; + std::shared_ptr a_recent_compact_block; + { + LOCK(cs_most_recent_block); + a_recent_block = most_recent_block; + a_recent_compact_block = most_recent_compact_block; + } if (mi != mapBlockIndex.end()) { if (mi->second->nChainTx && !mi->second->IsValid(BLOCK_VALID_SCRIPTS) && @@ -1054,11 +1062,6 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam // before ActivateBestChain but after AcceptBlock). // In this case, we need to run ActivateBestChain prior to checking the relay // conditions below. - std::shared_ptr a_recent_block; - { - LOCK(cs_most_recent_block); - a_recent_block = most_recent_block; - } CValidationState dummy; ActivateBestChain(dummy, Params(), a_recent_block); } @@ -1090,13 +1093,20 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam } // Pruned nodes may have deleted the block, so check whether // it's available before trying to send. - if (send && (mi->second->nStatus & BLOCK_HAVE_DATA)) { - // Send block from disk - CBlock block; - if (!ReadBlockFromDisk(block, (*mi).second, consensusParams)) - assert(!"cannot load block from disk"); + if (send && (mi->second->nStatus & BLOCK_HAVE_DATA)) + { + std::shared_ptr pblock; + if (a_recent_block && a_recent_block->GetHash() == (*mi).second->GetBlockHash()) { + pblock = a_recent_block; + } else { + // Send block from disk + std::shared_ptr pblockRead = std::make_shared(); + if (!ReadBlockFromDisk(*pblockRead, (*mi).second, consensusParams)) + assert(!"cannot load block from disk"); + pblock = pblockRead; + } if (inv.type == MSG_BLOCK) - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::BLOCK, block)); + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::BLOCK, *pblock)); else if (inv.type == MSG_FILTERED_BLOCK) { bool sendMerkleBlock = false; @@ -1105,7 +1115,7 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam LOCK(pfrom->cs_filter); if (pfrom->pfilter) { sendMerkleBlock = true; - merkleBlock = CMerkleBlock(block, *pfrom->pfilter); + merkleBlock = CMerkleBlock(*pblock, *pfrom->pfilter); } } if (sendMerkleBlock) { @@ -1118,7 +1128,7 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam // however we MUST always provide at least what the remote peer needs typedef std::pair PairType; BOOST_FOREACH(PairType& pair, merkleBlock.vMatchedTxn) - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, *block.vtx[pair.first])); + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, *pblock->vtx[pair.first])); } // else // no response @@ -1129,11 +1139,16 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam // they won't have a useful mempool to match against a compact block, // and we don't feel like constructing the object for them, so // instead we respond with the full, non-compact block. - if (CanDirectFetch(consensusParams) && mi->second->nHeight >= chainActive.Height() - MAX_CMPCTBLOCK_DEPTH) { - CBlockHeaderAndShortTxIDs cmpctblock(block); - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); - } else - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::BLOCK, block)); + if (CanDirectFetch(consensusParams) && mi->second->nHeight >= chainActive.Height() - MAX_CMPCTBLOCK_DEPTH) { + if (a_recent_compact_block && a_recent_compact_block->header.GetHash() == mi->second->GetBlockHash()) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, *a_recent_compact_block)); + } else { + CBlockHeaderAndShortTxIDs cmpctblock(*pblock); + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); + } + } else { + connman.PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCK, *pblock)); + } } // Trigger the peer node to send a getblocks request for the next batch of inventory From d1a6022605bb49494ca6a4fc65b447d57bca6197 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 11 Oct 2017 10:49:43 +0200 Subject: [PATCH 03/11] Merge #11113: [net] Ignore getheaders requests for very old side blocks eff4bd8 [test] P2P functional test for certain fingerprinting protections (Jim Posen) a2be3b6 [net] Ignore getheaders requests for very old side blocks (Jim Posen) Pull request description: Sending a getheaders message with an empty locator and a stop hash is a request for a single header by hash. The node will respond with headers for blocks not in the main chain as well as those in the main chain. To avoid fingerprinting, the node should, however, ignore requests for headers on side branches that are too old. This replicates the logic that currently exists for `getdata` requests for blocks. Tree-SHA512: e04ef61e2b73945be6ec5977b3c5680b6dc3667246f8bfb67afae1ecaba900c0b49b18bbbb74869f7a37ef70b6ed99e78ebe0ea0a1569369fad9e447d720ffc4 --- qa/pull-tester/rpc-tests.py | 1 + qa/rpc-tests/p2p-fingerprint.py | 158 ++++++++++++++++++++++++ qa/rpc-tests/test_framework/mininode.py | 4 +- src/net_processing.cpp | 37 ++++-- 4 files changed, 189 insertions(+), 11 deletions(-) create mode 100755 qa/rpc-tests/p2p-fingerprint.py diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py index 411e3e9cbaac..fbaede37ab25 100755 --- a/qa/pull-tester/rpc-tests.py +++ b/qa/pull-tester/rpc-tests.py @@ -94,6 +94,7 @@ 'p2p-leaktests.py', 'p2p-compactblocks.py', 'sporks.py', + 'p2p-fingerprint.py', ] ZMQ_SCRIPTS = [ diff --git a/qa/rpc-tests/p2p-fingerprint.py b/qa/rpc-tests/p2p-fingerprint.py new file mode 100755 index 000000000000..fe60c6cd46dc --- /dev/null +++ b/qa/rpc-tests/p2p-fingerprint.py @@ -0,0 +1,158 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test various fingerprinting protections. + +If an stale block more than a month old or its header are requested by a peer, +the node should pretend that it does not have it to avoid fingerprinting. +""" + +import time + +from test_framework.blocktools import (create_block, create_coinbase) +from test_framework.mininode import ( + CInv, + NetworkThread, + NodeConn, + NodeConnCB, + msg_headers, + msg_block, + msg_getdata, + msg_getheaders, + wait_until, +) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + p2p_port, +) + +class P2PFingerprintTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + + # Build a chain of blocks on top of given one + def build_chain(self, nblocks, prev_hash, prev_height, prev_median_time): + blocks = [] + for _ in range(nblocks): + coinbase = create_coinbase(prev_height + 1) + block_time = prev_median_time + 1 + block = create_block(int(prev_hash, 16), coinbase, block_time) + block.solve() + + blocks.append(block) + prev_hash = block.hash + prev_height += 1 + prev_median_time = block_time + return blocks + + # Send a getdata request for a given block hash + def send_block_request(self, block_hash, node): + msg = msg_getdata() + msg.inv.append(CInv(2, block_hash)) # 2 == "Block" + node.send_message(msg) + + # Send a getheaders request for a given single block hash + def send_header_request(self, block_hash, node): + msg = msg_getheaders() + msg.hashstop = block_hash + node.send_message(msg) + + # Check whether last block received from node has a given hash + def last_block_equals(self, expected_hash, node): + block_msg = node.last_message.get("block") + return block_msg and block_msg.block.rehash() == expected_hash + + # Check whether last block header received from node has a given hash + def last_header_equals(self, expected_hash, node): + headers_msg = node.last_message.get("headers") + return (headers_msg and + headers_msg.headers and + headers_msg.headers[0].rehash() == expected_hash) + + # Checks that stale blocks timestamped more than a month ago are not served + # by the node while recent stale blocks and old active chain blocks are. + # This does not currently test that stale blocks timestamped within the + # last month but that have over a month's worth of work are also withheld. + def run_test(self): + node0 = NodeConnCB() + + connections = [] + connections.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], node0)) + node0.add_connection(connections[0]) + + NetworkThread().start() + node0.wait_for_verack() + + # Set node time to 60 days ago + self.nodes[0].setmocktime(int(time.time()) - 60 * 24 * 60 * 60) + + # Generating a chain of 10 blocks + block_hashes = self.nodes[0].generate(nblocks=10) + + # Create longer chain starting 2 blocks before current tip + height = len(block_hashes) - 2 + block_hash = block_hashes[height - 1] + block_time = self.nodes[0].getblockheader(block_hash)["mediantime"] + 1 + new_blocks = self.build_chain(5, block_hash, height, block_time) + + # Force reorg to a longer chain + node0.send_message(msg_headers(new_blocks)) + node0.wait_for_getdata() + for block in new_blocks: + node0.send_and_ping(msg_block(block)) + + # Check that reorg succeeded + assert_equal(self.nodes[0].getblockcount(), 13) + + stale_hash = int(block_hashes[-1], 16) + + # Check that getdata request for stale block succeeds + self.send_block_request(stale_hash, node0) + test_function = lambda: self.last_block_equals(stale_hash, node0) + wait_until(test_function, timeout=3) + + # Check that getheader request for stale block header succeeds + self.send_header_request(stale_hash, node0) + test_function = lambda: self.last_header_equals(stale_hash, node0) + wait_until(test_function, timeout=3) + + # Longest chain is extended so stale is much older than chain tip + self.nodes[0].setmocktime(0) + tip = self.nodes[0].generate(nblocks=1)[0] + assert_equal(self.nodes[0].getblockcount(), 14) + + # Send getdata & getheaders to refresh last received getheader message + block_hash = int(tip, 16) + self.send_block_request(block_hash, node0) + self.send_header_request(block_hash, node0) + node0.sync_with_ping() + + # Request for very old stale block should now fail + self.send_block_request(stale_hash, node0) + time.sleep(3) + assert not self.last_block_equals(stale_hash, node0) + + # Request for very old stale block header should now fail + self.send_header_request(stale_hash, node0) + time.sleep(3) + assert not self.last_header_equals(stale_hash, node0) + + # Verify we can fetch very old blocks and headers on the active chain + block_hash = int(block_hashes[2], 16) + self.send_block_request(block_hash, node0) + self.send_header_request(block_hash, node0) + node0.sync_with_ping() + + self.send_block_request(block_hash, node0) + test_function = lambda: self.last_block_equals(block_hash, node0) + wait_until(test_function, timeout=3) + + self.send_header_request(block_hash, node0) + test_function = lambda: self.last_header_equals(block_hash, node0) + wait_until(test_function, timeout=3) + +if __name__ == '__main__': + P2PFingerprintTest().main() diff --git a/qa/rpc-tests/test_framework/mininode.py b/qa/rpc-tests/test_framework/mininode.py index 8e5962345bbc..bccf56923398 100755 --- a/qa/rpc-tests/test_framework/mininode.py +++ b/qa/rpc-tests/test_framework/mininode.py @@ -1177,8 +1177,8 @@ def __repr__(self): class msg_headers(object): command = b"headers" - def __init__(self): - self.headers = [] + def __init__(self, headers=None): + self.headers = headers if headers is not None else [] def deserialize(self, f): # comment in dashd indicates these should be deserialized as blocks diff --git a/src/net_processing.cpp b/src/net_processing.cpp index fdd34c20d297..6cf1f1c9c115 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -86,6 +86,14 @@ static std::vector> vExtraTxnForCompact GUAR static const uint64_t RANDOMIZER_ID_ADDRESS_RELAY = 0x3cac0035b5866b90ULL; // SHA256("main address relay")[0:8] +/// Age after which a stale block will no longer be served if requested as +/// protection against fingerprinting. Set to one month, denominated in seconds. +static const int STALE_RELAY_AGE_LIMIT = 30 * 24 * 60 * 60; + +/// Age after which a block is considered historical for purposes of rate +/// limiting block relay. Set to one week, denominated in seconds. +static const int HISTORICAL_BLOCK_AGE = 7 * 24 * 60 * 60; + // Internal stuff namespace { /** Number of nodes with fSyncStarted. */ @@ -761,6 +769,17 @@ bool IsBanned(NodeId pnode) // blockchain -> download logic notification // +// To prevent fingerprinting attacks, only send blocks/headers outside of the +// active chain if they are no more than a month older (both in time, and in +// best equivalent proof of work) than the best header chain we know about. +static bool StaleBlockRequestAllowed(const CBlockIndex* pindex, const Consensus::Params& consensusParams) +{ + AssertLockHeld(cs_main); + return (pindexBestHeader != nullptr) && + (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() < STALE_RELAY_AGE_LIMIT) && + (GetBlockProofEquivalentTime(*pindexBestHeader, *pindex, *pindexBestHeader, consensusParams) < STALE_RELAY_AGE_LIMIT); +} + PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn) : connman(connmanIn) { // Initialize global variables that cannot be constructed at startup. recentRejects.reset(new CRollingBloomFilter(120000, 0.000001)); @@ -1068,13 +1087,8 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam if (chainActive.Contains(mi->second)) { send = true; } else { - static const int nOneMonth = 30 * 24 * 60 * 60; - // To prevent fingerprinting attacks, only send blocks outside of the active - // chain if they are valid, and no more than a month older (both in time, and in - // best equivalent proof of work) than the best header chain we know about. - send = mi->second->IsValid(BLOCK_VALID_SCRIPTS) && (pindexBestHeader != NULL) && - (pindexBestHeader->GetBlockTime() - mi->second->GetBlockTime() < nOneMonth) && - (GetBlockProofEquivalentTime(*pindexBestHeader, *mi->second, *pindexBestHeader, consensusParams) < nOneMonth); + send = mi->second->IsValid(BLOCK_VALID_SCRIPTS) && + StaleBlockRequestAllowed(mi->second, consensusParams); if (!send) { LogPrintf("%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId()); } @@ -1082,8 +1096,7 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam } // disconnect node in case we have reached the outbound limit for serving historical blocks // never disconnect whitelisted nodes - static const int nOneWeek = 7 * 24 * 60 * 60; // assume > 1 week = historical - if (send && connman.OutboundTargetReached(true) && ( ((pindexBestHeader != NULL) && (pindexBestHeader->GetBlockTime() - mi->second->GetBlockTime() > nOneWeek)) || inv.type == MSG_FILTERED_BLOCK) && !pfrom->fWhitelisted) + if (send && connman.OutboundTargetReached(true) && ( ((pindexBestHeader != NULL) && (pindexBestHeader->GetBlockTime() - mi->second->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK) && !pfrom->fWhitelisted) { LogPrint("net", "historical block serving limit reached, disconnect peer=%d\n", pfrom->GetId()); @@ -1972,6 +1985,12 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (mi == mapBlockIndex.end()) return true; pindex = (*mi).second; + + if (!chainActive.Contains(pindex) && + !StaleBlockRequestAllowed(pindex, chainparams.GetConsensus())) { + LogPrintf("%s: ignoring request from peer=%i for old block header that isn't in the main chain\n", __func__, pfrom->GetId()); + return true; + } } else { From 9344dee8aa6fef6de33643a96bdecc601e1ff1e7 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 9 Nov 2017 19:55:02 +0100 Subject: [PATCH 04/11] Merge #11580: Do not send (potentially) invalid headers in response to getheaders 725b79a [test] Verify node doesn't send headers that haven't been fully validated (Russell Yanofsky) 3788a84 Do not send (potentially) invalid headers in response to getheaders (Matt Corallo) Pull request description: Nowhere else in the protocol do we send headers which are for blocks we have not fully validated except in response to getheaders messages with a null locator. On my public node I have not seen any such request (whether for an invalid block or not) in at least two years of debug.log output, indicating that this should have minimal impact. Tree-SHA512: c1f6e0cdcdfb78ea577d555f9b3ceb1b4b60eff4f6cf313bfd8b576c9562d797bea73abc23f7011f249ae36dd539c715f3d20487ac03ace60e84e1b77c0c1e1a --- qa/rpc-tests/sendheaders.py | 34 ++++++++++++++++++++++++++++++++++ src/net_processing.cpp | 22 +++++++++------------- 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/qa/rpc-tests/sendheaders.py b/qa/rpc-tests/sendheaders.py index f03042de74a1..4e4977d5fb39 100755 --- a/qa/rpc-tests/sendheaders.py +++ b/qa/rpc-tests/sendheaders.py @@ -10,6 +10,17 @@ receive inv's (omitted from testing description below, this is our control). Second node is used for creating reorgs. +test_null_locators +================== + +Sends two getheaders requests with null locator values. First request's hashstop +value refers to validated block, while second request's hashstop value refers to +a block which hasn't been validated. Verifies only the first request returns +headers. + +test_nonnull_locators +===================== + Part 1: No headers announcements before "sendheaders" a. node mines a block [expect: inv] send getdata for the block [expect: block] @@ -279,6 +290,29 @@ def run_test(self): inv_node.wait_for_verack() test_node.wait_for_verack() + self.test_null_locators(test_node) + self.test_nonnull_locators(test_node, inv_node) + + def test_null_locators(self, test_node): + tip = self.nodes[0].getblockheader(self.nodes[0].generate(1)[0]) + tip_hash = int(tip["hash"], 16) + + self.log.info("Verify getheaders with null locator and valid hashstop returns headers.") + test_node.clear_last_announcement() + test_node.get_headers(locator=[], hashstop=tip_hash) + assert_equal(test_node.check_last_announcement(headers=[tip_hash]), True) + + self.log.info("Verify getheaders with null locator and invalid hashstop does not return headers.") + block = create_block(int(tip["hash"], 16), create_coinbase(tip["height"] + 1), tip["mediantime"] + 1) + block.solve() + test_node.send_header_for_blocks([block]) + test_node.clear_last_announcement() + test_node.get_headers(locator=[], hashstop=int(block.hash, 16)) + test_node.sync_with_ping() + assert_equal(test_node.block_announced, False) + test_node.send_message(msg_block(block)) + + def test_nonnull_locators(self, test_node, inv_node): tip = int(self.nodes[0].getbestblockhash(), 16) # PART 1 diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6cf1f1c9c115..9395d7c47b0c 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -771,11 +771,13 @@ bool IsBanned(NodeId pnode) // To prevent fingerprinting attacks, only send blocks/headers outside of the // active chain if they are no more than a month older (both in time, and in -// best equivalent proof of work) than the best header chain we know about. -static bool StaleBlockRequestAllowed(const CBlockIndex* pindex, const Consensus::Params& consensusParams) +// best equivalent proof of work) than the best header chain we know about and +// we fully-validated them at some point. +static bool BlockRequestAllowed(const CBlockIndex* pindex, const Consensus::Params& consensusParams) { AssertLockHeld(cs_main); - return (pindexBestHeader != nullptr) && + if (chainActive.Contains(pindex)) return true; + return pindex->IsValid(BLOCK_VALID_SCRIPTS) && (pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() < STALE_RELAY_AGE_LIMIT) && (GetBlockProofEquivalentTime(*pindexBestHeader, *pindex, *pindexBestHeader, consensusParams) < STALE_RELAY_AGE_LIMIT); } @@ -1084,14 +1086,9 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam CValidationState dummy; ActivateBestChain(dummy, Params(), a_recent_block); } - if (chainActive.Contains(mi->second)) { - send = true; - } else { - send = mi->second->IsValid(BLOCK_VALID_SCRIPTS) && - StaleBlockRequestAllowed(mi->second, consensusParams); - if (!send) { - LogPrintf("%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId()); - } + send = BlockRequestAllowed(mi->second, consensusParams); + if (!send) { + LogPrintf("%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId()); } } // disconnect node in case we have reached the outbound limit for serving historical blocks @@ -1986,8 +1983,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr return true; pindex = (*mi).second; - if (!chainActive.Contains(pindex) && - !StaleBlockRequestAllowed(pindex, chainparams.GetConsensus())) { + if (!BlockRequestAllowed(pindex, chainparams.GetConsensus())) { LogPrintf("%s: ignoring request from peer=%i for old block header that isn't in the main chain\n", __func__, pfrom->GetId()); return true; } From 6085de378194bd338ddcd4f4bf6a27f942bdde3c Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 17 Jan 2017 17:42:46 -0500 Subject: [PATCH 05/11] Add ability to assert a lock is not held in DEBUG_LOCKORDER --- src/sync.cpp | 10 ++++++++++ src/sync.h | 3 +++ 2 files changed, 13 insertions(+) diff --git a/src/sync.cpp b/src/sync.cpp index ab4865658daf..3995d12a8d7f 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -162,6 +162,16 @@ void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, abort(); } +void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) +{ + for (const std::pair& i : *lockstack) { + if (i.first == cs) { + fprintf(stderr, "Assertion failed: lock %s held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld().c_str()); + abort(); + } + } +} + void DeleteLock(void* cs) { if (!lockdata.available) { diff --git a/src/sync.h b/src/sync.h index 467b5224d779..ce14d6410029 100644 --- a/src/sync.h +++ b/src/sync.h @@ -76,14 +76,17 @@ void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs void LeaveCritical(); std::string LocksHeld(); void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs); +void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs); void DeleteLock(void* cs); #else void static inline EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {} void static inline LeaveCritical() {} void static inline AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {} +void static inline AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {} void static inline DeleteLock(void* cs) {} #endif #define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs) +#define AssertLockNotHeld(cs) AssertLockNotHeldInternal(#cs, __FILE__, __LINE__, &cs) /** * Wrapped boost mutex: supports recursive locking, but no waiting From 7f54372bb6f2b284ce4409a7a39a73ec67a395ef Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 4 Dec 2017 17:10:45 -0500 Subject: [PATCH 06/11] Create new mutex for orphans, no cs_main in PLV::BlockConnected This should (marginally) speed up validationinterface queue draining by avoiding a cs_main lock in one client. --- src/net_processing.cpp | 41 ++++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 9395d7c47b0c..51a4afd06b89 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -77,12 +77,13 @@ struct COrphanTx { NodeId fromPeer; int64_t nTimeExpire; }; -std::map mapOrphanTransactions GUARDED_BY(cs_main); -std::map::iterator, IteratorComparator>> mapOrphanTransactionsByPrev GUARDED_BY(cs_main); -void EraseOrphansFor(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main); +static CCriticalSection g_cs_orphans; +std::map mapOrphanTransactions GUARDED_BY(g_cs_orphans); +std::map::iterator, IteratorComparator>> mapOrphanTransactionsByPrev GUARDED_BY(g_cs_orphans); +void EraseOrphansFor(NodeId peer); -static size_t vExtraTxnForCompactIt = 0; -static std::vector> vExtraTxnForCompact GUARDED_BY(cs_main); +static size_t vExtraTxnForCompactIt GUARDED_BY(g_cs_orphans) = 0; +static std::vector> vExtraTxnForCompact GUARDED_BY(g_cs_orphans); static const uint64_t RANDOMIZER_ID_ADDRESS_RELAY = 0x3cac0035b5866b90ULL; // SHA256("main address relay")[0:8] @@ -149,6 +150,13 @@ namespace { /** Number of peers from which we're downloading blocks. */ int nPeersWithValidatedDownloads = 0; + /* This comment is here to force a merge conflict when bitcoin#11560 is backported + * It introduces this member as int64_t while bitcoin#11824 changes it to atomic. + * bitcoin#11824 is partially backported already, which means you'll have to take the atomic + * version when you encounter this merge conflict! + std::atomic g_last_tip_update(0); + */ + /** Relay map, protected by cs_main. */ typedef std::map MapRelay; MapRelay mapRelay; @@ -613,7 +621,7 @@ void UnregisterNodeSignals(CNodeSignals& nodeSignals) // mapOrphanTransactions // -void AddToCompactExtraTransactions(const CTransactionRef& tx) +void AddToCompactExtraTransactions(const CTransactionRef& tx) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans) { size_t max_extra_txn = GetArg("-blockreconstructionextratxn", DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN); if (max_extra_txn <= 0) @@ -624,7 +632,7 @@ void AddToCompactExtraTransactions(const CTransactionRef& tx) vExtraTxnForCompactIt = (vExtraTxnForCompactIt + 1) % max_extra_txn; } -bool AddOrphanTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +bool AddOrphanTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans) { const uint256& hash = tx->GetHash(); if (mapOrphanTransactions.count(hash)) @@ -657,7 +665,7 @@ bool AddOrphanTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRE return true; } -int static EraseOrphanTx(uint256 hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +int static EraseOrphanTx(uint256 hash) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans) { std::map::iterator it = mapOrphanTransactions.find(hash); if (it == mapOrphanTransactions.end()) @@ -677,6 +685,7 @@ int static EraseOrphanTx(uint256 hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) void EraseOrphansFor(NodeId peer) { + LOCK(g_cs_orphans); int nErased = 0; std::map::iterator iter = mapOrphanTransactions.begin(); while (iter != mapOrphanTransactions.end()) @@ -691,8 +700,10 @@ void EraseOrphansFor(NodeId peer) } -unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) { + LOCK(g_cs_orphans); + unsigned int nEvicted = 0; static int64_t nNextSweep; int64_t nNow = GetTime(); @@ -791,7 +802,7 @@ void PeerLogicValidation::SyncTransaction(const CTransaction& tx, const CBlockIn if (nPosInBlock == CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK) return; - LOCK(cs_main); + LOCK(g_cs_orphans); std::vector vOrphanErase; // Which orphan pool entries must we evict? @@ -947,9 +958,13 @@ bool static AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main) recentRejects->reset(); } + { + LOCK(g_cs_orphans); + if (mapOrphanTransactions.count(inv.hash)) return true; + } + return recentRejects->contains(inv.hash) || mempool.exists(inv.hash) || - mapOrphanTransactions.count(inv.hash) || pcoinsTip->HaveCoinInCache(COutPoint(inv.hash, 0)) || // Best effort: only try output 0 and 1 pcoinsTip->HaveCoinInCache(COutPoint(inv.hash, 1)); } @@ -2111,7 +2126,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr mmetaman.DisallowMixing(dmn->proTxHash); } - LOCK(cs_main); + LOCK2(cs_main, g_cs_orphans); bool fMissingInputs = false; CValidationState state; @@ -2338,7 +2353,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr bool fBlockReconstructed = false; { - LOCK(cs_main); + LOCK2(cs_main, g_cs_orphans); // If AcceptBlockHeader returned true, it set pindex assert(pindex); UpdateBlockAvailability(pfrom->GetId(), pindex->GetBlockHash()); From f69c4370d0a43f798af0c7b3c4c5e4b1929d92a3 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 24 Dec 2017 11:58:20 -0500 Subject: [PATCH 07/11] Refactor ProcessGetData in anticipation of avoiding cs_main for ABC --- src/net_processing.cpp | 513 +++++++++++++++++++++-------------------- 1 file changed, 263 insertions(+), 250 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 51a4afd06b89..a9d6fab39076 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1058,306 +1058,319 @@ static void RelayAddress(const CAddress& addr, bool fReachable, CConnman& connma connman.ForEachNodeThen(std::move(sortfunc), std::move(pushfunc)); } -void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParams, CConnman& connman, const std::atomic& interruptMsgProc) +void static ProcessGetBlockData(CNode* pfrom, const Consensus::Params& consensusParams, const CInv& inv, CConnman& connman, const std::atomic& interruptMsgProc) { - std::deque::iterator it = pfrom->vRecvGetData.begin(); - std::vector vNotFound; - const CNetMsgMaker msgMaker(pfrom->GetSendVersion()); LOCK(cs_main); - while (it != pfrom->vRecvGetData.end()) { - // Don't bother if send buffer is too full to respond anyway - if (pfrom->fPauseSend) - break; + bool send = false; + std::shared_ptr a_recent_block; + std::shared_ptr a_recent_compact_block; + { + LOCK(cs_most_recent_block); + a_recent_block = most_recent_block; + a_recent_compact_block = most_recent_compact_block; + } - const CInv &inv = *it; - LogPrint("net", "ProcessGetData -- inv = %s\n", inv.ToString()); + { + BlockMap::iterator mi = mapBlockIndex.find(inv.hash); + if (mi != mapBlockIndex.end()) { - if (interruptMsgProc) - return; - - it++; + if (mi->second->nChainTx && !mi->second->IsValid(BLOCK_VALID_SCRIPTS) && + mi->second->IsValid(BLOCK_VALID_TREE)) { + // If we have the block and all of its parents, but have not yet validated it, + // we might be in the middle of connecting it (ie in the unlock of cs_main + // before ActivateBestChain but after AcceptBlock). + // In this case, we need to run ActivateBestChain prior to checking the relay + // conditions below. + CValidationState dummy; + ActivateBestChain(dummy, Params(), a_recent_block); + } + } + } + BlockMap::iterator mi = mapBlockIndex.find(inv.hash); + if (mi != mapBlockIndex.end()) { + send = BlockRequestAllowed(mi->second, consensusParams); + if (!send) { + LogPrintf("%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId()); + } + } + const CNetMsgMaker msgMaker(pfrom->GetSendVersion()); + // disconnect node in case we have reached the outbound limit for serving historical blocks + // never disconnect whitelisted nodes + if (send && connman.OutboundTargetReached(true) && ( ((pindexBestHeader != NULL) && (pindexBestHeader->GetBlockTime() - mi->second->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK) && !pfrom->fWhitelisted) + { + LogPrint("net", "historical block serving limit reached, disconnect peer=%d\n", pfrom->GetId()); - if (inv.type == MSG_BLOCK || inv.type == MSG_FILTERED_BLOCK || inv.type == MSG_CMPCT_BLOCK) + //disconnect node + pfrom->fDisconnect = true; + send = false; + } + // Pruned nodes may have deleted the block, so check whether + // it's available before trying to send. + if (send && (mi->second->nStatus & BLOCK_HAVE_DATA)) + { + std::shared_ptr pblock; + if (a_recent_block && a_recent_block->GetHash() == (*mi).second->GetBlockHash()) { + pblock = a_recent_block; + } else { + // Send block from disk + std::shared_ptr pblockRead = std::make_shared(); + if (!ReadBlockFromDisk(*pblockRead, (*mi).second, consensusParams)) + assert(!"cannot load block from disk"); + pblock = pblockRead; + } + if (inv.type == MSG_BLOCK) + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::BLOCK, *pblock)); + else if (inv.type == MSG_FILTERED_BLOCK) + { + bool sendMerkleBlock = false; + CMerkleBlock merkleBlock; { - bool send = false; - BlockMap::iterator mi = mapBlockIndex.find(inv.hash); - std::shared_ptr a_recent_block; - std::shared_ptr a_recent_compact_block; - { - LOCK(cs_most_recent_block); - a_recent_block = most_recent_block; - a_recent_compact_block = most_recent_compact_block; + LOCK(pfrom->cs_filter); + if (pfrom->pfilter) { + sendMerkleBlock = true; + merkleBlock = CMerkleBlock(*pblock, *pfrom->pfilter); } - if (mi != mapBlockIndex.end()) - { - if (mi->second->nChainTx && !mi->second->IsValid(BLOCK_VALID_SCRIPTS) && - mi->second->IsValid(BLOCK_VALID_TREE)) { - // If we have the block and all of its parents, but have not yet validated it, - // we might be in the middle of connecting it (ie in the unlock of cs_main - // before ActivateBestChain but after AcceptBlock). - // In this case, we need to run ActivateBestChain prior to checking the relay - // conditions below. - CValidationState dummy; - ActivateBestChain(dummy, Params(), a_recent_block); - } - send = BlockRequestAllowed(mi->second, consensusParams); - if (!send) { - LogPrintf("%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId()); - } + } + if (sendMerkleBlock) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MERKLEBLOCK, merkleBlock)); + // CMerkleBlock just contains hashes, so also push any transactions in the block the client did not see + // This avoids hurting performance by pointlessly requiring a round-trip + // Note that there is currently no way for a node to request any single transactions we didn't send here - + // they must either disconnect and retry or request the full block. + // Thus, the protocol spec specified allows for us to provide duplicate txn here, + // however we MUST always provide at least what the remote peer needs + typedef std::pair PairType; + BOOST_FOREACH(PairType& pair, merkleBlock.vMatchedTxn) + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, *pblock->vtx[pair.first])); + } + // else + // no response + } + else if (inv.type == MSG_CMPCT_BLOCK) + { + // If a peer is asking for old blocks, we're almost guaranteed + // they won't have a useful mempool to match against a compact block, + // and we don't feel like constructing the object for them, so + // instead we respond with the full, non-compact block. + if (CanDirectFetch(consensusParams) && mi->second->nHeight >= chainActive.Height() - MAX_CMPCTBLOCK_DEPTH) { + if (a_recent_compact_block && a_recent_compact_block->header.GetHash() == mi->second->GetBlockHash()) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, *a_recent_compact_block)); + } else { + CBlockHeaderAndShortTxIDs cmpctblock(*pblock); + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); } - // disconnect node in case we have reached the outbound limit for serving historical blocks - // never disconnect whitelisted nodes - if (send && connman.OutboundTargetReached(true) && ( ((pindexBestHeader != NULL) && (pindexBestHeader->GetBlockTime() - mi->second->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK) && !pfrom->fWhitelisted) - { - LogPrint("net", "historical block serving limit reached, disconnect peer=%d\n", pfrom->GetId()); + } else { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::BLOCK, *pblock)); + } + } - //disconnect node - pfrom->fDisconnect = true; - send = false; - } - // Pruned nodes may have deleted the block, so check whether - // it's available before trying to send. - if (send && (mi->second->nStatus & BLOCK_HAVE_DATA)) - { - std::shared_ptr pblock; - if (a_recent_block && a_recent_block->GetHash() == (*mi).second->GetBlockHash()) { - pblock = a_recent_block; - } else { - // Send block from disk - std::shared_ptr pblockRead = std::make_shared(); - if (!ReadBlockFromDisk(*pblockRead, (*mi).second, consensusParams)) - assert(!"cannot load block from disk"); - pblock = pblockRead; - } - if (inv.type == MSG_BLOCK) - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::BLOCK, *pblock)); - else if (inv.type == MSG_FILTERED_BLOCK) - { - bool sendMerkleBlock = false; - CMerkleBlock merkleBlock; - { - LOCK(pfrom->cs_filter); - if (pfrom->pfilter) { - sendMerkleBlock = true; - merkleBlock = CMerkleBlock(*pblock, *pfrom->pfilter); - } - } - if (sendMerkleBlock) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MERKLEBLOCK, merkleBlock)); - // CMerkleBlock just contains hashes, so also push any transactions in the block the client did not see - // This avoids hurting performance by pointlessly requiring a round-trip - // Note that there is currently no way for a node to request any single transactions we didn't send here - - // they must either disconnect and retry or request the full block. - // Thus, the protocol spec specified allows for us to provide duplicate txn here, - // however we MUST always provide at least what the remote peer needs - typedef std::pair PairType; - BOOST_FOREACH(PairType& pair, merkleBlock.vMatchedTxn) - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, *pblock->vtx[pair.first])); - } - // else - // no response - } - else if (inv.type == MSG_CMPCT_BLOCK) - { - // If a peer is asking for old blocks, we're almost guaranteed - // they won't have a useful mempool to match against a compact block, - // and we don't feel like constructing the object for them, so - // instead we respond with the full, non-compact block. - if (CanDirectFetch(consensusParams) && mi->second->nHeight >= chainActive.Height() - MAX_CMPCTBLOCK_DEPTH) { - if (a_recent_compact_block && a_recent_compact_block->header.GetHash() == mi->second->GetBlockHash()) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, *a_recent_compact_block)); - } else { - CBlockHeaderAndShortTxIDs cmpctblock(*pblock); - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); - } - } else { - connman.PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCK, *pblock)); - } - } + // Trigger the peer node to send a getblocks request for the next batch of inventory + if (inv.hash == pfrom->hashContinue) + { + // Bypass PushInventory, this must send even if redundant, + // and we want it right after the last block so they don't + // wait for other stuff first. + std::vector vInv; + vInv.push_back(CInv(MSG_BLOCK, chainActive.Tip()->GetBlockHash())); + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::INV, vInv)); + pfrom->hashContinue.SetNull(); + } + } +} - // Trigger the peer node to send a getblocks request for the next batch of inventory - if (inv.hash == pfrom->hashContinue) - { - // Bypass PushInventory, this must send even if redundant, - // and we want it right after the last block so they don't - // wait for other stuff first. - std::vector vInv; - vInv.push_back(CInv(MSG_BLOCK, chainActive.Tip()->GetBlockHash())); - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::INV, vInv)); - pfrom->hashContinue.SetNull(); - } - } +void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParams, CConnman& connman, const std::atomic& interruptMsgProc) +{ + std::deque::iterator it = pfrom->vRecvGetData.begin(); + std::vector vNotFound; + const CNetMsgMaker msgMaker(pfrom->GetSendVersion()); + { + LOCK(cs_main); + + while (it != pfrom->vRecvGetData.end() && it->IsKnownType()) { + if (interruptMsgProc) + return; + // Don't bother if send buffer is too full to respond anyway + if (pfrom->fPauseSend) + break; + + const CInv &inv = *it; + if (inv.type == MSG_BLOCK || inv.type == MSG_FILTERED_BLOCK || inv.type == MSG_CMPCT_BLOCK) { + break; } - else if (inv.IsKnownType()) - { - // Send stream from relay memory - bool push = false; - // Only serve MSG_TX from mapRelay. - // Otherwise we may send out a normal TX instead of a IX - if (inv.type == MSG_TX) { - auto mi = mapRelay.find(inv.hash); - if (mi != mapRelay.end()) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, *mi->second)); + it++; + + // Send stream from relay memory + bool push = false; + // Only serve MSG_TX from mapRelay. + // Otherwise we may send out a normal TX instead of a IX + if (inv.type == MSG_TX) { + auto mi = mapRelay.find(inv.hash); + if (mi != mapRelay.end()) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, *mi->second)); + push = true; + } else if (pfrom->timeLastMempoolReq) { + auto txinfo = mempool.info(inv.hash); + // To protect privacy, do not answer getdata using the mempool when + // that TX couldn't have been INVed in reply to a MEMPOOL request. + if (txinfo.tx && txinfo.nTime <= pfrom->timeLastMempoolReq) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, *txinfo.tx)); push = true; - } else if (pfrom->timeLastMempoolReq) { - auto txinfo = mempool.info(inv.hash); - // To protect privacy, do not answer getdata using the mempool when - // that TX couldn't have been INVed in reply to a MEMPOOL request. - if (txinfo.tx && txinfo.nTime <= pfrom->timeLastMempoolReq) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, *txinfo.tx)); - push = true; - } } } + } - if (!push && inv.type == MSG_TXLOCK_REQUEST) { - CTxLockRequest txLockRequest; - if(instantsend.GetTxLockRequest(inv.hash, txLockRequest)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TXLOCKREQUEST, txLockRequest)); - push = true; - } + if (!push && inv.type == MSG_TXLOCK_REQUEST) { + CTxLockRequest txLockRequest; + if(instantsend.GetTxLockRequest(inv.hash, txLockRequest)) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TXLOCKREQUEST, txLockRequest)); + push = true; } + } - if (!push && inv.type == MSG_TXLOCK_VOTE) { - CTxLockVote vote; - if(instantsend.GetTxLockVote(inv.hash, vote)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TXLOCKVOTE, vote)); - push = true; - } + if (!push && inv.type == MSG_TXLOCK_VOTE) { + CTxLockVote vote; + if(instantsend.GetTxLockVote(inv.hash, vote)) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TXLOCKVOTE, vote)); + push = true; } + } - if (!push && inv.type == MSG_SPORK) { - CSporkMessage spork; - if(sporkManager.GetSporkByHash(inv.hash, spork)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::SPORK, spork)); - push = true; - } + if (!push && inv.type == MSG_SPORK) { + CSporkMessage spork; + if(sporkManager.GetSporkByHash(inv.hash, spork)) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::SPORK, spork)); + push = true; } + } - if (!push && inv.type == MSG_DSTX) { - CPrivateSendBroadcastTx dstx = CPrivateSend::GetDSTX(inv.hash); - if(dstx) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::DSTX, dstx)); - push = true; - } + if (!push && inv.type == MSG_DSTX) { + CPrivateSendBroadcastTx dstx = CPrivateSend::GetDSTX(inv.hash); + if(dstx) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::DSTX, dstx)); + push = true; } + } - if (!push && inv.type == MSG_GOVERNANCE_OBJECT) { - LogPrint("net", "ProcessGetData -- MSG_GOVERNANCE_OBJECT: inv = %s\n", inv.ToString()); - CDataStream ss(SER_NETWORK, pfrom->GetSendVersion()); - bool topush = false; - { - if(governance.HaveObjectForHash(inv.hash)) { - ss.reserve(1000); - if(governance.SerializeObjectForHash(inv.hash, ss)) { - topush = true; - } + if (!push && inv.type == MSG_GOVERNANCE_OBJECT) { + LogPrint("net", "ProcessGetData -- MSG_GOVERNANCE_OBJECT: inv = %s\n", inv.ToString()); + CDataStream ss(SER_NETWORK, pfrom->GetSendVersion()); + bool topush = false; + { + if(governance.HaveObjectForHash(inv.hash)) { + ss.reserve(1000); + if(governance.SerializeObjectForHash(inv.hash, ss)) { + topush = true; } } - LogPrint("net", "ProcessGetData -- MSG_GOVERNANCE_OBJECT: topush = %d, inv = %s\n", topush, inv.ToString()); - if(topush) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNGOVERNANCEOBJECT, ss)); - push = true; - } } + LogPrint("net", "ProcessGetData -- MSG_GOVERNANCE_OBJECT: topush = %d, inv = %s\n", topush, inv.ToString()); + if(topush) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNGOVERNANCEOBJECT, ss)); + push = true; + } + } - if (!push && inv.type == MSG_GOVERNANCE_OBJECT_VOTE) { - CDataStream ss(SER_NETWORK, pfrom->GetSendVersion()); - bool topush = false; - { - if(governance.HaveVoteForHash(inv.hash)) { - ss.reserve(1000); - if(governance.SerializeVoteForHash(inv.hash, ss)) { - topush = true; - } + if (!push && inv.type == MSG_GOVERNANCE_OBJECT_VOTE) { + CDataStream ss(SER_NETWORK, pfrom->GetSendVersion()); + bool topush = false; + { + if(governance.HaveVoteForHash(inv.hash)) { + ss.reserve(1000); + if(governance.SerializeVoteForHash(inv.hash, ss)) { + topush = true; } } - if(topush) { - LogPrint("net", "ProcessGetData -- pushing: inv = %s\n", inv.ToString()); - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNGOVERNANCEOBJECTVOTE, ss)); - push = true; - } } + if(topush) { + LogPrint("net", "ProcessGetData -- pushing: inv = %s\n", inv.ToString()); + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNGOVERNANCEOBJECTVOTE, ss)); + push = true; + } + } - if (!push && (inv.type == MSG_QUORUM_FINAL_COMMITMENT)) { - llmq::CFinalCommitment o; - if (llmq::quorumBlockProcessor->GetMinableCommitmentByHash(inv.hash, o)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::QFCOMMITMENT, o)); - push = true; - } + if (!push && (inv.type == MSG_QUORUM_FINAL_COMMITMENT)) { + llmq::CFinalCommitment o; + if (llmq::quorumBlockProcessor->GetMinableCommitmentByHash(inv.hash, o)) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::QFCOMMITMENT, o)); + push = true; } + } - if (!push && (inv.type == MSG_QUORUM_CONTRIB)) { - llmq::CDKGContribution o; - if (llmq::quorumDKGSessionManager->GetContribution(inv.hash, o)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::QCONTRIB, o)); - push = true; - } + if (!push && (inv.type == MSG_QUORUM_CONTRIB)) { + llmq::CDKGContribution o; + if (llmq::quorumDKGSessionManager->GetContribution(inv.hash, o)) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::QCONTRIB, o)); + push = true; } - if (!push && (inv.type == MSG_QUORUM_COMPLAINT)) { - llmq::CDKGComplaint o; - if (llmq::quorumDKGSessionManager->GetComplaint(inv.hash, o)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::QCOMPLAINT, o)); - push = true; - } + } + if (!push && (inv.type == MSG_QUORUM_COMPLAINT)) { + llmq::CDKGComplaint o; + if (llmq::quorumDKGSessionManager->GetComplaint(inv.hash, o)) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::QCOMPLAINT, o)); + push = true; } - if (!push && (inv.type == MSG_QUORUM_JUSTIFICATION)) { - llmq::CDKGJustification o; - if (llmq::quorumDKGSessionManager->GetJustification(inv.hash, o)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::QJUSTIFICATION, o)); - push = true; - } + } + if (!push && (inv.type == MSG_QUORUM_JUSTIFICATION)) { + llmq::CDKGJustification o; + if (llmq::quorumDKGSessionManager->GetJustification(inv.hash, o)) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::QJUSTIFICATION, o)); + push = true; } - if (!push && (inv.type == MSG_QUORUM_PREMATURE_COMMITMENT)) { - llmq::CDKGPrematureCommitment o; - if (llmq::quorumDKGSessionManager->GetPrematureCommitment(inv.hash, o)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::QPCOMMITMENT, o)); - push = true; - } + } + if (!push && (inv.type == MSG_QUORUM_PREMATURE_COMMITMENT)) { + llmq::CDKGPrematureCommitment o; + if (llmq::quorumDKGSessionManager->GetPrematureCommitment(inv.hash, o)) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::QPCOMMITMENT, o)); + push = true; } - if (!push && (inv.type == MSG_QUORUM_DEBUG_STATUS)) { - llmq::CDKGDebugStatus o; - if (llmq::quorumDKGDebugManager->GetDebugStatus(inv.hash, o)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::QDEBUGSTATUS, o)); - push = true; - } + } + if (!push && (inv.type == MSG_QUORUM_DEBUG_STATUS)) { + llmq::CDKGDebugStatus o; + if (llmq::quorumDKGDebugManager->GetDebugStatus(inv.hash, o)) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::QDEBUGSTATUS, o)); + push = true; } - if (!push && (inv.type == MSG_QUORUM_RECOVERED_SIG)) { - llmq::CRecoveredSig o; - if (llmq::quorumSigningManager->GetRecoveredSigForGetData(inv.hash, o)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::QSIGREC, o)); - push = true; - } + } + if (!push && (inv.type == MSG_QUORUM_RECOVERED_SIG)) { + llmq::CRecoveredSig o; + if (llmq::quorumSigningManager->GetRecoveredSigForGetData(inv.hash, o)) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::QSIGREC, o)); + push = true; } + } - if (!push && (inv.type == MSG_CLSIG)) { - llmq::CChainLockSig o; - if (llmq::chainLocksHandler->GetChainLockByHash(inv.hash, o)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::CLSIG, o)); - push = true; - } + if (!push && (inv.type == MSG_CLSIG)) { + llmq::CChainLockSig o; + if (llmq::chainLocksHandler->GetChainLockByHash(inv.hash, o)) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::CLSIG, o)); + push = true; } + } - if (!push && (inv.type == MSG_ISLOCK)) { - llmq::CInstantSendLock o; - if (llmq::quorumInstantSendManager->GetInstantSendLockByHash(inv.hash, o)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::ISLOCK, o)); - push = true; - } + if (!push && (inv.type == MSG_ISLOCK)) { + llmq::CInstantSendLock o; + if (llmq::quorumInstantSendManager->GetInstantSendLockByHash(inv.hash, o)) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::ISLOCK, o)); + push = true; } - - if (!push) - vNotFound.push_back(inv); } + if (!push) + vNotFound.push_back(inv); + // Track requests for our stuff. GetMainSignals().Inventory(inv.hash); + } - if (inv.type == MSG_BLOCK || inv.type == MSG_FILTERED_BLOCK || inv.type == MSG_CMPCT_BLOCK) - break; + if (it != pfrom->vRecvGetData.end()) { + const CInv &inv = *it; + it++; + if (inv.type == MSG_BLOCK || inv.type == MSG_FILTERED_BLOCK || inv.type == MSG_CMPCT_BLOCK) { + ProcessGetBlockData(pfrom, consensusParams, inv, connman, interruptMsgProc); + } } - } + } // release cs_main pfrom->vRecvGetData.erase(pfrom->vRecvGetData.begin(), it); From 2eb55317476ce5325da0b1ad847a4571f6f6990a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 24 Dec 2017 11:59:02 -0500 Subject: [PATCH 08/11] Avoid cs_main in net_processing ActivateBestChain calls --- src/net_processing.cpp | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index a9d6fab39076..20850a16067d 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1060,8 +1060,6 @@ static void RelayAddress(const CAddress& addr, bool fReachable, CConnman& connma void static ProcessGetBlockData(CNode* pfrom, const Consensus::Params& consensusParams, const CInv& inv, CConnman& connman, const std::atomic& interruptMsgProc) { - LOCK(cs_main); - bool send = false; std::shared_ptr a_recent_block; std::shared_ptr a_recent_compact_block; @@ -1071,7 +1069,9 @@ void static ProcessGetBlockData(CNode* pfrom, const Consensus::Params& consensus a_recent_compact_block = most_recent_compact_block; } + bool need_activate_chain = false; { + LOCK(cs_main); BlockMap::iterator mi = mapBlockIndex.find(inv.hash); if (mi != mapBlockIndex.end()) { @@ -1082,11 +1082,16 @@ void static ProcessGetBlockData(CNode* pfrom, const Consensus::Params& consensus // before ActivateBestChain but after AcceptBlock). // In this case, we need to run ActivateBestChain prior to checking the relay // conditions below. - CValidationState dummy; - ActivateBestChain(dummy, Params(), a_recent_block); + need_activate_chain = true; } } + } // release cs_main before calling ActivateBestChain + if (need_activate_chain) { + CValidationState dummy; + ActivateBestChain(dummy, Params(), a_recent_block); } + + LOCK(cs_main); BlockMap::iterator mi = mapBlockIndex.find(inv.hash); if (mi != mapBlockIndex.end()) { send = BlockRequestAllowed(mi->second, consensusParams); @@ -1181,6 +1186,8 @@ void static ProcessGetBlockData(CNode* pfrom, const Consensus::Params& consensus void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParams, CConnman& connman, const std::atomic& interruptMsgProc) { + AssertLockNotHeld(cs_main); + std::deque::iterator it = pfrom->vRecvGetData.begin(); std::vector vNotFound; const CNetMsgMaker msgMaker(pfrom->GetSendVersion()); @@ -1362,15 +1369,15 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam // Track requests for our stuff. GetMainSignals().Inventory(inv.hash); } + } // release cs_main - if (it != pfrom->vRecvGetData.end()) { - const CInv &inv = *it; - it++; - if (inv.type == MSG_BLOCK || inv.type == MSG_FILTERED_BLOCK || inv.type == MSG_CMPCT_BLOCK) { - ProcessGetBlockData(pfrom, consensusParams, inv, connman, interruptMsgProc); - } + if (it != pfrom->vRecvGetData.end()) { + const CInv &inv = *it; + it++; + if (inv.type == MSG_BLOCK || inv.type == MSG_FILTERED_BLOCK || inv.type == MSG_CMPCT_BLOCK) { + ProcessGetBlockData(pfrom, consensusParams, inv, connman, interruptMsgProc); } - } // release cs_main + } pfrom->vRecvGetData.erase(pfrom->vRecvGetData.begin(), it); @@ -1977,7 +1984,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr inv.type = MSG_BLOCK; inv.hash = req.blockhash; pfrom->vRecvGetData.push_back(inv); - ProcessGetData(pfrom, chainparams.GetConsensus(), connman, interruptMsgProc); + // The message processing loop will go around again (without pausing) and we'll respond then (without cs_main) return true; } From 95192d5b56737af0641f889878070e5c9cec3362 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 4 Dec 2017 18:25:57 -0500 Subject: [PATCH 09/11] Require no cs_main lock for ProcessNewBlock/ActivateBestChain This requires the removal of some very liberal (incorrect) cs_mains sprinkled in some tests. It adds some chainActive.Tip() races, but the tests are all single-threaded anyway. --- src/test/miner_tests.cpp | 47 +++++++++++++++++--------------- src/validation.cpp | 3 ++ src/wallet/test/wallet_tests.cpp | 8 +++--- 3 files changed, 32 insertions(+), 26 deletions(-) diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 861f90c51794..c58986b4369d 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -210,7 +210,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) entry.dPriority = 111.0; entry.nHeight = 11; - LOCK(cs_main); fCheckpointsEnabled = false; // Simple block creation, nothing special yet: @@ -224,28 +223,30 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) auto createAndProcessEmptyBlock = [&]() { int i = chainActive.Height(); CBlock *pblock = &pemptyblocktemplate->block; // pointer for convenience - pblock->nVersion = 2; - pblock->nTime = chainActive.Tip()->GetMedianTimePast()+1; - CMutableTransaction txCoinbase(*pblock->vtx[0]); - txCoinbase.nVersion = 1; - txCoinbase.vin[0].scriptSig = CScript() << (chainActive.Height() + 1); - txCoinbase.vin[0].scriptSig.push_back(blockinfo[i].extranonce); - txCoinbase.vin[0].scriptSig.push_back(chainActive.Height()); - txCoinbase.vout[0].scriptPubKey = CScript(); - pblock->vtx[0] = MakeTransactionRef(std::move(txCoinbase)); - if (txFirst.size() == 0) - baseheight = chainActive.Height(); - if (txFirst.size() < 4) - txFirst.push_back(pblock->vtx[0]); - pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); - pblock->nNonce = blockinfo[i].nonce; - - // This will usually succeed in the first round as we take the nonce from blockinfo - // It's however usefull when adding new blocks with unknown nonces (you should add the found block to blockinfo) - while (!CheckProofOfWork(pblock->GetHash(), pblock->nBits, chainparams.GetConsensus())) { - pblock->nNonce++; + { + LOCK(cs_main); + pblock->nVersion = 2; + pblock->nTime = chainActive.Tip()->GetMedianTimePast()+1; + CMutableTransaction txCoinbase(*pblock->vtx[0]); + txCoinbase.nVersion = 1; + txCoinbase.vin[0].scriptSig = CScript() << (chainActive.Height() + 1); + txCoinbase.vin[0].scriptSig.push_back(blockinfo[i].extranonce); + txCoinbase.vin[0].scriptSig.push_back(chainActive.Height()); + txCoinbase.vout[0].scriptPubKey = CScript(); + pblock->vtx[0] = MakeTransactionRef(std::move(txCoinbase)); + if (txFirst.size() == 0) + baseheight = chainActive.Height(); + if (txFirst.size() < 4) + txFirst.push_back(pblock->vtx[0]); + pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); + pblock->nNonce = blockinfo[i].nonce; + + // This will usually succeed in the first round as we take the nonce from blockinfo + // It's however usefull when adding new blocks with unknown nonces (you should add the found block to blockinfo) + while (!CheckProofOfWork(pblock->GetHash(), pblock->nBits, chainparams.GetConsensus())) { + pblock->nNonce++; + } } - std::shared_ptr shared_pblock = std::make_shared(*pblock); BOOST_CHECK(ProcessNewBlock(chainparams, shared_pblock, true, NULL)); pblock->hashPrevBlock = pblock->GetHash(); @@ -256,6 +257,8 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) createAndProcessEmptyBlock(); } + LOCK(cs_main); + // Just to make sure we can still make simple blocks BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); diff --git a/src/validation.cpp b/src/validation.cpp index bf2e337e1201..ae8a13c7877a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2886,6 +2886,7 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, // far from a guarantee. Things in the P2P/RPC will often end up calling // us in the middle of ProcessNewBlock - do not assume pblock is set // sanely for performance or correctness! + AssertLockNotHeld(cs_main); CBlockIndex *pindexMostWork = NULL; CBlockIndex *pindexNewTip = NULL; @@ -3599,6 +3600,8 @@ static bool AcceptBlock(const std::shared_ptr& pblock, CValidation bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr pblock, bool fForceProcessing, bool *fNewBlock) { + AssertLockNotHeld(cs_main); + { CBlockIndex *pindex = NULL; if (fNewBlock) *fNewBlock = false; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 90157fc098bf..3313a54549b3 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -363,14 +363,14 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) { - LOCK(cs_main); - // Cap last block file size, and mine new block in a new block file. CBlockIndex* oldTip = chainActive.Tip(); GetBlockFileInfo(oldTip->GetBlockPos().nFile)->nSize = MAX_BLOCKFILE_SIZE; CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); CBlockIndex* newTip = chainActive.Tip(); + LOCK(cs_main); + // Verify ScanForWalletTransactions picks up transactions in both the old // and new block files. { @@ -435,8 +435,6 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) { CWallet *pwalletMainBackup = ::pwalletMain; - LOCK(cs_main); - // Create two blocks with same timestamp to verify that importwallet rescan // will pick up both blocks, not just the first. const int64_t BLOCK_TIME = chainActive.Tip()->GetBlockTimeMax() + 5; @@ -450,6 +448,8 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) SetMockTime(KEY_TIME); coinbaseTxns.emplace_back(*CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); + LOCK(cs_main); + // Import key into wallet and call dumpwallet to create backup file. { CWallet wallet; From 72af215a362e6b13f3494702e6a8acbaaa8a994b Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Tue, 12 Mar 2019 09:33:53 +0100 Subject: [PATCH 10/11] Fix CreateNewBlock_validity by not holding cs_main when calling createAndProcessEmptyBlock --- src/test/miner_tests.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index c58986b4369d..e9c7123b154d 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -257,6 +257,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) createAndProcessEmptyBlock(); } + { LOCK(cs_main); // Just to make sure we can still make simple blocks @@ -507,9 +508,13 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++) chainActive.Tip()->GetAncestor(chainActive.Tip()->nHeight - i)->nTime += 512; //Trick the MedianTimePast + } // unlock cs_main while calling createAndProcessEmptyBlock + // Mine an empty block createAndProcessEmptyBlock(); + LOCK(cs_main); + SetMockTime(chainActive.Tip()->GetMedianTimePast() + 1); BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); From 152a78eab2e3bc3ec7ba45f39332de4e7c96b9be Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Tue, 12 Mar 2019 10:15:45 +0100 Subject: [PATCH 11/11] Add compatibility code to P2PFingerprintTest until we catch up with backports --- qa/rpc-tests/p2p-fingerprint.py | 37 ++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/qa/rpc-tests/p2p-fingerprint.py b/qa/rpc-tests/p2p-fingerprint.py index fe60c6cd46dc..b59b66e1e42f 100755 --- a/qa/rpc-tests/p2p-fingerprint.py +++ b/qa/rpc-tests/p2p-fingerprint.py @@ -7,6 +7,7 @@ If an stale block more than a month old or its header are requested by a peer, the node should pretend that it does not have it to avoid fingerprinting. """ +import threading import time @@ -15,7 +16,7 @@ CInv, NetworkThread, NodeConn, - NodeConnCB, + SingleNodeConnCB, msg_headers, msg_block, msg_getdata, @@ -26,13 +27,24 @@ from test_framework.util import ( assert_equal, p2p_port, -) + start_nodes) class P2PFingerprintTest(BitcoinTestFramework): + def __init__(self): + BitcoinTestFramework.__init__(self) + # TODO When this asserts, you have probably backported bitcoin#11121, so you'll have to remove this constructor + assert(not callable(getattr(BitcoinTestFramework(), "set_test_params", None))) + self.set_test_params() + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 + # TODO also remove this when bitcoin#11121 is backported + def setup_network(self, split=False): + self.nodes = start_nodes(self.num_nodes, self.options.tmpdir) + self.is_network_split = False + # Build a chain of blocks on top of given one def build_chain(self, nblocks, prev_hash, prev_height, prev_median_time): blocks = [] @@ -77,7 +89,26 @@ def last_header_equals(self, expected_hash, node): # This does not currently test that stale blocks timestamped within the # last month but that have over a month's worth of work are also withheld. def run_test(self): - node0 = NodeConnCB() + # TODO remove this when mininode is up-to-date with Bitcoin + class MyNodeConnCB(SingleNodeConnCB): + def __init__(self): + SingleNodeConnCB.__init__(self) + self.cond = threading.Condition() + self.last_message = {} + + def deliver(self, conn, message): + SingleNodeConnCB.deliver(self, conn, message) + command = message.command.decode('ascii') + self.last_message[command] = message + with self.cond: + self.cond.notify_all() + + def wait_for_getdata(self): + with self.cond: + assert(self.cond.wait_for(lambda: "getdata" in self.last_message, timeout=15)) + + + node0 = MyNodeConnCB() connections = [] connections.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], node0))