diff options
author | Wladimir J. van der Laan <laanwj@protonmail.com> | 2020-01-20 17:34:37 +0100 |
---|---|---|
committer | Wladimir J. van der Laan <laanwj@protonmail.com> | 2020-01-20 19:39:41 +0100 |
commit | 98159132c3e7f3ea62dc8d3e0afc1e49a6000c3d (patch) | |
tree | fa36bf70769791e1d9b3a9fa5b6baf6701b1e267 | |
parent | bb123c6527c02a5936dbf7fee675b992b5c21936 (diff) | |
parent | cd67b1dcb8f1eca8c0c7cecc6f4de27c7efe41d5 (diff) | |
download | bitcoin-98159132c3e7f3ea62dc8d3e0afc1e49a6000c3d.tar.xz |
Merge #17792: 0.19: Backports
cd67b1dcb8f1eca8c0c7cecc6f4de27c7efe41d5 Use correct C++11 header for std::swap() (Hennadii Stepanov)
b8101fb7ac4bfa0e5c0ee2459b24bddaf59fe7c4 Fix comparison function signature (Hennadii Stepanov)
eac49073eb7c5d630dd9a285e36f743fa902c0ee Don't allow implementers to think ScriptHash(Witness*()) results in nesting computation (Gregory Sanders)
e2c45d89f7219fd5bcf19a6e04b291abbb4b5f95 IsUsedDestination shouldn't use key id as script id for ScriptHash (Gregory Sanders)
a5489c9892fc12cb70c6c7b017881a9218d0b041 IsUsedDestination should count any known single-key address (Gregory Sanders)
88729d804e39fbb42aa92c039afe3641edf9190c Fix issue with conflicted mempool tx in listsinceblock (Adam Jonas)
eafcea7a0ab17512f2b9e2a724685ca193920f04 gui: Fix duplicate wallet showing up (João Barbosa)
7e66d04770bfb21862e52736c4859d7a878cb906 Drop signal CClientUIInterface::LoadWallet (Russell Yanofsky)
179d55f0526082b46415a7f5b6c3742a403f3306 zmq: Fix due to invalid argument and multiple notifiers (João Barbosa)
Pull request description:
Backports
- #16963
- #17445
- #17258
- #17621
- #17924
- #17634
ACKs for top commit:
laanwj:
ACK cd67b1dcb8f1eca8c0c7cecc6f4de27c7efe41d5, checked that I got more or less the same result (including conflict resolution) backporting these commits
Tree-SHA512: 645786267cfb10a01a56f7cfd91ddead5f1475df5714595ae480237e04d40c5cfb7460b40532279cacd83e4b775a4ace68a258ec2184b8ad0e997a690a9245e5
-rw-r--r-- | src/cuckoocache.h | 4 | ||||
-rw-r--r-- | src/dummywallet.cpp | 10 | ||||
-rw-r--r-- | src/interfaces/chain.cpp | 1 | ||||
-rw-r--r-- | src/interfaces/chain.h | 5 | ||||
-rw-r--r-- | src/interfaces/handler.cpp | 14 | ||||
-rw-r--r-- | src/interfaces/handler.h | 4 | ||||
-rw-r--r-- | src/interfaces/node.cpp | 5 | ||||
-rw-r--r-- | src/prevector.h | 1 | ||||
-rw-r--r-- | src/qt/bantablemodel.cpp | 2 | ||||
-rw-r--r-- | src/qt/bitcoingui.cpp | 2 | ||||
-rw-r--r-- | src/qt/peertablemodel.cpp | 2 | ||||
-rw-r--r-- | src/qt/recentrequeststablemodel.cpp | 9 | ||||
-rw-r--r-- | src/qt/recentrequeststablemodel.h | 2 | ||||
-rw-r--r-- | src/qt/walletframe.cpp | 8 | ||||
-rw-r--r-- | src/qt/walletframe.h | 2 | ||||
-rw-r--r-- | src/script/standard.h | 5 | ||||
-rw-r--r-- | src/test/checkqueue_tests.cpp | 1 | ||||
-rw-r--r-- | src/ui_interface.cpp | 3 | ||||
-rw-r--r-- | src/ui_interface.h | 7 | ||||
-rw-r--r-- | src/validation.h | 1 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 4 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 43 | ||||
-rw-r--r-- | src/wallet/wallet.h | 8 | ||||
-rw-r--r-- | src/zmq/zmqpublishnotifier.cpp | 3 | ||||
-rwxr-xr-x | test/functional/interface_zmq.py | 4 | ||||
-rwxr-xr-x | test/functional/wallet_avoidreuse.py | 25 | ||||
-rwxr-xr-x | test/functional/wallet_listsinceblock.py | 49 |
27 files changed, 170 insertions, 54 deletions
diff --git a/src/cuckoocache.h b/src/cuckoocache.h index 4d0b094fa2..16a994393e 100644 --- a/src/cuckoocache.h +++ b/src/cuckoocache.h @@ -6,11 +6,11 @@ #define BITCOIN_CUCKOOCACHE_H #include <array> -#include <algorithm> #include <atomic> -#include <cstring> #include <cmath> +#include <cstring> #include <memory> +#include <utility> #include <vector> diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index 126e3479f3..80c79bf6c6 100644 --- a/src/dummywallet.cpp +++ b/src/dummywallet.cpp @@ -12,6 +12,8 @@ enum class WalletCreationStatus; namespace interfaces { class Chain; +class Handler; +class Wallet; } class DummyWalletInit : public WalletInitInterface { @@ -81,9 +83,13 @@ WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& throw std::logic_error("Wallet function called in non-wallet build."); } -namespace interfaces { +using LoadWalletFn = std::function<void(std::unique_ptr<interfaces::Wallet> wallet)>; +std::unique_ptr<interfaces::Handler> HandleLoadWallet(LoadWalletFn load_wallet) +{ + throw std::logic_error("Wallet function called in non-wallet build."); +} -class Wallet; +namespace interfaces { std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet) { diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index b8b9ecded9..aa44a69f08 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -339,7 +339,6 @@ public: void initMessage(const std::string& message) override { ::uiInterface.InitMessage(message); } void initWarning(const std::string& message) override { InitWarning(message); } void initError(const std::string& message) override { InitError(message); } - void loadWallet(std::unique_ptr<Wallet> wallet) override { ::uiInterface.LoadWallet(wallet); } void showProgress(const std::string& title, int progress, bool resume_possible) override { ::uiInterface.ShowProgress(title, progress, resume_possible); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index da670a3370..e099586000 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -43,7 +43,7 @@ class Wallet; //! asynchronously //! (https://github.com/bitcoin/bitcoin/pull/10973#issuecomment-380101269). //! -//! * The initMessages() and loadWallet() methods which the wallet uses to send +//! * The initMessage() and showProgress() methods which the wallet uses to send //! notifications to the GUI should go away when GUI and wallet can directly //! communicate with each other without going through the node //! (https://github.com/bitcoin/bitcoin/pull/15288#discussion_r253321096). @@ -208,9 +208,6 @@ public: //! Send init error. virtual void initError(const std::string& message) = 0; - //! Send wallet load notification to the GUI. - virtual void loadWallet(std::unique_ptr<Wallet> wallet) = 0; - //! Send progress indicator. virtual void showProgress(const std::string& title, int progress, bool resume_possible) = 0; diff --git a/src/interfaces/handler.cpp b/src/interfaces/handler.cpp index 92601fc4e9..4e235688fe 100644 --- a/src/interfaces/handler.cpp +++ b/src/interfaces/handler.cpp @@ -22,6 +22,15 @@ public: boost::signals2::scoped_connection m_connection; }; +class CleanupHandler : public Handler +{ +public: + explicit CleanupHandler(std::function<void()> cleanup) : m_cleanup(std::move(cleanup)) {} + ~CleanupHandler() override { if (!m_cleanup) return; m_cleanup(); m_cleanup = nullptr; } + void disconnect() override { if (!m_cleanup) return; m_cleanup(); m_cleanup = nullptr; } + std::function<void()> m_cleanup; +}; + } // namespace std::unique_ptr<Handler> MakeHandler(boost::signals2::connection connection) @@ -29,4 +38,9 @@ std::unique_ptr<Handler> MakeHandler(boost::signals2::connection connection) return MakeUnique<HandlerImpl>(std::move(connection)); } +std::unique_ptr<Handler> MakeHandler(std::function<void()> cleanup) +{ + return MakeUnique<CleanupHandler>(std::move(cleanup)); +} + } // namespace interfaces diff --git a/src/interfaces/handler.h b/src/interfaces/handler.h index c4c674cac5..46918bc22e 100644 --- a/src/interfaces/handler.h +++ b/src/interfaces/handler.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_INTERFACES_HANDLER_H #define BITCOIN_INTERFACES_HANDLER_H +#include <functional> #include <memory> namespace boost { @@ -30,6 +31,9 @@ public: //! Return handler wrapping a boost signal connection. std::unique_ptr<Handler> MakeHandler(boost::signals2::connection connection); +//! Return handler wrapping a cleanup function. +std::unique_ptr<Handler> MakeHandler(std::function<void()> cleanup); + } // namespace interfaces #endif // BITCOIN_INTERFACES_HANDLER_H diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index c80a8789fc..b4e1a46131 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -45,11 +45,10 @@ std::vector<fs::path> ListWalletDir(); std::vector<std::shared_ptr<CWallet>> GetWallets(); std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, std::string& error, std::string& warning); WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::string& warning, std::shared_ptr<CWallet>& result); +std::unique_ptr<interfaces::Handler> HandleLoadWallet(interfaces::Node::LoadWalletFn load_wallet); namespace interfaces { -class Wallet; - namespace { class NodeImpl : public Node @@ -286,7 +285,7 @@ public: } std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) override { - return MakeHandler(::uiInterface.LoadWallet_connect([fn](std::unique_ptr<Wallet>& wallet) { fn(std::move(wallet)); })); + return HandleLoadWallet(std::move(fn)); } std::unique_ptr<Handler> handleNotifyNumConnectionsChanged(NotifyNumConnectionsChangedFn fn) override { diff --git a/src/prevector.h b/src/prevector.h index 9d576321b6..34016879a5 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -14,6 +14,7 @@ #include <cstddef> #include <iterator> #include <type_traits> +#include <utility> #pragma pack(push, 1) /** Implements a drop-in replacement for std::vector<T> which stores up to N diff --git a/src/qt/bantablemodel.cpp b/src/qt/bantablemodel.cpp index efc726e09e..60a3178f96 100644 --- a/src/qt/bantablemodel.cpp +++ b/src/qt/bantablemodel.cpp @@ -10,7 +10,7 @@ #include <sync.h> #include <util/time.h> -#include <algorithm> +#include <utility> #include <QDebug> #include <QList> diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index e269c91d1c..784fdcb6e5 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -633,10 +633,10 @@ void BitcoinGUI::setWalletController(WalletController* wallet_controller) void BitcoinGUI::addWallet(WalletModel* walletModel) { if (!walletFrame) return; + if (!walletFrame->addWallet(walletModel)) return; const QString display_name = walletModel->getDisplayName(); setWalletActionsEnabled(true); rpcConsole->addWallet(walletModel); - walletFrame->addWallet(walletModel); m_wallet_selector->addItem(display_name, QVariant::fromValue(walletModel)); if (m_wallet_selector->count() == 2) { m_wallet_selector_label_action->setVisible(true); diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp index 99a9a12fe2..ed6fecce9d 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -11,7 +11,7 @@ #include <interfaces/node.h> #include <sync.h> -#include <algorithm> +#include <utility> #include <QDebug> #include <QList> diff --git a/src/qt/recentrequeststablemodel.cpp b/src/qt/recentrequeststablemodel.cpp index 1611ec823c..64bfb7d643 100644 --- a/src/qt/recentrequeststablemodel.cpp +++ b/src/qt/recentrequeststablemodel.cpp @@ -11,8 +11,7 @@ #include <clientversion.h> #include <streams.h> -#include <algorithm> - +#include <utility> RecentRequestsTableModel::RecentRequestsTableModel(WalletModel *parent) : QAbstractTableModel(parent), walletModel(parent) @@ -213,10 +212,10 @@ void RecentRequestsTableModel::updateDisplayUnit() updateAmountColumnTitle(); } -bool RecentRequestEntryLessThan::operator()(RecentRequestEntry &left, RecentRequestEntry &right) const +bool RecentRequestEntryLessThan::operator()(const RecentRequestEntry& left, const RecentRequestEntry& right) const { - RecentRequestEntry *pLeft = &left; - RecentRequestEntry *pRight = &right; + const RecentRequestEntry* pLeft = &left; + const RecentRequestEntry* pRight = &right; if (order == Qt::DescendingOrder) std::swap(pLeft, pRight); diff --git a/src/qt/recentrequeststablemodel.h b/src/qt/recentrequeststablemodel.h index 130b709d46..66e8d4f228 100644 --- a/src/qt/recentrequeststablemodel.h +++ b/src/qt/recentrequeststablemodel.h @@ -43,7 +43,7 @@ class RecentRequestEntryLessThan public: RecentRequestEntryLessThan(int nColumn, Qt::SortOrder fOrder): column(nColumn), order(fOrder) {} - bool operator()(RecentRequestEntry &left, RecentRequestEntry &right) const; + bool operator()(const RecentRequestEntry& left, const RecentRequestEntry& right) const; private: int column; diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp index 94413547d4..9f03e8b5a5 100644 --- a/src/qt/walletframe.cpp +++ b/src/qt/walletframe.cpp @@ -40,11 +40,11 @@ void WalletFrame::setClientModel(ClientModel *_clientModel) this->clientModel = _clientModel; } -void WalletFrame::addWallet(WalletModel *walletModel) +bool WalletFrame::addWallet(WalletModel *walletModel) { - if (!gui || !clientModel || !walletModel) return; + if (!gui || !clientModel || !walletModel) return false; - if (mapWalletViews.count(walletModel) > 0) return; + if (mapWalletViews.count(walletModel) > 0) return false; WalletView *walletView = new WalletView(platformStyle, this); walletView->setBitcoinGUI(gui); @@ -68,6 +68,8 @@ void WalletFrame::addWallet(WalletModel *walletModel) }); connect(walletView, &WalletView::outOfSyncWarningClicked, this, &WalletFrame::outOfSyncWarningClicked); + + return true; } void WalletFrame::setCurrentWallet(WalletModel* wallet_model) diff --git a/src/qt/walletframe.h b/src/qt/walletframe.h index 156653f47d..20fad08b0e 100644 --- a/src/qt/walletframe.h +++ b/src/qt/walletframe.h @@ -36,7 +36,7 @@ public: void setClientModel(ClientModel *clientModel); - void addWallet(WalletModel *walletModel); + bool addWallet(WalletModel *walletModel); void setCurrentWallet(WalletModel* wallet_model); void removeWallet(WalletModel* wallet_model); void removeAllWallets(); diff --git a/src/script/standard.h b/src/script/standard.h index e45e2d92cc..0c344c7733 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -81,9 +81,14 @@ struct PKHash : public uint160 using uint160::uint160; }; +struct WitnessV0KeyHash; struct ScriptHash : public uint160 { ScriptHash() : uint160() {} + // These don't do what you'd expect. + // Use ScriptHash(GetScriptForDestination(...)) instead. + explicit ScriptHash(const WitnessV0KeyHash& hash) = delete; + explicit ScriptHash(const PKHash& hash) = delete; explicit ScriptHash(const uint160& hash) : uint160(hash) {} explicit ScriptHash(const CScript& script); using uint160::uint160; diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp index d796444419..6c050ab964 100644 --- a/src/test/checkqueue_tests.cpp +++ b/src/test/checkqueue_tests.cpp @@ -18,6 +18,7 @@ #include <condition_variable> #include <unordered_set> +#include <utility> // BasicTestingSetup not sufficient because nScriptCheckThreads is not set // otherwise. diff --git a/src/ui_interface.cpp b/src/ui_interface.cpp index d310637145..3968464efc 100644 --- a/src/ui_interface.cpp +++ b/src/ui_interface.cpp @@ -16,7 +16,6 @@ struct UISignals { boost::signals2::signal<CClientUIInterface::NotifyNumConnectionsChangedSig> NotifyNumConnectionsChanged; boost::signals2::signal<CClientUIInterface::NotifyNetworkActiveChangedSig> NotifyNetworkActiveChanged; boost::signals2::signal<CClientUIInterface::NotifyAlertChangedSig> NotifyAlertChanged; - boost::signals2::signal<CClientUIInterface::LoadWalletSig> LoadWallet; boost::signals2::signal<CClientUIInterface::ShowProgressSig> ShowProgress; boost::signals2::signal<CClientUIInterface::NotifyBlockTipSig> NotifyBlockTip; boost::signals2::signal<CClientUIInterface::NotifyHeaderTipSig> NotifyHeaderTip; @@ -36,7 +35,6 @@ ADD_SIGNALS_IMPL_WRAPPER(InitMessage); ADD_SIGNALS_IMPL_WRAPPER(NotifyNumConnectionsChanged); ADD_SIGNALS_IMPL_WRAPPER(NotifyNetworkActiveChanged); ADD_SIGNALS_IMPL_WRAPPER(NotifyAlertChanged); -ADD_SIGNALS_IMPL_WRAPPER(LoadWallet); ADD_SIGNALS_IMPL_WRAPPER(ShowProgress); ADD_SIGNALS_IMPL_WRAPPER(NotifyBlockTip); ADD_SIGNALS_IMPL_WRAPPER(NotifyHeaderTip); @@ -48,7 +46,6 @@ void CClientUIInterface::InitMessage(const std::string& message) { return g_ui_s void CClientUIInterface::NotifyNumConnectionsChanged(int newNumConnections) { return g_ui_signals.NotifyNumConnectionsChanged(newNumConnections); } void CClientUIInterface::NotifyNetworkActiveChanged(bool networkActive) { return g_ui_signals.NotifyNetworkActiveChanged(networkActive); } void CClientUIInterface::NotifyAlertChanged() { return g_ui_signals.NotifyAlertChanged(); } -void CClientUIInterface::LoadWallet(std::unique_ptr<interfaces::Wallet>& wallet) { return g_ui_signals.LoadWallet(wallet); } void CClientUIInterface::ShowProgress(const std::string& title, int nProgress, bool resume_possible) { return g_ui_signals.ShowProgress(title, nProgress, resume_possible); } void CClientUIInterface::NotifyBlockTip(bool b, const CBlockIndex* i) { return g_ui_signals.NotifyBlockTip(b, i); } void CClientUIInterface::NotifyHeaderTip(bool b, const CBlockIndex* i) { return g_ui_signals.NotifyHeaderTip(b, i); } diff --git a/src/ui_interface.h b/src/ui_interface.h index 5e0380dc45..dad86ba697 100644 --- a/src/ui_interface.h +++ b/src/ui_interface.h @@ -18,10 +18,6 @@ class connection; } } // namespace boost -namespace interfaces { -class Wallet; -} // namespace interfaces - /** General change type (added, updated, removed). */ enum ChangeType { @@ -106,9 +102,6 @@ public: */ ADD_SIGNALS_DECL_WRAPPER(NotifyAlertChanged, void, ); - /** A wallet has been loaded. */ - ADD_SIGNALS_DECL_WRAPPER(LoadWallet, void, std::unique_ptr<interfaces::Wallet>& wallet); - /** * Show progress e.g. for verifychain. * resume_possible indicates shutting down now will result in the current progress action resuming upon restart. diff --git a/src/validation.h b/src/validation.h index 96d249b6d3..4490a75e18 100644 --- a/src/validation.h +++ b/src/validation.h @@ -22,7 +22,6 @@ #include <txdb.h> #include <versionbits.h> -#include <algorithm> #include <atomic> #include <exception> #include <map> diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 96fa50d42e..fe004a862d 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1599,7 +1599,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request) for (const std::pair<const uint256, CWalletTx>& pairWtx : pwallet->mapWallet) { CWalletTx tx = pairWtx.second; - if (depth == -1 || tx.GetDepthInMainChain(*locked_chain) < depth) { + if (depth == -1 || abs(tx.GetDepthInMainChain(*locked_chain)) < depth) { ListTransactions(*locked_chain, pwallet, tx, 0, true, transactions, filter, nullptr /* filter_label */); } } @@ -2924,7 +2924,7 @@ static UniValue listunspent(const JSONRPCRequest& request) CTxDestination address; const CScript& scriptPubKey = out.tx->tx->vout[out.i].scriptPubKey; bool fValidAddress = ExtractDestination(scriptPubKey, address); - bool reused = avoid_reuse && pwallet->IsUsedDestination(address); + bool reused = avoid_reuse && pwallet->IsUsedDestination(out.tx->GetHash(), out.i); if (destinations.size() && (!fValidAddress || !destinations.count(address))) continue; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 23f61602d2..4c3b633fe4 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -49,6 +49,7 @@ static const size_t OUTPUT_GROUP_MAX_ENTRIES = 10; static CCriticalSection cs_wallets; static std::vector<std::shared_ptr<CWallet>> vpwallets GUARDED_BY(cs_wallets); +static std::list<LoadWalletFn> g_load_wallet_fns GUARDED_BY(cs_wallets); bool AddWallet(const std::shared_ptr<CWallet>& wallet) { @@ -91,6 +92,13 @@ std::shared_ptr<CWallet> GetWallet(const std::string& name) return nullptr; } +std::unique_ptr<interfaces::Handler> HandleLoadWallet(LoadWalletFn load_wallet) +{ + LOCK(cs_wallets); + auto it = g_load_wallet_fns.emplace(g_load_wallet_fns.end(), std::move(load_wallet)); + return interfaces::MakeHandler([it] { LOCK(cs_wallets); g_load_wallet_fns.erase(it); }); +} + static Mutex g_wallet_release_mutex; static std::condition_variable g_wallet_release_cv; static std::set<std::string> g_unloading_wallet_set; @@ -1065,17 +1073,31 @@ void CWallet::SetUsedDestinationState(const uint256& hash, unsigned int n, bool } } -bool CWallet::IsUsedDestination(const CTxDestination& dst) const -{ - LOCK(cs_wallet); - return ::IsMine(*this, dst) && GetDestData(dst, "used", nullptr); -} - bool CWallet::IsUsedDestination(const uint256& hash, unsigned int n) const { + AssertLockHeld(cs_wallet); CTxDestination dst; const CWalletTx* srctx = GetWalletTx(hash); - return srctx && ExtractDestination(srctx->tx->vout[n].scriptPubKey, dst) && IsUsedDestination(dst); + if (srctx) { + assert(srctx->tx->vout.size() > n); + // When descriptor wallets arrive, these additional checks are + // likely superfluous and can be optimized out + for (const auto& keyid : GetAffectedKeys(srctx->tx->vout[n].scriptPubKey, *this)) { + WitnessV0KeyHash wpkh_dest(keyid); + if (GetDestData(wpkh_dest, "used", nullptr)) { + return true; + } + ScriptHash sh_wpkh_dest(GetScriptForDestination(wpkh_dest)); + if (GetDestData(sh_wpkh_dest, "used", nullptr)) { + return true; + } + PKHash pkh_dest(keyid); + if (GetDestData(pkh_dest, "used", nullptr)) { + return true; + } + } + } + return false; } bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) @@ -4585,7 +4607,12 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, } } - chain.loadWallet(interfaces::MakeWallet(walletInstance)); + { + LOCK(cs_wallets); + for (auto& load_wallet : g_load_wallet_fns) { + load_wallet(interfaces::MakeWallet(walletInstance)); + } + } // Register with the validation interface. It's ok to do this after rescan since we're still holding locked_chain. walletInstance->handleNotifications(); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 3428e8e001..ee4ac91588 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -36,6 +36,8 @@ #include <boost/signals2/signal.hpp> +using LoadWalletFn = std::function<void(std::unique_ptr<interfaces::Wallet> wallet)>; + //! Explicitly unload and delete the wallet. //! Blocks the current thread after signaling the unload intent so that all //! wallet clients release the wallet. @@ -49,6 +51,7 @@ bool HasWallets(); std::vector<std::shared_ptr<CWallet>> GetWallets(); std::shared_ptr<CWallet> GetWallet(const std::string& name); std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocation& location, std::string& error, std::string& warning); +std::unique_ptr<interfaces::Handler> HandleLoadWallet(LoadWalletFn load_wallet); enum class WalletCreationStatus { SUCCESS, @@ -1001,9 +1004,8 @@ public: bool IsSpent(interfaces::Chain::Lock& locked_chain, const uint256& hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - // Whether this or any UTXO with the same CTxDestination has been spent. - bool IsUsedDestination(const CTxDestination& dst) const; - bool IsUsedDestination(const uint256& hash, unsigned int n) const; + // Whether this or any known UTXO with the same single key has been spent. + bool IsUsedDestination(const uint256& hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void SetUsedDestinationState(const uint256& hash, unsigned int n, bool used); std::vector<OutputGroup> GroupOutputs(const std::vector<COutput>& outputs, bool single_coin) const; diff --git a/src/zmq/zmqpublishnotifier.cpp b/src/zmq/zmqpublishnotifier.cpp index ba89d1401d..233a45d19f 100644 --- a/src/zmq/zmqpublishnotifier.cpp +++ b/src/zmq/zmqpublishnotifier.cpp @@ -112,7 +112,8 @@ bool CZMQAbstractPublishNotifier::Initialize(void *pcontext) void CZMQAbstractPublishNotifier::Shutdown() { - assert(psocket); + // Early return if Initialize was not called + if (!psocket) return; int count = mapPublishNotifiers.count(address); diff --git a/test/functional/interface_zmq.py b/test/functional/interface_zmq.py index 5aea10fbce..89c55f31f3 100755 --- a/test/functional/interface_zmq.py +++ b/test/functional/interface_zmq.py @@ -59,6 +59,10 @@ class ZMQTest (BitcoinTestFramework): # Note that the publishing order is not defined in the documentation and # is subject to change. import zmq + + # Invalid zmq arguments don't take down the node, see #17185. + self.restart_node(0, ["-zmqpubrawtx=foo", "-zmqpubhashtx=bar"]) + address = 'tcp://127.0.0.1:28332' socket = self.ctx.socket(zmq.SUB) socket.set(zmq.RCVTIMEO, 60000) diff --git a/test/functional/wallet_avoidreuse.py b/test/functional/wallet_avoidreuse.py index 3c8064ea2d..55b30afdee 100755 --- a/test/functional/wallet_avoidreuse.py +++ b/test/functional/wallet_avoidreuse.py @@ -83,7 +83,12 @@ class AvoidReuseTest(BitcoinTestFramework): reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) self.test_fund_send_fund_senddirty() reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) - self.test_fund_send_fund_send() + self.test_fund_send_fund_send("legacy") + reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) + self.test_fund_send_fund_send("p2sh-segwit") + reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) + self.test_fund_send_fund_send("bech32") + def test_persistence(self): '''Test that wallet files persist the avoid_reuse flag.''' @@ -174,7 +179,7 @@ class AvoidReuseTest(BitcoinTestFramework): assert_approx(self.nodes[1].getbalance(), 5, 0.001) assert_approx(self.nodes[1].getbalance(avoid_reuse=False), 5, 0.001) - def test_fund_send_fund_send(self): + def test_fund_send_fund_send(self, second_addr_type): ''' Test the simple case where [1] generates a new address A, then [0] sends 10 BTC to A. @@ -184,7 +189,7 @@ class AvoidReuseTest(BitcoinTestFramework): [1] tries to spend 4 BTC (succeeds; change address sufficient) ''' - fundaddr = self.nodes[1].getnewaddress() + fundaddr = self.nodes[1].getnewaddress(label="", address_type="legacy") retaddr = self.nodes[0].getnewaddress() self.nodes[0].sendtoaddress(fundaddr, 10) @@ -205,7 +210,19 @@ class AvoidReuseTest(BitcoinTestFramework): # getbalances should show no used, 5 btc trusted assert_balances(self.nodes[1], mine={"used": 0, "trusted": 5}) - self.nodes[0].sendtoaddress(fundaddr, 10) + # For the second send, we transmute it to a related single-key address + # to make sure it's also detected as re-use + fund_spk = self.nodes[0].getaddressinfo(fundaddr)["scriptPubKey"] + fund_decoded = self.nodes[0].decodescript(fund_spk) + if second_addr_type == "p2sh-segwit": + new_fundaddr = fund_decoded["segwit"]["p2sh-segwit"] + elif second_addr_type == "bech32": + new_fundaddr = fund_decoded["segwit"]["addresses"][0] + else: + new_fundaddr = fundaddr + assert_equal(second_addr_type, "legacy") + + self.nodes[0].sendtoaddress(new_fundaddr, 10) self.nodes[0].generate(1) self.sync_all() diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py index 4aeb393255..455e89e310 100755 --- a/test/functional/wallet_listsinceblock.py +++ b/test/functional/wallet_listsinceblock.py @@ -5,6 +5,7 @@ """Test the listsincelast RPC.""" from test_framework.test_framework import BitcoinTestFramework +from test_framework.messages import BIP125_SEQUENCE_NUMBER from test_framework.util import ( assert_array_result, assert_equal, @@ -12,6 +13,7 @@ from test_framework.util import ( connect_nodes, ) +from decimal import Decimal class ListSinceBlockTest(BitcoinTestFramework): def set_test_params(self): @@ -33,6 +35,7 @@ class ListSinceBlockTest(BitcoinTestFramework): self.test_reorg() self.test_double_spend() self.test_double_send() + self.double_spends_filtered() def test_no_blockhash(self): txid = self.nodes[2].sendtoaddress(self.nodes[0].getnewaddress(), 1) @@ -291,5 +294,51 @@ class ListSinceBlockTest(BitcoinTestFramework): if tx['txid'] == txid1: assert_equal(tx['confirmations'], 2) + def double_spends_filtered(self): + ''' + `listsinceblock` was returning conflicted transactions even if they + occurred before the specified cutoff blockhash + ''' + spending_node = self.nodes[2] + dest_address = spending_node.getnewaddress() + + tx_input = dict( + sequence=BIP125_SEQUENCE_NUMBER, **next(u for u in spending_node.listunspent())) + rawtx = spending_node.createrawtransaction( + [tx_input], {dest_address: tx_input["amount"] - Decimal("0.00051000"), + spending_node.getrawchangeaddress(): Decimal("0.00050000")}) + signedtx = spending_node.signrawtransactionwithwallet(rawtx) + orig_tx_id = spending_node.sendrawtransaction(signedtx["hex"]) + original_tx = spending_node.gettransaction(orig_tx_id) + + double_tx = spending_node.bumpfee(orig_tx_id) + + # check that both transactions exist + block_hash = spending_node.listsinceblock( + spending_node.getblockhash(spending_node.getblockcount())) + original_found = False + double_found = False + for tx in block_hash['transactions']: + if tx['txid'] == original_tx['txid']: + original_found = True + if tx['txid'] == double_tx['txid']: + double_found = True + assert_equal(original_found, True) + assert_equal(double_found, True) + + lastblockhash = spending_node.generate(1)[0] + + # check that neither transaction exists + block_hash = spending_node.listsinceblock(lastblockhash) + original_found = False + double_found = False + for tx in block_hash['transactions']: + if tx['txid'] == original_tx['txid']: + original_found = True + if tx['txid'] == double_tx['txid']: + double_found = True + assert_equal(original_found, False) + assert_equal(double_found, False) + if __name__ == '__main__': ListSinceBlockTest().main() |