aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/init.cpp19
-rw-r--r--src/miner.cpp2
-rw-r--r--src/miner.h23
-rw-r--r--src/qt/guiutil.cpp12
-rw-r--r--src/qt/guiutil.h2
-rw-r--r--src/qt/receiverequestdialog.cpp8
-rw-r--r--src/test/mempool_tests.cpp46
-rw-r--r--src/txmempool.cpp4
-rw-r--r--src/txmempool.h74
-rw-r--r--src/util.cpp22
-rw-r--r--src/util.h1
-rw-r--r--src/wallet/db.cpp48
-rw-r--r--src/wallet/db.h2
-rw-r--r--test/functional/README.md16
-rwxr-xr-xtest/functional/bumpfee.py2
-rwxr-xr-xtest/functional/multiwallet.py16
-rwxr-xr-xtest/functional/segwit.py56
-rw-r--r--test/functional/test_framework/blocktools.py64
-rwxr-xr-xtest/functional/test_runner.py23
19 files changed, 268 insertions, 172 deletions
diff --git a/src/init.cpp b/src/init.cpp
index 7215e87359..b48802637b 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1143,23 +1143,10 @@ bool AppInitParameterInteraction()
static bool LockDataDirectory(bool probeOnly)
{
- std::string strDataDir = GetDataDir().string();
-
// Make sure only a single Bitcoin process is using the data directory.
- fs::path pathLockFile = GetDataDir() / ".lock";
- FILE* file = fsbridge::fopen(pathLockFile, "a"); // empty lock file; created if it doesn't exist.
- if (file) fclose(file);
-
- try {
- static boost::interprocess::file_lock lock(pathLockFile.string().c_str());
- if (!lock.try_lock()) {
- return InitError(strprintf(_("Cannot obtain a lock on data directory %s. %s is probably already running."), strDataDir, _(PACKAGE_NAME)));
- }
- if (probeOnly) {
- lock.unlock();
- }
- } catch(const boost::interprocess::interprocess_exception& e) {
- return InitError(strprintf(_("Cannot obtain a lock on data directory %s. %s is probably already running.") + " %s.", strDataDir, _(PACKAGE_NAME), e.what()));
+ fs::path datadir = GetDataDir();
+ if (!LockDirectory(datadir, ".lock", probeOnly)) {
+ return InitError(strprintf(_("Cannot obtain a lock on data directory %s. %s is probably already running."), datadir.string(), _(PACKAGE_NAME)));
}
return true;
}
diff --git a/src/miner.cpp b/src/miner.cpp
index 4e63ab4df0..dda52790c6 100644
--- a/src/miner.cpp
+++ b/src/miner.cpp
@@ -352,7 +352,7 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda
// Try to compare the mapTx entry to the mapModifiedTx entry
iter = mempool.mapTx.project<0>(mi);
if (modit != mapModifiedTx.get<ancestor_score>().end() &&
- CompareModifiedEntry()(*modit, CTxMemPoolModifiedEntry(iter))) {
+ CompareTxMemPoolEntryByAncestorFee()(*modit, CTxMemPoolModifiedEntry(iter))) {
// The best entry in mapModifiedTx has higher score
// than the one from mapTx.
// Switch which transaction (package) to consider
diff --git a/src/miner.h b/src/miner.h
index 698b4a4788..9c086332d4 100644
--- a/src/miner.h
+++ b/src/miner.h
@@ -41,6 +41,12 @@ struct CTxMemPoolModifiedEntry {
nSigOpCostWithAncestors = entry->GetSigOpCostWithAncestors();
}
+ int64_t GetModifiedFee() const { return iter->GetModifiedFee(); }
+ uint64_t GetSizeWithAncestors() const { return nSizeWithAncestors; }
+ CAmount GetModFeesWithAncestors() const { return nModFeesWithAncestors; }
+ size_t GetTxSize() const { return iter->GetTxSize(); }
+ const CTransaction& GetTx() const { return iter->GetTx(); }
+
CTxMemPool::txiter iter;
uint64_t nSizeWithAncestors;
CAmount nModFeesWithAncestors;
@@ -67,21 +73,6 @@ struct modifiedentry_iter {
}
};
-// This matches the calculation in CompareTxMemPoolEntryByAncestorFee,
-// except operating on CTxMemPoolModifiedEntry.
-// TODO: refactor to avoid duplication of this logic.
-struct CompareModifiedEntry {
- bool operator()(const CTxMemPoolModifiedEntry &a, const CTxMemPoolModifiedEntry &b) const
- {
- double f1 = (double)a.nModFeesWithAncestors * b.nSizeWithAncestors;
- double f2 = (double)b.nModFeesWithAncestors * a.nSizeWithAncestors;
- if (f1 == f2) {
- return CTxMemPool::CompareIteratorByHash()(a.iter, b.iter);
- }
- return f1 > f2;
- }
-};
-
// A comparator that sorts transactions based on number of ancestors.
// This is sufficient to sort an ancestor package in an order that is valid
// to appear in a block.
@@ -106,7 +97,7 @@ typedef boost::multi_index_container<
// Reuse same tag from CTxMemPool's similar index
boost::multi_index::tag<ancestor_score>,
boost::multi_index::identity<CTxMemPoolModifiedEntry>,
- CompareModifiedEntry
+ CompareTxMemPoolEntryByAncestorFee
>
>
> indexed_modified_transaction_set;
diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp
index 670d6108db..558d4f108c 100644
--- a/src/qt/guiutil.cpp
+++ b/src/qt/guiutil.cpp
@@ -995,6 +995,18 @@ QString formatBytes(uint64_t bytes)
return QString(QObject::tr("%1 GB")).arg(bytes / 1024 / 1024 / 1024);
}
+qreal calculateIdealFontSize(int width, const QString& text, QFont font, qreal minPointSize, qreal font_size) {
+ while(font_size >= minPointSize) {
+ font.setPointSizeF(font_size);
+ QFontMetrics fm(font);
+ if (fm.width(text) < width) {
+ break;
+ }
+ font_size -= 0.5;
+ }
+ return font_size;
+}
+
void ClickableLabel::mouseReleaseEvent(QMouseEvent *event)
{
Q_EMIT clicked(event->pos());
diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h
index ad0e22ccd6..71a69483f5 100644
--- a/src/qt/guiutil.h
+++ b/src/qt/guiutil.h
@@ -201,6 +201,8 @@ namespace GUIUtil
QString formatBytes(uint64_t bytes);
+ qreal calculateIdealFontSize(int width, const QString& text, QFont font, qreal minPointSize = 4, qreal startPointSize = 14);
+
class ClickableLabel : public QLabel
{
Q_OBJECT
diff --git a/src/qt/receiverequestdialog.cpp b/src/qt/receiverequestdialog.cpp
index 209397ca0c..d4cb0e5ba2 100644
--- a/src/qt/receiverequestdialog.cpp
+++ b/src/qt/receiverequestdialog.cpp
@@ -183,9 +183,13 @@ void ReceiveRequestDialog::update()
QPainter painter(&qrAddrImage);
painter.drawImage(0, 0, qrImage.scaled(QR_IMAGE_SIZE, QR_IMAGE_SIZE));
QFont font = GUIUtil::fixedPitchFont();
- font.setPixelSize(12);
- painter.setFont(font);
QRect paddedRect = qrAddrImage.rect();
+
+ // calculate ideal font size
+ qreal font_size = GUIUtil::calculateIdealFontSize(paddedRect.width() - 20, info.address, font);
+ font.setPointSizeF(font_size);
+
+ painter.setFont(font);
paddedRect.setHeight(QR_IMAGE_SIZE+12);
painter.drawText(paddedRect, Qt::AlignBottom|Qt::AlignCenter, info.address);
painter.end();
diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
index f56f341498..1766c6a093 100644
--- a/src/test/mempool_tests.cpp
+++ b/src/test/mempool_tests.cpp
@@ -287,35 +287,6 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx());
pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx());
- /* Now check the sort on the mining score index.
- * Final order should be:
- *
- * tx7 (2M)
- * tx2 (20k)
- * tx4 (15000)
- * tx1/tx5 (10000)
- * tx3/6 (0)
- * (Ties resolved by hash)
- */
- sortedOrder.clear();
- sortedOrder.push_back(tx7.GetHash().ToString());
- sortedOrder.push_back(tx2.GetHash().ToString());
- sortedOrder.push_back(tx4.GetHash().ToString());
- if (tx1.GetHash() < tx5.GetHash()) {
- sortedOrder.push_back(tx5.GetHash().ToString());
- sortedOrder.push_back(tx1.GetHash().ToString());
- } else {
- sortedOrder.push_back(tx1.GetHash().ToString());
- sortedOrder.push_back(tx5.GetHash().ToString());
- }
- if (tx3.GetHash() < tx6.GetHash()) {
- sortedOrder.push_back(tx6.GetHash().ToString());
- sortedOrder.push_back(tx3.GetHash().ToString());
- } else {
- sortedOrder.push_back(tx3.GetHash().ToString());
- sortedOrder.push_back(tx6.GetHash().ToString());
- }
- CheckSort<mining_score>(pool, sortedOrder);
}
BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
@@ -427,6 +398,23 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
sortedOrder.erase(sortedOrder.end()-2);
sortedOrder.insert(sortedOrder.begin(), tx7.GetHash().ToString());
CheckSort<ancestor_score>(pool, sortedOrder);
+
+ // High-fee parent, low-fee child
+ // tx7 -> tx8
+ CMutableTransaction tx8 = CMutableTransaction();
+ tx8.vin.resize(1);
+ tx8.vin[0].prevout = COutPoint(tx7.GetHash(), 0);
+ tx8.vin[0].scriptSig = CScript() << OP_11;
+ tx8.vout.resize(1);
+ tx8.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
+ tx8.vout[0].nValue = 10*COIN;
+
+ // Check that we sort by min(feerate, ancestor_feerate):
+ // set the fee so that the ancestor feerate is above tx1/5,
+ // but the transaction's own feerate is lower
+ pool.addUnchecked(tx8.GetHash(), entry.Fee(5000LL).FromTx(tx8));
+ sortedOrder.insert(sortedOrder.end()-1, tx8.GetHash().ToString());
+ CheckSort<ancestor_score>(pool, sortedOrder);
}
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index ffb024aef9..d1edde284f 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -906,8 +906,8 @@ bool CCoinsViewMemPool::GetCoin(const COutPoint &outpoint, Coin &coin) const {
size_t CTxMemPool::DynamicMemoryUsage() const {
LOCK(cs);
- // Estimate the overhead of mapTx to be 15 pointers + an allocation, as no exact formula for boost::multi_index_contained is implemented.
- return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 15 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(mapLinks) + memusage::DynamicUsage(vTxHashes) + cachedInnerUsage;
+ // Estimate the overhead of mapTx to be 12 pointers + an allocation, as no exact formula for boost::multi_index_contained is implemented.
+ return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 12 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(mapLinks) + memusage::DynamicUsage(vTxHashes) + cachedInnerUsage;
}
void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants, MemPoolRemovalReason reason) {
diff --git a/src/txmempool.h b/src/txmempool.h
index 512e70f8fa..d6f8e7094b 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -206,18 +206,14 @@ class CompareTxMemPoolEntryByDescendantScore
public:
bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) const
{
- bool fUseADescendants = UseDescendantScore(a);
- bool fUseBDescendants = UseDescendantScore(b);
+ double a_mod_fee, a_size, b_mod_fee, b_size;
- double aModFee = fUseADescendants ? a.GetModFeesWithDescendants() : a.GetModifiedFee();
- double aSize = fUseADescendants ? a.GetSizeWithDescendants() : a.GetTxSize();
-
- double bModFee = fUseBDescendants ? b.GetModFeesWithDescendants() : b.GetModifiedFee();
- double bSize = fUseBDescendants ? b.GetSizeWithDescendants() : b.GetTxSize();
+ GetModFeeAndSize(a, a_mod_fee, a_size);
+ GetModFeeAndSize(b, b_mod_fee, b_size);
// Avoid division by rewriting (a/b > c/d) as (a*d > c*b).
- double f1 = aModFee * bSize;
- double f2 = aSize * bModFee;
+ double f1 = a_mod_fee * b_size;
+ double f2 = a_size * b_mod_fee;
if (f1 == f2) {
return a.GetTime() >= b.GetTime();
@@ -225,12 +221,21 @@ public:
return f1 < f2;
}
- // Calculate which score to use for an entry (avoiding division).
- bool UseDescendantScore(const CTxMemPoolEntry &a) const
+ // Return the fee/size we're using for sorting this entry.
+ void GetModFeeAndSize(const CTxMemPoolEntry &a, double &mod_fee, double &size) const
{
+ // Compare feerate with descendants to feerate of the transaction, and
+ // return the fee/size for the max.
double f1 = (double)a.GetModifiedFee() * a.GetSizeWithDescendants();
double f2 = (double)a.GetModFeesWithDescendants() * a.GetTxSize();
- return f2 > f1;
+
+ if (f2 > f1) {
+ mod_fee = a.GetModFeesWithDescendants();
+ size = a.GetSizeWithDescendants();
+ } else {
+ mod_fee = a.GetModifiedFee();
+ size = a.GetTxSize();
+ }
}
};
@@ -261,33 +266,53 @@ public:
}
};
+/** \class CompareTxMemPoolEntryByAncestorScore
+ *
+ * Sort an entry by min(score/size of entry's tx, score/size with all ancestors).
+ */
class CompareTxMemPoolEntryByAncestorFee
{
public:
- bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) const
+ template<typename T>
+ bool operator()(const T& a, const T& b) const
{
- double aFees = a.GetModFeesWithAncestors();
- double aSize = a.GetSizeWithAncestors();
+ double a_mod_fee, a_size, b_mod_fee, b_size;
- double bFees = b.GetModFeesWithAncestors();
- double bSize = b.GetSizeWithAncestors();
+ GetModFeeAndSize(a, a_mod_fee, a_size);
+ GetModFeeAndSize(b, b_mod_fee, b_size);
// Avoid division by rewriting (a/b > c/d) as (a*d > c*b).
- double f1 = aFees * bSize;
- double f2 = aSize * bFees;
+ double f1 = a_mod_fee * b_size;
+ double f2 = a_size * b_mod_fee;
if (f1 == f2) {
return a.GetTx().GetHash() < b.GetTx().GetHash();
}
-
return f1 > f2;
}
+
+ // Return the fee/size we're using for sorting this entry.
+ template <typename T>
+ void GetModFeeAndSize(const T &a, double &mod_fee, double &size) const
+ {
+ // Compare feerate with ancestors to feerate of the transaction, and
+ // return the fee/size for the min.
+ double f1 = (double)a.GetModifiedFee() * a.GetSizeWithAncestors();
+ double f2 = (double)a.GetModFeesWithAncestors() * a.GetTxSize();
+
+ if (f1 > f2) {
+ mod_fee = a.GetModFeesWithAncestors();
+ size = a.GetSizeWithAncestors();
+ } else {
+ mod_fee = a.GetModifiedFee();
+ size = a.GetTxSize();
+ }
+ }
};
// Multi_index tag names
struct descendant_score {};
struct entry_time {};
-struct mining_score {};
struct ancestor_score {};
class CBlockPolicyEstimator;
@@ -356,7 +381,6 @@ public:
* - transaction hash
* - feerate [we use max(feerate of tx, feerate of tx with all descendants)]
* - time in mempool
- * - mining score (feerate modified by any fee deltas from PrioritiseTransaction)
*
* Note: the term "descendant" refers to in-mempool transactions that depend on
* this one, while "ancestor" refers to in-mempool transactions that a given
@@ -446,12 +470,6 @@ public:
boost::multi_index::identity<CTxMemPoolEntry>,
CompareTxMemPoolEntryByEntryTime
>,
- // sorted by score (for mining prioritization)
- boost::multi_index::ordered_unique<
- boost::multi_index::tag<mining_score>,
- boost::multi_index::identity<CTxMemPoolEntry>,
- CompareTxMemPoolEntryByScore
- >,
// sorted by fee rate with ancestors
boost::multi_index::ordered_non_unique<
boost::multi_index::tag<ancestor_score>,
diff --git a/src/util.cpp b/src/util.cpp
index 150bc503df..80eed24ffd 100644
--- a/src/util.cpp
+++ b/src/util.cpp
@@ -72,6 +72,7 @@
#include <boost/algorithm/string/case_conv.hpp> // for to_lower()
#include <boost/algorithm/string/predicate.hpp> // for startswith() and endswith()
+#include <boost/interprocess/sync/file_lock.hpp>
#include <boost/program_options/detail/config_file.hpp>
#include <boost/thread.hpp>
#include <openssl/crypto.h>
@@ -375,6 +376,27 @@ int LogPrintStr(const std::string &str)
return ret;
}
+bool LockDirectory(const fs::path& directory, const std::string lockfile_name, bool probe_only)
+{
+ fs::path pathLockFile = directory / lockfile_name;
+ FILE* file = fsbridge::fopen(pathLockFile, "a"); // empty lock file; created if it doesn't exist.
+ if (file) fclose(file);
+
+ try {
+ static std::map<std::string, boost::interprocess::file_lock> locks;
+ boost::interprocess::file_lock& lock = locks.emplace(pathLockFile.string(), pathLockFile.string().c_str()).first->second;
+ if (!lock.try_lock()) {
+ return false;
+ }
+ if (probe_only) {
+ lock.unlock();
+ }
+ } catch (const boost::interprocess::interprocess_exception& e) {
+ return error("Error while attempting to lock directory %s: %s", directory.string(), e.what());
+ }
+ return true;
+}
+
/** Interpret string as boolean, for argument parsing */
static bool InterpretBool(const std::string& strValue)
{
diff --git a/src/util.h b/src/util.h
index 6a0d6a31e7..277b4c66af 100644
--- a/src/util.h
+++ b/src/util.h
@@ -173,6 +173,7 @@ bool TruncateFile(FILE *file, unsigned int length);
int RaiseFileDescriptorLimit(int nMinFD);
void AllocateFileRange(FILE *file, unsigned int offset, unsigned int length);
bool RenameOver(fs::path src, fs::path dest);
+bool LockDirectory(const fs::path& directory, const std::string lockfile_name, bool probe_only=false);
bool TryCreateDirectories(const fs::path& p);
fs::path GetDefaultDataDir();
const fs::path &GetDataDir(bool fNetSpecific = true);
diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp
index 35ff0e1eec..23c6279128 100644
--- a/src/wallet/db.cpp
+++ b/src/wallet/db.cpp
@@ -95,7 +95,7 @@ void CDBEnv::Close()
EnvShutdown();
}
-bool CDBEnv::Open(const fs::path& pathIn)
+bool CDBEnv::Open(const fs::path& pathIn, bool retry)
{
if (fDbEnvInit)
return true;
@@ -103,6 +103,11 @@ bool CDBEnv::Open(const fs::path& pathIn)
boost::this_thread::interruption_point();
strPath = pathIn.string();
+ if (!LockDirectory(pathIn, ".walletlock")) {
+ LogPrintf("Cannot obtain a lock on wallet directory %s. Another instance of bitcoin may be using it.\n", strPath);
+ return false;
+ }
+
fs::path pathLogDir = pathIn / "database";
TryCreateDirectories(pathLogDir);
fs::path pathErrorFile = pathIn / "db.log";
@@ -134,7 +139,24 @@ bool CDBEnv::Open(const fs::path& pathIn)
S_IRUSR | S_IWUSR);
if (ret != 0) {
dbenv->close(0);
- return error("CDBEnv::Open: Error %d opening database environment: %s\n", ret, DbEnv::strerror(ret));
+ LogPrintf("CDBEnv::Open: Error %d opening database environment: %s\n", ret, DbEnv::strerror(ret));
+ if (retry) {
+ // try moving the database env out of the way
+ fs::path pathDatabaseBak = pathIn / strprintf("database.%d.bak", GetTime());
+ try {
+ fs::rename(pathLogDir, pathDatabaseBak);
+ LogPrintf("Moved old %s to %s. Retrying.\n", pathLogDir.string(), pathDatabaseBak.string());
+ } catch (const fs::filesystem_error&) {
+ // failure is ok (well, not really, but it's not worse than what we started with)
+ }
+ // try opening it again one more time
+ if (!Open(pathIn, false)) {
+ // if it still fails, it probably means we can't even create the database env
+ return false;
+ }
+ } else {
+ return false;
+ }
}
fDbEnvInit = true;
@@ -269,25 +291,11 @@ bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walle
return false;
}
- if (!bitdb.Open(walletDir))
- {
- // try moving the database env out of the way
- fs::path pathDatabase = walletDir / "database";
- fs::path pathDatabaseBak = walletDir / strprintf("database.%d.bak", GetTime());
- try {
- fs::rename(pathDatabase, pathDatabaseBak);
- LogPrintf("Moved old %s to %s. Retrying.\n", pathDatabase.string(), pathDatabaseBak.string());
- } catch (const fs::filesystem_error&) {
- // failure is ok (well, not really, but it's not worse than what we started with)
- }
-
- // try again
- if (!bitdb.Open(walletDir)) {
- // if it still fails, it probably means we can't even create the database env
- errorStr = strprintf(_("Error initializing wallet database environment %s!"), walletDir);
- return false;
- }
+ if (!bitdb.Open(walletDir, true)) {
+ errorStr = strprintf(_("Error initializing wallet database environment %s!"), walletDir);
+ return false;
}
+
return true;
}
diff --git a/src/wallet/db.h b/src/wallet/db.h
index c6f317927f..787135e400 100644
--- a/src/wallet/db.h
+++ b/src/wallet/db.h
@@ -68,7 +68,7 @@ public:
typedef std::pair<std::vector<unsigned char>, std::vector<unsigned char> > KeyValPair;
bool Salvage(const std::string& strFile, bool fAggressive, std::vector<KeyValPair>& vResult);
- bool Open(const fs::path& path);
+ bool Open(const fs::path& path, bool retry = 0);
void Close();
void Flush(bool fShutdown);
void CheckpointLSN(const std::string& strFile);
diff --git a/test/functional/README.md b/test/functional/README.md
index 4d52751b05..d6ce490ab3 100644
--- a/test/functional/README.md
+++ b/test/functional/README.md
@@ -27,6 +27,20 @@ don't have test cases for.
`set_test_params()`, `add_options()` and `setup_xxxx()` methods at the top of
the subclass, then locally-defined helper methods, then the `run_test()` method.
+#### Naming guidelines
+
+- Name the test `<area>_test.py`, where area can be one of the following:
+ - `feature` for tests for full features that aren't wallet/mining/mempool, eg `feature_rbf.py`
+ - `interface` for tests for other interfaces (REST, ZMQ, etc), eg `interface_rest.py`
+ - `mempool` for tests for mempool behaviour, eg `mempool_reorg.py`
+ - `mining` for tests for mining features, eg `mining_prioritisetransaction.py`
+ - `p2p` for tests that explicitly test the p2p interface, eg `p2p_disconnect_ban.py`
+ - `rpc` for tests for individual RPC methods or features, eg `rpc_listtransactions.py`
+ - `wallet` for tests for wallet features, eg `wallet_keypool.py`
+- use an underscore to separate words
+ - exception: for tests for specific RPCs or command line options which don't include underscores, name the test after the exact RPC or argument name, eg `rpc_decodescript.py`, not `rpc_decode_script.py`
+- Don't use the redundant word `test` in the name, eg `interface_zmq.py`, not `interface_zmq_test.py`
+
#### General test-writing advice
- Set `self.num_nodes` to the minimum number of nodes necessary for the test.
@@ -73,7 +87,7 @@ start the networking thread. (Continue with the test logic in your existing
thread.)
- Can be used to write tests where specific P2P protocol behavior is tested.
-Examples tests are `p2p-accept-block.py`, `p2p-compactblocks.py`.
+Examples tests are `p2p-acceptblock.py`, `p2p-compactblocks.py`.
#### Comptool
diff --git a/test/functional/bumpfee.py b/test/functional/bumpfee.py
index 5cbd9f5cf7..1e5620736b 100755
--- a/test/functional/bumpfee.py
+++ b/test/functional/bumpfee.py
@@ -14,7 +14,7 @@ added in the future, they should try to follow the same convention and not
make assumptions about execution order.
"""
-from segwit import send_to_witness
+from test_framework.blocktools import send_to_witness
from test_framework.test_framework import BitcoinTestFramework
from test_framework import blocktools
from test_framework.mininode import CTransaction
diff --git a/test/functional/multiwallet.py b/test/functional/multiwallet.py
index 12d9e9f48d..0891829127 100755
--- a/test/functional/multiwallet.py
+++ b/test/functional/multiwallet.py
@@ -15,8 +15,8 @@ from test_framework.util import assert_equal, assert_raises_rpc_error
class MultiWalletTest(BitcoinTestFramework):
def set_test_params(self):
self.setup_clean_chain = True
- self.num_nodes = 1
- self.extra_args = [['-wallet=w1', '-wallet=w2', '-wallet=w3', '-wallet=w']]
+ self.num_nodes = 2
+ self.extra_args = [['-wallet=w1', '-wallet=w2', '-wallet=w3', '-wallet=w'], []]
self.supports_cli = True
def run_test(self):
@@ -28,7 +28,7 @@ class MultiWalletTest(BitcoinTestFramework):
assert_equal(set(node.listwallets()), {"w1", "w2", "w3", "w"})
- self.stop_node(0)
+ self.stop_nodes()
# should not initialize if there are duplicate wallets
self.assert_start_raises_init_error(0, ['-wallet=w1', '-wallet=w1'], 'Error loading wallet w1. Duplicate -wallet filename specified.')
@@ -59,19 +59,21 @@ class MultiWalletTest(BitcoinTestFramework):
assert_equal(set(node.listwallets()), {"w4", "w5"})
w5 = wallet("w5")
w5.generate(1)
- self.stop_node(0)
# now if wallets/ exists again, but the rootdir is specified as the walletdir, w4 and w5 should still be loaded
os.rename(wallet_dir2, wallet_dir())
- self.start_node(0, ['-wallet=w4', '-wallet=w5', '-walletdir=' + data_dir()])
+ self.restart_node(0, ['-wallet=w4', '-wallet=w5', '-walletdir=' + data_dir()])
assert_equal(set(node.listwallets()), {"w4", "w5"})
w5 = wallet("w5")
w5_info = w5.getwalletinfo()
assert_equal(w5_info['immature_balance'], 50)
- self.stop_node(0)
+ competing_wallet_dir = os.path.join(self.options.tmpdir, 'competing_walletdir')
+ os.mkdir(competing_wallet_dir)
+ self.restart_node(0, ['-walletdir='+competing_wallet_dir])
+ self.assert_start_raises_init_error(1, ['-walletdir='+competing_wallet_dir], 'Error initializing wallet database environment')
- self.start_node(0, self.extra_args[0])
+ self.restart_node(0, self.extra_args[0])
w1 = wallet("w1")
w2 = wallet("w2")
diff --git a/test/functional/segwit.py b/test/functional/segwit.py
index 5a4a7468e5..7d5c760ad9 100755
--- a/test/functional/segwit.py
+++ b/test/functional/segwit.py
@@ -4,10 +4,18 @@
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test the SegWit changeover logic."""
+from test_framework.address import (
+ key_to_p2sh_p2wpkh,
+ key_to_p2wpkh,
+ program_to_witness,
+ script_to_p2sh_p2wsh,
+ script_to_p2wsh,
+)
+from test_framework.blocktools import witness_script, send_to_witness
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import *
from test_framework.mininode import sha256, CTransaction, CTxIn, COutPoint, CTxOut, COIN, ToHex, FromHex
-from test_framework.address import script_to_p2sh, key_to_p2pkh, key_to_p2sh_p2wpkh, key_to_p2wpkh, script_to_p2sh_p2wsh, script_to_p2wsh, program_to_witness
+from test_framework.address import script_to_p2sh, key_to_p2pkh
from test_framework.script import CScript, OP_HASH160, OP_CHECKSIG, OP_0, hash160, OP_EQUAL, OP_DUP, OP_EQUALVERIFY, OP_1, OP_2, OP_CHECKMULTISIG, OP_TRUE
from io import BytesIO
@@ -16,52 +24,6 @@ NODE_2 = 2
WIT_V0 = 0
WIT_V1 = 1
-# Create a scriptPubKey corresponding to either a P2WPKH output for the
-# given pubkey, or a P2WSH output of a 1-of-1 multisig for the given
-# pubkey. Returns the hex encoding of the scriptPubKey.
-def witness_script(use_p2wsh, pubkey):
- if (use_p2wsh == False):
- # P2WPKH instead
- pubkeyhash = hash160(hex_str_to_bytes(pubkey))
- pkscript = CScript([OP_0, pubkeyhash])
- else:
- # 1-of-1 multisig
- witness_program = CScript([OP_1, hex_str_to_bytes(pubkey), OP_1, OP_CHECKMULTISIG])
- scripthash = sha256(witness_program)
- pkscript = CScript([OP_0, scripthash])
- return bytes_to_hex_str(pkscript)
-
-# Return a transaction (in hex) that spends the given utxo to a segwit output,
-# optionally wrapping the segwit output using P2SH.
-def create_witness_tx(node, use_p2wsh, utxo, pubkey, encode_p2sh, amount):
- if use_p2wsh:
- program = CScript([OP_1, hex_str_to_bytes(pubkey), OP_1, OP_CHECKMULTISIG])
- addr = script_to_p2sh_p2wsh(program) if encode_p2sh else script_to_p2wsh(program)
- else:
- addr = key_to_p2sh_p2wpkh(pubkey) if encode_p2sh else key_to_p2wpkh(pubkey)
- if not encode_p2sh:
- assert_equal(node.validateaddress(addr)['scriptPubKey'], witness_script(use_p2wsh, pubkey))
- return node.createrawtransaction([utxo], {addr: amount})
-
-# Create a transaction spending a given utxo to a segwit output corresponding
-# to the given pubkey: use_p2wsh determines whether to use P2WPKH or P2WSH;
-# encode_p2sh determines whether to wrap in P2SH.
-# sign=True will have the given node sign the transaction.
-# insert_redeem_script will be added to the scriptSig, if given.
-def send_to_witness(use_p2wsh, node, utxo, pubkey, encode_p2sh, amount, sign=True, insert_redeem_script=""):
- tx_to_witness = create_witness_tx(node, use_p2wsh, utxo, pubkey, encode_p2sh, amount)
- if (sign):
- signed = node.signrawtransaction(tx_to_witness)
- assert("errors" not in signed or len(["errors"]) == 0)
- return node.sendrawtransaction(signed["hex"])
- else:
- if (insert_redeem_script):
- tx = FromHex(CTransaction(), tx_to_witness)
- tx.vin[0].scriptSig += CScript([hex_str_to_bytes(insert_redeem_script)])
- tx_to_witness = ToHex(tx)
-
- return node.sendrawtransaction(tx_to_witness)
-
def getutxo(txid):
utxo = {}
utxo["vout"] = 0
diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py
index 93af0037e9..642ef98a27 100644
--- a/test/functional/test_framework/blocktools.py
+++ b/test/functional/test_framework/blocktools.py
@@ -4,8 +4,24 @@
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Utilities for manipulating blocks and transactions."""
+from .address import (
+ key_to_p2sh_p2wpkh,
+ key_to_p2wpkh,
+ script_to_p2sh_p2wsh,
+ script_to_p2wsh,
+)
from .mininode import *
-from .script import CScript, OP_TRUE, OP_CHECKSIG, OP_RETURN
+from .script import (
+ CScript,
+ OP_0,
+ OP_1,
+ OP_CHECKMULTISIG,
+ OP_CHECKSIG,
+ OP_RETURN,
+ OP_TRUE,
+ hash160,
+)
+from .util import assert_equal
# Create a block (with regtest difficulty)
def create_block(hashprev, coinbase, nTime=None):
@@ -108,3 +124,49 @@ def get_legacy_sigopcount_tx(tx, fAccurate=True):
# scriptSig might be of type bytes, so convert to CScript for the moment
count += CScript(j.scriptSig).GetSigOpCount(fAccurate)
return count
+
+# Create a scriptPubKey corresponding to either a P2WPKH output for the
+# given pubkey, or a P2WSH output of a 1-of-1 multisig for the given
+# pubkey. Returns the hex encoding of the scriptPubKey.
+def witness_script(use_p2wsh, pubkey):
+ if (use_p2wsh == False):
+ # P2WPKH instead
+ pubkeyhash = hash160(hex_str_to_bytes(pubkey))
+ pkscript = CScript([OP_0, pubkeyhash])
+ else:
+ # 1-of-1 multisig
+ witness_program = CScript([OP_1, hex_str_to_bytes(pubkey), OP_1, OP_CHECKMULTISIG])
+ scripthash = sha256(witness_program)
+ pkscript = CScript([OP_0, scripthash])
+ return bytes_to_hex_str(pkscript)
+
+# Return a transaction (in hex) that spends the given utxo to a segwit output,
+# optionally wrapping the segwit output using P2SH.
+def create_witness_tx(node, use_p2wsh, utxo, pubkey, encode_p2sh, amount):
+ if use_p2wsh:
+ program = CScript([OP_1, hex_str_to_bytes(pubkey), OP_1, OP_CHECKMULTISIG])
+ addr = script_to_p2sh_p2wsh(program) if encode_p2sh else script_to_p2wsh(program)
+ else:
+ addr = key_to_p2sh_p2wpkh(pubkey) if encode_p2sh else key_to_p2wpkh(pubkey)
+ if not encode_p2sh:
+ assert_equal(node.validateaddress(addr)['scriptPubKey'], witness_script(use_p2wsh, pubkey))
+ return node.createrawtransaction([utxo], {addr: amount})
+
+# Create a transaction spending a given utxo to a segwit output corresponding
+# to the given pubkey: use_p2wsh determines whether to use P2WPKH or P2WSH;
+# encode_p2sh determines whether to wrap in P2SH.
+# sign=True will have the given node sign the transaction.
+# insert_redeem_script will be added to the scriptSig, if given.
+def send_to_witness(use_p2wsh, node, utxo, pubkey, encode_p2sh, amount, sign=True, insert_redeem_script=""):
+ tx_to_witness = create_witness_tx(node, use_p2wsh, utxo, pubkey, encode_p2sh, amount)
+ if (sign):
+ signed = node.signrawtransaction(tx_to_witness)
+ assert("errors" not in signed or len(["errors"]) == 0)
+ return node.sendrawtransaction(signed["hex"])
+ else:
+ if (insert_redeem_script):
+ tx = FromHex(CTransaction(), tx_to_witness)
+ tx.vin[0].scriptSig += CScript([hex_str_to_bytes(insert_redeem_script)])
+ tx_to_witness = ToHex(tx)
+
+ return node.sendrawtransaction(tx_to_witness)
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 957a2bc5a7..72ad300e7e 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -272,6 +272,7 @@ def main():
sys.exit(0)
check_script_list(config["environment"]["SRCDIR"])
+ check_script_prefixes()
if not args.keepcache:
shutil.rmtree("%s/test/cache" % config["environment"]["BUILDDIR"], ignore_errors=True)
@@ -470,6 +471,28 @@ class TestResult():
return self.status != "Failed"
+def check_script_prefixes():
+ """Check that no more than `EXPECTED_VIOLATION_COUNT` of the
+ test scripts don't start with one of the allowed name prefixes."""
+ EXPECTED_VIOLATION_COUNT = 77
+
+ # LEEWAY is provided as a transition measure, so that pull-requests
+ # that introduce new tests that don't conform with the naming
+ # convention don't immediately cause the tests to fail.
+ LEEWAY = 10
+
+ good_prefixes_re = re.compile("(example|feature|interface|mempool|mining|p2p|rpc|wallet)_")
+ bad_script_names = [script for script in ALL_SCRIPTS if good_prefixes_re.match(script) is None]
+
+ if len(bad_script_names) < EXPECTED_VIOLATION_COUNT:
+ print("{}HURRAY!{} Number of functional tests violating naming convention reduced!".format(BOLD[1], BOLD[0]))
+ print("Consider reducing EXPECTED_VIOLATION_COUNT from %d to %d" % (EXPECTED_VIOLATION_COUNT, len(bad_script_names)))
+ elif len(bad_script_names) > EXPECTED_VIOLATION_COUNT:
+ print("INFO: %d tests not meeting naming conventions (expected %d):" % (len(bad_script_names), EXPECTED_VIOLATION_COUNT))
+ print(" %s" % ("\n ".join(sorted(bad_script_names))))
+ assert len(bad_script_names) <= EXPECTED_VIOLATION_COUNT + LEEWAY, "Too many tests not following naming convention! (%d found, expected: <= %d)" % (len(bad_script_names), EXPECTED_VIOLATION_COUNT)
+
+
def check_script_list(src_dir):
"""Check scripts directory.
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-18 15:35:02 +0000