diff --git a/CMakeLists.txt b/CMakeLists.txt index 46c3e6baf63b..676c005186d2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -418,6 +418,7 @@ set(UTIL_SOURCES ./src/arith_uint256.cpp ./src/uint256.cpp ./src/util/threadnames.cpp + ./src/util/blockstatecatcher.h ./src/util/system.cpp ./src/utilstrencodings.cpp ./src/utilmoneystr.cpp diff --git a/doc/release-notes.md b/doc/release-notes.md index 283a76a5aa38..0e7433fcda3b 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -493,6 +493,35 @@ OpenSSL is no longer used by PIVX Core P2P and network changes ----------------------- +#### Removal of reject network messages from PIVX Core (BIP61) + +The command line option to enable BIP61 (`-enablebip61`) has been removed. + +Nodes on the network can not generally be trusted to send valid ("reject") +messages, so this should only ever be used when connected to a trusted node. +Please use the recommended alternatives if you rely on this feature: + +* Testing or debugging of implementations of the PIVX P2P network protocol + should be done by inspecting the log messages that are produced by a recent + version of PIVX Core. Bitcoin Core logs debug messages + (`-debug=`) to a stream (`-printtoconsole`) or to a file + (`-debuglogfile=`). + +* Testing the validity of a block can be achieved by specific RPCs: + - `submitblock` + +* Testing the validity of a transaction can be achieved by specific RPCs: + - `sendrawtransaction` + +* Wallets should not use the absence of "reject" messages to indicate a + transaction has propagated the network, nor should wallets use "reject" + messages to set transaction fees. Wallets should rather use fee estimation + to determine transaction fees and set replace-by-fee if desired. Thus, they + could wait until the transaction has confirmed (taking into account the fee + target they set (compare the RPC `estimatesmartfee`)) or listen for the + transaction announcement by other network peers to check for propagation. + +#### NAT-PMP Support - Added NAT-PMP port mapping support via [`libnatpmp`](https://miniupnp.tuxfamily.org/libnatpmp.html) diff --git a/src/Makefile.am b/src/Makefile.am index f433e718da77..3831b986bd96 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -285,6 +285,7 @@ BITCOIN_CORE_H = \ guiinterfaceutil.h \ uint256.h \ undo.h \ + util/blockstatecatcher.h \ util/memory.h \ util/system.h \ util/macros.h \ diff --git a/src/blockassembler.cpp b/src/blockassembler.cpp index f31c15bc7c13..b5a373469d14 100644 --- a/src/blockassembler.cpp +++ b/src/blockassembler.cpp @@ -31,7 +31,6 @@ #include #include -#include // Unconfirmed transactions in the memory pool often depend on other // transactions in the memory pool. When we select transactions from the @@ -269,7 +268,7 @@ bool BlockAssembler::TestPackage(uint64_t packageSize, unsigned int packageSigOp // are final. bool BlockAssembler::TestPackageFinality(const CTxMemPool::setEntries& package) { - for (const CTxMemPool::txiter it : package) { + for (const CTxMemPool::txiter& it : package) { if (!IsFinalTx(it->GetSharedTx(), nHeight)) return false; } @@ -298,7 +297,7 @@ void BlockAssembler::AddToBlock(CTxMemPool::txiter iter) void BlockAssembler::UpdatePackagesForAdded(const CTxMemPool::setEntries& alreadyAdded, indexed_modified_transaction_set& mapModifiedTx) { - for (const CTxMemPool::txiter it : alreadyAdded) { + for (const CTxMemPool::txiter& it : alreadyAdded) { CTxMemPool::setEntries descendants; mempool.CalculateDescendants(it, descendants); // Insert all descendants (not yet in block) into the modified set @@ -457,22 +456,22 @@ void BlockAssembler::addPackageTxs() SortForBlock(ancestors, iter, sortedEntries); for (size_t i = 0; i < sortedEntries.size(); ++i) { - CTxMemPool::txiter& iter = sortedEntries[i]; - if (iter->IsShielded()) { + CTxMemPool::txiter& iterSortedEntries = sortedEntries[i]; + if (iterSortedEntries->IsShielded()) { // Don't add SHIELD transactions if in maintenance (SPORK_20) if (sporkManager.IsSporkActive(SPORK_20_SAPLING_MAINTENANCE)) { break; } // Don't add SHIELD transactions if there's no reserved space left in the block - if (nSizeShielded + iter->GetTxSize() > MAX_BLOCK_SHIELDED_TXES_SIZE) { + if (nSizeShielded + iterSortedEntries->GetTxSize() > MAX_BLOCK_SHIELDED_TXES_SIZE) { break; } // Update cumulative size of SHIELD transactions in this block - nSizeShielded += iter->GetTxSize(); + nSizeShielded += iterSortedEntries->GetTxSize(); } - AddToBlock(iter); + AddToBlock(iterSortedEntries); // Erase from the modified set, if present - mapModifiedTx.erase(iter); + mapModifiedTx.erase(iterSortedEntries); } // Update transactions that depend on each of these diff --git a/src/miner.cpp b/src/miner.cpp index 1173b7c3dbc3..a5d752024c64 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -12,7 +12,6 @@ #include "amount.h" #include "blockassembler.h" -#include "consensus/tx_verify.h" // needed in case of no ENABLE_WALLET #include "consensus/params.h" #include "masternode-sync.h" #include "net.h" @@ -20,12 +19,12 @@ #include "primitives/block.h" #include "primitives/transaction.h" #include "timedata.h" +#include "util/blockstatecatcher.h" #include "util/system.h" #include "utilmoneystr.h" #ifdef ENABLE_WALLET #include "wallet/wallet.h" #endif -#include "validationinterface.h" #include "invalid.h" #include "policy/policy.h" @@ -79,8 +78,10 @@ bool ProcessBlockFound(const std::shared_ptr& pblock, CWallet& wal reservekey->KeepKey(); // Process this block the same as if we had received it from another node - CValidationState state; - if (!ProcessNewBlock(state, pblock, nullptr)) { + BlockStateCatcher sc(pblock->GetHash()); + sc.registerEvent(); + bool res = ProcessNewBlock(pblock, nullptr); + if (!res || sc.stateErrorFound()) { return error("PIVXMiner : ProcessNewBlock, block not accepted"); } diff --git a/src/net.cpp b/src/net.cpp index f244fd9e336a..85f63fa67672 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -417,15 +417,13 @@ void CNode::CloseSocketDisconnect() } } -bool CNode::DisconnectOldProtocol(int nVersionIn, int nVersionRequired, std::string strLastCommand) +bool CNode::DisconnectOldProtocol(int nVersionIn, int nVersionRequired) { fDisconnect = false; if (nVersionIn < nVersionRequired) { LogPrintf("%s : peer=%d using obsolete version %i; disconnecting\n", __func__, id, nVersionIn); - g_connman->PushMessage(this, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strLastCommand, REJECT_OBSOLETE, strprintf("Version must be %d or greater", ActiveProtocol()))); fDisconnect = true; } - return fDisconnect; } diff --git a/src/net.h b/src/net.h index 480d9265179c..2659cfbc704e 100644 --- a/src/net.h +++ b/src/net.h @@ -777,7 +777,7 @@ class CNode } void CloseSocketDisconnect(); - bool DisconnectOldProtocol(int nVersionIn, int nVersionRequired, std::string strLastCommand = ""); + bool DisconnectOldProtocol(int nVersionIn, int nVersionRequired); void copyStats(CNodeStats& stats); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 0da0214e2769..8121f975153a 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -101,12 +101,6 @@ int nPreferredDownload = 0; namespace { -struct CBlockReject { - unsigned char chRejectCode; - std::string strRejectReason; - uint256 hashBlock; -}; - class CNodeBlocks { @@ -136,8 +130,7 @@ class CNodeBlocks // Compute the number of the received blocks size_t nBlocks = 0; - for(auto point : points) - { + for (auto point : points) { nBlocks += point.second; } @@ -148,8 +141,7 @@ class CNodeBlocks bool banNode = (nAvgValue >= 1.5 * maxAvg && size >= maxAvg) || (nAvgValue >= maxAvg && nBlocks >= maxSize) || (nBlocks >= maxSize * 3); - if(banNode) - { + if (banNode) { // Clear the points and ban the node points.clear(); return state.DoS(100, error("block-spam ban node for sending spam")); @@ -201,8 +193,6 @@ struct CNodeState { bool fShouldBan; //! String name of this peer (debugging/logging purposes). const std::string name; - //! List of asynchronously-determined block rejections to notify this peer about. - std::vector rejects; //! The best known block we know this peer has announced. const CBlockIndex* pindexBestKnownBlock; //! The hash of the last unknown block this peer has announced. @@ -610,8 +600,43 @@ void Misbehaving(NodeId pnode, int howmuch) EXCLUSIVE_LOCKS_REQUIRED(cs_main) LogPrintf("Misbehaving: %s (%d -> %d)\n", state->name, state->nMisbehavior - howmuch, state->nMisbehavior); } +static void CheckBlockSpam(NodeId nodeId, const uint256& hashBlock) +{ + // Block spam filtering + if (!gArgs.GetBoolArg("-blockspamfilter", DEFAULT_BLOCK_SPAM_FILTER)) { + return; + } + CNodeState* nodestate = nullptr; + int blockReceivedHeight = 0; + { + LOCK(cs_main); + nodestate = State(nodeId); + if (!nodestate) { return; } + + const auto it = mapBlockIndex.find(hashBlock); + if (it == mapBlockIndex.end()) { return; } + blockReceivedHeight = it->second->nHeight; + } + + nodestate->nodeBlocks.onBlockReceived(blockReceivedHeight); + bool nodeStatus = true; + // UpdateState will return false if the node is attacking us or update the score and return true. + CValidationState state; + nodeStatus = nodestate->nodeBlocks.updateState(state, nodeStatus); + int nDoS = 0; + if (state.IsInvalid(nDoS)) { + if (nDoS > 0) { + LOCK(cs_main); + Misbehaving(nodeId, nDoS); + } + nodeStatus = false; + } + if (!nodeStatus) { + LogPrintf("Block spam protection: %s\n", hashBlock.ToString()); + } +} ////////////////////////////////////////////////////////////////////////////// @@ -686,11 +711,12 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta 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); } + + // Spam filter + CheckBlockSpam(it->second, block.GetHash()); } } @@ -1046,40 +1072,6 @@ void static ProcessGetData(CNode* pfrom, CConnman* connman, const std::atomicGetId()); - if(!nodestate) { - return; - } - - const auto it = mapBlockIndex.find(hashBlock); - if (it == mapBlockIndex.end()) { - return; - } - - nodestate->nodeBlocks.onBlockReceived(it->second->nHeight); - bool nodeStatus = true; - // UpdateState will return false if the node is attacking us or update the score and return true. - nodeStatus = nodestate->nodeBlocks.updateState(state, nodeStatus); - int nDoS = 0; - if (state.IsInvalid(nDoS)) { - if (nDoS > 0) { - LOCK(cs_main); - Misbehaving(pfrom->GetId(), nDoS); - } - nodeStatus = false; - } - - if (!nodeStatus) { - LogPrintf("Block spam protection: %s\n", hashBlock.ToString()); - } -} - bool fRequestedSporksIDB = false; bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vRecv, int64_t nTimeReceived, CConnman* connman, std::atomic& interruptMsgProc) { @@ -1092,7 +1084,6 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR if (strCommand == NetMsgType::VERSION) { // Each connection can only send one version message if (pfrom->nVersion != 0) { - connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_DUPLICATE, std::string("Duplicate version message"))); LOCK(cs_main); Misbehaving(pfrom->GetId(), 1); return false; @@ -1117,14 +1108,13 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR } if (pfrom->nServicesExpected & ~nServices) { LogPrint(BCLog::NET, "peer=%d does not offer the expected services (%08x offered, %08x expected); disconnecting\n", pfrom->id, nServices, pfrom->nServicesExpected); - connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_NONSTANDARD, - strprintf("Expected to offer services %08x", pfrom->nServicesExpected))); pfrom->fDisconnect = true; return false; } - if (pfrom->DisconnectOldProtocol(nVersion, ActiveProtocol(), strCommand)) + if (pfrom->DisconnectOldProtocol(nVersion, ActiveProtocol())) { return false; + } if (nVersion == 10300) nVersion = 300; @@ -1619,11 +1609,9 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR LogPrint(BCLog::MEMPOOLREJ, "%s from peer=%d %s was not accepted into the memory pool: %s\n", tx.GetHash().ToString(), pfrom->id, pfrom->cleanSubVer, FormatStateMessage(state)); - if (state.GetRejectCode() < REJECT_INTERNAL) // Never send AcceptToMemoryPool's internal codes over P2P - connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::REJECT, strCommand, state.GetRejectCode(), - state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), inv.hash)); - if (nDoS > 0) + if (nDoS > 0) { Misbehaving(pfrom->GetId(), nDoS); + } } } @@ -1706,30 +1694,17 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR } } else { pfrom->AddInventoryKnown(inv); - CValidationState state; if (!mapBlockIndex.count(hashBlock)) { { LOCK(cs_main); MarkBlockAsReceived(hashBlock); mapBlockSource.emplace(hashBlock, pfrom->GetId()); } - bool fAccepted = true; - ProcessNewBlock(state, pblock, nullptr, &fAccepted); - if (!fAccepted) { - CheckBlockSpam(state, pfrom, hashBlock); - } - int nDoS; - if(state.IsInvalid(nDoS)) { - assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes - connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::REJECT, strCommand, state.GetRejectCode(), - state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), inv.hash)); - if(nDoS > 0) { - TRY_LOCK(cs_main, lockMain); - if(lockMain) Misbehaving(pfrom->GetId(), nDoS); - } - } - //disconnect this node if its old protocol version - pfrom->DisconnectOldProtocol(pfrom->nVersion, ActiveProtocol(), strCommand); + ProcessNewBlock(pblock, nullptr); + + // Disconnect node if its running an old protocol version, + // used during upgrades, when the node is already connected. + pfrom->DisconnectOldProtocol(pfrom->nVersion, ActiveProtocol()); } else { LogPrint(BCLog::NET, "%s : Already processed block %s, skipping ProcessNewBlock()\n", __func__, pblock->GetHash().GetHex()); } @@ -1899,41 +1874,13 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR pfrom->fRelayTxes = true; } - - else if (strCommand == NetMsgType::REJECT) { - try { - std::string strMsg; - unsigned char ccode; - std::string strReason; - vRecv >> LIMITED_STRING(strMsg, CMessageHeader::COMMAND_SIZE) >> ccode >> LIMITED_STRING(strReason, MAX_REJECT_MESSAGE_LENGTH); - - std::ostringstream ss; - ss << strMsg << " code " << itostr(ccode) << ": " << strReason; - - if (strMsg == NetMsgType::BLOCK || strMsg == NetMsgType::TX) { - uint256 hash; - vRecv >> hash; - ss << ": hash " << hash.ToString(); - } - LogPrint(BCLog::NET, "Reject %s\n", SanitizeString(ss.str())); - } catch (const std::ios_base::failure& e) { - // Avoid feedback loops by preventing reject messages from triggering a new reject message. - LogPrint(BCLog::NET, "Unparseable reject message received\n"); - } - } else { - bool found = false; - const std::vector& allMessages = getAllNetMessageTypes(); - for (const std::string& msg : allMessages) { - if (msg == strCommand) { - found = true; - break; - } - } - - if (found) { + else { + // Tier two msg type search + const std::vector& allMessages = getTierTwoNetMessageTypes(); + if (std::find(allMessages.begin(), allMessages.end(), strCommand) != allMessages.end()) { // Check if the dispatcher can process this message first. If not, try going with the old flow. if (!masternodeSync.MessageDispatcher(pfrom, strCommand, vRecv)) { - //probably one the extensions + // Probably one the extensions mnodeman.ProcessMessage(pfrom, strCommand, vRecv); g_budgetman.ProcessMessage(pfrom, strCommand, vRecv); masternodePayments.ProcessMessageMasternodePayments(pfrom, strCommand, vRecv); @@ -2027,7 +1974,6 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic& inter if (!pfrom->vRecvGetData.empty()) fMoreWork = true; } catch (const std::ios_base::failure& e) { - connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_MALFORMED, std::string("error parsing message"))); if (strstr(e.what(), "end of data")) { // Allow exceptions from under-length message on vRecv LogPrint(BCLog::NET, "ProcessMessages(%s, %u bytes): Exception '%s' caught, normally caused by a message being shorter than its stated length\n", SanitizeString(strCommand), nMessageSize, e.what()); @@ -2111,10 +2057,6 @@ bool PeerLogicValidation::SendMessages(CNode* pto, std::atomic& interruptM CNodeState& state = *State(pto->GetId()); - for (const CBlockReject& reject : state.rejects) - connman->PushMessage(pto, msgMaker.Make(NetMsgType::REJECT, std::string(NetMsgType::BLOCK), reject.chRejectCode, reject.strRejectReason, reject.hashBlock)); - state.rejects.clear(); - if (state.fShouldBan) { state.fShouldBan = false; if (pto->fWhitelisted) diff --git a/src/protocol.cpp b/src/protocol.cpp index f40fc79aed7e..4a4379618f5d 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -36,7 +36,6 @@ const char* NOTFOUND = "notfound"; const char* FILTERLOAD = "filterload"; const char* FILTERADD = "filteradd"; const char* FILTERCLEAR = "filterclear"; -const char* REJECT = "reject"; const char* SENDHEADERS = "sendheaders"; const char* SPORK = "spork"; const char* GETSPORKS = "getsporks"; @@ -53,26 +52,6 @@ const char* SYNCSTATUSCOUNT = "ssc"; const char* GETMNLIST = "dseg"; }; // namespace NetMsgType -static const char* ppszTypeName[] = { - "ERROR", // Should never occur - NetMsgType::TX, - NetMsgType::BLOCK, - "filtered block", // Should never occur - "ix", // deprecated - "txlvote", // deprecated - NetMsgType::SPORK, - NetMsgType::MNWINNER, - "mnodescanerr", - NetMsgType::BUDGETVOTE, - NetMsgType::BUDGETPROPOSAL, - NetMsgType::FINALBUDGET, - NetMsgType::FINALBUDGETVOTE, - "mnq", - NetMsgType::MNBROADCAST, - NetMsgType::MNPING, - "dstx" // deprecated -}; - /** All known message types. Keep this in the same order as the list of * messages above and in protocol.h. */ @@ -97,12 +76,11 @@ const static std::string allNetMessageTypes[] = { NetMsgType::FILTERLOAD, NetMsgType::FILTERADD, NetMsgType::FILTERCLEAR, - NetMsgType::REJECT, NetMsgType::SENDHEADERS, "filtered block", // Should never occur "ix", // deprecated "txlvote", // deprecated - NetMsgType::SPORK, + NetMsgType::SPORK, // --- tiertwoNetMessageTypes start here --- NetMsgType::MNWINNER, "mnodescanerr", NetMsgType::BUDGETVOTE, @@ -120,6 +98,7 @@ const static std::string allNetMessageTypes[] = { NetMsgType::SYNCSTATUSCOUNT }; const static std::vector allNetMessageTypesVec(allNetMessageTypes, allNetMessageTypes + ARRAYLEN(allNetMessageTypes)); +const static std::vector tiertwoNetMessageTypesVec(std::find(allNetMessageTypesVec.begin(), allNetMessageTypesVec.end(), NetMsgType::SPORK), allNetMessageTypesVec.end()); CMessageHeader::CMessageHeader(const MessageStartChars& pchMessageStartIn) { @@ -209,25 +188,36 @@ bool operator<(const CInv& a, const CInv& b) return (a.type < b.type || (a.type == b.type && a.hash < b.hash)); } -bool CInv::IsKnownType() const -{ - return (type >= 1 && type < (int)ARRAYLEN(ppszTypeName)); -} - bool CInv::IsMasterNodeType() const{ return type > 2; } -const char* CInv::GetCommand() const +std::string CInv::GetCommand() const { - if (!IsKnownType()) { - LogPrint(BCLog::NET, "CInv::GetCommand() : type=%d unknown type", type); - return "UNKNOWN"; + std::string cmd; + switch (type) { + case MSG_TX: return cmd.append(NetMsgType::TX); + case MSG_BLOCK: return cmd.append(NetMsgType::BLOCK); + case MSG_FILTERED_BLOCK: return cmd.append(NetMsgType::MERKLEBLOCK); + case MSG_TXLOCK_REQUEST: return cmd.append("ix"); // Deprecated + case MSG_TXLOCK_VOTE: return cmd.append("txlvote"); // Deprecated + case MSG_SPORK: return cmd.append(NetMsgType::SPORK); + case MSG_MASTERNODE_WINNER: return cmd.append(NetMsgType::MNWINNER); + case MSG_MASTERNODE_SCANNING_ERROR: return cmd.append("mnodescanerr"); // Deprecated + case MSG_BUDGET_VOTE: return cmd.append(NetMsgType::BUDGETVOTE); + case MSG_BUDGET_PROPOSAL: return cmd.append(NetMsgType::BUDGETPROPOSAL); + case MSG_BUDGET_FINALIZED: return cmd.append(NetMsgType::FINALBUDGET); + case MSG_BUDGET_FINALIZED_VOTE: return cmd.append(NetMsgType::FINALBUDGETVOTE); + case MSG_MASTERNODE_QUORUM: return cmd.append("mnq"); // Unused + case MSG_MASTERNODE_ANNOUNCE: return cmd.append(NetMsgType::MNBROADCAST); + case MSG_MASTERNODE_PING: return cmd.append(NetMsgType::MNPING); + case MSG_DSTX: return cmd.append("dstx"); // Deprecated + default: + throw std::out_of_range(strprintf("%s: type=%d unknown type", __func__, type)); } - - return ppszTypeName[type]; } + std::string CInv::ToString() const { return strprintf("%s %s", GetCommand(), hash.ToString()); @@ -237,3 +227,8 @@ const std::vector& getAllNetMessageTypes() { return allNetMessageTypesVec; } + +const std::vector& getTierTwoNetMessageTypes() +{ + return tiertwoNetMessageTypesVec; +} diff --git a/src/protocol.h b/src/protocol.h index 0c3143dc79f1..41848b75adc3 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -201,13 +201,6 @@ extern const char* FILTERADD; * @see https://bitcoin.org/en/developer-reference#filterclear */ extern const char* FILTERCLEAR; -/** - * The reject message informs the receiving node that one of its previous - * messages has been rejected. - * @since protocol version 70002 as described by BIP61. - * @see https://bitcoin.org/en/developer-reference#reject - */ -extern const char* REJECT; /** * Indicates that a node prefers to receive new block announcements via a * "headers" message rather than an "inv". @@ -274,6 +267,9 @@ extern const char* SYNCSTATUSCOUNT; /* Get a vector of all valid message types (see above) */ const std::vector& getAllNetMessageTypes(); +/* Get a vector of all tier two valid message types (see above) */ +const std::vector& getTierTwoNetMessageTypes(); + /** nServices flags */ enum ServiceFlags : uint64_t { // Nothing @@ -333,6 +329,31 @@ class CAddress : public CService unsigned int nTime; }; +/** getdata message types */ +enum GetDataMsg +{ + UNDEFINED = 0, + MSG_TX, + MSG_BLOCK, + // Nodes may always request a MSG_FILTERED_BLOCK in a getdata, however, + // MSG_FILTERED_BLOCK should not appear in any invs except as a part of getdata. + MSG_FILTERED_BLOCK, + MSG_TXLOCK_REQUEST, // Deprecated + MSG_TXLOCK_VOTE, // Deprecated + MSG_SPORK, + MSG_MASTERNODE_WINNER, + MSG_MASTERNODE_SCANNING_ERROR, + MSG_BUDGET_VOTE, + MSG_BUDGET_PROPOSAL, + MSG_BUDGET_FINALIZED, + MSG_BUDGET_FINALIZED_VOTE, + MSG_MASTERNODE_QUORUM, + MSG_MASTERNODE_ANNOUNCE, + MSG_MASTERNODE_PING, + MSG_DSTX, + MSG_TYPE_MAX = MSG_DSTX +}; + /** inv message data */ class CInv { @@ -344,38 +365,15 @@ class CInv friend bool operator<(const CInv& a, const CInv& b); - bool IsKnownType() const; bool IsMasterNodeType() const; std::string ToString() const; - // TODO: make private (improves encapsulation) -public: + // TODO: make private (improve encapsulation) int type; uint256 hash; private: - const char* GetCommand() const; -}; - -enum { - MSG_TX = 1, - MSG_BLOCK, - // Nodes may always request a MSG_FILTERED_BLOCK in a getdata, however, - // MSG_FILTERED_BLOCK should not appear in any invs except as a part of getdata. - MSG_FILTERED_BLOCK, - MSG_TXLOCK_REQUEST, // Deprecated - MSG_TXLOCK_VOTE, // Deprecated - MSG_SPORK, - MSG_MASTERNODE_WINNER, - MSG_MASTERNODE_SCANNING_ERROR, - MSG_BUDGET_VOTE, - MSG_BUDGET_PROPOSAL, - MSG_BUDGET_FINALIZED, - MSG_BUDGET_FINALIZED_VOTE, - MSG_MASTERNODE_QUORUM, - MSG_MASTERNODE_ANNOUNCE, - MSG_MASTERNODE_PING, - MSG_DSTX + std::string GetCommand() const; }; #endif // BITCOIN_PROTOCOL_H diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index eac3f1f4c062..d026613c2e12 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -14,6 +14,7 @@ #include "miner.h" #include "net.h" #include "rpc/server.h" +#include "util/blockstatecatcher.h" #include "validationinterface.h" #ifdef ENABLE_WALLET #include "wallet/rpcwallet.h" @@ -34,6 +35,8 @@ UniValue generateBlocks(const Consensus::Params& consensus, { UniValue blockHashes(UniValue::VARR); + BlockStateCatcher sc(UINT256_ZERO); + sc.registerEvent(); while (nHeight < nHeightEnd && !ShutdownRequested()) { // Get available coins @@ -53,8 +56,9 @@ UniValue generateBlocks(const Consensus::Params& consensus, if (!SolveBlock(pblock, nHeight + 1)) continue; } - CValidationState state; - if (!ProcessNewBlock(state, pblock, nullptr)) + sc.setBlockHash(pblock->GetHash()); + bool res = ProcessNewBlock(pblock, nullptr); + if (!res || sc.stateErrorFound()) throw JSONRPCError(RPC_INTERNAL_ERROR, "ProcessNewBlock, block not accepted"); ++nHeight; @@ -723,25 +727,6 @@ UniValue getblocktemplate(const JSONRPCRequest& request) } #endif // ENABLE_MINING_RPC -class submitblock_StateCatcher : public CValidationInterface -{ -public: - uint256 hash; - bool found; - CValidationState state; - - submitblock_StateCatcher(const uint256& hashIn) : hash(hashIn), found(false), state(){}; - -protected: - virtual void BlockChecked(const CBlock& block, const CValidationState& stateIn) - { - if (block.GetHash() != hash) - return; - found = true; - state = stateIn; - }; -}; - UniValue submitblock(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) @@ -788,11 +773,9 @@ UniValue submitblock(const JSONRPCRequest& request) } } - CValidationState state; - submitblock_StateCatcher sc(block.GetHash()); - RegisterValidationInterface(&sc); - bool fAccepted = ProcessNewBlock(state, blockptr, nullptr); - UnregisterValidationInterface(&sc); + BlockStateCatcher sc(block.GetHash()); + sc.registerEvent(); + bool fAccepted = ProcessNewBlock(blockptr, nullptr); if (fBlockPresent) { if (fAccepted && !sc.found) return "duplicate-inconclusive"; @@ -801,9 +784,8 @@ UniValue submitblock(const JSONRPCRequest& request) if (fAccepted) { if (!sc.found) return "inconclusive"; - state = sc.state; } - return BIP22ValidationResult(state); + return BIP22ValidationResult(sc.state); } UniValue estimatefee(const JSONRPCRequest& request) diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index 04878963723b..8f48eb19ca1f 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -401,7 +401,7 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) CValidationState state; BOOST_CHECK(!ProcessSpecialTxsInBlock(block, &indexFake, state, true)); BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-protx-dup-owner-key"); - ProcessNewBlock(state, std::make_shared(block), nullptr); + ProcessNewBlock(std::make_shared(block), nullptr); // todo: move to check reject reason BOOST_CHECK_EQUAL(chainActive.Height(), nHeight); // bad block not connected } // Block with two ProReg txes using same operator key @@ -418,7 +418,7 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) CValidationState state; BOOST_CHECK(!ProcessSpecialTxsInBlock(block, &indexFake, state, true)); BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-protx-dup-operator-key"); - ProcessNewBlock(state, std::make_shared(block), nullptr); + ProcessNewBlock(std::make_shared(block), nullptr); // todo: move to check reject reason BOOST_CHECK_EQUAL(chainActive.Height(), nHeight); // bad block not connected } // Block with two ProReg txes using ip address @@ -432,7 +432,7 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) CValidationState state; BOOST_CHECK(!ProcessSpecialTxsInBlock(block, &indexFake, state, true)); BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-protx-dup-IP-address"); - ProcessNewBlock(state, std::make_shared(block), nullptr); + ProcessNewBlock(std::make_shared(block), nullptr); // todo: move to check reject reason BOOST_CHECK_EQUAL(chainActive.Height(), nHeight); // bad block not connected } @@ -501,8 +501,7 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) GetScriptForDestination(coinbaseKey.GetPubKey().GetID()))); pblock->vtx[0] = MakeTransactionRef(invalidCoinbaseTx); pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); - CValidationState state; - ProcessNewBlock(state, pblock, nullptr); + ProcessNewBlock(pblock, nullptr); // block not connected chainTip = WITH_LOCK(cs_main, return chainActive.Tip()); BOOST_CHECK(chainTip->nHeight == nHeight); @@ -610,7 +609,7 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) CValidationState state; BOOST_CHECK(!ProcessSpecialTxsInBlock(block, &indexFake, state, true)); BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-protx-dup-addr"); - ProcessNewBlock(state, std::make_shared(block), nullptr); + ProcessNewBlock(std::make_shared(block), nullptr); // todo: move to ProcessBlockAndCheckRejectionReason. BOOST_CHECK_EQUAL(chainActive.Height(), nHeight); // bad block not connected } @@ -684,7 +683,6 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) // ProUpReg: Try to change the voting key of a masternode that doesn't exist { - const CKey& votingKey = GetRandomKey(); auto tx = CreateProUpRegTx(utxos, GetRandHash(), GetRandomKey(), GetRandomKey(), GetRandomKey(), GenerateRandomAddress(), coinbaseKey); CValidationState state; @@ -728,7 +726,7 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) BOOST_CHECK_MESSAGE(!ProcessSpecialTxsInBlock(block, &indexFake, state, true), "Accepted block with duplicate operator key in ProUpReg txes"); BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-protx-dup-operator-key"); - ProcessNewBlock(state, std::make_shared(block), nullptr); + ProcessNewBlock(std::make_shared(block), nullptr); BOOST_CHECK_EQUAL(chainActive.Height(), nHeight); // bad block not connected } @@ -748,7 +746,7 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) BOOST_CHECK_MESSAGE(!ProcessSpecialTxsInBlock(block, &indexFake, state, true), "Accepted block with duplicate operator key in ProReg+ProUpReg txes"); BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-protx-dup-operator-key"); - ProcessNewBlock(state, std::make_shared(block), nullptr); + ProcessNewBlock(std::make_shared(block), nullptr); BOOST_CHECK_EQUAL(chainActive.Height(), nHeight); // bad block not connected } diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index bf447f336402..81b9b8a608bd 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -11,6 +11,7 @@ #include "miner.h" #include "pubkey.h" #include "uint256.h" +#include "util/blockstatecatcher.h" #include "util/system.h" #include "validation.h" #include "wallet/wallet.h" @@ -213,10 +214,11 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) txFirst.emplace_back(pblock->vtx[0]); pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); pblock->nNonce = blockinfo[i].nonce; - CValidationState state; - BOOST_CHECK(ProcessNewBlock(state, pblock, nullptr)); - BOOST_CHECK(state.IsValid()); + BlockStateCatcher stateCatcher(pblock->GetHash()); + stateCatcher.registerEvent(); + BOOST_CHECK(ProcessNewBlock(pblock, nullptr)); SyncWithValidationInterfaceQueue(); + BOOST_CHECK(stateCatcher.found && stateCatcher.state.IsValid()); pblock->hashPrevBlock = pblock->GetHash(); } diff --git a/src/test/test_pivx.cpp b/src/test/test_pivx.cpp index 459bcb344ae9..39dac4744be0 100644 --- a/src/test/test_pivx.cpp +++ b/src/test/test_pivx.cpp @@ -153,8 +153,7 @@ TestChainSetup::TestChainSetup(int blockCount) : TestingSetup(CBaseChainParams:: CBlock TestChainSetup::CreateAndProcessBlock(const std::vector& txns, const CScript& scriptPubKey, bool fNoMempoolTx) { CBlock block = CreateBlock(txns, scriptPubKey, fNoMempoolTx); - CValidationState state; - ProcessNewBlock(state, std::make_shared(block), nullptr); + ProcessNewBlock(std::make_shared(block), nullptr); return block; } diff --git a/src/test/util/blocksutil.cpp b/src/test/util/blocksutil.cpp index c5c88ebe700c..d1d5fe488a99 100644 --- a/src/test/util/blocksutil.cpp +++ b/src/test/util/blocksutil.cpp @@ -4,39 +4,19 @@ #include "test/util/blocksutil.h" #include "validation.h" -#include "validationinterface.h" +#include "util/blockstatecatcher.h" #include -class BlockStateCatcher : public CValidationInterface -{ -public: - uint256 hash; - bool found; - CValidationState state; - - BlockStateCatcher(const uint256& hashIn) : hash(hashIn), found(false), state(){}; - -protected: - virtual void BlockChecked(const CBlock& block, const CValidationState& stateIn) - { - if (block.GetHash() != hash) return; - found = true; - state = stateIn; - }; -}; - void ProcessBlockAndCheckRejectionReason(std::shared_ptr& pblock, const std::string& blockRejectionReason, int expectedChainHeight) { - CValidationState state; - BlockStateCatcher stateChecker(pblock->GetHash()); - RegisterValidationInterface(&stateChecker); - ProcessNewBlock(state, pblock, nullptr); - UnregisterValidationInterface(&stateChecker); - BOOST_CHECK(stateChecker.found); - state = stateChecker.state; + BlockStateCatcher stateCatcher(pblock->GetHash()); + stateCatcher.registerEvent(); + ProcessNewBlock(pblock, nullptr); + BOOST_CHECK(stateCatcher.found); + CValidationState state = stateCatcher.state; BOOST_CHECK_EQUAL(WITH_LOCK(cs_main, return chainActive.Height();), expectedChainHeight); BOOST_CHECK(!state.IsValid()); BOOST_CHECK_EQUAL(state.GetRejectReason(), blockRejectionReason); diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index d3d332dd11ec..c53a999ddb02 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -11,6 +11,7 @@ #include "pow.h" #include "random.h" #include "test/test_pivx.h" +#include "util/blockstatecatcher.h" #include "validation.h" #include "validationinterface.h" @@ -116,9 +117,8 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering) BuildChain(Params().GenesisBlock().GetHash(), 100, 15, 10, 500, blocks); } - CValidationState state; // Connect the genesis block and drain any outstanding events - BOOST_CHECK_MESSAGE(ProcessNewBlock(state, std::make_shared(Params().GenesisBlock()), nullptr), "Error: genesis not connected"); + BOOST_CHECK_MESSAGE(ProcessNewBlock(std::make_shared(Params().GenesisBlock()), nullptr), "Error: genesis not connected"); SyncWithValidationInterfaceQueue(); // subscribe to events (this subscriber will validate event ordering) @@ -132,21 +132,23 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering) boost::thread_group threads; for (int i = 0; i < 10; i++) { threads.create_thread([&blocks]() { - CValidationState state; for (int i = 0; i < 1000; i++) { auto block = blocks[GetRand(blocks.size() - 1)]; - ProcessNewBlock(state, block, nullptr); + ProcessNewBlock(block, nullptr); } + BlockStateCatcher sc(UINT256_ZERO); + sc.registerEvent(); // to make sure that eventually we process the full chain - do it here for (const auto& block : blocks) { if (block->vtx.size() == 1) { - bool processed = ProcessNewBlock(state, block, nullptr); + sc.setBlockHash(block->GetHash()); + bool processed = ProcessNewBlock(block, nullptr); // Future to do: "prevblk-not-found" here is the only valid reason to not check processed flag. - if (state.GetRejectReason() == "duplicate" || - state.GetRejectReason() == "prevblk-not-found" || - state.GetRejectReason() == "bad-prevblk") continue; - ASSERT_WITH_MSG(!processed, ("Error: " + state.GetRejectReason()).c_str()); + std::string stateReason = sc.state.GetRejectReason(); + if (sc.found && (stateReason == "duplicate" || stateReason == "prevblk-not-found" || + stateReason == "bad-prevblk" || stateReason == "blk-out-of-order")) continue; + ASSERT_WITH_MSG(processed, ("Error: " + sc.state.GetRejectReason()).c_str()); } } }); diff --git a/src/test/validation_tests.cpp b/src/test/validation_tests.cpp index 8eef74aa1fa4..fb90709eab07 100644 --- a/src/test/validation_tests.cpp +++ b/src/test/validation_tests.cpp @@ -7,6 +7,7 @@ #include "primitives/transaction.h" #include "sapling/sapling_validation.h" #include "test/librust/utiltest.h" +#include "util/blockstatecatcher.h" #include "wallet/test/wallet_test_fixture.h" #include @@ -97,10 +98,11 @@ void CheckBlockZcRejection(std::shared_ptr& pblock, int nHeight, CMutabl { pblock->vtx.emplace_back(MakeTransactionRef(mtx)); BOOST_CHECK(SolveBlock(pblock, nHeight)); - CValidationState state; - BOOST_CHECK(!ProcessNewBlock(state, pblock, nullptr)); - BOOST_CHECK(!state.IsValid()); - BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-blk-with-zc"); + BlockStateCatcher stateCatcher(pblock->GetHash()); + stateCatcher.registerEvent(); + BOOST_CHECK(!ProcessNewBlock(pblock, nullptr)); + BOOST_CHECK(stateCatcher.found && !stateCatcher.state.IsValid()); + BOOST_CHECK_EQUAL(stateCatcher.state.GetRejectReason(), "bad-blk-with-zc"); } void CheckMempoolZcRejection(CMutableTransaction& mtx) diff --git a/src/util/blockstatecatcher.h b/src/util/blockstatecatcher.h new file mode 100644 index 000000000000..2db568fc7e34 --- /dev/null +++ b/src/util/blockstatecatcher.h @@ -0,0 +1,37 @@ +// Copyright (c) 2021 The PIVX developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#ifndef PIVX_BLOCKSTATECATCHER_H +#define PIVX_BLOCKSTATECATCHER_H + +#include "validation.h" +#include "validationinterface.h" + +/** + * Validation interface listener used to get feedback from ProcessNewBlock result. + */ +class BlockStateCatcher : public CValidationInterface +{ +public: + uint256 hash; + bool found; + CValidationState state; + bool isRegistered{false}; + + BlockStateCatcher(const uint256& hashIn) : hash(hashIn), found(false), state(){}; + ~BlockStateCatcher() { if (isRegistered) UnregisterValidationInterface(this); } + void registerEvent() { RegisterValidationInterface(this); isRegistered = true; } + void setBlockHash(const uint256& _hash) { clear(); hash = _hash; } + void clear() { hash.SetNull(); found = false; state = CValidationState(); } + bool stateErrorFound() { return found && state.IsError(); } + +protected: + virtual void BlockChecked(const CBlock& block, const CValidationState& stateIn) { + if (block.GetHash() != hash) return; + found = true; + state = stateIn; + }; +}; + +#endif //PIVX_BLOCKSTATECATCHER_H diff --git a/src/validation.cpp b/src/validation.cpp index 91aa087f1461..1d435eaa8e5f 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -13,6 +13,7 @@ #include "addrman.h" #include "amount.h" #include "blocksignature.h" +#include "util/blockstatecatcher.h" #include "budget/budgetmanager.h" #include "chainparams.h" #include "checkpoints.h" @@ -22,7 +23,6 @@ #include "consensus/tx_verify.h" #include "consensus/validation.h" #include "consensus/zerocoin_verify.h" -#include "evo/deterministicmns.h" #include "evo/specialtx.h" #include "flatfile.h" #include "fs.h" @@ -1531,7 +1531,6 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd CAmount nValueOut = 0; CAmount nValueIn = 0; unsigned int nMaxBlockSigOps = MAX_BLOCK_SIGOPS_CURRENT; - uint256 hashBlock = block.GetHash(); // Sapling SaplingMerkleTree sapling_tree; @@ -2759,7 +2758,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo //out of order auto mi = mapBlockIndex.find(block.hashPrevBlock); if (mi == mapBlockIndex.end()) { - return false; + return state.Error("blk-out-of-order"); } pindexPrev = mi->second; } @@ -2935,32 +2934,6 @@ bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& sta return true; } -bool IsBlockHashInChain(const uint256& hashBlock) -{ - if (hashBlock.IsNull() || !mapBlockIndex.count(hashBlock)) - return false; - - return chainActive.Contains(mapBlockIndex[hashBlock]); -} - -bool IsTransactionInChain(const uint256& txId, int& nHeightTx, CTransactionRef& tx) -{ - uint256 hashBlock; - if (!GetTransaction(txId, tx, hashBlock, true)) - return false; - if (!IsBlockHashInChain(hashBlock)) - return false; - - nHeightTx = mapBlockIndex.at(hashBlock)->nHeight; - return true; -} - -bool IsTransactionInChain(const uint256& txId, int& nHeightTx) -{ - CTransactionRef tx; - return IsTransactionInChain(txId, nHeightTx, tx); -} - bool ContextualCheckBlock(const CBlock& block, CValidationState& state, CBlockIndex* const pindexPrev) { const int nHeight = pindexPrev == nullptr ? 0 : pindexPrev->nHeight + 1; @@ -3306,7 +3279,7 @@ static bool AcceptBlock(const CBlock& block, CValidationState& state, CBlockInde return true; } -bool ProcessNewBlock(CValidationState& state, const std::shared_ptr pblock, const FlatFilePos* dbp, bool* fAccepted) +bool ProcessNewBlock(const std::shared_ptr& pblock, const FlatFilePos* dbp) { AssertLockNotHeld(cs_main); @@ -3317,21 +3290,24 @@ bool ProcessNewBlock(CValidationState& state, const std::shared_ptrGetHash().GetHex(), FormatStateMessage(state)); } // Store to disk CBlockIndex* pindex = nullptr; bool ret = AcceptBlock(*pblock, state, &pindex, dbp); - if (fAccepted) *fAccepted = ret; CheckBlockIndex(); if (!ret) { + GetMainSignals().BlockChecked(*pblock, state); return error("%s : AcceptBlock FAILED", __func__); } newHeight = pindex->nHeight; } + CValidationState state; // Only used to report errors, not invalidity - ignore it if (!ActivateBestChain(state, pblock)) return error("%s : ActivateBestChain failed", __func__); @@ -3854,6 +3830,10 @@ bool LoadExternalBlockFile(FILE* fileIn, FlatFilePos* dbp) static std::multimap mapBlocksUnknownParent; int64_t nStart = GetTimeMillis(); + // Block checked event listener + BlockStateCatcher stateCatcher(UINT256_ZERO); + stateCatcher.registerEvent(); + int nLoaded = 0; try { // This takes over fileIn and calls fclose() on it in the CBufferedFile destructor @@ -3905,12 +3885,14 @@ bool LoadExternalBlockFile(FILE* fileIn, FlatFilePos* dbp) // process in case the block isn't known yet if (mapBlockIndex.count(hash) == 0 || (mapBlockIndex[hash]->nStatus & BLOCK_HAVE_DATA) == 0) { - CValidationState state; std::shared_ptr block_ptr = std::make_shared(block); - if (ProcessNewBlock(state, block_ptr, dbp)) + stateCatcher.setBlockHash(block_ptr->GetHash()); + if (ProcessNewBlock(block_ptr, dbp)) { nLoaded++; - if (state.IsError()) + } + if (stateCatcher.stateErrorFound()) { break; + } } else if (hash != Params().GetConsensus().hashGenesisBlock && mapBlockIndex[hash]->nHeight % 1000 == 0) { LogPrint(BCLog::REINDEX, "Block Import: already had block %s at height %d\n", hash.ToString(), mapBlockIndex[hash]->nHeight); } @@ -3927,11 +3909,10 @@ bool LoadExternalBlockFile(FILE* fileIn, FlatFilePos* dbp) if (ReadBlockFromDisk(block, it->second)) { LogPrint(BCLog::REINDEX, "%s: Processing out of order child %s of %s\n", __func__, block.GetHash().ToString(), head.ToString()); - CValidationState dummy; std::shared_ptr block_ptr = std::make_shared(block); - if (ProcessNewBlock(dummy, block_ptr, &it->second)) { + if (ProcessNewBlock(block_ptr, &it->second)) { nLoaded++; - queue.push_back(block.GetHash()); + queue.emplace_back(block.GetHash()); } } range.first++; diff --git a/src/validation.h b/src/validation.h index 30ab92c5fed2..61285d960452 100644 --- a/src/validation.h +++ b/src/validation.h @@ -101,8 +101,6 @@ static const unsigned int BLOCK_DOWNLOAD_WINDOW = 1024; static const unsigned int DATABASE_WRITE_INTERVAL = 60 * 60; /** Time to wait (in seconds) between flushing chainstate to disk. */ static const unsigned int DATABASE_FLUSH_INTERVAL = 24 * 60 * 60; -/** Maximum length of reject messages. */ -static const unsigned int MAX_REJECT_MESSAGE_LENGTH = 111; /** Average delay between local address broadcasts in seconds. */ static const unsigned int AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL = 24 * 24 * 60; /** Average delay between peer address broadcasts in seconds. */ @@ -160,14 +158,18 @@ extern CBlockIndex* pindexBestHeader; * block is made active. Note that it does not, however, guarantee that the * specific block passed to it has been checked for validity! * - * @param[out] state This may be set to an Error state if any error occurred processing it, including during validation/connection/etc of otherwise unrelated blocks during reorganisation; or it may be set to an Invalid state if pblock is itself invalid (but this is not guaranteed even when the block is checked). If you want to *possibly* get feedback on whether pblock is valid, you must also install a CValidationInterface - this will have its BlockChecked method called whenever *any* block completes validation. - * @param[in] pfrom The node which we are receiving the block from; it is added to mapBlockSource and may be penalised if the block is invalid. + * If you want to *possibly* get feedback on whether pblock is valid, you must + * install a CValidationInterface (see validationinterface.h) - this will have + * its BlockChecked method called whenever *any* block completes validation. + * + * Note that we guarantee that either the proof-of-work is valid on pblock, or + * (and possibly also) BlockChecked will have been called. + * * @param[in] pblock The block we want to process. * @param[out] dbp The already known disk position of pblock, or nullptr if not yet stored. - * @param[out] fAccepted Whether the block is accepted or not * @return True if state.IsValid() */ -bool ProcessNewBlock(CValidationState& state, const std::shared_ptr pblock, const FlatFilePos* dbp, bool* fAccepted = nullptr); +bool ProcessNewBlock(const std::shared_ptr& pblock, const FlatFilePos* dbp); /** Open a block file (blk?????.dat) */ FILE* OpenBlockFile(const FlatFilePos& pos, bool fReadOnly = false); @@ -251,10 +253,6 @@ bool CheckInputs(const CTransaction& tx, CValidationState& state, const CCoinsVi /** Apply the effects of this transaction on the UTXO set represented by view */ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight, bool fSkipInvalid = false); -bool IsTransactionInChain(const uint256& txId, int& nHeightTx, CTransactionRef& tx); -bool IsTransactionInChain(const uint256& txId, int& nHeightTx); -bool IsBlockHashInChain(const uint256& hashBlock); - /** * Check if transaction will be final in the next block to be created. * diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 7b683d0f23af..7583d3b4b2e6 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -14,13 +14,11 @@ #include "destination_io.h" #include "httpserver.h" #include "key_io.h" -#include "key_io.h" #include "masternode-sync.h" #include "net.h" #include "policy/feerate.h" #include "rpc/server.h" #include "sapling/sapling_operation.h" -#include "sapling/transaction_builder.h" #include "sapling/key_io_sapling.h" #include "spork.h" #include "timedata.h" diff --git a/src/zpivchain.cpp b/src/zpivchain.cpp index 56940b21a47f..46f194f8caa6 100644 --- a/src/zpivchain.cpp +++ b/src/zpivchain.cpp @@ -13,6 +13,31 @@ // 6 comes from OPCODE (1) + vch.size() (1) + BIGNUM size (4) #define SCRIPT_OFFSET 6 +static bool IsTransactionInChain(const uint256& txId, int& nHeightTx, CTransactionRef& tx) +{ + uint256 hashBlock; + if (!GetTransaction(txId, tx, hashBlock, true)) + return false; + + if (hashBlock.IsNull() || !mapBlockIndex.count(hashBlock)) { + return false; + } + + CBlockIndex* pindex = mapBlockIndex[hashBlock]; + if (!chainActive.Contains(pindex)) { + return false; + } + + nHeightTx = pindex->nHeight; + return true; +} + +static bool IsTransactionInChain(const uint256& txId, int& nHeightTx) +{ + CTransactionRef tx; + return IsTransactionInChain(txId, nHeightTx, tx); +} + bool IsSerialInBlockchain(const CBigNum& bnSerial, int& nHeightTx) { uint256 txHash; diff --git a/test/functional/p2p_leak.py b/test/functional/p2p_leak.py index 7c4e2733e0ef..f7eaaaa354ef 100755 --- a/test/functional/p2p_leak.py +++ b/test/functional/p2p_leak.py @@ -4,7 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test message sending before handshake completion. -A node should never send anything other than VERSION/VERACK/REJECT until it's +A node should never send anything other than VERSION/VERACK until it's received a VERACK. This test connects to a node and sends it a few messages, trying to intice it @@ -70,8 +70,6 @@ def on_open(self): for i in range(banscore): self.send_message(msg_verack()) - def on_reject(self, message): pass - # Node that never sends a version. This one just sits idle and hopes to receive # any message (it shouldn't!) class CNodeNoVersionIdle(CLazyNode): @@ -84,7 +82,6 @@ def __init__(self): self.version_received = False super().__init__() - def on_reject(self, message): pass def on_verack(self, message): pass # When version is received, don't reply with a verack. Instead, see if the # node will give us a message that it shouldn't. This is not an exhaustive diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 521e9258c230..af151776c582 100644 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1249,38 +1249,6 @@ def serialize(self): def __repr__(self): return "msg_headers(headers=%s)" % repr(self.headers) - -class msg_reject(): - command = b"reject" - REJECT_MALFORMED = 1 - - def __init__(self): - self.message = b"" - self.code = 0 - self.reason = b"" - self.data = 0 - - def deserialize(self, f): - self.message = deser_string(f) - self.code = struct.unpack("