aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rwxr-xr-xcontrib/devtools/github-merge.py3
-rw-r--r--contrib/seeds/README.md2
-rw-r--r--doc/developer-notes.md45
-rw-r--r--src/bitcoind.cpp17
-rw-r--r--src/core_memusage.h5
-rw-r--r--src/init.cpp9
-rw-r--r--src/net.cpp8
-rw-r--r--src/net.h4
-rw-r--r--src/qt/test/wallettests.cpp13
-rw-r--r--src/txmempool.cpp2
-rw-r--r--src/txmempool.h99
-rw-r--r--src/util.h3
-rw-r--r--src/validation.cpp154
-rw-r--r--src/validation.h4
-rw-r--r--src/wallet/rpcwallet.cpp2
-rwxr-xr-xtest/functional/bumpfee.py1
-rwxr-xr-xtest/functional/pruning.py15
17 files changed, 300 insertions, 86 deletions
diff --git a/contrib/devtools/github-merge.py b/contrib/devtools/github-merge.py
index 8fce648fc2..e9816f7d19 100755
--- a/contrib/devtools/github-merge.py
+++ b/contrib/devtools/github-merge.py
@@ -301,8 +301,7 @@ def main():
subprocess.check_call([GIT,'commit','-q','--gpg-sign','--amend','--no-edit'])
break
except subprocess.CalledProcessError as e:
- print("Error signing, exiting.",file=stderr)
- exit(1)
+ print("Error while signing, asking again.",file=stderr)
elif reply == 'x':
print("Not signing off on merge, exiting.",file=stderr)
exit(1)
diff --git a/contrib/seeds/README.md b/contrib/seeds/README.md
index afe902fd7f..139c03181f 100644
--- a/contrib/seeds/README.md
+++ b/contrib/seeds/README.md
@@ -8,7 +8,7 @@ and remove old versions as necessary.
The seeds compiled into the release are created from sipa's DNS seed data, like this:
- curl -s http://bitcoin.sipa.be/seeds.txt > seeds_main.txt
+ curl -s http://bitcoin.sipa.be/seeds.txt.gz | gzip -dc > seeds_main.txt
python3 makeseeds.py < seeds_main.txt > nodes_main.txt
python3 generate-seeds.py . > ../../src/chainparamsseeds.h
diff --git a/doc/developer-notes.md b/doc/developer-notes.md
index cf860a1bf2..ec6abda91e 100644
--- a/doc/developer-notes.md
+++ b/doc/developer-notes.md
@@ -3,41 +3,64 @@ Developer Notes
Various coding styles have been used during the history of the codebase,
and the result is not very consistent. However, we're now trying to converge to
-a single style, so please use it in new code. Old code will be converted
-gradually and you are encouraged to use the provided
-[clang-format-diff script](/contrib/devtools/README.md#clang-format-diffpy)
-to clean up the patch automatically before submitting a pull request.
+a single style, which is specified below. When writing patches, favor the new
+style over attempting to mimick the surrounding style, except for move-only
+commits.
+
+Do not submit patches solely to modify the style of existing code.
-- Basic rules specified in [src/.clang-format](/src/.clang-format).
+- **Indentation and whitespace rules** as specified in
+[src/.clang-format](/src/.clang-format). You can use the provided
+[clang-format-diff script](/contrib/devtools/README.md#clang-format-diffpy)
+tool to clean up patches automatically before submission.
- Braces on new lines for namespaces, classes, functions, methods.
- Braces on the same line for everything else.
- 4 space indentation (no tabs) for every block except namespaces.
- No indentation for `public`/`protected`/`private` or for `namespace`.
- No extra spaces inside parenthesis; don't do ( this )
- No space after function names; one space after `if`, `for` and `while`.
- - If an `if` only has a single-statement then-clause, it can appear
- on the same line as the if, without braces. In every other case,
- braces are required, and the then and else clauses must appear
+ - If an `if` only has a single-statement `then`-clause, it can appear
+ on the same line as the `if`, without braces. In every other case,
+ braces are required, and the `then` and `else` clauses must appear
correctly indented on a new line.
+
+- **Symbol naming conventions**. These are preferred in new code, but are not
+required when doing so would need changes to significant pieces of existing
+code.
+ - Variable and namespace names are all lowercase, and may use `_` to
+ separate words.
+ - Class member variables have a `m_` prefix.
+ - Global variables have a `g_` prefix.
+ - Constant names are all uppercase, and use `_` to separate words.
+ - Class names, function names and method names are CamelCase. Do not prefix
+ class names with `C`.
+
+- **Miscellaneous**
- `++i` is preferred over `i++`.
Block style example:
```c++
+int g_count = 0;
+
namespace foo
{
class Class
{
+ std::string m_name;
+
+public:
bool Function(const std::string& s, int n)
{
// Comment summarising what this section of code does
for (int i = 0; i < n; ++i) {
+ int total_sum = 0;
// When something fails, return early
if (!Something()) return false;
...
- if (SomethingElse()) {
- DoMore();
+ if (SomethingElse(i)) {
+ total_sum += ComputeSomething(g_count);
} else {
- DoLess();
+ DoSomething(m_name, total_sum);
}
}
diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp
index 31680a8ec7..5922e45801 100644
--- a/src/bitcoind.cpp
+++ b/src/bitcoind.cpp
@@ -117,17 +117,14 @@ bool AppInit(int argc, char* argv[])
return false;
}
- // Command-line RPC
- bool fCommandLine = false;
- for (int i = 1; i < argc; i++)
- if (!IsSwitchChar(argv[i][0]) && !boost::algorithm::istarts_with(argv[i], "bitcoin:"))
- fCommandLine = true;
-
- if (fCommandLine)
- {
- fprintf(stderr, "Error: There is no RPC client functionality in bitcoind anymore. Use the bitcoin-cli utility instead.\n");
- exit(EXIT_FAILURE);
+ // Error out when loose non-argument tokens are encountered on command line
+ for (int i = 1; i < argc; i++) {
+ if (!IsSwitchChar(argv[i][0])) {
+ fprintf(stderr, "Error: Command line contains unexpected token '%s', see bitcoind -h for a list of options.\n", argv[i]);
+ exit(EXIT_FAILURE);
+ }
}
+
// -server defaults to true for bitcoind but not for the GUI so do this here
SoftSetBoolArg("-server", true);
// Set this early so that parameter interactions go to console
diff --git a/src/core_memusage.h b/src/core_memusage.h
index 5e10182075..e4ccd54c42 100644
--- a/src/core_memusage.h
+++ b/src/core_memusage.h
@@ -63,4 +63,9 @@ static inline size_t RecursiveDynamicUsage(const CBlockLocator& locator) {
return memusage::DynamicUsage(locator.vHave);
}
+template<typename X>
+static inline size_t RecursiveDynamicUsage(const std::shared_ptr<X>& p) {
+ return p ? memusage::DynamicUsage(p) + RecursiveDynamicUsage(*p) : 0;
+}
+
#endif // BITCOIN_CORE_MEMUSAGE_H
diff --git a/src/init.cpp b/src/init.cpp
index 3bbdb16c3b..f343f1d965 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1386,11 +1386,6 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
}
}
- if (gArgs.IsArgSet("-seednode")) {
- BOOST_FOREACH(const std::string& strDest, gArgs.GetArgs("-seednode"))
- connman.AddOneShot(strDest);
- }
-
#if ENABLE_ZMQ
pzmqNotificationInterface = CZMQNotificationInterface::Create();
@@ -1659,6 +1654,10 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
connOptions.nMaxOutboundTimeframe = nMaxOutboundTimeframe;
connOptions.nMaxOutboundLimit = nMaxOutboundLimit;
+ if (gArgs.IsArgSet("-seednode")) {
+ connOptions.vSeedNodes = gArgs.GetArgs("-seednode");
+ }
+
if (!connman.Start(scheduler, strNodeError, connOptions))
return InitError(strNodeError);
diff --git a/src/net.cpp b/src/net.cpp
index 198d8f5fff..bac9b5732a 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1743,9 +1743,9 @@ void CConnman::ThreadOpenConnections()
// * Increase the number of connectable addresses in the tried table.
//
// Method:
- // * Choose a random address from new and attempt to connect to it if we can connect
+ // * Choose a random address from new and attempt to connect to it if we can connect
// successfully it is added to tried.
- // * Start attempting feeler connections only after node finishes making outbound
+ // * Start attempting feeler connections only after node finishes making outbound
// connections.
// * Only make a feeler connection once every few minutes.
//
@@ -2212,6 +2212,10 @@ bool CConnman::Start(CScheduler& scheduler, std::string& strNodeError, Options c
SetBestHeight(connOptions.nBestHeight);
+ for (const auto& strDest : connOptions.vSeedNodes) {
+ AddOneShot(strDest);
+ }
+
clientInterface = connOptions.uiInterface;
if (clientInterface) {
clientInterface->InitMessage(_("Loading P2P addresses..."));
diff --git a/src/net.h b/src/net.h
index bea04bc580..7f9ec1a57b 100644
--- a/src/net.h
+++ b/src/net.h
@@ -144,6 +144,7 @@ public:
unsigned int nReceiveFloodSize = 0;
uint64_t nMaxOutboundTimeframe = 0;
uint64_t nMaxOutboundLimit = 0;
+ std::vector<std::string> vSeedNodes;
};
CConnman(uint64_t seed0, uint64_t seed1);
~CConnman();
@@ -233,8 +234,6 @@ public:
void GetBanned(banmap_t &banmap);
void SetBanned(const banmap_t &banmap);
- void AddOneShot(const std::string& strDest);
-
bool AddNode(const std::string& node);
bool RemoveAddedNode(const std::string& node);
std::vector<AddedNodeInfo> GetAddedNodeInfo();
@@ -292,6 +291,7 @@ private:
};
void ThreadOpenAddedConnections();
+ void AddOneShot(const std::string& strDest);
void ProcessOneShot();
void ThreadOpenConnections();
void ThreadMessageHandler();
diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp
index 32362ccdfc..0e12a9d53e 100644
--- a/src/qt/test/wallettests.cpp
+++ b/src/qt/test/wallettests.cpp
@@ -90,6 +90,17 @@ QModelIndex FindTx(const QAbstractItemModel& model, const uint256& txid)
return {};
}
+//! Request context menu (call method that is public in qt5, but protected in qt4).
+void RequestContextMenu(QWidget* widget)
+{
+ class Qt4Hack : public QWidget
+ {
+ public:
+ using QWidget::customContextMenuRequested;
+ };
+ static_cast<Qt4Hack*>(widget)->customContextMenuRequested({});
+}
+
//! Invoke bumpfee on txid and check results.
void BumpFee(TransactionView& view, const uint256& txid, bool expectDisabled, std::string expectError, bool cancel)
{
@@ -102,7 +113,7 @@ void BumpFee(TransactionView& view, const uint256& txid, bool expectDisabled, st
QAction* action = view.findChild<QAction*>("bumpFeeAction");
table->selectionModel()->select(index, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows);
action->setEnabled(expectDisabled);
- table->customContextMenuRequested({});
+ RequestContextMenu(table);
QCOMPARE(action->isEnabled(), !expectDisabled);
action->setEnabled(true);
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 33df0536d0..852984426f 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -24,7 +24,7 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFe
spendsCoinbase(_spendsCoinbase), sigOpCost(_sigOpsCost), lockPoints(lp)
{
nTxWeight = GetTransactionWeight(*tx);
- nUsageSize = RecursiveDynamicUsage(*tx) + memusage::DynamicUsage(tx);
+ nUsageSize = RecursiveDynamicUsage(tx);
nCountWithDescendants = 1;
nSizeWithDescendants = GetTxSize();
diff --git a/src/txmempool.h b/src/txmempool.h
index a91eb5be54..671a8b596c 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -24,6 +24,7 @@
#include "boost/multi_index_container.hpp"
#include "boost/multi_index/ordered_index.hpp"
#include "boost/multi_index/hashed_index.hpp"
+#include <boost/multi_index/sequenced_index.hpp>
#include <boost/signals2/signal.hpp>
@@ -185,7 +186,7 @@ private:
const LockPoints& lp;
};
-// extracts a TxMemPoolEntry's transaction hash
+// extracts a transaction hash from CTxMempoolEntry or CTransactionRef
struct mempoolentry_txid
{
typedef uint256 result_type;
@@ -193,6 +194,11 @@ struct mempoolentry_txid
{
return entry.GetTx().GetHash();
}
+
+ result_type operator() (const CTransactionRef& tx) const
+ {
+ return tx->GetHash();
+ }
};
/** \class CompareTxMemPoolEntryByDescendantScore
@@ -662,4 +668,95 @@ public:
bool HaveCoins(const uint256 &txid) const;
};
+/**
+ * DisconnectedBlockTransactions
+
+ * During the reorg, it's desirable to re-add previously confirmed transactions
+ * to the mempool, so that anything not re-confirmed in the new chain is
+ * available to be mined. However, it's more efficient to wait until the reorg
+ * is complete and process all still-unconfirmed transactions at that time,
+ * since we expect most confirmed transactions to (typically) still be
+ * confirmed in the new chain, and re-accepting to the memory pool is expensive
+ * (and therefore better to not do in the middle of reorg-processing).
+ * Instead, store the disconnected transactions (in order!) as we go, remove any
+ * that are included in blocks in the new chain, and then process the remaining
+ * still-unconfirmed transactions at the end.
+ */
+
+// multi_index tag names
+struct txid_index {};
+struct insertion_order {};
+
+struct DisconnectedBlockTransactions {
+ typedef boost::multi_index_container<
+ CTransactionRef,
+ boost::multi_index::indexed_by<
+ // sorted by txid
+ boost::multi_index::hashed_unique<
+ boost::multi_index::tag<txid_index>,
+ mempoolentry_txid,
+ SaltedTxidHasher
+ >,
+ // sorted by order in the blockchain
+ boost::multi_index::sequenced<
+ boost::multi_index::tag<insertion_order>
+ >
+ >
+ > indexed_disconnected_transactions;
+
+ // It's almost certainly a logic bug if we don't clear out queuedTx before
+ // destruction, as we add to it while disconnecting blocks, and then we
+ // need to re-process remaining transactions to ensure mempool consistency.
+ // For now, assert() that we've emptied out this object on destruction.
+ // This assert() can always be removed if the reorg-processing code were
+ // to be refactored such that this assumption is no longer true (for
+ // instance if there was some other way we cleaned up the mempool after a
+ // reorg, besides draining this object).
+ ~DisconnectedBlockTransactions() { assert(queuedTx.empty()); }
+
+ indexed_disconnected_transactions queuedTx;
+ uint64_t cachedInnerUsage = 0;
+
+ // Estimate the overhead of queuedTx to be 6 pointers + an allocation, as
+ // no exact formula for boost::multi_index_contained is implemented.
+ size_t DynamicMemoryUsage() const {
+ return memusage::MallocUsage(sizeof(CTransactionRef) + 6 * sizeof(void*)) * queuedTx.size() + cachedInnerUsage;
+ }
+
+ void addTransaction(const CTransactionRef& tx)
+ {
+ queuedTx.insert(tx);
+ cachedInnerUsage += RecursiveDynamicUsage(tx);
+ }
+
+ // Remove entries based on txid_index, and update memory usage.
+ void removeForBlock(const std::vector<CTransactionRef>& vtx)
+ {
+ // Short-circuit in the common case of a block being added to the tip
+ if (queuedTx.empty()) {
+ return;
+ }
+ for (auto const &tx : vtx) {
+ auto it = queuedTx.find(tx->GetHash());
+ if (it != queuedTx.end()) {
+ cachedInnerUsage -= RecursiveDynamicUsage(*it);
+ queuedTx.erase(it);
+ }
+ }
+ }
+
+ // Remove an entry by insertion_order index, and update memory usage.
+ void removeEntry(indexed_disconnected_transactions::index<insertion_order>::type::iterator entry)
+ {
+ cachedInnerUsage -= RecursiveDynamicUsage(*entry);
+ queuedTx.get<insertion_order>().erase(entry);
+ }
+
+ void clear()
+ {
+ cachedInnerUsage = 0;
+ queuedTx.clear();
+ }
+};
+
#endif // BITCOIN_TXMEMPOOL_H
diff --git a/src/util.h b/src/util.h
index 229478d835..4386ddd550 100644
--- a/src/util.h
+++ b/src/util.h
@@ -241,7 +241,8 @@ bool SoftSetArg(const std::string& strArg, const std::string& strValue);
*/
bool SoftSetBoolArg(const std::string& strArg, bool fValue);
-// Forces a arg setting, used only in testing
+// Forces an arg setting. Called by SoftSetArg() if the arg hasn't already
+// been set. Also called directly in testing.
void ForceSetArg(const std::string& strArg, const std::string& strValue);
};
diff --git a/src/validation.cpp b/src/validation.cpp
index ac16af3ee7..381d1b01c2 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -342,6 +342,56 @@ static bool IsCurrentForFeeEstimation()
return true;
}
+/* Make mempool consistent after a reorg, by re-adding or recursively erasing
+ * disconnected block transactions from the mempool, and also removing any
+ * other transactions from the mempool that are no longer valid given the new
+ * tip/height.
+ *
+ * Note: we assume that disconnectpool only contains transactions that are NOT
+ * confirmed in the current chain nor already in the mempool (otherwise,
+ * in-mempool descendants of such transactions would be removed).
+ *
+ * Passing fAddToMempool=false will skip trying to add the transactions back,
+ * and instead just erase from the mempool as needed.
+ */
+
+void UpdateMempoolForReorg(DisconnectedBlockTransactions &disconnectpool, bool fAddToMempool)
+{
+ AssertLockHeld(cs_main);
+ std::vector<uint256> vHashUpdate;
+ // disconnectpool's insertion_order index sorts the entries from
+ // oldest to newest, but the oldest entry will be the last tx from the
+ // latest mined block that was disconnected.
+ // Iterate disconnectpool in reverse, so that we add transactions
+ // back to the mempool starting with the earliest transaction that had
+ // been previously seen in a block.
+ auto it = disconnectpool.queuedTx.get<insertion_order>().rbegin();
+ while (it != disconnectpool.queuedTx.get<insertion_order>().rend()) {
+ // ignore validation errors in resurrected transactions
+ CValidationState stateDummy;
+ if (!fAddToMempool || (*it)->IsCoinBase() || !AcceptToMemoryPool(mempool, stateDummy, *it, false, NULL, NULL, true)) {
+ // If the transaction doesn't make it in to the mempool, remove any
+ // transactions that depend on it (which would now be orphans).
+ mempool.removeRecursive(**it, MemPoolRemovalReason::REORG);
+ } else if (mempool.exists((*it)->GetHash())) {
+ vHashUpdate.push_back((*it)->GetHash());
+ }
+ ++it;
+ }
+ disconnectpool.queuedTx.clear();
+ // AcceptToMemoryPool/addUnchecked all assume that new mempool entries have
+ // no in-mempool children, which is generally not true when adding
+ // previously-confirmed transactions back to the mempool.
+ // UpdateTransactionsFromBlock finds descendants of any transactions in
+ // the disconnectpool that were added back and cleans up the mempool state.
+ mempool.UpdateTransactionsFromBlock(vHashUpdate);
+
+ // We also need to remove any now-immature transactions
+ mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS);
+ // Re-limit mempool size, in case we added any transactions
+ LimitMempoolSize(mempool, GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60);
+}
+
bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const CTransactionRef& ptx, bool fLimitFree,
bool* pfMissingInputs, int64_t nAcceptTime, std::list<CTransactionRef>* plTxnReplaced,
bool fOverrideMempoolLimit, const CAmount& nAbsurdFee, std::vector<uint256>& vHashTxnToUncache)
@@ -1814,6 +1864,16 @@ void PruneAndFlush() {
FlushStateToDisk(state, FLUSH_STATE_NONE);
}
+static void DoWarning(const std::string& strWarning)
+{
+ static bool fWarned = false;
+ SetMiscWarning(strWarning);
+ if (!fWarned) {
+ AlertNotify(strWarning);
+ fWarned = true;
+ }
+}
+
/** Update chainActive and related internal data structures. */
void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) {
chainActive.SetTip(pindexNew);
@@ -1823,7 +1883,6 @@ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) {
cvBlockChange.notify_all();
- static bool fWarned = false;
std::vector<std::string> warningMessages;
if (!IsInitialBlockDownload())
{
@@ -1833,15 +1892,11 @@ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) {
WarningBitsConditionChecker checker(bit);
ThresholdState state = checker.GetStateFor(pindex, chainParams.GetConsensus(), warningcache[bit]);
if (state == THRESHOLD_ACTIVE || state == THRESHOLD_LOCKED_IN) {
+ const std::string strWarning = strprintf(_("Warning: unknown new rules activated (versionbit %i)"), bit);
if (state == THRESHOLD_ACTIVE) {
- std::string strWarning = strprintf(_("Warning: unknown new rules activated (versionbit %i)"), bit);
- SetMiscWarning(strWarning);
- if (!fWarned) {
- AlertNotify(strWarning);
- fWarned = true;
- }
+ DoWarning(strWarning);
} else {
- warningMessages.push_back(strprintf("unknown new rules are about to activate (versionbit %i)", bit));
+ warningMessages.push_back(strWarning);
}
}
}
@@ -1854,16 +1909,12 @@ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) {
pindex = pindex->pprev;
}
if (nUpgraded > 0)
- warningMessages.push_back(strprintf("%d of last 100 blocks have unexpected version", nUpgraded));
+ warningMessages.push_back(strprintf(_("%d of last 100 blocks have unexpected version"), nUpgraded));
if (nUpgraded > 100/2)
{
std::string strWarning = _("Warning: Unknown block versions being mined! It's possible unknown rules are in effect");
// notify GetWarnings(), called by Qt and the JSON-RPC code to warn the user:
- SetMiscWarning(strWarning);
- if (!fWarned) {
- AlertNotify(strWarning);
- fWarned = true;
- }
+ DoWarning(strWarning);
}
}
LogPrintf("%s: new best=%s height=%d version=0x%08x log2_work=%.8g tx=%lu date='%s' progress=%f cache=%.1fMiB(%utx)", __func__,
@@ -1877,8 +1928,17 @@ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) {
}
-/** Disconnect chainActive's tip. You probably want to call mempool.removeForReorg and manually re-limit mempool size after this, with cs_main held. */
-bool static DisconnectTip(CValidationState& state, const CChainParams& chainparams, bool fBare = false)
+/** Disconnect chainActive's tip.
+ * After calling, the mempool will be in an inconsistent state, with
+ * transactions from disconnected blocks being added to disconnectpool. You
+ * should make the mempool consistent again by calling UpdateMempoolForReorg.
+ * with cs_main held.
+ *
+ * If disconnectpool is NULL, then no disconnected transactions are added to
+ * disconnectpool (note that the caller is responsible for mempool consistency
+ * in any case).
+ */
+bool static DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions *disconnectpool)
{
CBlockIndex *pindexDelete = chainActive.Tip();
assert(pindexDelete);
@@ -1901,25 +1961,17 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara
if (!FlushStateToDisk(state, FLUSH_STATE_IF_NEEDED))
return false;
- if (!fBare) {
- // Resurrect mempool transactions from the disconnected block.
- std::vector<uint256> vHashUpdate;
- for (const auto& it : block.vtx) {
- const CTransaction& tx = *it;
- // ignore validation errors in resurrected transactions
- CValidationState stateDummy;
- if (tx.IsCoinBase() || !AcceptToMemoryPool(mempool, stateDummy, it, false, NULL, NULL, true)) {
- mempool.removeRecursive(tx, MemPoolRemovalReason::REORG);
- } else if (mempool.exists(tx.GetHash())) {
- vHashUpdate.push_back(tx.GetHash());
- }
+ if (disconnectpool) {
+ // Save transactions to re-add to mempool at end of reorg
+ for (auto it = block.vtx.rbegin(); it != block.vtx.rend(); ++it) {
+ disconnectpool->addTransaction(*it);
+ }
+ while (disconnectpool->DynamicMemoryUsage() > MAX_DISCONNECTED_TX_POOL_SIZE * 1000) {
+ // Drop the earliest entry, and remove its children from the mempool.
+ auto it = disconnectpool->queuedTx.get<insertion_order>().begin();
+ mempool.removeRecursive(**it, MemPoolRemovalReason::REORG);
+ disconnectpool->removeEntry(it);
}
- // AcceptToMemoryPool/addUnchecked all assume that new mempool entries have
- // no in-mempool children, which is generally not true when adding
- // previously-confirmed transactions back to the mempool.
- // UpdateTransactionsFromBlock finds descendants of any transactions in this
- // block that were added back and cleans up the mempool state.
- mempool.UpdateTransactionsFromBlock(vHashUpdate);
}
// Update chainActive and related variables.
@@ -2007,7 +2059,7 @@ public:
*
* The block is added to connectTrace if connection succeeds.
*/
-bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace)
+bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool)
{
assert(pindexNew->pprev == chainActive.Tip());
// Read block from disk.
@@ -2049,6 +2101,7 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams,
LogPrint(BCLog::BENCH, " - Writing chainstate: %.2fms [%.2fs]\n", (nTime5 - nTime4) * 0.001, nTimeChainState * 0.000001);
// Remove conflicting transactions from the mempool.;
mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight);
+ disconnectpool.removeForBlock(blockConnecting.vtx);
// Update chainActive & related variables.
UpdateTip(pindexNew, chainparams);
@@ -2142,9 +2195,14 @@ static bool ActivateBestChainStep(CValidationState& state, const CChainParams& c
// Disconnect active blocks which are no longer in the best chain.
bool fBlocksDisconnected = false;
+ DisconnectedBlockTransactions disconnectpool;
while (chainActive.Tip() && chainActive.Tip() != pindexFork) {
- if (!DisconnectTip(state, chainparams))
+ if (!DisconnectTip(state, chainparams, &disconnectpool)) {
+ // This is likely a fatal error, but keep the mempool consistent,
+ // just in case. Only remove from the mempool in this case.
+ UpdateMempoolForReorg(disconnectpool, false);
return false;
+ }
fBlocksDisconnected = true;
}
@@ -2167,7 +2225,7 @@ static bool ActivateBestChainStep(CValidationState& state, const CChainParams& c
// Connect new blocks.
BOOST_REVERSE_FOREACH(CBlockIndex *pindexConnect, vpindexToConnect) {
- if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr<const CBlock>(), connectTrace)) {
+ if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr<const CBlock>(), connectTrace, disconnectpool)) {
if (state.IsInvalid()) {
// The block violates a consensus rule.
if (!state.CorruptionPossible())
@@ -2178,6 +2236,9 @@ static bool ActivateBestChainStep(CValidationState& state, const CChainParams& c
break;
} else {
// A system error occurred (disk space, database error, ...).
+ // Make the mempool consistent with the current tip, just in case
+ // any observers try to use it before shutdown.
+ UpdateMempoolForReorg(disconnectpool, false);
return false;
}
} else {
@@ -2192,8 +2253,9 @@ static bool ActivateBestChainStep(CValidationState& state, const CChainParams& c
}
if (fBlocksDisconnected) {
- mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS);
- LimitMempoolSize(mempool, GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60);
+ // If any blocks were disconnected, disconnectpool may be non empty. Add
+ // any disconnected transactions back to the mempool.
+ UpdateMempoolForReorg(disconnectpool, true);
}
mempool.check(pcoinsTip);
@@ -2342,6 +2404,7 @@ bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, C
setDirtyBlockIndex.insert(pindex);
setBlockIndexCandidates.erase(pindex);
+ DisconnectedBlockTransactions disconnectpool;
while (chainActive.Contains(pindex)) {
CBlockIndex *pindexWalk = chainActive.Tip();
pindexWalk->nStatus |= BLOCK_FAILED_CHILD;
@@ -2349,13 +2412,17 @@ bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, C
setBlockIndexCandidates.erase(pindexWalk);
// ActivateBestChain considers blocks already in chainActive
// unconditionally valid already, so force disconnect away from it.
- if (!DisconnectTip(state, chainparams)) {
- mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS);
+ if (!DisconnectTip(state, chainparams, &disconnectpool)) {
+ // It's probably hopeless to try to make the mempool consistent
+ // here if DisconnectTip failed, but we can try.
+ UpdateMempoolForReorg(disconnectpool, false);
return false;
}
}
- LimitMempoolSize(mempool, GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60);
+ // DisconnectTip will add transactions to disconnectpool; try to add these
+ // back to the mempool.
+ UpdateMempoolForReorg(disconnectpool, true);
// The resulting new best tip may not be in setBlockIndexCandidates anymore, so
// add it again.
@@ -2368,7 +2435,6 @@ bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, C
}
InvalidChainFound(pindex);
- mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS);
uiInterface.NotifyBlockTip(IsInitialBlockDownload(), pindex->pprev);
return true;
}
@@ -3482,7 +3548,7 @@ bool RewindBlockIndex(const CChainParams& params)
// of the blockchain).
break;
}
- if (!DisconnectTip(state, params, true)) {
+ if (!DisconnectTip(state, params, NULL)) {
return error("RewindBlockIndex: unable to disconnect block at height %i", pindex->nHeight);
}
// Occasionally flush state to disk.
diff --git a/src/validation.h b/src/validation.h
index b19c3ff4f7..02a9b5a369 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -70,6 +70,8 @@ static const unsigned int DEFAULT_DESCENDANT_LIMIT = 25;
static const unsigned int DEFAULT_DESCENDANT_SIZE_LIMIT = 101;
/** Default for -mempoolexpiry, expiration time for mempool transactions in hours */
static const unsigned int DEFAULT_MEMPOOL_EXPIRY = 336;
+/** Maximum kilobytes for transactions to store for processing during reorg */
+static const unsigned int MAX_DISCONNECTED_TX_POOL_SIZE = 20000;
/** The maximum size of a blk?????.dat file (since 0.8) */
static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB
/** The pre-allocation chunk size for blk?????.dat files (since 0.8) */
@@ -105,7 +107,7 @@ static const unsigned int DATABASE_FLUSH_INTERVAL = 24 * 60 * 60;
/** Maximum length of reject messages. */
static const unsigned int MAX_REJECT_MESSAGE_LENGTH = 111;
/** Average delay between local address broadcasts in seconds. */
-static const unsigned int AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL = 24 * 24 * 60;
+static const unsigned int AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL = 24 * 60 * 60;
/** Average delay between peer address broadcasts in seconds. */
static const unsigned int AVG_ADDRESS_BROADCAST_INTERVAL = 30;
/** Average delay between trickled inventory transmissions in seconds.
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index ae4f4f37cb..0860d3565c 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -2909,7 +2909,7 @@ UniValue bumpfee(const JSONRPCRequest& request)
UniValue errors(UniValue::VARR);
for (const std::string& err: feeBump.getErrors())
errors.push_back(err);
- result.push_back(errors);
+ result.push_back(Pair("errors", errors));
return result;
}
diff --git a/test/functional/bumpfee.py b/test/functional/bumpfee.py
index 54fd7740c1..aaed420690 100755
--- a/test/functional/bumpfee.py
+++ b/test/functional/bumpfee.py
@@ -88,6 +88,7 @@ def test_simple_bumpfee_succeeds(rbf_node, peer_node, dest_address):
sync_mempools((rbf_node, peer_node))
assert rbfid in rbf_node.getrawmempool() and rbfid in peer_node.getrawmempool()
bumped_tx = rbf_node.bumpfee(rbfid)
+ assert_equal(bumped_tx["errors"], [])
assert bumped_tx["fee"] - abs(rbftx["fee"]) > 0
# check that bumped_tx propogates, original tx was evicted and has a wallet conflict
sync_mempools((rbf_node, peer_node))
diff --git a/test/functional/pruning.py b/test/functional/pruning.py
index 17019c658b..7995e418c9 100755
--- a/test/functional/pruning.py
+++ b/test/functional/pruning.py
@@ -34,10 +34,11 @@ class PruneTest(BitcoinTestFramework):
# Create nodes 0 and 1 to mine.
# Create node 2 to test pruning.
+ self.full_node_default_args = ["-maxreceivebuffer=20000","-blockmaxsize=999000", "-checkblocks=5", "-limitdescendantcount=100", "-limitdescendantsize=5000", "-limitancestorcount=100", "-limitancestorsize=5000" ]
# Create nodes 3 and 4 to test manual pruning (they will be re-started with manual pruning later)
# Create nodes 5 to test wallet in prune mode, but do not connect
- self.extra_args = [["-maxreceivebuffer=20000", "-blockmaxsize=999000", "-checkblocks=5"],
- ["-maxreceivebuffer=20000", "-blockmaxsize=999000", "-checkblocks=5"],
+ self.extra_args = [self.full_node_default_args,
+ self.full_node_default_args,
["-maxreceivebuffer=20000", "-prune=550"],
["-maxreceivebuffer=20000", "-blockmaxsize=999000"],
["-maxreceivebuffer=20000", "-blockmaxsize=999000"],
@@ -97,12 +98,15 @@ class PruneTest(BitcoinTestFramework):
# Node 2 stays connected, so it hears about the stale blocks and then reorg's when node0 reconnects
# Stopping node 0 also clears its mempool, so it doesn't have node1's transactions to accidentally mine
self.stop_node(0)
- self.nodes[0]=start_node(0, self.options.tmpdir, ["-maxreceivebuffer=20000","-blockmaxsize=999000", "-checkblocks=5"], timewait=900)
+ self.nodes[0]=start_node(0, self.options.tmpdir, self.full_node_default_args, timewait=900)
# Mine 24 blocks in node 1
for i in range(24):
if j == 0:
mine_large_block(self.nodes[1], self.utxo_cache_1)
else:
+ # Add node1's wallet transactions back to the mempool, to
+ # avoid the mined blocks from being too small.
+ self.nodes[1].resendwallettransactions()
self.nodes[1].generate(1) #tx's already in mempool from previous disconnects
# Reorg back with 25 block chain from node 0
@@ -159,6 +163,11 @@ class PruneTest(BitcoinTestFramework):
self.log.info("Usage possibly still high bc of stale blocks in block files: %d" % calc_usage(self.prunedir))
self.log.info("Mine 220 more blocks so we have requisite history (some blocks will be big and cause pruning of previous chain)")
+
+ # Get node0's wallet transactions back in its mempool, to avoid the
+ # mined blocks from being too small.
+ self.nodes[0].resendwallettransactions()
+
for i in range(22):
# This can be slow, so do this in multiple RPC calls to avoid
# RPC timeouts.
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-11 02:24:26 +0000