From 1c800903ed816f03b783ecee36dd01982cc2f2b9 Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Mon, 9 Nov 2020 15:12:17 +0100 Subject: [PATCH 1/4] Remove createDelayedMasternodePing from header. createDelayedMasternodePing is a very implementation-detaily function. This removes it from the header and from being a static method inside CMasternodePing and just turns it into a helper function inside masternode.cpp in an anonymous namespace. --- divi/src/masternode.cpp | 41 ++++++++++++++++++++++++----------------- divi/src/masternode.h | 1 - 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/divi/src/masternode.cpp b/divi/src/masternode.cpp index 4847de257..57a6135e8 100644 --- a/divi/src/masternode.cpp +++ b/divi/src/masternode.cpp @@ -735,6 +735,27 @@ bool CMasternodeBroadcastFactory::provideSignatures( return true; } +namespace +{ + +CMasternodePing createDelayedMasternodePing(const CTxIn& newVin) +{ + CMasternodePing ping; + const int64_t offsetTimeBy45BlocksInSeconds = 60 * 45; + ping.vin = newVin; + const int depthOfTx = GetInputAge(ping.vin); + const int offset = std::min( std::max(0, depthOfTx), 12 ); + const auto* block = chainActive[chainActive.Height() - offset]; + ping.blockHash = block->GetBlockHash(); + ping.sigTime = std::max(block->GetBlockTime() + offsetTimeBy45BlocksInSeconds, GetAdjustedTime()); + ping.vchSig = std::vector(); + LogPrint("masternode","mnp - relay block-time & sigtime: %d vs. %d\n", block->GetBlockTime(), ping.sigTime); + + return ping; +} + +} // anonymous namespace + void CMasternodeBroadcastFactory::createWithoutSignatures( CTxIn txin, CService service, @@ -748,8 +769,10 @@ void CMasternodeBroadcastFactory::createWithoutSignatures( CBitcoinAddress(pubKeyCollateralAddressNew.GetID()).ToString(), pubKeyMasternodeNew.GetID().ToString()); - CMasternodePing mnp = (deferRelay)? CMasternodePing::createDelayedMasternodePing(txin): CMasternodePing(txin); mnbRet = CMasternodeBroadcast(service, txin, pubKeyCollateralAddressNew, pubKeyMasternodeNew, nMasternodeTier, PROTOCOL_VERSION); + const CMasternodePing mnp = (deferRelay + ? createDelayedMasternodePing(txin) + : CMasternodePing(txin)); mnbRet.lastPing = mnp; mnbRet.sigTime = mnp.sigTime; } @@ -940,22 +963,6 @@ CMasternodePing::CMasternodePing(CTxIn& newVin) vchSig = std::vector(); } -CMasternodePing CMasternodePing::createDelayedMasternodePing(CTxIn& newVin) -{ - CMasternodePing ping; - const int64_t offsetTimeBy45BlocksInSeconds = 60 * 45; - ping.vin = newVin; - int depthOfTx = GetInputAge(ping.vin); - int offset = std::min( std::max(0, depthOfTx), 12 ); - auto block = chainActive[chainActive.Height()-offset]; - ping.blockHash = block->GetBlockHash(); - ping.sigTime = std::max(block->GetBlockTime() + offsetTimeBy45BlocksInSeconds, GetAdjustedTime()); - ping.vchSig = std::vector(); - LogPrint("masternode","mnp - relay block-time & sigtime: %d vs. %d\n", block->GetBlockTime(), ping.sigTime); - - return ping; -} - std::string CMasternodePing::getMessageToSign() const { return vin.ToString() + blockHash.ToString() + boost::lexical_cast(sigTime); diff --git a/divi/src/masternode.h b/divi/src/masternode.h index d197260b2..7bafdc6ac 100644 --- a/divi/src/masternode.h +++ b/divi/src/masternode.h @@ -61,7 +61,6 @@ class CMasternodePing CMasternodePing(); CMasternodePing(CTxIn& newVin); - static CMasternodePing createDelayedMasternodePing(CTxIn& newVin); ADD_SERIALIZE_METHODS; From 9484179e14204c687a2be254e7588c5786adb7cf Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Fri, 23 Oct 2020 17:38:04 +0200 Subject: [PATCH 2/4] Use mocktime for GetTimeMicros. GetTimeMicros and GetTimeMillis are used in some of the networking code (e.g. for triggering periodic pings), but didn't respect mocktime previously. This causes issues like spurious node disconnects in regtest tests using mocktime. This change adds support for mocktime to those methods, which seems to solve those issues. --- divi/src/utiltime.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/divi/src/utiltime.cpp b/divi/src/utiltime.cpp index c3210850d..e86fa0911 100644 --- a/divi/src/utiltime.cpp +++ b/divi/src/utiltime.cpp @@ -36,13 +36,14 @@ void SetMockTime(int64_t nMockTimeIn) int64_t GetTimeMillis() { - return (boost::posix_time::ptime(boost::posix_time::microsec_clock::universal_time()) - - boost::posix_time::ptime(boost::gregorian::date(1970, 1, 1))) - .total_milliseconds(); + return GetTimeMicros() / 1000; } int64_t GetTimeMicros() { + if (nMockTime) + return 1000000 * nMockTime; + return (boost::posix_time::ptime(boost::posix_time::microsec_clock::universal_time()) - boost::posix_time::ptime(boost::gregorian::date(1970, 1, 1))) .total_microseconds(); From 31d1559e7fcf225c449d7d5c2060ece77e7f7fed Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Tue, 20 Oct 2020 17:07:05 +0200 Subject: [PATCH 3/4] Consolidate masternode input age logic. This simplifies and consolidates the logic used to verify confirmations and age of the collateral UTXO for masternodes. Instead of various functions we now have just one main function, which looks up the block index when the UTXO was confirmed. With this, we can check that it actually has the required confirmations, that the sigtime matches (is later than the block when it got the final confirmation necessary), and also cache the block hash for later input age computation. --- divi/src/main.cpp | 19 ------- divi/src/main.h | 1 - divi/src/masternode.cpp | 118 +++++++++++++++++++++++----------------- divi/src/masternode.h | 22 +++++--- 4 files changed, 82 insertions(+), 78 deletions(-) diff --git a/divi/src/main.cpp b/divi/src/main.cpp index f7211c170..da26ffb4f 100755 --- a/divi/src/main.cpp +++ b/divi/src/main.cpp @@ -823,25 +823,6 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& in return nSigOps; } -int GetInputAgeIX(uint256 nTXHash, CTxIn& vin) -{ - int sigs = 0; - int nResult = GetInputAge(vin); - if (nResult < 0) nResult = 0; - - if (nResult < 6) { - std::map::iterator i = mapTxLocks.find(nTXHash); - if (i != mapTxLocks.end()) { - sigs = (*i).second.CountSignatures(); - } - if (sigs >= SWIFTTX_SIGNATURES_REQUIRED) { - return nSwiftTXDepth + nResult; - } - } - - return -1; -} - int GetIXConfirmations(uint256 nTXHash) { int sigs = 0; diff --git a/divi/src/main.h b/divi/src/main.h index 52cf9af99..a4e4dd564 100644 --- a/divi/src/main.h +++ b/divi/src/main.h @@ -173,7 +173,6 @@ void FlushStateToDisk(); /** (try to) add transaction to memory pool **/ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransaction& tx, bool fLimitFree, bool* pfMissingInputs, bool fRejectInsaneFee = false, bool ignoreFees = false); -int GetInputAgeIX(uint256 nTXHash, CTxIn& vin); int GetIXConfirmations(uint256 nTXHash); struct CNodeStateStats { diff --git a/divi/src/masternode.cpp b/divi/src/masternode.cpp index 57a6135e8..11c7e9e5f 100644 --- a/divi/src/masternode.cpp +++ b/divi/src/masternode.cpp @@ -122,8 +122,7 @@ CMasternode::CMasternode() activeState = MASTERNODE_ENABLED; sigTime = GetAdjustedTime(); lastPing = CMasternodePing(); - cacheInputAge = 0; - cacheInputAgeBlock = 0; + collateralBlock.SetNull(); allowFreeTx = true; nActiveState = MASTERNODE_ENABLED; protocolVersion = PROTOCOL_VERSION; @@ -144,8 +143,7 @@ CMasternode::CMasternode(const CMasternode& other) activeState = other.activeState; sigTime = other.sigTime; lastPing = other.lastPing; - cacheInputAge = other.cacheInputAge; - cacheInputAgeBlock = other.cacheInputAgeBlock; + collateralBlock = other.collateralBlock; allowFreeTx = other.allowFreeTx; nActiveState = MASTERNODE_ENABLED; protocolVersion = other.protocolVersion; @@ -166,8 +164,7 @@ CMasternode::CMasternode(const CMasternodeBroadcast& mnb) activeState = MASTERNODE_ENABLED; sigTime = mnb.sigTime; lastPing = mnb.lastPing; - cacheInputAge = 0; - cacheInputAgeBlock = 0; + collateralBlock = mnb.collateralBlock; allowFreeTx = true; nActiveState = MASTERNODE_ENABLED; protocolVersion = mnb.protocolVersion; @@ -192,8 +189,7 @@ void CMasternode::swap(CMasternode& first, CMasternode& second) // nothrow swap(first.activeState, second.activeState); swap(first.sigTime, second.sigTime); swap(first.lastPing, second.lastPing); - swap(first.cacheInputAge, second.cacheInputAge); - swap(first.cacheInputAgeBlock, second.cacheInputAgeBlock); + swap(first.collateralBlock, second.collateralBlock); swap(first.allowFreeTx, second.allowFreeTx); swap(first.protocolVersion, second.protocolVersion); swap(first.nScanningErrorCount, second.nScanningErrorCount); @@ -244,14 +240,18 @@ bool CMasternode::IsEnabled() const int CMasternode::GetMasternodeInputAge() const { - if (chainActive.Tip() == NULL) return 0; + LOCK(cs_main); - if (cacheInputAge == 0) { - cacheInputAge = GetInputAge(vin); - cacheInputAgeBlock = chainActive.Tip()->nHeight; - } + const auto* pindex = GetCollateralBlock(); + if (pindex == nullptr) + return 0; + + assert(chainActive.Contains(pindex)); - return cacheInputAge + (chainActive.Tip()->nHeight - cacheInputAgeBlock); + const unsigned tipHeight = chainActive.Height(); + assert(tipHeight >= pindex->nHeight); + + return tipHeight - pindex->nHeight + 1; } std::string CMasternode::Status() const @@ -738,12 +738,12 @@ bool CMasternodeBroadcastFactory::provideSignatures( namespace { -CMasternodePing createDelayedMasternodePing(const CTxIn& newVin) +CMasternodePing createDelayedMasternodePing(const CMasternodeBroadcast& mnb) { CMasternodePing ping; const int64_t offsetTimeBy45BlocksInSeconds = 60 * 45; - ping.vin = newVin; - const int depthOfTx = GetInputAge(ping.vin); + ping.vin = mnb.vin; + const int depthOfTx = mnb.GetMasternodeInputAge(); const int offset = std::min( std::max(0, depthOfTx), 12 ); const auto* block = chainActive[chainActive.Height() - offset]; ping.blockHash = block->GetBlockHash(); @@ -771,7 +771,7 @@ void CMasternodeBroadcastFactory::createWithoutSignatures( mnbRet = CMasternodeBroadcast(service, txin, pubKeyCollateralAddressNew, pubKeyMasternodeNew, nMasternodeTier, PROTOCOL_VERSION); const CMasternodePing mnp = (deferRelay - ? createDelayedMasternodePing(txin) + ? createDelayedMasternodePing(mnbRet) : CMasternodePing(txin)); mnbRet.lastPing = mnp; mnbRet.sigTime = mnp.sigTime; @@ -868,6 +868,38 @@ bool CMasternodeBroadcast::CheckAndUpdate(int& nDos) return true; } +const CBlockIndex* CMasternode::GetCollateralBlock() const +{ + LOCK(cs_main); + + if (!collateralBlock.IsNull()) { + const auto mi = mapBlockIndex.find(collateralBlock); + if (mi != mapBlockIndex.end() && chainActive.Contains(mi->second)) + return mi->second; + } + + uint256 hashBlock; + CTransaction tx; + if (!GetTransaction(vin.prevout.hash, tx, hashBlock, true)) { + collateralBlock.SetNull(); + return nullptr; + } + + const auto mi = mapBlockIndex.find(hashBlock); + if (mi == mapBlockIndex.end() || mi->second == nullptr) { + collateralBlock.SetNull(); + return nullptr; + } + + if (!chainActive.Contains(mi->second)) { + collateralBlock.SetNull(); + return nullptr; + } + + collateralBlock = hashBlock; + return mi->second; +} + bool CMasternodeBroadcast::CheckInputs(int& nDoS) const { // we are a masternode with the same vin (i.e. already activated) and this mnb is ours (matches our Masternode privkey) @@ -889,25 +921,30 @@ bool CMasternodeBroadcast::CheckInputs(int& nDoS) const LogPrint("masternode", "mnb - Accepted Masternode entry\n"); - if (GetInputAge(vin) < MASTERNODE_MIN_CONFIRMATIONS) { + const CBlockIndex* pindexConf; + { + LOCK(cs_main); + const auto* pindexCollateral = GetCollateralBlock(); + if (pindexCollateral == nullptr) + pindexConf = nullptr; + else { + assert(chainActive.Contains(pindexCollateral)); + pindexConf = chainActive[pindexCollateral->nHeight + MASTERNODE_MIN_CONFIRMATIONS - 1]; + assert(pindexConf == nullptr || pindexConf->GetAncestor(pindexCollateral->nHeight) == pindexCollateral); + } + } + + if (pindexConf == nullptr) { LogPrint("masternode","mnb - Input must have at least %d confirmations\n", MASTERNODE_MIN_CONFIRMATIONS); return false; } // verify that sig time is legit in past // should be at least not earlier than block when 1000 PIV tx got MASTERNODE_MIN_CONFIRMATIONS - uint256 hashBlock = 0; - CTransaction tx2; - GetTransaction(vin.prevout.hash, tx2, hashBlock, true); - BlockMap::iterator mi = mapBlockIndex.find(hashBlock); - if (mi != mapBlockIndex.end() && (*mi).second) { - CBlockIndex* pMNIndex = (*mi).second; // block for 1000 DIVI tx -> 1 confirmation - CBlockIndex* pConfIndex = chainActive[pMNIndex->nHeight + MASTERNODE_MIN_CONFIRMATIONS - 1]; // block where tx got MASTERNODE_MIN_CONFIRMATIONS - if (pConfIndex->GetBlockTime() > sigTime) { - LogPrint("masternode","mnb - Bad sigTime %d for Masternode %s (%i conf block is at %d)\n", - sigTime, vin.prevout.hash.ToString(), MASTERNODE_MIN_CONFIRMATIONS, pConfIndex->GetBlockTime()); - return false; - } + if (pindexConf->GetBlockTime() > sigTime) { + LogPrint("masternode","mnb - Bad sigTime %d for Masternode %s (%i conf block is at %d)\n", + sigTime, vin.prevout.hash.ToString(), MASTERNODE_MIN_CONFIRMATIONS, pindexConf->GetBlockTime()); + return false; } return true; @@ -1068,22 +1105,3 @@ void CMasternodePing::Relay() const CInv inv(MSG_MASTERNODE_PING, GetHash()); RelayInv(inv); } - -int GetInputAge(const CTxIn& vin) -{ - CCoinsView viewDummy; - CCoinsViewCache view(&viewDummy); - { - LOCK(mempool.cs); - CCoinsViewMemPool viewMempool(pcoinsTip, mempool); - view.SetBackend(viewMempool); // temporarily switch cache backend to db+mempool view - - const CCoins* coins = view.AccessCoins(vin.prevout.hash); - - if (coins) { - if (coins->nHeight < 0) return 0; - return (chainActive.Tip()->nHeight + 1) - coins->nHeight; - } else - return -1; - } -} diff --git a/divi/src/masternode.h b/divi/src/masternode.h index 7bafdc6ac..f5d25256f 100644 --- a/divi/src/masternode.h +++ b/divi/src/masternode.h @@ -44,8 +44,6 @@ bool GetBlockHashForScoring(uint256& hash, int nBlockHeight); bool GetBlockHashForScoring(uint256& hash, const CBlockIndex* pindex, const int offset); -int GetInputAge(const CTxIn& vin); - // // The Masternode Ping Class : Contains a different serialize method for sending pings from masternodes throughout the network // @@ -128,8 +126,16 @@ class CMasternode mutable CCriticalSection cs; int64_t lastTimeChecked; - mutable int cacheInputAge; - mutable int cacheInputAgeBlock; +protected: + + /** Cached block hash of where the collateral output of this + * masternode got included. */ + mutable uint256 collateralBlock; + + /** Looks up and returns the block index when the collateral got + * included in the currently active chain. If it is not yet confirmed + * then this returns nullptr. */ + const CBlockIndex* GetCollateralBlock() const; public: enum state { @@ -163,7 +169,6 @@ class CMasternode CMasternode(const CMasternode& other); CMasternode(const CMasternodeBroadcast& mnb); - void swap(CMasternode& first, CMasternode& second); // nothrow CMasternode& operator=(CMasternode from); @@ -198,8 +203,7 @@ class CMasternode READWRITE(protocolVersion); READWRITE(activeState); READWRITE(lastPing); - READWRITE(cacheInputAge); - READWRITE(cacheInputAgeBlock); + READWRITE(collateralBlock); READWRITE(allowFreeTx); READWRITE(nScanningErrorCount); READWRITE(nLastScanningErrorBlockHeight); @@ -285,8 +289,10 @@ class CMasternodeBroadcast : public CMasternode if (!ser_action.ForRead ()) tier = static_cast (nTier); READWRITE(tier); - if (ser_action.ForRead ()) + if (ser_action.ForRead ()) { nTier = static_cast (tier); + collateralBlock.SetNull(); + } } uint256 GetHash() const From 9732e8042f1e0ac7370766612c6b0a7f4fb45ade Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Thu, 22 Oct 2020 15:53:40 +0200 Subject: [PATCH 4/4] Do not try to register a deferred masternode. When startmasternode is called with defer set to true, do not even attempt to register the masternode; just create the broadcast data and return it. This makes it possible to use the function even when the masternode is not yet valid, e.g. because the collateral does not have sufficient confirmations yet. --- divi/src/rpcmasternode.cpp | 76 +++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 39 deletions(-) diff --git a/divi/src/rpcmasternode.cpp b/divi/src/rpcmasternode.cpp index ef4118c25..88bec83e7 100644 --- a/divi/src/rpcmasternode.cpp +++ b/divi/src/rpcmasternode.cpp @@ -505,57 +505,55 @@ Value startmasternode(const Array& params, bool fHelp) "\nResult:\n" "\"status\" (string) status of masternode\n"); - std::string alias = params[0].get_str(); - bool deferRelay = (params.size() == 1)? false: params[1].get_bool(); + const std::string alias = params[0].get_str(); + const bool deferRelay = (params.size() == 1)? false: params[1].get_bool(); EnsureWalletIsUnlocked(); - Object result; - bool fFound = false; for(const auto& configEntry : masternodeConfig.getEntries()) { - if(configEntry.getAlias() == alias) + if(configEntry.getAlias() != alias) + continue; + + Object result; + std::string strError; + CMasternodeBroadcast mnb; + + if(!CMasternodeBroadcastFactory::Create( + configEntry, + strError, + mnb, + false, + deferRelay)) { - fFound = true; - std::string strError; - CMasternodeBroadcast mnb; - - if(!CMasternodeBroadcastFactory::Create( - configEntry, - strError, - mnb, - false, - deferRelay)) - { - break; - } + result.push_back(Pair("status", "failed")); + result.push_back(Pair("error", strError)); + return result; + } - if(CActiveMasternode::Register(mnb, deferRelay)) - { - result.push_back(Pair("status", "success")); - if(deferRelay) - { - CDataStream ss(SER_NETWORK,PROTOCOL_VERSION); - ss << mnb; - result.push_back(Pair("broadcastData", HexStr(ss.str()) )); - } - fFound = true; - } + if (deferRelay) + { + CDataStream ss(SER_NETWORK,PROTOCOL_VERSION); + ss << mnb; - if(!fFound) - { - result.push_back(Pair("status", "failed")); - result.push_back(Pair("error", strError)); - } + result.push_back(Pair("status", "success")); + result.push_back(Pair("broadcastData", HexStr(ss.str()))); - break; + return result; } - } - if(!fFound) - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid alias, couldn't find MN. Check your masternode.conf file"); + if(!CActiveMasternode::Register(mnb, deferRelay)) + { + result.push_back(Pair("status", "failed")); + result.push_back(Pair("error", strError)); + return result; + } - return result; + result.push_back(Pair("status", "success")); + return result; + } + + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid alias, couldn't find MN. Check your masternode.conf file"); } Value createmasternodekey(const Array& params, bool fHelp)