From 45151bd131741ce1b0607c8330b87def769879d0 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 8 Jun 2016 16:12:52 -0700 Subject: [PATCH 01/14] Move context-required checks from CheckBlockHeader to Contextual... --- src/validation.cpp | 28 ++++++++++++++-------------- src/validation.h | 6 +++--- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 45d93c6a48b3..c07075f813c7 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2027,7 +2027,7 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd int64_t nTimeStart = GetTimeMicros(); // Check it again in case a previous version let a bad block in - if (!CheckBlock(block, state, chainparams.GetConsensus(), GetAdjustedTime(), !fJustCheck, !fJustCheck)) + if (!CheckBlock(block, state, chainparams.GetConsensus(), !fJustCheck, !fJustCheck)) return error("%s: Consensus::CheckBlock: %s", __func__, FormatStateMessage(state)); // verify that the view's current state corresponds to the previous block @@ -3263,16 +3263,12 @@ bool FindUndoPos(CValidationState &state, int nFile, CDiskBlockPos &pos, unsigne return true; } -bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state, const Consensus::Params& consensusParams, int64_t nAdjustedTime, bool fCheckPOW) +bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state, const Consensus::Params& consensusParams, bool fCheckPOW) { // Check proof of work matches claimed amount if (fCheckPOW && !CheckProofOfWork(block.GetHash(), block.nBits, consensusParams)) return state.DoS(50, false, REJECT_INVALID, "high-hash", false, "proof of work failed"); - // Check timestamp - if (block.GetBlockTime() > nAdjustedTime + 2 * 60 * 60) - return state.Invalid(false, REJECT_INVALID, "time-too-new", "block timestamp too far in the future"); - // Check DevNet if (!consensusParams.hashDevnetGenesisBlock.IsNull() && block.hashPrevBlock == consensusParams.hashGenesisBlock && @@ -3284,7 +3280,7 @@ bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state, const return true; } -bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::Params& consensusParams, int64_t nAdjustedTime, bool fCheckPOW, bool fCheckMerkleRoot) +bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::Params& consensusParams, bool fCheckPOW, bool fCheckMerkleRoot) { // These are checks that are independent of context. @@ -3293,7 +3289,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P // Check that the header is valid (particularly PoW). This is mostly // redundant with the call in AcceptBlockHeader. - if (!CheckBlockHeader(block, state, consensusParams, nAdjustedTime, fCheckPOW)) + if (!CheckBlockHeader(block, state, consensusParams, fCheckPOW)) return false; // Check the merkle root. @@ -3389,7 +3385,7 @@ static bool CheckIndexAgainstCheckpoint(const CBlockIndex* pindexPrev, CValidati return true; } -bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& state, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev) +bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& state, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev, int64_t nAdjustedTime) { const int nHeight = pindexPrev == NULL ? 0 : pindexPrev->nHeight + 1; // Check proof of work @@ -3411,6 +3407,10 @@ bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& sta if (block.GetBlockTime() <= pindexPrev->GetMedianTimePast()) return state.Invalid(false, REJECT_INVALID, "time-too-old", "block's timestamp is too early"); + // Check timestamp + if (block.GetBlockTime() > nAdjustedTime + 2 * 60 * 60) + return state.Invalid(false, REJECT_INVALID, "time-too-new", "block timestamp too far in the future"); + // check for version 2, 3 and 4 upgrades if((block.nVersion < 2 && nHeight >= consensusParams.BIP34Height) || (block.nVersion < 3 && nHeight >= consensusParams.BIP66Height) || @@ -3493,7 +3493,7 @@ static bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state return true; } - if (!CheckBlockHeader(block, state, chainparams.GetConsensus(), GetAdjustedTime())) + if (!CheckBlockHeader(block, state, chainparams.GetConsensus())) return error("%s: Consensus::CheckBlockHeader: %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); // Get prev block index @@ -3509,7 +3509,7 @@ static bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state if (fCheckpointsEnabled && !CheckIndexAgainstCheckpoint(pindexPrev, state, chainparams, hash)) return error("%s: CheckIndexAgainstCheckpoint(): %s", __func__, state.GetRejectReason().c_str()); - if (!ContextualCheckBlockHeader(block, state, chainparams.GetConsensus(), pindexPrev)) + if (!ContextualCheckBlockHeader(block, state, chainparams.GetConsensus(), pindexPrev, GetAdjustedTime())) return error("%s: Consensus::ContextualCheckBlockHeader: %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); } if (pindex == NULL) @@ -3654,9 +3654,9 @@ bool TestBlockValidity(CValidationState& state, const CChainParams& chainparams, indexDummy.nHeight = pindexPrev->nHeight + 1; // NOTE: CheckBlockHeader is called by CheckBlock - if (!ContextualCheckBlockHeader(block, state, chainparams.GetConsensus(), pindexPrev)) + if (!ContextualCheckBlockHeader(block, state, chainparams.GetConsensus(), pindexPrev, GetAdjustedTime())) return error("%s: Consensus::ContextualCheckBlockHeader: %s", __func__, FormatStateMessage(state)); - if (!CheckBlock(block, state, chainparams.GetConsensus(), GetAdjustedTime(), fCheckPOW, fCheckMerkleRoot)) + if (!CheckBlock(block, state, chainparams.GetConsensus(), fCheckPOW, fCheckMerkleRoot)) return error("%s: Consensus::CheckBlock: %s", __func__, FormatStateMessage(state)); if (!ContextualCheckBlock(block, state, chainparams.GetConsensus(), pindexPrev)) return error("%s: Consensus::ContextualCheckBlock: %s", __func__, FormatStateMessage(state)); @@ -4042,7 +4042,7 @@ bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview, if (!ReadBlockFromDisk(block, pindex, chainparams.GetConsensus())) return error("VerifyDB(): *** ReadBlockFromDisk failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString()); // check level 1: verify block validity - if (nCheckLevel >= 1 && !CheckBlock(block, state, chainparams.GetConsensus(), GetAdjustedTime())) + if (nCheckLevel >= 1 && !CheckBlock(block, state, chainparams.GetConsensus())) return error("%s: *** found bad block at %d, hash=%s (%s)\n", __func__, pindex->nHeight, pindex->GetBlockHash().ToString(), FormatStateMessage(state)); // check level 2: verify undo validity diff --git a/src/validation.h b/src/validation.h index ac7e00d35590..4b1dc43c7e22 100644 --- a/src/validation.h +++ b/src/validation.h @@ -491,13 +491,13 @@ bool DisconnectBlocks(int blocks); void ReprocessBlocks(int nBlocks); /** Context-independent validity checks */ -bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state, const Consensus::Params& consensusParams, int64_t nAdjustedTime, bool fCheckPOW = true); -bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::Params& consensusParams, int64_t nAdjustedTime, bool fCheckPOW = true, bool fCheckMerkleRoot = true); +bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state, const Consensus::Params& consensusParams, bool fCheckPOW = true); +bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::Params& consensusParams, bool fCheckPOW = true, bool fCheckMerkleRoot = true); /** Context-dependent validity checks. * By "context", we mean only the previous block headers, but not the UTXO * set; UTXO-related validity checks are done in ConnectBlock(). */ -bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& state, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev); +bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& state, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev, int64_t nAdjustedTime); bool ContextualCheckBlock(const CBlock& block, CValidationState& state, const Consensus::Params& consensusParams, const CBlockIndex *pindexPrev); /** Check a block is completely valid from start to finish (only works on top of our current best block, with cs_main held) */ From 2c810d2c3f32aa235613ab196d7709cafb604fff Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Mon, 25 Jul 2016 17:22:37 -0400 Subject: [PATCH 02/14] Allow changing BIP9 parameters on regtest --- src/chainparams.cpp | 11 +++++++++++ src/chainparams.h | 5 +++++ src/init.cpp | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 76b4a9662db1..4c663b813758 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -624,6 +624,12 @@ class CRegTestParams : public CChainParams { // Regtest Dash BIP44 coin type is '1' (All coin's testnet default) nExtCoinType = 1; } + + void UpdateBIP9Parameters(Consensus::DeploymentPos d, int64_t nStartTime, int64_t nTimeout) + { + consensus.vDeployments[d].nStartTime = nStartTime; + consensus.vDeployments[d].nTimeout = nTimeout; + } }; static CRegTestParams regTestParams; @@ -658,3 +664,8 @@ void SelectParams(const std::string& network) SelectBaseParams(network); pCurrentParams = &Params(network); } + +void UpdateRegtestBIP9Parameters(Consensus::DeploymentPos d, int64_t nStartTime, int64_t nTimeout) +{ + regTestParams.UpdateBIP9Parameters(d, nStartTime, nTimeout); +} diff --git a/src/chainparams.h b/src/chainparams.h index 4fd9774fcd2e..607e6da16bde 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -133,4 +133,9 @@ CChainParams& Params(const std::string& chain); */ void SelectParams(const std::string& chain); +/** + * Allows modifying the BIP9 regtest parameters. + */ +void UpdateRegtestBIP9Parameters(Consensus::DeploymentPos d, int64_t nStartTime, int64_t nTimeout); + #endif // BITCOIN_CHAINPARAMS_H diff --git a/src/init.cpp b/src/init.cpp index eaffffbe6113..da61ce00b364 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -505,6 +505,7 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-limitancestorsize=", strprintf("Do not accept transactions whose size with all in-mempool ancestors exceeds kilobytes (default: %u)", DEFAULT_ANCESTOR_SIZE_LIMIT)); strUsage += HelpMessageOpt("-limitdescendantcount=", strprintf("Do not accept transactions if any ancestor would have or more in-mempool descendants (default: %u)", DEFAULT_DESCENDANT_LIMIT)); strUsage += HelpMessageOpt("-limitdescendantsize=", strprintf("Do not accept transactions if any ancestor would have more than kilobytes of in-mempool descendants (default: %u).", DEFAULT_DESCENDANT_SIZE_LIMIT)); + strUsage += HelpMessageOpt("-bip9params=deployment:start:end", "Use given start/end times for specified bip9 deployment (regtest-only)"); } std::string debugCategories = "addrman, alert, bench, coindb, db, http, leveldb, libevent, lock, mempool, mempoolrej, net, proxy, prune, rand, reindex, rpc, selectcoins, tor, zmq, " "dash (or specifically: gobject, instantsend, keepass, masternode, mnpayments, mnsync, privatesend, spork)"; // Don't translate these and qt below @@ -1220,6 +1221,41 @@ bool AppInitParameterInteraction() fEnableReplacement = (std::find(vstrReplacementModes.begin(), vstrReplacementModes.end(), "fee") != vstrReplacementModes.end()); } + if (!mapMultiArgs["-bip9params"].empty()) { + // Allow overriding bip9 parameters for testing + if (!Params().MineBlocksOnDemand()) { + return InitError("BIP9 parameters may only be overridden on regtest."); + } + const vector& deployments = mapMultiArgs["-bip9params"]; + for (auto i : deployments) { + std::vector vDeploymentParams; + boost::split(vDeploymentParams, i, boost::is_any_of(":")); + if (vDeploymentParams.size() != 3) { + return InitError("BIP9 parameters malformed, expecting deployment:start:end"); + } + int64_t nStartTime, nTimeout; + if (!ParseInt64(vDeploymentParams[1], &nStartTime)) { + return InitError(strprintf("Invalid nStartTime (%s)", vDeploymentParams[1])); + } + if (!ParseInt64(vDeploymentParams[2], &nTimeout)) { + return InitError(strprintf("Invalid nTimeout (%s)", vDeploymentParams[2])); + } + bool found = false; + for (int i=0; i<(int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++i) + { + if (vDeploymentParams[0].compare(VersionBitsDeploymentInfo[i].name) == 0) { + UpdateRegtestBIP9Parameters(Consensus::DeploymentPos(i), nStartTime, nTimeout); + found = true; + LogPrintf("Setting BIP9 activation parameters for %s to start=%ld, timeout=%ld\n", vDeploymentParams[0], nStartTime, nTimeout); + break; + } + } + if (!found) { + return InitError(strprintf("Invalid deployment (%s)", vDeploymentParams[0])); + } + } + } + return true; } From e326bda69e3729136fed3a7cedadf444630fb958 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Fri, 17 Jun 2016 21:17:25 -0400 Subject: [PATCH 03/14] Tests: refactor compact size serialization in mininode --- qa/rpc-tests/test_framework/mininode.py | 98 +++++++------------------ 1 file changed, 26 insertions(+), 72 deletions(-) diff --git a/qa/rpc-tests/test_framework/mininode.py b/qa/rpc-tests/test_framework/mininode.py index ecc75fe17304..41c8ab0795c0 100755 --- a/qa/rpc-tests/test_framework/mininode.py +++ b/qa/rpc-tests/test_framework/mininode.py @@ -77,7 +77,19 @@ def hash256(s): def dashhash(s): return dash_hash.getPoWHash(s) -def deser_string(f): +def ser_compact_size(l): + r = b"" + if l < 253: + r = struct.pack("B", l) + elif l < 0x10000: + r = struct.pack(" Date: Wed, 3 Aug 2016 11:05:16 +0200 Subject: [PATCH 04/14] Merge #8446: [Trivial] BIP9 parameters on regtest cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 0fc00be Do not shadow previous local variable (Pavel Janík) 115265b Trivial: bip -> BIP in help text and comment (Pavel Janík) --- src/init.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index da61ce00b364..1a24f7713e6c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -505,7 +505,7 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-limitancestorsize=", strprintf("Do not accept transactions whose size with all in-mempool ancestors exceeds kilobytes (default: %u)", DEFAULT_ANCESTOR_SIZE_LIMIT)); strUsage += HelpMessageOpt("-limitdescendantcount=", strprintf("Do not accept transactions if any ancestor would have or more in-mempool descendants (default: %u)", DEFAULT_DESCENDANT_LIMIT)); strUsage += HelpMessageOpt("-limitdescendantsize=", strprintf("Do not accept transactions if any ancestor would have more than kilobytes of in-mempool descendants (default: %u).", DEFAULT_DESCENDANT_SIZE_LIMIT)); - strUsage += HelpMessageOpt("-bip9params=deployment:start:end", "Use given start/end times for specified bip9 deployment (regtest-only)"); + strUsage += HelpMessageOpt("-bip9params=deployment:start:end", "Use given start/end times for specified BIP9 deployment (regtest-only)"); } std::string debugCategories = "addrman, alert, bench, coindb, db, http, leveldb, libevent, lock, mempool, mempoolrej, net, proxy, prune, rand, reindex, rpc, selectcoins, tor, zmq, " "dash (or specifically: gobject, instantsend, keepass, masternode, mnpayments, mnsync, privatesend, spork)"; // Don't translate these and qt below @@ -1222,7 +1222,7 @@ bool AppInitParameterInteraction() } if (!mapMultiArgs["-bip9params"].empty()) { - // Allow overriding bip9 parameters for testing + // Allow overriding BIP9 parameters for testing if (!Params().MineBlocksOnDemand()) { return InitError("BIP9 parameters may only be overridden on regtest."); } @@ -1241,10 +1241,10 @@ bool AppInitParameterInteraction() return InitError(strprintf("Invalid nTimeout (%s)", vDeploymentParams[2])); } bool found = false; - for (int i=0; i<(int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++i) + for (int j=0; j<(int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++j) { - if (vDeploymentParams[0].compare(VersionBitsDeploymentInfo[i].name) == 0) { - UpdateRegtestBIP9Parameters(Consensus::DeploymentPos(i), nStartTime, nTimeout); + if (vDeploymentParams[0].compare(VersionBitsDeploymentInfo[j].name) == 0) { + UpdateRegtestBIP9Parameters(Consensus::DeploymentPos(j), nStartTime, nTimeout); found = true; LogPrintf("Setting BIP9 activation parameters for %s to start=%ld, timeout=%ld\n", vDeploymentParams[0], nStartTime, nTimeout); break; From b2bc7809947c9107a3727fdce2bd17557aad6402 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Thu, 8 Feb 2018 08:43:25 +0100 Subject: [PATCH 05/14] Fix argument to wait_until --- qa/rpc-tests/test_framework/mininode.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qa/rpc-tests/test_framework/mininode.py b/qa/rpc-tests/test_framework/mininode.py index 41c8ab0795c0..b32b8dcfae7e 100755 --- a/qa/rpc-tests/test_framework/mininode.py +++ b/qa/rpc-tests/test_framework/mininode.py @@ -1145,7 +1145,7 @@ def sync_with_ping(self, timeout=30): def received_pong(): return (self.last_pong.nonce == self.ping_counter) self.send_message(msg_ping(nonce=self.ping_counter)) - success = wait_until(received_pong, timeout) + success = wait_until(received_pong, timeout=timeout) self.ping_counter += 1 return success From 64817fe1d30ba3a91327b9585848371c1d5f99a8 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Thu, 6 Oct 2016 14:21:11 -0400 Subject: [PATCH 06/14] [qa] Fix race condition in sendheaders.py Also de-duplicates code that has been moved to mininode --- qa/rpc-tests/sendheaders.py | 68 ++++++++++++++----------------------- 1 file changed, 26 insertions(+), 42 deletions(-) diff --git a/qa/rpc-tests/sendheaders.py b/qa/rpc-tests/sendheaders.py index 653b374873f0..37b98c576e83 100755 --- a/qa/rpc-tests/sendheaders.py +++ b/qa/rpc-tests/sendheaders.py @@ -80,20 +80,19 @@ Expect: disconnect. ''' -class BaseNode(NodeConnCB): +direct_fetch_response_time = 0.05 + +class BaseNode(SingleNodeConnCB): def __init__(self): - NodeConnCB.__init__(self) - self.connection = None + SingleNodeConnCB.__init__(self) self.last_inv = None self.last_headers = None self.last_block = None - self.ping_counter = 1 - self.last_pong = msg_pong(0) self.last_getdata = None - self.sleep_time = 0.05 self.block_announced = False self.last_getheaders = None self.disconnected = False + self.last_blockhash_announced = None def clear_last_announcement(self): with mininode_lock: @@ -101,9 +100,6 @@ def clear_last_announcement(self): self.last_inv = None self.last_headers = None - def add_connection(self, conn): - self.connection = conn - # Request data for a list of block hashes def get_data(self, block_hashes): msg = msg_getdata() @@ -122,17 +118,17 @@ def send_block_inv(self, blockhash): msg.inv = [CInv(2, blockhash)] self.connection.send_message(msg) - # Wrapper for the NodeConn's send_message function - def send_message(self, message): - self.connection.send_message(message) - def on_inv(self, conn, message): self.last_inv = message self.block_announced = True + self.last_blockhash_announced = message.inv[-1].hash def on_headers(self, conn, message): self.last_headers = message - self.block_announced = True + if len(message.headers): + self.block_announced = True + message.headers[-1].calc_sha256() + self.last_blockhash_announced = message.headers[-1].sha256 def on_block(self, conn, message): self.last_block = message.block @@ -141,9 +137,6 @@ def on_block(self, conn, message): def on_getdata(self, conn, message): self.last_getdata = message - def on_pong(self, conn, message): - self.last_pong = message - def on_getheaders(self, conn, message): self.last_getheaders = message @@ -157,7 +150,7 @@ def check_last_announcement(self, headers=None, inv=None): expect_headers = headers if headers != None else [] expect_inv = inv if inv != None else [] test_function = lambda: self.block_announced - self.sync(test_function) + assert(wait_until(test_function, timeout=60)) with mininode_lock: self.block_announced = False @@ -180,30 +173,14 @@ def check_last_announcement(self, headers=None, inv=None): return success # Syncing helpers - def sync(self, test_function, timeout=60): - while timeout > 0: - with mininode_lock: - if test_function(): - return - time.sleep(self.sleep_time) - timeout -= self.sleep_time - raise AssertionError("Sync failed to complete") - - def sync_with_ping(self, timeout=60): - self.send_message(msg_ping(nonce=self.ping_counter)) - test_function = lambda: self.last_pong.nonce == self.ping_counter - self.sync(test_function, timeout) - self.ping_counter += 1 - return - def wait_for_block(self, blockhash, timeout=60): test_function = lambda: self.last_block != None and self.last_block.sha256 == blockhash - self.sync(test_function, timeout) + assert(wait_until(test_function, timeout=timeout)) return def wait_for_getheaders(self, timeout=60): test_function = lambda: self.last_getheaders != None - self.sync(test_function, timeout) + assert(wait_until(test_function, timeout=timeout)) return def wait_for_getdata(self, hash_list, timeout=60): @@ -211,12 +188,17 @@ def wait_for_getdata(self, hash_list, timeout=60): return test_function = lambda: self.last_getdata != None and [x.hash for x in self.last_getdata.inv] == hash_list - self.sync(test_function, timeout) + assert(wait_until(test_function, timeout=timeout)) return def wait_for_disconnect(self, timeout=60): test_function = lambda: self.disconnected - self.sync(test_function, timeout) + assert(wait_until(test_function, timeout=timeout)) + return + + def wait_for_block_announcement(self, block_hash, timeout=60): + test_function = lambda: self.last_blockhash_announced == block_hash + assert(wait_until(test_function, timeout=timeout)) return def send_header_for_blocks(self, new_blocks): @@ -266,7 +248,9 @@ def mine_blocks(self, count): def mine_reorg(self, length): self.nodes[0].generate(length) # make sure all invalidated blocks are node0's sync_blocks(self.nodes, wait=0.1) - [x.clear_last_announcement() for x in self.p2p_connections] + for x in self.p2p_connections: + x.wait_for_block_announcement(int(self.nodes[0].getbestblockhash(), 16)) + x.clear_last_announcement() tip_height = self.nodes[1].getblockcount() hash_to_invalidate = self.nodes[1].getblockhash(tip_height-(length-1)) @@ -494,7 +478,7 @@ def run_test(self): test_node.send_header_for_blocks(blocks) test_node.sync_with_ping() - test_node.wait_for_getdata([x.sha256 for x in blocks], timeout=test_node.sleep_time) + test_node.wait_for_getdata([x.sha256 for x in blocks], timeout=direct_fetch_response_time) [ test_node.send_message(msg_block(x)) for x in blocks ] @@ -525,13 +509,13 @@ def run_test(self): # both blocks (same work as tip) test_node.send_header_for_blocks(blocks[1:2]) test_node.sync_with_ping() - test_node.wait_for_getdata([x.sha256 for x in blocks[0:2]], timeout=test_node.sleep_time) + test_node.wait_for_getdata([x.sha256 for x in blocks[0:2]], timeout=direct_fetch_response_time) # Announcing 16 more headers should trigger direct fetch for 14 more # blocks test_node.send_header_for_blocks(blocks[2:18]) test_node.sync_with_ping() - test_node.wait_for_getdata([x.sha256 for x in blocks[2:16]], timeout=test_node.sleep_time) + test_node.wait_for_getdata([x.sha256 for x in blocks[2:16]], timeout=direct_fetch_response_time) # Announcing 1 more header should not trigger any response test_node.last_getdata = None From 1d1c31052aa163820289858e5a9afb2c35b8118b Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Thu, 8 Feb 2018 09:02:47 +0100 Subject: [PATCH 07/14] Fix cmd args handling for -bip9params --- src/init.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 1a24f7713e6c..addcbab1078f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1221,12 +1221,12 @@ bool AppInitParameterInteraction() fEnableReplacement = (std::find(vstrReplacementModes.begin(), vstrReplacementModes.end(), "fee") != vstrReplacementModes.end()); } - if (!mapMultiArgs["-bip9params"].empty()) { + if (mapMultiArgs.count("-bip9params")) { // Allow overriding BIP9 parameters for testing - if (!Params().MineBlocksOnDemand()) { + if (!chainparams.MineBlocksOnDemand()) { return InitError("BIP9 parameters may only be overridden on regtest."); } - const vector& deployments = mapMultiArgs["-bip9params"]; + const std::vector& deployments = mapMultiArgs.at("-bip9params"); for (auto i : deployments) { std::vector vDeploymentParams; boost::split(vDeploymentParams, i, boost::is_any_of(":")); From 807ae74c210e7411b59fc33372edda26708daba9 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 18 Dec 2016 23:03:16 -0800 Subject: [PATCH 08/14] Make CBlockIndex*es in net_processing const --- src/net_processing.cpp | 44 +++++++++++++++++++++--------------------- src/validation.cpp | 6 ++++-- src/validation.h | 2 +- 3 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1cc41b4c0207..e87eb65fda3b 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -110,7 +110,7 @@ namespace { /** Blocks that are in flight, and that are in the queue to be downloaded. Protected by cs_main. */ struct QueuedBlock { uint256 hash; - CBlockIndex* pindex; //!< Optional. + const CBlockIndex* pindex; //!< Optional. bool fValidatedHeaders; //!< Whether this block has validated headers at the time of request. }; std::map::iterator> > mapBlocksInFlight; @@ -161,13 +161,13 @@ struct CNodeState { //! List of asynchronously-determined block rejections to notify this peer about. std::vector rejects; //! The best known block we know this peer has announced. - CBlockIndex *pindexBestKnownBlock; + const CBlockIndex *pindexBestKnownBlock; //! The hash of the last unknown block this peer has announced. uint256 hashLastUnknownBlock; //! The last full block we both have. - CBlockIndex *pindexLastCommonBlock; + const CBlockIndex *pindexLastCommonBlock; //! The best header we have sent our peer. - CBlockIndex *pindexBestHeaderSent; + const CBlockIndex *pindexBestHeaderSent; //! Length of current-streak of unconnecting headers announcements int nUnconnectingHeaders; //! Whether we've started headers synchronization with this peer. @@ -314,7 +314,7 @@ bool MarkBlockAsReceived(const uint256& hash) { } // Requires cs_main. -void MarkBlockAsInFlight(NodeId nodeid, const uint256& hash, const Consensus::Params& consensusParams, CBlockIndex *pindex = NULL) { +void MarkBlockAsInFlight(NodeId nodeid, const uint256& hash, const Consensus::Params& consensusParams, const CBlockIndex *pindex = NULL) { CNodeState *state = State(nodeid); assert(state != NULL); @@ -375,7 +375,7 @@ bool CanDirectFetch(const Consensus::Params &consensusParams) } // Requires cs_main -bool PeerHasHeader(CNodeState *state, CBlockIndex *pindex) +bool PeerHasHeader(CNodeState *state, const CBlockIndex *pindex) { if (state->pindexBestKnownBlock && pindex == state->pindexBestKnownBlock->GetAncestor(pindex->nHeight)) return true; @@ -386,7 +386,7 @@ bool PeerHasHeader(CNodeState *state, CBlockIndex *pindex) /** Find the last common ancestor two blocks have. * Both pa and pb must be non-NULL. */ -CBlockIndex* LastCommonAncestor(CBlockIndex* pa, CBlockIndex* pb) { +const CBlockIndex* LastCommonAncestor(const CBlockIndex* pa, const CBlockIndex* pb) { if (pa->nHeight > pb->nHeight) { pa = pa->GetAncestor(pb->nHeight); } else if (pb->nHeight > pa->nHeight) { @@ -405,7 +405,7 @@ CBlockIndex* LastCommonAncestor(CBlockIndex* pa, CBlockIndex* pb) { /** Update pindexLastCommonBlock and add not-in-flight missing successors to vBlocks, until it has * at most count entries. */ -void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector& vBlocks, NodeId& nodeStaller, const Consensus::Params& consensusParams) { +void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector& vBlocks, NodeId& nodeStaller, const Consensus::Params& consensusParams) { if (count == 0) return; @@ -433,8 +433,8 @@ void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vectorpindexLastCommonBlock == state->pindexBestKnownBlock) return; - std::vector vToFetch; - CBlockIndex *pindexWalk = state->pindexLastCommonBlock; + std::vector vToFetch; + const CBlockIndex *pindexWalk = state->pindexLastCommonBlock; // Never fetch further than the best block we know the peer has, or more than BLOCK_DOWNLOAD_WINDOW + 1 beyond the last // linked block we have in common with this peer. The +1 is so we can detect stalling, namely if we would be able to // download that next block if the window were 1 larger. @@ -457,7 +457,7 @@ void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vectorIsValid(BLOCK_VALID_TREE)) { // We consider the chain that this peer is on invalid. return; @@ -1550,7 +1550,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LOCK(cs_main); // Find the last block the caller has in the main chain - CBlockIndex* pindex = FindForkInGlobalIndex(chainActive, locator); + const CBlockIndex* pindex = FindForkInGlobalIndex(chainActive, locator); // Send the rest of the chain if (pindex) @@ -1598,7 +1598,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } CNodeState *nodestate = State(pfrom->GetId()); - CBlockIndex* pindex = NULL; + const CBlockIndex* pindex = NULL; if (locator.IsNull()) { // If locator is null, return the hashStop block @@ -1892,7 +1892,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr ReadCompactSize(vRecv); // ignore tx count; assume it is 0. } - CBlockIndex *pindexLast = NULL; + const CBlockIndex *pindexLast = NULL; { LOCK(cs_main); CNodeState *nodestate = State(pfrom->GetId()); @@ -1983,8 +1983,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // If this set of headers is valid and ends in a block with at least as // much work as our tip, download as much as possible. if (fCanDirectFetch && pindexLast->IsValid(BLOCK_VALID_TREE) && chainActive.Tip()->nChainWork <= pindexLast->nChainWork) { - std::vector vToFetch; - CBlockIndex *pindexWalk = pindexLast; + std::vector vToFetch; + const CBlockIndex *pindexWalk = pindexLast; // Calculate all the blocks we'd need to switch to pindexLast, up to a limit. while (pindexWalk && !chainActive.Contains(pindexWalk) && vToFetch.size() <= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { if (!(pindexWalk->nStatus & BLOCK_HAVE_DATA) && @@ -2005,7 +2005,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } else { std::vector vGetData; // Download as much as possible, from earliest to latest. - BOOST_REVERSE_FOREACH(CBlockIndex *pindex, vToFetch) { + BOOST_REVERSE_FOREACH(const CBlockIndex *pindex, vToFetch) { if (nodestate->nBlocksInFlight >= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { // Can't download any more from this peer break; @@ -2626,7 +2626,7 @@ bool SendMessages(CNode* pto, CConnman& connman, const std::atomic& interr LOCK(pto->cs_inventory); std::vector vHeaders; bool fRevertToInv = (!state.fPreferHeaders || pto->vBlockHashesToAnnounce.size() > MAX_BLOCKS_TO_ANNOUNCE); - CBlockIndex *pBestIndex = NULL; // last header queued for delivery + const CBlockIndex *pBestIndex = NULL; // last header queued for delivery ProcessBlockAvailability(pto->id); // ensure pindexBestKnownBlock is up-to-date if (!fRevertToInv) { @@ -2637,7 +2637,7 @@ bool SendMessages(CNode* pto, CConnman& connman, const std::atomic& interr BOOST_FOREACH(const uint256 &hash, pto->vBlockHashesToAnnounce) { BlockMap::iterator mi = mapBlockIndex.find(hash); assert(mi != mapBlockIndex.end()); - CBlockIndex *pindex = mi->second; + const CBlockIndex *pindex = mi->second; if (chainActive[pindex->nHeight] != pindex) { // Bail out if we reorged away from this block fRevertToInv = true; @@ -2685,7 +2685,7 @@ bool SendMessages(CNode* pto, CConnman& connman, const std::atomic& interr const uint256 &hashToAnnounce = pto->vBlockHashesToAnnounce.back(); BlockMap::iterator mi = mapBlockIndex.find(hashToAnnounce); assert(mi != mapBlockIndex.end()); - CBlockIndex *pindex = mi->second; + const CBlockIndex *pindex = mi->second; // Warn if we're announcing a block that is not on the main chain. // This should be very rare and could be optimized out. @@ -2927,10 +2927,10 @@ bool SendMessages(CNode* pto, CConnman& connman, const std::atomic& interr // std::vector vGetData; if (!pto->fClient && (fFetch || !IsInitialBlockDownload()) && state.nBlocksInFlight < MAX_BLOCKS_IN_TRANSIT_PER_PEER) { - std::vector vToDownload; + std::vector vToDownload; NodeId staller = -1; FindNextBlocksToDownload(pto->GetId(), MAX_BLOCKS_IN_TRANSIT_PER_PEER - state.nBlocksInFlight, vToDownload, staller, consensusParams); - BOOST_FOREACH(CBlockIndex *pindex, vToDownload) { + BOOST_FOREACH(const CBlockIndex *pindex, vToDownload) { vGetData.push_back(CInv(MSG_BLOCK, pindex->GetBlockHash())); MarkBlockAsInFlight(pto->GetId(), pindex->GetBlockHash(), consensusParams, pindex); LogPrint("net", "Requesting block %s (%d) peer=%d\n", pindex->GetBlockHash().ToString(), diff --git a/src/validation.cpp b/src/validation.cpp index c07075f813c7..f930ea2cab74 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3527,12 +3527,14 @@ static bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state } // Exposed wrapper for AcceptBlockHeader -bool ProcessNewBlockHeaders(const std::vector& headers, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex) +bool ProcessNewBlockHeaders(const std::vector& headers, CValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex) { { LOCK(cs_main); for (const CBlockHeader& header : headers) { - if (!AcceptBlockHeader(header, state, chainparams, ppindex)) { + // cast away the ppindex-returns-const CBlockIndex - we're just assigning it to a CBlockIndex* + // that we own and is updated non-const anyway + if (!AcceptBlockHeader(header, state, chainparams, const_cast(ppindex))) { return false; } } diff --git a/src/validation.h b/src/validation.h index 4b1dc43c7e22..b016c488e4fd 100644 --- a/src/validation.h +++ b/src/validation.h @@ -255,7 +255,7 @@ bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr& block, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex=NULL); +bool ProcessNewBlockHeaders(const std::vector& block, CValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex=NULL); /** Check whether enough disk space is available for an incoming block */ bool CheckDiskSpace(uint64_t nAdditionalBytes = 0); From c99dd97339e38e57f8c42fb4d360c1430d0de1f6 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 19 Dec 2016 00:47:08 -0800 Subject: [PATCH 09/14] [qa] Avoid race in preciousblock test. If node 0 is sufficiently fast to announce its block to node 1, node 1 might already have the block by the time the node_sync_via_rpc loop gets around to node 1, resulting in the submitblock result "duplicate-inconclusive" as node 1 has the block, but prefers an alternate chain. --- qa/rpc-tests/preciousblock.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qa/rpc-tests/preciousblock.py b/qa/rpc-tests/preciousblock.py index 3cefa51c0a7f..0d5f9842e341 100755 --- a/qa/rpc-tests/preciousblock.py +++ b/qa/rpc-tests/preciousblock.py @@ -102,7 +102,7 @@ def run_test(self): assert_equal(self.nodes[2].getblockcount(), 6) hashL = self.nodes[2].getbestblockhash() print("Connect nodes and check no reorg occurs") - node_sync_via_rpc(self.nodes[0:3]) + node_sync_via_rpc(self.nodes[1:3]) connect_nodes_bi(self.nodes,1,2) connect_nodes_bi(self.nodes,0,2) assert_equal(self.nodes[0].getbestblockhash(), hashH) From d28172f57ede13a2cd3a1ad2ad8b4d6a35aca2da Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 18 Dec 2016 23:24:11 -0800 Subject: [PATCH 10/14] Call AcceptBlock with the block's shared_ptr instead of CBlock& --- src/validation.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index f930ea2cab74..e0535e5276b9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3544,8 +3544,10 @@ bool ProcessNewBlockHeaders(const std::vector& headers, CValidatio } /** Store block on disk. If dbp is non-NULL, the file is known to already reside on disk */ -static bool AcceptBlock(const CBlock& block, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool fRequested, const CDiskBlockPos* dbp, bool* fNewBlock) +static bool AcceptBlock(const std::shared_ptr& pblock, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool fRequested, const CDiskBlockPos* dbp, bool* fNewBlock) { + const CBlock& block = *pblock; + if (fNewBlock) *fNewBlock = false; AssertLockHeld(cs_main); @@ -3625,7 +3627,7 @@ bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptrnPos = nBlockPos; blkdat.SetLimit(nBlockPos + nSize); blkdat.SetPos(nBlockPos); - CBlock block; + std::shared_ptr pblock = std::make_shared(); + CBlock& block = *pblock; blkdat >> block; nRewind = blkdat.GetPos(); @@ -4257,7 +4260,7 @@ bool LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, CDiskB if (mapBlockIndex.count(hash) == 0 || (mapBlockIndex[hash]->nStatus & BLOCK_HAVE_DATA) == 0) { LOCK(cs_main); CValidationState state; - if (AcceptBlock(block, state, chainparams, NULL, true, dbp, NULL)) + if (AcceptBlock(pblock, state, chainparams, NULL, true, dbp, NULL)) nLoaded++; if (state.IsError()) break; @@ -4284,16 +4287,17 @@ bool LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, CDiskB std::pair::iterator, std::multimap::iterator> range = mapBlocksUnknownParent.equal_range(head); while (range.first != range.second) { std::multimap::iterator it = range.first; - if (ReadBlockFromDisk(block, it->second, chainparams.GetConsensus())) + std::shared_ptr pblockrecursive = std::make_shared(); + if (ReadBlockFromDisk(*pblockrecursive, it->second, chainparams.GetConsensus())) { - LogPrint("reindex", "%s: Processing out of order child %s of %s\n", __func__, block.GetHash().ToString(), + LogPrint("reindex", "%s: Processing out of order child %s of %s\n", __func__, pblockrecursive->GetHash().ToString(), head.ToString()); LOCK(cs_main); CValidationState dummy; - if (AcceptBlock(block, dummy, chainparams, NULL, true, &it->second, NULL)) + if (AcceptBlock(pblockrecursive, dummy, chainparams, NULL, true, &it->second, NULL)) { nLoaded++; - queue.push_back(block.GetHash()); + queue.push_back(pblockrecursive->GetHash()); } } range.first++; From 15a8fcf993b23fb5eaceb68e4c468da4833d178f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 18 Dec 2016 23:26:20 -0800 Subject: [PATCH 11/14] Add a CValidationInterface::NewPoWValidBlock callback --- src/validation.cpp | 5 +++++ src/validationinterface.cpp | 3 +++ src/validationinterface.h | 6 ++++++ 3 files changed, 14 insertions(+) diff --git a/src/validation.cpp b/src/validation.cpp index e0535e5276b9..52f9c082dfd6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3593,6 +3593,11 @@ static bool AcceptBlock(const std::shared_ptr& pblock, CValidation return error("%s: %s", __func__, FormatStateMessage(state)); } + // Header is valid/has work, merkle tree is good...RELAY NOW + // (but if it does not build on our best tip, let the SendMessages loop relay it) + if (!IsInitialBlockDownload() && chainActive.Tip() == pindex->pprev) + GetMainSignals().NewPoWValidBlock(pindex, pblock); + int nHeight = pindex->nHeight; // Write block to history file diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 7c97f2c5baa8..56620c929479 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -25,6 +25,7 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) { g_signals.BlockChecked.connect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2)); g_signals.ScriptForMining.connect(boost::bind(&CValidationInterface::GetScriptForMining, pwalletIn, _1)); g_signals.BlockFound.connect(boost::bind(&CValidationInterface::ResetRequestCount, pwalletIn, _1)); + g_signals.NewPoWValidBlock.connect(boost::bind(&CValidationInterface::NewPoWValidBlock, pwalletIn, _1, _2)); } void UnregisterValidationInterface(CValidationInterface* pwalletIn) { @@ -38,6 +39,7 @@ void UnregisterValidationInterface(CValidationInterface* pwalletIn) { g_signals.NotifyTransactionLock.disconnect(boost::bind(&CValidationInterface::NotifyTransactionLock, pwalletIn, _1)); g_signals.SyncTransaction.disconnect(boost::bind(&CValidationInterface::SyncTransaction, pwalletIn, _1, _2, _3)); g_signals.UpdatedBlockTip.disconnect(boost::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, _1, _2, _3)); + g_signals.NewPoWValidBlock.disconnect(boost::bind(&CValidationInterface::NewPoWValidBlock, pwalletIn, _1, _2)); g_signals.NotifyHeaderTip.disconnect(boost::bind(&CValidationInterface::NotifyHeaderTip, pwalletIn, _1, _2)); g_signals.AcceptedBlockHeader.disconnect(boost::bind(&CValidationInterface::AcceptedBlockHeader, pwalletIn, _1)); } @@ -53,6 +55,7 @@ void UnregisterAllValidationInterfaces() { g_signals.NotifyTransactionLock.disconnect_all_slots(); g_signals.SyncTransaction.disconnect_all_slots(); g_signals.UpdatedBlockTip.disconnect_all_slots(); + g_signals.NewPoWValidBlock.disconnect_all_slots(); g_signals.NotifyHeaderTip.disconnect_all_slots(); g_signals.AcceptedBlockHeader.disconnect_all_slots(); } diff --git a/src/validationinterface.h b/src/validationinterface.h index b0db718dbebb..360e4005c240 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -8,6 +8,7 @@ #include #include +#include class CBlock; class CBlockIndex; @@ -43,6 +44,7 @@ class CValidationInterface { virtual void BlockChecked(const CBlock&, const CValidationState&) {} virtual void GetScriptForMining(boost::shared_ptr&) {}; virtual void ResetRequestCount(const uint256 &hash) {}; + virtual void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr& block) {}; friend void ::RegisterValidationInterface(CValidationInterface*); friend void ::UnregisterValidationInterface(CValidationInterface*); friend void ::UnregisterAllValidationInterfaces(); @@ -82,6 +84,10 @@ struct CMainSignals { boost::signals2::signal&)> ScriptForMining; /** Notifies listeners that a block has been successfully mined */ boost::signals2::signal BlockFound; + /** + * Notifies listeners that a block which builds directly on our current tip + * has been received and connected to the headers tree, though not validated yet */ + boost::signals2::signal&)> NewPoWValidBlock; }; CMainSignals& GetMainSignals(); From 592d8f073411ff4dee6e443c01dabbd311d0aef5 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 11 Jan 2017 14:47:52 -0800 Subject: [PATCH 12/14] Use a temp pindex to avoid a const_cast in ProcessNewBlockHeaders --- src/validation.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 52f9c082dfd6..c319a71f6a04 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3532,11 +3532,13 @@ bool ProcessNewBlockHeaders(const std::vector& headers, CValidatio { LOCK(cs_main); for (const CBlockHeader& header : headers) { - // cast away the ppindex-returns-const CBlockIndex - we're just assigning it to a CBlockIndex* - // that we own and is updated non-const anyway - if (!AcceptBlockHeader(header, state, chainparams, const_cast(ppindex))) { + CBlockIndex *pindex = NULL; // Use a temp pindex instead of ppindex to avoid a const_cast + if (!AcceptBlockHeader(header, state, chainparams, &pindex)) { return false; } + if (ppindex) { + *ppindex = pindex; + } } } NotifyHeaderTip(); From 662ec024ab0e85ebe490749b41171f4b5a96f04e Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Thu, 8 Feb 2018 09:19:36 +0100 Subject: [PATCH 13/14] Make peer id logging consistent ("peer=%d" instead of "peer %d") --- src/masternode-payments.cpp | 10 +++++----- src/masternode-sync.cpp | 4 ++-- src/masternodeman.cpp | 6 +++--- src/net_processing.cpp | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/masternode-payments.cpp b/src/masternode-payments.cpp index 1503b730884d..f7ed617f7326 100644 --- a/src/masternode-payments.cpp +++ b/src/masternode-payments.cpp @@ -307,7 +307,7 @@ void CMasternodePayments::ProcessMessage(CNode* pfrom, const std::string& strCom netfulfilledman.AddFulfilledRequest(pfrom->addr, NetMsgType::MASTERNODEPAYMENTSYNC); Sync(pfrom, connman); - LogPrintf("MASTERNODEPAYMENTSYNC -- Sent Masternode payment votes to peer %d\n", pfrom->id); + LogPrintf("MASTERNODEPAYMENTSYNC -- Sent Masternode payment votes to peer=%d\n", pfrom->id); } else if (strCommand == NetMsgType::MASTERNODEPAYMENTVOTE) { // Masternode Payments Vote for the Winner @@ -888,7 +888,7 @@ void CMasternodePayments::Sync(CNode* pnode, CConnman& connman) } } - LogPrintf("CMasternodePayments::Sync -- Sent %d votes to peer %d\n", nInvCount, pnode->id); + LogPrintf("CMasternodePayments::Sync -- Sent %d votes to peer=%d\n", nInvCount, pnode->id); CNetMsgMaker msgMaker(pnode->GetSendVersion()); connman.PushMessage(pnode, msgMaker.Make(NetMsgType::SYNCSTATUSCOUNT, MASTERNODE_SYNC_MNW, nInvCount)); } @@ -912,7 +912,7 @@ void CMasternodePayments::RequestLowDataPaymentBlocks(CNode* pnode, CConnman& co vToFetch.push_back(CInv(MSG_MASTERNODE_PAYMENT_BLOCK, pindex->GetBlockHash())); // We should not violate GETDATA rules if(vToFetch.size() == MAX_INV_SZ) { - LogPrintf("CMasternodePayments::SyncLowDataPaymentBlocks -- asking peer %d for %d blocks\n", pnode->id, MAX_INV_SZ); + LogPrintf("CMasternodePayments::SyncLowDataPaymentBlocks -- asking peer=%d for %d blocks\n", pnode->id, MAX_INV_SZ); connman.PushMessage(pnode, msgMaker.Make(NetMsgType::GETDATA, vToFetch)); // Start filling new batch vToFetch.clear(); @@ -960,7 +960,7 @@ void CMasternodePayments::RequestLowDataPaymentBlocks(CNode* pnode, CConnman& co } // We should not violate GETDATA rules if(vToFetch.size() == MAX_INV_SZ) { - LogPrintf("CMasternodePayments::SyncLowDataPaymentBlocks -- asking peer %d for %d payment blocks\n", pnode->id, MAX_INV_SZ); + LogPrintf("CMasternodePayments::SyncLowDataPaymentBlocks -- asking peer=%d for %d payment blocks\n", pnode->id, MAX_INV_SZ); connman.PushMessage(pnode, msgMaker.Make(NetMsgType::GETDATA, vToFetch)); // Start filling new batch vToFetch.clear(); @@ -969,7 +969,7 @@ void CMasternodePayments::RequestLowDataPaymentBlocks(CNode* pnode, CConnman& co } // Ask for the rest of it if(!vToFetch.empty()) { - LogPrintf("CMasternodePayments::SyncLowDataPaymentBlocks -- asking peer %d for %d payment blocks\n", pnode->id, vToFetch.size()); + LogPrintf("CMasternodePayments::SyncLowDataPaymentBlocks -- asking peer=%d for %d payment blocks\n", pnode->id, vToFetch.size()); connman.PushMessage(pnode, msgMaker.Make(NetMsgType::GETDATA, vToFetch)); } } diff --git a/src/masternode-sync.cpp b/src/masternode-sync.cpp index e42f575770ab..8c312fa698e9 100644 --- a/src/masternode-sync.cpp +++ b/src/masternode-sync.cpp @@ -227,7 +227,7 @@ void CMasternodeSync::ProcessTick(CConnman& connman) // We already fully synced from this node recently, // disconnect to free this connection slot for another peer. pnode->fDisconnect = true; - LogPrintf("CMasternodeSync::ProcessTick -- disconnecting from recently synced peer %d\n", pnode->id); + LogPrintf("CMasternodeSync::ProcessTick -- disconnecting from recently synced peer=%d\n", pnode->id); continue; } @@ -238,7 +238,7 @@ void CMasternodeSync::ProcessTick(CConnman& connman) netfulfilledman.AddFulfilledRequest(pnode->addr, "spork-sync"); // get current network sporks connman.PushMessage(pnode, msgMaker.Make(NetMsgType::GETSPORKS)); - LogPrintf("CMasternodeSync::ProcessTick -- nTick %d nRequestedMasternodeAssets %d -- requesting sporks from peer %d\n", nTick, nRequestedMasternodeAssets, pnode->id); + LogPrintf("CMasternodeSync::ProcessTick -- nTick %d nRequestedMasternodeAssets %d -- requesting sporks from peer=%d\n", nTick, nRequestedMasternodeAssets, pnode->id); } // INITIAL TIMEOUT diff --git a/src/masternodeman.cpp b/src/masternodeman.cpp index 9518a26be15b..5f522f1e2a89 100644 --- a/src/masternodeman.cpp +++ b/src/masternodeman.cpp @@ -909,18 +909,18 @@ void CMasternodeMan::ProcessMessage(CNode* pfrom, const std::string& strCommand, mapSeenMasternodePing.insert(std::make_pair(hashMNP, mnp)); if (vin.prevout == mnpair.first) { - LogPrintf("DSEG -- Sent 1 Masternode inv to peer %d\n", pfrom->id); + LogPrintf("DSEG -- Sent 1 Masternode inv to peer=%d\n", pfrom->id); return; } } if(vin == CTxIn()) { connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::SYNCSTATUSCOUNT, MASTERNODE_SYNC_LIST, nInvCount)); - LogPrintf("DSEG -- Sent %d Masternode invs to peer %d\n", nInvCount, pfrom->id); + LogPrintf("DSEG -- Sent %d Masternode invs to peer=%d\n", nInvCount, pfrom->id); return; } // smth weird happen - someone asked us for vin we have no idea about? - LogPrint("masternode", "DSEG -- No invs sent to peer %d\n", pfrom->id); + LogPrint("masternode", "DSEG -- No invs sent to peer=%d\n", pfrom->id); } else if (strCommand == NetMsgType::MNVERIFY) { // Masternode Verify diff --git a/src/net_processing.cpp b/src/net_processing.cpp index e87eb65fda3b..5028cf472349 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -586,7 +586,7 @@ void EraseOrphansFor(NodeId peer) nErased += EraseOrphanTx(maybeErase->second.tx->GetHash()); } } - if (nErased > 0) LogPrint("mempool", "Erased %d orphan tx from peer %d\n", nErased, peer); + if (nErased > 0) LogPrint("mempool", "Erased %d orphan tx from peer=%d\n", nErased, peer); } From 4ac4e96e8fb17037cd19cd9de79738e7ca9f9d83 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 16 Feb 2017 10:22:17 +0100 Subject: [PATCH 14/14] Merge #9765: Harden against mistakes handling invalid blocks ba803ef Harden against mistakes handling invalid blocks (Suhas Daftuar) --- qa/rpc-tests/p2p-fullblocktest.py | 4 ++-- src/validation.cpp | 16 +++++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/qa/rpc-tests/p2p-fullblocktest.py b/qa/rpc-tests/p2p-fullblocktest.py index dd35aeb53c8e..86eac83a07f3 100755 --- a/qa/rpc-tests/p2p-fullblocktest.py +++ b/qa/rpc-tests/p2p-fullblocktest.py @@ -399,7 +399,7 @@ def update_block(block_number, new_transactions): # Extend the b26 chain to make sure bitcoind isn't accepting b26 b27 = block(27, spend=out[7]) - yield rejected(RejectResult(16, b'bad-prevblk')) + yield rejected(RejectResult(0, b'bad-prevblk')) # Now try a too-large-coinbase script tip(15) @@ -411,7 +411,7 @@ def update_block(block_number, new_transactions): # Extend the b28 chain to make sure bitcoind isn't accepting b28 b29 = block(29, spend=out[7]) - yield rejected(RejectResult(16, b'bad-prevblk')) + yield rejected(RejectResult(0, b'bad-prevblk')) # b30 has a max-sized coinbase scriptSig. tip(23) diff --git a/src/validation.cpp b/src/validation.cpp index c319a71f6a04..ba97cb2c80a6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3586,7 +3586,7 @@ static bool AcceptBlock(const std::shared_ptr& pblock, CValidation } if (fNewBlock) *fNewBlock = true; - if (!CheckBlock(block, state, chainparams.GetConsensus(), GetAdjustedTime()) || + if (!CheckBlock(block, state, chainparams.GetConsensus()) || !ContextualCheckBlock(block, state, chainparams.GetConsensus(), pindex->pprev)) { if (state.IsInvalid() && !state.CorruptionPossible()) { pindex->nStatus |= BLOCK_FAILED_VALID; @@ -3628,13 +3628,19 @@ static bool AcceptBlock(const std::shared_ptr& pblock, CValidation bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr pblock, bool fForceProcessing, bool *fNewBlock) { { - LOCK(cs_main); - - // Store to disk CBlockIndex *pindex = NULL; if (fNewBlock) *fNewBlock = false; CValidationState state; - bool ret = AcceptBlock(pblock, state, chainparams, &pindex, fForceProcessing, NULL, fNewBlock); + // Ensure that CheckBlock() passes before calling AcceptBlock, as + // belt-and-suspenders. + bool ret = CheckBlock(*pblock, state, chainparams.GetConsensus()); + + LOCK(cs_main); + + if (ret) { + // Store to disk + ret = AcceptBlock(pblock, state, chainparams, &pindex, fForceProcessing, NULL, fNewBlock); + } CheckBlockIndex(chainparams.GetConsensus()); if (!ret) { GetMainSignals().BlockChecked(*pblock, state);