From 87e7d7280739cccfabaffbbfbbbcfa21e943da3a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 30 Sep 2016 17:40:03 -0400 Subject: [PATCH 1/8] Make validationinterface.UpdatedBlockTip more verbose In anticipation of making all the callbacks out of block processing flow through it. Note that vHashes will always have something in it since pindexFork != pindexNewTip. --- src/main.cpp | 6 ++---- src/validationinterface.cpp | 4 ++-- src/validationinterface.h | 4 ++-- src/zmq/zmqnotificationinterface.cpp | 7 +++++-- src/zmq/zmqnotificationinterface.h | 2 +- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index ab6721971..37a0a2f30 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3099,11 +3099,9 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, } }); } - // Notify external listeners about the new tip. - if (!vHashes.empty()) { - GetMainSignals().UpdatedBlockTip(pindexNewTip); - } } + // Notify external listeners about the new tip. + GetMainSignals().UpdatedBlockTip(pindexNewTip, pindexFork, fInitialDownload); } } while (pindexNewTip != pindexMostWork); CheckBlockIndex(chainparams.GetConsensus()); diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 6ddf37658..d0aa7b5f3 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -13,7 +13,7 @@ CMainSignals& GetMainSignals() } void RegisterValidationInterface(CValidationInterface* pwalletIn) { - g_signals.UpdatedBlockTip.connect(boost::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, _1)); + g_signals.UpdatedBlockTip.connect(boost::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, _1, _2, _3)); g_signals.SyncTransaction.connect(boost::bind(&CValidationInterface::SyncTransaction, pwalletIn, _1, _2, _3)); g_signals.UpdatedTransaction.connect(boost::bind(&CValidationInterface::UpdatedTransaction, pwalletIn, _1)); g_signals.SetBestChain.connect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1)); @@ -33,7 +33,7 @@ void UnregisterValidationInterface(CValidationInterface* pwalletIn) { g_signals.SetBestChain.disconnect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1)); g_signals.UpdatedTransaction.disconnect(boost::bind(&CValidationInterface::UpdatedTransaction, pwalletIn, _1)); g_signals.SyncTransaction.disconnect(boost::bind(&CValidationInterface::SyncTransaction, pwalletIn, _1, _2, _3)); - g_signals.UpdatedBlockTip.disconnect(boost::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, _1)); + g_signals.UpdatedBlockTip.disconnect(boost::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, _1, _2, _3)); } void UnregisterAllValidationInterfaces() { diff --git a/src/validationinterface.h b/src/validationinterface.h index 0c91ec830..683f8fe20 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -33,7 +33,7 @@ void SyncWithWallets(const CTransaction& tx, const CBlockIndex *pindex, int posI class CValidationInterface { protected: - virtual void UpdatedBlockTip(const CBlockIndex *pindex) {} + virtual void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) {} virtual void SyncTransaction(const CTransaction &tx, const CBlockIndex *pindex, int posInBlock) {} virtual void SetBestChain(const CBlockLocator &locator) {} virtual void UpdatedTransaction(const uint256 &hash) {} @@ -49,7 +49,7 @@ protected: struct CMainSignals { /** Notifies listeners of updated block chain tip */ - boost::signals2::signal UpdatedBlockTip; + boost::signals2::signal UpdatedBlockTip; /** Notifies listeners of updated transaction data (transaction, and optionally the block it is found in. */ boost::signals2::signal SyncTransaction; /** Notifies listeners of an updated transaction without new data (for now: a coinbase potentially becoming visible). */ diff --git a/src/zmq/zmqnotificationinterface.cpp b/src/zmq/zmqnotificationinterface.cpp index 376e7dec5..020cdfbdc 100644 --- a/src/zmq/zmqnotificationinterface.cpp +++ b/src/zmq/zmqnotificationinterface.cpp @@ -124,12 +124,15 @@ void CZMQNotificationInterface::Shutdown() } } -void CZMQNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindex) +void CZMQNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) { + if (fInitialDownload) + return; + for (std::list::iterator i = notifiers.begin(); i!=notifiers.end(); ) { CZMQAbstractNotifier *notifier = *i; - if (notifier->NotifyBlock(pindex)) + if (notifier->NotifyBlock(pindexNew)) { i++; } diff --git a/src/zmq/zmqnotificationinterface.h b/src/zmq/zmqnotificationinterface.h index a85344726..037470ec1 100644 --- a/src/zmq/zmqnotificationinterface.h +++ b/src/zmq/zmqnotificationinterface.h @@ -25,7 +25,7 @@ protected: // CValidationInterface void SyncTransaction(const CTransaction& tx, const CBlockIndex *pindex, int posInBlock); - void UpdatedBlockTip(const CBlockIndex *pindex); + void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload); private: CZMQNotificationInterface(); From 0278fb5f48ae9e42ec0772f85e201051077f633c Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 30 Sep 2016 18:19:57 -0400 Subject: [PATCH 2/8] Remove duplicate nBlocksEstimate cmp (we already checked IsIBD()) --- src/main.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 37a0a2f30..2eb641e54 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3087,12 +3087,9 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, } } // Relay inventory, but don't relay old inventory during initial block download. - int nBlockEstimate = 0; - if (fCheckpointsEnabled) - nBlockEstimate = Checkpoints::GetTotalBlocksEstimate(chainparams.Checkpoints()); if(connman) { - connman->ForEachNode([nNewHeight, nBlockEstimate, &vHashes](CNode* pnode) { - if (nNewHeight > (pnode->nStartingHeight != -1 ? pnode->nStartingHeight - 2000 : nBlockEstimate)) { + connman->ForEachNode([nNewHeight, &vHashes](CNode* pnode) { + if (nNewHeight > (pnode->nStartingHeight != -1 ? pnode->nStartingHeight - 2000 : 0)) { BOOST_REVERSE_FOREACH(const uint256& hash, vHashes) { pnode->PushBlockHash(hash); } From aefcb7b70c923ccd341329a2d5e22238dc14ac3b Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 4 Oct 2016 13:36:11 -0400 Subject: [PATCH 3/8] Move net-processing logic definitions together in main.h --- src/main.h | 56 +++++++++++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/src/main.h b/src/main.h index 2646d8f86..3c587569f 100644 --- a/src/main.h +++ b/src/main.h @@ -41,7 +41,6 @@ class CValidationInterface; class CValidationState; struct PrecomputedTransactionData; -struct CNodeStateStats; struct LockPoints; /** Default for DEFAULT_WHITELISTRELAY. */ @@ -206,11 +205,6 @@ static const unsigned int DEFAULT_CHECKLEVEL = 3; // Setting the target to > than 550MB will make it likely we can respect the target. static const uint64_t MIN_DISK_SPACE_FOR_BLOCK_FILES = 550 * 1024 * 1024; -/** Register with a network node to receive its signals */ -void RegisterNodeSignals(CNodeSignals& nodeSignals); -/** Unregister a network node */ -void UnregisterNodeSignals(CNodeSignals& nodeSignals); - /** * Process an incoming block. This only returns after the best known valid * block is made active. Note that it does not, however, guarantee that the @@ -240,15 +234,6 @@ bool InitBlockIndex(const CChainParams& chainparams); bool LoadBlockIndex(); /** Unload database information */ void UnloadBlockIndex(); -/** Process protocol messages received from a given node */ -bool ProcessMessages(CNode* pfrom, CConnman& connman); -/** - * Send queued protocol messages to be sent to a give node. - * - * @param[in] pto The node which we are sending messages to. - * @param[in] connman The connection manager for that node. - */ -bool SendMessages(CNode* pto, CConnman& connman); /** Run an instance of the script checking thread */ void ThreadScriptCheck(); /** Check whether we are doing an initial block download (synchronizing from disk or network) */ @@ -291,10 +276,6 @@ void UnlinkPrunedFiles(std::set& setFilesToPrune); /** Create a new block index entry for a given block hash */ CBlockIndex * InsertBlockIndex(uint256 hash); -/** Get statistics from node state */ -bool GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats); -/** Increase a node's misbehavior score. */ -void Misbehaving(NodeId nodeid, int howmuch); /** Flush all state, indexes and buffers to disk. */ void FlushStateToDisk(); /** Prune block files and flush state to disk. */ @@ -310,13 +291,6 @@ std::string FormatStateMessage(const CValidationState &state); /** Get the BIP9 state for a given deployment at the current tip. */ ThresholdState VersionBitsTipState(const Consensus::Params& params, Consensus::DeploymentPos pos); -struct CNodeStateStats { - int nMisbehavior; - int nSyncHeight; - int nCommonHeight; - std::vector vHeightInFlight; -}; - /** @@ -545,4 +519,34 @@ static const unsigned int REJECT_ALREADY_KNOWN = 0x101; /** Transaction conflicts with a transaction already known */ static const unsigned int REJECT_CONFLICT = 0x102; +// The following things handle network-processing logic +// (and should be moved to a separate file) + +/** Register with a network node to receive its signals */ +void RegisterNodeSignals(CNodeSignals& nodeSignals); +/** Unregister a network node */ +void UnregisterNodeSignals(CNodeSignals& nodeSignals); + +struct CNodeStateStats { + int nMisbehavior; + int nSyncHeight; + int nCommonHeight; + std::vector vHeightInFlight; +}; + +/** Get statistics from node state */ +bool GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats); +/** Increase a node's misbehavior score. */ +void Misbehaving(NodeId nodeid, int howmuch); + +/** Process protocol messages received from a given node */ +bool ProcessMessages(CNode* pfrom, CConnman& connman); +/** + * Send queued protocol messages to be sent to a give node. + * + * @param[in] pto The node which we are sending messages to. + * @param[in] connman The connection manager for that node. + */ +bool SendMessages(CNode* pto, CConnman& connman); + #endif // BITCOIN_MAIN_H From fef1010199b70026fd6d56ebac5c277552757307 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 4 Oct 2016 13:49:44 -0400 Subject: [PATCH 4/8] Use CValidationInterface from chain logic to notify peer logic This adds a new CValidationInterface subclass, defined in main.h, to receive notifications of UpdatedBlockTip and use that to push blocks to peers, instead of doing it directly from ActivateBestChain. --- src/init.cpp | 5 +++++ src/main.cpp | 56 ++++++++++++++++++++++++++++++---------------------- src/main.h | 11 +++++++++++ 3 files changed, 48 insertions(+), 24 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index eefef7ba0..7045b3e6e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -72,6 +72,7 @@ static const bool DEFAULT_DISABLE_SAFEMODE = false; static const bool DEFAULT_STOPAFTERBLOCKIMPORT = false; std::unique_ptr g_connman; +std::unique_ptr peerLogic; #if ENABLE_ZMQ static CZMQNotificationInterface* pzmqNotificationInterface = NULL; @@ -200,6 +201,8 @@ void Shutdown() pwalletMain->Flush(false); #endif MapPort(false); + UnregisterValidationInterface(peerLogic.get()); + peerLogic.reset(); g_connman.reset(); StopTorControl(); @@ -1102,6 +1105,8 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) g_connman = std::unique_ptr(new CConnman(GetRand(std::numeric_limits::max()), GetRand(std::numeric_limits::max()))); CConnman& connman = *g_connman; + peerLogic.reset(new PeerLogicValidation(&connman)); + RegisterValidationInterface(peerLogic.get()); RegisterNodeSignals(GetNodeSignals()); // sanitize comments per BIP-0014, format user agent and check total size diff --git a/src/main.cpp b/src/main.cpp index 2eb641e54..e042a7382 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3073,30 +3073,6 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, if (pindexFork != pindexNewTip) { uiInterface.NotifyBlockTip(fInitialDownload, pindexNewTip); - if (!fInitialDownload) { - // Find the hashes of all blocks that weren't previously in the best chain. - std::vector vHashes; - CBlockIndex *pindexToAnnounce = pindexNewTip; - while (pindexToAnnounce != pindexFork) { - vHashes.push_back(pindexToAnnounce->GetBlockHash()); - pindexToAnnounce = pindexToAnnounce->pprev; - if (vHashes.size() == MAX_BLOCKS_TO_ANNOUNCE) { - // Limit announcements in case of a huge reorganization. - // Rely on the peer's synchronization mechanism in that case. - break; - } - } - // Relay inventory, but don't relay old inventory during initial block download. - if(connman) { - connman->ForEachNode([nNewHeight, &vHashes](CNode* pnode) { - if (nNewHeight > (pnode->nStartingHeight != -1 ? pnode->nStartingHeight - 2000 : 0)) { - BOOST_REVERSE_FOREACH(const uint256& hash, vHashes) { - pnode->PushBlockHash(hash); - } - } - }); - } - } // Notify external listeners about the new tip. GetMainSignals().UpdatedBlockTip(pindexNewTip, pindexFork, fInitialDownload); } @@ -4682,6 +4658,38 @@ std::string GetWarnings(const std::string& strFor) +////////////////////////////////////////////////////////////////////////////// +// +// blockchain -> download logic notification +// + +void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) { + const int nNewHeight = pindexNew->nHeight; + + if (!fInitialDownload) { + // Find the hashes of all blocks that weren't previously in the best chain. + std::vector vHashes; + const CBlockIndex *pindexToAnnounce = pindexNew; + while (pindexToAnnounce != pindexFork) { + vHashes.push_back(pindexToAnnounce->GetBlockHash()); + pindexToAnnounce = pindexToAnnounce->pprev; + if (vHashes.size() == MAX_BLOCKS_TO_ANNOUNCE) { + // Limit announcements in case of a huge reorganization. + // Rely on the peer's synchronization mechanism in that case. + break; + } + } + // Relay inventory, but don't relay old inventory during initial block download. + connman->ForEachNode([nNewHeight, &vHashes](CNode* pnode) { + if (nNewHeight > (pnode->nStartingHeight != -1 ? pnode->nStartingHeight - 2000 : 0)) { + BOOST_REVERSE_FOREACH(const uint256& hash, vHashes) { + pnode->PushBlockHash(hash); + } + } + }); + } +} + ////////////////////////////////////////////////////////////////////////////// // // Messages diff --git a/src/main.h b/src/main.h index 3c587569f..bb5e26b0b 100644 --- a/src/main.h +++ b/src/main.h @@ -16,6 +16,7 @@ #include "net.h" #include "script/script_error.h" #include "sync.h" +#include "validationinterface.h" #include "versionbits.h" #include @@ -527,6 +528,16 @@ void RegisterNodeSignals(CNodeSignals& nodeSignals); /** Unregister a network node */ void UnregisterNodeSignals(CNodeSignals& nodeSignals); +class PeerLogicValidation : public CValidationInterface { +private: + CConnman* connman; + +public: + PeerLogicValidation(CConnman* connmanIn) : connman(connmanIn) {} + + virtual void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload); +}; + struct CNodeStateStats { int nMisbehavior; int nSyncHeight; From f5efa283931ce1d52c59234b58988a221d42ecb4 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 30 Sep 2016 18:38:05 -0400 Subject: [PATCH 5/8] Remove CConnman parameter from ProcessNewBlock/ActivateBestChain --- src/main.cpp | 10 +++++----- src/main.h | 4 ++-- src/rpc/blockchain.cpp | 4 ++-- src/rpc/mining.cpp | 4 ++-- src/test/miner_tests.cpp | 2 +- src/test/test_bitcoin.cpp | 2 +- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index e042a7382..4ad7348db 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3013,7 +3013,7 @@ static void NotifyHeaderTip() { * or an activated best chain. pblock is either NULL or a pointer to a block * that is already loaded (to avoid loading it again from disk). */ -bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, const CBlock *pblock, CConnman* connman) { +bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, const CBlock *pblock) { CBlockIndex *pindexMostWork = NULL; CBlockIndex *pindexNewTip = NULL; std::vector> txChanged; @@ -3703,7 +3703,7 @@ static bool AcceptBlock(const CBlock& block, CValidationState& state, const CCha return true; } -bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp, CConnman* connman) +bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp) { { LOCK(cs_main); @@ -3725,7 +3725,7 @@ bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, C NotifyHeaderTip(); - if (!ActivateBestChain(state, chainparams, pblock, connman)) + if (!ActivateBestChain(state, chainparams, pblock)) return error("%s: ActivateBestChain failed", __func__); return true; @@ -5764,7 +5764,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, pfrom->PushMessage(NetMsgType::GETDATA, invs); } else { CValidationState state; - ProcessNewBlock(state, chainparams, pfrom, &block, false, NULL, &connman); + ProcessNewBlock(state, chainparams, pfrom, &block, false, NULL); int nDoS; if (state.IsInvalid(nDoS)) { assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes @@ -5940,7 +5940,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, // Such an unrequested block may still be processed, subject to the // conditions in AcceptBlock(). bool forceProcessing = pfrom->fWhitelisted && !IsInitialBlockDownload(); - ProcessNewBlock(state, chainparams, pfrom, &block, forceProcessing, NULL, &connman); + ProcessNewBlock(state, chainparams, pfrom, &block, forceProcessing, NULL); int nDoS; if (state.IsInvalid(nDoS)) { assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes diff --git a/src/main.h b/src/main.h index bb5e26b0b..77179b25b 100644 --- a/src/main.h +++ b/src/main.h @@ -218,7 +218,7 @@ static const uint64_t MIN_DISK_SPACE_FOR_BLOCK_FILES = 550 * 1024 * 1024; * @param[out] dbp The already known disk position of pblock, or NULL if not yet stored. * @return True if state.IsValid() */ -bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp, CConnman* connman); +bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp); /** Check whether enough disk space is available for an incoming block */ bool CheckDiskSpace(uint64_t nAdditionalBytes = 0); /** Open a block file (blk?????.dat) */ @@ -250,7 +250,7 @@ std::string GetWarnings(const std::string& strFor); /** Retrieve a transaction (from memory pool, or from disk, if possible) */ bool GetTransaction(const uint256 &hash, CTransaction &tx, const Consensus::Params& params, uint256 &hashBlock, bool fAllowSlow = false); /** Find the best known block, and make it the tip of the block chain */ -bool ActivateBestChain(CValidationState& state, const CChainParams& chainparams, const CBlock* pblock = NULL, CConnman* connman = NULL); +bool ActivateBestChain(CValidationState& state, const CChainParams& chainparams, const CBlock* pblock = NULL); CAmount GetBlockSubsidy(int nHeight, const Consensus::Params& consensusParams); /** diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 41d862934..d85f6fed6 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1279,7 +1279,7 @@ UniValue invalidateblock(const UniValue& params, bool fHelp) } if (state.IsValid()) { - ActivateBestChain(state, Params(), NULL, g_connman.get()); + ActivateBestChain(state, Params(), NULL); } if (!state.IsValid()) { @@ -1317,7 +1317,7 @@ UniValue reconsiderblock(const UniValue& params, bool fHelp) } CValidationState state; - ActivateBestChain(state, Params(), NULL, g_connman.get()); + ActivateBestChain(state, Params(), NULL); if (!state.IsValid()) { throw JSONRPCError(RPC_DATABASE_ERROR, state.GetRejectReason()); diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 6b13aa5ba..a54931720 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -131,7 +131,7 @@ UniValue generateBlocks(boost::shared_ptr coinbaseScript, int nG continue; } CValidationState state; - if (!ProcessNewBlock(state, Params(), NULL, pblock, true, NULL, g_connman.get())) + if (!ProcessNewBlock(state, Params(), NULL, pblock, true, NULL)) throw JSONRPCError(RPC_INTERNAL_ERROR, "ProcessNewBlock, block not accepted"); ++nHeight; blockHashes.push_back(pblock->GetHash().GetHex()); @@ -757,7 +757,7 @@ UniValue submitblock(const UniValue& params, bool fHelp) CValidationState state; submitblock_StateCatcher sc(block.GetHash()); RegisterValidationInterface(&sc); - bool fAccepted = ProcessNewBlock(state, Params(), NULL, &block, true, NULL, g_connman.get()); + bool fAccepted = ProcessNewBlock(state, Params(), NULL, &block, true, NULL); UnregisterValidationInterface(&sc); if (fBlockPresent) { diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index d3aa2364d..15fceb963 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -222,7 +222,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); pblock->nNonce = blockinfo[i].nonce; CValidationState state; - BOOST_CHECK(ProcessNewBlock(state, chainparams, NULL, pblock, true, NULL, connman)); + BOOST_CHECK(ProcessNewBlock(state, chainparams, NULL, pblock, true, NULL)); BOOST_CHECK(state.IsValid()); pblock->hashPrevBlock = pblock->GetHash(); } diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 02843d852..f36c54865 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -124,7 +124,7 @@ TestChain100Setup::CreateAndProcessBlock(const std::vector& while (!CheckProofOfWork(block.GetHash(), block.nBits, chainparams.GetConsensus())) ++block.nNonce; CValidationState state; - ProcessNewBlock(state, chainparams, NULL, &block, true, NULL, connman); + ProcessNewBlock(state, chainparams, NULL, &block, true, NULL); CBlock result = block; delete pblocktemplate; From 12ee1fe018e99bba6c2b74940ece3b39a45ed8d3 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 4 Oct 2016 13:52:57 -0400 Subject: [PATCH 6/8] Always call UpdatedBlockTip, even if blocks were only disconnected --- src/main.cpp | 11 ++++------- src/zmq/zmqnotificationinterface.cpp | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 4ad7348db..189c265ec 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3028,7 +3028,6 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, const CBlockIndex *pindexFork; std::list txConflicted; bool fInitialDownload; - int nNewHeight; { LOCK(cs_main); CBlockIndex *pindexOldTip = chainActive.Tip(); @@ -3051,13 +3050,10 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, pindexNewTip = chainActive.Tip(); pindexFork = chainActive.FindFork(pindexOldTip); fInitialDownload = IsInitialBlockDownload(); - nNewHeight = chainActive.Height(); } // When we reach this point, we switched to a new tip (stored in pindexNewTip). // Notifications/callbacks that can run without cs_main - if(connman) - connman->SetBestHeight(nNewHeight); // throw all transactions though the signal-interface // while _not_ holding the cs_main lock @@ -3069,12 +3065,12 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, for(unsigned int i = 0; i < txChanged.size(); i++) SyncWithWallets(std::get<0>(txChanged[i]), std::get<1>(txChanged[i]), std::get<2>(txChanged[i])); + // Notify external listeners about the new tip. + GetMainSignals().UpdatedBlockTip(pindexNewTip, pindexFork, fInitialDownload); + // Always notify the UI if a new block tip was connected if (pindexFork != pindexNewTip) { uiInterface.NotifyBlockTip(fInitialDownload, pindexNewTip); - - // Notify external listeners about the new tip. - GetMainSignals().UpdatedBlockTip(pindexNewTip, pindexFork, fInitialDownload); } } while (pindexNewTip != pindexMostWork); CheckBlockIndex(chainparams.GetConsensus()); @@ -4665,6 +4661,7 @@ std::string GetWarnings(const std::string& strFor) void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) { const int nNewHeight = pindexNew->nHeight; + connman->SetBestHeight(nNewHeight); if (!fInitialDownload) { // Find the hashes of all blocks that weren't previously in the best chain. diff --git a/src/zmq/zmqnotificationinterface.cpp b/src/zmq/zmqnotificationinterface.cpp index 020cdfbdc..a0196fe18 100644 --- a/src/zmq/zmqnotificationinterface.cpp +++ b/src/zmq/zmqnotificationinterface.cpp @@ -126,7 +126,7 @@ void CZMQNotificationInterface::Shutdown() void CZMQNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) { - if (fInitialDownload) + if (fInitialDownload || pindexNew == pindexFork) // In IBD or blocks were disconnected without any new ones return; for (std::list::iterator i = notifiers.begin(); i!=notifiers.end(); ) From 7565e03b962cfa8d8e54aa0f9068f41194915523 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 1 Oct 2016 15:29:40 -0400 Subject: [PATCH 7/8] Remove SyncWithWallets wrapper function --- src/main.cpp | 8 ++++---- src/validationinterface.cpp | 4 ---- src/validationinterface.h | 4 ++-- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 189c265ec..2f3528832 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1542,7 +1542,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C } } - SyncWithWallets(tx, NULL); + GetMainSignals().SyncTransaction(tx, NULL, CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK); return true; } @@ -2775,7 +2775,7 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara // Let wallets know transactions went from 1-confirmed to // 0-confirmed or conflicted: BOOST_FOREACH(const CTransaction &tx, block.vtx) { - SyncWithWallets(tx, pindexDelete->pprev); + GetMainSignals().SyncTransaction(tx, pindexDelete->pprev, CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK); } return true; } @@ -3059,11 +3059,11 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, // while _not_ holding the cs_main lock BOOST_FOREACH(const CTransaction &tx, txConflicted) { - SyncWithWallets(tx, pindexNewTip); + GetMainSignals().SyncTransaction(tx, pindexNewTip, CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK); } // ... and about transactions that got confirmed: for(unsigned int i = 0; i < txChanged.size(); i++) - SyncWithWallets(std::get<0>(txChanged[i]), std::get<1>(txChanged[i]), std::get<2>(txChanged[i])); + GetMainSignals().SyncTransaction(std::get<0>(txChanged[i]), std::get<1>(txChanged[i]), std::get<2>(txChanged[i])); // Notify external listeners about the new tip. GetMainSignals().UpdatedBlockTip(pindexNewTip, pindexFork, fInitialDownload); diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index d0aa7b5f3..085c336cc 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -47,7 +47,3 @@ void UnregisterAllValidationInterfaces() { g_signals.SyncTransaction.disconnect_all_slots(); g_signals.UpdatedBlockTip.disconnect_all_slots(); } - -void SyncWithWallets(const CTransaction &tx, const CBlockIndex *pindex, int posInBlock) { - g_signals.SyncTransaction(tx, pindex, posInBlock); -} diff --git a/src/validationinterface.h b/src/validationinterface.h index 683f8fe20..a29859999 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -28,8 +28,6 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn); void UnregisterValidationInterface(CValidationInterface* pwalletIn); /** Unregister all wallets from core */ void UnregisterAllValidationInterfaces(); -/** Push an updated transaction to all registered wallets */ -void SyncWithWallets(const CTransaction& tx, const CBlockIndex *pindex, int posInBlock = -1); class CValidationInterface { protected: @@ -50,6 +48,8 @@ protected: struct CMainSignals { /** Notifies listeners of updated block chain tip */ boost::signals2::signal UpdatedBlockTip; + /** A posInBlock value for SyncTransaction which indicates the transaction was conflicted, disconnected, or not in a block */ + static const int SYNC_TRANSACTION_NOT_IN_BLOCK = -1; /** Notifies listeners of updated transaction data (transaction, and optionally the block it is found in. */ boost::signals2::signal SyncTransaction; /** Notifies listeners of an updated transaction without new data (for now: a coinbase potentially becoming visible). */ From a9aec5c24d8c4efe9e1ede54e8b8039b4b3f0835 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 4 Oct 2016 13:54:29 -0400 Subject: [PATCH 8/8] Use BlockChecked signal to send reject messages from mapBlockSource --- src/main.cpp | 32 ++++++++++++++++++++------------ src/main.h | 1 + 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 2f3528832..a6a43699c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1857,17 +1857,6 @@ void static InvalidChainFound(CBlockIndex* pindexNew) } void static InvalidBlockFound(CBlockIndex *pindex, const CValidationState &state) { - int nDoS = 0; - if (state.IsInvalid(nDoS)) { - std::map::iterator it = mapBlockSource.find(pindex->GetBlockHash()); - if (it != mapBlockSource.end() && State(it->second)) { - assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes - CBlockReject reject = {(unsigned char)state.GetRejectCode(), state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), pindex->GetBlockHash()}; - State(it->second)->rejects.push_back(reject); - if (nDoS > 0) - Misbehaving(it->second, nDoS); - } - } if (!state.CorruptionPossible()) { pindex->nStatus |= BLOCK_FAILED_VALID; setDirtyBlockIndex.insert(pindex); @@ -2814,7 +2803,6 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, InvalidBlockFound(pindexNew, state); return error("ConnectTip(): ConnectBlock %s failed", pindexNew->GetBlockHash().ToString()); } - mapBlockSource.erase(pindexNew->GetBlockHash()); nTime3 = GetTimeMicros(); nTimeConnectTotal += nTime3 - nTime2; LogPrint("bench", " - Connect total: %.2fms [%.2fs]\n", (nTime3 - nTime2) * 0.001, nTimeConnectTotal * 0.000001); assert(view.Flush()); @@ -4687,6 +4675,26 @@ void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CB } } +void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationState& state) { + LOCK(cs_main); + + const uint256 hash(block.GetHash()); + std::map::iterator it = mapBlockSource.find(hash); + + int nDoS = 0; + if (state.IsInvalid(nDoS)) { + if (it != mapBlockSource.end() && State(it->second)) { + assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes + CBlockReject reject = {(unsigned char)state.GetRejectCode(), state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), hash}; + State(it->second)->rejects.push_back(reject); + if (nDoS > 0) + Misbehaving(it->second, nDoS); + } + } + if (it != mapBlockSource.end()) + mapBlockSource.erase(it); +} + ////////////////////////////////////////////////////////////////////////////// // // Messages diff --git a/src/main.h b/src/main.h index 77179b25b..889b6a2da 100644 --- a/src/main.h +++ b/src/main.h @@ -536,6 +536,7 @@ public: PeerLogicValidation(CConnman* connmanIn) : connman(connmanIn) {} virtual void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload); + virtual void BlockChecked(const CBlock& block, const CValidationState& state); }; struct CNodeStateStats {