diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index 6e13fd173b..b6c07460f7 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -45,6 +45,7 @@ bench_bench_dash_SOURCES = \ bench/pool.cpp \ bench/rpc_blockchain.cpp \ bench/rpc_mempool.cpp \ + bench/strencodings.cpp \ bench/util_time.cpp \ bench/base58.cpp \ bench/bech32.cpp \ diff --git a/src/bench/strencodings.cpp b/src/bench/strencodings.cpp new file mode 100644 index 0000000000..4be7cd6d6f --- /dev/null +++ b/src/bench/strencodings.cpp @@ -0,0 +1,18 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include + +static void HexStrBench(benchmark::Bench& bench) +{ + auto const& data = benchmark::data::block813851; + bench.batch(data.size()).unit("byte").run([&] { + auto hex = HexStr(data); + ankerl::nanobench::doNotOptimizeAway(hex); + }); +} + +BENCHMARK(HexStrBench); diff --git a/src/netgroup.cpp b/src/netgroup.cpp index 3dc59725e1..c3162d0b28 100644 --- a/src/netgroup.cpp +++ b/src/netgroup.cpp @@ -71,7 +71,7 @@ std::vector NetGroupManager::GetGroup(const CNetAddr& address) co // ...for the last byte, push nBits and for the rest of the byte push 1's if (nBits > 0) { assert(num_bytes < addr_bytes.size()); - vchRet.push_back(addr_bytes[num_bytes] | ((1 << (8 - nBits)) - 1)); + vchRet.push_back(addr_bytes[num_bytes + nStartByte] | ((1 << (8 - nBits)) - 1)); } return vchRet; diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index af50ae0129..47b5801c51 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -156,10 +156,11 @@ static bool InitSettings() std::vector errors; if (!gArgs.ReadSettingsFile(&errors)) { - bilingual_str error = _("Settings file could not be read"); - InitError(Untranslated(strprintf("%s:\n%s\n", error.original, MakeUnorderedList(errors)))); + std::string error = QT_TRANSLATE_NOOP("dash-core", "Settings file could not be read"); + std::string error_translated = QCoreApplication::translate("dash-core", error.c_str()).toStdString(); + InitError(Untranslated(strprintf("%s:\n%s\n", error, MakeUnorderedList(errors)))); - QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error.translated)), QMessageBox::Reset | QMessageBox::Abort); + QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error_translated)), QMessageBox::Reset | QMessageBox::Abort); /*: Explanatory text shown on startup when the settings file cannot be read. Prompts user to make a choice between resetting or aborting. */ messagebox.setInformativeText(QObject::tr("Do you want to reset settings to default values, or to abort without making changes?")); @@ -178,10 +179,11 @@ static bool InitSettings() errors.clear(); if (!gArgs.WriteSettingsFile(&errors)) { - bilingual_str error = _("Settings file could not be written"); - InitError(Untranslated(strprintf("%s:\n%s\n", error.original, MakeUnorderedList(errors)))); + std::string error = QT_TRANSLATE_NOOP("dash-core", "Settings file could not be written"); + std::string error_translated = QCoreApplication::translate("dash-core", error.c_str()).toStdString(); + InitError(Untranslated(strprintf("%s:\n%s\n", error, MakeUnorderedList(errors)))); - QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error.translated)), QMessageBox::Ok); + QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error_translated)), QMessageBox::Ok); /*: Explanatory text shown on startup when the settings file could not be written. Prompts user to check that we have the ability to write to the file. Explains that the user has the option of running without a settings file.*/ diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index 8a88c6161b..4ada490a7a 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -67,7 +67,7 @@ CoinControlDialog::CoinControlDialog(CCoinControl& coin_control, WalletModel* _m contextMenu->addAction(tr("&Copy address"), this, &CoinControlDialog::copyAddress); contextMenu->addAction(tr("Copy &label"), this, &CoinControlDialog::copyLabel); contextMenu->addAction(tr("Copy &amount"), this, &CoinControlDialog::copyAmount); - copyTransactionHashAction = contextMenu->addAction(tr("Copy transaction &ID"), this, &CoinControlDialog::copyTransactionHash); + m_copy_transaction_outpoint_action = contextMenu->addAction(tr("Copy transaction &ID and output index"), this, &CoinControlDialog::copyTransactionOutpoint); contextMenu->addSeparator(); lockAction = contextMenu->addAction(tr("L&ock unspent"), this, &CoinControlDialog::lockCoin); unlockAction = contextMenu->addAction(tr("&Unlock unspent"), this, &CoinControlDialog::unlockCoin); @@ -235,7 +235,7 @@ void CoinControlDialog::showMenu(const QPoint &point) // disable some items (like Copy Transaction ID, lock, unlock) for tree roots in context menu if (item->data(COLUMN_ADDRESS, TxHashRole).toString().length() == 64) // transaction hash is 64 characters (this means it is a child node, so it is not a parent node in tree mode) { - copyTransactionHashAction->setEnabled(true); + m_copy_transaction_outpoint_action->setEnabled(true); if (model->wallet().isLockedCoin(COutPoint(uint256S(item->data(COLUMN_ADDRESS, TxHashRole).toString().toStdString()), item->data(COLUMN_ADDRESS, VOutRole).toUInt()))) { lockAction->setEnabled(false); @@ -249,7 +249,7 @@ void CoinControlDialog::showMenu(const QPoint &point) } else // this means click on parent node in tree mode -> disable all { - copyTransactionHashAction->setEnabled(false); + m_copy_transaction_outpoint_action->setEnabled(false); lockAction->setEnabled(false); unlockAction->setEnabled(false); } @@ -283,10 +283,14 @@ void CoinControlDialog::copyAddress() GUIUtil::setClipboard(contextMenuItem->text(COLUMN_ADDRESS)); } -// context menu action: copy transaction id -void CoinControlDialog::copyTransactionHash() +// context menu action: copy transaction id and vout index +void CoinControlDialog::copyTransactionOutpoint() { - GUIUtil::setClipboard(contextMenuItem->data(COLUMN_ADDRESS, TxHashRole).toString()); + const QString address = contextMenuItem->data(COLUMN_ADDRESS, TxHashRole).toString(); + const QString vout = contextMenuItem->data(COLUMN_ADDRESS, VOutRole).toString(); + const QString outpoint = QString("%1:%2").arg(address).arg(vout); + + GUIUtil::setClipboard(outpoint); } // context menu action: lock coin diff --git a/src/qt/coincontroldialog.h b/src/qt/coincontroldialog.h index 3336f2c396..601b4bcb5b 100644 --- a/src/qt/coincontroldialog.h +++ b/src/qt/coincontroldialog.h @@ -59,7 +59,7 @@ private: QMenu *contextMenu; QTreeWidgetItem *contextMenuItem; - QAction *copyTransactionHashAction; + QAction* m_copy_transaction_outpoint_action; QAction *lockAction; QAction *unlockAction; @@ -92,7 +92,7 @@ private Q_SLOTS: void copyAmount(); void copyLabel(); void copyAddress(); - void copyTransactionHash(); + void copyTransactionOutpoint(); void lockCoin(); void unlockCoin(); void clipboardQuantity(); diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index 14bcc39eed..40f74f73e7 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -89,6 +89,8 @@ void ForceActivation(); #endif +using namespace std::chrono_literals; + namespace GUIUtil { static RecursiveMutex cs_css; @@ -1729,6 +1731,16 @@ QString formatDurationStr(std::chrono::seconds dur) return str_list.join(" "); } +QString FormatPeerAge(std::chrono::seconds time_connected) +{ + const auto time_now{GetTime()}; + const auto age{time_now - time_connected}; + if (age >= 24h) return QObject::tr("%1 d").arg(age / 24h); + if (age >= 1h) return QObject::tr("%1 h").arg(age / 1h); + if (age >= 1min) return QObject::tr("%1 m").arg(age / 1min); + return QObject::tr("%1 s").arg(age / 1s); +} + QString formatServicesStr(quint64 mask) { QStringList strList; diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h index 8f3cb7df71..3507043262 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -420,6 +420,9 @@ namespace GUIUtil /** Convert seconds into a QString with days, hours, mins, secs */ QString formatDurationStr(std::chrono::seconds dur); + /** Convert peer connection time to a QString denominated in the most relevant unit. */ + QString FormatPeerAge(std::chrono::seconds time_connected); + /** Format CNodeStats.nServices bitmask into a user-readable string */ QString formatServicesStr(quint64 mask); diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp index ecd305a77d..182ff17b55 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -71,6 +71,8 @@ QVariant PeerTableModel::data(const QModelIndex& index, int role) const switch (column) { case NetNodeId: return (qint64)rec->nodeStats.nodeid; + case Age: + return GUIUtil::FormatPeerAge(rec->nodeStats.m_connected); case Address: // prepend to peer address down-arrow symbol for inbound connection and up-arrow for outbound connection return QString::fromStdString((rec->nodeStats.fInbound ? "↓ " : "↑ ") + rec->nodeStats.m_addr_name); @@ -91,6 +93,7 @@ QVariant PeerTableModel::data(const QModelIndex& index, int role) const } else if (role == Qt::TextAlignmentRole) { switch (column) { case NetNodeId: + case Age: return QVariant(Qt::AlignRight | Qt::AlignVCenter); case Address: return {}; diff --git a/src/qt/peertablemodel.h b/src/qt/peertablemodel.h index 0ff1b5dba7..cf101149fd 100644 --- a/src/qt/peertablemodel.h +++ b/src/qt/peertablemodel.h @@ -47,6 +47,7 @@ public: enum ColumnIndex { NetNodeId = 0, + Age, Address, ConnectionType, Network, @@ -84,6 +85,9 @@ private: /*: Title of Peers Table column which contains a unique number used to identify a connection. */ tr("Peer"), + /*: Title of Peers Table column which indicates the duration (length of time) + since the peer connection started. */ + tr("Age"), /*: Title of Peers Table column which contains the IP/Onion/I2P address of the connected peer. */ tr("Address"), diff --git a/src/qt/peertablesortproxy.cpp b/src/qt/peertablesortproxy.cpp index ad966fbcab..6d716366fa 100644 --- a/src/qt/peertablesortproxy.cpp +++ b/src/qt/peertablesortproxy.cpp @@ -24,6 +24,8 @@ bool PeerTableSortProxy::lessThan(const QModelIndex& left_index, const QModelInd switch (static_cast(left_index.column())) { case PeerTableModel::NetNodeId: return left_stats.nodeid < right_stats.nodeid; + case PeerTableModel::Age: + return left_stats.m_connected > right_stats.m_connected; case PeerTableModel::Address: return left_stats.m_addr_name.compare(right_stats.m_addr_name) < 0; case PeerTableModel::ConnectionType: diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index b76b05bced..cd9f2bddcd 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -713,11 +713,17 @@ void RPCConsole::setClientModel(ClientModel *model, int bestblock_height, int64_ ui->peerWidget->setColumnWidth(PeerTableModel::Subversion, SUBVERSION_COLUMN_WIDTH); ui->peerWidget->setColumnWidth(PeerTableModel::Ping, PING_COLUMN_WIDTH); } + ui->peerWidget->horizontalHeader()->setSectionResizeMode(PeerTableModel::Age, QHeaderView::ResizeToContents); ui->peerWidget->horizontalHeader()->setStretchLastSection(true); ui->peerWidget->setItemDelegateForColumn(PeerTableModel::NetNodeId, new PeerIdViewDelegate(this)); // create peer table context menu peersTableContextMenu = new QMenu(this); + //: Context menu action to copy the address of a peer + peersTableContextMenu->addAction(tr("&Copy address"), [this] { + GUIUtil::copyEntryData(ui->peerWidget, PeerTableModel::Address, Qt::DisplayRole); + }); + peersTableContextMenu->addSeparator(); peersTableContextMenu->addAction(tr("&Disconnect"), this, &RPCConsole::disconnectSelectedNode); peersTableContextMenu->addAction(ts.ban_for + " " + tr("1 &hour"), [this] { banSelectedNode(60 * 60); }); peersTableContextMenu->addAction(ts.ban_for + " " + tr("1 d&ay"), [this] { banSelectedNode(60 * 60 * 24); }); @@ -740,10 +746,18 @@ void RPCConsole::setClientModel(ClientModel *model, int bestblock_height, int64_ ui->banlistWidget->setColumnWidth(BanTableModel::Address, BANSUBNET_COLUMN_WIDTH); ui->banlistWidget->setColumnWidth(BanTableModel::Bantime, BANTIME_COLUMN_WIDTH); } + ui->banlistWidget->horizontalHeader()->setSectionResizeMode(BanTableModel::Address, QHeaderView::ResizeToContents); ui->banlistWidget->horizontalHeader()->setStretchLastSection(true); // create ban table context menu banTableContextMenu = new QMenu(this); + /*: Context menu action to copy the IP/Netmask of a banned peer. + IP/Netmask is the combination of a peer's IP address and its Netmask. + For IP address see: https://en.wikipedia.org/wiki/IP_address */ + banTableContextMenu->addAction(tr("&Copy IP/Netmask"), [this] { + GUIUtil::copyEntryData(ui->banlistWidget, BanTableModel::Address, Qt::DisplayRole); + }); + banTableContextMenu->addSeparator(); banTableContextMenu->addAction(tr("&Unban"), this, &RPCConsole::unbanSelectedNode); connect(ui->banlistWidget, &QTableView::customContextMenuRequested, this, &RPCConsole::showBanTableContextMenu); diff --git a/src/qt/splashscreen.cpp b/src/qt/splashscreen.cpp index dbe7aa7e65..005c3bcdb4 100644 --- a/src/qt/splashscreen.cpp +++ b/src/qt/splashscreen.cpp @@ -189,8 +189,8 @@ static void InitMessage(SplashScreen *splash, const std::string &message) static void ShowProgress(SplashScreen *splash, const std::string &title, int nProgress, bool resume_possible) { InitMessage(splash, title + std::string("\n") + - (resume_possible ? _("(press q to shutdown and continue later)").translated - : _("press q to shutdown").translated) + + (resume_possible ? SplashScreen::tr("(press q to shutdown and continue later)").toStdString() + : SplashScreen::tr("press q to shutdown").toStdString()) + strprintf("\n%d", nProgress) + "%"); } diff --git a/src/test/fs_tests.cpp b/src/test/fs_tests.cpp index 4a8803a205..726d19a775 100644 --- a/src/test/fs_tests.cpp +++ b/src/test/fs_tests.cpp @@ -152,7 +152,7 @@ BOOST_AUTO_TEST_CASE(rename) fs::remove(path2); } -#ifndef WIN32 +#ifndef __MINGW64__ // no symlinks on mingw BOOST_AUTO_TEST_CASE(create_directories) { // Test fs::create_directories workaround. @@ -174,7 +174,7 @@ BOOST_AUTO_TEST_CASE(create_directories) fs::remove(symlink); fs::remove(dir); } -#endif // WIN32 +#endif // __MINGW64__ BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 55d1cd50d9..f9897de326 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -207,6 +207,24 @@ BOOST_AUTO_TEST_CASE(util_HexStr) BOOST_CHECK_EQUAL(HexStr(in_s), out_exp); BOOST_CHECK_EQUAL(HexStr(in_b), out_exp); } + + { + auto input = std::string(); + for (size_t i=0; i<256; ++i) { + input.push_back(static_cast(i)); + } + + auto hex = HexStr(input); + BOOST_TEST_REQUIRE(hex.size() == 512); + static constexpr auto hexmap = std::string_view("0123456789abcdef"); + for (size_t i = 0; i < 256; ++i) { + auto upper = hexmap.find(hex[i * 2]); + auto lower = hexmap.find(hex[i * 2 + 1]); + BOOST_TEST_REQUIRE(upper != std::string_view::npos); + BOOST_TEST_REQUIRE(lower != std::string_view::npos); + BOOST_TEST_REQUIRE(i == upper*16 + lower); + } + } } BOOST_AUTO_TEST_CASE(span_write_bytes) diff --git a/src/txmempool.h b/src/txmempool.h index 9cdf1ccc1c..7e570afc3e 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -311,16 +311,6 @@ public: } }; -struct update_lock_points -{ - explicit update_lock_points(const LockPoints& _lp) : lp(_lp) { } - - void operator() (CTxMemPoolEntry &e) { e.UpdateLockPoints(lp); } - -private: - const LockPoints& lp; -}; - // Multi_index tag names struct descendant_score {}; struct entry_time {}; @@ -631,10 +621,14 @@ public: bool removeSpentIndex(const uint256 txhash); void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs); - /** After reorg, check if mempool entries are now non-final, premature coinbase spends, or have - * invalid lockpoints. Update lockpoints and remove entries (and descendants of entries) that - * are no longer valid. */ - void removeForReorg(CChain& chain, std::function check_final_and_mature) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); + /** After reorg, filter the entries that would no longer be valid in the next block, and update + * the entries' cached LockPoints if needed. The mempool does not have any knowledge of + * consensus rules. It just appplies the callable function and removes the ones for which it + * returns true. + * @param[in] filter_final_and_mature Predicate that checks the relevant validation rules + * and updates an entry's LockPoints. + * */ + void removeForReorg(CChain& chain, std::function filter_final_and_mature) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeProTxPubKeyConflicts(const CTransaction &tx, const CKeyID &keyId) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeProTxPubKeyConflicts(const CTransaction &tx, const CBLSLazyPublicKey &pubKey) EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 64f1a08425..95050042fb 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -493,17 +494,37 @@ std::string Capitalize(std::string str) return str; } +namespace { + +using ByteAsHex = std::array; + +constexpr std::array CreateByteToHexMap() +{ + constexpr char hexmap[16] = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'}; + + std::array byte_to_hex{}; + for (size_t i = 0; i < byte_to_hex.size(); ++i) { + byte_to_hex[i][0] = hexmap[i >> 4]; + byte_to_hex[i][1] = hexmap[i & 15]; + } + return byte_to_hex; +} + +} // namespace + std::string HexStr(const Span s) { std::string rv(s.size() * 2, '\0'); - static constexpr char hexmap[16] = { '0', '1', '2', '3', '4', '5', '6', '7', - '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; - auto it = rv.begin(); + static constexpr auto byte_to_hex = CreateByteToHexMap(); + static_assert(sizeof(byte_to_hex) == 512); + + char* it = rv.data(); for (uint8_t v : s) { - *it++ = hexmap[v >> 4]; - *it++ = hexmap[v & 15]; + std::memcpy(it, byte_to_hex[v].data(), 2); + it += 2; } - assert(it == rv.end()); + + assert(it == rv.data() + rv.size()); return rv; } diff --git a/src/validation.cpp b/src/validation.cpp index aa156b2d35..d19f12b500 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -378,41 +378,55 @@ void CChainState::MaybeUpdateMempoolForReorg( // the disconnectpool that were added back and cleans up the mempool state. m_mempool->UpdateTransactionsFromBlock(vHashUpdate); - const auto check_final_and_mature = [this, flags=STANDARD_LOCKTIME_VERIFY_FLAGS](CTxMemPool::txiter it) + // Predicate to use for filtering transactions in removeForReorg. + // Checks whether the transaction is still final and, if it spends a coinbase output, mature. + // Also updates valid entries' cached LockPoints if needed. + // If false, the tx is still valid and its lockpoints are updated. + // If true, the tx would be invalid in the next block; remove this entry and all of its descendants. + const auto filter_final_and_mature = [this, flags=STANDARD_LOCKTIME_VERIFY_FLAGS](CTxMemPool::txiter it) EXCLUSIVE_LOCKS_REQUIRED(m_mempool->cs, ::cs_main) { - bool should_remove = false; AssertLockHeld(m_mempool->cs); AssertLockHeld(::cs_main); const CTransaction& tx = it->GetTx(); + + // The transaction must be final. + if (!CheckFinalTx(m_chain.Tip(), tx, flags)) return true; LockPoints lp = it->GetLockPoints(); const bool validLP{TestLockPointValidity(m_chain, lp)}; CCoinsViewMemPool view_mempool(&CoinsTip(), *m_mempool); - if (!CheckFinalTx(m_chain.Tip(), tx, flags) - || !CheckSequenceLocks(m_chain.Tip(), view_mempool, tx, flags, &lp, validLP)) { - // Note if CheckSequenceLocks fails the LockPoints may still be invalid - // So it's critical that we remove the tx and not depend on the LockPoints. - should_remove = true; - } else if (it->GetSpendsCoinbase()) { + // CheckSequenceLocks checks if the transaction will be final in the next block to be + // created on top of the new chain. We use useExistingLockPoints=false so that, instead of + // using the information in lp (which might now refer to a block that no longer exists in + // the chain), it will update lp to contain LockPoints relevant to the new chain. + if (!CheckSequenceLocks(m_chain.Tip(), view_mempool, tx, flags, &lp, validLP)) { + // If CheckSequenceLocks fails, remove the tx and don't depend on the LockPoints. + return true; + } else if (!validLP) { + // If CheckSequenceLocks succeeded, it also updated the LockPoints. + // Now update the mempool entry lockpoints as well. + m_mempool->mapTx.modify(it, [&lp](CTxMemPoolEntry& e) { e.UpdateLockPoints(lp); }); + } + + // If the transaction spends any coinbase outputs, it must be mature. + if (it->GetSpendsCoinbase()) { for (const CTxIn& txin : tx.vin) { auto it2 = m_mempool->mapTx.find(txin.prevout.hash); if (it2 != m_mempool->mapTx.end()) continue; - const Coin &coin = CoinsTip().AccessCoin(txin.prevout); + const Coin& coin{CoinsTip().AccessCoin(txin.prevout)}; assert(!coin.IsSpent()); const auto mempool_spend_height{m_chain.Tip()->nHeight + 1}; - if (coin.IsSpent() || (coin.IsCoinBase() && mempool_spend_height - coin.nHeight < COINBASE_MATURITY)) { - should_remove = true; - break; + if (coin.IsCoinBase() && mempool_spend_height - coin.nHeight < COINBASE_MATURITY) { + return true; } } } - // CheckSequenceLocks updates lp. Update the mempool entry LockPoints. - if (!validLP) m_mempool->mapTx.modify(it, update_lock_points(lp)); - return should_remove; + // Transaction is still valid and cached LockPoints are updated. + return false; }; // We also need to remove any now-immature transactions - m_mempool->removeForReorg(m_chain, check_final_and_mature); + m_mempool->removeForReorg(m_chain, filter_final_and_mature); // Re-limit mempool size, in case we added any transactions LimitMempoolSize( *m_mempool, @@ -2482,9 +2496,9 @@ bool CChainState::FlushStateToDisk( full_flush_completed = true; TRACE5(utxocache, flush, (int64_t)(GetTimeMicros() - nNow.count()), // in microseconds (µs) - (u_int32_t)mode, - (u_int64_t)coins_count, - (u_int64_t)coins_mem_usage, + (uint32_t)mode, + (uint64_t)coins_count, + (uint64_t)coins_mem_usage, (bool)fFlushForPrune); } } diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 110f1ace98..4ffc08335c 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -98,7 +98,7 @@ void BerkeleyEnvironment::Close() if (ret != 0) LogPrintf("BerkeleyEnvironment::Close: Error %d closing database environment: %s\n", ret, DbEnv::strerror(ret)); if (!fMockDb) - DbEnv((u_int32_t)0).remove(strPath.c_str(), 0); + DbEnv((uint32_t)0).remove(strPath.c_str(), 0); if (error_file) fclose(error_file); @@ -246,7 +246,7 @@ const void* BerkeleyBatch::SafeDbt::get_data() const return m_dbt.get_data(); } -u_int32_t BerkeleyBatch::SafeDbt::get_size() const +uint32_t BerkeleyBatch::SafeDbt::get_size() const { return m_dbt.get_size(); } diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index 4ea0f460fe..f09e76047b 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -34,7 +34,7 @@ static const unsigned int DEFAULT_WALLET_DBLOGSIZE = 100; static const bool DEFAULT_WALLET_PRIVDB = true; struct WalletDatabaseFileId { - u_int8_t value[DB_FILE_ID_LEN]; + uint8_t value[DB_FILE_ID_LEN]; bool operator==(const WalletDatabaseFileId& rhs) const; }; @@ -182,7 +182,7 @@ class BerkeleyBatch : public DatabaseBatch // delegate to Dbt const void* get_data() const; - u_int32_t get_size() const; + uint32_t get_size() const; // conversion operator to access the underlying Dbt operator Dbt*(); diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 409944bdad..d4e93d5d15 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1802,7 +1802,7 @@ RPCHelpMan importdescriptors() { /* oneline_description */ "", {"timestamp | \"now\"", "integer / string"} }, {"internal", RPCArg::Type::BOOL, RPCArg::Default{false}, "Whether matching outputs should be treated as not incoming payments (e.g. change)"}, - {"label", RPCArg::Type::STR, RPCArg::Default{""}, "Label to assign to the address, only allowed with internal=false"}, + {"label", RPCArg::Type::STR, RPCArg::Default{""}, "Label to assign to the address, only allowed with internal=false. Disabled for ranged descriptors"}, }, }, }, diff --git a/test/functional/feature_dirsymlinks.py b/test/functional/feature_dirsymlinks.py index 85c8e27600..288754c04c 100755 --- a/test/functional/feature_dirsymlinks.py +++ b/test/functional/feature_dirsymlinks.py @@ -6,9 +6,8 @@ """ import os -import sys -from test_framework.test_framework import BitcoinTestFramework, SkipTest +from test_framework.test_framework import BitcoinTestFramework def rename_and_link(*, from_name, to_name): @@ -16,24 +15,27 @@ def rename_and_link(*, from_name, to_name): os.symlink(to_name, from_name) assert os.path.islink(from_name) and os.path.isdir(from_name) -class SymlinkTest(BitcoinTestFramework): - def skip_test_if_missing_module(self): - if sys.platform == 'win32': - raise SkipTest("Symlinks test skipped on Windows") +class SymlinkTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 def run_test(self): + dir_new_blocks = self.nodes[0].chain_path / "new_blocks" + dir_new_chainstate = self.nodes[0].chain_path / "new_chainstate" self.stop_node(0) - rename_and_link(from_name=os.path.join(self.nodes[0].datadir, self.chain, "blocks"), - to_name=os.path.join(self.nodes[0].datadir, self.chain, "newblocks")) - rename_and_link(from_name=os.path.join(self.nodes[0].datadir, self.chain, "chainstate"), - to_name=os.path.join(self.nodes[0].datadir, self.chain, "newchainstate")) + rename_and_link( + from_name=self.nodes[0].chain_path / "blocks", + to_name=dir_new_blocks, + ) + rename_and_link( + from_name=self.nodes[0].chain_path / "chainstate", + to_name=dir_new_chainstate, + ) self.start_node(0) -if __name__ == '__main__': +if __name__ == "__main__": SymlinkTest().main() diff --git a/test/functional/mempool_unbroadcast.py b/test/functional/mempool_unbroadcast.py index d200689352..45abcc5a64 100755 --- a/test/functional/mempool_unbroadcast.py +++ b/test/functional/mempool_unbroadcast.py @@ -9,21 +9,20 @@ import time from test_framework.p2p import P2PTxInvStore from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import ( - assert_equal, - create_confirmed_utxos, -) +from test_framework.util import assert_equal +from test_framework.wallet import MiniWallet MAX_INITIAL_BROADCAST_DELAY = 15 * 60 # 15 minutes in seconds class MempoolUnbroadcastTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 - - def skip_test_if_missing_module(self): - self.skip_if_no_wallet() + if self.is_wallet_compiled(): + self.requires_wallet = True def run_test(self): + self.wallet = MiniWallet(self.nodes[0]) + self.wallet.rescan_utxos() self.test_broadcast() self.test_txn_removal() @@ -31,30 +30,25 @@ class MempoolUnbroadcastTest(BitcoinTestFramework): self.log.info("Test that mempool reattempts delivery of locally submitted transaction") node = self.nodes[0] - min_relay_fee = node.getnetworkinfo()["relayfee"] - utxos = create_confirmed_utxos(self, min_relay_fee, node, 10) - self.disconnect_nodes(0, 1) self.log.info("Generate transactions that only node 0 knows about") - # generate a wallet txn - addr = node.getnewaddress() - wallet_tx_hsh = node.sendtoaddress(addr, 0.0001) + if self.is_wallet_compiled(): + # generate a wallet txn + addr = node.getnewaddress() + wallet_tx_hsh = node.sendtoaddress(addr, 0.0001) # generate a txn using sendrawtransaction - us0 = utxos.pop() - inputs = [{"txid": us0["txid"], "vout": us0["vout"]}] - outputs = {addr: 0.0001} - tx = node.createrawtransaction(inputs, outputs) - node.settxfee(min_relay_fee) - txF = node.fundrawtransaction(tx) - txFS = node.signrawtransactionwithwallet(txF["hex"]) + txFS = self.wallet.create_self_transfer(from_node=node) rpc_tx_hsh = node.sendrawtransaction(txFS["hex"]) # check transactions are in unbroadcast using rpc mempoolinfo = self.nodes[0].getmempoolinfo() - assert_equal(mempoolinfo['unbroadcastcount'], 2) + unbroadcast_count = 1 + if self.is_wallet_compiled(): + unbroadcast_count += 1 + assert_equal(mempoolinfo['unbroadcastcount'], unbroadcast_count) mempool = self.nodes[0].getrawmempool(True) for tx in mempool: assert_equal(mempool[tx]['unbroadcast'], True) @@ -62,7 +56,8 @@ class MempoolUnbroadcastTest(BitcoinTestFramework): # check that second node doesn't have these two txns mempool = self.nodes[1].getrawmempool() assert rpc_tx_hsh not in mempool - assert wallet_tx_hsh not in mempool + if self.is_wallet_compiled(): + assert wallet_tx_hsh not in mempool # ensure that unbroadcast txs are persisted to mempool.dat self.restart_node(0) @@ -75,7 +70,8 @@ class MempoolUnbroadcastTest(BitcoinTestFramework): self.sync_mempools(timeout=30) mempool = self.nodes[1].getrawmempool() assert rpc_tx_hsh in mempool - assert wallet_tx_hsh in mempool + if self.is_wallet_compiled(): + assert wallet_tx_hsh in mempool # check that transactions are no longer in first node's unbroadcast set mempool = self.nodes[0].getrawmempool(True) @@ -104,8 +100,7 @@ class MempoolUnbroadcastTest(BitcoinTestFramework): # since the node doesn't have any connections, it will not receive # any GETDATAs & thus the transaction will remain in the unbroadcast set. - addr = node.getnewaddress() - txhsh = node.sendtoaddress(addr, 0.0001) + txhsh = self.wallet.send_self_transfer(from_node=node)["txid"] # check transaction was removed from unbroadcast set due to presence in # a block diff --git a/test/functional/rpc_txoutproof.py b/test/functional/rpc_txoutproof.py index 34eb48a89e..9fed9cbe15 100755 --- a/test/functional/rpc_txoutproof.py +++ b/test/functional/rpc_txoutproof.py @@ -54,7 +54,7 @@ class MerkleBlockTest(BitcoinTestFramework): assert_equal(self.nodes[0].verifytxoutproof(self.nodes[0].gettxoutproof([txid1, txid2])), txlist) assert_equal(self.nodes[0].verifytxoutproof(self.nodes[0].gettxoutproof([txid1, txid2], blockhash)), txlist) - txin_spent = miniwallet.get_utxo() # Get the change from txid2 + txin_spent = miniwallet.get_utxo(txid=txid2) # Get the change from txid2 tx3 = miniwallet.send_self_transfer(from_node=self.nodes[0], utxo_to_spend=txin_spent) txid3 = tx3['txid'] self.generate(self.nodes[0], 1) diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py index 6d494ee2c6..d0285f3ad9 100644 --- a/test/functional/test_framework/wallet.py +++ b/test/functional/test_framework/wallet.py @@ -135,10 +135,9 @@ class MiniWallet: Args: txid: get the first utxo we find from a specific transaction - - Note: Can be used to get the change output immediately after a send_self_transfer """ index = -1 # by default the last utxo + self._utxos = sorted(self._utxos, key=lambda k: (k['value'], -k['height'])) # Put the largest utxo last if txid: utxo = next(filter(lambda utxo: txid == utxo['txid'], self._utxos)) index = self._utxos.index(utxo) @@ -155,8 +154,7 @@ class MiniWallet: def create_self_transfer(self, *, fee_rate=Decimal("0.003"), from_node, utxo_to_spend=None, mempool_valid=True, locktime=0, sequence=0): """Create and return a tx with the specified fee_rate. Fee may be exact or at most one satoshi higher than needed.""" - self._utxos = sorted(self._utxos, key=lambda k: (k['value'], -k['height'])) - utxo_to_spend = utxo_to_spend or self._utxos.pop() # Pick the largest utxo (if none provided) and hope it covers the fee + utxo_to_spend = utxo_to_spend or self.get_utxo() if self._priv_key is None: vsize = Decimal(85) # anyone-can-spend else: