diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 61fba06089..eb500f9b38 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -200,6 +200,14 @@ namespace { std::unique_ptr recentRejects GUARDED_BY(cs_main); uint256 hashRecentRejectsChainTip GUARDED_BY(cs_main); + /* + * Filter for transactions that have been recently confirmed. + * We use this to avoid requesting transactions that have already been + * confirnmed. + */ + RecursiveMutex g_cs_recent_confirmed_transactions; + std::unique_ptr g_recent_confirmed_transactions GUARDED_BY(g_cs_recent_confirmed_transactions); + /** Blocks that are in flight, and that are in the queue to be downloaded. */ struct QueuedBlock { uint256 hash; @@ -1280,6 +1288,16 @@ PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn, BanMan* banman, CS // Initialize global variables that cannot be constructed at startup. recentRejects.reset(new CRollingBloomFilter(120000, 0.000001)); + // Blocks don't typically have more than 4000 transactions, so this should + // be at least six blocks (~1 hr) worth of transactions that we can store. + // If the number of transactions appearing in a block goes up, or if we are + // seeing getdata requests more than an hour after initial announcement, we + // can increase this number. + // The false positive rate of 1/1M should come out to less than 1 + // transaction per day that would be inadvertently ignored (which is the + // same probability that we have in the reject filter). + g_recent_confirmed_transactions.reset(new CRollingBloomFilter(24000, 0.000001)); + const Consensus::Params& consensusParams = Params().GetConsensus(); // Stale tip checking and peer eviction are on two different timers, but we // don't want them to get out of sync due to drift in the scheduler, so we @@ -1297,51 +1315,74 @@ PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn, BanMan* banman, CS * Evict orphan txn pool entries (EraseOrphanTx) based on a newly connected * block. Also save the time of the last tip update. */ -void PeerLogicValidation::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindex, const std::vector& vtxConflicted) { - LOCK2(cs_main, g_cs_orphans); +void PeerLogicValidation::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindex, const std::vector& vtxConflicted) +{ + { + LOCK2(cs_main, g_cs_orphans); - std::vector vOrphanErase; - std::set orphanWorkSet; + std::vector vOrphanErase; + std::set orphanWorkSet; - for (const CTransactionRef& ptx : pblock->vtx) { - const CTransaction& tx = *ptx; + for (const CTransactionRef& ptx : pblock->vtx) { + const CTransaction& tx = *ptx; - // Which orphan pool entries we should reprocess and potentially try to accept into mempool again? - for (size_t i = 0; i < tx.vin.size(); i++) { - auto itByPrev = mapOrphanTransactionsByPrev.find(COutPoint(tx.GetHash(), (uint32_t)i)); - if (itByPrev == mapOrphanTransactionsByPrev.end()) continue; - for (const auto& elem : itByPrev->second) { - orphanWorkSet.insert(elem->first); + // Which orphan pool entries we should reprocess and potentially try to accept into mempool again? + for (size_t i = 0; i < tx.vin.size(); i++) { + auto itByPrev = mapOrphanTransactionsByPrev.find(COutPoint(tx.GetHash(), (uint32_t)i)); + if (itByPrev == mapOrphanTransactionsByPrev.end()) continue; + for (const auto& elem : itByPrev->second) { + orphanWorkSet.insert(elem->first); + } + } + + // Which orphan pool entries must we evict? + for (const auto& txin : tx.vin) { + auto itByPrev = mapOrphanTransactionsByPrev.find(txin.prevout); + if (itByPrev == mapOrphanTransactionsByPrev.end()) continue; + for (auto mi = itByPrev->second.begin(); mi != itByPrev->second.end(); ++mi) { + const CTransaction& orphanTx = *(*mi)->second.tx; + const uint256& orphanHash = orphanTx.GetHash(); + vOrphanErase.push_back(orphanHash); + } } } - // Which orphan pool entries must we evict? - for (const auto& txin : tx.vin) { - auto itByPrev = mapOrphanTransactionsByPrev.find(txin.prevout); - if (itByPrev == mapOrphanTransactionsByPrev.end()) continue; - for (auto mi = itByPrev->second.begin(); mi != itByPrev->second.end(); ++mi) { - const CTransaction& orphanTx = *(*mi)->second.tx; - const uint256& orphanHash = orphanTx.GetHash(); - vOrphanErase.push_back(orphanHash); + // Erase orphan transactions included or precluded by this block + if (vOrphanErase.size()) { + int nErased = 0; + for (const uint256& orphanHash : vOrphanErase) { + nErased += EraseOrphanTx(orphanHash); } + LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx included or conflicted by block\n", nErased); + } + + while (!orphanWorkSet.empty()) { + LogPrint(BCLog::MEMPOOL, "Trying to process %d orphans\n", orphanWorkSet.size()); + ProcessOrphanTx(connman, m_mempool, orphanWorkSet); + } + + g_last_tip_update = GetTime(); + } + { + LOCK(g_cs_recent_confirmed_transactions); + for (const auto& ptx : pblock->vtx) { + g_recent_confirmed_transactions->insert(ptx->GetHash()); } } +} - // Erase orphan transactions included or precluded by this block - if (vOrphanErase.size()) { - int nErased = 0; - for (const uint256& orphanHash : vOrphanErase) { - nErased += EraseOrphanTx(orphanHash); - } - LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx included or conflicted by block\n", nErased); - } - - while (!orphanWorkSet.empty()) { - LogPrint(BCLog::MEMPOOL, "Trying to process %d orphans\n", orphanWorkSet.size()); - ProcessOrphanTx(connman, m_mempool, orphanWorkSet); - } - - g_last_tip_update = GetTime(); +void PeerLogicValidation::BlockDisconnected(const std::shared_ptr &block, const CBlockIndex* pindex) +{ + // To avoid relay problems with transactions that were previously + // confirmed, clear our filter of recently confirmed transactions whenever + // there's a reorg. + // This means that in a 1-block reorg (where 1 block is disconnected and + // then another block reconnected), our filter will drop to having only one + // block's worth of transactions in it, but that should be fine, since + // presumably the most common case of relaying a confirmed transaction + // should be just after a new block containing it is found. + LOCK(g_cs_recent_confirmed_transactions); + g_recent_confirmed_transactions->reset(); } // All of the following cache a recent block, and are protected by cs_most_recent_block @@ -1498,7 +1539,11 @@ bool static AlreadyHave(const CInv& inv, const CTxMemPool& mempool, const llmq:: LOCK(g_cs_orphans); if (mapOrphanTransactions.count(inv.hash)) return true; } - const CCoinsViewCache& coins_cache = ::ChainstateActive().CoinsTip(); + + { + LOCK(g_cs_recent_confirmed_transactions); + if (g_recent_confirmed_transactions->contains(inv.hash)) return true; + } // When we receive an islock for a previously rejected transaction, we have to // drop the first-seen tx (which such a locked transaction was conflicting with) @@ -1519,8 +1564,6 @@ bool static AlreadyHave(const CInv& inv, const CTxMemPool& mempool, const llmq:: return (!fIgnoreRecentRejects && recentRejects->contains(inv.hash)) || (inv.type == MSG_DSTX && static_cast(CCoinJoin::GetDSTX(inv.hash))) || mempool.exists(inv.hash) || - coins_cache.HaveCoinInCache(COutPoint(inv.hash, 0)) || // Best effort: only try output 0 and 1 - coins_cache.HaveCoinInCache(COutPoint(inv.hash, 1)) || (g_txindex != nullptr && g_txindex->HasTx(inv.hash)); } diff --git a/src/net_processing.h b/src/net_processing.h index e6de758839..d2d7b8a95e 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -61,6 +61,7 @@ public: * Overridden from CValidationInterface. */ void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected, const std::vector& vtxConflicted) override; + void BlockDisconnected(const std::shared_ptr &block, const CBlockIndex* pindex) override; /** * Overridden from CValidationInterface. */ diff --git a/src/qt/bantablemodel.cpp b/src/qt/bantablemodel.cpp index 3d57fa0775..7f721bcb7d 100644 --- a/src/qt/bantablemodel.cpp +++ b/src/qt/bantablemodel.cpp @@ -6,12 +6,13 @@ #include #include // For banmap_t -#include #include -#include +#include #include +#include +#include bool BannedNodeLessThan::operator()(const CCombinedBan& left, const CCombinedBan& right) const { @@ -78,10 +79,9 @@ public: } }; -BanTableModel::BanTableModel(interfaces::Node& node, ClientModel *parent) : +BanTableModel::BanTableModel(interfaces::Node& node, QObject* parent) : QAbstractTableModel(parent), - m_node(node), - clientModel(parent) + m_node(node) { columns << tr("IP/Netmask") << tr("Banned Until"); priv.reset(new BanTablePriv()); diff --git a/src/qt/bantablemodel.h b/src/qt/bantablemodel.h index de0a6a7e4c..d683606b9a 100644 --- a/src/qt/bantablemodel.h +++ b/src/qt/bantablemodel.h @@ -12,7 +12,6 @@ #include #include -class ClientModel; class BanTablePriv; namespace interfaces { @@ -45,7 +44,7 @@ class BanTableModel : public QAbstractTableModel Q_OBJECT public: - explicit BanTableModel(interfaces::Node& node, ClientModel *parent = nullptr); + explicit BanTableModel(interfaces::Node& node, QObject* parent); ~BanTableModel(); void startAutoRefresh(); void stopAutoRefresh(); @@ -73,7 +72,6 @@ public Q_SLOTS: private: interfaces::Node& m_node; - ClientModel *clientModel; QStringList columns; std::unique_ptr priv; }; diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp index 287028b54e..39efe05ec5 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -4,7 +4,6 @@ #include -#include #include #include @@ -101,10 +100,9 @@ public: } }; -PeerTableModel::PeerTableModel(interfaces::Node& node, ClientModel *parent) : +PeerTableModel::PeerTableModel(interfaces::Node& node, QObject* parent) : QAbstractTableModel(parent), m_node(node), - clientModel(parent), timer(nullptr) { columns << tr("NodeId") << tr("Node/Service") << tr("Ping") << tr("Sent") << tr("Received") << tr("User Agent"); diff --git a/src/qt/peertablemodel.h b/src/qt/peertablemodel.h index 852fe2eed3..618551bdfb 100644 --- a/src/qt/peertablemodel.h +++ b/src/qt/peertablemodel.h @@ -13,7 +13,6 @@ #include #include -class ClientModel; class PeerTablePriv; namespace interfaces { @@ -51,7 +50,7 @@ class PeerTableModel : public QAbstractTableModel Q_OBJECT public: - explicit PeerTableModel(interfaces::Node& node, ClientModel *parent = nullptr); + explicit PeerTableModel(interfaces::Node& node, QObject* parent); ~PeerTableModel(); const CNodeCombinedStats *getNodeStats(int idx); int getRowByNodeId(NodeId nodeid); @@ -83,7 +82,6 @@ public Q_SLOTS: private: interfaces::Node& m_node; - ClientModel *clientModel; QStringList columns; std::unique_ptr priv; QTimer *timer; diff --git a/src/qt/utilitydialog.cpp b/src/qt/utilitydialog.cpp index fd2e18be27..d15681409e 100644 --- a/src/qt/utilitydialog.cpp +++ b/src/qt/utilitydialog.cpp @@ -11,7 +11,6 @@ #include -#include #include #include @@ -23,9 +22,10 @@ #include #include +#include #include -#include #include +#include #include /** "Help message" or "About" dialog box */ @@ -191,10 +191,9 @@ ShutdownWindow::ShutdownWindow(interfaces::Node& node, QWidget *parent, Qt::Wind GUIUtil::updateFonts(); } -QWidget *ShutdownWindow::showShutdownWindow(interfaces::Node& node, BitcoinGUI *window) +QWidget* ShutdownWindow::showShutdownWindow(interfaces::Node& node, QMainWindow* window) { - if (!window) - return nullptr; + assert(window != nullptr); // Show a simple window indicating shutdown status QWidget *shutdownWindow = new ShutdownWindow(node); diff --git a/src/qt/utilitydialog.h b/src/qt/utilitydialog.h index aa2de6e76f..52d92e551d 100644 --- a/src/qt/utilitydialog.h +++ b/src/qt/utilitydialog.h @@ -6,9 +6,11 @@ #define BITCOIN_QT_UTILITYDIALOG_H #include -#include +#include -class BitcoinGUI; +QT_BEGIN_NAMESPACE +class QMainWindow; +QT_END_NAMESPACE namespace interfaces { class Node; @@ -52,7 +54,7 @@ class ShutdownWindow : public QWidget public: explicit ShutdownWindow(interfaces::Node& node, QWidget *parent=nullptr, Qt::WindowFlags f=Qt::Widget); - static QWidget *showShutdownWindow(interfaces::Node& node, BitcoinGUI *window); + static QWidget* showShutdownWindow(interfaces::Node& node, QMainWindow* window); protected: void closeEvent(QCloseEvent *event) override; diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp index 395b3374d4..77d116f2c8 100644 --- a/src/qt/walletframe.cpp +++ b/src/qt/walletframe.cpp @@ -60,7 +60,6 @@ bool WalletFrame::addWallet(WalletModel *walletModel) if (mapWalletViews.count(walletModel) > 0) return false; WalletView* walletView = new WalletView(this); - walletView->setBitcoinGUI(gui); walletView->setClientModel(clientModel); walletView->setWalletModel(walletModel); walletView->showOutOfSyncWarning(bOutOfSync); @@ -76,6 +75,14 @@ bool WalletFrame::addWallet(WalletModel *walletModel) mapWalletViews[walletModel] = walletView; connect(walletView, &WalletView::outOfSyncWarningClicked, this, &WalletFrame::outOfSyncWarningClicked); + connect(walletView, &WalletView::transactionClicked, gui, &BitcoinGUI::gotoHistoryPage); + connect(walletView, &WalletView::coinsSent, gui, &BitcoinGUI::gotoHistoryPage); + connect(walletView, &WalletView::message, [this](const QString& title, const QString& message, unsigned int style) { + gui->message(title, message, style); + }); + connect(walletView, &WalletView::encryptionStatusChanged, gui, &BitcoinGUI::updateWalletStatus); + connect(walletView, &WalletView::incomingTransaction, gui, &BitcoinGUI::incomingTransaction); + connect(walletView, &WalletView::hdEnabledStatusChanged, gui, &BitcoinGUI::updateWalletStatus); return true; } diff --git a/src/qt/walletview.cpp b/src/qt/walletview.cpp index 721d3ae343..acfb243528 100644 --- a/src/qt/walletview.cpp +++ b/src/qt/walletview.cpp @@ -6,7 +6,6 @@ #include #include -#include #include #include #include @@ -94,10 +93,13 @@ WalletView::WalletView(QWidget* parent) : addWidget(governanceListPage); } + connect(overviewPage, &OverviewPage::transactionClicked, this, &WalletView::transactionClicked); // Clicking on a transaction on the overview pre-selects the transaction on the transaction history page connect(overviewPage, &OverviewPage::transactionClicked, transactionView, static_cast(&TransactionView::focusTransaction)); connect(overviewPage, &OverviewPage::outOfSyncWarningClicked, this, &WalletView::requestedSyncWarningInfo); + connect(sendCoinsPage, &SendCoinsDialog::coinsSent, this, &WalletView::coinsSent); + connect(coinJoinCoinsPage, &SendCoinsDialog::coinsSent, this, &WalletView::coinsSent); // Highlight transaction after send connect(sendCoinsPage, &SendCoinsDialog::coinsSent, transactionView, static_cast(&TransactionView::focusTransaction)); connect(coinJoinCoinsPage, &SendCoinsDialog::coinsSent, transactionView, static_cast(&TransactionView::focusTransaction)); @@ -122,33 +124,6 @@ WalletView::~WalletView() { } -void WalletView::setBitcoinGUI(BitcoinGUI *gui) -{ - if (gui) - { - // Clicking on a transaction on the overview page simply sends you to transaction history page - connect(overviewPage, &OverviewPage::transactionClicked, gui, &BitcoinGUI::gotoHistoryPage); - - // Navigate to transaction history page after send - connect(sendCoinsPage, &SendCoinsDialog::coinsSent, gui, &BitcoinGUI::gotoHistoryPage); - connect(coinJoinCoinsPage, &SendCoinsDialog::coinsSent, gui, &BitcoinGUI::gotoHistoryPage); - - // Receive and report messages - connect(this, &WalletView::message, [gui](const QString &title, const QString &message, unsigned int style) { - gui->message(title, message, style); - }); - - // Pass through encryption status changed signals - connect(this, &WalletView::encryptionStatusChanged, gui, &BitcoinGUI::updateWalletStatus); - - // Pass through transaction notifications - connect(this, &WalletView::incomingTransaction, gui, &BitcoinGUI::incomingTransaction); - - // Connect HD enabled state signal - connect(this, &WalletView::hdEnabledStatusChanged, gui, &BitcoinGUI::updateWalletStatus); - } -} - void WalletView::setClientModel(ClientModel *_clientModel) { this->clientModel = _clientModel; diff --git a/src/qt/walletview.h b/src/qt/walletview.h index 544d56b0ae..9e1dab06e6 100644 --- a/src/qt/walletview.h +++ b/src/qt/walletview.h @@ -11,7 +11,6 @@ #include -class BitcoinGUI; class ClientModel; class OverviewPage; class ReceiveCoinsDialog; @@ -41,7 +40,6 @@ public: explicit WalletView(QWidget* parent); ~WalletView(); - void setBitcoinGUI(BitcoinGUI *gui); /** Set the client model. The client model represents the part of the core that communicates with the P2P network, and is wallet-agnostic. */ @@ -73,7 +71,7 @@ private: TransactionView *transactionView; - QProgressDialog *progressDialog; + QProgressDialog* progressDialog{nullptr}; QLabel *transactionSum; public Q_SLOTS: @@ -131,6 +129,8 @@ public Q_SLOTS: /** Update selected DASH amount from transactionview */ void trxAmount(QString amount); Q_SIGNALS: + void transactionClicked(); + void coinsSent(); /** Fired when a message should be reported to the user */ void message(const QString &title, const QString &message, unsigned int style); /** Encryption status of wallet changed */ diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 3e11c378e9..6cd82e8788 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -228,6 +228,27 @@ BOOST_AUTO_TEST_CASE(util_ParseParameters) BOOST_CHECK(testArgs.GetArgs("-ccc").size() == 2); } +BOOST_AUTO_TEST_CASE(util_ParseInvalidParameters) +{ + TestArgsManager test; + test.SetupArgs({{"-registered", ArgsManager::ALLOW_ANY}}); + + const char* argv[] = {"ignored", "-registered"}; + std::string error; + BOOST_CHECK(test.ParseParameters(2, (char**)argv, error)); + BOOST_CHECK_EQUAL(error, ""); + + argv[1] = "-unregistered"; + BOOST_CHECK(!test.ParseParameters(2, (char**)argv, error)); + BOOST_CHECK_EQUAL(error, "Invalid parameter -unregistered"); + + // Make sure registered parameters prefixed with a chain name trigger errors. + // (Previously, they were accepted and ignored.) + argv[1] = "-test.registered"; + BOOST_CHECK(!test.ParseParameters(2, (char**)argv, error)); + BOOST_CHECK_EQUAL(error, "Invalid parameter -test.registered"); +} + static void TestParse(const std::string& str, bool expected_bool, int64_t expected_int) { TestArgsManager test; @@ -716,7 +737,8 @@ struct ArgsMergeTestingSetup : public BasicTestingSetup { void ForEachMergeSetup(Fn&& fn) { ActionList arg_actions = {}; - ForEachNoDup(arg_actions, SET, SECTION_NEGATE, [&] { + // command_line_options do not have sections. Only iterate over SET and NEGATE + ForEachNoDup(arg_actions, SET, NEGATE, [&] { ActionList conf_actions = {}; ForEachNoDup(conf_actions, SET, SECTION_NEGATE, [&] { for (bool soft_set : {false, true}) { @@ -876,7 +898,7 @@ BOOST_FIXTURE_TEST_CASE(util_ArgsMerge, ArgsMergeTestingSetup) // Results file is formatted like: // // || | | - BOOST_CHECK_EQUAL(out_sha_hex, "b835eef5977d69114eb039a976201f8c7121f34fe2b7ea2b73cafb516e5c9dc8"); + BOOST_CHECK_EQUAL(out_sha_hex, "8fd4877bb8bf337badca950ede6c917441901962f160e52514e06a60dea46cde"); } // Similar test as above, but for ArgsManager::GetChainName function. diff --git a/src/util/system.cpp b/src/util/system.cpp index cf9f62a109..e3b8b07dc3 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -356,21 +356,18 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin std::string section; util::SettingsValue value = InterpretOption(section, key, val); Optional flags = GetArgFlags('-' + key); - if (flags) { - if (!CheckValid(key, value, *flags, error)) { - return false; - } - // Weird behavior preserved for backwards compatibility: command - // line options with section prefixes are allowed but ignored. It - // would be better if these options triggered the Invalid parameter - // error below. - if (section.empty()) { - m_settings.command_line_options[key].push_back(value); - } - } else { - error = strprintf("Invalid parameter -%s", key); + + // Unknown command line options and command line options with dot + // characters (which are returned from InterpretOption with nonempty + // section strings) are not valid. + if (!flags || !section.empty()) { + error = strprintf("Invalid parameter %s", argv[i]); return false; } + + if (!CheckValid(key, value, *flags, error)) return false; + + m_settings.command_line_options[key].push_back(value); } // we do not allow -includeconf from command line, so we clear it here diff --git a/src/validation.cpp b/src/validation.cpp index 3ea889d1ce..64aaf62325 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1293,12 +1293,16 @@ static void CheckForkWarningConditionsOnNewFork(CBlockIndex* pindexNewForkTip) E CheckForkWarningConditions(); } +// Called both upon regular invalid block discovery *and* InvalidateBlock void static InvalidChainFound(CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { statsClient.inc("warnings.InvalidChainFound", 1.0f); if (!pindexBestInvalid || pindexNew->nChainWork > pindexBestInvalid->nChainWork) pindexBestInvalid = pindexNew; + if (pindexBestHeader != nullptr && pindexBestHeader->GetAncestor(pindexNew->nHeight) == pindexNew) { + pindexBestHeader = ::ChainActive().Tip(); + } LogPrintf("%s: invalid block=%s height=%d log2_work=%.8f date=%s\n", __func__, pindexNew->GetBlockHash().ToString(), pindexNew->nHeight, @@ -1326,6 +1330,8 @@ void static ConflictingChainFound(CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIR CheckForkWarningConditions(); } +// Same as InvalidChainFound, above, except not called directly from InvalidateBlock, +// which does its own setBlockIndexCandidates manageent. void CChainState::InvalidBlockFound(CBlockIndex *pindex, const CValidationState &state) { statsClient.inc("warnings.InvalidBlockFound", 1.0f); if (state.GetReason() != ValidationInvalidReason::BLOCK_MUTATED) { diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index d8bde945c2..0cbcdf4546 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -53,7 +53,7 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const #if HAVE_SYSTEM argsman.AddArg("-instantsendnotify=", "Execute command when a wallet InstantSend transaction is successfully locked (%s in cmd is replaced by TxID)", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); #endif - argsman.AddArg("-keypool=", strprintf("Set key pool size to (default: %u)", DEFAULT_KEYPOOL_SIZE), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); + argsman.AddArg("-keypool=", strprintf("Set key pool size to (default: %u). Warning: Smaller sizes may increase the risk of losing funds when restoring from an old backup, if none of the addresses in the original keypool have been used.", DEFAULT_KEYPOOL_SIZE), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-rescan=", "Rescan the block chain for missing wallet transactions on startup" " (1 = start from wallet creation time, 2 = start from genesis block)", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-spendzeroconfchange", strprintf("Spend unconfirmed change when sending transactions (default: %u)", DEFAULT_SPEND_ZEROCONF_CHANGE), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 359f8a2b2b..1e0c51cbdb 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -69,6 +69,11 @@ std::vector GetAffectedKeys(const CScript& spk, const SigningProvider& p * keys (by default 1000) ahead of the last used key and scans for the * addresses of those keys. This avoids the risk of not seeing transactions * involving the wallet's addresses, or of re-using the same address. + * In the unlikely case where none of the addresses in the `gap limit` are + * used on-chain, the look-ahead will not be incremented to keep + * a constant size and addresses beyond this range will not be detected by an + * old backup. For this reason, it is not recommended to decrease keypool size + * lower than default value. * * There is an external keypool (for addresses to hand out) and an internal keypool * (for change addresses). diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index aea8a81e92..1ceec4129d 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -23,7 +23,7 @@ class ConfArgsTest(BitcoinTestFramework): conf.write('includeconf={}\n'.format(inc_conf_file_path)) self.nodes[0].assert_start_raises_init_error( - expected_msg='Error: Error parsing command line arguments: Invalid parameter -dash_cli', + expected_msg='Error: Error parsing command line arguments: Invalid parameter -dash_cli=1', extra_args=['-dash_cli=1'], ) with open(inc_conf_file_path, 'w', encoding='utf-8') as conf: diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh index c777078595..d958ffb81f 100755 --- a/test/lint/lint-circular-dependencies.sh +++ b/test/lint/lint-circular-dependencies.sh @@ -13,11 +13,7 @@ EXPECTED_CIRCULAR_DEPENDENCIES=( "index/txindex -> validation -> index/txindex" "policy/fees -> txmempool -> policy/fees" "qt/addresstablemodel -> qt/walletmodel -> qt/addresstablemodel" - "qt/bantablemodel -> qt/clientmodel -> qt/bantablemodel" - "qt/bitcoingui -> qt/utilitydialog -> qt/bitcoingui" "qt/bitcoingui -> qt/walletframe -> qt/bitcoingui" - "qt/bitcoingui -> qt/walletview -> qt/bitcoingui" - "qt/clientmodel -> qt/peertablemodel -> qt/clientmodel" "qt/paymentserver -> qt/walletmodel -> qt/paymentserver" "qt/recentrequeststablemodel -> qt/walletmodel -> qt/recentrequeststablemodel" "qt/transactiontablemodel -> qt/walletmodel -> qt/transactiontablemodel"