diff options
Diffstat (limited to 'src')
56 files changed, 634 insertions, 745 deletions
diff --git a/src/Makefile.test.include b/src/Makefile.test.include index abfd66efad..019799eb0b 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -83,6 +83,7 @@ BITCOIN_TESTS =\ test/sigopcount_tests.cpp \ test/skiplist_tests.cpp \ test/streams_tests.cpp \ + test/sync_tests.cpp \ test/timedata_tests.cpp \ test/torcontrol_tests.cpp \ test/transaction_tests.cpp \ diff --git a/src/bench/mempool_eviction.cpp b/src/bench/mempool_eviction.cpp index 0ec7c158cc..3908a7d231 100644 --- a/src/bench/mempool_eviction.cpp +++ b/src/bench/mempool_eviction.cpp @@ -16,7 +16,7 @@ static void AddTx(const CTransactionRef& tx, const CAmount& nFee, CTxMemPool& po bool spendsCoinbase = false; unsigned int sigOpCost = 4; LockPoints lp; - pool.addUnchecked(tx->GetHash(), CTxMemPoolEntry( + pool.addUnchecked(CTxMemPoolEntry( tx, nFee, nTime, nHeight, spendsCoinbase, sigOpCost, lp)); } diff --git a/src/blockfilter.cpp b/src/blockfilter.cpp index 38fcfacf7f..91623fe70a 100644 --- a/src/blockfilter.cpp +++ b/src/blockfilter.cpp @@ -208,7 +208,7 @@ static GCSFilter::ElementSet BasicFilterElements(const CBlock& block, for (const CTransactionRef& tx : block.vtx) { for (const CTxOut& txout : tx->vout) { const CScript& script = txout.scriptPubKey; - if (script[0] == OP_RETURN) continue; + if (script.empty() || script[0] == OP_RETURN) continue; elements.emplace(script.begin(), script.end()); } } @@ -216,6 +216,7 @@ static GCSFilter::ElementSet BasicFilterElements(const CBlock& block, for (const CTxUndo& tx_undo : block_undo.vtxundo) { for (const Coin& prevout : tx_undo.vprevout) { const CScript& script = prevout.out.scriptPubKey; + if (script.empty()) continue; elements.emplace(script.begin(), script.end()); } } diff --git a/src/fs.cpp b/src/fs.cpp index 570ed3e2ee..2dbc13643b 100644 --- a/src/fs.cpp +++ b/src/fs.cpp @@ -1,5 +1,12 @@ #include <fs.h> +#ifndef WIN32 +#include <fcntl.h> +#else +#include <codecvt> +#include <windows.h> +#endif + namespace fsbridge { FILE *fopen(const fs::path& p, const char *mode) @@ -7,9 +14,82 @@ FILE *fopen(const fs::path& p, const char *mode) return ::fopen(p.string().c_str(), mode); } -FILE *freopen(const fs::path& p, const char *mode, FILE *stream) +#ifndef WIN32 + +static std::string GetErrorReason() { + return std::strerror(errno); +} + +FileLock::FileLock(const fs::path& file) +{ + fd = open(file.string().c_str(), O_RDWR); + if (fd == -1) { + reason = GetErrorReason(); + } +} + +FileLock::~FileLock() +{ + if (fd != -1) { + close(fd); + } +} + +bool FileLock::TryLock() +{ + if (fd == -1) { + return false; + } + struct flock lock; + lock.l_type = F_WRLCK; + lock.l_whence = SEEK_SET; + lock.l_start = 0; + lock.l_len = 0; + if (fcntl(fd, F_SETLK, &lock) == -1) { + reason = GetErrorReason(); + return false; + } + return true; +} +#else + +static std::string GetErrorReason() { + wchar_t* err; + FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, + nullptr, GetLastError(), MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), reinterpret_cast<WCHAR*>(&err), 0, nullptr); + std::wstring err_str(err); + LocalFree(err); + return std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>>().to_bytes(err_str); +} + +FileLock::FileLock(const fs::path& file) +{ + hFile = CreateFileW(file.wstring().c_str(), GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); + if (hFile == INVALID_HANDLE_VALUE) { + reason = GetErrorReason(); + } +} + +FileLock::~FileLock() +{ + if (hFile != INVALID_HANDLE_VALUE) { + CloseHandle(hFile); + } +} + +bool FileLock::TryLock() { - return ::freopen(p.string().c_str(), mode, stream); + if (hFile == INVALID_HANDLE_VALUE) { + return false; + } + _OVERLAPPED overlapped = {0}; + if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, 0, 0, &overlapped)) { + reason = GetErrorReason(); + return false; + } + return true; } +#endif } // fsbridge @@ -18,7 +18,26 @@ namespace fs = boost::filesystem; /** Bridge operations to C stdio */ namespace fsbridge { FILE *fopen(const fs::path& p, const char *mode); - FILE *freopen(const fs::path& p, const char *mode, FILE *stream); + + class FileLock + { + public: + FileLock() = delete; + FileLock(const FileLock&) = delete; + FileLock(FileLock&&) = delete; + explicit FileLock(const fs::path& file); + ~FileLock(); + bool TryLock(); + std::string GetReason() { return reason; } + + private: + std::string reason; +#ifndef WIN32 + int fd = -1; +#else + void* hFile = (void*)-1; // INVALID_HANDLE_VALUE +#endif + }; }; #endif // BITCOIN_FS_H diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 0fdc3e5ff3..326f7f6b64 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -69,7 +69,7 @@ class WorkQueue { private: /** Mutex protects entire object */ - std::mutex cs; + Mutex cs; std::condition_variable cond; std::deque<std::unique_ptr<WorkItem>> queue; bool running; @@ -88,7 +88,7 @@ public: /** Enqueue a work item */ bool Enqueue(WorkItem* item) { - std::unique_lock<std::mutex> lock(cs); + LOCK(cs); if (queue.size() >= maxDepth) { return false; } @@ -102,7 +102,7 @@ public: while (true) { std::unique_ptr<WorkItem> i; { - std::unique_lock<std::mutex> lock(cs); + WAIT_LOCK(cs, lock); while (running && queue.empty()) cond.wait(lock); if (!running) @@ -116,7 +116,7 @@ public: /** Interrupt and exit loops */ void Interrupt() { - std::unique_lock<std::mutex> lock(cs); + LOCK(cs); running = false; cond.notify_all(); } diff --git a/src/init.cpp b/src/init.cpp index 2131a6adc0..27966319d6 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -51,13 +51,13 @@ #ifndef WIN32 #include <signal.h> +#include <sys/stat.h> #endif #include <boost/algorithm/string/classification.hpp> #include <boost/algorithm/string/replace.hpp> #include <boost/algorithm/string/split.hpp> #include <boost/bind.hpp> -#include <boost/interprocess/sync/file_lock.hpp> #include <boost/thread.hpp> #include <openssl/crypto.h> @@ -561,17 +561,17 @@ static void BlockNotifyCallback(bool initialSync, const CBlockIndex *pBlockIndex } static bool fHaveGenesis = false; -static CWaitableCriticalSection cs_GenesisWait; -static CConditionVariable condvar_GenesisWait; +static Mutex g_genesis_wait_mutex; +static std::condition_variable g_genesis_wait_cv; static void BlockNotifyGenesisWait(bool, const CBlockIndex *pBlockIndex) { if (pBlockIndex != nullptr) { { - WaitableLock lock_GenesisWait(cs_GenesisWait); + LOCK(g_genesis_wait_mutex); fHaveGenesis = true; } - condvar_GenesisWait.notify_all(); + g_genesis_wait_cv.notify_all(); } } @@ -1661,12 +1661,12 @@ bool AppInitMain() // Wait for genesis block to be processed { - WaitableLock lock(cs_GenesisWait); + WAIT_LOCK(g_genesis_wait_mutex, lock); // We previously could hang here if StartShutdown() is called prior to // ThreadImport getting started, so instead we just wait on a timer to // check ShutdownRequested() regularly. while (!fHaveGenesis && !ShutdownRequested()) { - condvar_GenesisWait.wait_for(lock, std::chrono::milliseconds(500)); + g_genesis_wait_cv.wait_for(lock, std::chrono::milliseconds(500)); } uiInterface.NotifyBlockTip_disconnect(BlockNotifyGenesisWait); } diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 01467f3a1c..566ae37509 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -39,12 +39,11 @@ public: bool commit(WalletValueMap value_map, WalletOrderForm order_form, - std::string from_account, std::string& reject_reason) override { LOCK2(cs_main, m_wallet.cs_wallet); CValidationState state; - if (!m_wallet.CommitTransaction(m_tx, std::move(value_map), std::move(order_form), std::move(from_account), m_key, g_connman.get(), state)) { + if (!m_wallet.CommitTransaction(m_tx, std::move(value_map), std::move(order_form), m_key, g_connman.get(), state)) { reject_reason = state.GetRejectReason(); return false; } diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index ae54d42442..44240a5726 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -289,7 +289,6 @@ public: //! Send pending transaction and commit to wallet. virtual bool commit(WalletValueMap value_map, WalletOrderForm order_form, - std::string from_account, std::string& reject_reason) = 0; }; diff --git a/src/logging.cpp b/src/logging.cpp index 6557dddffb..0ae4f8121e 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -219,13 +219,13 @@ void BCLog::Logger::LogPrintStr(const std::string &str) // reopen the log file, if requested if (m_reopen_file) { m_reopen_file = false; - m_fileout = fsbridge::freopen(m_file_path, "a", m_fileout); - if (!m_fileout) { - return; + FILE* new_fileout = fsbridge::fopen(m_file_path, "a"); + if (new_fileout) { + setbuf(new_fileout, nullptr); // unbuffered + fclose(m_fileout); + m_fileout = new_fileout; } - setbuf(m_fileout, nullptr); // unbuffered } - FileWriteStr(strTimestamped, m_fileout); } } diff --git a/src/net.cpp b/src/net.cpp index df2cb54b7a..e9e9cf4c92 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1227,7 +1227,7 @@ void CConnman::ThreadSocketHandler() if(vNodesSize != nPrevNodeCount) { nPrevNodeCount = vNodesSize; if(clientInterface) - clientInterface->NotifyNumConnectionsChanged(nPrevNodeCount); + clientInterface->NotifyNumConnectionsChanged(vNodesSize); } // @@ -2065,7 +2065,7 @@ void CConnman::ThreadMessageHandler() pnode->Release(); } - std::unique_lock<std::mutex> lock(mutexMsgProc); + WAIT_LOCK(mutexMsgProc, lock); if (!fMoreWork) { condMsgProc.wait_until(lock, std::chrono::steady_clock::now() + std::chrono::milliseconds(100), [this] { return fMsgProcWake; }); } @@ -2347,7 +2347,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) flagInterruptMsgProc = false; { - std::unique_lock<std::mutex> lock(mutexMsgProc); + LOCK(mutexMsgProc); fMsgProcWake = false; } @@ -427,7 +427,7 @@ private: bool fMsgProcWake; std::condition_variable condMsgProc; - std::mutex mutexMsgProc; + Mutex mutexMsgProc; std::atomic<bool> flagInterruptMsgProc; CThreadInterrupt interruptNet; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index a6a8814dff..9f950831c4 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -70,7 +70,7 @@ struct COrphanTx { NodeId fromPeer; int64_t nTimeExpire; }; -static CCriticalSection g_cs_orphans; +CCriticalSection g_cs_orphans; std::map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(g_cs_orphans); void EraseOrphansFor(NodeId peer); diff --git a/src/netbase.cpp b/src/netbase.cpp index 9750173987..4b63757f3d 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -19,8 +19,6 @@ #include <fcntl.h> #endif -#include <boost/algorithm/string/case_conv.hpp> // for to_lower() - #if !defined(MSG_NOSIGNAL) #define MSG_NOSIGNAL 0 #endif @@ -37,7 +35,7 @@ static const int SOCKS5_RECV_TIMEOUT = 20 * 1000; static std::atomic<bool> interruptSocks5Recv(false); enum Network ParseNetwork(std::string net) { - boost::to_lower(net); + Downcase(net); if (net == "ipv4") return NET_IPV4; if (net == "ipv6") return NET_IPV6; if (net == "onion") return NET_ONION; diff --git a/src/policy/fees.h b/src/policy/fees.h index 136fb481f7..2733c5a7de 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -22,51 +22,6 @@ class CTxMemPoolEntry; class CTxMemPool; class TxConfirmStats; -/** \class CBlockPolicyEstimator - * The BlockPolicyEstimator is used for estimating the feerate needed - * for a transaction to be included in a block within a certain number of - * blocks. - * - * At a high level the algorithm works by grouping transactions into buckets - * based on having similar feerates and then tracking how long it - * takes transactions in the various buckets to be mined. It operates under - * the assumption that in general transactions of higher feerate will be - * included in blocks before transactions of lower feerate. So for - * example if you wanted to know what feerate you should put on a transaction to - * be included in a block within the next 5 blocks, you would start by looking - * at the bucket with the highest feerate transactions and verifying that a - * sufficiently high percentage of them were confirmed within 5 blocks and - * then you would look at the next highest feerate bucket, and so on, stopping at - * the last bucket to pass the test. The average feerate of transactions in this - * bucket will give you an indication of the lowest feerate you can put on a - * transaction and still have a sufficiently high chance of being confirmed - * within your desired 5 blocks. - * - * Here is a brief description of the implementation: - * When a transaction enters the mempool, we track the height of the block chain - * at entry. All further calculations are conducted only on this set of "seen" - * transactions. Whenever a block comes in, we count the number of transactions - * in each bucket and the total amount of feerate paid in each bucket. Then we - * calculate how many blocks Y it took each transaction to be mined. We convert - * from a number of blocks to a number of periods Y' each encompassing "scale" - * blocks. This is tracked in 3 different data sets each up to a maximum - * number of periods. Within each data set we have an array of counters in each - * feerate bucket and we increment all the counters from Y' up to max periods - * representing that a tx was successfully confirmed in less than or equal to - * that many periods. We want to save a history of this information, so at any - * time we have a counter of the total number of transactions that happened in a - * given feerate bucket and the total number that were confirmed in each of the - * periods or less for any bucket. We save this history by keeping an - * exponentially decaying moving average of each one of these stats. This is - * done for a different decay in each of the 3 data sets to keep relevant data - * from different time horizons. Furthermore we also keep track of the number - * unmined (in mempool or left mempool without being included in a block) - * transactions in each bucket and for how many blocks they have been - * outstanding and use both of these numbers to increase the number of transactions - * we've seen in that feerate bucket when calculating an estimate for any number - * of confirmations below the number of blocks they've been outstanding. - */ - /* Identifier for each of the 3 different TxConfirmStats which will track * history over different time horizons. */ enum class FeeEstimateHorizon { @@ -130,7 +85,50 @@ struct FeeCalculation int returnedTarget = 0; }; -/** +/** \class CBlockPolicyEstimator + * The BlockPolicyEstimator is used for estimating the feerate needed + * for a transaction to be included in a block within a certain number of + * blocks. + * + * At a high level the algorithm works by grouping transactions into buckets + * based on having similar feerates and then tracking how long it + * takes transactions in the various buckets to be mined. It operates under + * the assumption that in general transactions of higher feerate will be + * included in blocks before transactions of lower feerate. So for + * example if you wanted to know what feerate you should put on a transaction to + * be included in a block within the next 5 blocks, you would start by looking + * at the bucket with the highest feerate transactions and verifying that a + * sufficiently high percentage of them were confirmed within 5 blocks and + * then you would look at the next highest feerate bucket, and so on, stopping at + * the last bucket to pass the test. The average feerate of transactions in this + * bucket will give you an indication of the lowest feerate you can put on a + * transaction and still have a sufficiently high chance of being confirmed + * within your desired 5 blocks. + * + * Here is a brief description of the implementation: + * When a transaction enters the mempool, we track the height of the block chain + * at entry. All further calculations are conducted only on this set of "seen" + * transactions. Whenever a block comes in, we count the number of transactions + * in each bucket and the total amount of feerate paid in each bucket. Then we + * calculate how many blocks Y it took each transaction to be mined. We convert + * from a number of blocks to a number of periods Y' each encompassing "scale" + * blocks. This is tracked in 3 different data sets each up to a maximum + * number of periods. Within each data set we have an array of counters in each + * feerate bucket and we increment all the counters from Y' up to max periods + * representing that a tx was successfully confirmed in less than or equal to + * that many periods. We want to save a history of this information, so at any + * time we have a counter of the total number of transactions that happened in a + * given feerate bucket and the total number that were confirmed in each of the + * periods or less for any bucket. We save this history by keeping an + * exponentially decaying moving average of each one of these stats. This is + * done for a different decay in each of the 3 data sets to keep relevant data + * from different time horizons. Furthermore we also keep track of the number + * unmined (in mempool or left mempool without being included in a block) + * transactions in each bucket and for how many blocks they have been + * outstanding and use both of these numbers to increase the number of transactions + * we've seen in that feerate bucket when calculating an estimate for any number + * of confirmations below the number of blocks they've been outstanding. + * * We want to be able to estimate feerates that are needed on tx's to be included in * a certain number of blocks. Every time a block is added to the best chain, this class records * stats on the transactions included in that block diff --git a/src/prevector.h b/src/prevector.h index 7a13b98214..6ddb6f321f 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -248,32 +248,32 @@ public: prevector() : _size(0), _union{{}} {} - explicit prevector(size_type n) : _size(0) { + explicit prevector(size_type n) : prevector() { resize(n); } - explicit prevector(size_type n, const T& val) : _size(0) { + explicit prevector(size_type n, const T& val) : prevector() { change_capacity(n); _size += n; fill(item_ptr(0), n, val); } template<typename InputIterator> - prevector(InputIterator first, InputIterator last) : _size(0) { + prevector(InputIterator first, InputIterator last) : prevector() { size_type n = last - first; change_capacity(n); _size += n; fill(item_ptr(0), first, last); } - prevector(const prevector<N, T, Size, Diff>& other) : _size(0) { + prevector(const prevector<N, T, Size, Diff>& other) : prevector() { size_type n = other.size(); change_capacity(n); _size += n; fill(item_ptr(0), other.begin(), other.end()); } - prevector(prevector<N, T, Size, Diff>&& other) : _size(0) { + prevector(prevector<N, T, Size, Diff>&& other) : prevector() { swap(other); } diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index 61cd6f76cc..dcaca10557 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -212,7 +212,7 @@ private: void setEncryptionStatus(int status); /** Set the hd-enabled status as shown in the UI. - @param[in] status current hd enabled status + @param[in] hdEnabled current hd enabled status @see WalletModel::EncryptionStatus */ void setHDStatus(int hdEnabled); diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 00fc73574e..ad13b20ebe 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -142,7 +142,7 @@ public: * - Within single quotes, no escaping is possible and no special interpretation takes place * * @param[in] node optional node to execute command on - * @param[out] result stringified Result from the executed command(chain) + * @param[out] strResult stringified result from the executed command(chain) * @param[in] strCommand Command line to split * @param[in] fExecute set true if you want the command to be executed * @param[out] pstrFilteredOut Command line, filtered to remove any sensitive data diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 18a5bda9c3..f7cc94ae32 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -253,7 +253,7 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(WalletModelTransaction &tran auto& newTx = transaction.getWtx(); std::string rejectReason; - if (!newTx->commit({} /* mapValue */, std::move(vOrderForm), {} /* fromAccount */, rejectReason)) + if (!newTx->commit({} /* mapValue */, std::move(vOrderForm), rejectReason)) return SendCoinsReturn(TransactionCommitFailed, QString::fromStdString(rejectReason)); CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); diff --git a/src/random.cpp b/src/random.cpp index 6c5ad5ac96..503d5b3636 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -12,6 +12,7 @@ #include <wincrypt.h> #endif #include <logging.h> // for LogPrint() +#include <sync.h> // for WAIT_LOCK #include <utiltime.h> // for GetTime() #include <stdlib.h> @@ -295,7 +296,7 @@ void RandAddSeedSleep() } -static std::mutex cs_rng_state; +static Mutex cs_rng_state; static unsigned char rng_state[32] = {0}; static uint64_t rng_counter = 0; @@ -305,7 +306,7 @@ static void AddDataToRng(void* data, size_t len) { hasher.Write((const unsigned char*)data, len); unsigned char buf[64]; { - std::unique_lock<std::mutex> lock(cs_rng_state); + WAIT_LOCK(cs_rng_state, lock); hasher.Write(rng_state, sizeof(rng_state)); hasher.Write((const unsigned char*)&rng_counter, sizeof(rng_counter)); ++rng_counter; @@ -337,7 +338,7 @@ void GetStrongRandBytes(unsigned char* out, int num) // Combine with and update state { - std::unique_lock<std::mutex> lock(cs_rng_state); + WAIT_LOCK(cs_rng_state, lock); hasher.Write(rng_state, sizeof(rng_state)); hasher.Write((const unsigned char*)&rng_counter, sizeof(rng_counter)); ++rng_counter; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index a77fea8ea8..f0d767bbfc 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -50,7 +50,7 @@ struct CUpdatedBlock int height; }; -static std::mutex cs_blockchange; +static Mutex cs_blockchange; static std::condition_variable cond_blockchange; static CUpdatedBlock latestblock; @@ -225,7 +225,7 @@ static UniValue waitfornewblock(const JSONRPCRequest& request) CUpdatedBlock block; { - std::unique_lock<std::mutex> lock(cs_blockchange); + WAIT_LOCK(cs_blockchange, lock); block = latestblock; if(timeout) cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&block]{return latestblock.height != block.height || latestblock.hash != block.hash || !IsRPCRunning(); }); @@ -267,7 +267,7 @@ static UniValue waitforblock(const JSONRPCRequest& request) CUpdatedBlock block; { - std::unique_lock<std::mutex> lock(cs_blockchange); + WAIT_LOCK(cs_blockchange, lock); if(timeout) cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&hash]{return latestblock.hash == hash || !IsRPCRunning();}); else @@ -310,7 +310,7 @@ static UniValue waitforblockheight(const JSONRPCRequest& request) CUpdatedBlock block; { - std::unique_lock<std::mutex> lock(cs_blockchange); + WAIT_LOCK(cs_blockchange, lock); if(timeout) cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&height]{return latestblock.height >= height || !IsRPCRunning();}); else diff --git a/src/rpc/blockchain.h b/src/rpc/blockchain.h index 544bc62c36..add335eb8a 100644 --- a/src/rpc/blockchain.h +++ b/src/rpc/blockchain.h @@ -16,8 +16,7 @@ class UniValue; static constexpr int NUM_GETBLOCKSTATS_PERCENTILES = 5; /** - * Get the difficulty of the net wrt to the given block index, or the chain tip if - * not provided. + * Get the difficulty of the net wrt to the given block index. * * @return A floating point number that is a multiple of the main net minimum * difficulty (4295032833 hashes). diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 623b0bd86a..9cc4e77768 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -470,7 +470,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) { checktxtime = std::chrono::steady_clock::now() + std::chrono::minutes(1); - WaitableLock lock(g_best_block_mutex); + WAIT_LOCK(g_best_block_mutex, lock); while (g_best_block == hashWatchedChain && IsRPCRunning()) { if (g_best_block_cv.wait_until(lock, checktxtime) == std::cv_status::timeout) diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index e46bf2f765..7d7e83fea8 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -16,7 +16,6 @@ #include <boost/bind.hpp> #include <boost/signals2/signal.hpp> -#include <boost/algorithm/string/case_conv.hpp> // for to_upper() #include <boost/algorithm/string/classification.hpp> #include <boost/algorithm/string/split.hpp> @@ -192,9 +191,7 @@ std::string CRPCTable::help(const std::string& strCommand, const JSONRPCRequest& if (!category.empty()) strRet += "\n"; category = pcmd->category; - std::string firstLetter = category.substr(0,1); - boost::to_upper(firstLetter); - strRet += "== " + firstLetter + category.substr(1) + " ==\n"; + strRet += "== " + Capitalize(category) + " ==\n"; } } strRet += strHelp + "\n"; diff --git a/src/sync.cpp b/src/sync.cpp index 255eb4f00b..c9aa98dcd6 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -100,7 +100,11 @@ static void potential_deadlock_detected(const std::pair<void*, void*>& mismatch, } LogPrintf(" %s\n", i.second.ToString()); } - assert(false); + if (g_debug_lockorder_abort) { + fprintf(stderr, "Assertion failed: detected inconsistent lock order at %s:%i, details in debug log.\n", __FILE__, __LINE__); + abort(); + } + throw std::logic_error("potential deadlock detected"); } static void push_lock(void* c, const CLockLocation& locklocation) @@ -189,4 +193,6 @@ void DeleteLock(void* cs) } } +bool g_debug_lockorder_abort = true; + #endif /* DEBUG_LOCKORDER */ diff --git a/src/sync.h b/src/sync.h index 11c1253757..40709bdd7f 100644 --- a/src/sync.h +++ b/src/sync.h @@ -46,14 +46,42 @@ LEAVE_CRITICAL_SECTION(mutex); // no RAII // // /////////////////////////////// +#ifdef DEBUG_LOCKORDER +void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false); +void LeaveCritical(); +std::string LocksHeld(); +void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs); +void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs); +void DeleteLock(void* cs); + /** - * Template mixin that adds -Wthread-safety locking - * annotations to a subset of the mutex API. + * Call abort() if a potential lock order deadlock bug is detected, instead of + * just logging information and throwing a logic_error. Defaults to true, and + * set to false in DEBUG_LOCKORDER unit tests. + */ +extern bool g_debug_lockorder_abort; +#else +void static inline EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {} +void static inline LeaveCritical() {} +void static inline AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs) {} +void static inline AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {} +void static inline DeleteLock(void* cs) {} +#endif +#define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs) +#define AssertLockNotHeld(cs) AssertLockNotHeldInternal(#cs, __FILE__, __LINE__, &cs) + +/** + * Template mixin that adds -Wthread-safety locking annotations and lock order + * checking to a subset of the mutex API. */ template <typename PARENT> class LOCKABLE AnnotatedMixin : public PARENT { public: + ~AnnotatedMixin() { + DeleteLock((void*)this); + } + void lock() EXCLUSIVE_LOCK_FUNCTION() { PARENT::lock(); @@ -68,64 +96,36 @@ public: { return PARENT::try_lock(); } -}; -#ifdef DEBUG_LOCKORDER -void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false); -void LeaveCritical(); -std::string LocksHeld(); -void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs); -void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs); -void DeleteLock(void* cs); -#else -void static inline EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {} -void static inline LeaveCritical() {} -void static inline AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs) {} -void static inline AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {} -void static inline DeleteLock(void* cs) {} -#endif -#define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs) -#define AssertLockNotHeld(cs) AssertLockNotHeldInternal(#cs, __FILE__, __LINE__, &cs) + using UniqueLock = std::unique_lock<PARENT>; +}; /** * Wrapped mutex: supports recursive locking, but no waiting * TODO: We should move away from using the recursive lock by default. */ -class CCriticalSection : public AnnotatedMixin<std::recursive_mutex> -{ -public: - ~CCriticalSection() { - DeleteLock((void*)this); - } -}; +typedef AnnotatedMixin<std::recursive_mutex> CCriticalSection; /** Wrapped mutex: supports waiting but not recursive locking */ -typedef AnnotatedMixin<std::mutex> CWaitableCriticalSection; - -/** Just a typedef for std::condition_variable, can be wrapped later if desired */ -typedef std::condition_variable CConditionVariable; - -/** Just a typedef for std::unique_lock, can be wrapped later if desired */ -typedef std::unique_lock<std::mutex> WaitableLock; +typedef AnnotatedMixin<std::mutex> Mutex; #ifdef DEBUG_LOCKCONTENTION void PrintLockContention(const char* pszName, const char* pszFile, int nLine); #endif -/** Wrapper around std::unique_lock<CCriticalSection> */ -class SCOPED_LOCKABLE CCriticalBlock +/** Wrapper around std::unique_lock style lock for Mutex. */ +template <typename Mutex, typename Base = typename Mutex::UniqueLock> +class SCOPED_LOCKABLE UniqueLock : public Base { private: - std::unique_lock<CCriticalSection> lock; - void Enter(const char* pszName, const char* pszFile, int nLine) { - EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex())); + EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex())); #ifdef DEBUG_LOCKCONTENTION - if (!lock.try_lock()) { + if (!Base::try_lock()) { PrintLockContention(pszName, pszFile, nLine); #endif - lock.lock(); + Base::lock(); #ifdef DEBUG_LOCKCONTENTION } #endif @@ -133,15 +133,15 @@ private: bool TryEnter(const char* pszName, const char* pszFile, int nLine) { - EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex()), true); - lock.try_lock(); - if (!lock.owns_lock()) + EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex()), true); + Base::try_lock(); + if (!Base::owns_lock()) LeaveCritical(); - return lock.owns_lock(); + return Base::owns_lock(); } public: - CCriticalBlock(CCriticalSection& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(mutexIn) : lock(mutexIn, std::defer_lock) + UniqueLock(Mutex& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(mutexIn) : Base(mutexIn, std::defer_lock) { if (fTry) TryEnter(pszName, pszFile, nLine); @@ -149,35 +149,41 @@ public: Enter(pszName, pszFile, nLine); } - CCriticalBlock(CCriticalSection* pmutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(pmutexIn) + UniqueLock(Mutex* pmutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(pmutexIn) { if (!pmutexIn) return; - lock = std::unique_lock<CCriticalSection>(*pmutexIn, std::defer_lock); + *static_cast<Base*>(this) = Base(*pmutexIn, std::defer_lock); if (fTry) TryEnter(pszName, pszFile, nLine); else Enter(pszName, pszFile, nLine); } - ~CCriticalBlock() UNLOCK_FUNCTION() + ~UniqueLock() UNLOCK_FUNCTION() { - if (lock.owns_lock()) + if (Base::owns_lock()) LeaveCritical(); } operator bool() { - return lock.owns_lock(); + return Base::owns_lock(); } }; +template<typename MutexArg> +using DebugLock = UniqueLock<typename std::remove_reference<typename std::remove_pointer<MutexArg>::type>::type>; + #define PASTE(x, y) x ## y #define PASTE2(x, y) PASTE(x, y) -#define LOCK(cs) CCriticalBlock PASTE2(criticalblock, __COUNTER__)(cs, #cs, __FILE__, __LINE__) -#define LOCK2(cs1, cs2) CCriticalBlock criticalblock1(cs1, #cs1, __FILE__, __LINE__), criticalblock2(cs2, #cs2, __FILE__, __LINE__) -#define TRY_LOCK(cs, name) CCriticalBlock name(cs, #cs, __FILE__, __LINE__, true) +#define LOCK(cs) DebugLock<decltype(cs)> PASTE2(criticalblock, __COUNTER__)(cs, #cs, __FILE__, __LINE__) +#define LOCK2(cs1, cs2) \ + DebugLock<decltype(cs1)> criticalblock1(cs1, #cs1, __FILE__, __LINE__); \ + DebugLock<decltype(cs2)> criticalblock2(cs2, #cs2, __FILE__, __LINE__); +#define TRY_LOCK(cs, name) DebugLock<decltype(cs)> name(cs, #cs, __FILE__, __LINE__, true) +#define WAIT_LOCK(cs, name) DebugLock<decltype(cs)> name(cs, #cs, __FILE__, __LINE__) #define ENTER_CRITICAL_SECTION(cs) \ { \ diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index d2c7c8cb1d..5131fe8235 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -63,7 +63,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) CBlock block(BuildBlockTestCase()); LOCK(pool.cs); - pool.addUnchecked(block.vtx[2]->GetHash(), entry.FromTx(block.vtx[2])); + pool.addUnchecked(entry.FromTx(block.vtx[2])); BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); // Do a simple ShortTxIDs RT @@ -163,7 +163,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) CBlock block(BuildBlockTestCase()); LOCK(pool.cs); - pool.addUnchecked(block.vtx[2]->GetHash(), entry.FromTx(block.vtx[2])); + pool.addUnchecked(entry.FromTx(block.vtx[2])); BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); uint256 txhash; @@ -233,7 +233,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) CBlock block(BuildBlockTestCase()); LOCK(pool.cs); - pool.addUnchecked(block.vtx[1]->GetHash(), entry.FromTx(block.vtx[1])); + pool.addUnchecked(entry.FromTx(block.vtx[1])); BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); uint256 txhash; diff --git a/src/test/blockfilter_tests.cpp b/src/test/blockfilter_tests.cpp index e3cb05f09e..773de343ea 100644 --- a/src/test/blockfilter_tests.cpp +++ b/src/test/blockfilter_tests.cpp @@ -41,7 +41,7 @@ BOOST_AUTO_TEST_CASE(gcsfilter_test) BOOST_AUTO_TEST_CASE(blockfilter_basic_test) { - CScript included_scripts[5], excluded_scripts[2]; + CScript included_scripts[5], excluded_scripts[3]; // First two are outputs on a single transaction. included_scripts[0] << std::vector<unsigned char>(0, 65) << OP_CHECKSIG; @@ -67,6 +67,7 @@ BOOST_AUTO_TEST_CASE(blockfilter_basic_test) CMutableTransaction tx_2; tx_2.vout.emplace_back(300, included_scripts[2]); tx_2.vout.emplace_back(0, excluded_scripts[0]); + tx_2.vout.emplace_back(400, excluded_scripts[2]); // Script is empty CBlock block; block.vtx.push_back(MakeTransactionRef(tx_1)); @@ -74,8 +75,9 @@ BOOST_AUTO_TEST_CASE(blockfilter_basic_test) CBlockUndo block_undo; block_undo.vtxundo.emplace_back(); - block_undo.vtxundo.back().vprevout.emplace_back(CTxOut(400, included_scripts[3]), 1000, true); - block_undo.vtxundo.back().vprevout.emplace_back(CTxOut(500, included_scripts[4]), 10000, false); + block_undo.vtxundo.back().vprevout.emplace_back(CTxOut(500, included_scripts[3]), 1000, true); + block_undo.vtxundo.back().vprevout.emplace_back(CTxOut(600, included_scripts[4]), 10000, false); + block_undo.vtxundo.back().vprevout.emplace_back(CTxOut(700, excluded_scripts[2]), 100000, false); BlockFilter block_filter(BlockFilterType::BASIC, block, block_undo); const GCSFilter& filter = block_filter.GetFilter(); diff --git a/src/test/data/blockfilters.json b/src/test/data/blockfilters.json index 2f00728ff6..134b788eed 100644 --- a/src/test/data/blockfilters.json +++ b/src/test/data/blockfilters.json @@ -3,6 +3,8 @@ [0,"000000000933ea01ad0ee984209779baaec3ced90fa3f408719526f8d77f4943","0100000000000000000000000000000000000000000000000000000000000000000000003ba3edfd7a7b12b27ac72c3e67768f617fc81bc3888a51323a9fb8aa4b1e5e4adae5494dffff001d1aa4ae180101000000010000000000000000000000000000000000000000000000000000000000000000ffffffff4d04ffff001d0104455468652054696d65732030332f4a616e2f32303039204368616e63656c6c6f72206f6e206272696e6b206f66207365636f6e64206261696c6f757420666f722062616e6b73ffffffff0100f2052a01000000434104678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5fac00000000",[],"0000000000000000000000000000000000000000000000000000000000000000","019dfca8","21584579b7eb08997773e5aeff3a7f932700042d0ed2a6129012b7d7ae81b750","Genesis block"], [2,"000000006c02c8ea6e4ff69651f7fcde348fb9d557a06e6957b65552002a7820","0100000006128e87be8b1b4dea47a7247d5528d2702c96826c7a648497e773b800000000e241352e3bec0a95a6217e10c3abb54adfa05abb12c126695595580fb92e222032e7494dffff001d00d235340101000000010000000000000000000000000000000000000000000000000000000000000000ffffffff0e0432e7494d010e062f503253482fffffffff0100f2052a010000002321038a7f6ef1c8ca0c588aa53fa860128077c9e6c11e6830f4d7ee4e763a56b7718fac00000000",[],"d7bdac13a59d745b1add0d2ce852f1a0442e8945fc1bf3848d3cbffd88c24fe1","0174a170","186afd11ef2b5e7e3504f2e8cbf8df28a1fd251fe53d60dff8b1467d1b386cf0",""], [3,"000000008b896e272758da5297bcd98fdc6d97c9b765ecec401e286dc1fdbe10","0100000020782a005255b657696ea057d5b98f34defcf75196f64f6eeac8026c0000000041ba5afc532aae03151b8aa87b65e1594f97504a768e010c98c0add79216247186e7494dffff001d058dc2b60101000000010000000000000000000000000000000000000000000000000000000000000000ffffffff0e0486e7494d0151062f503253482fffffffff0100f2052a01000000232103f6d9ff4c12959445ca5549c811683bf9c88e637b222dd2e0311154c4c85cf423ac00000000",[],"186afd11ef2b5e7e3504f2e8cbf8df28a1fd251fe53d60dff8b1467d1b386cf0","016cf7a0","8d63aadf5ab7257cb6d2316a57b16f517bff1c6388f124ec4c04af1212729d2a",""], +[49291,"0000000018b07dca1b28b4b5a119f6d6e71698ce1ed96f143f54179ce177a19c","02000000abfaf47274223ca2fea22797e44498240e482cb4c2f2baea088962f800000000604b5b52c32305b15d7542071d8b04e750a547500005d4010727694b6e72a776e55d0d51ffff001d211806480201000000010000000000000000000000000000000000000000000000000000000000000000ffffffff0d038bc0000102062f503253482fffffffff01a078072a01000000232102971dd6034ed0cf52450b608d196c07d6345184fcb14deb277a6b82d526a6163dac0000000001000000081cefd96060ecb1c4fbe675ad8a4f8bdc61d634c52b3a1c4116dee23749fe80ff000000009300493046022100866859c21f306538152e83f115bcfbf59ab4bb34887a88c03483a5dff9895f96022100a6dfd83caa609bf0516debc2bf65c3df91813a4842650a1858b3f61cfa8af249014730440220296d4b818bb037d0f83f9f7111665f49532dfdcbec1e6b784526e9ac4046eaa602204acf3a5cb2695e8404d80bf49ab04828bcbe6fc31d25a2844ced7a8d24afbdff01ffffffff1cefd96060ecb1c4fbe675ad8a4f8bdc61d634c52b3a1c4116dee23749fe80ff020000009400483045022100e87899175991aa008176cb553c6f2badbb5b741f328c9845fcab89f8b18cae2302200acce689896dc82933015e7230e5230d5cff8a1ffe82d334d60162ac2c5b0c9601493046022100994ad29d1e7b03e41731a4316e5f4992f0d9b6e2efc40a1ccd2c949b461175c502210099b69fdc2db00fbba214f16e286f6a49e2d8a0d5ffc6409d87796add475478d601ffffffff1e4a6d2d280ea06680d6cf8788ac90344a9c67cca9b06005bbd6d3f6945c8272010000009500493046022100a27400ba52fd842ce07398a1de102f710a10c5599545e6c95798934352c2e4df022100f6383b0b14c9f64b6718139f55b6b9494374755b86bae7d63f5d3e583b57255a01493046022100fdf543292f34e1eeb1703b264965339ec4a450ec47585009c606b3edbc5b617b022100a5fbb1c8de8aaaa582988cdb23622838e38de90bebcaab3928d949aa502a65d401ffffffff1e4a6d2d280ea06680d6cf8788ac90344a9c67cca9b06005bbd6d3f6945c8272020000009400493046022100ac626ac3051f875145b4fe4cfe089ea895aac73f65ab837b1ac30f5d875874fa022100bc03e79fa4b7eb707fb735b95ff6613ca33adeaf3a0607cdcead4cfd3b51729801483045022100b720b04a5c5e2f61b7df0fcf334ab6fea167b7aaede5695d3f7c6973496adbf1022043328c4cc1cdc3e5db7bb895ccc37133e960b2fd3ece98350f774596badb387201ffffffff23a8733e349c97d6cd90f520fdd084ba15ce0a395aad03cd51370602bb9e5db3010000004a00483045022100e8556b72c5e9c0da7371913a45861a61c5df434dfd962de7b23848e1a28c86ca02205d41ceda00136267281be0974be132ac4cda1459fe2090ce455619d8b91045e901ffffffff6856d609b881e875a5ee141c235e2a82f6b039f2b9babe82333677a5570285a6000000006a473044022040a1c631554b8b210fbdf2a73f191b2851afb51d5171fb53502a3a040a38d2c0022040d11cf6e7b41fe1b66c3d08f6ada1aee07a047cb77f242b8ecc63812c832c9a012102bcfad931b502761e452962a5976c79158a0f6d307ad31b739611dac6a297c256ffffffff6856d609b881e875a5ee141c235e2a82f6b039f2b9babe82333677a5570285a601000000930048304502205b109df098f7e932fbf71a45869c3f80323974a826ee2770789eae178a21bfc8022100c0e75615e53ee4b6e32b9bb5faa36ac539e9c05fa2ae6b6de5d09c08455c8b9601483045022009fb7d27375c47bea23b24818634df6a54ecf72d52e0c1268fb2a2c84f1885de022100e0ed4f15d62e7f537da0d0f1863498f9c7c0c0a4e00e4679588c8d1a9eb20bb801ffffffffa563c3722b7b39481836d5edfc1461f97335d5d1e9a23ade13680d0e2c1c371f030000006c493046022100ecc38ae2b1565643dc3c0dad5e961a5f0ea09cab28d024f92fa05c922924157e022100ebc166edf6fbe4004c72bfe8cf40130263f98ddff728c8e67b113dbd621906a601210211a4ed241174708c07206601b44a4c1c29e5ad8b1f731c50ca7e1d4b2a06dc1fffffffff02d0223a00000000001976a91445db0b779c0b9fa207f12a8218c94fc77aff504588ac80f0fa02000000000000000000",["5221033423007d8f263819a2e42becaaf5b06f34cb09919e06304349d950668209eaed21021d69e2b68c3960903b702af7829fadcd80bd89b158150c85c4a75b2c8cb9c39452ae","52210279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f8179821021d69e2b68c3960903b702af7829fadcd80bd89b158150c85c4a75b2c8cb9c39452ae","522102a7ae1e0971fc1689bd66d2a7296da3a1662fd21a53c9e38979e0f090a375c12d21022adb62335f41eb4e27056ac37d462cda5ad783fa8e0e526ed79c752475db285d52ae","52210279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f8179821022adb62335f41eb4e27056ac37d462cda5ad783fa8e0e526ed79c752475db285d52ae","512103b9d1d0e2b4355ec3cdef7c11a5c0beff9e8b8d8372ab4b4e0aaf30e80173001951ae","76a9149144761ebaccd5b4bbdc2a35453585b5637b2f8588ac","522103f1848b40621c5d48471d9784c8174ca060555891ace6d2b03c58eece946b1a9121020ee5d32b54d429c152fdc7b1db84f2074b0564d35400d89d11870f9273ec140c52ae","76a914f4fa1cc7de742d135ea82c17adf0bb9cf5f4fb8388ac"],"ed47705334f4643892ca46396eb3f4196a5e30880589e4009ef38eae895d4a13","0afbc2920af1b027f31f87b592276eb4c32094bb4d3697021b4c6380","b6d98692cec5145f67585f3434ec3c2b3030182e1cb3ec58b855c5c164dfaaa3","Tx pays to empty output script"], +[180480,"00000000fd3ceb2404ff07a785c7fdcc76619edc8ed61bd25134eaa22084366a","020000006058aa080a655aa991a444bd7d1f2defd9a3bbe68aabb69030cf3b4e00000000d2e826bfd7ef0beaa891a7eedbc92cd6a544a6cb61c7bdaa436762eb2123ef9790f5f552ffff001d0002c90f0501000000010000000000000000000000000000000000000000000000000000000000000000ffffffff0e0300c102024608062f503253482fffffffff01c0c6072a01000000232102e769e60137a4df6b0df8ebd387cca44c4c57ae74cc0114a8e8317c8f3bfd85e9ac00000000010000000381a0802911a01ffb025c4dea0bc77963e8c1bb46313b71164c53f72f37fe5248010000000151ffffffffc904b267833d215e2128bd9575242232ac2bc311550c7fc1f0ef6f264b40d14c010000000151ffffffffdf0915666649dba81886519c531649b7b02180b4af67d6885e871299e9d5f775000000000151ffffffff0180817dcb00000000232103bb52138972c48a132fc1f637858c5189607dd0f7fe40c4f20f6ad65f2d389ba4ac0000000001000000018da38b434fba82d66052af74fc5e4e94301b114d9bc03f819dc876398404c8b4010000006c493046022100fe738b7580dc5fb5168e51fc61b5aed211125eb71068031009a22d9bbad752c5022100be5086baa384d40bcab0fa586e4f728397388d86e18b66cc417dc4f7fa4f9878012103f233299455134caa2687bdf15cb0becdfb03bd0ff2ff38e65ec6b7834295c34fffffffff022ebc1400000000001976a9147779b7fba1c1e06b717069b80ca170e8b04458a488ac9879c40f000000001976a9142a0307cd925dbb66b534c4db33003dd18c57015788ac0000000001000000026139a62e3422a602de36c873a225c1d3ca5aeee598539ceecb9f0dc8d1ad0f83010000006b483045022100ad9f32b4a0a2ddc19b5a74eba78123e57616f1b3cfd72ce68c03ea35a3dda1f002200dbd22aa6da17213df5e70dfc3b2611d40f70c98ed9626aa5e2cde9d97461f0a012103ddb295d2f1e8319187738fb4b230fdd9aa29d0e01647f69f6d770b9ab24eea90ffffffff983c82c87cf020040d671956525014d5c2b28c6d948c85e1a522362c0059eeae010000006b4830450221009ca544274c786d30a5d5d25e17759201ea16d3aedddf0b9e9721246f7ef6b32e02202cfa5564b6e87dfd9fd98957820e4d4e6238baeb0f65fe305d91506bb13f5f4f012103c99113deac0d5d044e3ac0346abc02501542af8c8d3759f1382c72ff84e704f7ffffffff02c0c62d00000000001976a914ae19d27efe12f5a886dc79af37ad6805db6f922d88ac70ce2000000000001976a9143b8d051d37a07ea1042067e93efe63dbf73920b988ac000000000100000002be566e8cd9933f0c75c4a82c027f7d0c544d5c101d0607ef6ae5d07b98e7f1dc000000006b483045022036a8cdfd5ea7ebc06c2bfb6e4f942bbf9a1caeded41680d11a3a9f5d8284abad022100cacb92a5be3f39e8bc14db1710910ef7b395fa1e18f45d41c28d914fcdde33be012102bf59abf110b5131fae0a3ce1ec379329b4c896a6ae5d443edb68529cc2bc7816ffffffff96cf67645b76ceb23fe922874847456a15feee1655082ff32d25a6bf2c0dfc90000000006a47304402203471ca2001784a5ac0abab583581f2613523da47ec5f53df833c117b5abd81500220618a2847723d57324f2984678db556dbca1a72230fc7e39df04c2239942ba942012102925c9794fd7bb9f8b29e207d5fc491b1150135a21f505041858889fa4edf436fffffffff026c840f00000000001976a914797fb8777d7991d8284d88bfd421ce520f0f843188ac00ca9a3b000000001976a9146d10f3f592699265d10b106eda37c3ce793f7a8588ac00000000",["","","","76a9142903b138c24be9e070b3e73ec495d77a204615e788ac","76a91433a1941fd9a37b9821d376f5a51bd4b52fa50e2888ac","76a914e4374e8155d0865742ca12b8d4d14d41b57d682f88ac","76a914001fa7459a6cfc64bdc178ba7e7a21603bb2568f88ac","76a914f6039952bc2b307aeec5371bfb96b66078ec17f688ac"],"b109139671dbedc2b6fcd499a5480a7461ae458af8ff9411d819aa64ba6995d1","0db414c859a07e8205876354a210a75042d0463404913d61a8e068e58a3ae2aa080026","a0af77e0a7ed20ea78d2def3200cc24f08217dcd51755c7c7feb0e2ba8316c2d","Tx spends from empty output script"], [926485,"000000000000015d6077a411a8f5cc95caf775ccf11c54e27df75ce58d187313","0000002060bbab0edbf3ef8a49608ee326f8fd75c473b7e3982095e2d100000000000000c30134f8c9b6d2470488d7a67a888f6fa12f8692e0c3411fbfb92f0f68f67eedae03ca57ef13021acc22dc4105010000000001010000000000000000000000000000000000000000000000000000000000000000ffffffff2f0315230e0004ae03ca57043e3d1e1d0c8796bf579aef0c0000000000122f4e696e6a61506f6f6c2f5345475749542fffffffff038427a112000000001976a914876fbb82ec05caa6af7a3b5e5a983aae6c6cc6d688ac0000000000000000266a24aa21a9ed5c748e121c0fe146d973a4ac26fa4a68b0549d46ee22d25f50a5e46fe1b377ee00000000000000002952534b424c4f434b3acd16772ad61a3c5f00287480b720f6035d5e54c9efc71be94bb5e3727f10909001200000000000000000000000000000000000000000000000000000000000000000000000000100000000010145310e878941a1b2bc2d33797ee4d89d95eaaf2e13488063a2aa9a74490f510a0100000023220020b6744de4f6ec63cc92f7c220cdefeeb1b1bed2b66c8e5706d80ec247d37e65a1ffffffff01002d3101000000001976a9143ebc40e411ed3c76f86711507ab952300890397288ac0400473044022001dd489a5d4e2fbd8a3ade27177f6b49296ba7695c40dbbe650ea83f106415fd02200b23a0602d8ff1bdf79dee118205fc7e9b40672bf31563e5741feb53fb86388501483045022100f88f040e90cc5dc6c6189d04718376ac19ed996bf9e4a3c29c3718d90ffd27180220761711f16c9e3a44f71aab55cbc0634907a1fa8bb635d971a9a01d368727bea10169522103b3623117e988b76aaabe3d63f56a4fc88b228a71e64c4cc551d1204822fe85cb2103dd823066e096f72ed617a41d3ca56717db335b1ea47a1b4c5c9dbdd0963acba621033d7c89bd9da29fa8d44db7906a9778b53121f72191184a9fee785c39180e4be153ae00000000010000000120925534261de4dcebb1ed5ab1b62bfe7a3ef968fb111dc2c910adfebc6e3bdf010000006b483045022100f50198f5ae66211a4f485190abe4dc7accdabe3bc214ebc9ea7069b97097d46e0220316a70a03014887086e335fc1b48358d46cd6bdc9af3b57c109c94af76fc915101210316cff587a01a2736d5e12e53551b18d73780b83c3bfb4fcf209c869b11b6415effffffff0220a10700000000001976a91450333046115eaa0ac9e0216565f945070e44573988ac2e7cd01a000000001976a914c01a7ca16b47be50cbdbc60724f701d52d75156688ac00000000010000000203a25f58630d7a1ea52550365fd2156683f56daf6ca73a4b4bbd097e66516322010000006a47304402204efc3d70e4ca3049c2a425025edf22d5ca355f9ec899dbfbbeeb2268533a0f2b02204780d3739653035af4814ea52e1396d021953f948c29754edd0ee537364603dc012103f7a897e4dbecab2264b21917f90664ea8256189ea725d28740cf7ba5d85b5763ffffffff03a25f58630d7a1ea52550365fd2156683f56daf6ca73a4b4bbd097e66516322000000006a47304402202d96defdc5b4af71d6ba28c9a6042c2d5ee7bc6de565d4db84ef517445626e03022022da80320e9e489c8f41b74833dfb6a54a4eb5087cdb46eb663eef0b25caa526012103f7a897e4dbecab2264b21917f90664ea8256189ea725d28740cf7ba5d85b5763ffffffff0200e1f5050000000017a914b7e6f7ff8658b2d1fb107e3d7be7af4742e6b1b3876f88fc00000000001976a914913bcc2be49cb534c20474c4dee1e9c4c317e7eb88ac0000000001000000043ffd60d3818431c495b89be84afac205d5d1ed663009291c560758bbd0a66df5010000006b483045022100f344607de9df42049688dcae8ff1db34c0c7cd25ec05516e30d2bc8f12ac9b2f022060b648f6a21745ea6d9782e17bcc4277b5808326488a1f40d41e125879723d3a012103f7a897e4dbecab2264b21917f90664ea8256189ea725d28740cf7ba5d85b5763ffffffffa5379401cce30f84731ef1ba65ce27edf2cc7ce57704507ebe8714aa16a96b92010000006a473044022020c37a63bf4d7f564c2192528709b6a38ab8271bd96898c6c2e335e5208661580220435c6f1ad4d9305d2c0a818b2feb5e45d443f2f162c0f61953a14d097fd07064012103f7a897e4dbecab2264b21917f90664ea8256189ea725d28740cf7ba5d85b5763ffffffff70e731e193235ff12c3184510895731a099112ffca4b00246c60003c40f843ce000000006a473044022053760f74c29a879e30a17b5f03a5bb057a5751a39f86fa6ecdedc36a1b7db04c022041d41c9b95f00d2d10a0373322a9025dba66c942196bc9d8adeb0e12d3024728012103f7a897e4dbecab2264b21917f90664ea8256189ea725d28740cf7ba5d85b5763ffffffff66b7a71b3e50379c8e85fc18fe3f1a408fc985f257036c34702ba205cef09f6f000000006a4730440220499bf9e2db3db6e930228d0661395f65431acae466634d098612fd80b08459ee022040e069fc9e3c60009f521cef54c38aadbd1251aee37940e6018aadb10f194d6a012103f7a897e4dbecab2264b21917f90664ea8256189ea725d28740cf7ba5d85b5763ffffffff0200e1f5050000000017a9148fc37ad460fdfbd2b44fe446f6e3071a4f64faa6878f447f0b000000001976a914913bcc2be49cb534c20474c4dee1e9c4c317e7eb88ac00000000",["a914feb8a29635c56d9cd913122f90678756bf23887687","76a914c01a7ca16b47be50cbdbc60724f701d52d75156688ac","76a914913bcc2be49cb534c20474c4dee1e9c4c317e7eb88ac","76a914913bcc2be49cb534c20474c4dee1e9c4c317e7eb88ac","76a914913bcc2be49cb534c20474c4dee1e9c4c317e7eb88ac","76a914913bcc2be49cb534c20474c4dee1e9c4c317e7eb88ac","76a914913bcc2be49cb534c20474c4dee1e9c4c317e7eb88ac","76a914913bcc2be49cb534c20474c4dee1e9c4c317e7eb88ac"],"da49977ba1ee0d620a2c4f8f646b03cd0d230f5c6c994722e3ba884889f0be1a","09027acea61b6cc3fb33f5d52f7d088a6b2f75d234e89ca800","4cd9dd007a325199102f1fc0b7d77ca25ee3c84d46018c4353ecfcb56c0d3e7a","Duplicate pushdata 913bcc2be49cb534c20474c4dee1e9c4c317e7eb"], [987876,"0000000000000c00901f2049055e2a437c819d79a3d54fd63e6af796cd7b8a79","000000202694f74969fdb542090e95a56bc8aa2d646e27033850e32f1c5f000000000000f7e53676b3f12d5beb524ed617f2d25f5a93b5f4f52c1ba2678260d72712f8dd0a6dfe5740257e1a4b1768960101000000010000000000000000000000000000000000000000000000000000000000000000ffffffff1603e4120ff9c30a1c216900002f424d4920546573742fffffff0001205fa012000000001e76a914c486de584a735ec2f22da7cd9681614681f92173d83d0aa68688ac00000000",[],"e9d729b72d533c29abe5276d5cf6c152f3723f10efe000b1e0c9ca5265a8beb6","010c0b40","e6137ae5a8424c40da1e5023c16975cc97b09300b4c050e6b1c713add3836c40","Coinbase tx has unparseable output script"], [1263442,"000000006f27ddfe1dd680044a34548f41bed47eba9e6f0b310da21423bc5f33","000000201c8d1a529c39a396db2db234d5ec152fa651a2872966daccbde028b400000000083f14492679151dbfaa1a825ef4c18518e780c1f91044180280a7d33f4a98ff5f45765aaddc001d38333b9a02010000000001010000000000000000000000000000000000000000000000000000000000000000ffffffff230352471300fe5f45765afe94690a000963676d696e6572343208000000000000000000ffffffff024423a804000000001976a914f2c25ac3d59f3d674b1d1d0a25c27339aaac0ba688ac0000000000000000266a24aa21a9edcb26cb3052426b9ebb4d19c819ef87c19677bbf3a7c46ef0855bd1b2abe83491012000000000000000000000000000000000000000000000000000000000000000000000000002000000000101d20978463906ba4ff5e7192494b88dd5eb0de85d900ab253af909106faa22cc5010000000004000000014777ff000000000016001446c29eabe8208a33aa1023c741fa79aa92e881ff0347304402207d7ca96134f2bcfdd6b536536fdd39ad17793632016936f777ebb32c22943fda02206014d2fb8a6aa58279797f861042ba604ebd2f8f61e5bddbd9d3be5a245047b201004b632103eeaeba7ce5dc2470221e9517fb498e8d6bd4e73b85b8be655196972eb9ccd5566754b2752103a40b74d43df244799d041f32ce1ad515a6cd99501701540e38750d883ae21d3a68ac00000000",["002027a5000c7917f785d8fc6e5a55adfca8717ecb973ebb7743849ff956d896a7ed"],"a4a4d6c6034da8aa06f01fe71f1fffbd79e032006b07f6c7a2c60a66aa310c01","0385acb4f0fe889ef0","3588f34fbbc11640f9ed40b2a66a4e096215d50389691309c1dac74d4268aa81","Includes witness data"] diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index fdf7397bff..52bbe96b96 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -31,7 +31,8 @@ struct COrphanTx { NodeId fromPeer; int64_t nTimeExpire; }; -extern std::map<uint256, COrphanTx> mapOrphanTransactions; +extern CCriticalSection g_cs_orphans; +extern std::map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(g_cs_orphans); static CService ip(uint32_t i) { @@ -324,7 +325,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) static CTransactionRef RandomOrphan() { std::map<uint256, COrphanTx>::iterator it; - LOCK(cs_main); + LOCK2(cs_main, g_cs_orphans); it = mapOrphanTransactions.lower_bound(InsecureRand256()); if (it == mapOrphanTransactions.end()) it = mapOrphanTransactions.begin(); @@ -394,7 +395,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans) BOOST_CHECK(!AddOrphanTx(MakeTransactionRef(tx), i)); } - LOCK(cs_main); + LOCK2(cs_main, g_cs_orphans); // Test EraseOrphansFor: for (NodeId i = 0; i < 3; i++) { diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index e416de098e..0e15464fd9 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -63,17 +63,17 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) BOOST_CHECK_EQUAL(testPool.size(), poolSize); // Just the parent: - testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent)); + testPool.addUnchecked(entry.FromTx(txParent)); poolSize = testPool.size(); testPool.removeRecursive(txParent); BOOST_CHECK_EQUAL(testPool.size(), poolSize - 1); // Parent, children, grandchildren: - testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent)); + testPool.addUnchecked(entry.FromTx(txParent)); for (int i = 0; i < 3; i++) { - testPool.addUnchecked(txChild[i].GetHash(), entry.FromTx(txChild[i])); - testPool.addUnchecked(txGrandChild[i].GetHash(), entry.FromTx(txGrandChild[i])); + testPool.addUnchecked(entry.FromTx(txChild[i])); + testPool.addUnchecked(entry.FromTx(txGrandChild[i])); } // Remove Child[0], GrandChild[0] should be removed: poolSize = testPool.size(); @@ -95,8 +95,8 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) // Add children and grandchildren, but NOT the parent (simulate the parent being in a block) for (int i = 0; i < 3; i++) { - testPool.addUnchecked(txChild[i].GetHash(), entry.FromTx(txChild[i])); - testPool.addUnchecked(txGrandChild[i].GetHash(), entry.FromTx(txGrandChild[i])); + testPool.addUnchecked(entry.FromTx(txChild[i])); + testPool.addUnchecked(entry.FromTx(txGrandChild[i])); } // Now remove the parent, as might happen if a block-re-org occurs but the parent cannot be // put into the mempool (maybe because it is non-standard): @@ -128,28 +128,28 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) tx1.vout.resize(1); tx1.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx1.vout[0].nValue = 10 * COIN; - pool.addUnchecked(tx1.GetHash(), entry.Fee(10000LL).FromTx(tx1)); + pool.addUnchecked(entry.Fee(10000LL).FromTx(tx1)); /* highest fee */ CMutableTransaction tx2 = CMutableTransaction(); tx2.vout.resize(1); tx2.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx2.vout[0].nValue = 2 * COIN; - pool.addUnchecked(tx2.GetHash(), entry.Fee(20000LL).FromTx(tx2)); + pool.addUnchecked(entry.Fee(20000LL).FromTx(tx2)); /* lowest fee */ CMutableTransaction tx3 = CMutableTransaction(); tx3.vout.resize(1); tx3.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx3.vout[0].nValue = 5 * COIN; - pool.addUnchecked(tx3.GetHash(), entry.Fee(0LL).FromTx(tx3)); + pool.addUnchecked(entry.Fee(0LL).FromTx(tx3)); /* 2nd highest fee */ CMutableTransaction tx4 = CMutableTransaction(); tx4.vout.resize(1); tx4.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx4.vout[0].nValue = 6 * COIN; - pool.addUnchecked(tx4.GetHash(), entry.Fee(15000LL).FromTx(tx4)); + pool.addUnchecked(entry.Fee(15000LL).FromTx(tx4)); /* equal fee rate to tx1, but newer */ CMutableTransaction tx5 = CMutableTransaction(); @@ -157,7 +157,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) tx5.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx5.vout[0].nValue = 11 * COIN; entry.nTime = 1; - pool.addUnchecked(tx5.GetHash(), entry.Fee(10000LL).FromTx(tx5)); + pool.addUnchecked(entry.Fee(10000LL).FromTx(tx5)); BOOST_CHECK_EQUAL(pool.size(), 5U); std::vector<std::string> sortedOrder; @@ -175,7 +175,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) tx6.vout.resize(1); tx6.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx6.vout[0].nValue = 20 * COIN; - pool.addUnchecked(tx6.GetHash(), entry.Fee(0LL).FromTx(tx6)); + pool.addUnchecked(entry.Fee(0LL).FromTx(tx6)); BOOST_CHECK_EQUAL(pool.size(), 6U); // Check that at this point, tx6 is sorted low sortedOrder.insert(sortedOrder.begin(), tx6.GetHash().ToString()); @@ -198,7 +198,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) BOOST_CHECK_EQUAL(pool.CalculateMemPoolAncestors(entry.Fee(2000000LL).FromTx(tx7), setAncestorsCalculated, 100, 1000000, 1000, 1000000, dummy), true); BOOST_CHECK(setAncestorsCalculated == setAncestors); - pool.addUnchecked(tx7.GetHash(), entry.FromTx(tx7), setAncestors); + pool.addUnchecked(entry.FromTx(tx7), setAncestors); BOOST_CHECK_EQUAL(pool.size(), 7U); // Now tx6 should be sorted higher (high fee child): tx7, tx6, tx2, ... @@ -216,7 +216,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) tx8.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx8.vout[0].nValue = 10 * COIN; setAncestors.insert(pool.mapTx.find(tx7.GetHash())); - pool.addUnchecked(tx8.GetHash(), entry.Fee(0LL).Time(2).FromTx(tx8), setAncestors); + pool.addUnchecked(entry.Fee(0LL).Time(2).FromTx(tx8), setAncestors); // Now tx8 should be sorted low, but tx6/tx both high sortedOrder.insert(sortedOrder.begin(), tx8.GetHash().ToString()); @@ -230,7 +230,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) tx9.vout.resize(1); tx9.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx9.vout[0].nValue = 1 * COIN; - pool.addUnchecked(tx9.GetHash(), entry.Fee(0LL).Time(3).FromTx(tx9), setAncestors); + pool.addUnchecked(entry.Fee(0LL).Time(3).FromTx(tx9), setAncestors); // tx9 should be sorted low BOOST_CHECK_EQUAL(pool.size(), 9U); @@ -256,7 +256,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) BOOST_CHECK_EQUAL(pool.CalculateMemPoolAncestors(entry.Fee(200000LL).Time(4).FromTx(tx10), setAncestorsCalculated, 100, 1000000, 1000, 1000000, dummy), true); BOOST_CHECK(setAncestorsCalculated == setAncestors); - pool.addUnchecked(tx10.GetHash(), entry.FromTx(tx10), setAncestors); + pool.addUnchecked(entry.FromTx(tx10), setAncestors); /** * tx8 and tx9 should both now be sorted higher @@ -301,14 +301,14 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) tx1.vout.resize(1); tx1.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx1.vout[0].nValue = 10 * COIN; - pool.addUnchecked(tx1.GetHash(), entry.Fee(10000LL).FromTx(tx1)); + pool.addUnchecked(entry.Fee(10000LL).FromTx(tx1)); /* highest fee */ CMutableTransaction tx2 = CMutableTransaction(); tx2.vout.resize(1); tx2.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx2.vout[0].nValue = 2 * COIN; - pool.addUnchecked(tx2.GetHash(), entry.Fee(20000LL).FromTx(tx2)); + pool.addUnchecked(entry.Fee(20000LL).FromTx(tx2)); uint64_t tx2Size = GetVirtualTransactionSize(tx2); /* lowest fee */ @@ -316,21 +316,21 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) tx3.vout.resize(1); tx3.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx3.vout[0].nValue = 5 * COIN; - pool.addUnchecked(tx3.GetHash(), entry.Fee(0LL).FromTx(tx3)); + pool.addUnchecked(entry.Fee(0LL).FromTx(tx3)); /* 2nd highest fee */ CMutableTransaction tx4 = CMutableTransaction(); tx4.vout.resize(1); tx4.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx4.vout[0].nValue = 6 * COIN; - pool.addUnchecked(tx4.GetHash(), entry.Fee(15000LL).FromTx(tx4)); + pool.addUnchecked(entry.Fee(15000LL).FromTx(tx4)); /* equal fee rate to tx1, but newer */ CMutableTransaction tx5 = CMutableTransaction(); tx5.vout.resize(1); tx5.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx5.vout[0].nValue = 11 * COIN; - pool.addUnchecked(tx5.GetHash(), entry.Fee(10000LL).FromTx(tx5)); + pool.addUnchecked(entry.Fee(10000LL).FromTx(tx5)); BOOST_CHECK_EQUAL(pool.size(), 5U); std::vector<std::string> sortedOrder; @@ -359,7 +359,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) tx6.vout[0].nValue = 20 * COIN; uint64_t tx6Size = GetVirtualTransactionSize(tx6); - pool.addUnchecked(tx6.GetHash(), entry.Fee(0LL).FromTx(tx6)); + pool.addUnchecked(entry.Fee(0LL).FromTx(tx6)); BOOST_CHECK_EQUAL(pool.size(), 6U); // Ties are broken by hash if (tx3.GetHash() < tx6.GetHash()) @@ -381,7 +381,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) /* set the fee to just below tx2's feerate when including ancestor */ CAmount fee = (20000/tx2Size)*(tx7Size + tx6Size) - 1; - pool.addUnchecked(tx7.GetHash(), entry.Fee(fee).FromTx(tx7)); + pool.addUnchecked(entry.Fee(fee).FromTx(tx7)); BOOST_CHECK_EQUAL(pool.size(), 7U); sortedOrder.insert(sortedOrder.begin()+1, tx7.GetHash().ToString()); CheckSort<ancestor_score>(pool, sortedOrder); @@ -413,7 +413,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) // Check that we sort by min(feerate, ancestor_feerate): // set the fee so that the ancestor feerate is above tx1/5, // but the transaction's own feerate is lower - pool.addUnchecked(tx8.GetHash(), entry.Fee(5000LL).FromTx(tx8)); + pool.addUnchecked(entry.Fee(5000LL).FromTx(tx8)); sortedOrder.insert(sortedOrder.end()-1, tx8.GetHash().ToString()); CheckSort<ancestor_score>(pool, sortedOrder); } @@ -431,7 +431,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) tx1.vout.resize(1); tx1.vout[0].scriptPubKey = CScript() << OP_1 << OP_EQUAL; tx1.vout[0].nValue = 10 * COIN; - pool.addUnchecked(tx1.GetHash(), entry.Fee(10000LL).FromTx(tx1)); + pool.addUnchecked(entry.Fee(10000LL).FromTx(tx1)); CMutableTransaction tx2 = CMutableTransaction(); tx2.vin.resize(1); @@ -439,7 +439,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) tx2.vout.resize(1); tx2.vout[0].scriptPubKey = CScript() << OP_2 << OP_EQUAL; tx2.vout[0].nValue = 10 * COIN; - pool.addUnchecked(tx2.GetHash(), entry.Fee(5000LL).FromTx(tx2)); + pool.addUnchecked(entry.Fee(5000LL).FromTx(tx2)); pool.TrimToSize(pool.DynamicMemoryUsage()); // should do nothing BOOST_CHECK(pool.exists(tx1.GetHash())); @@ -449,7 +449,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) BOOST_CHECK(pool.exists(tx1.GetHash())); BOOST_CHECK(!pool.exists(tx2.GetHash())); - pool.addUnchecked(tx2.GetHash(), entry.FromTx(tx2)); + pool.addUnchecked(entry.FromTx(tx2)); CMutableTransaction tx3 = CMutableTransaction(); tx3.vin.resize(1); tx3.vin[0].prevout = COutPoint(tx2.GetHash(), 0); @@ -457,7 +457,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) tx3.vout.resize(1); tx3.vout[0].scriptPubKey = CScript() << OP_3 << OP_EQUAL; tx3.vout[0].nValue = 10 * COIN; - pool.addUnchecked(tx3.GetHash(), entry.Fee(20000LL).FromTx(tx3)); + pool.addUnchecked(entry.Fee(20000LL).FromTx(tx3)); pool.TrimToSize(pool.DynamicMemoryUsage() * 3 / 4); // tx3 should pay for tx2 (CPFP) BOOST_CHECK(!pool.exists(tx1.GetHash())); @@ -520,10 +520,10 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) tx7.vout[1].scriptPubKey = CScript() << OP_7 << OP_EQUAL; tx7.vout[1].nValue = 10 * COIN; - pool.addUnchecked(tx4.GetHash(), entry.Fee(7000LL).FromTx(tx4)); - pool.addUnchecked(tx5.GetHash(), entry.Fee(1000LL).FromTx(tx5)); - pool.addUnchecked(tx6.GetHash(), entry.Fee(1100LL).FromTx(tx6)); - pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7)); + pool.addUnchecked(entry.Fee(7000LL).FromTx(tx4)); + pool.addUnchecked(entry.Fee(1000LL).FromTx(tx5)); + pool.addUnchecked(entry.Fee(1100LL).FromTx(tx6)); + pool.addUnchecked(entry.Fee(9000LL).FromTx(tx7)); // we only require this to remove, at max, 2 txn, because it's not clear what we're really optimizing for aside from that pool.TrimToSize(pool.DynamicMemoryUsage() - 1); @@ -532,8 +532,8 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) BOOST_CHECK(!pool.exists(tx7.GetHash())); if (!pool.exists(tx5.GetHash())) - pool.addUnchecked(tx5.GetHash(), entry.Fee(1000LL).FromTx(tx5)); - pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7)); + pool.addUnchecked(entry.Fee(1000LL).FromTx(tx5)); + pool.addUnchecked(entry.Fee(9000LL).FromTx(tx7)); pool.TrimToSize(pool.DynamicMemoryUsage() / 2); // should maximize mempool size by only removing 5/7 BOOST_CHECK(pool.exists(tx4.GetHash())); @@ -541,8 +541,8 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) BOOST_CHECK(pool.exists(tx6.GetHash())); BOOST_CHECK(!pool.exists(tx7.GetHash())); - pool.addUnchecked(tx5.GetHash(), entry.Fee(1000LL).FromTx(tx5)); - pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7)); + pool.addUnchecked(entry.Fee(1000LL).FromTx(tx5)); + pool.addUnchecked(entry.Fee(9000LL).FromTx(tx7)); std::vector<CTransactionRef> vtx; SetMockTime(42); @@ -603,7 +603,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests) // [tx1] // CTransactionRef tx1 = make_tx(/* output_values */ {10 * COIN}); - pool.addUnchecked(tx1->GetHash(), entry.Fee(10000LL).FromTx(tx1)); + pool.addUnchecked(entry.Fee(10000LL).FromTx(tx1)); // Ancestors / descendants should be 1 / 1 (itself / itself) pool.GetTransactionAncestry(tx1->GetHash(), ancestors, descendants); @@ -615,7 +615,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests) // [tx1].0 <- [tx2] // CTransactionRef tx2 = make_tx(/* output_values */ {495 * CENT, 5 * COIN}, /* inputs */ {tx1}); - pool.addUnchecked(tx2->GetHash(), entry.Fee(10000LL).FromTx(tx2)); + pool.addUnchecked(entry.Fee(10000LL).FromTx(tx2)); // Ancestors / descendants should be: // transaction ancestors descendants @@ -634,7 +634,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests) // [tx1].0 <- [tx2].0 <- [tx3] // CTransactionRef tx3 = make_tx(/* output_values */ {290 * CENT, 200 * CENT}, /* inputs */ {tx2}); - pool.addUnchecked(tx3->GetHash(), entry.Fee(10000LL).FromTx(tx3)); + pool.addUnchecked(entry.Fee(10000LL).FromTx(tx3)); // Ancestors / descendants should be: // transaction ancestors descendants @@ -659,7 +659,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests) // \---1 <- [tx4] // CTransactionRef tx4 = make_tx(/* output_values */ {290 * CENT, 250 * CENT}, /* inputs */ {tx2}, /* input_indices */ {1}); - pool.addUnchecked(tx4->GetHash(), entry.Fee(10000LL).FromTx(tx4)); + pool.addUnchecked(entry.Fee(10000LL).FromTx(tx4)); // Ancestors / descendants should be: // transaction ancestors descendants @@ -696,13 +696,13 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests) CTransactionRef& tyi = *ty[i]; tyi = make_tx(/* output_values */ {v}, /* inputs */ i > 0 ? std::vector<CTransactionRef>{*ty[i - 1]} : std::vector<CTransactionRef>{}); v -= 50 * CENT; - pool.addUnchecked(tyi->GetHash(), entry.Fee(10000LL).FromTx(tyi)); + pool.addUnchecked(entry.Fee(10000LL).FromTx(tyi)); pool.GetTransactionAncestry(tyi->GetHash(), ancestors, descendants); BOOST_CHECK_EQUAL(ancestors, i+1); BOOST_CHECK_EQUAL(descendants, i+1); } CTransactionRef ty6 = make_tx(/* output_values */ {5 * COIN}, /* inputs */ {tx3, ty5}); - pool.addUnchecked(ty6->GetHash(), entry.Fee(10000LL).FromTx(ty6)); + pool.addUnchecked(entry.Fee(10000LL).FromTx(ty6)); // Ancestors / descendants should be: // transaction ancestors descendants diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index c4793472d4..eb0e4219d3 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -115,19 +115,19 @@ static void TestPackageSelection(const CChainParams& chainparams, const CScript& tx.vout[0].nValue = 5000000000LL - 1000; // This tx has a low fee: 1000 satoshis uint256 hashParentTx = tx.GetHash(); // save this txid for later use - mempool.addUnchecked(hashParentTx, entry.Fee(1000).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + mempool.addUnchecked(entry.Fee(1000).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); // This tx has a medium fee: 10000 satoshis tx.vin[0].prevout.hash = txFirst[1]->GetHash(); tx.vout[0].nValue = 5000000000LL - 10000; uint256 hashMediumFeeTx = tx.GetHash(); - mempool.addUnchecked(hashMediumFeeTx, entry.Fee(10000).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + mempool.addUnchecked(entry.Fee(10000).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); // This tx has a high fee, but depends on the first transaction tx.vin[0].prevout.hash = hashParentTx; tx.vout[0].nValue = 5000000000LL - 1000 - 50000; // 50k satoshi fee uint256 hashHighFeeTx = tx.GetHash(); - mempool.addUnchecked(hashHighFeeTx, entry.Fee(50000).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); + mempool.addUnchecked(entry.Fee(50000).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); std::unique_ptr<CBlockTemplate> pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); BOOST_CHECK(pblocktemplate->block.vtx[1]->GetHash() == hashParentTx); @@ -138,7 +138,7 @@ static void TestPackageSelection(const CChainParams& chainparams, const CScript& tx.vin[0].prevout.hash = hashHighFeeTx; tx.vout[0].nValue = 5000000000LL - 1000 - 50000; // 0 fee uint256 hashFreeTx = tx.GetHash(); - mempool.addUnchecked(hashFreeTx, entry.Fee(0).FromTx(tx)); + mempool.addUnchecked(entry.Fee(0).FromTx(tx)); size_t freeTxSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION); // Calculate a fee on child transaction that will put the package just @@ -148,7 +148,7 @@ static void TestPackageSelection(const CChainParams& chainparams, const CScript& tx.vin[0].prevout.hash = hashFreeTx; tx.vout[0].nValue = 5000000000LL - 1000 - 50000 - feeToUse; uint256 hashLowFeeTx = tx.GetHash(); - mempool.addUnchecked(hashLowFeeTx, entry.Fee(feeToUse).FromTx(tx)); + mempool.addUnchecked(entry.Fee(feeToUse).FromTx(tx)); pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); // Verify that the free tx and the low fee tx didn't get selected for (size_t i=0; i<pblocktemplate->block.vtx.size(); ++i) { @@ -162,7 +162,7 @@ static void TestPackageSelection(const CChainParams& chainparams, const CScript& mempool.removeRecursive(tx); tx.vout[0].nValue -= 2; // Now we should be just over the min relay fee hashLowFeeTx = tx.GetHash(); - mempool.addUnchecked(hashLowFeeTx, entry.Fee(feeToUse+2).FromTx(tx)); + mempool.addUnchecked(entry.Fee(feeToUse+2).FromTx(tx)); pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); BOOST_CHECK(pblocktemplate->block.vtx[4]->GetHash() == hashFreeTx); BOOST_CHECK(pblocktemplate->block.vtx[5]->GetHash() == hashLowFeeTx); @@ -175,7 +175,7 @@ static void TestPackageSelection(const CChainParams& chainparams, const CScript& tx.vout[0].nValue = 5000000000LL - 100000000; tx.vout[1].nValue = 100000000; // 1BTC output uint256 hashFreeTx2 = tx.GetHash(); - mempool.addUnchecked(hashFreeTx2, entry.Fee(0).SpendsCoinbase(true).FromTx(tx)); + mempool.addUnchecked(entry.Fee(0).SpendsCoinbase(true).FromTx(tx)); // This tx can't be mined by itself tx.vin[0].prevout.hash = hashFreeTx2; @@ -183,7 +183,7 @@ static void TestPackageSelection(const CChainParams& chainparams, const CScript& feeToUse = blockMinFeeRate.GetFee(freeTxSize); tx.vout[0].nValue = 5000000000LL - 100000000 - feeToUse; uint256 hashLowFeeTx2 = tx.GetHash(); - mempool.addUnchecked(hashLowFeeTx2, entry.Fee(feeToUse).SpendsCoinbase(false).FromTx(tx)); + mempool.addUnchecked(entry.Fee(feeToUse).SpendsCoinbase(false).FromTx(tx)); pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); // Verify that this tx isn't selected. @@ -196,7 +196,7 @@ static void TestPackageSelection(const CChainParams& chainparams, const CScript& // as well. tx.vin[0].prevout.n = 1; tx.vout[0].nValue = 100000000 - 10000; // 10k satoshi fee - mempool.addUnchecked(tx.GetHash(), entry.Fee(10000).FromTx(tx)); + mempool.addUnchecked(entry.Fee(10000).FromTx(tx)); pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); BOOST_CHECK(pblocktemplate->block.vtx[8]->GetHash() == hashLowFeeTx2); } @@ -277,7 +277,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) hash = tx.GetHash(); bool spendsCoinbase = i == 0; // only first tx spends coinbase // If we don't set the # of sig ops in the CTxMemPoolEntry, template creation fails - mempool.addUnchecked(hash, entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(spendsCoinbase).FromTx(tx)); + mempool.addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(spendsCoinbase).FromTx(tx)); tx.vin[0].prevout.hash = hash; } @@ -292,7 +292,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) hash = tx.GetHash(); bool spendsCoinbase = i == 0; // only first tx spends coinbase // If we do set the # of sig ops in the CTxMemPoolEntry, template creation passes - mempool.addUnchecked(hash, entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(spendsCoinbase).SigOpsCost(80).FromTx(tx)); + mempool.addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(spendsCoinbase).SigOpsCost(80).FromTx(tx)); tx.vin[0].prevout.hash = hash; } BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); @@ -312,7 +312,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vout[0].nValue -= LOWFEE; hash = tx.GetHash(); bool spendsCoinbase = i == 0; // only first tx spends coinbase - mempool.addUnchecked(hash, entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(spendsCoinbase).FromTx(tx)); + mempool.addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(spendsCoinbase).FromTx(tx)); tx.vin[0].prevout.hash = hash; } BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); @@ -320,7 +320,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) // orphan in mempool, template creation fails hash = tx.GetHash(); - mempool.addUnchecked(hash, entry.Fee(LOWFEE).Time(GetTime()).FromTx(tx)); + mempool.addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).FromTx(tx)); BOOST_CHECK_EXCEPTION(AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); mempool.clear(); @@ -329,7 +329,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vin[0].prevout.hash = txFirst[1]->GetHash(); tx.vout[0].nValue = BLOCKSUBSIDY-HIGHFEE; hash = tx.GetHash(); - mempool.addUnchecked(hash, entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + mempool.addUnchecked(entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); tx.vin[0].prevout.hash = hash; tx.vin.resize(2); tx.vin[1].scriptSig = CScript() << OP_1; @@ -337,7 +337,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vin[1].prevout.n = 0; tx.vout[0].nValue = tx.vout[0].nValue+BLOCKSUBSIDY-HIGHERFEE; //First txn output + fresh coinbase - new txn fee hash = tx.GetHash(); - mempool.addUnchecked(hash, entry.Fee(HIGHERFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + mempool.addUnchecked(entry.Fee(HIGHERFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); mempool.clear(); @@ -348,7 +348,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vout[0].nValue = 0; hash = tx.GetHash(); // give it a fee so it'll get mined - mempool.addUnchecked(hash, entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); + mempool.addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); // Should throw bad-cb-multiple BOOST_CHECK_EXCEPTION(AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-cb-multiple")); mempool.clear(); @@ -359,10 +359,10 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vout[0].nValue = BLOCKSUBSIDY-HIGHFEE; tx.vout[0].scriptPubKey = CScript() << OP_1; hash = tx.GetHash(); - mempool.addUnchecked(hash, entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + mempool.addUnchecked(entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); tx.vout[0].scriptPubKey = CScript() << OP_2; hash = tx.GetHash(); - mempool.addUnchecked(hash, entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + mempool.addUnchecked(entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); BOOST_CHECK_EXCEPTION(AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); mempool.clear(); @@ -401,12 +401,12 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) script = CScript() << OP_0; tx.vout[0].scriptPubKey = GetScriptForDestination(CScriptID(script)); hash = tx.GetHash(); - mempool.addUnchecked(hash, entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + mempool.addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); tx.vin[0].prevout.hash = hash; tx.vin[0].scriptSig = CScript() << std::vector<unsigned char>(script.begin(), script.end()); tx.vout[0].nValue -= LOWFEE; hash = tx.GetHash(); - mempool.addUnchecked(hash, entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); + mempool.addUnchecked(entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); // Should throw block-validation-failed BOOST_CHECK_EXCEPTION(AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("block-validation-failed")); mempool.clear(); @@ -440,7 +440,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vout[0].scriptPubKey = CScript() << OP_1; tx.nLockTime = 0; hash = tx.GetHash(); - mempool.addUnchecked(hash, entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + mempool.addUnchecked(entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); BOOST_CHECK(CheckFinalTx(tx, flags)); // Locktime passes BOOST_CHECK(!TestSequenceLocks(tx, flags)); // Sequence locks fail BOOST_CHECK(SequenceLocks(tx, flags, &prevheights, CreateBlockIndex(chainActive.Tip()->nHeight + 2))); // Sequence locks pass on 2nd block @@ -450,7 +450,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vin[0].nSequence = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG | (((chainActive.Tip()->GetMedianTimePast()+1-chainActive[1]->GetMedianTimePast()) >> CTxIn::SEQUENCE_LOCKTIME_GRANULARITY) + 1); // txFirst[1] is the 3rd block prevheights[0] = baseheight + 2; hash = tx.GetHash(); - mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx)); + mempool.addUnchecked(entry.Time(GetTime()).FromTx(tx)); BOOST_CHECK(CheckFinalTx(tx, flags)); // Locktime passes BOOST_CHECK(!TestSequenceLocks(tx, flags)); // Sequence locks fail @@ -466,7 +466,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) prevheights[0] = baseheight + 3; tx.nLockTime = chainActive.Tip()->nHeight + 1; hash = tx.GetHash(); - mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx)); + mempool.addUnchecked(entry.Time(GetTime()).FromTx(tx)); BOOST_CHECK(!CheckFinalTx(tx, flags)); // Locktime fails BOOST_CHECK(TestSequenceLocks(tx, flags)); // Sequence locks pass BOOST_CHECK(IsFinalTx(tx, chainActive.Tip()->nHeight + 2, chainActive.Tip()->GetMedianTimePast())); // Locktime passes on 2nd block @@ -477,7 +477,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) prevheights.resize(1); prevheights[0] = baseheight + 4; hash = tx.GetHash(); - mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx)); + mempool.addUnchecked(entry.Time(GetTime()).FromTx(tx)); BOOST_CHECK(!CheckFinalTx(tx, flags)); // Locktime fails BOOST_CHECK(TestSequenceLocks(tx, flags)); // Sequence locks pass BOOST_CHECK(IsFinalTx(tx, chainActive.Tip()->nHeight + 2, chainActive.Tip()->GetMedianTimePast() + 1)); // Locktime passes 1 second later diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 9c1af6bdf6..8072eb922d 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -302,4 +302,22 @@ BOOST_AUTO_TEST_CASE(netbase_getgroup) BOOST_CHECK(CreateInternal("baz.net").GetGroup() == internal_group); } +BOOST_AUTO_TEST_CASE(netbase_parsenetwork) +{ + BOOST_CHECK_EQUAL(ParseNetwork("ipv4"), NET_IPV4); + BOOST_CHECK_EQUAL(ParseNetwork("ipv6"), NET_IPV6); + BOOST_CHECK_EQUAL(ParseNetwork("onion"), NET_ONION); + BOOST_CHECK_EQUAL(ParseNetwork("tor"), NET_ONION); + + BOOST_CHECK_EQUAL(ParseNetwork("IPv4"), NET_IPV4); + BOOST_CHECK_EQUAL(ParseNetwork("IPv6"), NET_IPV6); + BOOST_CHECK_EQUAL(ParseNetwork("ONION"), NET_ONION); + BOOST_CHECK_EQUAL(ParseNetwork("TOR"), NET_ONION); + + BOOST_CHECK_EQUAL(ParseNetwork(":)"), NET_UNROUTABLE); + BOOST_CHECK_EQUAL(ParseNetwork("tÖr"), NET_UNROUTABLE); + BOOST_CHECK_EQUAL(ParseNetwork("\xfe\xff"), NET_UNROUTABLE); + BOOST_CHECK_EQUAL(ParseNetwork(""), NET_UNROUTABLE); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 26a0b495d9..2022ed6659 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -58,7 +58,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) for (int k = 0; k < 4; k++) { // add 4 fee txs tx.vin[0].prevout.n = 10000*blocknum+100*j+k; // make transaction unique uint256 hash = tx.GetHash(); - mpool.addUnchecked(hash, entry.Fee(feeV[j]).Time(GetTime()).Height(blocknum).FromTx(tx)); + mpool.addUnchecked(entry.Fee(feeV[j]).Time(GetTime()).Height(blocknum).FromTx(tx)); txHashes[j].push_back(hash); } } @@ -129,7 +129,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) for (int k = 0; k < 4; k++) { // add 4 fee txs tx.vin[0].prevout.n = 10000*blocknum+100*j+k; uint256 hash = tx.GetHash(); - mpool.addUnchecked(hash, entry.Fee(feeV[j]).Time(GetTime()).Height(blocknum).FromTx(tx)); + mpool.addUnchecked(entry.Fee(feeV[j]).Time(GetTime()).Height(blocknum).FromTx(tx)); txHashes[j].push_back(hash); } } @@ -164,7 +164,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) for (int k = 0; k < 4; k++) { // add 4 fee txs tx.vin[0].prevout.n = 10000*blocknum+100*j+k; uint256 hash = tx.GetHash(); - mpool.addUnchecked(hash, entry.Fee(feeV[j]).Time(GetTime()).Height(blocknum).FromTx(tx)); + mpool.addUnchecked(entry.Fee(feeV[j]).Time(GetTime()).Height(blocknum).FromTx(tx)); CTransactionRef ptx = mpool.get(hash); if (ptx) block.push_back(ptx); diff --git a/src/test/scheduler_tests.cpp b/src/test/scheduler_tests.cpp index 2af0ab22da..12ec00ce02 100644 --- a/src/test/scheduler_tests.cpp +++ b/src/test/scheduler_tests.cpp @@ -54,7 +54,7 @@ BOOST_AUTO_TEST_CASE(manythreads) boost::mutex counterMutex[10]; int counter[10] = { 0 }; - FastRandomContext rng(42); + FastRandomContext rng{/* fDeterministic */ true}; auto zeroToNine = [](FastRandomContext& rc) -> int { return rc.randrange(10); }; // [0, 9] auto randomMsec = [](FastRandomContext& rc) -> int { return -11 + (int)rc.randrange(1012); }; // [-11, 1000] auto randomDelta = [](FastRandomContext& rc) -> int { return -1000 + (int)rc.randrange(2001); }; // [-1000, 1000] @@ -138,11 +138,13 @@ BOOST_AUTO_TEST_CASE(singlethreadedscheduler_ordered) // the callbacks should run in exactly the order in which they were enqueued for (int i = 0; i < 100; ++i) { queue1.AddToProcessQueue([i, &counter1]() { - assert(i == counter1++); + bool expectation = i == counter1++; + assert(expectation); }); queue2.AddToProcessQueue([i, &counter2]() { - assert(i == counter2++); + bool expectation = i == counter2++; + assert(expectation); }); } diff --git a/src/test/sync_tests.cpp b/src/test/sync_tests.cpp new file mode 100644 index 0000000000..df0380546e --- /dev/null +++ b/src/test/sync_tests.cpp @@ -0,0 +1,52 @@ +// Copyright (c) 2012-2017 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <sync.h> +#include <test/test_bitcoin.h> + +#include <boost/test/unit_test.hpp> + +namespace { +template <typename MutexType> +void TestPotentialDeadLockDetected(MutexType& mutex1, MutexType& mutex2) +{ + { + LOCK2(mutex1, mutex2); + } + bool error_thrown = false; + try { + LOCK2(mutex2, mutex1); + } catch (const std::logic_error& e) { + BOOST_CHECK_EQUAL(e.what(), "potential deadlock detected"); + error_thrown = true; + } + #ifdef DEBUG_LOCKORDER + BOOST_CHECK(error_thrown); + #else + BOOST_CHECK(!error_thrown); + #endif +} +} // namespace + +BOOST_FIXTURE_TEST_SUITE(sync_tests, BasicTestingSetup) + +BOOST_AUTO_TEST_CASE(potential_deadlock_detected) +{ + #ifdef DEBUG_LOCKORDER + bool prev = g_debug_lockorder_abort; + g_debug_lockorder_abort = false; + #endif + + CCriticalSection rmutex1, rmutex2; + TestPotentialDeadLockDetected(rmutex1, rmutex2); + + Mutex mutex1, mutex2; + TestPotentialDeadLockDetected(mutex1, mutex2); + + #ifdef DEBUG_LOCKORDER + g_debug_lockorder_abort = prev; + #endif +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index b2960446eb..8e2f5abe66 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1217,4 +1217,43 @@ BOOST_AUTO_TEST_CASE(test_DirIsWritable) fs::remove(tmpdirname); } +BOOST_AUTO_TEST_CASE(test_ToLower) +{ + BOOST_CHECK_EQUAL(ToLower('@'), '@'); + BOOST_CHECK_EQUAL(ToLower('A'), 'a'); + BOOST_CHECK_EQUAL(ToLower('Z'), 'z'); + BOOST_CHECK_EQUAL(ToLower('['), '['); + BOOST_CHECK_EQUAL(ToLower(0), 0); + BOOST_CHECK_EQUAL(ToLower(255), 255); + + std::string testVector; + Downcase(testVector); + BOOST_CHECK_EQUAL(testVector, ""); + + testVector = "#HODL"; + Downcase(testVector); + BOOST_CHECK_EQUAL(testVector, "#hodl"); + + testVector = "\x00\xfe\xff"; + Downcase(testVector); + BOOST_CHECK_EQUAL(testVector, "\x00\xfe\xff"); +} + +BOOST_AUTO_TEST_CASE(test_ToUpper) +{ + BOOST_CHECK_EQUAL(ToUpper('`'), '`'); + BOOST_CHECK_EQUAL(ToUpper('a'), 'A'); + BOOST_CHECK_EQUAL(ToUpper('z'), 'Z'); + BOOST_CHECK_EQUAL(ToUpper('{'), '{'); + BOOST_CHECK_EQUAL(ToUpper(0), 0); + BOOST_CHECK_EQUAL(ToUpper(255), 255); +} + +BOOST_AUTO_TEST_CASE(test_Capitalize) +{ + BOOST_CHECK_EQUAL(Capitalize(""), ""); + BOOST_CHECK_EQUAL(Capitalize("bitcoin"), "Bitcoin"); + BOOST_CHECK_EQUAL(Capitalize("\x00\xfe\xff"), "\x00\xfe\xff"); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/threadinterrupt.cpp b/src/threadinterrupt.cpp index d08d52e4a9..340106ed99 100644 --- a/src/threadinterrupt.cpp +++ b/src/threadinterrupt.cpp @@ -5,6 +5,8 @@ #include <threadinterrupt.h> +#include <sync.h> + CThreadInterrupt::CThreadInterrupt() : flag(false) {} CThreadInterrupt::operator bool() const @@ -20,7 +22,7 @@ void CThreadInterrupt::reset() void CThreadInterrupt::operator()() { { - std::unique_lock<std::mutex> lock(mut); + LOCK(mut); flag.store(true, std::memory_order_release); } cond.notify_all(); @@ -28,7 +30,7 @@ void CThreadInterrupt::operator()() bool CThreadInterrupt::sleep_for(std::chrono::milliseconds rel_time) { - std::unique_lock<std::mutex> lock(mut); + WAIT_LOCK(mut, lock); return !cond.wait_for(lock, rel_time, [this]() { return flag.load(std::memory_order_acquire); }); } diff --git a/src/threadinterrupt.h b/src/threadinterrupt.h index c30bd3d657..9c6fccfcde 100644 --- a/src/threadinterrupt.h +++ b/src/threadinterrupt.h @@ -5,6 +5,8 @@ #ifndef BITCOIN_THREADINTERRUPT_H #define BITCOIN_THREADINTERRUPT_H +#include <sync.h> + #include <atomic> #include <chrono> #include <condition_variable> @@ -28,7 +30,7 @@ public: private: std::condition_variable cond; - std::mutex mut; + Mutex mut; std::atomic<bool> flag; }; diff --git a/src/threadsafety.h b/src/threadsafety.h index a2f45e5fce..47e6b2ea38 100644 --- a/src/threadsafety.h +++ b/src/threadsafety.h @@ -10,7 +10,7 @@ // TL;DR Add GUARDED_BY(mutex) to member variables. The others are // rarely necessary. Ex: int nFoo GUARDED_BY(cs_foo); // -// See http://clang.llvm.org/docs/LanguageExtensions.html#threadsafety +// See https://clang.llvm.org/docs/ThreadSafetyAnalysis.html // for documentation. The clang compiler can do advanced static analysis // of locking when given the -Wthread-safety option. #define LOCKABLE __attribute__((lockable)) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index c7ff7b5477..05217149e3 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -20,13 +20,10 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee, int64_t _nTime, unsigned int _entryHeight, - bool _spendsCoinbase, int64_t _sigOpsCost, LockPoints lp): - tx(_tx), nFee(_nFee), nTime(_nTime), entryHeight(_entryHeight), + bool _spendsCoinbase, int64_t _sigOpsCost, LockPoints lp) + : tx(_tx), nFee(_nFee), nTxWeight(GetTransactionWeight(*tx)), nUsageSize(RecursiveDynamicUsage(tx)), nTime(_nTime), entryHeight(_entryHeight), spendsCoinbase(_spendsCoinbase), sigOpCost(_sigOpsCost), lockPoints(lp) { - nTxWeight = GetTransactionWeight(*tx); - nUsageSize = RecursiveDynamicUsage(tx); - nCountWithDescendants = 1; nSizeWithDescendants = GetTxSize(); nModFeesWithDescendants = nFee; @@ -355,7 +352,7 @@ void CTxMemPool::AddTransactionsUpdated(unsigned int n) nTransactionsUpdated += n; } -void CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool validFeeEstimate) +void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAncestors, bool validFeeEstimate) { NotifyEntryAdded(entry.GetSharedTx()); // Add to memory pool without checking anything. @@ -367,7 +364,7 @@ void CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, // Update transaction for any feeDelta created by PrioritiseTransaction // TODO: refactor so that the fee delta is calculated before inserting // into mapTx. - std::map<uint256, CAmount>::const_iterator pos = mapDeltas.find(hash); + std::map<uint256, CAmount>::const_iterator pos = mapDeltas.find(entry.GetTx().GetHash()); if (pos != mapDeltas.end()) { const CAmount &delta = pos->second; if (delta) { @@ -923,13 +920,13 @@ int CTxMemPool::Expire(int64_t time) { return stage.size(); } -void CTxMemPool::addUnchecked(const uint256&hash, const CTxMemPoolEntry &entry, bool validFeeEstimate) +void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, bool validFeeEstimate) { setEntries setAncestors; uint64_t nNoLimit = std::numeric_limits<uint64_t>::max(); std::string dummy; CalculateMemPoolAncestors(entry, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy); - return addUnchecked(hash, entry, setAncestors, validFeeEstimate); + return addUnchecked(entry, setAncestors, validFeeEstimate); } void CTxMemPool::UpdateChild(txiter entry, txiter child, bool add) diff --git a/src/txmempool.h b/src/txmempool.h index f8915cec31..2163b5eb21 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -65,14 +65,14 @@ class CTxMemPool; class CTxMemPoolEntry { private: - CTransactionRef tx; - CAmount nFee; //!< Cached to avoid expensive parent-transaction lookups - size_t nTxWeight; //!< ... and avoid recomputing tx weight (also used for GetTxSize()) - size_t nUsageSize; //!< ... and total memory usage - int64_t nTime; //!< Local time when entering the mempool - unsigned int entryHeight; //!< Chain height when entering the mempool - bool spendsCoinbase; //!< keep track of transactions that spend a coinbase - int64_t sigOpCost; //!< Total sigop cost + const CTransactionRef tx; + const CAmount nFee; //!< Cached to avoid expensive parent-transaction lookups + const size_t nTxWeight; //!< ... and avoid recomputing tx weight (also used for GetTxSize()) + const size_t nUsageSize; //!< ... and total memory usage + const int64_t nTime; //!< Local time when entering the mempool + const unsigned int entryHeight; //!< Chain height when entering the mempool + const bool spendsCoinbase; //!< keep track of transactions that spend a coinbase + const int64_t sigOpCost; //!< Total sigop cost int64_t feeDelta; //!< Used for determining the priority of the transaction for mining in a block LockPoints lockPoints; //!< Track the height and time at which tx was final @@ -540,8 +540,8 @@ public: // Note that addUnchecked is ONLY called from ATMP outside of tests // and any other callers may break wallet's in-mempool tracking (due to // lack of CValidationInterface::TransactionAddedToMempool callbacks). - void addUnchecked(const uint256& hash, const CTxMemPoolEntry& entry, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs); - void addUnchecked(const uint256& hash, const CTxMemPoolEntry& entry, setEntries& setAncestors, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs); + void addUnchecked(const CTxMemPoolEntry& entry, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs); + void addUnchecked(const CTxMemPoolEntry& entry, setEntries& setAncestors, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN); void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main); diff --git a/src/util.cpp b/src/util.cpp index 1aab85264f..3bb52e9b3d 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -71,7 +71,6 @@ #include <malloc.h> #endif -#include <boost/interprocess/sync/file_lock.hpp> #include <boost/thread.hpp> #include <openssl/crypto.h> #include <openssl/rand.h> @@ -139,7 +138,7 @@ instance_of_cinit; * cleans them up and thus automatically unlocks them, or ReleaseDirectoryLocks * is called. */ -static std::map<std::string, std::unique_ptr<boost::interprocess::file_lock>> dir_locks; +static std::map<std::string, std::unique_ptr<fsbridge::FileLock>> dir_locks; /** Mutex to protect dir_locks. */ static std::mutex cs_dir_locks; @@ -156,18 +155,13 @@ bool LockDirectory(const fs::path& directory, const std::string lockfile_name, b // Create empty lock file if it doesn't exist. FILE* file = fsbridge::fopen(pathLockFile, "a"); if (file) fclose(file); - - try { - auto lock = MakeUnique<boost::interprocess::file_lock>(pathLockFile.string().c_str()); - if (!lock->try_lock()) { - return false; - } - if (!probe_only) { - // Lock successful and we're not just probing, put it into the map - dir_locks.emplace(pathLockFile.string(), std::move(lock)); - } - } catch (const boost::interprocess::interprocess_exception& e) { - return error("Error while attempting to lock directory %s: %s", directory.string(), e.what()); + auto lock = MakeUnique<fsbridge::FileLock>(pathLockFile); + if (!lock->TryLock()) { + return error("Error while attempting to lock directory %s: %s", directory.string(), lock->GetReason()); + } + if (!probe_only) { + // Lock successful and we're not just probing, put it into the map + dir_locks.emplace(pathLockFile.string(), std::move(lock)); } return true; } @@ -222,7 +216,7 @@ public: /** Determine whether to use config settings in the default section, * See also comments around ArgsManager::ArgsManager() below. */ - static inline bool UseDefaultSection(const ArgsManager& am, const std::string& arg) + static inline bool UseDefaultSection(const ArgsManager& am, const std::string& arg) EXCLUSIVE_LOCKS_REQUIRED(am.cs_args) { return (am.m_network == CBaseChainParams::MAIN || am.m_network_only_args.count(arg) == 0); } @@ -301,7 +295,7 @@ public: /* Special test for -testnet and -regtest args, because we * don't want to be confused by craziness like "[regtest] testnet=1" */ - static inline bool GetNetBoolArg(const ArgsManager &am, const std::string& net_arg) + static inline bool GetNetBoolArg(const ArgsManager &am, const std::string& net_arg) EXCLUSIVE_LOCKS_REQUIRED(am.cs_args) { std::pair<bool,std::string> found_result(false,std::string()); found_result = GetArgHelper(am.m_override_args, net_arg, true); @@ -378,6 +372,8 @@ ArgsManager::ArgsManager() : void ArgsManager::WarnForSectionOnlyArgs() { + LOCK(cs_args); + // if there's no section selected, don't worry if (m_network.empty()) return; @@ -406,6 +402,7 @@ void ArgsManager::WarnForSectionOnlyArgs() void ArgsManager::SelectConfigNetwork(const std::string& network) { + LOCK(cs_args); m_network = network; } @@ -474,6 +471,7 @@ bool ArgsManager::IsArgKnown(const std::string& key) const arg_no_net = std::string("-") + key.substr(option_index + 1, std::string::npos); } + LOCK(cs_args); for (const auto& arg_map : m_available_args) { if (arg_map.second.count(arg_no_net)) return true; } @@ -577,6 +575,7 @@ void ArgsManager::AddArg(const std::string& name, const std::string& help, const eq_index = name.size(); } + LOCK(cs_args); std::map<std::string, Arg>& arg_map = m_available_args[cat]; auto ret = arg_map.emplace(name.substr(0, eq_index), Arg(name.substr(eq_index, name.size() - eq_index), help, debug_only)); assert(ret.second); // Make sure an insertion actually happened @@ -594,6 +593,7 @@ std::string ArgsManager::GetHelpMessage() const const bool show_debug = gArgs.GetBoolArg("-help-debug", false); std::string usage = ""; + LOCK(cs_args); for (const auto& arg_map : m_available_args) { switch(arg_map.first) { case OptionsCategory::OPTIONS: @@ -886,7 +886,12 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) } // if there is an -includeconf in the override args, but it is empty, that means the user // passed '-noincludeconf' on the command line, in which case we should not include anything - if (m_override_args.count("-includeconf") == 0) { + bool emptyIncludeConf; + { + LOCK(cs_args); + emptyIncludeConf = m_override_args.count("-includeconf") == 0; + } + if (emptyIncludeConf) { std::string chain_id = GetChainName(); std::vector<std::string> includeconf(GetArgs("-includeconf")); { @@ -946,6 +951,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) std::string ArgsManager::GetChainName() const { + LOCK(cs_args); bool fRegTest = ArgsManagerHelper::GetNetBoolArg(*this, "-regtest"); bool fTestNet = ArgsManagerHelper::GetNetBoolArg(*this, "-testnet"); diff --git a/src/util.h b/src/util.h index e93489c1ed..7bf9fdbe12 100644 --- a/src/util.h +++ b/src/util.h @@ -142,11 +142,11 @@ protected: }; mutable CCriticalSection cs_args; - std::map<std::string, std::vector<std::string>> m_override_args; - std::map<std::string, std::vector<std::string>> m_config_args; - std::string m_network; - std::set<std::string> m_network_only_args; - std::map<OptionsCategory, std::map<std::string, Arg>> m_available_args; + std::map<std::string, std::vector<std::string>> m_override_args GUARDED_BY(cs_args); + std::map<std::string, std::vector<std::string>> m_config_args GUARDED_BY(cs_args); + std::string m_network GUARDED_BY(cs_args); + std::set<std::string> m_network_only_args GUARDED_BY(cs_args); + std::map<OptionsCategory, std::map<std::string, Arg>> m_available_args GUARDED_BY(cs_args); bool ReadConfigStream(std::istream& stream, std::string& error, bool ignore_invalid_keys = false); @@ -262,7 +262,10 @@ public: /** * Clear available arguments */ - void ClearArgs() { m_available_args.clear(); } + void ClearArgs() { + LOCK(cs_args); + m_available_args.clear(); + } /** * Get the help string diff --git a/src/utilstrencodings.cpp b/src/utilstrencodings.cpp index e66e2c238c..2326383586 100644 --- a/src/utilstrencodings.cpp +++ b/src/utilstrencodings.cpp @@ -7,6 +7,7 @@ #include <tinyformat.h> +#include <algorithm> #include <cstdlib> #include <cstring> #include <errno.h> @@ -584,3 +585,15 @@ bool ParseHDKeypath(const std::string& keypath_str, std::vector<uint32_t>& keypa } return true; } + +void Downcase(std::string& str) +{ + std::transform(str.begin(), str.end(), str.begin(), [](unsigned char c){return ToLower(c);}); +} + +std::string Capitalize(std::string str) +{ + if (str.empty()) return str; + str[0] = ToUpper(str.front()); + return str; +} diff --git a/src/utilstrencodings.h b/src/utilstrencodings.h index 8f29f8f322..846a3917a2 100644 --- a/src/utilstrencodings.h +++ b/src/utilstrencodings.h @@ -186,4 +186,48 @@ bool ConvertBits(const O& outfn, I it, I end) { /** Parse an HD keypaths like "m/7/0'/2000". */ bool ParseHDKeypath(const std::string& keypath_str, std::vector<uint32_t>& keypath); +/** + * Converts the given character to its lowercase equivalent. + * This function is locale independent. It only converts uppercase + * characters in the standard 7-bit ASCII range. + * @param[in] c the character to convert to lowercase. + * @return the lowercase equivalent of c; or the argument + * if no conversion is possible. + */ +constexpr unsigned char ToLower(unsigned char c) +{ + return (c >= 'A' && c <= 'Z' ? (c - 'A') + 'a' : c); +} + +/** + * Converts the given string to its lowercase equivalent. + * This function is locale independent. It only converts uppercase + * characters in the standard 7-bit ASCII range. + * @param[in,out] str the string to convert to lowercase. + */ +void Downcase(std::string& str); + +/** + * Converts the given character to its uppercase equivalent. + * This function is locale independent. It only converts lowercase + * characters in the standard 7-bit ASCII range. + * @param[in] c the character to convert to uppercase. + * @return the uppercase equivalent of c; or the argument + * if no conversion is possible. + */ +constexpr unsigned char ToUpper(unsigned char c) +{ + return (c >= 'a' && c <= 'z' ? (c - 'a') + 'A' : c); +} + +/** + * Capitalizes the first character of the given string. + * This function is locale independent. It only capitalizes the + * first character of the argument if it has an uppercase equivalent + * in the standard 7-bit ASCII range. + * @param[in] str the string to capitalize. + * @return string with the first letter capitalized. + */ +std::string Capitalize(std::string str); + #endif // BITCOIN_UTILSTRENCODINGS_H diff --git a/src/validation.cpp b/src/validation.cpp index 3e51221a5e..a9eea5edd9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -217,8 +217,8 @@ CCriticalSection cs_main; BlockMap& mapBlockIndex = g_chainstate.mapBlockIndex; CChain& chainActive = g_chainstate.chainActive; CBlockIndex *pindexBestHeader = nullptr; -CWaitableCriticalSection g_best_block_mutex; -CConditionVariable g_best_block_cv; +Mutex g_best_block_mutex; +std::condition_variable g_best_block_cv; uint256 g_best_block; int nScriptCheckThreads = 0; std::atomic_bool fImporting(false); @@ -959,7 +959,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool bool validForFeeEstimation = !fReplacementTransaction && !bypass_limits && IsCurrentForFeeEstimation() && pool.HasNoInputsOf(tx); // Store transaction in memory - pool.addUnchecked(hash, entry, setAncestors, validForFeeEstimation); + pool.addUnchecked(entry, setAncestors, validForFeeEstimation); // trim mempool and check if tx was trimmed if (!bypass_limits) { @@ -2239,7 +2239,7 @@ void static UpdateTip(const CBlockIndex *pindexNew, const CChainParams& chainPar mempool.AddTransactionsUpdated(1); { - WaitableLock lock(g_best_block_mutex); + LOCK(g_best_block_mutex); g_best_block = pindexNew->GetBlockHash(); g_best_block_cv.notify_all(); } @@ -2448,7 +2448,7 @@ bool CChainState::ConnectTip(CValidationState& state, const CChainParams& chainp if (!rv) { if (state.IsInvalid()) InvalidBlockFound(pindexNew, state); - return error("ConnectTip(): ConnectBlock %s failed", pindexNew->GetBlockHash().ToString()); + return error("%s: ConnectBlock %s failed, %s", __func__, pindexNew->GetBlockHash().ToString(), FormatStateMessage(state)); } nTime3 = GetTimeMicros(); nTimeConnectTotal += nTime3 - nTime2; LogPrint(BCLog::BENCH, " - Connect total: %.2fms [%.2fs (%.2fms/blk)]\n", (nTime3 - nTime2) * MILLI, nTimeConnectTotal * MICRO, nTimeConnectTotal * MILLI / nBlocksTotal); diff --git a/src/validation.h b/src/validation.h index c4c9b8b5ba..3df6456eca 100644 --- a/src/validation.h +++ b/src/validation.h @@ -151,8 +151,8 @@ extern BlockMap& mapBlockIndex; extern uint64_t nLastBlockTx; extern uint64_t nLastBlockWeight; extern const std::string strMessageMagic; -extern CWaitableCriticalSection g_best_block_mutex; -extern CConditionVariable g_best_block_cv; +extern Mutex g_best_block_mutex; +extern std::condition_variable g_best_block_cv; extern uint256 g_best_block; extern std::atomic_bool fImporting; extern std::atomic_bool fReindex; diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 2b8a0f6467..677bf74b5d 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -242,7 +242,7 @@ Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransacti CReserveKey reservekey(wallet); CValidationState state; - if (!wallet->CommitTransaction(tx, std::move(mapValue), oldWtx.vOrderForm, oldWtx.strFromAccount, reservekey, g_connman.get(), state)) { + if (!wallet->CommitTransaction(tx, std::move(mapValue), oldWtx.vOrderForm, reservekey, g_connman.get(), state)) { // NOTE: CommitTransaction never returns false, so this should never happen. errors.push_back(strprintf("The transaction was rejected: %s", FormatStateMessage(state))); return Result::WALLET_ERROR; diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index b9f267210e..b36287ff50 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -78,7 +78,7 @@ void WalletInit::AddWalletOptions() const gArgs.AddArg("-walletnotify=<cmd>", "Execute command when a wallet transaction changes (%s in cmd is replaced by TxID)", false, OptionsCategory::WALLET); gArgs.AddArg("-walletrbf", strprintf("Send transactions with full-RBF opt-in enabled (RPC only, default: %u)", DEFAULT_WALLET_RBF), false, OptionsCategory::WALLET); gArgs.AddArg("-zapwallettxes=<mode>", "Delete all wallet transactions and only recover those parts of the blockchain through -rescan on startup" - " (1 = keep tx meta data e.g. account owner and payment request information, 2 = drop tx meta data)", false, OptionsCategory::WALLET); + " (1 = keep tx meta data e.g. payment request information, 2 = drop tx meta data)", false, OptionsCategory::WALLET); gArgs.AddArg("-dblogsize=<n>", strprintf("Flush wallet database activity from memory to disk log every <n> megabytes (default: %u)", DEFAULT_WALLET_DBLOGSIZE), true, OptionsCategory::WALLET_DEBUG_TEST); gArgs.AddArg("-flushwallet", strprintf("Run a thread to flush wallet periodically (default: %u)", DEFAULT_FLUSHWALLET), true, OptionsCategory::WALLET_DEBUG_TEST); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index ccd8e19aee..31eb1e902f 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -196,16 +196,6 @@ static UniValue getnewaddress(const JSONRPCRequest& request) return EncodeDestination(dest); } -CTxDestination GetLabelDestination(CWallet* const pwallet, const std::string& label, bool bForceNew=false) -{ - CTxDestination dest; - if (!pwallet->GetLabelDestination(dest, label, bForceNew)) { - throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first"); - } - - return dest; -} - static UniValue getrawchangeaddress(const JSONRPCRequest& request) { std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); @@ -296,20 +286,6 @@ static UniValue setlabel(const JSONRPCRequest& request) pwallet->SetAddressBook(dest, label, "send"); } - // Detect when there are no addresses using this label. - // If so, delete the account record for it. Labels, unlike addresses, can be deleted, - // and if we wouldn't do this, the record would stick around forever. - bool found_address = false; - for (const std::pair<const CTxDestination, CAddressBookData>& item : pwallet->mapAddressBook) { - if (item.second.name == label) { - found_address = true; - break; - } - } - if (!found_address) { - pwallet->DeleteLabel(old_label); - } - return NullUniValue; } @@ -347,7 +323,7 @@ static CTransactionRef SendMoney(CWallet * const pwallet, const CTxDestination & throw JSONRPCError(RPC_WALLET_ERROR, strError); } CValidationState state; - if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, "" /* account */, reservekey, g_connman.get(), state)) { + if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, reservekey, g_connman.get(), state)) { strError = strprintf("Error: The transaction was rejected! Reason given: %s", FormatStateMessage(state)); throw JSONRPCError(RPC_WALLET_ERROR, strError); } @@ -903,7 +879,7 @@ static UniValue sendmany(const JSONRPCRequest& request) EnsureWalletIsUnlocked(pwallet); // Check funds - if (totalAmount > pwallet->GetLegacyBalance(ISMINE_SPENDABLE, nMinDepth, nullptr)) { + if (totalAmount > pwallet->GetLegacyBalance(ISMINE_SPENDABLE, nMinDepth)) { throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Wallet has insufficient funds"); } @@ -920,7 +896,7 @@ static UniValue sendmany(const JSONRPCRequest& request) if (!fCreated) throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason); CValidationState state; - if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, "" /* account */, keyChange, g_connman.get(), state)) { + if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, keyChange, g_connman.get(), state)) { strFailReason = strprintf("Transaction commit failed:: %s", FormatStateMessage(state)); throw JSONRPCError(RPC_WALLET_ERROR, strFailReason); } @@ -1403,11 +1379,10 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest) static void ListTransactions(CWallet* const pwallet, const CWalletTx& wtx, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { CAmount nFee; - std::string dummy_account; std::list<COutputEntry> listReceived; std::list<COutputEntry> listSent; - wtx.GetAmounts(listReceived, listSent, nFee, dummy_account, filter); + wtx.GetAmounts(listReceived, listSent, nFee, filter); bool involvesWatchonly = wtx.IsFromMe(ISMINE_WATCH_ONLY); @@ -1562,10 +1537,8 @@ UniValue listtransactions(const JSONRPCRequest& request) // iterate backwards until we have nCount items to return: for (CWallet::TxItems::const_reverse_iterator it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) { - CWalletTx *const pwtx = (*it).second.first; - if (pwtx != nullptr) { - ListTransactions(pwallet, *pwtx, 0, true, ret, filter); - } + CWalletTx *const pwtx = (*it).second; + ListTransactions(pwallet, *pwtx, 0, true, ret, filter); if ((int)ret.size() >= (nCount+nFrom)) break; } } @@ -4067,7 +4040,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) const CTransaction txConst(*psbtx.tx); // Fill transaction with out data but don't sign - bool bip32derivs = request.params[4].isNull() ? false : request.params[5].get_bool(); + bool bip32derivs = request.params[4].isNull() ? false : request.params[4].get_bool(); FillPSBT(pwallet, psbtx, &txConst, 1, false, bip32derivs); // Serialize the PSBT diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index f44311b0be..3a8e6f751a 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -297,7 +297,7 @@ public: CCoinControl dummy; BOOST_CHECK(wallet->CreateTransaction({recipient}, tx, reservekey, fee, changePos, error, dummy)); CValidationState state; - BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, {}, reservekey, nullptr, state)); + BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, reservekey, nullptr, state)); CMutableTransaction blocktx; { LOCK(wallet->cs_wallet); @@ -320,7 +320,11 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) // Confirm ListCoins initially returns 1 coin grouped under coinbaseKey // address. - auto list = wallet->ListCoins(); + std::map<CTxDestination, std::vector<COutput>> list; + { + LOCK2(cs_main, wallet->cs_wallet); + list = wallet->ListCoins(); + } BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress); BOOST_CHECK_EQUAL(list.begin()->second.size(), 1U); @@ -333,7 +337,10 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) // coinbaseKey pubkey, even though the change address has a different // pubkey. AddTx(CRecipient{GetScriptForRawPubKey({}), 1 * COIN, false /* subtract fee */}); - list = wallet->ListCoins(); + { + LOCK2(cs_main, wallet->cs_wallet); + list = wallet->ListCoins(); + } BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress); BOOST_CHECK_EQUAL(list.begin()->second.size(), 2U); @@ -359,7 +366,10 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) } // Confirm ListCoins still returns same result as before, despite coins // being locked. - list = wallet->ListCoins(); + { + LOCK2(cs_main, wallet->cs_wallet); + list = wallet->ListCoins(); + } BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress); BOOST_CHECK_EQUAL(list.begin()->second.size(), 2U); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 913334a767..6cbfbc1f90 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -585,7 +585,6 @@ void CWallet::SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator> ran // nTimeReceived not copied on purpose copyTo->nTimeSmart = copyFrom->nTimeSmart; copyTo->fFromMe = copyFrom->fFromMe; - copyTo->strFromAccount = copyFrom->strFromAccount; // nOrderPos not copied on purpose // cached members not copied on purpose } @@ -735,44 +734,30 @@ DBErrors CWallet::ReorderTransactions() // Old wallets didn't have any defined order for transactions // Probably a bad idea to change the output of this - // First: get all CWalletTx and CAccountingEntry into a sorted-by-time multimap. - typedef std::pair<CWalletTx*, CAccountingEntry*> TxPair; - typedef std::multimap<int64_t, TxPair > TxItems; + // First: get all CWalletTx into a sorted-by-time multimap. + typedef std::multimap<int64_t, CWalletTx*> TxItems; TxItems txByTime; for (auto& entry : mapWallet) { CWalletTx* wtx = &entry.second; - txByTime.insert(std::make_pair(wtx->nTimeReceived, TxPair(wtx, nullptr))); - } - std::list<CAccountingEntry> acentries; - batch.ListAccountCreditDebit("", acentries); - for (CAccountingEntry& entry : acentries) - { - txByTime.insert(std::make_pair(entry.nTime, TxPair(nullptr, &entry))); + txByTime.insert(std::make_pair(wtx->nTimeReceived, wtx)); } nOrderPosNext = 0; std::vector<int64_t> nOrderPosOffsets; for (TxItems::iterator it = txByTime.begin(); it != txByTime.end(); ++it) { - CWalletTx *const pwtx = (*it).second.first; - CAccountingEntry *const pacentry = (*it).second.second; - int64_t& nOrderPos = (pwtx != nullptr) ? pwtx->nOrderPos : pacentry->nOrderPos; + CWalletTx *const pwtx = (*it).second; + int64_t& nOrderPos = pwtx->nOrderPos; if (nOrderPos == -1) { nOrderPos = nOrderPosNext++; nOrderPosOffsets.push_back(nOrderPos); - if (pwtx) - { - if (!batch.WriteTx(*pwtx)) - return DBErrors::LOAD_FAIL; - } - else - if (!batch.WriteAccountingEntry(pacentry->nEntryNo, *pacentry)) - return DBErrors::LOAD_FAIL; + if (!batch.WriteTx(*pwtx)) + return DBErrors::LOAD_FAIL; } else { @@ -789,14 +774,8 @@ DBErrors CWallet::ReorderTransactions() continue; // Since we're changing the order, write it back - if (pwtx) - { - if (!batch.WriteTx(*pwtx)) - return DBErrors::LOAD_FAIL; - } - else - if (!batch.WriteAccountingEntry(pacentry->nEntryNo, *pacentry)) - return DBErrors::LOAD_FAIL; + if (!batch.WriteTx(*pwtx)) + return DBErrors::LOAD_FAIL; } } batch.WriteOrderPosNext(nOrderPosNext); @@ -816,80 +795,6 @@ int64_t CWallet::IncOrderPosNext(WalletBatch *batch) return nRet; } -bool CWallet::AccountMove(std::string strFrom, std::string strTo, CAmount nAmount, std::string strComment) -{ - WalletBatch batch(*database); - if (!batch.TxnBegin()) - return false; - - int64_t nNow = GetAdjustedTime(); - - // Debit - CAccountingEntry debit; - debit.nOrderPos = IncOrderPosNext(&batch); - debit.strAccount = strFrom; - debit.nCreditDebit = -nAmount; - debit.nTime = nNow; - debit.strOtherAccount = strTo; - debit.strComment = strComment; - AddAccountingEntry(debit, &batch); - - // Credit - CAccountingEntry credit; - credit.nOrderPos = IncOrderPosNext(&batch); - credit.strAccount = strTo; - credit.nCreditDebit = nAmount; - credit.nTime = nNow; - credit.strOtherAccount = strFrom; - credit.strComment = strComment; - AddAccountingEntry(credit, &batch); - - if (!batch.TxnCommit()) - return false; - - return true; -} - -bool CWallet::GetLabelDestination(CTxDestination &dest, const std::string& label, bool bForceNew) -{ - WalletBatch batch(*database); - - CAccount account; - batch.ReadAccount(label, account); - - if (!bForceNew) { - if (!account.vchPubKey.IsValid()) - bForceNew = true; - else { - // Check if the current key has been used (TODO: check other addresses with the same key) - CScript scriptPubKey = GetScriptForDestination(GetDestinationForKey(account.vchPubKey, m_default_address_type)); - for (std::map<uint256, CWalletTx>::iterator it = mapWallet.begin(); - it != mapWallet.end() && account.vchPubKey.IsValid(); - ++it) - for (const CTxOut& txout : (*it).second.tx->vout) - if (txout.scriptPubKey == scriptPubKey) { - bForceNew = true; - break; - } - } - } - - // Generate a new key - if (bForceNew) { - if (!GetKeyFromPool(account.vchPubKey, false)) - return false; - - LearnRelatedScripts(account.vchPubKey, m_default_address_type); - dest = GetDestinationForKey(account.vchPubKey, m_default_address_type); - SetAddressBook(dest, label, "receive"); - batch.WriteAccount(label, account); - } else { - dest = GetDestinationForKey(account.vchPubKey, m_default_address_type); - } - - return true; -} - void CWallet::MarkDirty() { { @@ -944,7 +849,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) if (fInsertedNew) { wtx.nTimeReceived = GetAdjustedTime(); wtx.nOrderPos = IncOrderPosNext(&batch); - wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, TxPair(&wtx, nullptr))); + wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); wtx.nTimeSmart = ComputeTimeSmart(wtx); AddToSpends(hash); } @@ -1019,7 +924,7 @@ void CWallet::LoadToWallet(const CWalletTx& wtxIn) CWalletTx& wtx = ins.first->second; wtx.BindWallet(this); if (/* insertion took place */ ins.second) { - wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, TxPair(&wtx, nullptr))); + wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); } AddToSpends(hash); for (const CTxIn& txin : wtx.tx->vin) { @@ -1615,12 +1520,11 @@ int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet, } void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived, - std::list<COutputEntry>& listSent, CAmount& nFee, std::string& strSentAccount, const isminefilter& filter) const + std::list<COutputEntry>& listSent, CAmount& nFee, const isminefilter& filter) const { nFee = 0; listReceived.clear(); listSent.clear(); - strSentAccount = strFromAccount; // Compute fee: CAmount nDebit = GetDebit(filter); @@ -2189,7 +2093,7 @@ CAmount CWallet::GetImmatureWatchOnlyBalance() const // wallet, and then subtracts the values of TxIns spending from the wallet. This // also has fewer restrictions on which unconfirmed transactions are considered // trusted. -CAmount CWallet::GetLegacyBalance(const isminefilter& filter, int minDepth, const std::string* account) const +CAmount CWallet::GetLegacyBalance(const isminefilter& filter, int minDepth) const { LOCK2(cs_main, cs_wallet); @@ -2208,21 +2112,17 @@ CAmount CWallet::GetLegacyBalance(const isminefilter& filter, int minDepth, cons for (const CTxOut& out : wtx.tx->vout) { if (outgoing && IsChange(out)) { debit -= out.nValue; - } else if (IsMine(out) & filter && depth >= minDepth && (!account || *account == GetLabelName(out.scriptPubKey))) { + } else if (IsMine(out) & filter && depth >= minDepth) { balance += out.nValue; } } // For outgoing txs, subtract amount debited. - if (outgoing && (!account || *account == wtx.strFromAccount)) { + if (outgoing) { balance -= debit; } } - if (account) { - balance += WalletBatch(*database).GetAccountCreditDebit(*account); - } - return balance; } @@ -2352,20 +2252,12 @@ void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const std::map<CTxDestination, std::vector<COutput>> CWallet::ListCoins() const { - // TODO: Add AssertLockHeld(cs_wallet) here. - // - // Because the return value from this function contains pointers to - // CWalletTx objects, callers to this function really should acquire the - // cs_wallet lock before calling it. However, the current caller doesn't - // acquire this lock yet. There was an attempt to add the missing lock in - // https://github.com/bitcoin/bitcoin/pull/10340, but that change has been - // postponed until after https://github.com/bitcoin/bitcoin/pull/10244 to - // avoid adding some extra complexity to the Qt code. + AssertLockHeld(cs_main); + AssertLockHeld(cs_wallet); std::map<CTxDestination, std::vector<COutput>> result; std::vector<COutput> availableCoins; - LOCK2(cs_main, cs_wallet); AvailableCoins(availableCoins); for (auto& coin : availableCoins) { @@ -3054,7 +2946,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac /** * Call after CreateTransaction unless you want to abort */ -bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, std::string fromAccount, CReserveKey& reservekey, CConnman* connman, CValidationState& state) +bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, CReserveKey& reservekey, CConnman* connman, CValidationState& state) { { LOCK2(cs_main, cs_wallet); @@ -3062,7 +2954,6 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve CWalletTx wtxNew(this, std::move(tx)); wtxNew.mapValue = std::move(mapValue); wtxNew.vOrderForm = std::move(orderForm); - wtxNew.strFromAccount = std::move(fromAccount); wtxNew.fTimeReceivedIsTxTime = true; wtxNew.fFromMe = true; @@ -3102,31 +2993,6 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve return true; } -void CWallet::ListAccountCreditDebit(const std::string& strAccount, std::list<CAccountingEntry>& entries) { - WalletBatch batch(*database); - return batch.ListAccountCreditDebit(strAccount, entries); -} - -bool CWallet::AddAccountingEntry(const CAccountingEntry& acentry) -{ - WalletBatch batch(*database); - - return AddAccountingEntry(acentry, &batch); -} - -bool CWallet::AddAccountingEntry(const CAccountingEntry& acentry, WalletBatch *batch) -{ - if (!batch->WriteAccountingEntry(++nAccountingEntryNumber, acentry)) { - return false; - } - - laccentries.push_back(acentry); - CAccountingEntry & entry = laccentries.back(); - wtxOrdered.insert(std::make_pair(entry.nOrderPos, TxPair(nullptr, &entry))); - - return true; -} - DBErrors CWallet::LoadWallet(bool& fFirstRunRet) { LOCK2(cs_main, cs_wallet); @@ -3660,12 +3526,6 @@ std::set<CTxDestination> CWallet::GetLabelAddresses(const std::string& label) co return result; } -void CWallet::DeleteLabel(const std::string& label) -{ - WalletBatch batch(*database); - batch.EraseAccount(label); -} - bool CReserveKey::GetReservedKey(CPubKey& pubkey, bool internal) { if (nIndex == -1) @@ -3856,19 +3716,14 @@ unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx) const int64_t latestTolerated = latestNow + 300; const TxItems& txOrdered = wtxOrdered; for (auto it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) { - CWalletTx* const pwtx = it->second.first; + CWalletTx* const pwtx = it->second; if (pwtx == &wtx) { continue; } - CAccountingEntry* const pacentry = it->second.second; int64_t nSmartTime; - if (pwtx) { - nSmartTime = pwtx->nTimeSmart; - if (!nSmartTime) { - nSmartTime = pwtx->nTimeReceived; - } - } else { - nSmartTime = pacentry->nTime; + nSmartTime = pwtx->nTimeSmart; + if (!nSmartTime) { + nSmartTime = pwtx->nTimeReceived; } if (nSmartTime <= latestTolerated) { latestEntry = nSmartTime; @@ -4313,7 +4168,6 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(const std::string& name, copyTo->nTimeReceived = copyFrom->nTimeReceived; copyTo->nTimeSmart = copyFrom->nTimeSmart; copyTo->fFromMe = copyFrom->fFromMe; - copyTo->strFromAccount = copyFrom->strFromAccount; copyTo->nOrderPos = copyFrom->nOrderPos; batch.WriteTx(*copyTo); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 5f550319ed..da326517c0 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -340,9 +340,8 @@ public: * externally and came in through the network or sendrawtransaction RPC. */ char fFromMe; - std::string strFromAccount; int64_t nOrderPos; //!< position in ordered transaction list - std::multimap<int64_t, std::pair<CWalletTx*, CAccountingEntry*>>::const_iterator m_it_wtxOrdered; + std::multimap<int64_t, CWalletTx*>::const_iterator m_it_wtxOrdered; // memory only mutable bool fDebitCached; @@ -379,7 +378,6 @@ public: nTimeReceived = 0; nTimeSmart = 0; fFromMe = false; - strFromAccount.clear(); fDebitCached = false; fCreditCached = false; fImmatureCreditCached = false; @@ -408,7 +406,7 @@ public: char fSpent = false; mapValue_t mapValueCopy = mapValue; - mapValueCopy["fromaccount"] = strFromAccount; + mapValueCopy["fromaccount"] = ""; WriteOrderPos(nOrderPos, mapValueCopy); if (nTimeSmart) { mapValueCopy["timesmart"] = strprintf("%u", nTimeSmart); @@ -429,7 +427,6 @@ public: std::vector<CMerkleTx> vUnused; //!< Used to be vtxPrev s >> vUnused >> mapValue >> vOrderForm >> fTimeReceivedIsTxTime >> nTimeReceived >> fFromMe >> fSpent; - strFromAccount = std::move(mapValue["fromaccount"]); ReadOrderPos(nOrderPos, mapValue); nTimeSmart = mapValue.count("timesmart") ? (unsigned int)atoi64(mapValue["timesmart"]) : 0; @@ -474,7 +471,7 @@ public: } void GetAmounts(std::list<COutputEntry>& listReceived, - std::list<COutputEntry>& listSent, CAmount& nFee, std::string& strSentAccount, const isminefilter& filter) const; + std::list<COutputEntry>& listSent, CAmount& nFee, const isminefilter& filter) const; bool IsFromMe(const isminefilter& filter) const { @@ -569,89 +566,6 @@ public: } }; -/** - * DEPRECATED Internal transfers. - * Database key is acentry<account><counter>. - */ -class CAccountingEntry -{ -public: - std::string strAccount; - CAmount nCreditDebit; - int64_t nTime; - std::string strOtherAccount; - std::string strComment; - mapValue_t mapValue; - int64_t nOrderPos; //!< position in ordered transaction list - uint64_t nEntryNo; - - CAccountingEntry() - { - SetNull(); - } - - void SetNull() - { - nCreditDebit = 0; - nTime = 0; - strAccount.clear(); - strOtherAccount.clear(); - strComment.clear(); - nOrderPos = -1; - nEntryNo = 0; - } - - template <typename Stream> - void Serialize(Stream& s) const { - int nVersion = s.GetVersion(); - if (!(s.GetType() & SER_GETHASH)) { - s << nVersion; - } - //! Note: strAccount is serialized as part of the key, not here. - s << nCreditDebit << nTime << strOtherAccount; - - mapValue_t mapValueCopy = mapValue; - WriteOrderPos(nOrderPos, mapValueCopy); - - std::string strCommentCopy = strComment; - if (!mapValueCopy.empty() || !_ssExtra.empty()) { - CDataStream ss(s.GetType(), s.GetVersion()); - ss.insert(ss.begin(), '\0'); - ss << mapValueCopy; - ss.insert(ss.end(), _ssExtra.begin(), _ssExtra.end()); - strCommentCopy.append(ss.str()); - } - s << strCommentCopy; - } - - template <typename Stream> - void Unserialize(Stream& s) { - int nVersion = s.GetVersion(); - if (!(s.GetType() & SER_GETHASH)) { - s >> nVersion; - } - //! Note: strAccount is serialized as part of the key, not here. - s >> nCreditDebit >> nTime >> LIMITED_STRING(strOtherAccount, 65536) >> LIMITED_STRING(strComment, 65536); - - size_t nSepPos = strComment.find("\0", 0, 1); - mapValue.clear(); - if (std::string::npos != nSepPos) { - CDataStream ss(std::vector<char>(strComment.begin() + nSepPos + 1, strComment.end()), s.GetType(), s.GetVersion()); - ss >> mapValue; - _ssExtra = std::vector<char>(ss.begin(), ss.end()); - } - ReadOrderPos(nOrderPos, mapValue); - if (std::string::npos != nSepPos) { - strComment.erase(nSepPos); - } - - mapValue.erase("n"); - } - -private: - std::vector<char> _ssExtra; -}; - struct CoinSelectionParams { bool use_bnb = true; @@ -826,10 +740,8 @@ public: } std::map<uint256, CWalletTx> mapWallet; - std::list<CAccountingEntry> laccentries; - typedef std::pair<CWalletTx*, CAccountingEntry*> TxPair; - typedef std::multimap<int64_t, TxPair > TxItems; + typedef std::multimap<int64_t, CWalletTx*> TxItems; TxItems wtxOrdered; int64_t nOrderPosNext = 0; @@ -852,7 +764,7 @@ public: /** * Return list of available coins and locked coins grouped by non-change output address. */ - std::map<CTxDestination, std::vector<COutput>> ListCoins() const; + std::map<CTxDestination, std::vector<COutput>> ListCoins() const EXCLUSIVE_LOCKS_REQUIRED(cs_main, cs_wallet); /** * Find non-change parent output. @@ -941,8 +853,6 @@ public: */ int64_t IncOrderPosNext(WalletBatch *batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); DBErrors ReorderTransactions(); - bool AccountMove(std::string strFrom, std::string strTo, CAmount nAmount, std::string strComment = "") EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - bool GetLabelDestination(CTxDestination &dest, const std::string& label, bool bForceNew = false); void MarkDirty(); bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose=true); @@ -962,7 +872,7 @@ public: CAmount GetImmatureBalance() const; CAmount GetUnconfirmedWatchOnlyBalance() const; CAmount GetImmatureWatchOnlyBalance() const; - CAmount GetLegacyBalance(const isminefilter& filter, int minDepth, const std::string* account) const; + CAmount GetLegacyBalance(const isminefilter& filter, int minDepth) const; CAmount GetAvailableBalance(const CCoinControl* coinControl = nullptr) const; OutputType TransactionChangeType(OutputType change_type, const std::vector<CRecipient>& vecSend); @@ -981,11 +891,8 @@ public: */ bool CreateTransaction(const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign = true); - bool CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, std::string fromAccount, CReserveKey& reservekey, CConnman* connman, CValidationState& state); + bool CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, CReserveKey& reservekey, CConnman* connman, CValidationState& state); - void ListAccountCreditDebit(const std::string& strAccount, std::list<CAccountingEntry>& entries); - bool AddAccountingEntry(const CAccountingEntry&); - bool AddAccountingEntry(const CAccountingEntry&, WalletBatch *batch); bool DummySignTx(CMutableTransaction &txNew, const std::set<CTxOut> &txouts, bool use_max_sig = false) const { std::vector<CTxOut> v_txouts(txouts.size()); @@ -999,7 +906,7 @@ public: unsigned int m_confirm_target{DEFAULT_TX_CONFIRM_TARGET}; bool m_spend_zero_conf_change{DEFAULT_SPEND_ZEROCONF_CHANGE}; bool m_signal_rbf{DEFAULT_WALLET_RBF}; - bool m_allow_fallback_fee{true}; //<! will be defined via chainparams + bool m_allow_fallback_fee{true}; //!< will be defined via chainparams CFeeRate m_min_fee{DEFAULT_TRANSACTION_MINFEE}; //!< Override with -mintxfee /** * If fee estimation does not have enough data to provide estimates, use this fee instead. @@ -1044,7 +951,6 @@ public: std::map<CTxDestination, CAmount> GetAddressBalances() EXCLUSIVE_LOCKS_REQUIRED(cs_main); std::set<CTxDestination> GetLabelAddresses(const std::string& label) const; - void DeleteLabel(const std::string& label); isminetype IsMine(const CTxIn& txin) const; /** @@ -1253,37 +1159,6 @@ public: void KeepScript() override { KeepKey(); } }; - -/** - * DEPRECATED Account information. - * Stored in wallet with key "acc"+string account name. - */ -class CAccount -{ -public: - CPubKey vchPubKey; - - CAccount() - { - SetNull(); - } - - void SetNull() - { - vchPubKey = CPubKey(); - } - - ADD_SERIALIZE_METHODS; - - template <typename Stream, typename Operation> - inline void SerializationOp(Stream& s, Operation ser_action) { - int nVersion = s.GetVersion(); - if (!(s.GetType() & SER_GETHASH)) - READWRITE(nVersion); - READWRITE(vchPubKey); - } -}; - /** RAII object to check and reserve a wallet rescan */ class WalletRescanReserver { diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 1b787be198..5ce4d080e5 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -17,6 +17,7 @@ #include <wallet/wallet.h> #include <atomic> +#include <string> #include <boost/thread.hpp> @@ -150,82 +151,6 @@ bool WalletBatch::WriteMinVersion(int nVersion) return WriteIC(std::string("minversion"), nVersion); } -bool WalletBatch::ReadAccount(const std::string& strAccount, CAccount& account) -{ - account.SetNull(); - return m_batch.Read(std::make_pair(std::string("acc"), strAccount), account); -} - -bool WalletBatch::WriteAccount(const std::string& strAccount, const CAccount& account) -{ - return WriteIC(std::make_pair(std::string("acc"), strAccount), account); -} - -bool WalletBatch::EraseAccount(const std::string& strAccount) -{ - return EraseIC(std::make_pair(std::string("acc"), strAccount)); -} - -bool WalletBatch::WriteAccountingEntry(const uint64_t nAccEntryNum, const CAccountingEntry& acentry) -{ - return WriteIC(std::make_pair(std::string("acentry"), std::make_pair(acentry.strAccount, nAccEntryNum)), acentry); -} - -CAmount WalletBatch::GetAccountCreditDebit(const std::string& strAccount) -{ - std::list<CAccountingEntry> entries; - ListAccountCreditDebit(strAccount, entries); - - CAmount nCreditDebit = 0; - for (const CAccountingEntry& entry : entries) - nCreditDebit += entry.nCreditDebit; - - return nCreditDebit; -} - -void WalletBatch::ListAccountCreditDebit(const std::string& strAccount, std::list<CAccountingEntry>& entries) -{ - bool fAllAccounts = (strAccount == "*"); - - Dbc* pcursor = m_batch.GetCursor(); - if (!pcursor) - throw std::runtime_error(std::string(__func__) + ": cannot create DB cursor"); - bool setRange = true; - while (true) - { - // Read next record - CDataStream ssKey(SER_DISK, CLIENT_VERSION); - if (setRange) - ssKey << std::make_pair(std::string("acentry"), std::make_pair((fAllAccounts ? std::string("") : strAccount), uint64_t(0))); - CDataStream ssValue(SER_DISK, CLIENT_VERSION); - int ret = m_batch.ReadAtCursor(pcursor, ssKey, ssValue, setRange); - setRange = false; - if (ret == DB_NOTFOUND) - break; - else if (ret != 0) - { - pcursor->close(); - throw std::runtime_error(std::string(__func__) + ": error scanning DB"); - } - - // Unserialize - std::string strType; - ssKey >> strType; - if (strType != "acentry") - break; - CAccountingEntry acentry; - ssKey >> acentry.strAccount; - if (!fAllAccounts && acentry.strAccount != strAccount) - break; - - ssValue >> acentry; - ssKey >> acentry.nEntryNo; - entries.push_back(acentry); - } - - pcursor->close(); -} - class CWalletScanState { public: unsigned int nKeys; @@ -284,9 +209,10 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, { char fTmp; char fUnused; - ssValue >> fTmp >> fUnused >> wtx.strFromAccount; - strErr = strprintf("LoadWallet() upgrading tx ver=%d %d '%s' %s", - wtx.fTimeReceivedIsTxTime, fTmp, wtx.strFromAccount, hash.ToString()); + std::string unused_string; + ssValue >> fTmp >> fUnused >> unused_string; + strErr = strprintf("LoadWallet() upgrading tx ver=%d %d %s", + wtx.fTimeReceivedIsTxTime, fTmp, hash.ToString()); wtx.fTimeReceivedIsTxTime = fTmp; } else @@ -302,24 +228,6 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, pwallet->LoadToWallet(wtx); } - else if (strType == "acentry") - { - std::string strAccount; - ssKey >> strAccount; - uint64_t nNumber; - ssKey >> nNumber; - if (nNumber > pwallet->nAccountingEntryNumber) { - pwallet->nAccountingEntryNumber = nNumber; - } - - if (!wss.fAnyUnordered) - { - CAccountingEntry acentry; - ssValue >> acentry; - if (acentry.nOrderPos == -1) - wss.fAnyUnordered = true; - } - } else if (strType == "watchs") { wss.nWatchKeys++; @@ -510,7 +418,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, return false; } } else if (strType != "bestblock" && strType != "bestblock_nomerkle" && - strType != "minversion") { + strType != "minversion" && strType != "acentry") { wss.m_unknown_records++; } } catch (...) @@ -625,12 +533,6 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) if (wss.fAnyUnordered) result = pwallet->ReorderTransactions(); - pwallet->laccentries.clear(); - ListAccountCreditDebit("*", pwallet->laccentries); - for (CAccountingEntry& entry : pwallet->laccentries) { - pwallet->wtxOrdered.insert(make_pair(entry.nOrderPos, CWallet::TxPair(nullptr, &entry))); - } - return result; } diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 0e6cb1bba4..5584407a56 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -31,8 +31,6 @@ static const bool DEFAULT_FLUSHWALLET = true; -class CAccount; -class CAccountingEntry; struct CBlockLocator; class CKeyPool; class CMasterKey; @@ -199,21 +197,11 @@ public: bool WriteMinVersion(int nVersion); - /// This writes directly to the database, and will not update the CWallet's cached accounting entries! - /// Use wallet.AddAccountingEntry instead, to write *and* update its caches. - bool WriteAccountingEntry(const uint64_t nAccEntryNum, const CAccountingEntry& acentry); - bool ReadAccount(const std::string& strAccount, CAccount& account); - bool WriteAccount(const std::string& strAccount, const CAccount& account); - bool EraseAccount(const std::string& strAccount); - /// Write destination data key,value tuple to database bool WriteDestData(const std::string &address, const std::string &key, const std::string &value); /// Erase destination data tuple from wallet database bool EraseDestData(const std::string &address, const std::string &key); - CAmount GetAccountCreditDebit(const std::string& strAccount); - void ListAccountCreditDebit(const std::string& strAccount, std::list<CAccountingEntry>& acentries); - DBErrors LoadWallet(CWallet* pwallet); DBErrors FindWalletTx(std::vector<uint256>& vTxHash, std::vector<CWalletTx>& vWtx); DBErrors ZapWalletTx(std::vector<CWalletTx>& vWtx); |