From f6c3e84ca0ecdf30b700aff6f4d1b5f5076dc106 Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Wed, 9 Jun 2021 16:45:39 +0200 Subject: [PATCH 01/10] Add ToString method for CCoins. This adds CCoins::ToString as a helper method that prints a representation of a CCoins instance. The method is not called for now in the code, but can be useful both for future logging and also just for ad-hoc log statements when debugging CCoins-related code. --- divi/src/coins.cpp | 14 ++++++++++++++ divi/src/coins.h | 1 + 2 files changed, 15 insertions(+) diff --git a/divi/src/coins.cpp b/divi/src/coins.cpp index d675654d1..75969e6d7 100644 --- a/divi/src/coins.cpp +++ b/divi/src/coins.cpp @@ -8,6 +8,7 @@ #include "random.h" #include +#include #include "FeeAndPriorityCalculator.h" @@ -139,6 +140,19 @@ bool CCoins::Spend(const int nPos) return Spend(nPos, undo); } +std::string CCoins::ToString() const +{ + std::ostringstream res; + res << "CCoins(coinbase=" << fCoinBase << ", coinstake=" << fCoinStake; + res << ", height=" << nHeight << ", version=" << nVersion << "):"; + res << std::endl; + + for (const auto& out : vout) + res << " " << out.ToString() << std::endl; + + return res.str(); +} + bool CCoinsView::GetCoins(const uint256& txid, CCoins& coins) const { return false; } bool CCoinsView::HaveCoins(const uint256& txid) const { return false; } diff --git a/divi/src/coins.h b/divi/src/coins.h index 479ac8189..6ffd86eed 100644 --- a/divi/src/coins.h +++ b/divi/src/coins.h @@ -227,6 +227,7 @@ class CCoins Cleanup(); } + std::string ToString() const; }; From fcb85b50096364e9ce49aebc8cccf73d389179fb Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Tue, 8 Jun 2021 14:30:55 +0200 Subject: [PATCH 02/10] Type alias for DependingTransactionsMap. Introduce and use a type alias for the std::map type that holds the data for mapping outputs to depending orphan transactions in BlockMemoryPoolTransactionCollector. This is quite a length type when spelled out, and used in a couple of functions as argument. Using a type alias makes it more readable and also easier to adjust in the future as needed. --- divi/src/BlockMemoryPoolTransactionCollector.cpp | 10 +++++----- divi/src/BlockMemoryPoolTransactionCollector.h | 11 +++++++---- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/divi/src/BlockMemoryPoolTransactionCollector.cpp b/divi/src/BlockMemoryPoolTransactionCollector.cpp index 68f44549d..44fcfe7e0 100644 --- a/divi/src/BlockMemoryPoolTransactionCollector.cpp +++ b/divi/src/BlockMemoryPoolTransactionCollector.cpp @@ -106,7 +106,7 @@ void BlockMemoryPoolTransactionCollector::RecordOrphanTransaction ( std::shared_ptr& porphan, const CTransaction& tx, const CTxIn& txin, - std::map>>& dependentTransactions) const + DependingTransactionsMap& dependentTransactions) const { if (porphan == nullptr) porphan = std::make_shared(&tx); @@ -138,7 +138,7 @@ void BlockMemoryPoolTransactionCollector::ComputeTransactionPriority ( } void BlockMemoryPoolTransactionCollector::AddDependingTransactionsToPriorityQueue ( - std::map>>& dependentTransactions, + DependingTransactionsMap& dependentTransactions, const uint256& hash, std::vector& vecPriority, TxPriorityCompare& comparer) const @@ -184,7 +184,7 @@ void BlockMemoryPoolTransactionCollector::AddTransactionToBlock ( std::vector BlockMemoryPoolTransactionCollector::PrioritizeMempoolTransactions ( const int& nHeight, - std::map>>& dependentTransactions, + DependingTransactionsMap& dependentTransactions, CCoinsViewCache& view) const { std::vector vecPriority; @@ -272,7 +272,7 @@ std::vector BlockMemoryPoolTransactionCollector::Pri std::vector& vecPriority, const int& nHeight, CCoinsViewCache& view, - std::map>>& dependentTransactions) const + DependingTransactionsMap& dependentTransactions) const { std::vector prioritizedTransactions; @@ -350,7 +350,7 @@ void BlockMemoryPoolTransactionCollector::AddTransactionsToBlockIfPossible ( CCoinsViewCache& view, CBlockTemplate& blocktemplate) const { - std::map>> dependentTransactions; + DependingTransactionsMap dependentTransactions; std::vector vecPriority = PrioritizeMempoolTransactions(nHeight, dependentTransactions, view); diff --git a/divi/src/BlockMemoryPoolTransactionCollector.h b/divi/src/BlockMemoryPoolTransactionCollector.h index 011bf6c3b..3e5f86187 100644 --- a/divi/src/BlockMemoryPoolTransactionCollector.h +++ b/divi/src/BlockMemoryPoolTransactionCollector.h @@ -50,6 +50,8 @@ class CChain; class BlockMemoryPoolTransactionCollector: public I_BlockTransactionCollector { private: + using DependingTransactionsMap = std::map>>; + CCoinsViewCache* baseCoinsViewCache_; const CChain& activeChain_; CTxMemPool& mempool_; @@ -58,13 +60,14 @@ class BlockMemoryPoolTransactionCollector: public I_BlockTransactionCollector const unsigned blockMaxSize_; const unsigned blockPrioritySize_; const unsigned blockMinSize_; + private: void UpdateTime(CBlockHeader* block, const CBlockIndex* pindexPrev) const; void RecordOrphanTransaction ( std::shared_ptr& porphan, const CTransaction& tx, const CTxIn& txin, - std::map>>& mapDependers) const; + DependingTransactionsMap& mapDependers) const; void ComputeTransactionPriority ( double& dPriority, @@ -74,7 +77,7 @@ class BlockMemoryPoolTransactionCollector: public I_BlockTransactionCollector std::vector& vecPriority, const CTransaction* mempoolTx) const; void AddDependingTransactionsToPriorityQueue ( - std::map>>& mapDependers, + DependingTransactionsMap& mapDependers, const uint256& hash, std::vector& vecPriority, TxPriorityCompare& comparer) const; @@ -93,7 +96,7 @@ class BlockMemoryPoolTransactionCollector: public I_BlockTransactionCollector std::vector PrioritizeMempoolTransactions ( const int& nHeight, - std::map>>& mapDependers, + DependingTransactionsMap& mapDependers, CCoinsViewCache& view) const; void PrioritizeFeePastPrioritySize ( @@ -107,7 +110,7 @@ class BlockMemoryPoolTransactionCollector: public I_BlockTransactionCollector std::vector& vecPriority, const int& nHeight, CCoinsViewCache& view, - std::map>>& mapDependers) const; + DependingTransactionsMap& mapDependers) const; void AddTransactionsToBlockIfPossible ( const int& nHeight, CCoinsViewCache& view, From 84f94e838ad046b5d447e789135c22b8d61d0644 Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Tue, 8 Jun 2021 14:32:15 +0200 Subject: [PATCH 03/10] Remove begin_ptr/end_ptr from Logging.cpp. Logging.cpp defined a "private" version of the begin_ptr and end_ptr templates that are also defined in serialize.h. This replaces that with simply using serialize.h. In addition to avoiding code duplication, this also makes sure that the code still works even if Logging.cpp were ever to include directly or indirectly serialize.h (where without this commit, it would lead to an ambiguous template call). --- divi/src/Logging.cpp | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/divi/src/Logging.cpp b/divi/src/Logging.cpp index 3c0a2b7ff..d114f356f 100644 --- a/divi/src/Logging.cpp +++ b/divi/src/Logging.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include bool fDebug = false; @@ -25,17 +26,6 @@ LOG_FORMAT_WITH_TOSTRING(uint256) namespace { -template -inline T* begin_ptr(std::vector& v) -{ - return v.empty() ? NULL : &v[0]; -} -/** Get begin pointer of vector (const version) */ -template -inline const T* begin_ptr(const std::vector& v) -{ - return v.empty() ? NULL : &v[0]; -} FILE* fileout = nullptr; From 0c977828a431760c10877de1ea8b2393ef6d7617 Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Tue, 8 Jun 2021 15:11:03 +0200 Subject: [PATCH 04/10] Remove dead definition. TransactionsByHash is defined in VaultManager.h, but actually never used in the code at all. --- divi/src/VaultManager.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/divi/src/VaultManager.h b/divi/src/VaultManager.h index cb3ac75a1..5215592ac 100644 --- a/divi/src/VaultManager.h +++ b/divi/src/VaultManager.h @@ -14,7 +14,6 @@ class CTransaction; class CBlock; class CWalletTx; class uint256; -using TransactionsByHash = std::map; class I_VaultManagerDatabase; class WalletTransactionRecord; @@ -46,4 +45,4 @@ class VaultManager const CWalletTx& GetTransaction(const uint256&) const; const ManagedScripts& GetManagedScriptLimits() const; }; -#endif// VAULT_MANAGER_H \ No newline at end of file +#endif// VAULT_MANAGER_H From 5e8433c0400b248fb2a311c12d65668bb08c07eb Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Tue, 8 Jun 2021 15:11:22 +0200 Subject: [PATCH 05/10] Use auto for iterators. Instead of spelling out iterator types, use auto. --- divi/src/OrphanTransactions.cpp | 4 ++-- divi/src/test/coins_tests.cpp | 6 +++--- divi/src/txdb.cpp | 2 +- divi/src/txmempool.cpp | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/divi/src/OrphanTransactions.cpp b/divi/src/OrphanTransactions.cpp index 83ef7216f..b117f7099 100644 --- a/divi/src/OrphanTransactions.cpp +++ b/divi/src/OrphanTransactions.cpp @@ -27,7 +27,7 @@ std::map > mapOrphanTransactionsByPrev; const std::set& GetOrphanSpendingTransactionIds(const uint256& txHash) { static std::set emptySet; - std::map >::const_iterator it = mapOrphanTransactionsByPrev.find(txHash); + const auto it = mapOrphanTransactionsByPrev.find(txHash); if(it == mapOrphanTransactionsByPrev.end()) return emptySet; return it->second; @@ -76,7 +76,7 @@ void EraseOrphanTx(uint256 hash) if (it == mapOrphanTransactions.end()) return; BOOST_FOREACH (const CTxIn& txin, it->second.tx.vin) { - std::map >::iterator itPrev = mapOrphanTransactionsByPrev.find(txin.prevout.hash); + const auto itPrev = mapOrphanTransactionsByPrev.find(txin.prevout.hash); if (itPrev == mapOrphanTransactionsByPrev.end()) continue; itPrev->second.erase(hash); diff --git a/divi/src/test/coins_tests.cpp b/divi/src/test/coins_tests.cpp index 2916b797b..2f71a26d3 100644 --- a/divi/src/test/coins_tests.cpp +++ b/divi/src/test/coins_tests.cpp @@ -24,7 +24,7 @@ class CCoinsViewTest : public CCoinsView public: bool GetCoins(const uint256& txid, CCoins& coins) const { - std::map::const_iterator it = map_.find(txid); + auto it = map_.find(txid); if (it == map_.end()) { return false; } @@ -46,7 +46,7 @@ class CCoinsViewTest : public CCoinsView bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock) { - for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end(); ) { + for (auto it = mapCoins.begin(); it != mapCoins.end(); ) { map_[it->first] = it->second.coins; if (it->second.coins.IsPruned() && insecure_rand() % 3 == 0) { // Randomly delete empty entries on write. @@ -128,7 +128,7 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) // Once every 1000 iterations and at the end, verify the full cache. if (insecure_rand() % 1000 == 1 || i == NUM_SIMULATION_ITERATIONS - 1) { - for (std::map::iterator it = result.begin(); it != result.end(); it++) { + for (auto it = result.begin(); it != result.end(); it++) { const CCoins* coins = stack.back()->AccessCoins(it->first); if (coins) { BOOST_CHECK(*coins == it->second); diff --git a/divi/src/txdb.cpp b/divi/src/txdb.cpp index 1b9b636f6..532cc291a 100644 --- a/divi/src/txdb.cpp +++ b/divi/src/txdb.cpp @@ -84,7 +84,7 @@ bool CCoinsViewDB::BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock) CLevelDBBatch batch; size_t count = 0; size_t changed = 0; - for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end();) { + for (auto it = mapCoins.begin(); it != mapCoins.end();) { if (it->second.flags & CCoinsCacheEntry::DIRTY) { BatchWriteCoins(batch, it->first, it->second.coins); changed++; diff --git a/divi/src/txmempool.cpp b/divi/src/txmempool.cpp index 025c89026..6b0e14028 100644 --- a/divi/src/txmempool.cpp +++ b/divi/src/txmempool.cpp @@ -389,7 +389,7 @@ void CTxMemPool::pruneSpent(const uint256& hashTx, CCoins& coins) { LOCK(cs); - std::map::iterator it = mapNextTx.lower_bound(COutPoint(hashTx, 0)); + auto it = mapNextTx.lower_bound(COutPoint(hashTx, 0)); // iterate over all COutPoints in mapNextTx whose hash equals the provided hashTx while (it != mapNextTx.end() && it->first.hash == hashTx) { From 788307c7c98249fc034c8f09436a003a53b647b4 Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Tue, 8 Jun 2021 14:35:40 +0200 Subject: [PATCH 06/10] Use const& for some arguments. Pass some arguments as const& instead of by value, and mark some variables as const where it is possible. Also includes a trivial change to serialisation, where we explicitly list the two fields in COutPoint instead of serialising as flat data. --- divi/src/addressindex.h | 4 ++-- divi/src/main.cpp | 2 +- divi/src/main.h | 2 +- divi/src/primitives/transaction.cpp | 7 +++++-- divi/src/primitives/transaction.h | 7 ++++--- divi/src/rpcmisc.cpp | 4 ++-- divi/src/rpcrawtransaction.cpp | 2 +- divi/src/spentindex.h | 2 +- divi/src/txdb.cpp | 2 +- divi/src/txdb.h | 2 +- divi/src/wallet.cpp | 2 +- divi/src/wallet.h | 2 +- 12 files changed, 21 insertions(+), 17 deletions(-) diff --git a/divi/src/addressindex.h b/divi/src/addressindex.h index 0c54284d3..0cd6c5694 100644 --- a/divi/src/addressindex.h +++ b/divi/src/addressindex.h @@ -18,7 +18,7 @@ struct CMempoolAddressDelta uint256 prevhash; unsigned int prevout; - CMempoolAddressDelta(int64_t t, CAmount a, uint256 hash, unsigned int out) { + CMempoolAddressDelta(int64_t t, CAmount a, const uint256& hash, unsigned int out) { time = t; amount = a; prevhash = hash; @@ -240,7 +240,7 @@ struct CAddressUnspentKey { index = ser_readdata32(s); } - CAddressUnspentKey(unsigned int addressType, uint160 addressHash, uint256 txid, size_t indexValue) { + CAddressUnspentKey(unsigned int addressType, uint160 addressHash, const uint256& txid, size_t indexValue) { type = addressType; hashBytes = addressHash; txhash = txid; diff --git a/divi/src/main.cpp b/divi/src/main.cpp index bc8cb5eb0..09809557e 100755 --- a/divi/src/main.cpp +++ b/divi/src/main.cpp @@ -709,7 +709,7 @@ bool GetAddressUnspent(bool addresIndexEnabled, return true; } -bool GetSpentIndex(CSpentIndexKey &key, CSpentIndexValue &value) +bool GetSpentIndex(const CSpentIndexKey &key, CSpentIndexValue &value) { if (!fSpentIndex) return false; diff --git a/divi/src/main.h b/divi/src/main.h index 7238ee81e..e13590fbb 100644 --- a/divi/src/main.h +++ b/divi/src/main.h @@ -184,6 +184,6 @@ bool GetAddressUnspent(bool addresIndexEnabled, int type, std::vector > &unspentOutputs); -bool GetSpentIndex(CSpentIndexKey &key, CSpentIndexValue &value); +bool GetSpentIndex(const CSpentIndexKey &key, CSpentIndexValue &value); #endif // BITCOIN_MAIN_H diff --git a/divi/src/primitives/transaction.cpp b/divi/src/primitives/transaction.cpp index 06df3b609..1f4a9c2cc 100644 --- a/divi/src/primitives/transaction.cpp +++ b/divi/src/primitives/transaction.cpp @@ -18,7 +18,10 @@ COutPoint::COutPoint() { SetNull(); } -COutPoint::COutPoint(uint256 hashIn, uint32_t nIn) { hash = hashIn; n = nIn; } +COutPoint::COutPoint(const uint256& hashIn, uint32_t nIn) + : hash(hashIn), n(nIn) +{} + void COutPoint::SetNull() { hash.SetNull(); n = (uint32_t) -1; } bool COutPoint::IsNull() const { return (hash.IsNull() && n == (uint32_t) -1); } bool operator<(const COutPoint& a, const COutPoint& b) @@ -80,7 +83,7 @@ CTxIn::CTxIn(COutPoint prevoutIn, CScript scriptSigIn, uint32_t nSequenceIn) nSequence = nSequenceIn; } -CTxIn::CTxIn(uint256 hashPrevTx, uint32_t nOut, CScript scriptSigIn, uint32_t nSequenceIn) +CTxIn::CTxIn(const uint256& hashPrevTx, uint32_t nOut, CScript scriptSigIn, uint32_t nSequenceIn) { prevout = COutPoint(hashPrevTx, nOut); scriptSig = scriptSigIn; diff --git a/divi/src/primitives/transaction.h b/divi/src/primitives/transaction.h index 2d83c9a8a..fb30da67a 100644 --- a/divi/src/primitives/transaction.h +++ b/divi/src/primitives/transaction.h @@ -22,13 +22,14 @@ class COutPoint uint32_t n; COutPoint(); - COutPoint(uint256 hashIn, uint32_t nIn); + COutPoint(const uint256& hashIn, uint32_t nIn); ADD_SERIALIZE_METHODS; template inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) { - READWRITE(FLATDATA(*this)); + READWRITE(hash); + READWRITE(n); } void SetNull(); @@ -56,7 +57,7 @@ class CTxIn CTxIn(); explicit CTxIn(COutPoint prevoutIn, CScript scriptSigIn=CScript(), uint32_t nSequenceIn=std::numeric_limits::max()); - CTxIn(uint256 hashPrevTx, uint32_t nOut, CScript scriptSigIn=CScript(), uint32_t nSequenceIn=std::numeric_limits::max()); + CTxIn(const uint256& hashPrevTx, uint32_t nOut, CScript scriptSigIn=CScript(), uint32_t nSequenceIn=std::numeric_limits::max()); ADD_SERIALIZE_METHODS; diff --git a/divi/src/rpcmisc.cpp b/divi/src/rpcmisc.cpp index 6f39de845..4c071a47f 100644 --- a/divi/src/rpcmisc.cpp +++ b/divi/src/rpcmisc.cpp @@ -75,7 +75,7 @@ bool GetAddressIndex(bool addresIndexEnabled, std::string GetWarnings(std::string strFor); -bool GetSpentIndex(CSpentIndexKey &key, CSpentIndexValue &value); +bool GetSpentIndex(const CSpentIndexKey &key, CSpentIndexValue &value); bool GetAddressUnspent(bool addresIndexEnabled, CBlockTreeDB* pblocktree, @@ -1016,7 +1016,7 @@ Value getspentinfo(const Array& params, bool fHelp) uint256 txid = ParseHashV(txidValue, "txid"); int outputIndex = indexValue.get_int(); - CSpentIndexKey key(txid, outputIndex); + const CSpentIndexKey key(txid, outputIndex); CSpentIndexValue value; if (!GetSpentIndex(key, value)) { diff --git a/divi/src/rpcrawtransaction.cpp b/divi/src/rpcrawtransaction.cpp index 916891519..5d674444f 100644 --- a/divi/src/rpcrawtransaction.cpp +++ b/divi/src/rpcrawtransaction.cpp @@ -93,7 +93,7 @@ void TxToJSONExpanded(const CTransaction& tx, const uint256 hashBlock, Object& e // Add address and value info if spentindex enabled CSpentIndexValue spentInfo; - CSpentIndexKey spentKey(txin.prevout.hash, txin.prevout.n); + const CSpentIndexKey spentKey(txin.prevout.hash, txin.prevout.n); if (GetSpentIndex(spentKey, spentInfo)) { in.push_back(Pair("value", ValueFromAmount(spentInfo.satoshis))); in.push_back(Pair("valueSat", spentInfo.satoshis)); diff --git a/divi/src/spentindex.h b/divi/src/spentindex.h index 8994f04f1..6a05effb2 100644 --- a/divi/src/spentindex.h +++ b/divi/src/spentindex.h @@ -22,7 +22,7 @@ struct CSpentIndexKey { READWRITE(outputIndex); } - CSpentIndexKey(uint256 t, unsigned int i) { + CSpentIndexKey(const uint256& t, unsigned int i) { txid = t; outputIndex = i; } diff --git a/divi/src/txdb.cpp b/divi/src/txdb.cpp index 532cc291a..278bab1fd 100644 --- a/divi/src/txdb.cpp +++ b/divi/src/txdb.cpp @@ -437,7 +437,7 @@ bool CBlockTreeDB::ReadAddressIndex(uint160 addressHash, int type, return true; } -bool CBlockTreeDB::ReadSpentIndex(CSpentIndexKey &key, CSpentIndexValue &value) { +bool CBlockTreeDB::ReadSpentIndex(const CSpentIndexKey &key, CSpentIndexValue &value) { return Read(make_pair(DB_SPENTINDEX, key), value); } diff --git a/divi/src/txdb.h b/divi/src/txdb.h index d21eb9e15..83bd882e6 100644 --- a/divi/src/txdb.h +++ b/divi/src/txdb.h @@ -71,7 +71,7 @@ class CBlockTreeDB : public CLevelDBWrapper bool ReadAddressIndex(uint160 addressHash, int type, std::vector > &addressIndex, int start = 0, int end = 0); - bool ReadSpentIndex(CSpentIndexKey &key, CSpentIndexValue &value); + bool ReadSpentIndex(const CSpentIndexKey &key, CSpentIndexValue &value); bool UpdateSpentIndex(const std::vector >&vect); bool UpdateAddressUnspentIndex(const std::vector >&vect); bool ReadAddressUnspentIndex(uint160 addressHash, int type, diff --git a/divi/src/wallet.cpp b/divi/src/wallet.cpp index 7facaca86..a50170693 100644 --- a/divi/src/wallet.cpp +++ b/divi/src/wallet.cpp @@ -2831,7 +2831,7 @@ void CWallet::UnlockAllCoins() setLockedCoins.clear(); } -bool CWallet::IsLockedCoin(uint256 hash, unsigned int n) const +bool CWallet::IsLockedCoin(const uint256& hash, unsigned int n) const { AssertLockHeld(cs_wallet); // setLockedCoins COutPoint outpt(hash, n); diff --git a/divi/src/wallet.h b/divi/src/wallet.h index 2ac72231d..fdd320f76 100644 --- a/divi/src/wallet.h +++ b/divi/src/wallet.h @@ -214,7 +214,7 @@ class CWallet : public CCryptoKeyStore, public NotificationInterface, public I_K CAmount& nValueRet); bool IsTrusted(const CWalletTx& walletTransaction) const; - bool IsLockedCoin(uint256 hash, unsigned int n) const; + bool IsLockedCoin(const uint256& hash, unsigned int n) const; void LockCoin(const COutPoint& output); void UnlockCoin(const COutPoint& output); void UnlockAllCoins(); From 259145a99456c6ee5df712bfea75c4dab931f555 Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Tue, 8 Jun 2021 14:36:44 +0200 Subject: [PATCH 07/10] Use emplace_back. Instead of constructing an object and inserting it, use emplace_back for adding to transaction inputs. --- divi/src/divi-tx.cpp | 3 +-- divi/src/rpcrawtransaction.cpp | 3 +-- divi/src/test/BlockSignature_tests.cpp | 4 ++-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/divi/src/divi-tx.cpp b/divi/src/divi-tx.cpp index 0df96e8c4..1cf38fc22 100644 --- a/divi/src/divi-tx.cpp +++ b/divi/src/divi-tx.cpp @@ -204,8 +204,7 @@ static void MutateTxAddInput(CMutableTransaction& tx, const string& strInput) throw runtime_error("invalid TX input vout"); // append to transaction input list - CTxIn txin(txid, vout); - tx.vin.push_back(txin); + tx.vin.emplace_back(COutPoint(txid, vout)); } static void MutateTxAddOutAddr(CMutableTransaction& tx, const string& strInput) diff --git a/divi/src/rpcrawtransaction.cpp b/divi/src/rpcrawtransaction.cpp index 5d674444f..ce5b324a1 100644 --- a/divi/src/rpcrawtransaction.cpp +++ b/divi/src/rpcrawtransaction.cpp @@ -481,8 +481,7 @@ Value createrawtransaction(const Array& params, bool fHelp) if (nOutput < 0) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, vout must be positive"); - CTxIn in(COutPoint(txid, nOutput)); - rawTx.vin.push_back(in); + rawTx.vin.emplace_back(COutPoint(txid, nOutput)); } set setAddress; diff --git a/divi/src/test/BlockSignature_tests.cpp b/divi/src/test/BlockSignature_tests.cpp index 2cb2d6348..07f3932ee 100644 --- a/divi/src/test/BlockSignature_tests.cpp +++ b/divi/src/test/BlockSignature_tests.cpp @@ -78,8 +78,8 @@ class BlockSignatureTestFixture CScript outputScript = (isP2SH)? ConvertToP2SH(redeemScript):redeemScript; vaultKeyStore.AddCScript(redeemScript); - coinstake.vin.push_back( CTxIn(uint256S("0x25"),0, createDummyVaultScriptSig(redeemScript)) ); - coinstake.vout.push_back( CTxOut(0,outputScript) ); + coinstake.vin.emplace_back(uint256S("0x25"), 0, createDummyVaultScriptSig(redeemScript)); + coinstake.vout.emplace_back(0, outputScript); block.vtx.emplace_back(); block.vtx.push_back(coinstake); From 85daeca236172cf30f682866e17cce6633926cb1 Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Tue, 8 Jun 2021 14:48:15 +0200 Subject: [PATCH 08/10] Use override keyword. In CCoinsViewMemPool, mark the overriding methods actually as override. This ensures that the compiler will enforce and catch any potential changes to the superclass method signature. --- divi/src/coins.h | 18 +++++++++--------- divi/src/init.cpp | 2 +- divi/src/test/coins_tests.cpp | 10 +++++----- divi/src/txdb.h | 10 +++++----- divi/src/txmempool.h | 4 ++-- 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/divi/src/coins.h b/divi/src/coins.h index 6ffd86eed..2b61512f8 100644 --- a/divi/src/coins.h +++ b/divi/src/coins.h @@ -311,12 +311,12 @@ class CCoinsViewBacked : public CCoinsView public: CCoinsViewBacked(CCoinsView* viewIn); - bool GetCoins(const uint256& txid, CCoins& coins) const; - bool HaveCoins(const uint256& txid) const; - uint256 GetBestBlock() const; + bool GetCoins(const uint256& txid, CCoins& coins) const override; + bool HaveCoins(const uint256& txid) const override; + uint256 GetBestBlock() const override; void SetBackend(CCoinsView& viewIn); - bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock); - bool GetStats(CCoinsStats& stats) const; + bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock) override; + bool GetStats(CCoinsStats& stats) const override; }; class CCoinsViewCache; @@ -372,11 +372,11 @@ class CCoinsViewCache : public CCoinsViewBacked ~CCoinsViewCache(); // Standard CCoinsView methods - bool GetCoins(const uint256& txid, CCoins& coins) const; - bool HaveCoins(const uint256& txid) const; - uint256 GetBestBlock() const; + bool GetCoins(const uint256& txid, CCoins& coins) const override; + bool HaveCoins(const uint256& txid) const override; + uint256 GetBestBlock() const override; void SetBestBlock(const uint256& hashBlock); - bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock); + bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock) override; /** * Return a pointer to CCoins in the cache, or NULL if not found. This is diff --git a/divi/src/init.cpp b/divi/src/init.cpp index a8c320be8..28ac9814e 100644 --- a/divi/src/init.cpp +++ b/divi/src/init.cpp @@ -168,7 +168,7 @@ class CCoinsViewErrorCatcher : public CCoinsViewBacked { public: CCoinsViewErrorCatcher(CCoinsView* view) : CCoinsViewBacked(view) {} - bool GetCoins(const uint256& txid, CCoins& coins) const + bool GetCoins(const uint256& txid, CCoins& coins) const override { try { return CCoinsViewBacked::GetCoins(txid, coins); diff --git a/divi/src/test/coins_tests.cpp b/divi/src/test/coins_tests.cpp index 2f71a26d3..a6cfd876c 100644 --- a/divi/src/test/coins_tests.cpp +++ b/divi/src/test/coins_tests.cpp @@ -22,7 +22,7 @@ class CCoinsViewTest : public CCoinsView std::map map_; public: - bool GetCoins(const uint256& txid, CCoins& coins) const + bool GetCoins(const uint256& txid, CCoins& coins) const override { auto it = map_.find(txid); if (it == map_.end()) { @@ -36,15 +36,15 @@ class CCoinsViewTest : public CCoinsView return true; } - bool HaveCoins(const uint256& txid) const + bool HaveCoins(const uint256& txid) const override { CCoins coins; return GetCoins(txid, coins); } - uint256 GetBestBlock() const { return hashBestBlock_; } + uint256 GetBestBlock() const override { return hashBestBlock_; } - bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock) + bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock) override { for (auto it = mapCoins.begin(); it != mapCoins.end(); ) { map_[it->first] = it->second.coins; @@ -59,7 +59,7 @@ class CCoinsViewTest : public CCoinsView return true; } - bool GetStats(CCoinsStats& stats) const { return false; } + bool GetStats(CCoinsStats& stats) const override { return false; } }; } diff --git a/divi/src/txdb.h b/divi/src/txdb.h index 83bd882e6..08c265f24 100644 --- a/divi/src/txdb.h +++ b/divi/src/txdb.h @@ -39,11 +39,11 @@ class CCoinsViewDB : public CCoinsView public: CCoinsViewDB(size_t nCacheSize, bool fMemory = false, bool fWipe = false); - bool GetCoins(const uint256& txid, CCoins& coins) const; - bool HaveCoins(const uint256& txid) const; - uint256 GetBestBlock() const; - bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock); - bool GetStats(CCoinsStats& stats) const; + bool GetCoins(const uint256& txid, CCoins& coins) const override; + bool HaveCoins(const uint256& txid) const override; + uint256 GetBestBlock() const override; + bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock) override; + bool GetStats(CCoinsStats& stats) const override; }; /** Access to the block database (blocks/index/) */ diff --git a/divi/src/txmempool.h b/divi/src/txmempool.h index 8664e1d0d..472281cbd 100644 --- a/divi/src/txmempool.h +++ b/divi/src/txmempool.h @@ -222,8 +222,8 @@ class CCoinsViewMemPool : public CCoinsViewBacked public: CCoinsViewMemPool(CCoinsView* baseIn, CTxMemPool& mempoolIn); - bool GetCoins(const uint256& txid, CCoins& coins) const; - bool HaveCoins(const uint256& txid) const; + bool GetCoins(const uint256& txid, CCoins& coins) const override; + bool HaveCoins(const uint256& txid) const override; }; #endif // BITCOIN_TXMEMPOOL_H From 6c4fd02759dc756f240c0c0b3f37d3783f51096c Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Tue, 8 Jun 2021 14:50:43 +0200 Subject: [PATCH 09/10] Fix typo in variable name. Fix a mistyped variable name. Also use the previous transaction hash directly when emitting a notification, instead of relying on the prevout value of the spending transaction. This is more explicit and decouples the meaning (e.g. for segwit light in the future). --- divi/src/wallet.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/divi/src/wallet.cpp b/divi/src/wallet.cpp index a50170693..24953166d 100644 --- a/divi/src/wallet.cpp +++ b/divi/src/wallet.cpp @@ -2263,10 +2263,10 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey) // Notify that old coins are spent { - set updated_hahes; + std::set updated_hashes; BOOST_FOREACH (const CTxIn& txin, wtxNew.vin) { // notify only once - if (updated_hahes.find(txin.prevout.hash) != updated_hahes.end()) continue; + if (updated_hashes.find(txin.prevout.hash) != updated_hashes.end()) continue; CWalletTx* coinPtr = const_cast(GetWalletTx(txin.prevout.hash)); if(!coinPtr) @@ -2275,8 +2275,8 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey) assert(coinPtr); } coinPtr->RecomputeCachedQuantities(); - NotifyTransactionChanged(this, txin.prevout.hash, CT_UPDATED); - updated_hahes.insert(txin.prevout.hash); + NotifyTransactionChanged(this, coinPtr->GetHash(), CT_UPDATED); + updated_hashes.insert(txin.prevout.hash); } } From 022d6537695e26547766e444c33ee8b5b43a4890 Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Tue, 8 Jun 2021 14:44:51 +0200 Subject: [PATCH 10/10] Lookup also bare txid in rpcrawtransaction. rpcrawtransaction.cpp contains two places that use heuristic transaction lookups by txid: One in the spent index to see if there are transations spending its outputs, and one in sendrawtransaction to check the UTXO set as a cheap heuristic on whether or not the transaction is already contained in the blockchain. This extends both places to try looking up both the txid and also the bare txid. At most one of them can ever match (unless there is a collision in SHA-256), so it doesn't hurt, but it will ensure that the code works independent of the state of segwit light. --- divi/src/rpcrawtransaction.cpp | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/divi/src/rpcrawtransaction.cpp b/divi/src/rpcrawtransaction.cpp index ce5b324a1..562f90523 100644 --- a/divi/src/rpcrawtransaction.cpp +++ b/divi/src/rpcrawtransaction.cpp @@ -120,10 +120,15 @@ void TxToJSONExpanded(const CTransaction& tx, const uint256 hashBlock, Object& e ScriptPubKeyToJSON(txout.scriptPubKey, o, true); out.push_back(Pair("scriptPubKey", o)); - // Add spent information if spentindex is enabled + // Add spent information if spentindex is enabled. We don't know + // whether or not segwit light is in effect for the transaction, + // so we simply try looking up by both txid and bare txid as at + // most one of them can match anyway. CSpentIndexValue spentInfo; - CSpentIndexKey spentKey(txid, i); - if (GetSpentIndex(spentKey, spentInfo)) { + bool found = GetSpentIndex(CSpentIndexKey(txid, i), spentInfo); + if (!found) + found = GetSpentIndex(CSpentIndexKey(tx.GetBareTxid(), i), spentInfo); + if (found) { out.push_back(Pair("spentTxId", spentInfo.txid.GetHex())); out.push_back(Pair("spentIndex", (int)spentInfo.inputIndex)); out.push_back(Pair("spentHeight", spentInfo.blockHeight)); @@ -878,9 +883,19 @@ Value sendrawtransaction(const Array& params, bool fHelp) fOverrideFees = params[1].get_bool(); CCoinsViewCache& view = *pcoinsTip; + const bool fHaveMempool = mempool.exists(hashTx); + + /* We use the UTXO set as heuristic about whether or not a transaction + has already been confirmed. This is not 100% accurate (as all outputs + could have been spent), but useful in practice. Since we don't know + whether or not segwit light has been in effect for the transaction, + we just try locating both txid and bare txid as at most one of them + can match anyway. */ const CCoins* existingCoins = view.AccessCoins(hashTx); - bool fHaveMempool = mempool.exists(hashTx); - bool fHaveChain = existingCoins && existingCoins->nHeight < 1000000000; + if (existingCoins == nullptr) + existingCoins = view.AccessCoins(tx.GetBareTxid()); + const bool fHaveChain = existingCoins && existingCoins->nHeight < 1000000000; + if (!fHaveMempool && !fHaveChain) { // push to local node and sync with wallets std::pair feeTotalsAndStatus = ComputeFeeTotalsAndIfInputsAreKnown(tx);