aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rwxr-xr-xcontrib/devtools/github-merge.py3
-rw-r--r--doc/developer-notes.md45
-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/net_processing.cpp8
-rw-r--r--src/policy/fees.cpp6
-rw-r--r--src/policy/fees.h2
-rw-r--r--src/qt/test/wallettests.cpp93
-rw-r--r--src/qt/transactionview.cpp7
-rw-r--r--src/script/interpreter.cpp14
-rw-r--r--src/script/script.h3
-rw-r--r--src/txdb.cpp6
-rw-r--r--src/txmempool.cpp2
-rw-r--r--src/txmempool.h99
-rw-r--r--src/util.h3
-rw-r--r--src/validation.cpp159
-rw-r--r--src/validation.h4
-rw-r--r--src/wallet/feebumper.cpp2
-rw-r--r--src/wallet/rpcwallet.cpp2
-rw-r--r--src/wallet/wallet.cpp18
-rwxr-xr-xtest/functional/blockchain.py26
-rwxr-xr-xtest/functional/bumpfee.py1
-rwxr-xr-xtest/functional/pruning.py15
25 files changed, 426 insertions, 118 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/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/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/net_processing.cpp b/src/net_processing.cpp
index aca88f2660..3102c2ef9a 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -754,8 +754,8 @@ void PeerLogicValidation::BlockConnected(const std::shared_ptr<const CBlock>& pb
const CTransaction& tx = *ptx;
// Which orphan pool entries must we evict?
- for (size_t j = 0; j < tx.vin.size(); j++) {
- auto itByPrev = mapOrphanTransactionsByPrev.find(tx.vin[j].prevout);
+ for (const auto& txin : tx.vin) {
+ auto itByPrev = mapOrphanTransactionsByPrev.find(txin.prevout);
if (itByPrev == mapOrphanTransactionsByPrev.end()) continue;
for (auto mi = itByPrev->second.begin(); mi != itByPrev->second.end(); ++mi) {
const CTransaction& orphanTx = *(*mi)->second.tx;
@@ -1553,10 +1553,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
uint32_t nFetchFlags = GetFetchFlags(pfrom);
- for (unsigned int nInv = 0; nInv < vInv.size(); nInv++)
+ for (CInv &inv : vInv)
{
- CInv &inv = vInv[nInv];
-
if (interruptMsgProc)
return true;
diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp
index bad3de4b44..66083fe1ee 100644
--- a/src/policy/fees.cpp
+++ b/src/policy/fees.cpp
@@ -604,8 +604,8 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight,
unsigned int countedTxs = 0;
// Update averages with data points from current block
- for (unsigned int i = 0; i < entries.size(); i++) {
- if (processBlockTx(nBlockHeight, entries[i]))
+ for (const auto& entry : entries) {
+ if (processBlockTx(nBlockHeight, entry))
countedTxs++;
}
@@ -786,7 +786,7 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun
* This is necessary to preserve monotonically increasing estimates.
* For non-conservative estimates we do the same thing for 2*target, but
* for conservative estimates we want to skip these shorter horizons
- * checks for 2*target becuase we are taking the max over all time
+ * checks for 2*target because we are taking the max over all time
* horizons so we already have monotonically increasing estimates and
* the purpose of conservative estimates is not to let short term
* fluctuations lower our estimates by too much.
diff --git a/src/policy/fees.h b/src/policy/fees.h
index f527c85004..e99fec2c39 100644
--- a/src/policy/fees.h
+++ b/src/policy/fees.h
@@ -51,7 +51,7 @@ class TxConfirmStats;
* blocks. This is is tracked in 3 different data sets each up to a maximum
* number of periods. Within each data set we have an array of counters in each
* feerate bucket and we increment all the counters from Y' up to max periods
- * representing that a tx was successfullly confirmed in less than or equal to
+ * representing that a tx was successfully confirmed in less than or equal to
* that many periods. We want to save a history of this information, so at any
* time we have a counter of the total number of transactions that happened in a
* given feerate bucket and the total number that were confirmed in each of the
diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp
index 798d333d63..0e12a9d53e 100644
--- a/src/qt/test/wallettests.cpp
+++ b/src/qt/test/wallettests.cpp
@@ -8,26 +8,46 @@
#include "qt/sendcoinsdialog.h"
#include "qt/sendcoinsentry.h"
#include "qt/transactiontablemodel.h"
+#include "qt/transactionview.h"
#include "qt/walletmodel.h"
#include "test/test_bitcoin.h"
#include "validation.h"
#include "wallet/wallet.h"
#include <QAbstractButton>
+#include <QAction>
#include <QApplication>
+#include <QCheckBox>
+#include <QPushButton>
#include <QTimer>
#include <QVBoxLayout>
namespace
{
-//! Press "Yes" button in modal send confirmation dialog.
-void ConfirmSend()
+//! Press "Ok" button in message box dialog.
+void ConfirmMessage(QString* text = nullptr)
{
- QTimer::singleShot(0, makeCallback([](Callback* callback) {
+ QTimer::singleShot(0, makeCallback([text](Callback* callback) {
+ for (QWidget* widget : QApplication::topLevelWidgets()) {
+ if (widget->inherits("QMessageBox")) {
+ QMessageBox* messageBox = qobject_cast<QMessageBox*>(widget);
+ if (text) *text = messageBox->text();
+ messageBox->defaultButton()->click();
+ }
+ }
+ delete callback;
+ }), SLOT(call()));
+}
+
+//! Press "Yes" or "Cancel" buttons in modal send confirmation dialog.
+void ConfirmSend(QString* text = nullptr, bool cancel = false)
+{
+ QTimer::singleShot(0, makeCallback([text, cancel](Callback* callback) {
for (QWidget* widget : QApplication::topLevelWidgets()) {
if (widget->inherits("SendConfirmationDialog")) {
SendConfirmationDialog* dialog = qobject_cast<SendConfirmationDialog*>(widget);
- QAbstractButton* button = dialog->button(QMessageBox::Yes);
+ if (text) *text = dialog->text();
+ QAbstractButton* button = dialog->button(cancel ? QMessageBox::Cancel : QMessageBox::Yes);
button->setEnabled(true);
button->click();
}
@@ -37,12 +57,16 @@ void ConfirmSend()
}
//! Send coins to address and return txid.
-uint256 SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CBitcoinAddress& address, CAmount amount)
+uint256 SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CBitcoinAddress& address, CAmount amount, bool rbf)
{
QVBoxLayout* entries = sendCoinsDialog.findChild<QVBoxLayout*>("entries");
SendCoinsEntry* entry = qobject_cast<SendCoinsEntry*>(entries->itemAt(0)->widget());
entry->findChild<QValidatedLineEdit*>("payTo")->setText(QString::fromStdString(address.ToString()));
entry->findChild<BitcoinAmountField*>("payAmount")->setValue(amount);
+ sendCoinsDialog.findChild<QFrame*>("frameFee")
+ ->findChild<QFrame*>("frameFeeSelection")
+ ->findChild<QCheckBox*>("optInRBF")
+ ->setCheckState(rbf ? Qt::Checked : Qt::Unchecked);
uint256 txid;
boost::signals2::scoped_connection c(wallet.NotifyTransactionChanged.connect([&txid](CWallet*, const uint256& hash, ChangeType status) {
if (status == CT_NEW) txid = hash;
@@ -66,6 +90,43 @@ 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)
+{
+ QTableView* table = view.findChild<QTableView*>("transactionView");
+ QModelIndex index = FindTx(*table->selectionModel()->model(), txid);
+ QVERIFY2(index.isValid(), "Could not find BumpFee txid");
+
+ // Select row in table, invoke context menu, and make sure bumpfee action is
+ // enabled or disabled as expected.
+ QAction* action = view.findChild<QAction*>("bumpFeeAction");
+ table->selectionModel()->select(index, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows);
+ action->setEnabled(expectDisabled);
+ RequestContextMenu(table);
+ QCOMPARE(action->isEnabled(), !expectDisabled);
+
+ action->setEnabled(true);
+ QString text;
+ if (expectError.empty()) {
+ ConfirmSend(&text, cancel);
+ } else {
+ ConfirmMessage(&text);
+ }
+ action->trigger();
+ QVERIFY(text.indexOf(QString::fromStdString(expectError)) != -1);
+}
+
//! Simple qt wallet tests.
//
// Test widgets can be debugged interactively calling show() on them and
@@ -81,9 +142,11 @@ QModelIndex FindTx(const QAbstractItemModel& model, const uint256& txid)
// src/qt/test/test_bitcoin-qt -platform cocoa # macOS
void TestSendCoins()
{
- // Set up wallet and chain with 101 blocks (1 mature block for spending).
+ // Set up wallet and chain with 105 blocks (5 mature blocks for spending).
TestChain100Setup test;
- test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey()));
+ for (int i = 0; i < 5; ++i) {
+ test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey()));
+ }
bitdb.MakeMock();
std::unique_ptr<CWalletDBWrapper> dbw(new CWalletDBWrapper(&bitdb, "wallet_test.dat"));
CWallet wallet(std::move(dbw));
@@ -100,19 +163,27 @@ void TestSendCoins()
// Create widgets for sending coins and listing transactions.
std::unique_ptr<const PlatformStyle> platformStyle(PlatformStyle::instantiate("other"));
SendCoinsDialog sendCoinsDialog(platformStyle.get());
+ TransactionView transactionView(platformStyle.get());
OptionsModel optionsModel;
WalletModel walletModel(platformStyle.get(), &wallet, &optionsModel);
sendCoinsDialog.setModel(&walletModel);
+ transactionView.setModel(&walletModel);
// Send two transactions, and verify they are added to transaction list.
TransactionTableModel* transactionTableModel = walletModel.getTransactionTableModel();
- QCOMPARE(transactionTableModel->rowCount({}), 101);
- uint256 txid1 = SendCoins(wallet, sendCoinsDialog, CBitcoinAddress(CKeyID()), 5 * COIN);
- uint256 txid2 = SendCoins(wallet, sendCoinsDialog, CBitcoinAddress(CKeyID()), 10 * COIN);
- QCOMPARE(transactionTableModel->rowCount({}), 103);
+ QCOMPARE(transactionTableModel->rowCount({}), 105);
+ uint256 txid1 = SendCoins(wallet, sendCoinsDialog, CBitcoinAddress(CKeyID()), 5 * COIN, false /* rbf */);
+ uint256 txid2 = SendCoins(wallet, sendCoinsDialog, CBitcoinAddress(CKeyID()), 10 * COIN, true /* rbf */);
+ QCOMPARE(transactionTableModel->rowCount({}), 107);
QVERIFY(FindTx(*transactionTableModel, txid1).isValid());
QVERIFY(FindTx(*transactionTableModel, txid2).isValid());
+ // Call bumpfee. Test disabled, canceled, enabled, then failing cases.
+ BumpFee(transactionView, txid1, true /* expect disabled */, "not BIP 125 replaceable" /* expected error */, false /* cancel */);
+ BumpFee(transactionView, txid2, false /* expect disabled */, {} /* expected error */, true /* cancel */);
+ BumpFee(transactionView, txid2, false /* expect disabled */, {} /* expected error */, false /* cancel */);
+ BumpFee(transactionView, txid2, false /* expect disabled */, "already bumped" /* expected error */, false /* cancel */);
+
bitdb.Flush(true);
bitdb.Reset();
}
diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp
index 9008c81634..5da10e41b9 100644
--- a/src/qt/transactionview.cpp
+++ b/src/qt/transactionview.cpp
@@ -136,10 +136,12 @@ TransactionView::TransactionView(const PlatformStyle *platformStyle, QWidget *pa
view->installEventFilter(this);
transactionView = view;
+ transactionView->setObjectName("transactionView");
// Actions
abandonAction = new QAction(tr("Abandon transaction"), this);
bumpFeeAction = new QAction(tr("Increase transaction fee"), this);
+ bumpFeeAction->setObjectName("bumpFeeAction");
QAction *copyAddressAction = new QAction(tr("Copy address"), this);
QAction *copyLabelAction = new QAction(tr("Copy label"), this);
QAction *copyAmountAction = new QAction(tr("Copy amount"), this);
@@ -150,6 +152,7 @@ TransactionView::TransactionView(const PlatformStyle *platformStyle, QWidget *pa
QAction *showDetailsAction = new QAction(tr("Show transaction details"), this);
contextMenu = new QMenu(this);
+ contextMenu->setObjectName("contextMenu");
contextMenu->addAction(copyAddressAction);
contextMenu->addAction(copyLabelAction);
contextMenu->addAction(copyAmountAction);
@@ -380,7 +383,7 @@ void TransactionView::contextualMenu(const QPoint &point)
if(index.isValid())
{
- contextMenu->exec(QCursor::pos());
+ contextMenu->popup(transactionView->viewport()->mapToGlobal(point));
}
}
@@ -416,7 +419,7 @@ void TransactionView::bumpFee()
// Bump tx fee over the walletModel
if (model->bumpFee(hash)) {
// Update the table
- model->getTransactionTableModel()->updateTransaction(hashQStr, CT_UPDATED, false);
+ model->getTransactionTableModel()->updateTransaction(hashQStr, CT_UPDATED, true);
}
}
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index f4e5313a78..222cff59ea 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -1028,7 +1028,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
}
// Size limits
- if (stack.size() + altstack.size() > 1000)
+ if (stack.size() + altstack.size() > MAX_STACK_SIZE)
return set_error(serror, SCRIPT_ERR_STACK_SIZE);
}
}
@@ -1142,24 +1142,24 @@ public:
uint256 GetPrevoutHash(const CTransaction& txTo) {
CHashWriter ss(SER_GETHASH, 0);
- for (unsigned int n = 0; n < txTo.vin.size(); n++) {
- ss << txTo.vin[n].prevout;
+ for (const auto& txin : txTo.vin) {
+ ss << txin.prevout;
}
return ss.GetHash();
}
uint256 GetSequenceHash(const CTransaction& txTo) {
CHashWriter ss(SER_GETHASH, 0);
- for (unsigned int n = 0; n < txTo.vin.size(); n++) {
- ss << txTo.vin[n].nSequence;
+ for (const auto& txin : txTo.vin) {
+ ss << txin.nSequence;
}
return ss.GetHash();
}
uint256 GetOutputsHash(const CTransaction& txTo) {
CHashWriter ss(SER_GETHASH, 0);
- for (unsigned int n = 0; n < txTo.vout.size(); n++) {
- ss << txTo.vout[n];
+ for (const auto& txout : txTo.vout) {
+ ss << txout;
}
return ss.GetHash();
}
diff --git a/src/script/script.h b/src/script/script.h
index d7aaa04f80..95a5999a13 100644
--- a/src/script/script.h
+++ b/src/script/script.h
@@ -30,6 +30,9 @@ static const int MAX_PUBKEYS_PER_MULTISIG = 20;
// Maximum script length in bytes
static const int MAX_SCRIPT_SIZE = 10000;
+// Maximum number of values on script interpreter stack
+static const int MAX_STACK_SIZE = 1000;
+
// Threshold for nLockTime: below this value it is interpreted as block number,
// otherwise as UNIX timestamp.
static const unsigned int LOCKTIME_THRESHOLD = 500000000; // Tue Nov 5 00:53:20 1985 UTC
diff --git a/src/txdb.cpp b/src/txdb.cpp
index 42dc31760b..76aab23983 100644
--- a/src/txdb.cpp
+++ b/src/txdb.cpp
@@ -98,7 +98,11 @@ CCoinsViewCursor *CCoinsViewDB::Cursor() const
that restriction. */
i->pcursor->Seek(DB_COINS);
// Cache key of first record
- i->pcursor->GetKey(i->keyTmp);
+ if (i->pcursor->Valid()) {
+ i->pcursor->GetKey(i->keyTmp);
+ } else {
+ i->keyTmp.first = 0; // Make sure Valid() and GetKey() return false
+ }
return i;
}
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 89358f9603..381d1b01c2 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -309,7 +309,6 @@ bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp, bool
return EvaluateSequenceLocks(index, lockPair);
}
-
void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age) {
int expired = pool.Expire(GetTime() - age);
if (expired != 0) {
@@ -343,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)
@@ -1815,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);
@@ -1824,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())
{
@@ -1834,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);
}
}
}
@@ -1855,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__,
@@ -1878,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);
@@ -1902,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.
@@ -2008,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.
@@ -2050,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);
@@ -2143,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;
}
@@ -2168,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())
@@ -2179,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 {
@@ -2193,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);
@@ -2343,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;
@@ -2350,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.
@@ -2369,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;
}
@@ -2817,8 +2882,8 @@ bool ContextualCheckBlock(const CBlock& block, CValidationState& state, const Co
// No witness data is allowed in blocks that don't commit to witness data, as this would otherwise leave room for spam
if (!fHaveWitness) {
- for (size_t i = 0; i < block.vtx.size(); i++) {
- if (block.vtx[i]->HasWitness()) {
+ for (const auto& tx : block.vtx) {
+ if (tx->HasWitness()) {
return state.DoS(100, false, REJECT_INVALID, "unexpected-witness", true, strprintf("%s : unexpected witness data found", __func__));
}
}
@@ -3483,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/feebumper.cpp b/src/wallet/feebumper.cpp
index c10a9eccd9..99120d290c 100644
--- a/src/wallet/feebumper.cpp
+++ b/src/wallet/feebumper.cpp
@@ -255,7 +255,7 @@ bool CFeeBumper::commit(CWallet *pWallet)
}
CWalletTx& oldWtx = pWallet->mapWallet[txid];
- // make sure the transaction still has no descendants and hasen't been mined in the meantime
+ // make sure the transaction still has no descendants and hasn't been mined in the meantime
if (!preconditionChecks(pWallet, oldWtx)) {
return false;
}
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/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 997515a04b..bc98435249 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1788,8 +1788,8 @@ bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const
{
CMutableTransaction tx1 = *this->tx;
CMutableTransaction tx2 = *_tx.tx;
- for (unsigned int i = 0; i < tx1.vin.size(); i++) tx1.vin[i].scriptSig = CScript();
- for (unsigned int i = 0; i < tx2.vin.size(); i++) tx2.vin[i].scriptSig = CScript();
+ for (auto& txin : tx1.vin) txin.scriptSig = CScript();
+ for (auto& txin : tx2.vin) txin.scriptSig = CScript();
return CTransaction(tx1) == CTransaction(tx2);
}
@@ -2268,10 +2268,10 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMin
if (nTotalLower == nTargetValue)
{
- for (unsigned int i = 0; i < vValue.size(); ++i)
+ for (const auto& input : vValue)
{
- setCoinsRet.insert(vValue[i]);
- nValueRet += vValue[i].txout.nValue;
+ setCoinsRet.insert(input);
+ nValueRet += input.txout.nValue;
}
return true;
}
@@ -2401,7 +2401,7 @@ bool CWallet::SignTransaction(CMutableTransaction &tx)
// sign the new tx
CTransaction txNewConst(tx);
int nIn = 0;
- for (auto& input : tx.vin) {
+ for (const auto& input : tx.vin) {
std::map<uint256, CWalletTx>::const_iterator mi = mapWallet.find(input.prevout.hash);
if(mi == mapWallet.end() || input.prevout.n >= mi->second.tx->vout.size()) {
return false;
@@ -3340,11 +3340,11 @@ std::set< std::set<CTxDestination> > CWallet::GetAddressGroupings()
}
// group lone addrs by themselves
- for (unsigned int i = 0; i < pcoin->tx->vout.size(); i++)
- if (IsMine(pcoin->tx->vout[i]))
+ for (const auto& txout : pcoin->tx->vout)
+ if (IsMine(txout))
{
CTxDestination address;
- if(!ExtractDestination(pcoin->tx->vout[i].scriptPubKey, address))
+ if(!ExtractDestination(txout.scriptPubKey, address))
continue;
grouping.insert(address);
groupings.insert(grouping);
diff --git a/test/functional/blockchain.py b/test/functional/blockchain.py
index 8596f1edaf..b6112c7280 100755
--- a/test/functional/blockchain.py
+++ b/test/functional/blockchain.py
@@ -32,7 +32,7 @@ class BlockchainTest(BitcoinTestFramework):
def __init__(self):
super().__init__()
self.setup_clean_chain = False
- self.num_nodes = 2
+ self.num_nodes = 1
def run_test(self):
self._test_gettxoutsetinfo()
@@ -50,9 +50,33 @@ class BlockchainTest(BitcoinTestFramework):
assert_equal(res['height'], 200)
assert_equal(res['txouts'], 200)
assert_equal(res['bytes_serialized'], 13924),
+ assert_equal(res['bestblock'], node.getblockhash(200))
assert_equal(len(res['bestblock']), 64)
assert_equal(len(res['hash_serialized']), 64)
+ self.log.info("Test that gettxoutsetinfo() works for blockchain with just the genesis block")
+ b1hash = node.getblockhash(1)
+ node.invalidateblock(b1hash)
+
+ res2 = node.gettxoutsetinfo()
+ assert_equal(res2['transactions'], 0)
+ assert_equal(res2['total_amount'], Decimal('0'))
+ assert_equal(res2['height'], 0)
+ assert_equal(res2['txouts'], 0)
+ assert_equal(res2['bestblock'], node.getblockhash(0))
+ assert_equal(len(res2['hash_serialized']), 64)
+
+ self.log.info("Test that gettxoutsetinfo() returns the same result after invalidate/reconsider block")
+ node.reconsiderblock(b1hash)
+
+ res3 = node.gettxoutsetinfo()
+ assert_equal(res['total_amount'], res3['total_amount'])
+ assert_equal(res['transactions'], res3['transactions'])
+ assert_equal(res['height'], res3['height'])
+ assert_equal(res['txouts'], res3['txouts'])
+ assert_equal(res['bestblock'], res3['bestblock'])
+ assert_equal(res['hash_serialized'], res3['hash_serialized'])
+
def _test_getblockheader(self):
node = self.nodes[0]
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.