diff options
41 files changed, 410 insertions, 273 deletions
diff --git a/.travis.yml b/.travis.yml index 0de7ca6f75..4bb0b1f9fb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ sudo: required dist: trusty os: linux -language: generic +language: minimal cache: directories: - depends/built @@ -40,7 +40,6 @@ env: before_install: - export PATH=$(echo $PATH | tr ':' "\n" | sed '/\/opt\/python/d' | tr "\n" ":" | sed "s|::|:|g") - - export PATH=$(echo $PATH | tr ':' "\n" | sed '/\/opt\/pyenv/d' | tr "\n" ":" | sed "s|::|:|g") install: - if [ -n "$DPKG_ADD_ARCH" ]; then sudo dpkg --add-architecture "$DPKG_ADD_ARCH" ; fi - if [ -n "$PACKAGES" ]; then travis_retry sudo apt-get update; fi diff --git a/Makefile.am b/Makefile.am index 3b62a10603..ad522f5a56 100644 --- a/Makefile.am +++ b/Makefile.am @@ -44,6 +44,9 @@ DIST_CONTRIB = $(top_srcdir)/contrib/bitcoin-cli.bash-completion \ $(top_srcdir)/contrib/bitcoind.bash-completion \ $(top_srcdir)/contrib/init \ $(top_srcdir)/contrib/rpm +DIST_SHARE = \ + $(top_srcdir)/share/genbuild.sh \ + $(top_srcdir)/share/rpcuser BIN_CHECKS=$(top_srcdir)/contrib/devtools/symbol-check.py \ $(top_srcdir)/contrib/devtools/security-check.py @@ -213,7 +216,7 @@ endif dist_noinst_SCRIPTS = autogen.sh -EXTRA_DIST = $(top_srcdir)/share/genbuild.sh test/functional/test_runner.py test/functional $(DIST_CONTRIB) $(DIST_DOCS) $(WINDOWS_PACKAGING) $(OSX_PACKAGING) $(BIN_CHECKS) +EXTRA_DIST = $(DIST_SHARE) test/functional/test_runner.py test/functional $(DIST_CONTRIB) $(DIST_DOCS) $(WINDOWS_PACKAGING) $(OSX_PACKAGING) $(BIN_CHECKS) EXTRA_DIST += \ test/util/bitcoin-util-test.py \ diff --git a/contrib/verify-commits/gpg.sh b/contrib/verify-commits/gpg.sh index b01e2a6d39..abd8f5fd9f 100755 --- a/contrib/verify-commits/gpg.sh +++ b/contrib/verify-commits/gpg.sh @@ -46,6 +46,11 @@ for LINE in $(echo "$GPG_RES"); do REVSIG=true GOODREVSIG="[GNUPG:] GOODSIG ${LINE#* * *}" ;; + "[GNUPG:] EXPKEYSIG "*) + [ "$BITCOIN_VERIFY_COMMITS_ALLOW_REVSIG" != 1 ] && exit 1 + REVSIG=true + GOODREVSIG="[GNUPG:] GOODSIG ${LINE#* * *}" + ;; esac done if ! $VALID; then diff --git a/contrib/verify-commits/verify-commits.sh b/contrib/verify-commits/verify-commits.sh index 74b7f38375..7194b040eb 100755 --- a/contrib/verify-commits/verify-commits.sh +++ b/contrib/verify-commits/verify-commits.sh @@ -39,7 +39,7 @@ PREV_COMMIT="" while true; do if [ "$CURRENT_COMMIT" = $VERIFIED_ROOT ]; then echo "There is a valid path from "$CURRENT_COMMIT" to $VERIFIED_ROOT where all commits are signed!" - exit 0; + exit 0 fi if [ "$CURRENT_COMMIT" = $VERIFIED_SHA512_ROOT ]; then diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 85c9cd6934..afdac16da4 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -230,7 +230,6 @@ public: vSeeds.emplace_back("testnet-seed.bitcoin.jonasschnelli.ch", true); vSeeds.emplace_back("seed.tbtc.petertodd.org", true); vSeeds.emplace_back("testnet-seed.bluematt.me", false); - vSeeds.emplace_back("testnet-seed.bitcoin.schildbach.de", false); base58Prefixes[PUBKEY_ADDRESS] = std::vector<unsigned char>(1,111); base58Prefixes[SCRIPT_ADDRESS] = std::vector<unsigned char>(1,196); diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 5923871691..31b6a3705b 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -481,6 +481,8 @@ void StopHTTPServer() } if (eventBase) { LogPrint(BCLog::HTTP, "Waiting for HTTP event thread to exit\n"); + // Exit the event loop as soon as there are no active events. + event_base_loopexit(eventBase, nullptr); // Give event loop a few seconds to exit (to send back last RPC responses), then break it // Before this was solved with event_base_loopexit, but that didn't work as expected in // at least libevent 2.0.21 and always introduced a delay. In libevent diff --git a/src/init.cpp b/src/init.cpp index f63ad7f5d3..6557434880 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -588,7 +588,7 @@ void CleanupBlockRevFiles() LogPrintf("Removing unusable blk?????.dat and rev?????.dat files for -reindex with -prune\n"); fs::path blocksdir = GetDataDir() / "blocks"; for (fs::directory_iterator it(blocksdir); it != fs::directory_iterator(); it++) { - if (is_regular_file(*it) && + if (fs::is_regular_file(*it) && it->path().filename().string().length() == 12 && it->path().filename().string().substr(8,4) == ".dat") { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f83a20102e..50ac76924b 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2187,7 +2187,16 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr mapBlockSource.emplace(pblock->GetHash(), std::make_pair(pfrom->GetId(), false)); } bool fNewBlock = false; - ProcessNewBlock(chainparams, pblock, true, &fNewBlock); + // Setting fForceProcessing to true means that we bypass some of + // our anti-DoS protections in AcceptBlock, which filters + // unrequested blocks that might be trying to waste our resources + // (eg disk space). Because we only try to reconstruct blocks when + // we're close to caught up (via the CanDirectFetch() requirement + // above, combined with the behavior of not requesting blocks until + // we have a chain with at least nMinimumChainWork), and we ignore + // compact blocks with less work than our tip, it is safe to treat + // reconstructed compact blocks as having been requested. + ProcessNewBlock(chainparams, pblock, /*fForceProcessing=*/true, &fNewBlock); if (fNewBlock) { pfrom->nLastBlockTime = GetTime(); } else { @@ -2267,7 +2276,11 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr bool fNewBlock = false; // Since we requested this block (it was in mapBlocksInFlight), force it to be processed, // even if it would not be a candidate for new tip (missing previous block, chain not long enough, etc) - ProcessNewBlock(chainparams, pblock, true, &fNewBlock); + // This bypasses some anti-DoS logic in AcceptBlock (eg to prevent + // disk-space attacks), but this should be safe due to the + // protections in the compact block handler -- see related comment + // in compact block optimistic reconstruction handling. + ProcessNewBlock(chainparams, pblock, /*fForceProcessing=*/true, &fNewBlock); if (fNewBlock) { pfrom->nLastBlockTime = GetTime(); } else { diff --git a/src/netbase.cpp b/src/netbase.cpp index 5a560bc95a..82040605c5 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -291,7 +291,7 @@ struct ProxyCredentials std::string password; }; -/** Convert SOCKS5 reply to a an error message */ +/** Convert SOCKS5 reply to an error message */ std::string Socks5ErrorString(uint8_t err) { switch(err) { diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index dcf49de5f1..c7e57671c0 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -180,6 +180,7 @@ TxConfirmStats::TxConfirmStats(const std::vector<double>& defaultBuckets, : buckets(defaultBuckets), bucketMap(defaultBucketMap) { decay = _decay; + assert(_scale != 0 && "_scale must be non-zero"); scale = _scale; confAvg.resize(maxPeriods); for (unsigned int i = 0; i < maxPeriods; i++) { @@ -418,6 +419,9 @@ void TxConfirmStats::Read(CAutoFile& filein, int nFileVersion, size_t numBuckets throw std::runtime_error("Corrupt estimates file. Decay must be between 0 and 1 (non-inclusive)"); } filein >> scale; + if (scale == 0) { + throw std::runtime_error("Corrupt estimates file. Scale must be non-zero"); + } } filein >> avg; diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index 6952eb5064..207e441b6b 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -582,8 +582,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) QString toolTipDust = tr("This label turns red if any recipient receives an amount smaller than the current dust threshold."); // how many satoshis the estimated fee can vary per byte we guess wrong - assert(nBytes != 0); - double dFeeVary = (double)nPayFee / nBytes; + double dFeeVary = (nBytes != 0) ? (double)nPayFee / nBytes : 0; QString toolTip4 = tr("Can vary +/- %1 satoshi(s) per input.").arg(dFeeVary); diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index d520d7d4be..4bd63f4649 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -984,6 +984,18 @@ QString formatNiceTimeOffset(qint64 secs) return timeBehindText; } +QString formatBytes(uint64_t bytes) +{ + if(bytes < 1024) + return QString(QObject::tr("%1 B")).arg(bytes); + if(bytes < 1024 * 1024) + return QString(QObject::tr("%1 KB")).arg(bytes / 1024); + if(bytes < 1024 * 1024 * 1024) + return QString(QObject::tr("%1 MB")).arg(bytes / 1024 / 1024); + + return QString(QObject::tr("%1 GB")).arg(bytes / 1024 / 1024 / 1024); +} + void ClickableLabel::mouseReleaseEvent(QMouseEvent *event) { Q_EMIT clicked(event->pos()); diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h index d10818d0c8..7622816f7f 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -199,6 +199,8 @@ namespace GUIUtil QString formatNiceTimeOffset(qint64 secs); + QString formatBytes(uint64_t bytes); + class ClickableLabel : public QLabel { Q_OBJECT diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp index 42934f8055..8b2a7e7047 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -33,6 +33,10 @@ bool NodeLessThan::operator()(const CNodeCombinedStats &left, const CNodeCombine return pLeft->cleanSubVer.compare(pRight->cleanSubVer) < 0; case PeerTableModel::Ping: return pLeft->dMinPing < pRight->dMinPing; + case PeerTableModel::Sent: + return pLeft->nSendBytes < pRight->nSendBytes; + case PeerTableModel::Received: + return pLeft->nRecvBytes < pRight->nRecvBytes; } return false; @@ -114,7 +118,7 @@ PeerTableModel::PeerTableModel(ClientModel *parent) : clientModel(parent), timer(0) { - columns << tr("NodeId") << tr("Node/Service") << tr("User Agent") << tr("Ping"); + columns << tr("NodeId") << tr("Node/Service") << tr("Ping") << tr("Sent") << tr("Received") << tr("User Agent"); priv.reset(new PeerTablePriv()); // default to unsorted priv->sortColumn = -1; @@ -173,10 +177,20 @@ QVariant PeerTableModel::data(const QModelIndex &index, int role) const return QString::fromStdString(rec->nodeStats.cleanSubVer); case Ping: return GUIUtil::formatPingTime(rec->nodeStats.dMinPing); + case Sent: + return GUIUtil::formatBytes(rec->nodeStats.nSendBytes); + case Received: + return GUIUtil::formatBytes(rec->nodeStats.nRecvBytes); } } else if (role == Qt::TextAlignmentRole) { - if (index.column() == Ping) - return (QVariant)(Qt::AlignRight | Qt::AlignVCenter); + switch (index.column()) { + case Ping: + case Sent: + case Received: + return QVariant(Qt::AlignRight | Qt::AlignVCenter); + default: + return QVariant(); + } } return QVariant(); diff --git a/src/qt/peertablemodel.h b/src/qt/peertablemodel.h index cc47b67ec9..ec91d07127 100644 --- a/src/qt/peertablemodel.h +++ b/src/qt/peertablemodel.h @@ -55,8 +55,10 @@ public: enum ColumnIndex { NetNodeId = 0, Address = 1, - Subversion = 2, - Ping = 3 + Ping = 2, + Sent = 3, + Received = 4, + Subversion = 5 }; /** @name Methods overridden from QAbstractTableModel diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index d895fc1663..068c40e1e6 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -935,18 +935,6 @@ void RPCConsole::on_sldGraphRange_valueChanged(int value) setTrafficGraphRange(mins); } -QString RPCConsole::FormatBytes(quint64 bytes) -{ - if(bytes < 1024) - return QString(tr("%1 B")).arg(bytes); - if(bytes < 1024 * 1024) - return QString(tr("%1 KB")).arg(bytes / 1024); - if(bytes < 1024 * 1024 * 1024) - return QString(tr("%1 MB")).arg(bytes / 1024 / 1024); - - return QString(tr("%1 GB")).arg(bytes / 1024 / 1024 / 1024); -} - void RPCConsole::setTrafficGraphRange(int mins) { ui->trafficGraph->setGraphRangeMins(mins); @@ -955,8 +943,8 @@ void RPCConsole::setTrafficGraphRange(int mins) void RPCConsole::updateTrafficStats(quint64 totalBytesIn, quint64 totalBytesOut) { - ui->lblBytesIn->setText(FormatBytes(totalBytesIn)); - ui->lblBytesOut->setText(FormatBytes(totalBytesOut)); + ui->lblBytesIn->setText(GUIUtil::formatBytes(totalBytesIn)); + ui->lblBytesOut->setText(GUIUtil::formatBytes(totalBytesOut)); } void RPCConsole::peerSelected(const QItemSelection &selected, const QItemSelection &deselected) @@ -1050,8 +1038,8 @@ void RPCConsole::updateNodeDetail(const CNodeCombinedStats *stats) ui->peerServices->setText(GUIUtil::formatServicesStr(stats->nodeStats.nServices)); ui->peerLastSend->setText(stats->nodeStats.nLastSend ? GUIUtil::formatDurationStr(GetSystemTimeInSeconds() - stats->nodeStats.nLastSend) : tr("never")); ui->peerLastRecv->setText(stats->nodeStats.nLastRecv ? GUIUtil::formatDurationStr(GetSystemTimeInSeconds() - stats->nodeStats.nLastRecv) : tr("never")); - ui->peerBytesSent->setText(FormatBytes(stats->nodeStats.nSendBytes)); - ui->peerBytesRecv->setText(FormatBytes(stats->nodeStats.nRecvBytes)); + ui->peerBytesSent->setText(GUIUtil::formatBytes(stats->nodeStats.nSendBytes)); + ui->peerBytesRecv->setText(GUIUtil::formatBytes(stats->nodeStats.nRecvBytes)); ui->peerConnTime->setText(GUIUtil::formatDurationStr(GetSystemTimeInSeconds() - stats->nodeStats.nTimeConnected)); ui->peerPingTime->setText(GUIUtil::formatPingTime(stats->nodeStats.dPingTime)); ui->peerPingWait->setText(GUIUtil::formatPingTime(stats->nodeStats.dPingWait)); diff --git a/src/qt/rpcconsole.h b/src/qt/rpcconsole.h index da06818f87..ad6e84a44a 100644 --- a/src/qt/rpcconsole.h +++ b/src/qt/rpcconsole.h @@ -123,7 +123,6 @@ Q_SIGNALS: void cmdRequest(const QString &command); private: - static QString FormatBytes(quint64 bytes); void startExecutor(); void setTrafficGraphRange(int mins); /** show detailed information on ui about selected node */ diff --git a/src/validation.cpp b/src/validation.cpp index 7f6697c780..866e0c9fba 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -939,6 +939,9 @@ bool GetTransaction(const uint256 &hash, CTransactionRef &txOut, const Consensus return error("%s: txid mismatch", __func__); return true; } + + // transaction not found in index, nothing more can be done + return false; } if (fAllowSlow) { // use coin database to locate block that contains transaction, and scan it @@ -3132,6 +3135,12 @@ static bool AcceptBlock(const std::shared_ptr<const CBlock>& pblock, CValidation if (pindex->nTx != 0) return true; // This is a previously-processed block that was pruned if (!fHasMoreWork) return true; // Don't process less-work chains if (fTooFarAhead) return true; // Block height is too high + + // Protect against DoS attacks from low-work chains. + // If our tip is behind, a peer could try to send us + // low-work blocks on a fake chain that we would never + // request; don't process these. + if (pindex->nChainWork < nMinimumChainWork) return true; } if (fNewBlock) *fNewBlock = true; @@ -4287,8 +4296,9 @@ bool LoadMempool(void) } int64_t count = 0; - int64_t skipped = 0; + int64_t expired = 0; int64_t failed = 0; + int64_t already_there = 0; int64_t nNow = GetTime(); try { @@ -4319,10 +4329,18 @@ bool LoadMempool(void) if (state.IsValid()) { ++count; } else { - ++failed; + // mempool may contain the transaction already, e.g. from + // wallet(s) having loaded it while we were processing + // mempool transactions; consider these as valid, instead of + // failed, but mark them as 'already there' + if (mempool.exists(tx->GetHash())) { + ++already_there; + } else { + ++failed; + } } } else { - ++skipped; + ++expired; } if (ShutdownRequested()) return false; @@ -4338,7 +4356,7 @@ bool LoadMempool(void) return false; } - LogPrintf("Imported mempool transactions from disk: %i successes, %i failed, %i expired\n", count, failed, skipped); + LogPrintf("Imported mempool transactions from disk: %i succeeded, %i failed, %i expired, %i already there\n", count, failed, expired, already_there); return true; } diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index d66ba48421..459d289a42 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -20,6 +20,40 @@ #include <boost/thread.hpp> +namespace { +//! Make sure database has a unique fileid within the environment. If it +//! doesn't, throw an error. BDB caches do not work properly when more than one +//! open database has the same fileid (values written to one database may show +//! up in reads to other databases). +//! +//! BerkeleyDB generates unique fileids by default +//! (https://docs.oracle.com/cd/E17275_01/html/programmer_reference/program_copy.html), +//! so bitcoin should never create different databases with the same fileid, but +//! this error can be triggered if users manually copy database files. +void CheckUniqueFileid(const CDBEnv& env, const std::string& filename, Db& db) +{ + if (env.IsMock()) return; + + u_int8_t fileid[DB_FILE_ID_LEN]; + int ret = db.get_mpf()->get_fileid(fileid); + if (ret != 0) { + throw std::runtime_error(strprintf("CDB: Can't open database %s (get_fileid failed with %d)", filename, ret)); + } + + for (const auto& item : env.mapDb) { + u_int8_t item_fileid[DB_FILE_ID_LEN]; + if (item.second && item.second->get_mpf()->get_fileid(item_fileid) == 0 && + memcmp(fileid, item_fileid, sizeof(fileid)) == 0) { + const char* item_filename = nullptr; + item.second->get_dbname(&item_filename, nullptr); + throw std::runtime_error(strprintf("CDB: Can't open database %s (duplicates fileid %s from %s)", filename, + HexStr(std::begin(item_fileid), std::end(item_fileid)), + item_filename ? item_filename : "(unknown database)")); + } + } +} +} // namespace + // // CDB // @@ -379,35 +413,34 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb if (!env->Open(GetDataDir())) throw std::runtime_error("CDB: Failed to open database environment."); - strFile = strFilename; - ++env->mapFileUseCount[strFile]; - pdb = env->mapDb[strFile]; + pdb = env->mapDb[strFilename]; if (pdb == nullptr) { int ret; - pdb = new Db(env->dbenv, 0); + std::unique_ptr<Db> pdb_temp(new Db(env->dbenv, 0)); bool fMockDb = env->IsMock(); if (fMockDb) { - DbMpoolFile* mpf = pdb->get_mpf(); + DbMpoolFile* mpf = pdb_temp->get_mpf(); ret = mpf->set_flags(DB_MPOOL_NOFILE, 1); - if (ret != 0) - throw std::runtime_error(strprintf("CDB: Failed to configure for no temp file backing for database %s", strFile)); + if (ret != 0) { + throw std::runtime_error(strprintf("CDB: Failed to configure for no temp file backing for database %s", strFilename)); + } } - ret = pdb->open(nullptr, // Txn pointer - fMockDb ? nullptr : strFile.c_str(), // Filename - fMockDb ? strFile.c_str() : "main", // Logical db name - DB_BTREE, // Database type - nFlags, // Flags + ret = pdb_temp->open(nullptr, // Txn pointer + fMockDb ? nullptr : strFilename.c_str(), // Filename + fMockDb ? strFilename.c_str() : "main", // Logical db name + DB_BTREE, // Database type + nFlags, // Flags 0); if (ret != 0) { - delete pdb; - pdb = nullptr; - --env->mapFileUseCount[strFile]; - strFile = ""; throw std::runtime_error(strprintf("CDB: Error %d, can't open database %s", ret, strFilename)); } + CheckUniqueFileid(*env, strFilename, *pdb_temp); + + pdb = pdb_temp.release(); + env->mapDb[strFilename] = pdb; if (fCreate && !Exists(std::string("version"))) { bool fTmp = fReadOnly; @@ -415,9 +448,9 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb WriteVersion(CLIENT_VERSION); fReadOnly = fTmp; } - - env->mapDb[strFile] = pdb; } + ++env->mapFileUseCount[strFilename]; + strFile = strFilename; } } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index d6ea2a9db7..3ec4a5efb4 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -961,7 +961,7 @@ UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, const int6 pwallet->SetAddressBook(vchAddress, label, "receive"); if (pwallet->HaveKey(vchAddress)) { - return false; + throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script"); } pwallet->mapKeyMetadata[vchAddress].nCreateTime = timestamp; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index e23ef57db0..d6989add89 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3233,7 +3233,7 @@ UniValue rescanblockchain(const JSONRPCRequest& request) "}\n" "\nExamples:\n" + HelpExampleCli("rescanblockchain", "100000 120000") - + HelpExampleRpc("rescanblockchain", "100000 120000") + + HelpExampleRpc("rescanblockchain", "100000, 120000") ); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3672ecea5b..543bef32ad 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3881,7 +3881,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) // Top up the keypool if (!walletInstance->TopUpKeyPool()) { InitError(_("Unable to generate initial keys") += "\n"); - return NULL; + return nullptr; } walletInstance->SetBestChain(chainActive.GetLocator()); diff --git a/test/functional/importmulti.py b/test/functional/importmulti.py index c1a42870ec..a691595f15 100755 --- a/test/functional/importmulti.py +++ b/test/functional/importmulti.py @@ -160,6 +160,18 @@ class ImportMultiTest (BitcoinTestFramework): assert_equal(address_assert['ismine'], True) assert_equal(address_assert['timestamp'], timestamp) + self.log.info("Should not import an address with private key if is already imported") + result = self.nodes[1].importmulti([{ + "scriptPubKey": { + "address": address['address'] + }, + "timestamp": "now", + "keys": [ self.nodes[0].dumpprivkey(address['address']) ] + }]) + assert_equal(result[0]['success'], False) + assert_equal(result[0]['error']['code'], -4) + assert_equal(result[0]['error']['message'], 'The wallet already contains the private key for this address or script') + # Address + Private key + watchonly self.log.info("Should not import an address with private key and with watchonly") address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress()) diff --git a/test/functional/multiwallet.py b/test/functional/multiwallet.py index f55da76819..7a0fbce477 100755 --- a/test/functional/multiwallet.py +++ b/test/functional/multiwallet.py @@ -7,6 +7,7 @@ Verify that a bitcoind node can load multiple wallet files """ import os +import shutil from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_raises_rpc_error @@ -29,6 +30,11 @@ class MultiWalletTest(BitcoinTestFramework): os.mkdir(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w11')) self.assert_start_raises_init_error(0, ['-wallet=w11'], 'Error loading wallet w11. -wallet filename must be a regular file.') + # should not initialize if one wallet is a copy of another + shutil.copyfile(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w2'), + os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w22')) + self.assert_start_raises_init_error(0, ['-wallet=w2', '-wallet=w22'], 'duplicates fileid') + # should not initialize if wallet file is a symlink os.symlink(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w1'), os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w12')) self.assert_start_raises_init_error(0, ['-wallet=w12'], 'Error loading wallet w12. -wallet filename must be a regular file.') diff --git a/test/functional/p2p-acceptblock.py b/test/functional/p2p-acceptblock.py index 5b6429b410..27ae0c27e1 100755 --- a/test/functional/p2p-acceptblock.py +++ b/test/functional/p2p-acceptblock.py @@ -8,17 +8,22 @@ Since behavior differs when receiving unrequested blocks from whitelisted peers versus non-whitelisted peers, this tests the behavior of both (effectively two separate tests running in parallel). -Setup: two nodes, node0 and node1, not connected to each other. Node0 does not +Setup: three nodes, node0+node1+node2, not connected to each other. Node0 does not whitelist localhost, but node1 does. They will each be on their own chain for -this test. +this test. Node2 will have nMinimumChainWork set to 0x10, so it won't process +low-work unrequested blocks. -We have one NodeConn connection to each, test_node and white_node respectively. +We have one NodeConn connection to each, test_node, white_node, and min_work_node, +respectively. The test: 1. Generate one block on each node, to leave IBD. 2. Mine a new block on each tip, and deliver to each node from node's peer. - The tip should advance. + The tip should advance for node0 and node1, but node2 should skip processing + due to nMinimumChainWork. + +Node2 is unused in tests 3-7: 3. Mine a block that forks the previous block, and deliver to each node from corresponding peer. @@ -46,6 +51,10 @@ The test: 7. Send Node0 the missing block again. Node0 should process and the tip should advance. + +8. Test Node2 is able to sync when connected to node0 (which should have sufficient +work on its chain). + """ from test_framework.mininode import * @@ -62,51 +71,60 @@ class AcceptBlockTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True - self.num_nodes = 2 - self.extra_args = [[], ["-whitelist=127.0.0.1"]] + self.num_nodes = 3 + self.extra_args = [[], ["-whitelist=127.0.0.1"], ["-minimumchainwork=0x10"]] def setup_network(self): # Node0 will be used to test behavior of processing unrequested blocks # from peers which are not whitelisted, while Node1 will be used for # the whitelisted case. + # Node2 will be used for non-whitelisted peers to test the interaction + # with nMinimumChainWork. self.setup_nodes() def run_test(self): # Setup the p2p connections and start up the network thread. test_node = NodeConnCB() # connects to node0 (not whitelisted) white_node = NodeConnCB() # connects to node1 (whitelisted) + min_work_node = NodeConnCB() # connects to node2 (not whitelisted) connections = [] connections.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test_node)) connections.append(NodeConn('127.0.0.1', p2p_port(1), self.nodes[1], white_node)) + connections.append(NodeConn('127.0.0.1', p2p_port(2), self.nodes[2], min_work_node)) test_node.add_connection(connections[0]) white_node.add_connection(connections[1]) + min_work_node.add_connection(connections[2]) NetworkThread().start() # Start up network handling in another thread # Test logic begins here test_node.wait_for_verack() white_node.wait_for_verack() + min_work_node.wait_for_verack() - # 1. Have both nodes mine a block (leave IBD) + # 1. Have nodes mine a block (nodes1/2 leave IBD) [ n.generate(1) for n in self.nodes ] tips = [ int("0x" + n.getbestblockhash(), 0) for n in self.nodes ] # 2. Send one block that builds on each tip. - # This should be accepted. + # This should be accepted by nodes 1/2 blocks_h2 = [] # the height 2 blocks on each node's chain block_time = int(time.time()) + 1 - for i in range(2): + for i in range(3): blocks_h2.append(create_block(tips[i], create_coinbase(2), block_time)) blocks_h2[i].solve() block_time += 1 test_node.send_message(msg_block(blocks_h2[0])) white_node.send_message(msg_block(blocks_h2[1])) + min_work_node.send_message(msg_block(blocks_h2[2])) - [ x.sync_with_ping() for x in [test_node, white_node] ] + for x in [test_node, white_node, min_work_node]: + x.sync_with_ping() assert_equal(self.nodes[0].getblockcount(), 2) assert_equal(self.nodes[1].getblockcount(), 2) - self.log.info("First height 2 block accepted by both nodes") + assert_equal(self.nodes[2].getblockcount(), 1) + self.log.info("First height 2 block accepted by node0/node1; correctly rejected by node2") # 3. Send another block that builds on the original tip. blocks_h2f = [] # Blocks at height 2 that fork off the main chain @@ -116,7 +134,8 @@ class AcceptBlockTest(BitcoinTestFramework): test_node.send_message(msg_block(blocks_h2f[0])) white_node.send_message(msg_block(blocks_h2f[1])) - [ x.sync_with_ping() for x in [test_node, white_node] ] + for x in [test_node, white_node]: + x.sync_with_ping() for x in self.nodes[0].getchaintips(): if x['hash'] == blocks_h2f[0].hash: assert_equal(x['status'], "headers-only") @@ -135,7 +154,8 @@ class AcceptBlockTest(BitcoinTestFramework): test_node.send_message(msg_block(blocks_h3[0])) white_node.send_message(msg_block(blocks_h3[1])) - [ x.sync_with_ping() for x in [test_node, white_node] ] + for x in [test_node, white_node]: + x.sync_with_ping() # Since the earlier block was not processed by node0, the new block # can't be fully validated. for x in self.nodes[0].getchaintips(): @@ -217,6 +237,11 @@ class AcceptBlockTest(BitcoinTestFramework): assert_equal(self.nodes[0].getblockcount(), 290) self.log.info("Successfully reorged to longer chain from non-whitelisted peer") + # 8. Connect node2 to node0 and ensure it is able to sync + connect_nodes(self.nodes[0], 2) + sync_blocks([self.nodes[0], self.nodes[2]]) + self.log.info("Successfully synced nodes 2 and 0") + [ c.disconnect_node() for c in connections ] if __name__ == '__main__': diff --git a/test/functional/p2p-fullblocktest.py b/test/functional/p2p-fullblocktest.py index 1d969fc7c1..f19b845a32 100755 --- a/test/functional/p2p-fullblocktest.py +++ b/test/functional/p2p-fullblocktest.py @@ -20,7 +20,7 @@ from test_framework.key import CECKey from test_framework.script import * import struct -class PreviousSpendableOutput(object): +class PreviousSpendableOutput(): def __init__(self, tx = CTransaction(), n = -1): self.tx = tx self.n = n # the output we're spending diff --git a/test/functional/p2p-segwit.py b/test/functional/p2p-segwit.py index a9ef47559b..f803367668 100755 --- a/test/functional/p2p-segwit.py +++ b/test/functional/p2p-segwit.py @@ -89,7 +89,7 @@ class TestNode(NodeConnCB): assert_equal(self.connection.rpc.getbestblockhash() == block.hash, accepted) # Used to keep track of anyone-can-spend outputs that we can use in the tests -class UTXO(object): +class UTXO(): def __init__(self, sha256, n, nValue): self.sha256 = sha256 self.n = n diff --git a/test/functional/replace-by-fee.py b/test/functional/replace-by-fee.py index 269d57775c..815e964848 100755 --- a/test/functional/replace-by-fee.py +++ b/test/functional/replace-by-fee.py @@ -72,8 +72,14 @@ class ReplaceByFeeTest(BitcoinTestFramework): ["-mempoolreplacement=0"]] def run_test(self): + # Leave IBD + self.nodes[0].generate(1) + make_utxo(self.nodes[0], 1*COIN) + # Ensure nodes are synced + self.sync_all() + self.log.info("Running test simple doublespend...") self.test_simple_doublespend() @@ -110,13 +116,18 @@ class ReplaceByFeeTest(BitcoinTestFramework): """Simple doublespend""" tx0_outpoint = make_utxo(self.nodes[0], int(1.1*COIN)) + # make_utxo may have generated a bunch of blocks, so we need to sync + # before we can spend the coins generated, or else the resulting + # transactions might not be accepted by our peers. + self.sync_all() + tx1a = CTransaction() tx1a.vin = [CTxIn(tx0_outpoint, nSequence=0)] tx1a.vout = [CTxOut(1*COIN, CScript([b'a']))] tx1a_hex = txToHex(tx1a) tx1a_txid = self.nodes[0].sendrawtransaction(tx1a_hex, True) - self.sync_all([self.nodes]) + self.sync_all() # Should fail because we haven't changed the fee tx1b = CTransaction() diff --git a/test/functional/sendheaders.py b/test/functional/sendheaders.py index fe577dc20a..60d107b248 100755 --- a/test/functional/sendheaders.py +++ b/test/functional/sendheaders.py @@ -225,6 +225,10 @@ class SendHeadersTest(BitcoinTestFramework): inv_node.wait_for_verack() test_node.wait_for_verack() + # Ensure verack's have been processed by our peer + inv_node.sync_with_ping() + test_node.sync_with_ping() + tip = int(self.nodes[0].getbestblockhash(), 16) # PART 1 diff --git a/test/functional/test_framework/authproxy.py b/test/functional/test_framework/authproxy.py index 747bda309c..bd3a3b3fab 100644 --- a/test/functional/test_framework/authproxy.py +++ b/test/functional/test_framework/authproxy.py @@ -33,24 +33,17 @@ ServiceProxy class: - uses standard Python json lib """ -try: - import http.client as httplib -except ImportError: - import httplib import base64 import decimal +import http.client import json import logging import socket import time -try: - import urllib.parse as urlparse -except ImportError: - import urlparse - -USER_AGENT = "AuthServiceProxy/0.1" +import urllib.parse HTTP_TIMEOUT = 30 +USER_AGENT = "AuthServiceProxy/0.1" log = logging.getLogger("BitcoinRPC") @@ -60,7 +53,7 @@ class JSONRPCException(Exception): errmsg = '%(message)s (%(code)i)' % rpc_error except (KeyError, TypeError): errmsg = '' - Exception.__init__(self, errmsg) + super().__init__(errmsg) self.error = rpc_error @@ -69,28 +62,18 @@ def EncodeDecimal(o): return str(o) raise TypeError(repr(o) + " is not JSON serializable") -class AuthServiceProxy(object): +class AuthServiceProxy(): __id_count = 0 # ensure_ascii: escape unicode as \uXXXX, passed to json.dumps def __init__(self, service_url, service_name=None, timeout=HTTP_TIMEOUT, connection=None, ensure_ascii=True): self.__service_url = service_url self._service_name = service_name - self.ensure_ascii = ensure_ascii # can be toggled on the fly by tests - self.__url = urlparse.urlparse(service_url) - if self.__url.port is None: - port = 80 - else: - port = self.__url.port - (user, passwd) = (self.__url.username, self.__url.password) - try: - user = user.encode('utf8') - except AttributeError: - pass - try: - passwd = passwd.encode('utf8') - except AttributeError: - pass + self.ensure_ascii = ensure_ascii # can be toggled on the fly by tests + self.__url = urllib.parse.urlparse(service_url) + port = 80 if self.__url.port is None else self.__url.port + user = None if self.__url.username is None else self.__url.username.encode('utf8') + passwd = None if self.__url.password is None else self.__url.password.encode('utf8') authpair = user + b':' + passwd self.__auth_header = b'Basic ' + base64.b64encode(authpair) @@ -98,11 +81,9 @@ class AuthServiceProxy(object): # Callables re-use the connection of the original proxy self.__conn = connection elif self.__url.scheme == 'https': - self.__conn = httplib.HTTPSConnection(self.__url.hostname, port, - timeout=timeout) + self.__conn = http.client.HTTPSConnection(self.__url.hostname, port, timeout=timeout) else: - self.__conn = httplib.HTTPConnection(self.__url.hostname, port, - timeout=timeout) + self.__conn = http.client.HTTPConnection(self.__url.hostname, port, timeout=timeout) def __getattr__(self, name): if name.startswith('__') and name.endswith('__'): @@ -124,14 +105,14 @@ class AuthServiceProxy(object): try: self.__conn.request(method, path, postdata, headers) return self._get_response() - except httplib.BadStatusLine as e: - if e.line == "''": # if connection was closed, try again + except http.client.BadStatusLine as e: + if e.line == "''": # if connection was closed, try again self.__conn.close() self.__conn.request(method, path, postdata, headers) return self._get_response() else: raise - except (BrokenPipeError,ConnectionResetError): + except (BrokenPipeError, ConnectionResetError): # Python 3.5+ raises BrokenPipeError instead of BadStatusLine when the connection was reset # ConnectionResetError happens on FreeBSD with Python 3.4 self.__conn.close() @@ -141,8 +122,8 @@ class AuthServiceProxy(object): def get_request(self, *args, **argsn): AuthServiceProxy.__id_count += 1 - log.debug("-%s-> %s %s"%(AuthServiceProxy.__id_count, self._service_name, - json.dumps(args, default=EncodeDecimal, ensure_ascii=self.ensure_ascii))) + log.debug("-%s-> %s %s" % (AuthServiceProxy.__id_count, self._service_name, + json.dumps(args, default=EncodeDecimal, ensure_ascii=self.ensure_ascii))) if args and argsn: raise ValueError('Cannot handle both named and positional arguments') return {'version': '1.1', @@ -163,7 +144,7 @@ class AuthServiceProxy(object): def batch(self, rpc_call_list): postdata = json.dumps(list(rpc_call_list), default=EncodeDecimal, ensure_ascii=self.ensure_ascii) - log.debug("--> "+postdata) + log.debug("--> " + postdata) return self._request('POST', self.__url.path, postdata.encode('utf-8')) def _get_response(self): @@ -190,9 +171,9 @@ class AuthServiceProxy(object): response = json.loads(responsedata, parse_float=decimal.Decimal) elapsed = time.time() - req_start_time if "error" in response and response["error"] is None: - log.debug("<-%s- [%.6f] %s"%(response["id"], elapsed, json.dumps(response["result"], default=EncodeDecimal, ensure_ascii=self.ensure_ascii))) + log.debug("<-%s- [%.6f] %s" % (response["id"], elapsed, json.dumps(response["result"], default=EncodeDecimal, ensure_ascii=self.ensure_ascii))) else: - log.debug("<-- [%.6f] %s"%(elapsed,responsedata)) + log.debug("<-- [%.6f] %s" % (elapsed, responsedata)) return response def __truediv__(self, relative_uri): diff --git a/test/functional/test_framework/blockstore.py b/test/functional/test_framework/blockstore.py index 4b2170a03f..ad04722488 100644 --- a/test/functional/test_framework/blockstore.py +++ b/test/functional/test_framework/blockstore.py @@ -10,7 +10,7 @@ import dbm.dumb as dbmd logger = logging.getLogger("TestFramework.blockstore") -class BlockStore(object): +class BlockStore(): """BlockStore helper class. BlockStore keeps a map of blocks and implements helper functions for @@ -127,7 +127,7 @@ class BlockStore(object): locator.vHave = r return locator -class TxStore(object): +class TxStore(): def __init__(self, datadir): self.txDB = dbmd.open(datadir + "/transactions", 'c') diff --git a/test/functional/test_framework/comptool.py b/test/functional/test_framework/comptool.py index bfbc0c3b03..b0417e02d8 100755 --- a/test/functional/test_framework/comptool.py +++ b/test/functional/test_framework/comptool.py @@ -27,7 +27,7 @@ logger=logging.getLogger("TestFramework.comptool") global mininode_lock -class RejectResult(object): +class RejectResult(): """Outcome that expects rejection of a transaction or block.""" def __init__(self, code, reason=b''): self.code = code @@ -156,13 +156,13 @@ class TestNode(NodeConnCB): # across all connections. (If outcome of final tx is specified as true # or false, then only the last tx is tested against outcome.) -class TestInstance(object): +class TestInstance(): def __init__(self, objects=None, sync_every_block=True, sync_every_tx=False): self.blocks_and_transactions = objects if objects else [] self.sync_every_block = sync_every_block self.sync_every_tx = sync_every_tx -class TestManager(object): +class TestManager(): def __init__(self, testgen, datadir): self.test_generator = testgen diff --git a/test/functional/test_framework/coverage.py b/test/functional/test_framework/coverage.py index 84049e76bc..ddc3c515b2 100644 --- a/test/functional/test_framework/coverage.py +++ b/test/functional/test_framework/coverage.py @@ -14,7 +14,7 @@ import os REFERENCE_FILENAME = 'rpc_interface.txt' -class AuthServiceProxyWrapper(object): +class AuthServiceProxyWrapper(): """ An object that wraps AuthServiceProxy to record specific RPC calls. diff --git a/test/functional/test_framework/key.py b/test/functional/test_framework/key.py index 85a6158a2f..aa91fb5b0d 100644 --- a/test/functional/test_framework/key.py +++ b/test/functional/test_framework/key.py @@ -84,7 +84,7 @@ def _check_result(val, func, args): ssl.EC_KEY_new_by_curve_name.restype = ctypes.c_void_p ssl.EC_KEY_new_by_curve_name.errcheck = _check_result -class CECKey(object): +class CECKey(): """Wrapper around OpenSSL's EC_KEY""" POINT_CONVERSION_COMPRESSED = 2 diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py index c6f596156a..345ecfe76d 100755 --- a/test/functional/test_framework/mininode.py +++ b/test/functional/test_framework/mininode.py @@ -219,7 +219,7 @@ def ToHex(obj): # Objects that map to bitcoind objects, which can be serialized/deserialized -class CAddress(object): +class CAddress(): def __init__(self): self.nServices = 1 self.pchReserved = b"\x00" * 10 + b"\xff" * 2 @@ -246,7 +246,7 @@ class CAddress(object): MSG_WITNESS_FLAG = 1<<30 -class CInv(object): +class CInv(): typemap = { 0: "Error", 1: "TX", @@ -275,7 +275,7 @@ class CInv(object): % (self.typemap[self.type], self.hash) -class CBlockLocator(object): +class CBlockLocator(): def __init__(self): self.nVersion = MY_VERSION self.vHave = [] @@ -295,7 +295,7 @@ class CBlockLocator(object): % (self.nVersion, repr(self.vHave)) -class COutPoint(object): +class COutPoint(): def __init__(self, hash=0, n=0): self.hash = hash self.n = n @@ -314,7 +314,7 @@ class COutPoint(object): return "COutPoint(hash=%064x n=%i)" % (self.hash, self.n) -class CTxIn(object): +class CTxIn(): def __init__(self, outpoint=None, scriptSig=b"", nSequence=0): if outpoint is None: self.prevout = COutPoint() @@ -342,7 +342,7 @@ class CTxIn(object): self.nSequence) -class CTxOut(object): +class CTxOut(): def __init__(self, nValue=0, scriptPubKey=b""): self.nValue = nValue self.scriptPubKey = scriptPubKey @@ -363,7 +363,7 @@ class CTxOut(object): bytes_to_hex_str(self.scriptPubKey)) -class CScriptWitness(object): +class CScriptWitness(): def __init__(self): # stack is a vector of strings self.stack = [] @@ -378,7 +378,7 @@ class CScriptWitness(object): return True -class CTxInWitness(object): +class CTxInWitness(): def __init__(self): self.scriptWitness = CScriptWitness() @@ -395,7 +395,7 @@ class CTxInWitness(object): return self.scriptWitness.is_null() -class CTxWitness(object): +class CTxWitness(): def __init__(self): self.vtxinwit = [] @@ -423,7 +423,7 @@ class CTxWitness(object): return True -class CTransaction(object): +class CTransaction(): def __init__(self, tx=None): if tx is None: self.nVersion = 1 @@ -526,7 +526,7 @@ class CTransaction(object): % (self.nVersion, repr(self.vin), repr(self.vout), repr(self.wit), self.nLockTime) -class CBlockHeader(object): +class CBlockHeader(): def __init__(self, header=None): if header is None: self.set_null() @@ -666,7 +666,7 @@ class CBlock(CBlockHeader): time.ctime(self.nTime), self.nBits, self.nNonce, repr(self.vtx)) -class CUnsignedAlert(object): +class CUnsignedAlert(): def __init__(self): self.nVersion = 1 self.nRelayUntil = 0 @@ -721,7 +721,7 @@ class CUnsignedAlert(object): self.strComment, self.strStatusBar, self.strReserved) -class CAlert(object): +class CAlert(): def __init__(self): self.vchMsg = b"" self.vchSig = b"" @@ -741,7 +741,7 @@ class CAlert(object): % (len(self.vchMsg), len(self.vchSig)) -class PrefilledTransaction(object): +class PrefilledTransaction(): def __init__(self, index=0, tx = None): self.index = index self.tx = tx @@ -767,7 +767,7 @@ class PrefilledTransaction(object): return "PrefilledTransaction(index=%d, tx=%s)" % (self.index, repr(self.tx)) # This is what we send on the wire, in a cmpctblock message. -class P2PHeaderAndShortIDs(object): +class P2PHeaderAndShortIDs(): def __init__(self): self.header = CBlockHeader() self.nonce = 0 @@ -819,7 +819,7 @@ def calculate_shortid(k0, k1, tx_hash): # This version gets rid of the array lengths, and reinterprets the differential # encoding into indices that can be used for lookup. -class HeaderAndShortIDs(object): +class HeaderAndShortIDs(): def __init__(self, p2pheaders_and_shortids = None): self.header = CBlockHeader() self.nonce = 0 @@ -880,7 +880,7 @@ class HeaderAndShortIDs(object): return "HeaderAndShortIDs(header=%s, nonce=%d, shortids=%s, prefilledtxn=%s" % (repr(self.header), self.nonce, repr(self.shortids), repr(self.prefilled_txn)) -class BlockTransactionsRequest(object): +class BlockTransactionsRequest(): def __init__(self, blockhash=0, indexes = None): self.blockhash = blockhash @@ -920,7 +920,7 @@ class BlockTransactionsRequest(object): return "BlockTransactionsRequest(hash=%064x indexes=%s)" % (self.blockhash, repr(self.indexes)) -class BlockTransactions(object): +class BlockTransactions(): def __init__(self, blockhash=0, transactions = None): self.blockhash = blockhash @@ -944,7 +944,7 @@ class BlockTransactions(object): # Objects that correspond to messages on the wire -class msg_version(object): +class msg_version(): command = b"version" def __init__(self): @@ -1012,7 +1012,7 @@ class msg_version(object): self.strSubVer, self.nStartingHeight, self.nRelay) -class msg_verack(object): +class msg_verack(): command = b"verack" def __init__(self): @@ -1028,7 +1028,7 @@ class msg_verack(object): return "msg_verack()" -class msg_addr(object): +class msg_addr(): command = b"addr" def __init__(self): @@ -1044,7 +1044,7 @@ class msg_addr(object): return "msg_addr(addrs=%s)" % (repr(self.addrs)) -class msg_alert(object): +class msg_alert(): command = b"alert" def __init__(self): @@ -1063,7 +1063,7 @@ class msg_alert(object): return "msg_alert(alert=%s)" % (repr(self.alert), ) -class msg_inv(object): +class msg_inv(): command = b"inv" def __init__(self, inv=None): @@ -1082,7 +1082,7 @@ class msg_inv(object): return "msg_inv(inv=%s)" % (repr(self.inv)) -class msg_getdata(object): +class msg_getdata(): command = b"getdata" def __init__(self, inv=None): @@ -1098,7 +1098,7 @@ class msg_getdata(object): return "msg_getdata(inv=%s)" % (repr(self.inv)) -class msg_getblocks(object): +class msg_getblocks(): command = b"getblocks" def __init__(self): @@ -1121,7 +1121,7 @@ class msg_getblocks(object): % (repr(self.locator), self.hashstop) -class msg_tx(object): +class msg_tx(): command = b"tx" def __init__(self, tx=CTransaction()): @@ -1142,7 +1142,7 @@ class msg_witness_tx(msg_tx): return self.tx.serialize_with_witness() -class msg_block(object): +class msg_block(): command = b"block" def __init__(self, block=None): @@ -1162,7 +1162,7 @@ class msg_block(object): # for cases where a user needs tighter control over what is sent over the wire # note that the user must supply the name of the command, and the data -class msg_generic(object): +class msg_generic(): def __init__(self, command, data=None): self.command = command self.data = data @@ -1179,7 +1179,7 @@ class msg_witness_block(msg_block): r = self.block.serialize(with_witness=True) return r -class msg_getaddr(object): +class msg_getaddr(): command = b"getaddr" def __init__(self): @@ -1195,7 +1195,7 @@ class msg_getaddr(object): return "msg_getaddr()" -class msg_ping_prebip31(object): +class msg_ping_prebip31(): command = b"ping" def __init__(self): @@ -1211,7 +1211,7 @@ class msg_ping_prebip31(object): return "msg_ping() (pre-bip31)" -class msg_ping(object): +class msg_ping(): command = b"ping" def __init__(self, nonce=0): @@ -1229,7 +1229,7 @@ class msg_ping(object): return "msg_ping(nonce=%08x)" % self.nonce -class msg_pong(object): +class msg_pong(): command = b"pong" def __init__(self, nonce=0): @@ -1247,7 +1247,7 @@ class msg_pong(object): return "msg_pong(nonce=%08x)" % self.nonce -class msg_mempool(object): +class msg_mempool(): command = b"mempool" def __init__(self): @@ -1262,7 +1262,7 @@ class msg_mempool(object): def __repr__(self): return "msg_mempool()" -class msg_sendheaders(object): +class msg_sendheaders(): command = b"sendheaders" def __init__(self): @@ -1282,7 +1282,7 @@ class msg_sendheaders(object): # number of entries # vector of hashes # hash_stop (hash of last desired block header, 0 to get as many as possible) -class msg_getheaders(object): +class msg_getheaders(): command = b"getheaders" def __init__(self): @@ -1307,7 +1307,7 @@ class msg_getheaders(object): # headers message has # <count> <vector of block headers> -class msg_headers(object): +class msg_headers(): command = b"headers" def __init__(self, headers=None): @@ -1327,7 +1327,7 @@ class msg_headers(object): return "msg_headers(headers=%s)" % repr(self.headers) -class msg_reject(object): +class msg_reject(): command = b"reject" REJECT_MALFORMED = 1 @@ -1358,7 +1358,7 @@ class msg_reject(object): return "msg_reject: %s %d %s [%064x]" \ % (self.message, self.code, self.reason, self.data) -class msg_feefilter(object): +class msg_feefilter(): command = b"feefilter" def __init__(self, feerate=0): @@ -1375,7 +1375,7 @@ class msg_feefilter(object): def __repr__(self): return "msg_feefilter(feerate=%08x)" % self.feerate -class msg_sendcmpct(object): +class msg_sendcmpct(): command = b"sendcmpct" def __init__(self): @@ -1395,7 +1395,7 @@ class msg_sendcmpct(object): def __repr__(self): return "msg_sendcmpct(announce=%s, version=%lu)" % (self.announce, self.version) -class msg_cmpctblock(object): +class msg_cmpctblock(): command = b"cmpctblock" def __init__(self, header_and_shortids = None): @@ -1413,7 +1413,7 @@ class msg_cmpctblock(object): def __repr__(self): return "msg_cmpctblock(HeaderAndShortIDs=%s)" % repr(self.header_and_shortids) -class msg_getblocktxn(object): +class msg_getblocktxn(): command = b"getblocktxn" def __init__(self): @@ -1431,7 +1431,7 @@ class msg_getblocktxn(object): def __repr__(self): return "msg_getblocktxn(block_txn_request=%s)" % (repr(self.block_txn_request)) -class msg_blocktxn(object): +class msg_blocktxn(): command = b"blocktxn" def __init__(self): @@ -1454,7 +1454,7 @@ class msg_witness_blocktxn(msg_blocktxn): r += self.block_transactions.serialize(with_witness=True) return r -class NodeConnCB(object): +class NodeConnCB(): """Callback and helper functions for P2P connection to a bitcoind node. Individual testcases should subclass this and override the on_* methods @@ -1615,7 +1615,6 @@ class NodeConnCB(object): test_function = lambda: self.last_message.get("pong") and self.last_message["pong"].nonce == self.ping_counter wait_until(test_function, timeout=timeout, lock=mininode_lock) self.ping_counter += 1 - return True # The actual NodeConn class # This class provides an interface for a p2p connection to a specified node diff --git a/test/functional/test_framework/script.py b/test/functional/test_framework/script.py index 8f5339a02a..a4c046bd3d 100644 --- a/test/functional/test_framework/script.py +++ b/test/functional/test_framework/script.py @@ -370,7 +370,7 @@ class CScriptTruncatedPushDataError(CScriptInvalidError): super(CScriptTruncatedPushDataError, self).__init__(msg) # This is used, eg, for blockchain heights in coinbase scripts (bip34) -class CScriptNum(object): +class CScriptNum(): def __init__(self, d=0): self.value = d diff --git a/test/functional/test_framework/socks5.py b/test/functional/test_framework/socks5.py index 0070844168..7b40c47fbf 100644 --- a/test/functional/test_framework/socks5.py +++ b/test/functional/test_framework/socks5.py @@ -31,7 +31,7 @@ def recvall(s, n): return rv ### Implementation classes -class Socks5Configuration(object): +class Socks5Configuration(): """Proxy configuration.""" def __init__(self): self.addr = None # Bind address (must be set) @@ -39,7 +39,7 @@ class Socks5Configuration(object): self.unauth = False # Support unauthenticated self.auth = False # Support authentication -class Socks5Command(object): +class Socks5Command(): """Information about an incoming socks5 command.""" def __init__(self, cmd, atyp, addr, port, username, password): self.cmd = cmd # Command (one of Command.*) @@ -51,7 +51,7 @@ class Socks5Command(object): def __repr__(self): return 'Socks5Command(%s,%s,%s,%s,%s,%s)' % (self.cmd, self.atyp, self.addr, self.port, self.username, self.password) -class Socks5Connection(object): +class Socks5Connection(): def __init__(self, serv, conn, peer): self.serv = serv self.conn = conn @@ -122,7 +122,7 @@ class Socks5Connection(object): finally: self.conn.close() -class Socks5Server(object): +class Socks5Server(): def __init__(self, conf): self.conf = conf self.s = socket.socket(conf.af) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 381513ab9e..8df50474f3 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -43,7 +43,7 @@ TEST_EXIT_PASSED = 0 TEST_EXIT_FAILED = 1 TEST_EXIT_SKIPPED = 77 -class BitcoinTestFramework(object): +class BitcoinTestFramework(): """Base class for a bitcoin test script. Individual bitcoin test scripts should subclass this class and override the set_test_params() and run_test() methods. @@ -102,8 +102,11 @@ class BitcoinTestFramework(object): check_json_precision() + self.options.cachedir = os.path.abspath(self.options.cachedir) + # Set up temp directory and start logging if self.options.tmpdir: + self.options.tmpdir = os.path.abspath(self.options.tmpdir) os.makedirs(self.options.tmpdir, exist_ok=False) else: self.options.tmpdir = tempfile.mkdtemp(prefix="test") diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 8c4651f6e0..80a5ffefb4 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -124,6 +124,7 @@ BASE_SCRIPTS= [ 'resendwallettransactions.py', 'minchainwork.py', 'p2p-fingerprint.py', + 'uacomment.py', ] EXTENDED_SCRIPTS = [ @@ -459,7 +460,7 @@ def check_script_list(src_dir): # On travis this warning is an error to prevent merging incomplete commits into master sys.exit(1) -class RPCCoverage(object): +class RPCCoverage(): """ Coverage reporting utilities for test_runner. diff --git a/test/functional/uacomment.py b/test/functional/uacomment.py new file mode 100755 index 0000000000..0b2c64ab69 --- /dev/null +++ b/test/functional/uacomment.py @@ -0,0 +1,35 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test the -uacomment option.""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + +class UacommentTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + self.setup_clean_chain = True + + def run_test(self): + self.log.info("test multiple -uacomment") + test_uacomment = self.nodes[0].getnetworkinfo()["subversion"][-12:-1] + assert_equal(test_uacomment, "(testnode0)") + + self.restart_node(0, ["-uacomment=foo"]) + foo_uacomment = self.nodes[0].getnetworkinfo()["subversion"][-17:-1] + assert_equal(foo_uacomment, "(testnode0; foo)") + + self.log.info("test -uacomment max length") + self.stop_node(0) + expected = "Total length of network version string (286) exceeds maximum length (256). Reduce the number or size of uacomments." + self.assert_start_raises_init_error(0, ["-uacomment=" + 'a' * 256], expected) + + self.log.info("test -uacomment unsafe characters") + for unsafe_char in ['/', ':', '(', ')']: + expected = "User Agent comment (" + unsafe_char + ") contains unsafe characters" + self.assert_start_raises_init_error(0, ["-uacomment=" + unsafe_char], expected) + +if __name__ == '__main__': + UacommentTest().main() diff --git a/test/functional/zmq_test.py b/test/functional/zmq_test.py index 382ef5bae2..165f9192dd 100755 --- a/test/functional/zmq_test.py +++ b/test/functional/zmq_test.py @@ -2,7 +2,7 @@ # Copyright (c) 2015-2016 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. -"""Test the ZMQ API.""" +"""Test the ZMQ notification interface.""" import configparser import os import struct @@ -13,6 +13,25 @@ from test_framework.util import (assert_equal, hash256, ) +class ZMQSubscriber: + def __init__(self, socket, topic): + self.sequence = 0 + self.socket = socket + self.topic = topic + + import zmq + self.socket.setsockopt(zmq.SUBSCRIBE, self.topic) + + def receive(self): + topic, body, seq = self.socket.recv_multipart() + # Topic should match the subscriber topic. + assert_equal(topic, self.topic) + # Sequence should be incremental. + assert_equal(struct.unpack('<I', seq)[-1], self.sequence) + self.sequence += 1 + return body + + class ZMQTest (BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 @@ -24,26 +43,33 @@ class ZMQTest (BitcoinTestFramework): except ImportError: raise SkipTest("python3-zmq module not available.") - # Check that bitcoin has been built with ZMQ enabled + # Check that bitcoin has been built with ZMQ enabled. config = configparser.ConfigParser() if not self.options.configfile: - self.options.configfile = os.path.dirname(__file__) + "/../config.ini" + self.options.configfile = os.path.abspath(os.path.join(os.path.dirname(__file__), "../config.ini")) config.read_file(open(self.options.configfile)) if not config["components"].getboolean("ENABLE_ZMQ"): raise SkipTest("bitcoind has not been built with zmq enabled.") - self.zmqContext = zmq.Context() - self.zmqSubSocket = self.zmqContext.socket(zmq.SUB) - self.zmqSubSocket.set(zmq.RCVTIMEO, 60000) - self.zmqSubSocket.setsockopt(zmq.SUBSCRIBE, b"hashblock") - self.zmqSubSocket.setsockopt(zmq.SUBSCRIBE, b"hashtx") - self.zmqSubSocket.setsockopt(zmq.SUBSCRIBE, b"rawblock") - self.zmqSubSocket.setsockopt(zmq.SUBSCRIBE, b"rawtx") - ip_address = "tcp://127.0.0.1:28332" - self.zmqSubSocket.connect(ip_address) - self.extra_args = [['-zmqpubhashblock=%s' % ip_address, '-zmqpubhashtx=%s' % ip_address, - '-zmqpubrawblock=%s' % ip_address, '-zmqpubrawtx=%s' % ip_address], []] + # Initialize ZMQ context and socket. + # All messages are received in the same socket which means + # that this test fails if the publishing order changes. + # Note that the publishing order is not defined in the documentation and + # is subject to change. + address = "tcp://127.0.0.1:28332" + self.zmq_context = zmq.Context() + socket = self.zmq_context.socket(zmq.SUB) + socket.set(zmq.RCVTIMEO, 60000) + socket.connect(address) + + # Subscribe to all available topics. + self.hashblock = ZMQSubscriber(socket, b"hashblock") + self.hashtx = ZMQSubscriber(socket, b"hashtx") + self.rawblock = ZMQSubscriber(socket, b"rawblock") + self.rawtx = ZMQSubscriber(socket, b"rawtx") + + self.extra_args = [["-zmqpub%s=%s" % (sub.topic.decode(), address) for sub in [self.hashblock, self.hashtx, self.rawblock, self.rawtx]], []] self.add_nodes(self.num_nodes, self.extra_args) self.start_nodes() @@ -51,103 +77,45 @@ class ZMQTest (BitcoinTestFramework): try: self._zmq_test() finally: - # Destroy the zmq context - self.log.debug("Destroying zmq context") - self.zmqContext.destroy(linger=None) + # Destroy the ZMQ context. + self.log.debug("Destroying ZMQ context") + self.zmq_context.destroy(linger=None) def _zmq_test(self): - genhashes = self.nodes[0].generate(1) + num_blocks = 5 + self.log.info("Generate %(n)d blocks (and %(n)d coinbase txes)" % {"n": num_blocks}) + genhashes = self.nodes[0].generate(num_blocks) self.sync_all() - self.log.info("Wait for tx") - msg = self.zmqSubSocket.recv_multipart() - topic = msg[0] - assert_equal(topic, b"hashtx") - txhash = msg[1] - msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, 0) # must be sequence 0 on hashtx - - # rawtx - msg = self.zmqSubSocket.recv_multipart() - topic = msg[0] - assert_equal(topic, b"rawtx") - body = msg[1] - msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, 0) # must be sequence 0 on rawtx - - # Check that the rawtx hashes to the hashtx - assert_equal(hash256(body), txhash) - - self.log.info("Wait for block") - msg = self.zmqSubSocket.recv_multipart() - topic = msg[0] - assert_equal(topic, b"hashblock") - body = msg[1] - msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, 0) # must be sequence 0 on hashblock - blkhash = bytes_to_hex_str(body) - assert_equal(genhashes[0], blkhash) # blockhash from generate must be equal to the hash received over zmq - - # rawblock - msg = self.zmqSubSocket.recv_multipart() - topic = msg[0] - assert_equal(topic, b"rawblock") - body = msg[1] - msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, 0) #must be sequence 0 on rawblock - - # Check the hash of the rawblock's header matches generate - assert_equal(genhashes[0], bytes_to_hex_str(hash256(body[:80]))) - - self.log.info("Generate 10 blocks (and 10 coinbase txes)") - n = 10 - genhashes = self.nodes[1].generate(n) - self.sync_all() + for x in range(num_blocks): + # Should receive the coinbase txid. + txid = self.hashtx.receive() + + # Should receive the coinbase raw transaction. + hex = self.rawtx.receive() + assert_equal(hash256(hex), txid) - zmqHashes = [] - zmqRawHashed = [] - blockcount = 0 - for x in range(n * 4): - msg = self.zmqSubSocket.recv_multipart() - topic = msg[0] - body = msg[1] - if topic == b"hashblock": - zmqHashes.append(bytes_to_hex_str(body)) - msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, blockcount + 1) - blockcount += 1 - if topic == b"rawblock": - zmqRawHashed.append(bytes_to_hex_str(hash256(body[:80]))) - msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, blockcount) - - for x in range(n): - assert_equal(genhashes[x], zmqHashes[x]) # blockhash from generate must be equal to the hash received over zmq - assert_equal(genhashes[x], zmqRawHashed[x]) + # Should receive the generated block hash. + hash = bytes_to_hex_str(self.hashblock.receive()) + assert_equal(genhashes[x], hash) + # The block should only have the coinbase txid. + assert_equal([bytes_to_hex_str(txid)], self.nodes[1].getblock(hash)["tx"]) + + # Should receive the generated raw block. + block = self.rawblock.receive() + assert_equal(genhashes[x], bytes_to_hex_str(hash256(block[:80]))) self.log.info("Wait for tx from second node") - # test tx from a second node - hashRPC = self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1.0) + payment_txid = self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1.0) self.sync_all() - # now we should receive a zmq msg because the tx was broadcast - msg = self.zmqSubSocket.recv_multipart() - topic = msg[0] - assert_equal(topic, b"hashtx") - body = msg[1] - hashZMQ = bytes_to_hex_str(body) - msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, blockcount + 1) - - msg = self.zmqSubSocket.recv_multipart() - topic = msg[0] - assert_equal(topic, b"rawtx") - body = msg[1] - hashedZMQ = bytes_to_hex_str(hash256(body)) - msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, blockcount+1) - assert_equal(hashRPC, hashZMQ) # txid from sendtoaddress must be equal to the hash received over zmq - assert_equal(hashRPC, hashedZMQ) + # Should receive the broadcasted txid. + txid = self.hashtx.receive() + assert_equal(payment_txid, bytes_to_hex_str(txid)) + + # Should receive the broadcasted raw transaction. + hex = self.rawtx.receive() + assert_equal(payment_txid, bytes_to_hex_str(hash256(hex))) if __name__ == '__main__': ZMQTest().main() |