From 2db55b476fc45f41361596f3a535ea3face10e29 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 8 Nov 2017 08:33:35 +0100 Subject: [PATCH 01/27] Merge #11524: [net] De-duplicate connection eviction logic 5ce7cb9 [net] De-duplicate connection eviction logic (Thomas Snider) Pull request description: While reviewing the safeguards against deliberate node isolation on the network by malicious actors, I found a good de-duplication candidate. I think this form is much more legible (the type of `cutoffs` notwithstanding). ReverseCompareNodeTimeConnected is not included in the list since the cutoff size is a function of the remaining number of nodes in the candidate eviction set. Tree-SHA512: ed17999fa9250dcf8448329219324477117e4ecd2d41dedd72ad253e44630eef50b3232c420f1862ebbfb9b8c94efbba1a235b519e39ff5946865c7d69a75280 --- src/net.cpp | 49 ++++++++++++++++++++----------------------------- 1 file changed, 20 insertions(+), 29 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index c111a411ae13..cae156799e9c 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1007,6 +1007,16 @@ static bool CompareNodeTXTime(const NodeEvictionCandidate &a, const NodeEviction return a.nTimeConnected > b.nTimeConnected; } + +//! Sort an array by the specified comparator, then erase the last K elements. +template +static void EraseLastKElements(std::vector &elements, Comparator comparator, size_t k) +{ + std::sort(elements.begin(), elements.end(), comparator); + size_t eraseSize = std::min(k, elements.size()); + elements.erase(elements.end() - eraseSize, elements.end()); +} + /** Try to find a connection to evict when the node is full. * Extreme care must be taken to avoid opening the node to attacker * triggered network partitioning. @@ -1056,42 +1066,23 @@ bool CConnman::AttemptToEvictConnection() } } - if (vEvictionCandidates.empty()) return false; - // Protect connections with certain characteristics // Deterministically select 4 peers to protect by netgroup. // An attacker cannot predict which netgroups will be protected - std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), CompareNetGroupKeyed); - vEvictionCandidates.erase(vEvictionCandidates.end() - std::min(4, static_cast(vEvictionCandidates.size())), vEvictionCandidates.end()); - - if (vEvictionCandidates.empty()) return false; - + EraseLastKElements(vEvictionCandidates, CompareNetGroupKeyed, 4); // Protect the 8 nodes with the lowest minimum ping time. // An attacker cannot manipulate this metric without physically moving nodes closer to the target. - std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), ReverseCompareNodeMinPingTime); - vEvictionCandidates.erase(vEvictionCandidates.end() - std::min(8, static_cast(vEvictionCandidates.size())), vEvictionCandidates.end()); - - if (vEvictionCandidates.empty()) return false; - + EraseLastKElements(vEvictionCandidates, ReverseCompareNodeMinPingTime, 8); // Protect 4 nodes that most recently sent us transactions. // An attacker cannot manipulate this metric without performing useful work. - std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), CompareNodeTXTime); - vEvictionCandidates.erase(vEvictionCandidates.end() - std::min(4, static_cast(vEvictionCandidates.size())), vEvictionCandidates.end()); - - if (vEvictionCandidates.empty()) return false; - + EraseLastKElements(vEvictionCandidates, CompareNodeTXTime, 4); // Protect 4 nodes that most recently sent us blocks. // An attacker cannot manipulate this metric without performing useful work. - std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), CompareNodeBlockTime); - vEvictionCandidates.erase(vEvictionCandidates.end() - std::min(4, static_cast(vEvictionCandidates.size())), vEvictionCandidates.end()); - - if (vEvictionCandidates.empty()) return false; - + EraseLastKElements(vEvictionCandidates, CompareNodeBlockTime, 4); // Protect the half of the remaining nodes which have been connected the longest. // This replicates the non-eviction implicit behavior, and precludes attacks that start later. - std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), ReverseCompareNodeTimeConnected); - vEvictionCandidates.erase(vEvictionCandidates.end() - static_cast(vEvictionCandidates.size() / 2), vEvictionCandidates.end()); + EraseLastKElements(vEvictionCandidates, ReverseCompareNodeTimeConnected, vEvictionCandidates.size() / 2); if (vEvictionCandidates.empty()) return false; @@ -1102,12 +1093,12 @@ bool CConnman::AttemptToEvictConnection() int64_t nMostConnectionsTime = 0; std::map > mapNetGroupNodes; for (const NodeEvictionCandidate &node : vEvictionCandidates) { - mapNetGroupNodes[node.nKeyedNetGroup].push_back(node); - int64_t grouptime = mapNetGroupNodes[node.nKeyedNetGroup][0].nTimeConnected; - size_t groupsize = mapNetGroupNodes[node.nKeyedNetGroup].size(); + std::vector &group = mapNetGroupNodes[node.nKeyedNetGroup]; + group.push_back(node); + int64_t grouptime = group[0].nTimeConnected; - if (groupsize > nMostConnections || (groupsize == nMostConnections && grouptime > nMostConnectionsTime)) { - nMostConnections = groupsize; + if (group.size() > nMostConnections || (group.size() == nMostConnections && grouptime > nMostConnectionsTime)) { + nMostConnections = group.size(); nMostConnectionsTime = grouptime; naMostConnections = node.nKeyedNetGroup; } From 78cd1f06771bce6f751ba9c906c3f8a043ee5d53 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 8 Nov 2017 13:03:10 -0500 Subject: [PATCH 02/27] =?UTF-8?q?Merge=20#11635:=20trivial:=20Fix=20typo?= =?UTF-8?q?=20=E2=80=93=20alreardy=20=E2=86=92=20already?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 7536b08c1 trivial: Fix typo – alreardy → already (practicalswift) Pull request description: Fix typo: alreardy → already. Tree-SHA512: b53f7540e516bb0a106873983aea8cb35f8e6690153b2c737ede79be7dae7b9e5f644859204ab6419de0f900ffac50bfdd775259b24ad8c12993b4c792fe836b --- src/validation.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/validation.cpp b/src/validation.cpp index 3b77081a3d15..8315a4626651 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -188,7 +188,7 @@ namespace { * BLOCK_FAILED_VALID blocks in a set should be just fine and work just as * well. * - * Because we alreardy walk mapBlockIndex in height-order at startup, we go + * Because we already walk mapBlockIndex in height-order at startup, we go * ahead and mark descendants of invalid blocks as FAILED_CHILD at that time, * instead of putting things in this set. */ From 948863c5b337acd88ed58503a96175f168913ec3 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Sun, 22 Oct 2017 15:33:01 -1000 Subject: [PATCH 03/27] Merge #11499: [Qt] Add upload and download info to the peerlist (debug menu) 6b1891e2c Add Sent and Received information to the debug menu peer list (Aaron Golliver) 8e4aa35ff move human-readable byte formatting to guiutil (Aaron Golliver) Pull request description: Makes the peer list display how much you've uploaded/downloaded from each peer. Here's a screenshot ~~[outdated](https://i.imgur.com/MhPbItp.png)~~, [current](https://i.imgur.com/K1htrVv.png) of how it looks. You can now sort to see who are the peers you've uploaded the most too. I also moved `RPCConsole::FormatBytes` to `guiutil::formatBytes` so I could use it in the peerlist Tree-SHA512: 8845ef406e4cbe7f981879a78c063542ce90f50f45c8fa3514ba3e6e1164b4c70bb2093c4e1cac268aef0328b7b63545bc1dfa435c227f28fdb4cb0a596800f5 --- src/qt/guiutil.cpp | 12 ++++++++++++ src/qt/guiutil.h | 2 ++ src/qt/peertablemodel.cpp | 20 +++++++++++++++++--- src/qt/peertablemodel.h | 6 ++++-- src/qt/rpcconsole.cpp | 20 ++++---------------- src/qt/rpcconsole.h | 1 - 6 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index eb8214ea837e..3e92c398f752 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -1083,6 +1083,18 @@ QString formatNiceTimeOffset(qint64 secs) return timeBehindText; } +QString formatBytes(uint64_t bytes) +{ + if(bytes < 1024) + return QString(QObject::tr("%1 B")).arg(bytes); + if(bytes < 1024 * 1024) + return QString(QObject::tr("%1 KB")).arg(bytes / 1024); + if(bytes < 1024 * 1024 * 1024) + return QString(QObject::tr("%1 MB")).arg(bytes / 1024 / 1024); + + return QString(QObject::tr("%1 GB")).arg(bytes / 1024 / 1024 / 1024); +} + void ClickableLabel::mouseReleaseEvent(QMouseEvent *event) { Q_EMIT clicked(event->pos()); diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h index b17214fece11..155eebe7580b 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -249,6 +249,8 @@ namespace GUIUtil QString formatNiceTimeOffset(qint64 secs); + QString formatBytes(uint64_t bytes); + class ClickableLabel : public QLabel { Q_OBJECT diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp index b81b644da98d..dd69e17bd929 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -33,6 +33,10 @@ bool NodeLessThan::operator()(const CNodeCombinedStats &left, const CNodeCombine return pLeft->cleanSubVer.compare(pRight->cleanSubVer) < 0; case PeerTableModel::Ping: return pLeft->dMinPing < pRight->dMinPing; + case PeerTableModel::Sent: + return pLeft->nSendBytes < pRight->nSendBytes; + case PeerTableModel::Received: + return pLeft->nRecvBytes < pRight->nRecvBytes; } return false; @@ -114,7 +118,7 @@ PeerTableModel::PeerTableModel(ClientModel *parent) : clientModel(parent), timer(0) { - columns << tr("NodeId") << tr("Node/Service") << tr("User Agent") << tr("Ping"); + columns << tr("NodeId") << tr("Node/Service") << tr("Ping") << tr("Sent") << tr("Received") << tr("User Agent"); priv.reset(new PeerTablePriv()); // default to unsorted priv->sortColumn = -1; @@ -173,10 +177,20 @@ QVariant PeerTableModel::data(const QModelIndex &index, int role) const return QString::fromStdString(rec->nodeStats.cleanSubVer); case Ping: return GUIUtil::formatPingTime(rec->nodeStats.dMinPing); + case Sent: + return GUIUtil::formatBytes(rec->nodeStats.nSendBytes); + case Received: + return GUIUtil::formatBytes(rec->nodeStats.nRecvBytes); } } else if (role == Qt::TextAlignmentRole) { - if (index.column() == Ping) - return (QVariant)(Qt::AlignRight | Qt::AlignVCenter); + switch (index.column()) { + case Ping: + case Sent: + case Received: + return QVariant(Qt::AlignRight | Qt::AlignVCenter); + default: + return QVariant(); + } } return QVariant(); diff --git a/src/qt/peertablemodel.h b/src/qt/peertablemodel.h index 3e4f0a68c460..af98417f3467 100644 --- a/src/qt/peertablemodel.h +++ b/src/qt/peertablemodel.h @@ -55,8 +55,10 @@ class PeerTableModel : public QAbstractTableModel enum ColumnIndex { NetNodeId = 0, Address = 1, - Subversion = 2, - Ping = 3 + Ping = 2, + Sent = 3, + Received = 4, + Subversion = 5 }; /** @name Methods overridden from QAbstractTableModel diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index cd6a4f9c983a..0f568f639c15 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -1043,18 +1043,6 @@ void RPCConsole::on_sldGraphRange_valueChanged(int value) setTrafficGraphRange(static_cast(value)); } -QString RPCConsole::FormatBytes(quint64 bytes) -{ - if(bytes < 1024) - return QString(tr("%1 B")).arg(bytes); - if(bytes < 1024 * 1024) - return QString(tr("%1 KB")).arg(bytes / 1024); - if(bytes < 1024 * 1024 * 1024) - return QString(tr("%1 MB")).arg(bytes / 1024 / 1024); - - return QString(tr("%1 GB")).arg(bytes / 1024 / 1024 / 1024); -} - void RPCConsole::setTrafficGraphRange(TrafficGraphData::GraphRange range) { ui->trafficGraph->setGraphRangeMins(range); @@ -1063,8 +1051,8 @@ void RPCConsole::setTrafficGraphRange(TrafficGraphData::GraphRange range) void RPCConsole::updateTrafficStats(quint64 totalBytesIn, quint64 totalBytesOut) { - ui->lblBytesIn->setText(FormatBytes(totalBytesIn)); - ui->lblBytesOut->setText(FormatBytes(totalBytesOut)); + ui->lblBytesIn->setText(GUIUtil::formatBytes(totalBytesIn)); + ui->lblBytesOut->setText(GUIUtil::formatBytes(totalBytesOut)); } void RPCConsole::peerSelected(const QItemSelection &selected, const QItemSelection &deselected) @@ -1158,8 +1146,8 @@ void RPCConsole::updateNodeDetail(const CNodeCombinedStats *stats) ui->peerServices->setText(GUIUtil::formatServicesStr(stats->nodeStats.nServices)); ui->peerLastSend->setText(stats->nodeStats.nLastSend ? GUIUtil::formatDurationStr(GetSystemTimeInSeconds() - stats->nodeStats.nLastSend) : tr("never")); ui->peerLastRecv->setText(stats->nodeStats.nLastRecv ? GUIUtil::formatDurationStr(GetSystemTimeInSeconds() - stats->nodeStats.nLastRecv) : tr("never")); - ui->peerBytesSent->setText(FormatBytes(stats->nodeStats.nSendBytes)); - ui->peerBytesRecv->setText(FormatBytes(stats->nodeStats.nRecvBytes)); + ui->peerBytesSent->setText(GUIUtil::formatBytes(stats->nodeStats.nSendBytes)); + ui->peerBytesRecv->setText(GUIUtil::formatBytes(stats->nodeStats.nRecvBytes)); ui->peerConnTime->setText(GUIUtil::formatDurationStr(GetSystemTimeInSeconds() - stats->nodeStats.nTimeConnected)); ui->peerPingTime->setText(GUIUtil::formatPingTime(stats->nodeStats.dPingTime)); ui->peerPingWait->setText(GUIUtil::formatPingTime(stats->nodeStats.dPingWait)); diff --git a/src/qt/rpcconsole.h b/src/qt/rpcconsole.h index 748a00476129..9bec0d365509 100644 --- a/src/qt/rpcconsole.h +++ b/src/qt/rpcconsole.h @@ -140,7 +140,6 @@ public Q_SLOTS: void handleRestart(QStringList args); private: - static QString FormatBytes(quint64 bytes); void startExecutor(); void setTrafficGraphRange(TrafficGraphData::GraphRange range); /** Build parameter list for restart */ From 72d8f89c919165632da6ddc5c3f7721dda8b7db1 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Sat, 28 Oct 2017 16:17:35 +0200 Subject: [PATCH 04/27] Merge #10409: [tests] Add fuzz testing for BlockTransactions and BlockTransactionsRequest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fd3a2f3 [tests] Add fuzz testing for BlockTransactions and BlockTransactionsRequest (practicalswift) Pull request description: The `BlockTransactions` deserialization code is reachable with tainted data via `ProcessMessage(…, "BLOCKTXN", vRecv [tainted], …)`. The same thing applies to `BlockTransactionsRequest` which is reachable via `"GETBLOCKTXN"`. Tree-SHA512: 64560ea344bc6145b940472f99866b808725745b060dedfb315be400bd94e55399f50b982149645bd7af7ed9935fd28751d7daf0d3f94a8e2ed3bc52e3325ffb --- src/test/test_dash_fuzzy.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/test/test_dash_fuzzy.cpp b/src/test/test_dash_fuzzy.cpp index dc38d1abf1dd..ed99ae27bf0b 100644 --- a/src/test/test_dash_fuzzy.cpp +++ b/src/test/test_dash_fuzzy.cpp @@ -19,6 +19,7 @@ #include "undo.h" #include "version.h" #include "pubkey.h" +#include "blockencodings.h" #include #include @@ -45,6 +46,8 @@ enum TEST_ID { CBLOOMFILTER_DESERIALIZE, CDISKBLOCKINDEX_DESERIALIZE, CTXOUTCOMPRESSOR_DESERIALIZE, + BLOCKTRANSACTIONS_DESERIALIZE, + BLOCKTRANSACTIONSREQUEST_DESERIALIZE, TEST_ID_END }; @@ -249,6 +252,26 @@ int do_fuzz() break; } + case BLOCKTRANSACTIONS_DESERIALIZE: + { + try + { + BlockTransactions bt; + ds >> bt; + } catch (const std::ios_base::failure& e) {return 0;} + + break; + } + case BLOCKTRANSACTIONSREQUEST_DESERIALIZE: + { + try + { + BlockTransactionsRequest btr; + ds >> btr; + } catch (const std::ios_base::failure& e) {return 0;} + + break; + } default: return 0; } From 0160203306aba00662a8d9c4d694eeac1af40c74 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 4 Oct 2017 15:07:38 +0200 Subject: [PATCH 05/27] Merge #11435: build: Make "make clean" remove all files created when running "make check" f35d033 build: Make "make clean" remove all files created when running "make check" (practicalswift) Pull request description: Make `make clean` remove all files created when running `make check`. More specifically: remove also `obj/build.h` and `bench/data/block413567.raw.h` as part of `make clean`. Before this patch: ```bash $ git clone https://github.com/bitcoin/bitcoin.git $ cd bitcoin/ $ ./autogen.sh $ ./configure $ cp -r ../bitcoin ../bitcoin-before-make $ make check $ make clean $ cp -r ../bitcoin ../bitcoin-after-make-and-make-clean $ cd .. $ diff -rq bitcoin-before-make/ bitcoin-after-make-and-make-clean/ | grep -E "^Only in bitcoin-after-make-and-make-clean/" | grep -v dirstamp Only in bitcoin-after-make-and-make-clean/src/bench/data: block413567.raw.h Only in bitcoin-after-make-and-make-clean/src/obj: build.h $ ``` After this patch: ```bash $ git clone https://github.com/bitcoin/bitcoin.git $ cd bitcoin/ $ ./autogen.sh $ ./configure $ cp -r ../bitcoin ../bitcoin-before-make $ make check $ make clean $ cp -r ../bitcoin ../bitcoin-after-make-and-make-clean $ cd .. $ diff -rq bitcoin-before-make/ bitcoin-after-make-and-make-clean/ | grep -E "^Only in bitcoin-after-make-and-make-clean/" | grep -v dirstamp $ ``` Tree-SHA512: 953e8423485ffd415f0ade6abe0b4c407454f67c332140ef019d89db425bb4a831327b3f634b8d69b17325dcfc6e3ac72dc2ba1ce5462158eecc3c05645e93ba --- src/Makefile.am | 3 +-- src/Makefile.bench.include | 9 +++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index c3243fcb9c3e..1b9664675932 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -669,11 +669,10 @@ CLEANFILES += univalue/*.gcda univalue/*.gcno CLEANFILES += wallet/*.gcda wallet/*.gcno CLEANFILES += wallet/test/*.gcda wallet/test/*.gcno CLEANFILES += zmq/*.gcda zmq/*.gcno +CLEANFILES += obj/build.h IMMER_DIST = immer -DISTCLEANFILES = obj/build.h - EXTRA_DIST = $(CTAES_DIST) EXTRA_DIST += $(IMMER_DIST) diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index 8f08f5e2b577..20192f9fed9f 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -7,11 +7,12 @@ bin_PROGRAMS += bench/bench_dash BENCH_SRCDIR = bench BENCH_BINARY = bench/bench_dash$(EXEEXT) -RAW_TEST_FILES = \ +RAW_BENCH_FILES = \ bench/data/block813851.raw -GENERATED_TEST_FILES = $(RAW_TEST_FILES:.raw=.raw.h) +GENERATED_BENCH_FILES = $(RAW_BENCH_FILES:.raw=.raw.h) bench_bench_dash_SOURCES = \ + $(RAW_BENCH_FILES) \ bench/bench_dash.cpp \ bench/bench.cpp \ bench/bench.h \ @@ -36,7 +37,7 @@ bench_bench_dash_SOURCES = \ bench/prevector.cpp \ bench/string_cast.cpp -nodist_bench_bench_dash_SOURCES = $(GENERATED_TEST_FILES) +nodist_bench_bench_dash_SOURCES = $(GENERATED_BENCH_FILES) bench_bench_dash_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(EVENT_CLFAGS) $(EVENT_PTHREADS_CFLAGS) -I$(builddir)/bench/ bench_bench_dash_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) @@ -64,7 +65,7 @@ endif bench_bench_dash_LDADD += $(BACKTRACE_LIB) $(BOOST_LIBS) $(BDB_LIBS) $(SSL_LIBS) $(CRYPTO_LIBS) $(MINIUPNPC_LIBS) $(EVENT_PTHREADS_LIBS) $(EVENT_LIBS) $(BLS_LIBS) bench_bench_dash_LDFLAGS = $(LDFLAGS_WRAP_EXCEPTIONS) $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) -CLEAN_BITCOIN_BENCH = bench/*.gcda bench/*.gcno $(GENERATED_TEST_FILES) +CLEAN_BITCOIN_BENCH = bench/*.gcda bench/*.gcno $(GENERATED_BENCH_FILES) CLEANFILES += $(CLEAN_BITCOIN_BENCH) From 980a053af836ef560e9461665c1f1dc6a519a375 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 5 Oct 2017 15:03:09 +0200 Subject: [PATCH 06/27] Merge #11107: Fix races in AppInitMain and others with lock and atomic bools c626dcb50 Make fUseCrypto atomic (MeshCollider) 731065b11 Consistent parameter names in txdb.h (MeshCollider) 35aeabec6 Make fReindex atomic to avoid race (MeshCollider) 58d91af59 Fix race for mapBlockIndex in AppInitMain (MeshCollider) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/11106 Also makes fReindex atomic as suggested in @TheBlueMatt comment below, and makes fUseCrypto atomic as suggested in 10916 https://github.com/bitcoin/bitcoin/pull/11107/commits/d291e7635b0ef4156c2805c6c4ee1adad91f0307 just renames the parameters in the txdb header file to make them consistent with those used in the cpp file, noticed it when looking for uses of fReindex Tree-SHA512: b378aa7289fd505b76565cd4d48dcdc04ac5540283ea1c80442170b0f13cb6df771b1a94dd54b7fec3478a7b4668c224ec9d795f16937782724c5d020edd3a42 --- src/init.cpp | 12 +++++++++--- src/txdb.h | 8 ++++---- src/validation.cpp | 4 ++-- src/validation.h | 2 +- src/wallet/crypter.h | 4 +++- 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 542c64580899..d5efb219e17c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2189,9 +2189,15 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) // ********************************************************* Step 12: start node + int chain_active_height; + //// debug print - LogPrintf("mapBlockIndex.size() = %u\n", mapBlockIndex.size()); - LogPrintf("chainActive.Height() = %d\n", chainActive.Height()); + { + LOCK(cs_main); + LogPrintf("mapBlockIndex.size() = %u\n", mapBlockIndex.size()); + chain_active_height = chainActive.Height(); + } + LogPrintf("chainActive.Height() = %d\n", chain_active_height); if (gArgs.GetBoolArg("-listenonion", DEFAULT_LISTEN_ONION)) StartTorControl(threadGroup, scheduler); @@ -2206,7 +2212,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) connOptions.nMaxOutbound = std::min(MAX_OUTBOUND_CONNECTIONS, connOptions.nMaxConnections); connOptions.nMaxAddnode = MAX_ADDNODE_CONNECTIONS; connOptions.nMaxFeeler = 1; - connOptions.nBestHeight = chainActive.Height(); + connOptions.nBestHeight = chain_active_height; connOptions.uiInterface = &uiInterface; connOptions.m_msgproc = peerLogic.get(); connOptions.nSendBufferMaxSize = 1000*gArgs.GetArg("-maxsendbuffer", DEFAULT_MAXSENDBUFFER); diff --git a/src/txdb.h b/src/txdb.h index 092204668fbb..4f8019d3ed6b 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -117,13 +117,13 @@ class CBlockTreeDB : public CDBWrapper CBlockTreeDB& operator=(const CBlockTreeDB&) = delete; bool WriteBatchSync(const std::vector >& fileInfo, int nLastFile, const std::vector& blockinfo); - bool ReadBlockFileInfo(int nFile, CBlockFileInfo &fileinfo); + bool ReadBlockFileInfo(int nFile, CBlockFileInfo &info); bool ReadLastBlockFile(int &nFile); - bool WriteReindexing(bool fReindex); - bool ReadReindexing(bool &fReindex); + bool WriteReindexing(bool fReindexing); + bool ReadReindexing(bool &fReindexing); bool HasTxIndex(const uint256 &txid); bool ReadTxIndex(const uint256 &txid, CDiskTxPos &pos); - bool WriteTxIndex(const std::vector > &list); + bool WriteTxIndex(const std::vector > &vect); bool ReadSpentIndex(CSpentIndexKey &key, CSpentIndexValue &value); bool UpdateSpentIndex(const std::vector >&vect); bool UpdateAddressUnspentIndex(const std::vector >&vect); diff --git a/src/validation.cpp b/src/validation.cpp index 8315a4626651..7b69a61b2b64 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -81,7 +81,7 @@ CWaitableCriticalSection csBestBlock; CConditionVariable cvBlockChange; int nScriptCheckThreads = 0; std::atomic_bool fImporting(false); -bool fReindex = false; +std::atomic_bool fReindex(false); bool fTxIndex = true; bool fAddressIndex = false; bool fTimestampIndex = false; @@ -3968,7 +3968,7 @@ bool static LoadBlockIndexDB(const CChainParams& chainparams) // Check whether we need to continue reindexing bool fReindexing = false; pblocktree->ReadReindexing(fReindexing); - fReindex |= fReindexing; + if(fReindexing) fReindex = true; // Check whether we have a transaction index pblocktree->ReadFlag("txindex", fTxIndex); diff --git a/src/validation.h b/src/validation.h index 841d856844fb..4ee306055634 100644 --- a/src/validation.h +++ b/src/validation.h @@ -170,7 +170,7 @@ extern const std::string strMessageMagic; extern CWaitableCriticalSection csBestBlock; extern CConditionVariable cvBlockChange; extern std::atomic_bool fImporting; -extern bool fReindex; +extern std::atomic_bool fReindex; extern int nScriptCheckThreads; extern bool fTxIndex; extern bool fAddressIndex; diff --git a/src/wallet/crypter.h b/src/wallet/crypter.h index 18ecdd71dfbd..71ddfa73d649 100644 --- a/src/wallet/crypter.h +++ b/src/wallet/crypter.h @@ -9,6 +9,8 @@ #include "serialize.h" #include "support/allocators/secure.h" +#include + const unsigned int WALLET_CRYPTO_KEY_SIZE = 32; const unsigned int WALLET_CRYPTO_SALT_SIZE = 8; const unsigned int WALLET_CRYPTO_IV_SIZE = 16; @@ -124,7 +126,7 @@ class CCryptoKeyStore : public CBasicKeyStore //! if fUseCrypto is true, mapKeys must be empty //! if fUseCrypto is false, vMasterKey must be empty - bool fUseCrypto; + std::atomic fUseCrypto; //! keeps track of whether Unlock has run a thorough check before bool fDecryptionThoroughlyChecked; From aef1eb00a4626a128aa51ae96a7aa1b90538209c Mon Sep 17 00:00:00 2001 From: Pasta Date: Sun, 22 Dec 2019 00:07:33 -0600 Subject: [PATCH 07/27] remove explicit on FreespaceChecker Signed-off-by: Pasta --- src/qt/intro.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qt/intro.cpp b/src/qt/intro.cpp index 4812e5c5abfe..d64a14068de7 100644 --- a/src/qt/intro.cpp +++ b/src/qt/intro.cpp @@ -44,7 +44,7 @@ class FreespaceChecker : public QObject Q_OBJECT public: - explicit FreespaceChecker(Intro *intro); + FreespaceChecker(Intro *intro); enum Status { ST_OK, From 6d4839fd8942fffab36bfa12d568ca29a74ac6dd Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 9 Nov 2017 13:07:40 +0100 Subject: [PATCH 08/27] Merge #11552: Improve wallet-accounts test bc9c0a7 Improve wallet-accounts test (Russell Yanofsky) Pull request description: Add comments and - Verify sending to a account causes getaccountaddress to generate new addresses. - Verify sending to a account causes getreceivedbyaccount to return amount received. - Verify ways setaccount updates the accounts of existing addresses. Tree-SHA512: 4facc8d10fb23847081e8081839d4050faa855cca40f34012b82297bf1a8cfdb975b26314bc518dc6b3f5997452a7acf309f9627e0dff359347c628b37014bfa --- test/functional/wallet-accounts.py | 139 ++++++++++++++++++++++------- 1 file changed, 106 insertions(+), 33 deletions(-) diff --git a/test/functional/wallet-accounts.py b/test/functional/wallet-accounts.py index e267dcfada0c..ea976a35437b 100755 --- a/test/functional/wallet-accounts.py +++ b/test/functional/wallet-accounts.py @@ -74,62 +74,135 @@ def run_test(self): # otherwise we're off by exactly the fee amount as that's mined # and matures in the next 100 blocks node.sendfrom("", common_address, fee) - accounts = ["a", "b", "c", "d", "e"] amount_to_send = 1.0 - account_addresses = dict() + + # Create accounts and make sure subsequent account API calls + # recognize the account/address associations. + accounts = [Account(name) for name in ("a", "b", "c", "d", "e")] for account in accounts: - address = node.getaccountaddress(account) - account_addresses[account] = address - - node.getnewaddress(account) - assert_equal(node.getaccount(address), account) - assert(address in node.getaddressesbyaccount(account)) - - node.sendfrom("", address, amount_to_send) - + account.add_receive_address(node.getaccountaddress(account.name)) + account.verify(node) + + # Send a transaction to each account, and make sure this forces + # getaccountaddress to generate a new receiving address. + for account in accounts: + node.sendtoaddress(account.receive_address, amount_to_send) + account.add_receive_address(node.getaccountaddress(account.name)) + account.verify(node) + + # Check the amounts received. node.generate(1) - - for i in range(len(accounts)): - from_account = accounts[i] + for account in accounts: + assert_equal( + node.getreceivedbyaddress(account.addresses[0]), amount_to_send) + assert_equal(node.getreceivedbyaccount(account.name), amount_to_send) + + # Check that sendfrom account reduces listaccounts balances. + for i, account in enumerate(accounts): to_account = accounts[(i+1) % len(accounts)] - to_address = account_addresses[to_account] - node.sendfrom(from_account, to_address, amount_to_send) - + node.sendfrom(account.name, to_account.receive_address, amount_to_send) node.generate(1) - for account in accounts: - address = node.getaccountaddress(account) - assert(address != account_addresses[account]) - assert_equal(node.getreceivedbyaccount(account), 2) - node.move(account, "", node.getbalance(account)) - + account.add_receive_address(node.getaccountaddress(account.name)) + account.verify(node) + assert_equal(node.getreceivedbyaccount(account.name), 2) + node.move(account.name, "", node.getbalance(account.name)) + account.verify(node) node.generate(101) - expected_account_balances = {"": 52000} for account in accounts: - expected_account_balances[account] = 0 - + expected_account_balances[account.name] = 0 assert_equal(node.listaccounts(), expected_account_balances) - assert_equal(node.getbalance(""), 52000) + # Check that setaccount can assign an account to a new unused address. for account in accounts: address = node.getaccountaddress("") - node.setaccount(address, account) - assert(address in node.getaddressesbyaccount(account)) + node.setaccount(address, account.name) + account.add_address(address) + account.verify(node) assert(address not in node.getaddressesbyaccount("")) + # Check that addmultisigaddress can assign accounts. for account in accounts: addresses = [] for x in range(10): addresses.append(node.getnewaddress()) - multisig_address = node.addmultisigaddress(5, addresses, account) + multisig_address = node.addmultisigaddress(5, addresses, account.name) + account.add_address(multisig_address) + account.verify(node) node.sendfrom("", multisig_address, 50) - node.generate(101) - for account in accounts: - assert_equal(node.getbalance(account), 50) + assert_equal(node.getbalance(account.name), 50) + + # Check that setaccount can change the account of an address from a + # different account. + change_account(node, accounts[0].addresses[0], accounts[0], accounts[1]) + + # Check that setaccount can change the account of an address which + # is the receiving address of a different account. + change_account(node, accounts[0].receive_address, accounts[0], accounts[1]) + + # Check that setaccount can set the account of an address already + # in the account. This is a no-op. + change_account(node, accounts[2].addresses[0], accounts[2], accounts[2]) + + # Check that setaccount can set the account of an address which is + # already the receiving address of the account. It would probably make + # sense for this to be a no-op, but right now it resets the receiving + # address, causing getaccountaddress to return a brand new address. + change_account(node, accounts[2].receive_address, accounts[2], accounts[2]) + +class Account: + def __init__(self, name): + # Account name + self.name = name + # Current receiving address associated with this account. + self.receive_address = None + # List of all addresses assigned with this account + self.addresses = [] + + def add_address(self, address): + assert_equal(address not in self.addresses, True) + self.addresses.append(address) + + def add_receive_address(self, address): + self.add_address(address) + self.receive_address = address + + def verify(self, node): + if self.receive_address is not None: + assert self.receive_address in self.addresses + assert_equal(node.getaccountaddress(self.name), self.receive_address) + + for address in self.addresses: + assert_equal(node.getaccount(address), self.name) + + assert_equal( + set(node.getaddressesbyaccount(self.name)), set(self.addresses)) + + +def change_account(node, address, old_account, new_account): + assert_equal(address in old_account.addresses, True) + node.setaccount(address, new_account.name) + + old_account.addresses.remove(address) + new_account.add_address(address) + + # Calling setaccount on an address which was previously the receiving + # address of a different account should reset the receiving address of + # the old account, causing getaccountaddress to return a brand new + # address. + if address == old_account.receive_address: + new_address = node.getaccountaddress(old_account.name) + assert_equal(new_address not in old_account.addresses, True) + assert_equal(new_address not in new_account.addresses, True) + old_account.add_receive_address(new_address) + + old_account.verify(node) + new_account.verify(node) + if __name__ == '__main__': WalletAccountsTest().main() From 9c914a1c142f714eace2557c040c7be228120253 Mon Sep 17 00:00:00 2001 From: Pasta Date: Sun, 22 Dec 2019 17:00:23 -0600 Subject: [PATCH 09/27] Merge #10529: Improve bitcoind systemd service file 16be7dd Improve bitcoind systemd service file (Florian Schmaus) Pull request description: Add comment how further options can be added or existing ones modified. Use /run/${RuntimeDirectory} for PID file. Remove TimeoutStopSec, TimeoutStartSec, StartLimitInterval, StartLimitBurst directives as those should be set indivdually. Remove Group to user the bitcoin user's default group. Changed Restart from 'always' to 'on-failure' (can also be overwritten individually). Tree-SHA512: f76674c11fd6e3faaf786aa05686926523d9c875aad6b776337f800108fdb716470286805c532b494f8cf713cb5eea6b735e1c7c238ffb407a5cc909dda41aa4 --- contrib/init/dashd.service | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/contrib/init/dashd.service b/contrib/init/dashd.service index a256c6a03a02..ea4548dfb2fd 100644 --- a/contrib/init/dashd.service +++ b/contrib/init/dashd.service @@ -1,22 +1,25 @@ +# It is not recommended to modify this file in-place, because it will +# be overwritten during package upgrades. If you want to add further +# options or overwrite existing ones then use +# $ systemctl edit dashd.service +# See "man systemd.service" for details. + +# Note that almost all daemon options could be specified in +# /etc/dash/dash.conf + [Unit] -Description=Dash's distributed currency daemon +Description=Dash daemon After=network.target [Service] +ExecStart=/usr/bin/dashd -daemon -conf=/etc/dash/dash.conf -pid=/run/dashd/dashd.pid +# Creates /run/dash owned by dashcore +RuntimeDirectory=dashd User=dashcore -Group=dashcore - Type=forking -PIDFile=/var/lib/dashd/dashd.pid -ExecStart=/usr/bin/dashd -daemon -pid=/var/lib/dashd/dashd.pid \ --conf=/etc/dashcore/dash.conf -datadir=/var/lib/dashd -disablewallet - -Restart=always +PIDFile=/run/dashd/dashd.pid +Restart=on-failure PrivateTmp=true -TimeoutStopSec=60s -TimeoutStartSec=2s -StartLimitInterval=120s -StartLimitBurst=5 [Install] WantedBy=multi-user.target From 79e841fac78275fb6a858573dfdf7fbc96fc997c Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 9 Nov 2017 13:31:58 +0100 Subject: [PATCH 10/27] Merge #11594: Improve -disablewallet parameter interaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 7963335 Fix -disablewallet default value (João Barbosa) b411c2a Improve -disablewallet parameter interaction (João Barbosa) Pull request description: The first commit logs a message for each configured wallet if `-disablewallet` is set: ``` bitcoind -printtoconsole -regtest -disablewallet -wallet=foo -wallet=bar ... WalletParameterInteraction: parameter interaction: -disablewallet -> ignoring -wallet=foo WalletParameterInteraction: parameter interaction: -disablewallet -> ignoring -wallet=bar ``` It also moves up the `-disablewallet` check which avoids the unnecessary `-wallet` soft set. The second commit fixes the default value of `-disablewallet`, currently the value is correct, but it should use `DEFAULT_DISABLE_WALLET`. The third commit can be dropped or squashed, just took the opportunity to fix the coding style there. Tree-SHA512: bec13d2b2be5adf4680c77212020ed27dd05f15c4c73542d2005d91108bf704e2df1707ed2bec696e584ecd40eff7a63e25201fd70400222aa5a8da6aed6afeb --- src/wallet/init.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 2e05ccd01f97..c57e91dc5181 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -64,11 +64,16 @@ std::string GetWalletHelpString(bool showDebug) bool WalletParameterInteraction() { - gArgs.SoftSetArg("-wallet", DEFAULT_WALLET_DAT); - const bool is_multiwallet = gArgs.GetArgs("-wallet").size() > 1; + if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) { + for (const std::string& wallet : gArgs.GetArgs("-wallet")) { + LogPrintf("%s: parameter interaction: -disablewallet -> ignoring -wallet=%s\n", __func__, wallet); + } - if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) return true; + } + + gArgs.SoftSetArg("-wallet", DEFAULT_WALLET_DAT); + const bool is_multiwallet = gArgs.GetArgs("-wallet").size() > 1; if (gArgs.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY) && gArgs.SoftSetBoolArg("-walletbroadcast", false)) { LogPrintf("%s: parameter interaction: -blocksonly=1 -> setting -walletbroadcast=0\n", __func__); @@ -192,15 +197,18 @@ bool WalletParameterInteraction() void RegisterWalletRPC(CRPCTable &t) { - if (gArgs.GetBoolArg("-disablewallet", false)) return; + if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) { + return; + } RegisterWalletRPCCommands(t); } bool VerifyWallets() { - if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) + if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) { return true; + } uiInterface.InitMessage(_("Verifying wallet(s)...")); From 3bae57af8dc256d3cca3819df47c85488958c6ee Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 9 Nov 2017 13:38:33 +0100 Subject: [PATCH 11/27] Merge #10696: Remove redundant nullptr checks before deallocation b109a1c Remove redundant nullptr checks before deallocation (practicalswift) Pull request description: Rationale: * `delete ptr` is a no-op if `ptr` is `nullptr` Tree-SHA512: c98ce769125c4912186a8403cc08a59cfba85b7141af645c709b4c4eb90dd9cbdd6ed8076d50099d1e4ec2bf75917d1af6844082ec42bbb4d94d229a710e051c --- src/net.cpp | 3 +-- src/qt/paymentrequestplus.cpp | 3 +-- src/qt/paymentserver.cpp | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index cae156799e9c..4cf72c3ce8d7 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3239,8 +3239,7 @@ CNode::~CNode() { CloseSocket(hSocket); - if (pfilter) - delete pfilter; + delete pfilter; } void CNode::AskFor(const CInv& inv, int64_t doubleRequestDelay) diff --git a/src/qt/paymentrequestplus.cpp b/src/qt/paymentrequestplus.cpp index 165e49915045..b52dc3455f35 100644 --- a/src/qt/paymentrequestplus.cpp +++ b/src/qt/paymentrequestplus.cpp @@ -197,8 +197,7 @@ bool PaymentRequestPlus::getMerchant(X509_STORE* certStore, QString& merchant) c qWarning() << "PaymentRequestPlus::getMerchant: SSL error: " << err.what(); } - if (website) - delete[] website; + delete[] website; X509_STORE_CTX_free(store_ctx); for (unsigned int i = 0; i < certs.size(); i++) X509_free(certs[i]); diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index 497fd3b79d78..42bcf3b79700 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -363,8 +363,7 @@ void PaymentServer::initNetManager() { if (!optionsModel) return; - if (netManager != nullptr) - delete netManager; + delete netManager; // netManager is used to fetch paymentrequests given in dash: URIs netManager = new QNetworkAccessManager(this); From 354f57a838ade93d2888dc08269ca4868cf5e18d Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 9 Nov 2017 14:22:22 +0100 Subject: [PATCH 12/27] Merge #10368: [wallet] Remove helper conversion operator from wallet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 5a5e4e9 [wallet] Remove CTransaction&() helper conversion operator from wallet implementation. (Karl-Johan Alm) Pull request description: The `CTransaction&()` operator in `CMerkleTx` makes conversion into `CTransaction`s transparent, but was marked as to-be-removed in favor of explicitly getting the `tx` ivar, presumably as the operator can lead to ambiguous behavior and makes the code harder to follow. This PR removes the operator and adapts callers. This includes some cases of `static_cast(wtx)` → `*wtx.tx`, which is definitely an improvement. Tree-SHA512: 95856fec7194d6a79615ea1c322abfcd6bcedf6ffd0cfa89bbdd332ce13035fa52dd4b828d20df673072dde1be64b79c513529a6f422dd5f0961ce722a32d56a --- src/qt/transactiondesc.cpp | 2 +- src/qt/transactionrecord.cpp | 2 +- src/qt/transactiontablemodel.cpp | 2 +- src/qt/walletmodeltransaction.cpp | 2 +- src/wallet/rpcdump.cpp | 2 +- src/wallet/rpcwallet.cpp | 2 +- src/wallet/test/accounting_tests.cpp | 4 ++-- src/wallet/wallet.cpp | 23 ++++++++++++----------- src/wallet/wallet.h | 4 ---- src/wallet/walletdb.cpp | 2 +- 10 files changed, 21 insertions(+), 24 deletions(-) diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index fdb3ef7736c6..032063021fdd 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -25,7 +25,7 @@ QString TransactionDesc::FormatTxStatus(const CWalletTx& wtx) { AssertLockHeld(cs_main); - if (!CheckFinalTx(wtx)) + if (!CheckFinalTx(*wtx.tx)) { if (wtx.tx->nLockTime < LOCKTIME_THRESHOLD) return tr("Open for %n more block(s)", "", wtx.tx->nLockTime - chainActive.Height()); diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index bf54f0744839..a1bca6bce39e 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -285,7 +285,7 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx, int chainLockHeight) status.label = QString::fromStdString(addrBookIt->second.name); } - if (!CheckFinalTx(wtx)) + if (!CheckFinalTx(*wtx.tx)) { if (wtx.tx->nLockTime < LOCKTIME_THRESHOLD) { diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 9cdabba8bd86..af51573c3441 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -244,7 +244,7 @@ class TransactionTablePriv std::map::iterator mi = wallet->mapWallet.find(rec->hash); if(mi != wallet->mapWallet.end()) { - std::string strHex = EncodeHexTx(static_cast(mi->second)); + std::string strHex = EncodeHexTx(*mi->second.tx); return QString::fromStdString(strHex); } return QString(); diff --git a/src/qt/walletmodeltransaction.cpp b/src/qt/walletmodeltransaction.cpp index 8d7c8ff87d86..0703b029470a 100644 --- a/src/qt/walletmodeltransaction.cpp +++ b/src/qt/walletmodeltransaction.cpp @@ -33,7 +33,7 @@ CWalletTx *WalletModelTransaction::getTransaction() const unsigned int WalletModelTransaction::getTransactionSize() { - return (!walletTransaction ? 0 : (::GetSerializeSize(walletTransaction->tx, SER_NETWORK, PROTOCOL_VERSION))); + return (!walletTransaction ? 0 : (::GetSerializeSize(*walletTransaction->tx, SER_NETWORK, PROTOCOL_VERSION))); } CAmount WalletModelTransaction::getTransactionFee() const diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 28c948892110..d9017dcfd395 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -341,7 +341,7 @@ UniValue importprunedfunds(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); - if (pwallet->IsMine(wtx)) { + if (pwallet->IsMine(*wtx.tx)) { pwallet->AddToWallet(wtx, false); return NullUniValue; } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 757f07424a16..291b47da422f 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1997,7 +1997,7 @@ UniValue gettransaction(const JSONRPCRequest& request) ListTransactions(pwallet, wtx, "*", 0, false, details, filter); entry.push_back(Pair("details", details)); - std::string strHex = EncodeHexTx(static_cast(wtx)); + std::string strHex = EncodeHexTx(*wtx.tx); entry.push_back(Pair("hex", strHex)); return entry; diff --git a/src/wallet/test/accounting_tests.cpp b/src/wallet/test/accounting_tests.cpp index d597bd911dcc..cb4b889633e3 100644 --- a/src/wallet/test/accounting_tests.cpp +++ b/src/wallet/test/accounting_tests.cpp @@ -83,7 +83,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) wtx.mapValue["comment"] = "y"; { - CMutableTransaction tx(wtx); + CMutableTransaction tx(*wtx.tx); --tx.nLockTime; // Just to change the hash :) wtx.SetTx(MakeTransactionRef(std::move(tx))); } @@ -93,7 +93,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) wtx.mapValue["comment"] = "x"; { - CMutableTransaction tx(wtx); + CMutableTransaction tx(*wtx.tx); --tx.nLockTime; // Just to change the hash :) wtx.SetTx(MakeTransactionRef(std::move(tx))); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 08a81a4ba7fc..f39adf415755 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1493,6 +1493,7 @@ int CWallet::GetRealOutpointPrivateSendRounds(const COutPoint& outpoint, int nRo uint256 hash = outpoint.hash; unsigned int nout = outpoint.n; + // TODO wtx should refer to a CWalletTx object, not a pointer, based on surrounding code const CWalletTx* wtx = GetWalletTx(hash); if(wtx != nullptr) { @@ -1500,7 +1501,7 @@ int CWallet::GetRealOutpointPrivateSendRounds(const COutPoint& outpoint, int nRo if (mdwi == mDenomWtxes.end()) { // not known yet, let's add it LogPrint(BCLog::PRIVATESEND, "GetRealOutpointPrivateSendRounds INSERTING %s\n", hash.ToString()); - mDenomWtxes[hash] = CMutableTransaction(*wtx); + mDenomWtxes[hash] = CMutableTransaction(*wtx->tx); } else if(mDenomWtxes[hash].vout[nout].nRounds != -10) { // found and it's not an initial value, just return it return mDenomWtxes[hash].vout[nout].nRounds; @@ -2043,7 +2044,7 @@ bool CWalletTx::RelayWalletTransaction(CConnman* connman) LogPrintf("Relaying wtx %s\n", hash.ToString()); if (connman) { - connman->RelayTransaction((CTransaction)*this); + connman->RelayTransaction(*tx); return true; } } @@ -2075,7 +2076,7 @@ CAmount CWalletTx::GetDebit(const isminefilter& filter) const debit += nDebitCached; else { - nDebitCached = pwallet->GetDebit(*this, ISMINE_SPENDABLE); + nDebitCached = pwallet->GetDebit(*tx, ISMINE_SPENDABLE); fDebitCached = true; debit += nDebitCached; } @@ -2086,7 +2087,7 @@ CAmount CWalletTx::GetDebit(const isminefilter& filter) const debit += nWatchDebitCached; else { - nWatchDebitCached = pwallet->GetDebit(*this, ISMINE_WATCH_ONLY); + nWatchDebitCached = pwallet->GetDebit(*tx, ISMINE_WATCH_ONLY); fWatchDebitCached = true; debit += nWatchDebitCached; } @@ -2108,7 +2109,7 @@ CAmount CWalletTx::GetCredit(const isminefilter& filter) const credit += nCreditCached; else { - nCreditCached = pwallet->GetCredit(*this, ISMINE_SPENDABLE); + nCreditCached = pwallet->GetCredit(*tx, ISMINE_SPENDABLE); fCreditCached = true; credit += nCreditCached; } @@ -2119,7 +2120,7 @@ CAmount CWalletTx::GetCredit(const isminefilter& filter) const credit += nWatchCreditCached; else { - nWatchCreditCached = pwallet->GetCredit(*this, ISMINE_WATCH_ONLY); + nWatchCreditCached = pwallet->GetCredit(*tx, ISMINE_WATCH_ONLY); fWatchCreditCached = true; credit += nWatchCreditCached; } @@ -2133,7 +2134,7 @@ CAmount CWalletTx::GetImmatureCredit(bool fUseCache) const { if (fUseCache && fImmatureCreditCached) return nImmatureCreditCached; - nImmatureCreditCached = pwallet->GetCredit(*this, ISMINE_SPENDABLE); + nImmatureCreditCached = pwallet->GetCredit(*tx, ISMINE_SPENDABLE); fImmatureCreditCached = true; return nImmatureCreditCached; } @@ -2177,7 +2178,7 @@ CAmount CWalletTx::GetImmatureWatchOnlyCredit(const bool& fUseCache) const { if (fUseCache && fImmatureWatchCreditCached) return nImmatureWatchCreditCached; - nImmatureWatchCreditCached = pwallet->GetCredit(*this, ISMINE_WATCH_ONLY); + nImmatureWatchCreditCached = pwallet->GetCredit(*tx, ISMINE_WATCH_ONLY); fImmatureWatchCreditCached = true; return nImmatureWatchCreditCached; } @@ -2297,7 +2298,7 @@ CAmount CWalletTx::GetChange() const { if (fChangeCached) return nChangeCached; - nChangeCached = pwallet->GetChange(*this); + nChangeCached = pwallet->GetChange(*tx); fChangeCached = true; return nChangeCached; } @@ -2311,7 +2312,7 @@ bool CWalletTx::InMempool() const bool CWalletTx::IsTrusted() const { // Quick answer in most cases - if (!CheckFinalTx(*this)) + if (!CheckFinalTx(*tx)) return false; int nDepth = GetDepthInMainChain(); if (nDepth >= 1) @@ -2677,7 +2678,7 @@ void CWallet::AvailableCoins(std::vector &vCoins, bool fOnlySafe, const for (auto pcoin : GetSpendableTXs()) { const uint256& wtxid = pcoin->GetHash(); - if (!CheckFinalTx(*pcoin)) + if (!CheckFinalTx(*pcoin->tx)) continue; if (pcoin->IsCoinBase() && pcoin->GetBlocksToMaturity() > 0) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 2ca5aee8f844..728799d5863f 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -229,10 +229,6 @@ class CMerkleTx Init(); } - /** Helper conversion operator to allow passing CMerkleTx where CTransaction is expected. - * TODO: adapt callers and remove this operator. */ - operator const CTransaction&() const { return *tx; } - void Init() { hashBlock = uint256(); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index d98b51343df1..3c9b96a617f8 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -275,7 +275,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, CWalletTx wtx; ssValue >> wtx; CValidationState state; - if (!(CheckTransaction(wtx, state) && (wtx.GetHash() == hash) && state.IsValid())) + if (!(CheckTransaction(*wtx.tx, state) && (wtx.GetHash() == hash) && state.IsValid())) return false; // Undo serialize changes in 31600 From 134392f42123bd0be846e5c2a6f7be5884fe6580 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 9 Nov 2017 15:19:55 +0100 Subject: [PATCH 13/27] Merge #11074: Assert that CWallet::SyncMetaData finds oldest transaction. 6c4042a Assert that CWallet::SyncMetaData finds oldest transaction. (Eelis) Pull request description: Without this assert, the Clang static analyzer warns about subsequent dereferencing of copyFrom, because it can't be sure that it's not nullptr. See #9573. Tree-SHA512: 83cbcb32c52c94fcfefbc90ec7de2011dacd6bdb0da35adc401b8d8dda6a86de2fa0403e2158592268c2cf15eef4f3d887d98c90f1031d4735d5f4bf9dbc1d23 --- src/wallet/wallet.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index f39adf415755..576f5a7c8969 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -679,6 +679,9 @@ void CWallet::SyncMetaData(std::pair ran copyFrom = &mapWallet[hash]; } } + + assert(copyFrom); + // Now copy data from copyFrom to rest: for (TxSpends::iterator it = range.first; it != range.second; ++it) { From bb43baf7c49667bbddebe38cbd3c192913f79fe7 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 10 Nov 2017 10:43:50 -0500 Subject: [PATCH 14/27] Merge #11316: [qt] Add use available balance in send coins dialog (CryptAxe, promag) d052e3847 [qt] Add use available balance in send coins dialog (CryptAxe) Pull request description: This is an alternative to #11098 to handle #11033 where a new button `Use available balance` is added to each entry. When activated, the available balance is calculated by using the coin control (if any) and then it's subtracted the remaining recipient amounts. If this amount is positive then the `Subtract fee from amount` is automatically selected. Comparing to #11098, this has the advantage to avoid the fair amount division over the recipients and allows to fine adjust the amounts in multiple iterations. Started from @CryptAxe commit 89e9eda to credit some code. screen shot 2017-09-13 at 01 32 44 screen shot 2017-09-13 at 01 44 57 Tree-SHA512: 01d20c13fd8b6c2a0ca1d74d3a9027c6922e6dccd3b08e59d5a72636be7072ed5eca7ebc5d431299497dd3374e83753220ad4174d8bc46dadb4b2f54973036a5 --- src/qt/forms/sendcoinsentry.ui | 9 ++++++++- src/qt/sendcoinsdialog.cpp | 26 ++++++++++++++++++++++++++ src/qt/sendcoinsdialog.h | 1 + src/qt/sendcoinsentry.cpp | 16 ++++++++++++++++ src/qt/sendcoinsentry.h | 4 ++++ 5 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/qt/forms/sendcoinsentry.ui b/src/qt/forms/sendcoinsentry.ui index 20433f7a7675..c2330854ce84 100644 --- a/src/qt/forms/sendcoinsentry.ui +++ b/src/qt/forms/sendcoinsentry.ui @@ -151,7 +151,7 @@ - + @@ -165,6 +165,13 @@ + + + + Use available balance + + + diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index b3d15b8e68b8..1d7aa9a46bae 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -497,6 +497,7 @@ SendCoinsEntry *SendCoinsDialog::addEntry() entry->setModel(model); ui->entries->addWidget(entry); connect(entry, SIGNAL(removeEntry(SendCoinsEntry*)), this, SLOT(removeEntry(SendCoinsEntry*))); + connect(entry, SIGNAL(useAvailableBalance(SendCoinsEntry*)), this, SLOT(useAvailableBalance(SendCoinsEntry*))); connect(entry, SIGNAL(payAmountChanged()), this, SLOT(coinControlUpdateLabels())); connect(entry, SIGNAL(subtractFeeFromAmountChanged()), this, SLOT(coinControlUpdateLabels())); @@ -707,6 +708,31 @@ void SendCoinsDialog::on_buttonMinimizeFee_clicked() minimizeFeeSection(true); } +void SendCoinsDialog::useAvailableBalance(SendCoinsEntry* entry) +{ + // Get CCoinControl instance if CoinControl is enabled or create a new one. + CCoinControl coin_control; + if (model->getOptionsModel()->getCoinControlFeatures()) { + coin_control = *CoinControlDialog::coinControl; + } + + // Calculate available amount to send. + CAmount amount = model->getBalance(&coin_control); + for (int i = 0; i < ui->entries->count(); ++i) { + SendCoinsEntry* e = qobject_cast(ui->entries->itemAt(i)->widget()); + if (e && !e->isHidden() && e != entry) { + amount -= e->getValue().amount; + } + } + + if (amount > 0) { + entry->checkSubtractFeeFromAmount(); + entry->setAmount(amount); + } else { + entry->setAmount(0); + } +} + void SendCoinsDialog::setMinimumFee() { ui->customFee->setValue(GetRequiredFee(1000)); diff --git a/src/qt/sendcoinsdialog.h b/src/qt/sendcoinsdialog.h index 0d95bc091e8e..60e9259f4365 100644 --- a/src/qt/sendcoinsdialog.h +++ b/src/qt/sendcoinsdialog.h @@ -79,6 +79,7 @@ private Q_SLOTS: void on_buttonChooseFee_clicked(); void on_buttonMinimizeFee_clicked(); void removeEntry(SendCoinsEntry* entry); + void useAvailableBalance(SendCoinsEntry* entry); void updateDisplayUnit(); void coinControlFeatureChanged(bool); void coinControlButtonClicked(); diff --git a/src/qt/sendcoinsentry.cpp b/src/qt/sendcoinsentry.cpp index 92292a8423ad..c07764a2737c 100644 --- a/src/qt/sendcoinsentry.cpp +++ b/src/qt/sendcoinsentry.cpp @@ -50,6 +50,7 @@ SendCoinsEntry::SendCoinsEntry(const PlatformStyle *_platformStyle, QWidget *par connect(ui->deleteButton, SIGNAL(clicked()), this, SLOT(deleteClicked())); connect(ui->deleteButton_is, SIGNAL(clicked()), this, SLOT(deleteClicked())); connect(ui->deleteButton_s, SIGNAL(clicked()), this, SLOT(deleteClicked())); + connect(ui->useAvailableBalanceButton, SIGNAL(clicked()), this, SLOT(useAvailableBalanceClicked())); } SendCoinsEntry::~SendCoinsEntry() @@ -114,11 +115,21 @@ void SendCoinsEntry::clear() updateDisplayUnit(); } +void SendCoinsEntry::checkSubtractFeeFromAmount() +{ + ui->checkboxSubtractFeeFromAmount->setChecked(true); +} + void SendCoinsEntry::deleteClicked() { Q_EMIT removeEntry(this); } +void SendCoinsEntry::useAvailableBalanceClicked() +{ + Q_EMIT useAvailableBalance(this); +} + bool SendCoinsEntry::validate() { if (!model) @@ -230,6 +241,11 @@ void SendCoinsEntry::setAddress(const QString &address) ui->payAmount->setFocus(); } +void SendCoinsEntry::setAmount(const CAmount &amount) +{ + ui->payAmount->setValue(amount); +} + bool SendCoinsEntry::isClear() { return ui->payTo->text().isEmpty() && ui->payTo_is->text().isEmpty() && ui->payTo_s->text().isEmpty(); diff --git a/src/qt/sendcoinsentry.h b/src/qt/sendcoinsentry.h index a8be670c2aa0..557fea4e5476 100644 --- a/src/qt/sendcoinsentry.h +++ b/src/qt/sendcoinsentry.h @@ -38,6 +38,7 @@ class SendCoinsEntry : public QStackedWidget void setValue(const SendCoinsRecipient &value); void setAddress(const QString &address); + void setAmount(const CAmount &amount); /** Set up the tab chain manually, as Qt messes up the tab chain by default in some cases * (issue https://bugreports.qt-project.org/browse/QTBUG-10907). @@ -48,14 +49,17 @@ class SendCoinsEntry : public QStackedWidget public Q_SLOTS: void clear(); + void checkSubtractFeeFromAmount(); Q_SIGNALS: void removeEntry(SendCoinsEntry *entry); + void useAvailableBalance(SendCoinsEntry* entry); void payAmountChanged(); void subtractFeeFromAmountChanged(); private Q_SLOTS: void deleteClicked(); + void useAvailableBalanceClicked(); void on_payTo_textChanged(const QString &address); void on_addressBookButton_clicked(); void on_pasteButton_clicked(); From efea72890cd3424855aafc6c93d59165bde3c5e9 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 10 Nov 2017 11:52:27 -0500 Subject: [PATCH 15/27] Merge #11623: tests: Add missing locks to tests 109a85899 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see #11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9 Signed-off-by: Pasta # Conflicts: # src/test/test_bitcoin.cpp --- src/qt/test/wallettests.cpp | 5 +++- src/test/DoS_tests.cpp | 44 +++++++++++++++++++++++----- src/test/blockencodings_tests.cpp | 3 ++ src/test/mempool_tests.cpp | 2 ++ src/test/test_bitcoin.cpp | 0 src/test/txvalidationcache_tests.cpp | 6 +++- src/wallet/test/wallet_tests.cpp | 5 ++++ 7 files changed, 56 insertions(+), 9 deletions(-) create mode 100644 src/test/test_bitcoin.cpp diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 6454a04c7cb7..278e80f32fd2 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -94,7 +94,10 @@ void TestSendCoins() wallet.SetAddressBook(test.coinbaseKey.GetPubKey().GetID(), "", "receive"); wallet.AddKeyPubKey(test.coinbaseKey, test.coinbaseKey.GetPubKey()); } - wallet.ScanForWalletTransactions(chainActive.Genesis(), nullptr, true); + { + LOCK(cs_main); + wallet.ScanForWalletTransactions(chainActive.Genesis(), nullptr, true); + } wallet.SetBroadcastTransactions(true); // Create widgets for sending coins and listing transactions. diff --git a/src/test/DoS_tests.cpp b/src/test/DoS_tests.cpp index df9370b06dc5..122974a33a64 100644 --- a/src/test/DoS_tests.cpp +++ b/src/test/DoS_tests.cpp @@ -66,11 +66,14 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) dummyNode1.fSuccessfullyConnected = true; // This test requires that we have a chain with non-zero work. + LOCK(cs_main); BOOST_CHECK(chainActive.Tip() != nullptr); BOOST_CHECK(chainActive.Tip()->nChainWork > 0); // Test starts here + LOCK(dummyNode1.cs_sendProcessing); peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in getheaders + LOCK(dummyNode1.cs_vSend); BOOST_CHECK(dummyNode1.vSendMsg.size() > 0); dummyNode1.vSendMsg.clear(); @@ -183,7 +186,11 @@ BOOST_AUTO_TEST_CASE(DoS_banning) peerLogic->InitializeNode(&dummyNode1); dummyNode1.nVersion = 1; dummyNode1.fSuccessfullyConnected = true; - Misbehaving(dummyNode1.GetId(), 100); // Should get banned + { + LOCK(cs_main); + Misbehaving(dummyNode1.GetId(), 100); // Should get banned + } + LOCK(dummyNode1.cs_sendProcessing); peerLogic->SendMessages(&dummyNode1, interruptDummy); BOOST_CHECK(connman->IsBanned(addr1)); BOOST_CHECK(!connman->IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned @@ -194,11 +201,18 @@ BOOST_AUTO_TEST_CASE(DoS_banning) peerLogic->InitializeNode(&dummyNode2); dummyNode2.nVersion = 1; dummyNode2.fSuccessfullyConnected = true; - Misbehaving(dummyNode2.GetId(), 50); + { + LOCK(cs_main); + Misbehaving(dummyNode2.GetId(), 50); + } + LOCK(dummyNode2.cs_sendProcessing); peerLogic->SendMessages(&dummyNode2, interruptDummy); BOOST_CHECK(!connman->IsBanned(addr2)); // 2 not banned yet... BOOST_CHECK(connman->IsBanned(addr1)); // ... but 1 still should be - Misbehaving(dummyNode2.GetId(), 50); + { + LOCK(cs_main); + Misbehaving(dummyNode2.GetId(), 50); + } peerLogic->SendMessages(&dummyNode2, interruptDummy); BOOST_CHECK(connman->IsBanned(addr2)); @@ -219,13 +233,23 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) peerLogic->InitializeNode(&dummyNode1); dummyNode1.nVersion = 1; dummyNode1.fSuccessfullyConnected = true; - Misbehaving(dummyNode1.GetId(), 100); + { + LOCK(cs_main); + Misbehaving(dummyNode1.GetId(), 100); + } + LOCK(dummyNode1.cs_sendProcessing); peerLogic->SendMessages(&dummyNode1, interruptDummy); BOOST_CHECK(!connman->IsBanned(addr1)); - Misbehaving(dummyNode1.GetId(), 10); + { + LOCK(cs_main); + Misbehaving(dummyNode1.GetId(), 10); + } peerLogic->SendMessages(&dummyNode1, interruptDummy); BOOST_CHECK(!connman->IsBanned(addr1)); - Misbehaving(dummyNode1.GetId(), 1); + { + LOCK(cs_main); + Misbehaving(dummyNode1.GetId(), 1); + } peerLogic->SendMessages(&dummyNode1, interruptDummy); BOOST_CHECK(connman->IsBanned(addr1)); gArgs.ForceSetArg("-banscore", std::to_string(DEFAULT_BANSCORE_THRESHOLD)); @@ -249,7 +273,11 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) dummyNode.nVersion = 1; dummyNode.fSuccessfullyConnected = true; - Misbehaving(dummyNode.GetId(), 100); + { + LOCK(cs_main); + Misbehaving(dummyNode.GetId(), 100); + } + LOCK(dummyNode.cs_sendProcessing); peerLogic->SendMessages(&dummyNode, interruptDummy); BOOST_CHECK(connman->IsBanned(addr)); @@ -266,6 +294,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) CTransactionRef RandomOrphan() { std::map::iterator it; + LOCK(cs_main); it = mapOrphanTransactions.lower_bound(InsecureRand256()); if (it == mapOrphanTransactions.end()) it = mapOrphanTransactions.begin(); @@ -335,6 +364,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans) BOOST_CHECK(!AddOrphanTx(MakeTransactionRef(tx), i)); } + LOCK(cs_main); // Test EraseOrphansFor: for (NodeId i = 0; i < 3; i++) { diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index c8e1c7783e70..852beafa3e8e 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -62,6 +62,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) CBlock block(BuildBlockTestCase()); pool.addUnchecked(block.vtx[2]->GetHash(), entry.FromTx(*block.vtx[2])); + LOCK(pool.cs); BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); // Do a simple ShortTxIDs RT @@ -161,6 +162,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) CBlock block(BuildBlockTestCase()); pool.addUnchecked(block.vtx[2]->GetHash(), entry.FromTx(*block.vtx[2])); + LOCK(pool.cs); BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); uint256 txhash; @@ -227,6 +229,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) CBlock block(BuildBlockTestCase()); pool.addUnchecked(block.vtx[1]->GetHash(), entry.FromTx(*block.vtx[1])); + LOCK(pool.cs); BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); uint256 txhash; diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 4618f222bfd9..dcc154d67dac 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -164,6 +164,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) sortedOrder[2] = tx1.GetHash().ToString(); // 10000 sortedOrder[3] = tx4.GetHash().ToString(); // 15000 sortedOrder[4] = tx2.GetHash().ToString(); // 20000 + LOCK(pool.cs); CheckSort(pool, sortedOrder); /* low fee but with high fee child */ @@ -374,6 +375,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) } sortedOrder[4] = tx3.GetHash().ToString(); // 0 + LOCK(pool.cs); CheckSort(pool, sortedOrder); /* low fee parent with high fee child */ diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index ff8406b60ed8..bba02acc5fb7 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -65,6 +65,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, TestChain100Setup) // Test 1: block with both of those transactions should be rejected. block = CreateAndProcessBlock(spends, scriptPubKey); + LOCK(cs_main); BOOST_CHECK(chainActive.Tip()->GetBlockHash() != block.GetHash()); // Test 2: ... and should be rejected if spend1 is in the memory pool @@ -146,7 +147,10 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) { // Test that passing CheckInputs with one set of script flags doesn't imply // that we would pass again with a different set of flags. - InitScriptExecutionCache(); + { + LOCK(cs_main); + InitScriptExecutionCache(); + } CScript p2pk_scriptPubKey = CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG; CScript p2sh_scriptPubKey = GetScriptForDestination(CScriptID(p2pk_scriptPubKey)); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 62349e6131ce..f70566c0c971 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -489,6 +489,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) vpwallets[0] = &wallet; ::importwallet(request); + LOCK(wallet.cs_wallet); BOOST_CHECK_EQUAL(wallet.mapWallet.size(), 3); BOOST_CHECK_EQUAL(coinbaseTxns.size(), 103); for (size_t i = 0; i < coinbaseTxns.size(); ++i) { @@ -534,6 +535,7 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64 SetMockTime(mockTime); CBlockIndex* block = nullptr; if (blockTime > 0) { + LOCK(cs_main); auto inserted = mapBlockIndex.emplace(GetRandHash(), new CBlockIndex); assert(inserted.second); const uint256& hash = inserted.first->first; @@ -547,6 +549,7 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64 wtx.SetMerkleBranch(block, 0); } wallet.AddToWallet(wtx); + LOCK(wallet.cs_wallet); return wallet.mapWallet.at(wtx.GetHash()).nTimeSmart; } @@ -583,6 +586,7 @@ BOOST_AUTO_TEST_CASE(ComputeTimeSmart) BOOST_AUTO_TEST_CASE(LoadReceiveRequests) { CTxDestination dest = CKeyID(); + LOCK(pwalletMain->cs_wallet); pwalletMain->AddDestData(dest, "misc", "val_misc"); pwalletMain->AddDestData(dest, "rr0", "val_rr0"); pwalletMain->AddDestData(dest, "rr1", "val_rr1"); @@ -625,6 +629,7 @@ class ListCoinsTestingSetup : public TestChain100Setup BOOST_CHECK(wallet->CreateTransaction({recipient}, wtx, reservekey, fee, changePos, error, dummy)); CValidationState state; BOOST_CHECK(wallet->CommitTransaction(wtx, reservekey, nullptr, state)); + LOCK(wallet->cs_wallet); auto it = wallet->mapWallet.find(wtx.GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); CreateAndProcessBlock({CMutableTransaction(*it->second.tx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); From be98dc38942f302c6ca5982862bef8a510053c06 Mon Sep 17 00:00:00 2001 From: Pasta Date: Fri, 27 Dec 2019 17:45:49 -0600 Subject: [PATCH 16/27] test_dash continued Signed-off-by: Pasta --- src/test/test_dash.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/test_dash.cpp b/src/test/test_dash.cpp index c494bcb3e3c6..f5b6ea2924ac 100644 --- a/src/test/test_dash.cpp +++ b/src/test/test_dash.cpp @@ -204,7 +204,10 @@ CBlock TestChainSetup::CreateBlock(const std::vector& txns, // IncrementExtraNonce creates a valid coinbase and merkleRoot unsigned int extraNonce = 0; - IncrementExtraNonce(&block, chainActive.Tip(), extraNonce); + { + LOCK(cs_main); + IncrementExtraNonce(&block, chainActive.Tip(), extraNonce); + } while (!CheckProofOfWork(block.GetHash(), block.nBits, chainparams.GetConsensus())) ++block.nNonce; From 767030b9ca37b8c75c9e430aaf20d562f8a25ffe Mon Sep 17 00:00:00 2001 From: Pasta Date: Fri, 27 Dec 2019 17:46:15 -0600 Subject: [PATCH 17/27] remove test_bitcoin.cpp Signed-off-by: Pasta --- src/test/test_bitcoin.cpp | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 src/test/test_bitcoin.cpp diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp deleted file mode 100644 index e69de29bb2d1..000000000000 From a969c8d0be5d67c524a7cb4e02a0cc114369c1b9 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 10 Nov 2017 14:22:36 -0500 Subject: [PATCH 18/27] Merge #11353: Small refactor of CCoinsViewCache::BatchWrite() 5b9748f97 Small refactor of CCoinsViewCache::BatchWrite() (Dan Raviv) Pull request description: `std::unordered_map::erase( const_iterator pos )` returns an iterator to the element following the removed one. Use that to optimize (probably minor-performance-wise, and definitely code-structure-wise) the implementation of `CCoinsViewCache::BatchWrite()`. Tree-SHA512: 00abc838ad91771cfcddd45688841c9414869b75289d09b483a7f0ba835614fe189e9c8aca8a80e3de78ee397ec14083ae52e2e92b7863b3b6eb0d0cb892c9dd --- src/coins.cpp | 94 ++++++++++++++++++++++++++------------------------- 1 file changed, 48 insertions(+), 46 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 4023cdeb95e3..c7365b2774b8 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -146,56 +146,58 @@ void CCoinsViewCache::SetBestBlock(const uint256 &hashBlockIn) { } bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn) { - for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end();) { - if (it->second.flags & CCoinsCacheEntry::DIRTY) { // Ignore non-dirty entries (optimization). - CCoinsMap::iterator itUs = cacheCoins.find(it->first); - if (itUs == cacheCoins.end()) { - // The parent cache does not have an entry, while the child does - // We can ignore it if it's both FRESH and pruned in the child - if (!(it->second.flags & CCoinsCacheEntry::FRESH && it->second.coin.IsSpent())) { - // Otherwise we will need to create it in the parent - // and move the data up and mark it as dirty - CCoinsCacheEntry& entry = cacheCoins[it->first]; - entry.coin = std::move(it->second.coin); - cachedCoinsUsage += entry.coin.DynamicMemoryUsage(); - entry.flags = CCoinsCacheEntry::DIRTY; - // We can mark it FRESH in the parent if it was FRESH in the child - // Otherwise it might have just been flushed from the parent's cache - // and already exist in the grandparent - if (it->second.flags & CCoinsCacheEntry::FRESH) - entry.flags |= CCoinsCacheEntry::FRESH; + for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end(); it = mapCoins.erase(it)) { + // Ignore non-dirty entries (optimization). + if (!(it->second.flags & CCoinsCacheEntry::DIRTY)) { + continue; + } + CCoinsMap::iterator itUs = cacheCoins.find(it->first); + if (itUs == cacheCoins.end()) { + // The parent cache does not have an entry, while the child does + // We can ignore it if it's both FRESH and pruned in the child + if (!(it->second.flags & CCoinsCacheEntry::FRESH && it->second.coin.IsSpent())) { + // Otherwise we will need to create it in the parent + // and move the data up and mark it as dirty + CCoinsCacheEntry& entry = cacheCoins[it->first]; + entry.coin = std::move(it->second.coin); + cachedCoinsUsage += entry.coin.DynamicMemoryUsage(); + entry.flags = CCoinsCacheEntry::DIRTY; + // We can mark it FRESH in the parent if it was FRESH in the child + // Otherwise it might have just been flushed from the parent's cache + // and already exist in the grandparent + if (it->second.flags & CCoinsCacheEntry::FRESH) { + entry.flags |= CCoinsCacheEntry::FRESH; } - } else { - // Assert that the child cache entry was not marked FRESH if the - // parent cache entry has unspent outputs. If this ever happens, - // it means the FRESH flag was misapplied and there is a logic - // error in the calling code. - if ((it->second.flags & CCoinsCacheEntry::FRESH) && !itUs->second.coin.IsSpent()) - throw std::logic_error("FRESH flag misapplied to cache entry for base transaction with spendable outputs"); + } + } else { + // Assert that the child cache entry was not marked FRESH if the + // parent cache entry has unspent outputs. If this ever happens, + // it means the FRESH flag was misapplied and there is a logic + // error in the calling code. + if ((it->second.flags & CCoinsCacheEntry::FRESH) && !itUs->second.coin.IsSpent()) { + throw std::logic_error("FRESH flag misapplied to cache entry for base transaction with spendable outputs"); + } - // Found the entry in the parent cache - if ((itUs->second.flags & CCoinsCacheEntry::FRESH) && it->second.coin.IsSpent()) { - // The grandparent does not have an entry, and the child is - // modified and being pruned. This means we can just delete - // it from the parent. - cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage(); - cacheCoins.erase(itUs); - } else { - // A normal modification. - cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage(); - itUs->second.coin = std::move(it->second.coin); - cachedCoinsUsage += itUs->second.coin.DynamicMemoryUsage(); - itUs->second.flags |= CCoinsCacheEntry::DIRTY; - // NOTE: It is possible the child has a FRESH flag here in - // the event the entry we found in the parent is pruned. But - // we must not copy that FRESH flag to the parent as that - // pruned state likely still needs to be communicated to the - // grandparent. - } + // Found the entry in the parent cache + if ((itUs->second.flags & CCoinsCacheEntry::FRESH) && it->second.coin.IsSpent()) { + // The grandparent does not have an entry, and the child is + // modified and being pruned. This means we can just delete + // it from the parent. + cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage(); + cacheCoins.erase(itUs); + } else { + // A normal modification. + cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage(); + itUs->second.coin = std::move(it->second.coin); + cachedCoinsUsage += itUs->second.coin.DynamicMemoryUsage(); + itUs->second.flags |= CCoinsCacheEntry::DIRTY; + // NOTE: It is possible the child has a FRESH flag here in + // the event the entry we found in the parent is pruned. But + // we must not copy that FRESH flag to the parent as that + // pruned state likely still needs to be communicated to the + // grandparent. } } - CCoinsMap::iterator itOld = it++; - mapCoins.erase(itOld); } hashBlock = hashBlockIn; return true; From 1990f11450f6d993a52d470f9d3c4cbc33d4175f Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 10 Nov 2017 15:33:28 -0500 Subject: [PATCH 19/27] Merge #11269: [Mempool] CTxMemPoolEntry::UpdateAncestorState: modifySiagOps param type 203a4aa31 Fix CTxMemPoolEntry::UpdateAncestorState: modifySigOps param type int -> int64_t (donaloconnor) Pull request description: CTxMemPoolEntry::CTxMemPoolEntry's modifySigOps parameter is int while update_ancestor_state::modifySigOpsCost is int64_t. This issue was raised in #11165. It looks like the function paramaters were not changed in commit 72abd2c This will avoid unexpected truncation of int64_t -> int Tree-SHA512: 314c703f217e104336456859066d18fb0d12c4f9f32835e17490a6f29eb05951184095039e4e57edacef8ad35dd75c6d97d9af656a52209dd0c3779b4ffa0914 --- src/txmempool.cpp | 2 +- src/txmempool.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 1a012fab7b12..bae5292b8ed7 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -319,7 +319,7 @@ void CTxMemPoolEntry::UpdateDescendantState(int64_t modifySize, CAmount modifyFe assert(int64_t(nCountWithDescendants) > 0); } -void CTxMemPoolEntry::UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int modifySigOps) +void CTxMemPoolEntry::UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps) { nSizeWithAncestors += modifySize; assert(int64_t(nSizeWithAncestors) > 0); diff --git a/src/txmempool.h b/src/txmempool.h index d81b3210e9d7..46bbe5a5c915 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -113,7 +113,7 @@ class CTxMemPoolEntry // Adjusts the descendant state. void UpdateDescendantState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount); // Adjusts the ancestor state - void UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int modifySigOps); + void UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps); // Updates the fee delta used for mining priority score, and the // modified fees with descendants. void UpdateFeeDelta(int64_t feeDelta); From c5a7046e93d60bc726c2dae8e91581745ac3b74f Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 10 Nov 2017 12:48:00 -0800 Subject: [PATCH 20/27] Merge #11258: [rpc] Add initialblockdownload to getblockchaininfo 11413646b [trivial] (whitespace only) fix getblockchaininfo alignment (John Newbery) bd9c18171 [rpc] Add initialblockdownload to getblockchaininfo (John Newbery) Pull request description: Exposing whether the node is in IBD would help for testing, and may be useful in general, particularly for developers. First discussed in #10357 here: https://github.com/bitcoin/bitcoin/pull/10357#pullrequestreview-59963870 > ... we could simplify this (and possibly other) tests by just adding a way to know if a node is in IBD. I'd like to do that, but I'm not sure it makes sense to complicate this PR with discussion over how that information should be made available. Eg it's not clear to me that the notion of being in IBD is worth exposing to the casual user, versus a hidden rpc call or something, since the definition has changed over time, and may continue to change in the future. But I still do agree that at least for testing purposes it would be far simpler to expose the field somehow... This PR currently implements the simplest way of doing this: adding an `initialblockdownload` field to `getblockchaininfo`. Other approaches we could take: 1. add a new debug RPC method that exposes `IBD` and potentially other information. 2. add a parameter to `getblockchaininfo`, eg `debug_info`, which would cause it to return debug information including IBD 3. add a query string to the url `?debug=true` which would cause RPCs to return additional debug information. I quite like the idea of (3). Feedback on these and other approaches very much welcomed! @sdaftuar @laanwj Tree-SHA512: a6dedd47f8c9bd38769cc597524466250041136feb33500644b9c48d0ffe4e3eeeb2587b5bbc6420364ebdd2667df807fbb50416f9a7913bbf11a14ea86dc0d4 --- src/rpc/blockchain.cpp | 62 ++++++++++++++++++----------------- test/functional/blockchain.py | 2 ++ 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 81ec5c1b5178..bb6b7aa74083 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1396,45 +1396,46 @@ UniValue getblockchaininfo(const JSONRPCRequest& request) "Returns an object containing various state info regarding blockchain processing.\n" "\nResult:\n" "{\n" - " \"chain\": \"xxxx\", (string) current network name as defined in BIP70 (main, test, regtest)\n" - " \"blocks\": xxxxxx, (numeric) the current number of blocks processed in the server\n" - " \"headers\": xxxxxx, (numeric) the current number of headers we have validated\n" - " \"bestblockhash\": \"...\", (string) the hash of the currently best block\n" - " \"difficulty\": xxxxxx, (numeric) the current difficulty\n" - " \"mediantime\": xxxxxx, (numeric) median time for the current best block\n" + " \"chain\": \"xxxx\", (string) current network name as defined in BIP70 (main, test, regtest)\n" + " \"blocks\": xxxxxx, (numeric) the current number of blocks processed in the server\n" + " \"headers\": xxxxxx, (numeric) the current number of headers we have validated\n" + " \"bestblockhash\": \"...\", (string) the hash of the currently best block\n" + " \"difficulty\": xxxxxx, (numeric) the current difficulty\n" + " \"mediantime\": xxxxxx, (numeric) median time for the current best block\n" " \"verificationprogress\": xxxx, (numeric) estimate of verification progress [0..1]\n" - " \"chainwork\": \"xxxx\" (string) total amount of work in active chain, in hexadecimal\n" - " \"size_on_disk\": xxxxxx, (numeric) the estimated size of the block and undo files on disk\n" - " \"pruned\": xx, (boolean) if the blocks are subject to pruning\n" - " \"pruneheight\": xxxxxx, (numeric) lowest-height complete block stored (only present if pruning is enabled)\n" - " \"automatic_pruning\": xx, (boolean) whether automatic pruning is enabled (only present if pruning is enabled)\n" + " \"initialblockdownload\": xxxx, (bool) (debug information) estimate of whether this node is in Initial Block Download mode.\n" + " \"chainwork\": \"xxxx\" (string) total amount of work in active chain, in hexadecimal\n" + " \"size_on_disk\": xxxxxx, (numeric) the estimated size of the block and undo files on disk\n" + " \"pruned\": xx, (boolean) if the blocks are subject to pruning\n" + " \"pruneheight\": xxxxxx, (numeric) lowest-height complete block stored (only present if pruning is enabled)\n" + " \"automatic_pruning\": xx, (boolean) whether automatic pruning is enabled (only present if pruning is enabled)\n" " \"prune_target_size\": xxxxxx, (numeric) the target size used by pruning (only present if automatic pruning is enabled)\n" - " \"softforks\": [ (array) status of softforks in progress\n" + " \"softforks\": [ (array) status of softforks in progress\n" " {\n" - " \"id\": \"xxxx\", (string) name of softfork\n" - " \"version\": xx, (numeric) block version\n" - " \"reject\": { (object) progress toward rejecting pre-softfork blocks\n" - " \"status\": xx, (boolean) true if threshold reached\n" + " \"id\": \"xxxx\", (string) name of softfork\n" + " \"version\": xx, (numeric) block version\n" + " \"reject\": { (object) progress toward rejecting pre-softfork blocks\n" + " \"status\": xx, (boolean) true if threshold reached\n" " },\n" " }, ...\n" " ],\n" - " \"bip9_softforks\": { (object) status of BIP9 softforks in progress\n" - " \"xxxx\" : { (string) name of the softfork\n" - " \"status\": \"xxxx\", (string) one of \"defined\", \"started\", \"locked_in\", \"active\", \"failed\"\n" - " \"bit\": xx, (numeric) the bit (0-28) in the block version field used to signal this softfork (only for \"started\" status)\n" - " \"startTime\": xx, (numeric) the minimum median time past of a block at which the bit gains its meaning\n" - " \"timeout\": xx, (numeric) the median time past of a block at which the deployment is considered failed if not yet locked in\n" - " \"since\": xx, (numeric) height of the first block to which the status applies\n" - " \"statistics\": { (object) numeric statistics about BIP9 signalling for a softfork (only for \"started\" status)\n" - " \"period\": xx, (numeric) the length in blocks of the BIP9 signalling period \n" - " \"threshold\": xx, (numeric) the number of blocks with the version bit set required to activate the feature \n" - " \"elapsed\": xx, (numeric) the number of blocks elapsed since the beginning of the current period \n" - " \"count\": xx, (numeric) the number of blocks with the version bit set in the current period \n" - " \"possible\": xx (boolean) returns false if there are not enough blocks left in this period to pass activation threshold \n" + " \"bip9_softforks\": { (object) status of BIP9 softforks in progress\n" + " \"xxxx\" : { (string) name of the softfork\n" + " \"status\": \"xxxx\", (string) one of \"defined\", \"started\", \"locked_in\", \"active\", \"failed\"\n" + " \"bit\": xx, (numeric) the bit (0-28) in the block version field used to signal this softfork (only for \"started\" status)\n" + " \"startTime\": xx, (numeric) the minimum median time past of a block at which the bit gains its meaning\n" + " \"timeout\": xx, (numeric) the median time past of a block at which the deployment is considered failed if not yet locked in\n" + " \"since\": xx, (numeric) height of the first block to which the status applies\n" + " \"statistics\": { (object) numeric statistics about BIP9 signalling for a softfork (only for \"started\" status)\n" + " \"period\": xx, (numeric) the length in blocks of the BIP9 signalling period \n" + " \"threshold\": xx, (numeric) the number of blocks with the version bit set required to activate the feature \n" + " \"elapsed\": xx, (numeric) the number of blocks elapsed since the beginning of the current period \n" + " \"count\": xx, (numeric) the number of blocks with the version bit set in the current period \n" + " \"possible\": xx (boolean) returns false if there are not enough blocks left in this period to pass activation threshold \n" " }\n" " }\n" " }\n" - " \"warnings\" : \"...\", (string) any network and blockchain warnings.\n" + " \"warnings\" : \"...\", (string) any network and blockchain warnings.\n" "}\n" "\nExamples:\n" + HelpExampleCli("getblockchaininfo", "") @@ -1451,6 +1452,7 @@ UniValue getblockchaininfo(const JSONRPCRequest& request) obj.push_back(Pair("difficulty", (double)GetDifficulty())); obj.push_back(Pair("mediantime", (int64_t)chainActive.Tip()->GetMedianTimePast())); obj.push_back(Pair("verificationprogress", GuessVerificationProgress(Params().TxData(), chainActive.Tip()))); + obj.push_back(Pair("initialblockdownload", IsInitialBlockDownload())); obj.push_back(Pair("chainwork", chainActive.Tip()->nChainWork.GetHex())); obj.push_back(Pair("size_on_disk", CalculateCurrentUsage())); obj.push_back(Pair("pruned", fPruneMode)); diff --git a/test/functional/blockchain.py b/test/functional/blockchain.py index 0f7ae48c980c..b92877122d70 100755 --- a/test/functional/blockchain.py +++ b/test/functional/blockchain.py @@ -5,6 +5,7 @@ """Test RPCs related to blockchainstate. Test the following RPCs: + - getblockchaininfo - gettxoutsetinfo - getdifficulty - getbestblockhash @@ -67,6 +68,7 @@ def _test_getblockchaininfo(self): 'chainwork', 'difficulty', 'headers', + 'initialblockdownload', 'mediantime', 'pruned', 'size_on_disk', From cc5b2f163c563b942b0bb928ca92b59caf6f3e2f Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 11 Nov 2017 12:35:32 -0500 Subject: [PATCH 21/27] Merge #11055: [wallet] [rpc] getreceivedbyaddress should return error if called with address not owned by the wallet 5e0ba8f8c [wallet] getreceivedbyaddress should return error if address is not mine (John Newbery) ea0cd24f7 [tests] Tidy up receivedby.py (John Newbery) Pull request description: Two commits: - First commit tidies up the `receivedby.py` test (and speeds it up by factor of two) - Second commit changes getreceivedbyaddress to return error if the address is not found in wallet, and adds test to `receivedby.py` Tree-SHA512: e41342dcbd037a6b440cbe4ecd3b8ed589e18e477333f0d866f3564e948e0f5231e497d5ffb66da4e6680eb772d9f0cf839125098bb68b92d04a5ee35c6c0a81 --- src/wallet/rpcwallet.cpp | 2 +- test/functional/receivedby.py | 118 ++++++++++++++-------------------- 2 files changed, 50 insertions(+), 70 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 291b47da422f..96ebf0bf677f 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -708,7 +708,7 @@ UniValue getreceivedbyaddress(const JSONRPCRequest& request) } CScript scriptPubKey = GetScriptForDestination(dest); if (!IsMine(*pwallet, scriptPubKey)) { - return ValueFromAmount(0); + throw JSONRPCError(RPC_WALLET_ERROR, "Address not found in wallet"); } // Minimum confirmations diff --git a/test/functional/receivedby.py b/test/functional/receivedby.py index c06477c745cc..08cae0d7f8f8 100755 --- a/test/functional/receivedby.py +++ b/test/functional/receivedby.py @@ -3,97 +3,83 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the listreceivedbyaddress RPC.""" +from decimal import Decimal from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import * - -def get_sub_array_from_array(object_array, to_match): - ''' - Finds and returns a sub array from an array of arrays. - to_match should be a unique idetifier of a sub array - ''' - for item in object_array: - all_match = True - for key,value in to_match.items(): - if item[key] != value: - all_match = False - if not all_match: - continue - return item - return [] +from test_framework.util import (assert_array_result, + assert_equal, + assert_raises_rpc_error, + ) class ReceivedByTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 - self.set_cache_mocktime() def run_test(self): - ''' - listreceivedbyaddress Test - ''' + # Generate block to get out of IBD + self.nodes[0].generate(1) + + self.log.info("listreceivedbyaddress Test") + # Send from node 0 to 1 addr = self.nodes[1].getnewaddress() txid = self.nodes[0].sendtoaddress(addr, 0.1) self.sync_all() - #Check not listed in listreceivedbyaddress because has 0 confirmations + # Check not listed in listreceivedbyaddress because has 0 confirmations assert_array_result(self.nodes[1].listreceivedbyaddress(), - {"address":addr}, - { }, - True) - #Bury Tx under 10 block so it will be returned by listreceivedbyaddress + {"address": addr}, + {}, + True) + # Bury Tx under 10 block so it will be returned by listreceivedbyaddress self.nodes[1].generate(10) self.sync_all() assert_array_result(self.nodes[1].listreceivedbyaddress(), - {"address":addr}, - {"address":addr, "account":"", "amount":Decimal("0.1"), "confirmations":10, "txids":[txid,]}) - #With min confidence < 10 + {"address": addr}, + {"address": addr, "account": "", "amount": Decimal("0.1"), "confirmations": 10, "txids": [txid, ]}) + # With min confidence < 10 assert_array_result(self.nodes[1].listreceivedbyaddress(5), - {"address":addr}, - {"address":addr, "account":"", "amount":Decimal("0.1"), "confirmations":10, "txids":[txid,]}) - #With min confidence > 10, should not find Tx - assert_array_result(self.nodes[1].listreceivedbyaddress(11),{"address":addr},{ },True) + {"address": addr}, + {"address": addr, "account": "", "amount": Decimal("0.1"), "confirmations": 10, "txids": [txid, ]}) + # With min confidence > 10, should not find Tx + assert_array_result(self.nodes[1].listreceivedbyaddress(11), {"address": addr}, {}, True) - #Empty Tx + # Empty Tx addr = self.nodes[1].getnewaddress() assert_array_result(self.nodes[1].listreceivedbyaddress(0, False, True), {"address":addr}, {"address":addr, "account":"", "amount":0, "confirmations":0, "txids":[]}) - ''' - getreceivedbyaddress Test - ''' + self.log.info("getreceivedbyaddress Test") + # Send from node 0 to 1 addr = self.nodes[1].getnewaddress() txid = self.nodes[0].sendtoaddress(addr, 0.1) self.sync_all() - #Check balance is 0 because of 0 confirmations + # Check balance is 0 because of 0 confirmations balance = self.nodes[1].getreceivedbyaddress(addr) - if balance != Decimal("0.0"): - raise AssertionError("Wrong balance returned by getreceivedbyaddress, %0.2f"%(balance)) + assert_equal(balance, Decimal("0.0")) - #Check balance is 0.1 - balance = self.nodes[1].getreceivedbyaddress(addr,0) - if balance != Decimal("0.1"): - raise AssertionError("Wrong balance returned by getreceivedbyaddress, %0.2f"%(balance)) + # Check balance is 0.1 + balance = self.nodes[1].getreceivedbyaddress(addr, 0) + assert_equal(balance, Decimal("0.1")) - #Bury Tx under 10 block so it will be returned by the default getreceivedbyaddress + # Bury Tx under 10 block so it will be returned by the default getreceivedbyaddress self.nodes[1].generate(10) self.sync_all() balance = self.nodes[1].getreceivedbyaddress(addr) - if balance != Decimal("0.1"): - raise AssertionError("Wrong balance returned by getreceivedbyaddress, %0.2f"%(balance)) + assert_equal(balance, Decimal("0.1")) + + # Trying to getreceivedby for an address the wallet doesn't own should return an error + assert_raises_rpc_error(-4, "Address not found in wallet", self.nodes[0].getreceivedbyaddress, addr) + + self.log.info("listreceivedbyaccount + getreceivedbyaccount Test") - ''' - listreceivedbyaccount + getreceivedbyaccount Test - ''' - #set pre-state + # set pre-state addrArr = self.nodes[1].getnewaddress() account = self.nodes[1].getaccount(addrArr) - received_by_account_json = get_sub_array_from_array(self.nodes[1].listreceivedbyaccount(),{"account":account}) - if len(received_by_account_json) == 0: - raise AssertionError("No accounts found in node") + received_by_account_json = [r for r in self.nodes[1].listreceivedbyaccount() if r["account"] == account][0] balance_by_account = self.nodes[1].getreceivedbyaccount(account) txid = self.nodes[0].sendtoaddress(addr, 0.1) @@ -101,40 +87,34 @@ def run_test(self): # listreceivedbyaccount should return received_by_account_json because of 0 confirmations assert_array_result(self.nodes[1].listreceivedbyaccount(), - {"account":account}, - received_by_account_json) + {"account": account}, + received_by_account_json) # getreceivedbyaddress should return same balance because of 0 confirmations balance = self.nodes[1].getreceivedbyaccount(account) - if balance != balance_by_account: - raise AssertionError("Wrong balance returned by getreceivedbyaccount, %0.2f"%(balance)) + assert_equal(balance, balance_by_account) self.nodes[1].generate(10) self.sync_all() # listreceivedbyaccount should return updated account balance assert_array_result(self.nodes[1].listreceivedbyaccount(), - {"account":account}, - {"account":received_by_account_json["account"], "amount":(received_by_account_json["amount"] + Decimal("0.1"))}) + {"account": account}, + {"account": received_by_account_json["account"], "amount": (received_by_account_json["amount"] + Decimal("0.1"))}) # getreceivedbyaddress should return updates balance balance = self.nodes[1].getreceivedbyaccount(account) - if balance != balance_by_account + Decimal("0.1"): - raise AssertionError("Wrong balance returned by getreceivedbyaccount, %0.2f"%(balance)) + assert_equal(balance, balance_by_account + Decimal("0.1")) - #Create a new account named "mynewaccount" that has a 0 balance + # Create a new account named "mynewaccount" that has a 0 balance self.nodes[1].getaccountaddress("mynewaccount") - received_by_account_json = get_sub_array_from_array(self.nodes[1].listreceivedbyaccount(0, False, True),{"account":"mynewaccount"}) - if len(received_by_account_json) == 0: - raise AssertionError("No accounts found in node") + received_by_account_json = [r for r inself.nodes[1].listreceivedbyaccount(0, False, True)if r["account"] == "mynewaccount"][0] # Test includeempty of listreceivedbyaccount - if received_by_account_json["amount"] != Decimal("0.0"): - raise AssertionError("Wrong balance returned by listreceivedbyaccount, %0.2f"%(received_by_account_json["amount"])) + assert_equal(received_by_account_json["amount"], Decimal("0.0")) # Test getreceivedbyaccount for 0 amount accounts balance = self.nodes[1].getreceivedbyaccount("mynewaccount") - if balance != Decimal("0.0"): - raise AssertionError("Wrong balance returned by getreceivedbyaccount, %0.2f"%(balance)) + assert_equal(balance, Decimal("0.0")) if __name__ == '__main__': ReceivedByTest().main() From abc0fb08bec292c123bb10250422bd12cd48f5dd Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 11 Nov 2017 13:29:46 -0500 Subject: [PATCH 22/27] Merge #3716: GUI: Receive: Remove option to reuse a previous address 927f4ff5a GUI: Receive: Remove option to reuse a previous address (Luke Dashjr) Pull request description: This was justified by the need to "resent" an invoice, but now that we have the request history, that need should be gone. Tree-SHA512: 4ade4eb84a21bbbd8dcc3a2c9580d416e113284b5bdf350c22051c233101fe0ee31659c54a7a46e7136f9c999acb61efbbb3f97aeb2fa7b2b1e1daec02ca0837 --- src/qt/forms/receivecoinsdialog.ui | 22 ++-------------------- src/qt/receivecoinsdialog.cpp | 22 ++-------------------- 2 files changed, 4 insertions(+), 40 deletions(-) diff --git a/src/qt/forms/receivecoinsdialog.ui b/src/qt/forms/receivecoinsdialog.ui index cc19bb21259c..ea953290411f 100644 --- a/src/qt/forms/receivecoinsdialog.ui +++ b/src/qt/forms/receivecoinsdialog.ui @@ -28,23 +28,6 @@ - - - - Reuse one of the previously used receiving addresses.<br>Reusing addresses has security and privacy issues.<br>Do not use this unless re-generating a payment request made before. - - - R&euse an existing receiving address (not recommended) - - - - - - - - - - @@ -127,7 +110,7 @@ - + @@ -176,7 +159,7 @@ - + @@ -308,7 +291,6 @@ reqLabel reqAmount reqMessage - reuseAddress receiveButton clearButton recentRequestsView diff --git a/src/qt/receivecoinsdialog.cpp b/src/qt/receivecoinsdialog.cpp index a98ef412e883..0fb4e905962c 100644 --- a/src/qt/receivecoinsdialog.cpp +++ b/src/qt/receivecoinsdialog.cpp @@ -106,7 +106,6 @@ void ReceiveCoinsDialog::clear() ui->reqAmount->clear(); ui->reqLabel->setText(""); ui->reqMessage->setText(""); - ui->reuseAddress->setChecked(false); updateDisplayUnit(); } @@ -135,25 +134,8 @@ void ReceiveCoinsDialog::on_receiveButton_clicked() QString address; QString label = ui->reqLabel->text(); - if(ui->reuseAddress->isChecked()) - { - /* Choose existing receiving address */ - AddressBookPage dlg(platformStyle, AddressBookPage::ForSelection, AddressBookPage::ReceivingTab, this); - dlg.setModel(model->getAddressTableModel()); - if(dlg.exec()) - { - address = dlg.getReturnValue(); - if(label.isEmpty()) /* If no label provided, use the previously used label */ - { - label = model->getAddressTableModel()->labelForAddress(address); - } - } else { - return; - } - } else { - /* Generate new receiving address */ - address = model->getAddressTableModel()->addRow(AddressTableModel::Receive, label, ""); - } + /* Generate new receiving address */ + address = model->getAddressTableModel()->addRow(AddressTableModel::Receive, label, ""); SendCoinsRecipient info(address, label, ui->reqAmount->value(), ui->reqMessage->text()); ReceiveRequestDialog *dialog = new ReceiveRequestDialog(this); From aeaf3c5ef9f2af515e620ac93f5cd4344be44c5c Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 11 Nov 2017 18:07:23 -0500 Subject: [PATCH 23/27] Merge #10749: Use compile-time constants instead of unnamed enumerations (remove "enum hack") 1e65f0f33 Use compile-time constants instead of unnamed enumerations (remove "enum hack") (practicalswift) Pull request description: Use compile-time constants instead of unnamed enumerations (remove "enum hack"). Tree-SHA512: 1b6ebb2755398c5ebab6cce125b1dfc39cbd1504d98d55136b32703fe935c4070360ab3b2f52b1da48ba9f3b01082d204f3d87c92ccb5c8c333731f7f972e128 --- src/arith_uint256.h | 2 +- src/chain.h | 2 +- src/consensus/consensus.h | 11 ++++------- src/protocol.h | 17 +++++++---------- src/uint256.h | 2 +- 5 files changed, 14 insertions(+), 20 deletions(-) diff --git a/src/arith_uint256.h b/src/arith_uint256.h index 0ee4cf9622d8..89a5272f1f9f 100644 --- a/src/arith_uint256.h +++ b/src/arith_uint256.h @@ -25,7 +25,7 @@ template class base_uint { protected: - enum { WIDTH=BITS/32 }; + static constexpr int WIDTH = BITS / 32; uint32_t pn[WIDTH]; public: diff --git a/src/chain.h b/src/chain.h index f29580e7fbdc..4e3bbbad2d7b 100644 --- a/src/chain.h +++ b/src/chain.h @@ -304,7 +304,7 @@ class CBlockIndex return (int64_t)nTimeMax; } - enum { nMedianTimeSpan=11 }; + static constexpr int nMedianTimeSpan = 11; int64_t GetMedianTimePast() const { diff --git a/src/consensus/consensus.h b/src/consensus/consensus.h index 0efe580c1e08..cd631168f3cb 100644 --- a/src/consensus/consensus.h +++ b/src/consensus/consensus.h @@ -24,12 +24,9 @@ static const unsigned int MAX_TX_EXTRA_PAYLOAD = 10000; static const int COINBASE_MATURITY = 100; /** Flags for nSequence and nLockTime locks */ -enum { - /* Interpret sequence numbers as relative lock-time constraints. */ - LOCKTIME_VERIFY_SEQUENCE = (1 << 0), - - /* Use GetMedianTimePast() instead of nTime for end point timestamp. */ - LOCKTIME_MEDIAN_TIME_PAST = (1 << 1), -}; +/** Interpret sequence numbers as relative lock-time constraints. */ +static constexpr unsigned int LOCKTIME_VERIFY_SEQUENCE = (1 << 0); +/** Use GetMedianTimePast() instead of nTime for end point timestamp. */ +static constexpr unsigned int LOCKTIME_MEDIAN_TIME_PAST = (1 << 1); #endif // BITCOIN_CONSENSUS_CONSENSUS_H diff --git a/src/protocol.h b/src/protocol.h index 4bf0cdf00a69..25a3cc708fda 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -27,16 +27,13 @@ class CMessageHeader { public: - enum { - MESSAGE_START_SIZE = 4, - COMMAND_SIZE = 12, - MESSAGE_SIZE_SIZE = 4, - CHECKSUM_SIZE = 4, - - MESSAGE_SIZE_OFFSET = MESSAGE_START_SIZE + COMMAND_SIZE, - CHECKSUM_OFFSET = MESSAGE_SIZE_OFFSET + MESSAGE_SIZE_SIZE, - HEADER_SIZE = MESSAGE_START_SIZE + COMMAND_SIZE + MESSAGE_SIZE_SIZE + CHECKSUM_SIZE - }; + static constexpr size_t MESSAGE_START_SIZE = 4; + static constexpr size_t COMMAND_SIZE = 12; + static constexpr size_t MESSAGE_SIZE_SIZE = 4; + static constexpr size_t CHECKSUM_SIZE = 4; + static constexpr size_t MESSAGE_SIZE_OFFSET = MESSAGE_START_SIZE + COMMAND_SIZE; + static constexpr size_t CHECKSUM_OFFSET = MESSAGE_SIZE_OFFSET + MESSAGE_SIZE_SIZE; + static constexpr size_t HEADER_SIZE = MESSAGE_START_SIZE + COMMAND_SIZE + MESSAGE_SIZE_SIZE + CHECKSUM_SIZE; typedef unsigned char MessageStartChars[MESSAGE_START_SIZE]; explicit CMessageHeader(const MessageStartChars& pchMessageStartIn); diff --git a/src/uint256.h b/src/uint256.h index a1ad3db12900..9d74c292a31c 100644 --- a/src/uint256.h +++ b/src/uint256.h @@ -20,7 +20,7 @@ template class base_blob { protected: - enum { WIDTH=BITS/8 }; + static constexpr int WIDTH = BITS / 8; uint8_t data[WIDTH]; public: base_blob() From 62e947dfb946abeacca1b184f029deb219b158f6 Mon Sep 17 00:00:00 2001 From: Pasta Date: Wed, 22 Jan 2020 12:16:35 -0600 Subject: [PATCH 24/27] fix receivedby.py Signed-off-by: Pasta --- test/functional/receivedby.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/receivedby.py b/test/functional/receivedby.py index 08cae0d7f8f8..cf66aa592336 100755 --- a/test/functional/receivedby.py +++ b/test/functional/receivedby.py @@ -107,7 +107,7 @@ def run_test(self): # Create a new account named "mynewaccount" that has a 0 balance self.nodes[1].getaccountaddress("mynewaccount") - received_by_account_json = [r for r inself.nodes[1].listreceivedbyaccount(0, False, True)if r["account"] == "mynewaccount"][0] + received_by_account_json = [r for r in self.nodes[1].listreceivedbyaccount(0, True) if r["account"] == "mynewaccount"][0] # Test includeempty of listreceivedbyaccount assert_equal(received_by_account_json["amount"], Decimal("0.0")) From e21129552d873a5977c0008d0400967867639430 Mon Sep 17 00:00:00 2001 From: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com> Date: Wed, 22 Jan 2020 12:32:05 -0600 Subject: [PATCH 25/27] fix test/functional/receivedby.py Co-Authored-By: UdjinM6 --- test/functional/receivedby.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/receivedby.py b/test/functional/receivedby.py index cf66aa592336..95e664366933 100755 --- a/test/functional/receivedby.py +++ b/test/functional/receivedby.py @@ -107,7 +107,7 @@ def run_test(self): # Create a new account named "mynewaccount" that has a 0 balance self.nodes[1].getaccountaddress("mynewaccount") - received_by_account_json = [r for r in self.nodes[1].listreceivedbyaccount(0, True) if r["account"] == "mynewaccount"][0] + received_by_account_json = [r for r in self.nodes[1].listreceivedbyaccount(0, False, True) if r["account"] == "mynewaccount"][0] # Test includeempty of listreceivedbyaccount assert_equal(received_by_account_json["amount"], Decimal("0.0")) From 7bbe84b3cb8987d9ce611e031b1b806cf2932798 Mon Sep 17 00:00:00 2001 From: Pasta Date: Wed, 22 Jan 2020 16:12:42 -0600 Subject: [PATCH 26/27] Revert "remove explicit on FreespaceChecker" This reverts commit aef1eb00a4626a128aa51ae96a7aa1b90538209c. --- src/qt/intro.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qt/intro.cpp b/src/qt/intro.cpp index d64a14068de7..4812e5c5abfe 100644 --- a/src/qt/intro.cpp +++ b/src/qt/intro.cpp @@ -44,7 +44,7 @@ class FreespaceChecker : public QObject Q_OBJECT public: - FreespaceChecker(Intro *intro); + explicit FreespaceChecker(Intro *intro); enum Status { ST_OK, From 0ca31d12ed00d6abfa88a785755021fa6f27d4e5 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 22 Jan 2020 23:42:33 +0300 Subject: [PATCH 27/27] Fix tests --- src/test/txvalidationcache_tests.cpp | 21 ++++++++++++++++----- src/wallet/test/wallet_tests.cpp | 8 +++++++- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index bba02acc5fb7..a8ff74219e83 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -65,19 +65,27 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, TestChain100Setup) // Test 1: block with both of those transactions should be rejected. block = CreateAndProcessBlock(spends, scriptPubKey); - LOCK(cs_main); - BOOST_CHECK(chainActive.Tip()->GetBlockHash() != block.GetHash()); + { + LOCK(cs_main); + BOOST_CHECK(chainActive.Tip()->GetBlockHash() != block.GetHash()); + } // Test 2: ... and should be rejected if spend1 is in the memory pool BOOST_CHECK(ToMemPool(spends[0])); block = CreateAndProcessBlock(spends, scriptPubKey); - BOOST_CHECK(chainActive.Tip()->GetBlockHash() != block.GetHash()); + { + LOCK(cs_main); + BOOST_CHECK(chainActive.Tip()->GetBlockHash() != block.GetHash()); + } mempool.clear(); // Test 3: ... and should be rejected if spend2 is in the memory pool BOOST_CHECK(ToMemPool(spends[1])); block = CreateAndProcessBlock(spends, scriptPubKey); - BOOST_CHECK(chainActive.Tip()->GetBlockHash() != block.GetHash()); + { + LOCK(cs_main); + BOOST_CHECK(chainActive.Tip()->GetBlockHash() != block.GetHash()); + } mempool.clear(); // Final sanity test: first spend in mempool, second in block, that's OK: @@ -85,7 +93,10 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, TestChain100Setup) oneSpend.push_back(spends[0]); BOOST_CHECK(ToMemPool(spends[1])); block = CreateAndProcessBlock(oneSpend, scriptPubKey); - BOOST_CHECK(chainActive.Tip()->GetBlockHash() == block.GetHash()); + { + LOCK(cs_main); + BOOST_CHECK(chainActive.Tip()->GetBlockHash() == block.GetHash()); + } // spends[1] should have been removed from the mempool when the // block with spends[0] is accepted: BOOST_CHECK_EQUAL(mempool.size(), 0); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index f70566c0c971..b8d4f0c7d606 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -629,10 +629,16 @@ class ListCoinsTestingSetup : public TestChain100Setup BOOST_CHECK(wallet->CreateTransaction({recipient}, wtx, reservekey, fee, changePos, error, dummy)); CValidationState state; BOOST_CHECK(wallet->CommitTransaction(wtx, reservekey, nullptr, state)); + CMutableTransaction blocktx; + { + LOCK(wallet->cs_wallet); + blocktx = CMutableTransaction(*wallet->mapWallet.at(wtx.tx->GetHash()).tx); + } + CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); + LOCK(cs_main); LOCK(wallet->cs_wallet); auto it = wallet->mapWallet.find(wtx.GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); - CreateAndProcessBlock({CMutableTransaction(*it->second.tx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); it->second.SetMerkleBranch(chainActive.Tip(), 1); return it->second; }