diff --git a/divi/src/ActiveChainManager.cpp b/divi/src/ActiveChainManager.cpp index 62e6b476d..867074ef7 100644 --- a/divi/src/ActiveChainManager.cpp +++ b/divi/src/ActiveChainManager.cpp @@ -87,19 +87,21 @@ bool ActiveChainManager::DisconnectBlock( return error("DisconnectBlock() : block and undo data inconsistent"); } + const BlockUtxoHasher utxoHasher; + bool fClean = true; IndexDatabaseUpdates indexDBUpdates; // undo transactions in reverse order for (int transactionIndex = block.vtx.size() - 1; transactionIndex >= 0; transactionIndex--) { const CTransaction& tx = block.vtx[transactionIndex]; - const TxReversalStatus status = UpdateCoinsReversingTransaction(tx,view,blockUndo.vtxundo[transactionIndex-1],pindex->nHeight); + const TransactionLocationReference txLocationReference(utxoHasher, tx, pindex->nHeight, transactionIndex); + const TxReversalStatus status = UpdateCoinsReversingTransaction(tx, txLocationReference, view, blockUndo.vtxundo[transactionIndex-1]); if(!CheckTxReversalStatus(status,fClean)) { return false; } if(!pfClean) { - TransactionLocationReference txLocationReference(tx.GetHash(),pindex->nHeight,transactionIndex); IndexDatabaseUpdateCollector::ReverseTransaction(tx,txLocationReference,view,indexDBUpdates); } } @@ -136,4 +138,4 @@ void ActiveChainManager::DisconnectBlock( int64_t nStart = GetTimeMicros(); status = DisconnectBlock(block,state,pindex,coins); LogPrint("bench", "- Disconnect block: %.2fms\n", (GetTimeMicros() - nStart) * 0.001); -} \ No newline at end of file +} diff --git a/divi/src/BlockMemoryPoolTransactionCollector.cpp b/divi/src/BlockMemoryPoolTransactionCollector.cpp index 4ba830f52..1407823f1 100644 --- a/divi/src/BlockMemoryPoolTransactionCollector.cpp +++ b/divi/src/BlockMemoryPoolTransactionCollector.cpp @@ -203,7 +203,7 @@ std::vector BlockMemoryPoolTransactionCollector::PrioritizeMempoolTr // Read prev transaction if (!view.HaveCoins(txin.prevout.hash)) { CTransaction prevTx; - if(!mempool_.lookup(txin.prevout.hash, prevTx)) { + if(!mempool_.lookupOutpoint(txin.prevout.hash, prevTx)) { // This should never happen; all transactions in the memory // pool should connect to either transactions in the chain // or other transactions in the memory pool. @@ -338,7 +338,7 @@ std::vector BlockMemoryPoolTransactionCollector::Pri nBlockSigOps += nTxSigOps; CTxUndo txundo; - UpdateCoinsWithTransaction(tx, view, txundo, nHeight); + UpdateCoinsWithTransaction(tx, view, txundo, mempool_.GetUtxoHasher(), nHeight); if (fPrintPriority) { LogPrintf("priority %.1f fee %s txid %s\n", @@ -346,7 +346,7 @@ std::vector BlockMemoryPoolTransactionCollector::Pri } // Add transactions that depend on this one to the priority queue - AddDependingTransactionsToPriorityQueue(dependentTransactions, hash, vecPriority, comparer); + AddDependingTransactionsToPriorityQueue(dependentTransactions, mempool_.GetUtxoHasher().GetUtxoHash(tx), vecPriority, comparer); } LogPrintf("CreateNewBlock(): total size %u\n", nBlockSize); diff --git a/divi/src/BlockTransactionChecker.cpp b/divi/src/BlockTransactionChecker.cpp index 7b75aaae3..98fa9793e 100644 --- a/divi/src/BlockTransactionChecker.cpp +++ b/divi/src/BlockTransactionChecker.cpp @@ -38,12 +38,14 @@ void TransactionLocationRecorder::RecordTxLocationData( BlockTransactionChecker::BlockTransactionChecker( const CBlock& block, + const TransactionUtxoHasher& utxoHasher, CValidationState& state, CBlockIndex* pindex, CCoinsViewCache& view, const int blocksToSkipChecksFor ): blockundo_(block.vtx.size() - 1) , block_(block) + , utxoHasher_(utxoHasher) , state_(state) , pindex_(pindex) , view_(view) @@ -59,7 +61,7 @@ bool BlockTransactionChecker::Check(const CBlockRewards& nExpectedMint,bool fJus pindex_->nMint = 0; for (unsigned int i = 0; i < block_.vtx.size(); i++) { const CTransaction& tx = block_.vtx[i]; - const TransactionLocationReference txLocationRef(tx.GetHash(),pindex_->nHeight,i); + const TransactionLocationReference txLocationRef(utxoHasher_, tx, pindex_->nHeight, i); if(!txInputChecker_.TotalSigOpsAreBelowMaximum(tx)) { @@ -79,7 +81,7 @@ bool BlockTransactionChecker::Check(const CBlockRewards& nExpectedMint,bool fJus } IndexDatabaseUpdateCollector::RecordTransaction(tx,txLocationRef,view_, indexDatabaseUpdates); - UpdateCoinsWithTransaction(tx, view_, blockundo_.vtxundo[i>0u? i-1: 0u], pindex_->nHeight); + UpdateCoinsWithTransaction(tx, view_, blockundo_.vtxundo[i>0u? i-1: 0u], utxoHasher_, pindex_->nHeight); txLocationRecorder_.RecordTxLocationData(tx,indexDatabaseUpdates.txLocationData); } return true; @@ -93,4 +95,4 @@ bool BlockTransactionChecker::WaitForScriptsToBeChecked() CBlockUndo& BlockTransactionChecker::getBlockUndoData() { return blockundo_; -} \ No newline at end of file +} diff --git a/divi/src/BlockTransactionChecker.h b/divi/src/BlockTransactionChecker.h index 68bbbf4a0..43335788d 100644 --- a/divi/src/BlockTransactionChecker.h +++ b/divi/src/BlockTransactionChecker.h @@ -13,6 +13,7 @@ class CBlock; class CValidationState; class CCoinsViewCache; class CBlockRewards; +class TransactionUtxoHasher; class TransactionLocationRecorder { @@ -35,6 +36,7 @@ class BlockTransactionChecker private: CBlockUndo blockundo_; const CBlock& block_; + const TransactionUtxoHasher& utxoHasher_; CValidationState& state_; CBlockIndex* pindex_; CCoinsViewCache& view_; @@ -43,6 +45,7 @@ class BlockTransactionChecker public: BlockTransactionChecker( const CBlock& block, + const TransactionUtxoHasher& utxoHasher, CValidationState& state, CBlockIndex* pindex, CCoinsViewCache& view, @@ -56,4 +59,4 @@ class BlockTransactionChecker CBlockUndo& getBlockUndoData(); }; -#endif// BLOCK_TRANSACTION_CHECKER_H \ No newline at end of file +#endif// BLOCK_TRANSACTION_CHECKER_H diff --git a/divi/src/IndexDatabaseUpdateCollector.cpp b/divi/src/IndexDatabaseUpdateCollector.cpp index 8d0a8466c..73eb56909 100644 --- a/divi/src/IndexDatabaseUpdateCollector.cpp +++ b/divi/src/IndexDatabaseUpdateCollector.cpp @@ -81,7 +81,7 @@ void CollectUpdatesFromOutputs( std::make_pair(CAddressIndexKey(addressType, uint160(hashBytes), txLocationRef.blockHeight, txLocationRef.transactionIndex, txLocationRef.hash, k, false), out.nValue)); // record unspent output indexDatabaseUpdates.addressUnspentIndex.push_back( - std::make_pair(CAddressUnspentKey(addressType, uint160(hashBytes), txLocationRef.hash, k), CAddressUnspentValue(out.nValue, out.scriptPubKey, txLocationRef.blockHeight))); + std::make_pair(CAddressUnspentKey(addressType, uint160(hashBytes), txLocationRef.utxoHash, k), CAddressUnspentValue(out.nValue, out.scriptPubKey, txLocationRef.blockHeight))); } else { continue; } @@ -156,7 +156,7 @@ static void CollectUpdatesFromOutputs( // undo unspent index indexDBUpdates.addressUnspentIndex.push_back( std::make_pair( - CAddressUnspentKey(addressType, uint160(hashBytes), txLocationReference.hash, k), + CAddressUnspentKey(addressType, uint160(hashBytes), txLocationReference.utxoHash, k), CAddressUnspentValue())); } @@ -182,4 +182,4 @@ void IndexDatabaseUpdateCollector::ReverseTransaction( { ReverseSpending::CollectUpdatesFromOutputs(tx,txLocationRef,indexDatabaseUpdates); ReverseSpending::CollectUpdatesFromInputs(tx,txLocationRef,view, indexDatabaseUpdates); -} \ No newline at end of file +} diff --git a/divi/src/IndexDatabaseUpdates.cpp b/divi/src/IndexDatabaseUpdates.cpp index b29a9896d..7ed210ee0 100644 --- a/divi/src/IndexDatabaseUpdates.cpp +++ b/divi/src/IndexDatabaseUpdates.cpp @@ -1,5 +1,8 @@ #include +#include +#include + IndexDatabaseUpdates::IndexDatabaseUpdates( ): addressIndex() , addressUnspentIndex() @@ -9,11 +12,13 @@ IndexDatabaseUpdates::IndexDatabaseUpdates( } TransactionLocationReference::TransactionLocationReference( - uint256 hashValue, + const TransactionUtxoHasher& utxoHasher, + const CTransaction& tx, unsigned blockheightValue, int transactionIndexValue - ): hash(hashValue) + ): hash(tx.GetHash()) + , utxoHash(utxoHasher.GetUtxoHash(tx)) , blockHeight(blockheightValue) , transactionIndex(transactionIndexValue) { -} \ No newline at end of file +} diff --git a/divi/src/IndexDatabaseUpdates.h b/divi/src/IndexDatabaseUpdates.h index 677130e3a..a724b1a7e 100644 --- a/divi/src/IndexDatabaseUpdates.h +++ b/divi/src/IndexDatabaseUpdates.h @@ -6,6 +6,9 @@ #include #include +class CTransaction; +class TransactionUtxoHasher; + struct IndexDatabaseUpdates { std::vector > addressIndex; @@ -19,12 +22,14 @@ struct IndexDatabaseUpdates struct TransactionLocationReference { uint256 hash; + uint256 utxoHash; unsigned blockHeight; int transactionIndex; TransactionLocationReference( - uint256 hashValue, + const TransactionUtxoHasher& utxoHasher, + const CTransaction& tx, unsigned blockheightValue, int transactionIndexValue); }; -#endif// INDEX_DATABASE_UPDATES_H \ No newline at end of file +#endif// INDEX_DATABASE_UPDATES_H diff --git a/divi/src/Makefile.test.include b/divi/src/Makefile.test.include index 140e12caf..994155373 100644 --- a/divi/src/Makefile.test.include +++ b/divi/src/Makefile.test.include @@ -63,6 +63,7 @@ BITCOIN_TESTS =\ test/MockBlockIncentivesPopulator.h \ test/MockBlockSubsidyProvider.h \ test/MockPoSStakeModifierService.h \ + test/MockUtxoHasher.cpp \ test/MockVaultManagerDatabase.h \ test/monthlywalletbackupcreator_tests.cpp \ test/ActiveMasternode_tests.cpp \ @@ -101,6 +102,7 @@ BITCOIN_TESTS =\ test/PoSStakeModifierService_tests.cpp \ test/LegacyPoSStakeModifierService_tests.cpp \ test/LotteryWinnersCalculatorTests.cpp \ + test/UtxoCheckingAndUpdating_tests.cpp \ test/VaultManager_tests.cpp \ test/multi_wallet_tests.cpp diff --git a/divi/src/MasternodeBroadcastFactory.cpp b/divi/src/MasternodeBroadcastFactory.cpp index fac4d3730..7207e8863 100644 --- a/divi/src/MasternodeBroadcastFactory.cpp +++ b/divi/src/MasternodeBroadcastFactory.cpp @@ -17,7 +17,10 @@ extern CChain chainActive; extern bool fReindex; extern bool fImporting; -bool CMasternodeBroadcastFactory::checkBlockchainSync(std::string& strErrorRet, bool fOffline) +namespace +{ + +bool checkBlockchainSync(std::string& strErrorRet, bool fOffline) { if (!fOffline && !IsBlockchainSynced()) { strErrorRet = "Sync in progress. Must wait until sync is complete to start Masternode"; @@ -26,7 +29,7 @@ bool CMasternodeBroadcastFactory::checkBlockchainSync(std::string& strErrorRet, } return true; } -bool CMasternodeBroadcastFactory::setMasternodeKeys( +bool setMasternodeKeys( const std::string& strKeyMasternode, std::pair& masternodeKeyPair, std::string& strErrorRet) @@ -38,7 +41,7 @@ bool CMasternodeBroadcastFactory::setMasternodeKeys( } return true; } -bool CMasternodeBroadcastFactory::setMasternodeCollateralKeys( +bool setMasternodeCollateralKeys( const std::string& txHash, const std::string& outputIndex, const std::string& service, @@ -63,7 +66,7 @@ bool CMasternodeBroadcastFactory::setMasternodeCollateralKeys( return true; } -bool CMasternodeBroadcastFactory::checkMasternodeCollateral( +bool checkMasternodeCollateral( const CTxIn& txin, const std::string& txHash, const std::string& outputIndex, @@ -95,7 +98,7 @@ bool CMasternodeBroadcastFactory::checkMasternodeCollateral( return true; } -bool CMasternodeBroadcastFactory::createArgumentsFromConfig( +bool createArgumentsFromConfig( const CMasternodeConfig::CMasternodeEntry configEntry, std::string& strErrorRet, bool fOffline, @@ -121,6 +124,8 @@ bool CMasternodeBroadcastFactory::createArgumentsFromConfig( return true; } +} // anonymous namespace + bool CMasternodeBroadcastFactory::Create( const CMasternodeConfig::CMasternodeEntry configEntry, CPubKey pubkeyCollateralAddress, @@ -278,10 +283,10 @@ CMasternodePing createDelayedMasternodePing(const CMasternodeBroadcast& mnb) } // anonymous namespace void CMasternodeBroadcastFactory::createWithoutSignatures( - CTxIn txin, - CService service, - CPubKey pubKeyCollateralAddressNew, - CPubKey pubKeyMasternodeNew, + const CTxIn& txin, + const CService& service, + const CPubKey& pubKeyCollateralAddressNew, + const CPubKey& pubKeyMasternodeNew, const MasternodeTier nMasternodeTier, bool deferRelay, CMasternodeBroadcast& mnbRet) @@ -299,12 +304,12 @@ void CMasternodeBroadcastFactory::createWithoutSignatures( } bool CMasternodeBroadcastFactory::Create( - CTxIn txin, - CService service, - CKey keyCollateralAddressNew, - CPubKey pubKeyCollateralAddressNew, - CKey keyMasternodeNew, - CPubKey pubKeyMasternodeNew, + const CTxIn& txin, + const CService& service, + const CKey& keyCollateralAddressNew, + const CPubKey& pubKeyCollateralAddressNew, + const CKey& keyMasternodeNew, + const CPubKey& pubKeyMasternodeNew, const MasternodeTier nMasternodeTier, std::string& strErrorRet, CMasternodeBroadcast& mnbRet, diff --git a/divi/src/MasternodeBroadcastFactory.h b/divi/src/MasternodeBroadcastFactory.h index e7edd478a..2f576f060 100644 --- a/divi/src/MasternodeBroadcastFactory.h +++ b/divi/src/MasternodeBroadcastFactory.h @@ -37,10 +37,10 @@ class CMasternodeBroadcastFactory std::string& strErrorRet); private: static void createWithoutSignatures( - CTxIn txin, - CService service, - CPubKey pubKeyCollateralAddressNew, - CPubKey pubKeyMasternodeNew, + const CTxIn& txin, + const CService& service, + const CPubKey& pubKeyCollateralAddressNew, + const CPubKey& pubKeyMasternodeNew, MasternodeTier nMasternodeTier, bool deferRelay, CMasternodeBroadcast& mnbRet); @@ -57,45 +57,15 @@ class CMasternodeBroadcastFactory CMasternodeBroadcast& mnb, std::string& strErrorRet); - static bool Create(CTxIn vin, - CService service, - CKey keyCollateralAddressNew, - CPubKey pubKeyCollateralAddressNew, - CKey keyMasternodeNew, - CPubKey pubKeyMasternodeNew, - MasternodeTier nMasternodeTier, - std::string& strErrorRet, - CMasternodeBroadcast& mnbRet, - bool deferRelay); - static bool checkBlockchainSync( - std::string& strErrorRet, bool fOffline); - static bool setMasternodeKeys( - const std::string& strKeyMasternode, - std::pair& masternodeKeyPair, - std::string& strErrorRet); - static bool setMasternodeCollateralKeys( - const std::string& txHash, - const std::string& outputIndex, - const std::string& service, - bool collateralPrivKeyIsRemote, - CTxIn& txin, - std::pair& masternodeCollateralKeyPair, - std::string& error); - static bool checkMasternodeCollateral( - const CTxIn& txin, - const std::string& txHash, - const std::string& outputIndex, - const std::string& service, - MasternodeTier& nMasternodeTier, - std::string& strErrorRet); - static bool createArgumentsFromConfig( - const CMasternodeConfig::CMasternodeEntry configEntry, - std::string& strErrorRet, - bool fOffline, - bool collateralPrivKeyIsRemote, - CTxIn& txin, - std::pair& masternodeKeyPair, - std::pair& masternodeCollateralKeyPair, - MasternodeTier& nMasternodeTier); + static bool Create(const CTxIn& vin, + const CService& service, + const CKey& keyCollateralAddressNew, + const CPubKey& pubKeyCollateralAddressNew, + const CKey& keyMasternodeNew, + const CPubKey& pubKeyMasternodeNew, + MasternodeTier nMasternodeTier, + std::string& strErrorRet, + CMasternodeBroadcast& mnbRet, + bool deferRelay); }; -#endif //MASTERNODE_BROADCAST_FACTORY_H \ No newline at end of file +#endif //MASTERNODE_BROADCAST_FACTORY_H diff --git a/divi/src/UtxoCheckingAndUpdating.cpp b/divi/src/UtxoCheckingAndUpdating.cpp index 26e71d9aa..b0aeef600 100644 --- a/divi/src/UtxoCheckingAndUpdating.cpp +++ b/divi/src/UtxoCheckingAndUpdating.cpp @@ -10,15 +10,22 @@ #include #include #include +#include extern BlockMap mapBlockIndex; -void UpdateCoinsWithTransaction(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo& txundo, int nHeight) +uint256 BlockUtxoHasher::GetUtxoHash(const CTransaction& tx) const +{ + return tx.GetHash(); +} + +void UpdateCoinsWithTransaction(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo& txundo, + const TransactionUtxoHasher& hasher, const int nHeight) { // mark inputs spent if (!tx.IsCoinBase() ) { txundo.vprevout.reserve(tx.vin.size()); - BOOST_FOREACH (const CTxIn& txin, tx.vin) { + for (const auto& txin : tx.vin) { txundo.vprevout.push_back(CTxInUndo()); bool ret = inputs.ModifyCoins(txin.prevout.hash)->Spend(txin.prevout.n, txundo.vprevout.back()); assert(ret); @@ -26,12 +33,12 @@ void UpdateCoinsWithTransaction(const CTransaction& tx, CCoinsViewCache& inputs, } // add outputs - inputs.ModifyCoins(tx.GetHash())->FromTx(tx, nHeight); + inputs.ModifyCoins(hasher.GetUtxoHash(tx))->FromTx(tx, nHeight); } static bool RemoveTxOutputsFromCache( const CTransaction& tx, - const int blockHeight, + const TransactionLocationReference& txLocationReference, CCoinsViewCache& view) { bool outputsAvailable = true; @@ -40,10 +47,10 @@ static bool RemoveTxOutputsFromCache( // have outputs available even in the block itself, so we handle that case // specially with outsEmpty. CCoins outsEmpty; - CCoinsModifier outs = view.ModifyCoins(tx.GetHash()); + CCoinsModifier outs = view.ModifyCoins(txLocationReference.utxoHash); outs->ClearUnspendable(); - CCoins outsBlock(tx, blockHeight); + CCoins outsBlock(tx, txLocationReference.blockHeight); // The CCoins serialization does not serialize negative numbers. // No network rules currently depend on the version here, so an inconsistency is harmless // but it must be corrected before txout nversion ever influences a network rule. @@ -88,10 +95,10 @@ static void UpdateCoinsForRestoredInputs( coins->vout[out.n] = undo.txout; } -TxReversalStatus UpdateCoinsReversingTransaction(const CTransaction& tx, CCoinsViewCache& inputs, const CTxUndo& txundo, int nHeight) +TxReversalStatus UpdateCoinsReversingTransaction(const CTransaction& tx, const TransactionLocationReference& txLocationReference, CCoinsViewCache& inputs, const CTxUndo& txundo) { bool fClean = true; - fClean = fClean && RemoveTxOutputsFromCache(tx,nHeight,inputs); + fClean = fClean && RemoveTxOutputsFromCache(tx, txLocationReference, inputs); if(tx.IsCoinBase()) return fClean? TxReversalStatus::OK : TxReversalStatus::CONTINUE_WITH_ERRORS; if (txundo.vprevout.size() != tx.vin.size()) { diff --git a/divi/src/UtxoCheckingAndUpdating.h b/divi/src/UtxoCheckingAndUpdating.h index e68ef7177..318286292 100644 --- a/divi/src/UtxoCheckingAndUpdating.h +++ b/divi/src/UtxoCheckingAndUpdating.h @@ -3,11 +3,14 @@ #include #include #include +#include class CTransaction; class CValidationState; class CCoinsViewCache; class CTxUndo; +class TransactionLocationReference; + enum class TxReversalStatus { ABORT_NO_OTHER_ERRORS, @@ -15,8 +18,49 @@ enum class TxReversalStatus CONTINUE_WITH_ERRORS, OK, }; -void UpdateCoinsWithTransaction(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo& txundo, int nHeight); -TxReversalStatus UpdateCoinsReversingTransaction(const CTransaction& tx, CCoinsViewCache& inputs, const CTxUndo& txundo, int nHeight); + +/** Implementations of this interface translate transactions into the hashes + * that will be used to refer to the UTXOs their outputs create. + * + * This class abstracts away the segwit-light fork and its activation + * from the places that need to refer to / update UTXOs. + * + * For unit tests, this class can be subclassed and mocked. */ +class TransactionUtxoHasher +{ + +public: + + TransactionUtxoHasher() = default; + virtual ~TransactionUtxoHasher() = default; + + TransactionUtxoHasher(const TransactionUtxoHasher&) = delete; + void operator=(const TransactionUtxoHasher&) = delete; + + virtual uint256 GetUtxoHash(const CTransaction& tx) const = 0; + +}; + +/** A TransactionUtxoHasher for transactions contained in a particular + * block, e.g. for processing that block's connect or disconnect. Initially + * these are just the txid (as also with upstream Bitcoin), but for + * segwit-light, they are changed to the bare txid. + * + * This class abstracts away the segwit-light fork and its activation + * from the places that need to refer to / update UTXOs. + * + * For unit tests, this class can be subclassed and mocked. */ +class BlockUtxoHasher : public TransactionUtxoHasher +{ + +public: + + uint256 GetUtxoHash(const CTransaction& tx) const override; + +}; + +void UpdateCoinsWithTransaction(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo& txundo, const TransactionUtxoHasher& hasher, int nHeight); +TxReversalStatus UpdateCoinsReversingTransaction(const CTransaction& tx, const TransactionLocationReference& txLocationReference, CCoinsViewCache& inputs, const CTxUndo& txundo); bool CheckInputs( const CTransaction& tx, CValidationState& state, @@ -36,4 +80,4 @@ bool CheckInputs( bool cacheStore, std::vector* pvChecks = nullptr, bool connectBlockDoS = false); -#endif// UTXO_CHECKING_AND_UPDATING_H \ No newline at end of file +#endif// UTXO_CHECKING_AND_UPDATING_H diff --git a/divi/src/main.cpp b/divi/src/main.cpp index a66520311..091e9928c 100755 --- a/divi/src/main.cpp +++ b/divi/src/main.cpp @@ -1202,7 +1202,10 @@ void Misbehaving(NodeId pnode, int howmuch) LogPrintf("Misbehaving: %s (%d -> %d)\n", state->name, state->nMisbehavior - howmuch, state->nMisbehavior); } -void static InvalidChainFound(CBlockIndex* pindexNew) +namespace +{ + +void InvalidChainFound(CBlockIndex* pindexNew) { if (!pindexBestInvalid || pindexNew->nChainWork > pindexBestInvalid->nChainWork) pindexBestInvalid = pindexNew; @@ -1217,7 +1220,7 @@ void static InvalidChainFound(CBlockIndex* pindexNew) CheckForkWarningConditions(); } -void static InvalidBlockFound(CBlockIndex* pindex, const CValidationState& state) +void InvalidBlockFound(CBlockIndex* pindex, const CValidationState& state) { int nDoS = 0; if (state.IsInvalid(nDoS)) { @@ -1237,7 +1240,7 @@ void static InvalidBlockFound(CBlockIndex* pindex, const CValidationState& state } } -void static FlushBlockFile(bool fFinalize = false) +void FlushBlockFile(bool fFinalize = false) { LOCK(cs_LastBlockFile); @@ -1260,9 +1263,35 @@ void static FlushBlockFile(bool fFinalize = false) } } -bool FindUndoPos(CValidationState& state, int nFile, CDiskBlockPos& pos, unsigned int nAddSize); +bool FindUndoPos(CValidationState& state, int nFile, CDiskBlockPos& pos, unsigned int nAddSize) +{ + pos.nFile = nFile; + + LOCK(cs_LastBlockFile); + + unsigned int nNewSize; + pos.nPos = vinfoBlockFile[nFile].nUndoSize; + nNewSize = vinfoBlockFile[nFile].nUndoSize += nAddSize; + setDirtyFileInfo.insert(nFile); + + unsigned int nOldChunks = (pos.nPos + UNDOFILE_CHUNK_SIZE - 1) / UNDOFILE_CHUNK_SIZE; + unsigned int nNewChunks = (nNewSize + UNDOFILE_CHUNK_SIZE - 1) / UNDOFILE_CHUNK_SIZE; + if (nNewChunks > nOldChunks) { + if (CheckDiskSpace(nNewChunks * UNDOFILE_CHUNK_SIZE - pos.nPos)) { + FILE* file = OpenUndoFile(pos); + if (file) { + LogPrintf("Pre-allocating up to position 0x%x in rev%05u.dat\n", nNewChunks * UNDOFILE_CHUNK_SIZE, pos.nFile); + AllocateFileRange(file, pos.nPos, nNewChunks * UNDOFILE_CHUNK_SIZE - pos.nPos); + fclose(file); + } + } else + return state.Error("out of disk space"); + } + + return true; +} -static int64_t nTimeTotal = 0; +int64_t nTimeTotal = 0; void VerifyBestBlockIsAtPreviousBlock(const CBlockIndex* pindex, CCoinsViewCache& view) { @@ -1272,7 +1301,7 @@ void VerifyBestBlockIsAtPreviousBlock(const CBlockIndex* pindex, CCoinsViewCache assert(hashPrevBlock == view.GetBestBlock()); } -bool CheckEnforcedPoSBlocksAndBIP30(const CChainParams& chainParameters, const CBlock& block, CValidationState& state, const CBlockIndex* pindex, const CCoinsViewCache& view) +bool CheckEnforcedPoSBlocksAndBIP30(const CChainParams& chainParameters, const TransactionUtxoHasher& utxoHasher, const CBlock& block, CValidationState& state, const CBlockIndex* pindex, const CCoinsViewCache& view) { if (pindex->nHeight <= chainParameters.LAST_POW_BLOCK() && block.IsProofOfStake()) return state.DoS(100, error("%s : PoS period not active",__func__), @@ -1284,7 +1313,7 @@ bool CheckEnforcedPoSBlocksAndBIP30(const CChainParams& chainParameters, const C // Enforce BIP30. for (const auto& tx : block.vtx) { - const CCoins* coins = view.AccessCoins(tx.GetHash()); + const CCoins* coins = view.AccessCoins(utxoHasher.GetUtxoHash(tx)); if (coins && !coins->IsPruned()) return state.DoS(100, error("%s : tried to overwrite transaction",__func__), REJECT_INVALID, "bad-txns-BIP30"); @@ -1391,6 +1420,8 @@ bool UpdateDBIndicesForNewBlock( return true; } +} // anonymous namespace + bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pindex, CCoinsViewCache& view, bool fJustCheck, bool fAlreadyChecked) { AssertLockHeld(cs_main); @@ -1402,6 +1433,8 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin LogWalletBalance(pwalletMain); static const CChainParams& chainParameters = Params(); + const BlockUtxoHasher utxoHasher; + VerifyBestBlockIsAtPreviousBlock(pindex,view); if (block.GetHash() == Params().HashGenesisBlock()) { @@ -1409,7 +1442,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin view.SetBestBlock(pindex->GetBlockHash()); return true; } - if(!CheckEnforcedPoSBlocksAndBIP30(chainParameters,block,state,pindex,view)) + if(!CheckEnforcedPoSBlocksAndBIP30(chainParameters,utxoHasher,block,state,pindex,view)) { return false; } @@ -1427,7 +1460,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin const int blocksToSkipChecksFor = checkpointsVerifier.GetTotalBlocksEstimate(); IndexDatabaseUpdates indexDatabaseUpdates; CBlockRewards nExpectedMint = subsidiesContainer.blockSubsidiesProvider().GetBlockSubsidity(pindex->nHeight); - BlockTransactionChecker blockTxChecker(block,state,pindex,view,blocksToSkipChecksFor); + BlockTransactionChecker blockTxChecker(block, utxoHasher, state, pindex, view, blocksToSkipChecksFor); if(!blockTxChecker.Check(nExpectedMint,fJustCheck,indexDatabaseUpdates)) { @@ -2140,34 +2173,6 @@ bool FindBlockPos(CValidationState& state, CDiskBlockPos& pos, unsigned int nAdd return true; } -bool FindUndoPos(CValidationState& state, int nFile, CDiskBlockPos& pos, unsigned int nAddSize) -{ - pos.nFile = nFile; - - LOCK(cs_LastBlockFile); - - unsigned int nNewSize; - pos.nPos = vinfoBlockFile[nFile].nUndoSize; - nNewSize = vinfoBlockFile[nFile].nUndoSize += nAddSize; - setDirtyFileInfo.insert(nFile); - - unsigned int nOldChunks = (pos.nPos + UNDOFILE_CHUNK_SIZE - 1) / UNDOFILE_CHUNK_SIZE; - unsigned int nNewChunks = (nNewSize + UNDOFILE_CHUNK_SIZE - 1) / UNDOFILE_CHUNK_SIZE; - if (nNewChunks > nOldChunks) { - if (CheckDiskSpace(nNewChunks * UNDOFILE_CHUNK_SIZE - pos.nPos)) { - FILE* file = OpenUndoFile(pos); - if (file) { - LogPrintf("Pre-allocating up to position 0x%x in rev%05u.dat\n", nNewChunks * UNDOFILE_CHUNK_SIZE, pos.nFile); - AllocateFileRange(file, pos.nPos, nNewChunks * UNDOFILE_CHUNK_SIZE - pos.nPos); - fclose(file); - } - } else - return state.Error("out of disk space"); - } - - return true; -} - bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckMerkleRoot) { // These are checks that are independent of context. @@ -2734,6 +2739,7 @@ bool static LoadBlockIndexDB(string& strError) strError = "The wallet has been not been closed gracefully and has caused corruption of blocks stored to disk. Data directory is in an unusable state"; return false; } + const BlockUtxoHasher utxoHasher; std::vector vtxundo; vtxundo.reserve(block.vtx.size() - 1); @@ -2743,7 +2749,7 @@ bool static LoadBlockIndexDB(string& strError) CTxUndo undoDummy; if (i > 0) vtxundo.push_back(CTxUndo()); - UpdateCoinsWithTransaction(block.vtx[i], view, i == 0 ? undoDummy : vtxundo.back(), pindex->nHeight); + UpdateCoinsWithTransaction(block.vtx[i], view, i == 0 ? undoDummy : vtxundo.back(), utxoHasher, pindex->nHeight); view.SetBestBlock(hashBlock); } diff --git a/divi/src/masternode-sync.h b/divi/src/masternode-sync.h index 1ce4a3b33..c15d852e1 100644 --- a/divi/src/masternode-sync.h +++ b/divi/src/masternode-sync.h @@ -85,7 +85,7 @@ class CMasternodeSync bool MasternodeWinnersListIsSync(CNode* pnode, const int64_t now); void Process(bool networkIsRegtest); bool IsSynced() const; - bool IsMasternodeListSynced() { return RequestedMasternodeAssets > MASTERNODE_SYNC_LIST; } + bool IsMasternodeListSynced() const { return RequestedMasternodeAssets > MASTERNODE_SYNC_LIST; } void AskForMN(CNode* pnode, const CTxIn& vin) const; }; diff --git a/divi/src/masternode.cpp b/divi/src/masternode.cpp index d2391d2c2..cf457f42b 100644 --- a/divi/src/masternode.cpp +++ b/divi/src/masternode.cpp @@ -258,7 +258,10 @@ bool CMasternode::IsValidNetAddr() const (IsReachable(addr) && addr.IsRoutable()); } -CMasternodeBroadcast::CMasternodeBroadcast(CService newAddr, CTxIn newVin, CPubKey pubKeyCollateralAddressNew, CPubKey pubKeyMasternodeNew, const MasternodeTier nMasternodeTier, int protocolVersionIn) +CMasternodeBroadcast::CMasternodeBroadcast( + const CService& newAddr, const CTxIn& newVin, + const CPubKey& pubKeyCollateralAddressNew, const CPubKey& pubKeyMasternodeNew, + const MasternodeTier nMasternodeTier, const int protocolVersionIn) { vin = newVin; addr = newAddr; diff --git a/divi/src/masternode.h b/divi/src/masternode.h index f987234c1..b561097fc 100644 --- a/divi/src/masternode.h +++ b/divi/src/masternode.h @@ -146,15 +146,19 @@ class CMasternode class CMasternodeBroadcast : public CMasternode { -public: - CMasternodeBroadcast() = default; +private: CMasternodeBroadcast( - CService newAddr, - CTxIn newVin, - CPubKey pubKeyCollateralAddress, - CPubKey pubKeyMasternode, + const CService& newAddr, + const CTxIn& newVin, + const CPubKey& pubKeyCollateralAddress, + const CPubKey& pubKeyMasternode, MasternodeTier nMasternodeTier, int protocolVersionIn); + + friend class CMasternodeBroadcastFactory; + +public: + CMasternodeBroadcast() = default; CMasternodeBroadcast(const CMasternode& mn); void Relay() const; diff --git a/divi/src/masternodeconfig.cpp b/divi/src/masternodeconfig.cpp index 91b9debb9..c48f8304d 100644 --- a/divi/src/masternodeconfig.cpp +++ b/divi/src/masternodeconfig.cpp @@ -23,10 +23,11 @@ boost::filesystem::path GetMasternodeConfigFile() if (!pathConfigFile.is_complete()) pathConfigFile = GetDataDir() / pathConfigFile; return pathConfigFile; } -void CMasternodeConfig::add(std::string alias, std::string ip, std::string privKey, std::string txHash, std::string outputIndex) + +void CMasternodeConfig::add(const std::string& alias, const std::string& ip, const std::string& privKey, + const std::string& txHash, const std::string& outputIndex) { - CMasternodeEntry cme(alias, ip, privKey, txHash, outputIndex); - entries.push_back(cme); + entries.emplace_back(alias, ip, privKey, txHash, outputIndex); } bool CMasternodeConfig::read(std::string& strErr) @@ -94,15 +95,16 @@ CMasternodeConfig::CMasternodeConfig() { entries = std::vector(); } + const std::vector& CMasternodeConfig::getEntries() const { return entries; } -int CMasternodeConfig::getCount() +int CMasternodeConfig::getCount() const { int c = -1; - BOOST_FOREACH (CMasternodeEntry e, entries) { + for (const auto& e : entries) { if (e.getAlias() != "") c++; } return c; diff --git a/divi/src/masternodeconfig.h b/divi/src/masternodeconfig.h index d5ac185d9..2d34edb4a 100644 --- a/divi/src/masternodeconfig.h +++ b/divi/src/masternodeconfig.h @@ -25,7 +25,9 @@ class CMasternodeConfig std::string outputIndex; public: - CMasternodeEntry(std::string alias, std::string ip, std::string privKey, std::string txHash, std::string outputIndex) + CMasternodeEntry(const std::string& alias, const std::string& ip, + const std::string& privKey, + const std::string& txHash, const std::string& outputIndex) { this->alias = alias; this->ip = ip; @@ -91,9 +93,10 @@ class CMasternodeConfig void clear(); bool read(std::string& strErr); - void add(std::string alias, std::string ip, std::string privKey, std::string txHash, std::string outputIndex); + void add(const std::string& alias, const std::string& ip, const std::string& privKey, + const std::string& txHash, const std::string& outputIndex); const std::vector& getEntries() const; - int getCount(); + int getCount() const; private: std::vector entries; diff --git a/divi/src/test/MockUtxoHasher.cpp b/divi/src/test/MockUtxoHasher.cpp new file mode 100644 index 000000000..2170dd638 --- /dev/null +++ b/divi/src/test/MockUtxoHasher.cpp @@ -0,0 +1,26 @@ +#include "MockUtxoHasher.h" + +#include "hash.h" +#include "primitives/transaction.h" + +uint256 MockUtxoHasher::Add(const CTransaction& tx) +{ + ++cnt; + const uint256 value = Hash(&cnt, (&cnt) + 1); + customHashes.emplace(tx.GetHash(), value); + return value; +} + +void MockUtxoHasher::UseBareTxid(const CTransaction& tx) +{ + customHashes.emplace(tx.GetHash(), tx.GetBareTxid()); +} + +uint256 MockUtxoHasher::GetUtxoHash(const CTransaction& tx) const +{ + const uint256 txid = tx.GetHash(); + const auto mit = customHashes.find(txid); + if (mit != customHashes.end()) + return mit->second; + return txid; +} diff --git a/divi/src/test/MockUtxoHasher.h b/divi/src/test/MockUtxoHasher.h new file mode 100644 index 000000000..9ea8f0ade --- /dev/null +++ b/divi/src/test/MockUtxoHasher.h @@ -0,0 +1,43 @@ +#ifndef MOCKUTXOHASHER_H +#define MOCKUTXOHASHER_H + +#include "UtxoCheckingAndUpdating.h" + +#include "uint256.h" + +#include + +class CTransaction; + +/** A TransactionUtxoHasher instance that returns normal txid's (as per before + * segwit-light) by default, but can be instructed to return custom hashes + * for certain transactions. */ +class MockUtxoHasher : public TransactionUtxoHasher +{ + +private: + + /** Custom hashes to return for given txid's. */ + std::map customHashes; + + /** Internal counter to produce unique fake hashes. */ + unsigned cnt = 0; + +public: + + MockUtxoHasher() + {} + + /** Marks the given transaction for returning a custom hash. The hash + * is generated uniquely and returned from the function. */ + uint256 Add(const CTransaction& tx); + + /** Marks the given transaction for using the bare txid rather than + * the normal txid. */ + void UseBareTxid(const CTransaction& tx); + + uint256 GetUtxoHash(const CTransaction& tx) const override; + +}; + +#endif // MOCKUTXOHASHER_H diff --git a/divi/src/test/UtxoCheckingAndUpdating_tests.cpp b/divi/src/test/UtxoCheckingAndUpdating_tests.cpp new file mode 100644 index 000000000..4c2324971 --- /dev/null +++ b/divi/src/test/UtxoCheckingAndUpdating_tests.cpp @@ -0,0 +1,54 @@ +#include +#include + +#include "coins.h" +#include "MockUtxoHasher.h" +#include "primitives/transaction.h" + +namespace +{ + +class UpdateCoinsTestFixture +{ + +private: + + /** Empty coins used as base in the cached view. */ + CCoinsView dummyCoins; + +protected: + + CCoinsViewCache coins; + MockUtxoHasher utxoHasher; + + UpdateCoinsTestFixture() + : coins(&dummyCoins) + {} + +}; + +BOOST_FIXTURE_TEST_SUITE(UpdateCoins_tests, UpdateCoinsTestFixture) + +BOOST_AUTO_TEST_CASE(addsCorrectOutputs) +{ + CMutableTransaction mtx; + mtx.vout.emplace_back(1, CScript() << OP_TRUE); + const CTransaction tx1(mtx); + + mtx.vout.clear(); + mtx.vout.emplace_back(2, CScript() << OP_TRUE); + const CTransaction tx2(mtx); + const auto id2 = utxoHasher.Add(tx2); + + CTxUndo txundo; + UpdateCoinsWithTransaction(tx1, coins, txundo, utxoHasher, 101); + UpdateCoinsWithTransaction(tx2, coins, txundo, utxoHasher, 102); + + BOOST_CHECK(coins.HaveCoins(tx1.GetHash())); + BOOST_CHECK(!coins.HaveCoins(tx2.GetHash())); + BOOST_CHECK(coins.HaveCoins(id2)); +} + +BOOST_AUTO_TEST_SUITE_END() + +} // anonymous namespace diff --git a/divi/src/test/mempool_tests.cpp b/divi/src/test/mempool_tests.cpp index af1b6ad18..32a73712a 100644 --- a/divi/src/test/mempool_tests.cpp +++ b/divi/src/test/mempool_tests.cpp @@ -2,39 +2,67 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include "main.h" #include "txmempool.h" +#include "blockmap.h" +#include "MockUtxoHasher.h" + #include #include +extern CChain chainActive; +extern BlockMap mapBlockIndex; + class MempoolTestFixture { +private: + + /** Empty coins view used to back the cached view we actually use. */ + CCoinsView emptyView; + + /** Tip of our fake chain. */ + CBlockIndex tip; + protected: /** A parent transaction. */ CMutableTransaction txParent; - /** Three children of the parent. */ + /** Three children of the parent. They use the bare txid for their UTXOs + * in our UTXO hasher. */ CMutableTransaction txChild[3]; /** Three grand children. */ CMutableTransaction txGrandChild[3]; + /** Coins view with the parent inputs. */ + CCoinsViewCache view; + /** The test mempool instance. */ CTxMemPool testPool; public: MempoolTestFixture() - : testPool(CFeeRate(0)) + : view(&emptyView), testPool(CFeeRate(0)) { - txParent.vin.resize(2); + std::unique_ptr utxoHasher(new MockUtxoHasher()); + + CMutableTransaction base; + base.vout.emplace_back(99000LL, CScript() << OP_11 << OP_EQUAL); + view.ModifyCoins(base.GetHash())->FromTx(base, 0); + + tip.pprev = nullptr; + tip.nHeight = 0; + mapBlockIndex[tip.GetBlockHeader().GetHash()] = &tip; + view.SetBestBlock(tip.GetBlockHeader().GetHash()); + chainActive.SetTip(&tip); + + txParent.vin.resize(1); txParent.vin[0].scriptSig = CScript() << OP_11; - /* Add a second input to make sure the transaction does not qualify as - coinbase and thus has a bare txid unequal to its normal hash. */ - txParent.vin[1].scriptSig = CScript() << OP_12; + txParent.vin[0].prevout.hash = base.GetHash(); + txParent.vin[0].prevout.n = 0; txParent.vout.resize(3); for (int i = 0; i < 3; i++) { @@ -52,18 +80,39 @@ class MempoolTestFixture txChild[i].vout.resize(1); txChild[i].vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; txChild[i].vout[0].nValue = 11000LL; + utxoHasher->UseBareTxid(txChild[i]); } for (int i = 0; i < 3; i++) { txGrandChild[i].vin.resize(1); txGrandChild[i].vin[0].scriptSig = CScript() << OP_11; - txGrandChild[i].vin[0].prevout.hash = txChild[i].GetHash(); + txGrandChild[i].vin[0].prevout.hash = txChild[i].GetBareTxid(); txGrandChild[i].vin[0].prevout.n = 0; txGrandChild[i].vout.resize(1); txGrandChild[i].vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; txGrandChild[i].vout[0].nValue = 11000LL; } + + testPool.setSanityCheck(true); + testPool.SetUtxoHasherForTesting(std::move(utxoHasher)); + testPool.clear(); + } + + ~MempoolTestFixture() + { + mapBlockIndex.clear(); + } + + /** Adds the parent, childs and grandchilds to the mempool. */ + void AddAll() + { + testPool.addUnchecked(txParent.GetHash(), CTxMemPoolEntry(txParent, 0, 0, 0.0, 1)); + for (int i = 0; i < 3; i++) + { + testPool.addUnchecked(txChild[i].GetHash(), CTxMemPoolEntry(txChild[i], 0, 0, 0.0, 1)); + testPool.addUnchecked(txGrandChild[i].GetHash(), CTxMemPoolEntry(txGrandChild[i], 0, 0, 0.0, 1)); + } } }; @@ -87,12 +136,10 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) removed.clear(); // Parent, children, grandchildren: - testPool.addUnchecked(txParent.GetHash(), CTxMemPoolEntry(txParent, 0, 0, 0.0, 1)); - for (int i = 0; i < 3; i++) - { - testPool.addUnchecked(txChild[i].GetHash(), CTxMemPoolEntry(txChild[i], 0, 0, 0.0, 1)); - testPool.addUnchecked(txGrandChild[i].GetHash(), CTxMemPoolEntry(txGrandChild[i], 0, 0, 0.0, 1)); - } + AddAll(); + + testPool.check(&view); + // Remove Child[0], GrandChild[0] should be removed: testPool.remove(txChild[0], removed, true); BOOST_CHECK_EQUAL(removed.size(), 2); @@ -127,12 +174,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexByBareTxid) CTransaction tx; std::list removed; - testPool.addUnchecked(txParent.GetHash(), CTxMemPoolEntry(txParent, 0, 0, 0.0, 1)); - for (int i = 0; i < 3; ++i) - { - testPool.addUnchecked(txChild[i].GetHash(), CTxMemPoolEntry(txChild[i], 0, 0, 0.0, 1)); - testPool.addUnchecked(txGrandChild[i].GetHash(), CTxMemPoolEntry(txGrandChild[i], 0, 0, 0.0, 1)); - } + AddAll(); BOOST_CHECK(testPool.lookupBareTxid(txParent.GetBareTxid(), tx)); BOOST_CHECK(tx.GetHash() == txParent.GetHash()); @@ -144,4 +186,28 @@ BOOST_AUTO_TEST_CASE(MempoolIndexByBareTxid) BOOST_CHECK(!testPool.lookupBareTxid(txGrandChild[0].GetBareTxid(), tx)); } +BOOST_AUTO_TEST_CASE(MempoolOutpointLookup) +{ + CTransaction tx; + CCoins coins; + + AddAll(); + CCoinsViewMemPool viewPool(&view, testPool); + + BOOST_CHECK(testPool.lookupOutpoint(txParent.GetHash(), tx)); + BOOST_CHECK(!testPool.lookupOutpoint(txParent.GetBareTxid(), tx)); + BOOST_CHECK(!testPool.lookupOutpoint(txChild[0].GetHash(), tx)); + BOOST_CHECK(testPool.lookupOutpoint(txChild[0].GetBareTxid(), tx)); + + BOOST_CHECK(viewPool.HaveCoins(txParent.GetHash())); + BOOST_CHECK(viewPool.GetCoins(txParent.GetHash(), coins)); + BOOST_CHECK(!viewPool.HaveCoins(txParent.GetBareTxid())); + BOOST_CHECK(!viewPool.GetCoins(txParent.GetBareTxid(), coins)); + + BOOST_CHECK(!viewPool.HaveCoins(txChild[0].GetHash())); + BOOST_CHECK(!viewPool.GetCoins(txChild[0].GetHash(), coins)); + BOOST_CHECK(viewPool.HaveCoins(txChild[0].GetBareTxid())); + BOOST_CHECK(viewPool.GetCoins(txChild[0].GetBareTxid(), coins)); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/divi/src/txmempool.cpp b/divi/src/txmempool.cpp index 86b7e1e7f..b32f834d1 100644 --- a/divi/src/txmempool.cpp +++ b/divi/src/txmempool.cpp @@ -359,6 +359,23 @@ class CMinerPolicyEstimator } }; +namespace +{ + +/** The UTXO hasher used in mempool logic. */ +class MempoolUtxoHasher : public TransactionUtxoHasher +{ + +public: + + uint256 GetUtxoHash(const CTransaction& tx) const override + { + return tx.GetHash(); + } + +}; + +} // anonymous namespace CTxMemPool::CTxMemPool(const CFeeRate& _minRelayFee) : nTransactionsUpdated(0), minRelayFee(_minRelayFee) @@ -374,6 +391,8 @@ CTxMemPool::CTxMemPool(const CFeeRate& _minRelayFee) : nTransactionsUpdated(0), // Confirmation times for very-low-fee transactions that take more // than an hour or three to confirm are highly variable. minerPolicyEstimator = new CMinerPolicyEstimator(25); + + utxoHasher.reset(new MempoolUtxoHasher()); } CTxMemPool::~CTxMemPool() @@ -426,6 +445,16 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry& entry) return true; } +const TransactionUtxoHasher& CTxMemPool::GetUtxoHasher() const +{ + return *utxoHasher; +} + +void CTxMemPool::SetUtxoHasherForTesting(std::unique_ptr hasher) +{ + utxoHasher = std::move(hasher); +} + void CTxMemPool::addAddressIndex(const CTxMemPoolEntry &entry, const CCoinsViewCache &view) { LOCK(cs); @@ -578,7 +607,7 @@ void CTxMemPool::remove(const CTransaction& origTx, std::list& rem // happen during chain re-orgs if origTx isn't re-accepted into // the mempool for any reason. for (unsigned int i = 0; i < origTx.vout.size(); i++) { - std::map::iterator it = mapNextTx.find(COutPoint(origTx.GetHash(), i)); + auto it = mapNextTx.find(COutPoint(utxoHasher->GetUtxoHash(origTx), i)); if (it == mapNextTx.end()) continue; txToRemove.push_back(it->second.ptx->GetHash()); @@ -592,7 +621,7 @@ void CTxMemPool::remove(const CTransaction& origTx, std::list& rem const CTransaction& tx = mapTx[hash].GetTx(); if (fRecursive) { for (unsigned int i = 0; i < tx.vout.size(); i++) { - std::map::iterator it = mapNextTx.find(COutPoint(hash, i)); + auto it = mapNextTx.find(COutPoint(utxoHasher->GetUtxoHash(tx), i)); if (it == mapNextTx.end()) continue; txToRemove.push_back(it->second.ptx->GetHash()); @@ -619,7 +648,7 @@ void CTxMemPool::removeCoinbaseSpends(const CCoinsViewCache* pcoins, unsigned in const CTransaction& tx = entry.second.GetTx(); for (const auto& txin : tx.vin) { CTransaction tx2; - if (lookup(txin.prevout.hash, tx2)) + if (lookupOutpoint(txin.prevout.hash, tx2)) continue; const CCoins* coins = pcoins->AccessCoins(txin.prevout.hash); if (fSanityCheck) assert(coins); @@ -704,7 +733,7 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const for (const auto& txin : tx.vin) { // Check that every mempool transaction's inputs refer to available coins, or other mempool tx's. CTransaction tx2; - if (lookup(txin.prevout.hash, tx2)) { + if (lookupOutpoint(txin.prevout.hash, tx2)) { assert(tx2.vout.size() > txin.prevout.n && !tx2.vout[txin.prevout.n].IsNull()); fDependsWait = true; } else { @@ -724,7 +753,7 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const CValidationState state; CTxUndo undo; assert(CheckInputs(tx, state, mempoolDuplicate, false, 0, false, NULL)); - UpdateCoinsWithTransaction(tx, mempoolDuplicate, undo, 1000000); + UpdateCoinsWithTransaction(tx, mempoolDuplicate, undo, *utxoHasher, 1000000); } } unsigned int stepsSinceLastRemove = 0; @@ -739,7 +768,7 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const } else { assert(CheckInputs(entry->GetTx(), state, mempoolDuplicate, false, 0, false, NULL)); CTxUndo undo; - UpdateCoinsWithTransaction(entry->GetTx(), mempoolDuplicate, undo, 1000000); + UpdateCoinsWithTransaction(entry->GetTx(), mempoolDuplicate, undo, *utxoHasher, 1000000); stepsSinceLastRemove = 0; } } @@ -784,6 +813,20 @@ bool CTxMemPool::lookupBareTxid(const uint256& btxid, CTransaction& result) cons return true; } +bool CTxMemPool::lookupOutpoint(const uint256& hash, CTransaction& result) const +{ + /* The TransactionUtxoHasher can only tell us the txid to use once we + know the transaction already. Thus we check both txid and bare txid + in our index; if one of them matches, we then cross-check with the + then-known transaction that it actually should hash to that UTXO. */ + if (lookup(hash, result) && utxoHasher->GetUtxoHash(result) == hash) + return true; + if (lookupBareTxid(hash, result) && utxoHasher->GetUtxoHash(result) == hash) + return true; + + return false; +} + CFeeRate CTxMemPool::estimateFee(int nBlocks) const { LOCK(cs); @@ -863,7 +906,7 @@ bool CCoinsViewMemPool::GetCoins(const uint256& txid, CCoins& coins) const // conflict with the underlying cache, and it cannot have pruned entries (as it contains full) // transactions. First checking the underlying cache risks returning a pruned entry instead. CTransaction tx; - if (mempool.lookup(txid, tx)) { + if (mempool.lookupOutpoint(txid, tx)) { coins = CCoins(tx, MEMPOOL_HEIGHT); return true; } @@ -872,5 +915,9 @@ bool CCoinsViewMemPool::GetCoins(const uint256& txid, CCoins& coins) const bool CCoinsViewMemPool::HaveCoins(const uint256& txid) const { - return mempool.exists(txid) || base->HaveCoins(txid); + CTransaction dummy; + if (mempool.lookupOutpoint(txid, dummy)) + return true; + + return base->HaveCoins(txid); } diff --git a/divi/src/txmempool.h b/divi/src/txmempool.h index 538cc8e2b..d42002253 100644 --- a/divi/src/txmempool.h +++ b/divi/src/txmempool.h @@ -8,6 +8,7 @@ #define BITCOIN_TXMEMPOOL_H #include +#include #include "addressindex.h" #include "spentindex.h" @@ -18,6 +19,7 @@ #include "sync.h" class CAutoFile; +class TransactionUtxoHasher; inline double AllowFreeThreshold() { @@ -101,6 +103,7 @@ class CTxMemPool bool fSanityCheck; //! Normally false, true if -checkmempool or -regtest unsigned int nTransactionsUpdated; CMinerPolicyEstimator* minerPolicyEstimator; + std::unique_ptr utxoHasher; CFeeRate minRelayFee; //! Passed to constructor to avoid dependency on main uint64_t totalTxSize; //! sum of all mempool tx' byte sizes @@ -152,6 +155,13 @@ class CTxMemPool unsigned int GetTransactionsUpdated() const; void AddTransactionsUpdated(unsigned int n); + /** Returns the UTXO hasher instance used in the mempool. */ + const TransactionUtxoHasher& GetUtxoHasher() const; + + /** Replaces the UTXO hasher used in the mempool with the given instance, + * which allows dependency injection for unit tests. */ + void SetUtxoHasherForTesting(std::unique_ptr hasher); + void addAddressIndex(const CTxMemPoolEntry &entry, const CCoinsViewCache &view); bool getAddressIndex(std::vector > &addresses, std::vector > &results); @@ -192,6 +202,11 @@ class CTxMemPool bool lookup(const uint256& hash, CTransaction& result) const; bool lookupBareTxid(const uint256& btxid, CTransaction& result) const; + /** Looks up a transaction by its outpoint for spending. This takes the + * state of segwit light activation into account for deciding whether to + * use the normal txid or the bare txid map. */ + bool lookupOutpoint(const uint256& hash, CTransaction& result) const; + /** Estimate fee rate needed to get into the next nBlocks */ CFeeRate estimateFee(int nBlocks) const;