diff --git a/src/cuckoocache.h b/src/cuckoocache.h index 4d0b094fa..16a994393 100644 --- a/src/cuckoocache.h +++ b/src/cuckoocache.h @@ -6,11 +6,11 @@ #define BITCOIN_CUCKOOCACHE_H #include -#include #include -#include #include +#include #include +#include #include diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index 126e3479f..80c79bf6c 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 wallet)>; +std::unique_ptr HandleLoadWallet(LoadWalletFn load_wallet) +{ + throw std::logic_error("Wallet function called in non-wallet build."); +} -class Wallet; +namespace interfaces { std::unique_ptr MakeWallet(const std::shared_ptr& wallet) { diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index b8b9ecded..aa44a69f0 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) 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 da670a337..e09958600 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) = 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 92601fc4e..4e235688f 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 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 m_cleanup; +}; + } // namespace std::unique_ptr MakeHandler(boost::signals2::connection connection) @@ -29,4 +38,9 @@ std::unique_ptr MakeHandler(boost::signals2::connection connection) return MakeUnique(std::move(connection)); } +std::unique_ptr MakeHandler(std::function cleanup) +{ + return MakeUnique(std::move(cleanup)); +} + } // namespace interfaces diff --git a/src/interfaces/handler.h b/src/interfaces/handler.h index c4c674cac..46918bc22 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 #include namespace boost { @@ -30,6 +31,9 @@ public: //! Return handler wrapping a boost signal connection. std::unique_ptr MakeHandler(boost::signals2::connection connection); +//! Return handler wrapping a cleanup function. +std::unique_ptr MakeHandler(std::function cleanup); + } // namespace interfaces #endif // BITCOIN_INTERFACES_HANDLER_H diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index c80a8789f..b4e1a4613 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -45,11 +45,10 @@ std::vector ListWalletDir(); std::vector> GetWallets(); std::shared_ptr 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& result); +std::unique_ptr HandleLoadWallet(interfaces::Node::LoadWalletFn load_wallet); namespace interfaces { -class Wallet; - namespace { class NodeImpl : public Node @@ -286,7 +285,7 @@ public: } std::unique_ptr handleLoadWallet(LoadWalletFn fn) override { - return MakeHandler(::uiInterface.LoadWallet_connect([fn](std::unique_ptr& wallet) { fn(std::move(wallet)); })); + return HandleLoadWallet(std::move(fn)); } std::unique_ptr handleNotifyNumConnectionsChanged(NotifyNumConnectionsChangedFn fn) override { diff --git a/src/prevector.h b/src/prevector.h index 9d576321b..34016879a 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -14,6 +14,7 @@ #include #include #include +#include #pragma pack(push, 1) /** Implements a drop-in replacement for std::vector which stores up to N diff --git a/src/qt/bantablemodel.cpp b/src/qt/bantablemodel.cpp index efc726e09..60a3178f9 100644 --- a/src/qt/bantablemodel.cpp +++ b/src/qt/bantablemodel.cpp @@ -10,7 +10,7 @@ #include #include -#include +#include #include #include diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index e269c91d1..784fdcb6e 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 99a9a12fe..ed6fecce9 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -11,7 +11,7 @@ #include #include -#include +#include #include #include diff --git a/src/qt/recentrequeststablemodel.cpp b/src/qt/recentrequeststablemodel.cpp index 1611ec823..64bfb7d64 100644 --- a/src/qt/recentrequeststablemodel.cpp +++ b/src/qt/recentrequeststablemodel.cpp @@ -11,8 +11,7 @@ #include #include -#include - +#include 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 130b709d4..66e8d4f22 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 94413547d..9f03e8b5a 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 156653f47..20fad08b0 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 e45e2d92c..0c344c773 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 d79644441..6c050ab96 100644 --- a/src/test/checkqueue_tests.cpp +++ b/src/test/checkqueue_tests.cpp @@ -18,6 +18,7 @@ #include #include +#include // BasicTestingSetup not sufficient because nScriptCheckThreads is not set // otherwise. diff --git a/src/ui_interface.cpp b/src/ui_interface.cpp index d31063714..3968464ef 100644 --- a/src/ui_interface.cpp +++ b/src/ui_interface.cpp @@ -16,7 +16,6 @@ struct UISignals { boost::signals2::signal NotifyNumConnectionsChanged; boost::signals2::signal NotifyNetworkActiveChanged; boost::signals2::signal NotifyAlertChanged; - boost::signals2::signal LoadWallet; boost::signals2::signal ShowProgress; boost::signals2::signal NotifyBlockTip; boost::signals2::signal 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& 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 5e0380dc4..dad86ba69 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& 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 96d249b6d..4490a75e1 100644 --- a/src/validation.h +++ b/src/validation.h @@ -22,7 +22,6 @@ #include #include -#include #include #include #include diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 96fa50d42..fe004a862 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& 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 23f61602d..4c3b633fe 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> vpwallets GUARDED_BY(cs_wallets); +static std::list g_load_wallet_fns GUARDED_BY(cs_wallets); bool AddWallet(const std::shared_ptr& wallet) { @@ -91,6 +92,13 @@ std::shared_ptr GetWallet(const std::string& name) return nullptr; } +std::unique_ptr 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 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::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 3428e8e00..ee4ac9158 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -36,6 +36,8 @@ #include +using LoadWalletFn = std::function 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> GetWallets(); std::shared_ptr GetWallet(const std::string& name); std::shared_ptr LoadWallet(interfaces::Chain& chain, const WalletLocation& location, std::string& error, std::string& warning); +std::unique_ptr 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 GroupOutputs(const std::vector& outputs, bool single_coin) const; diff --git a/src/zmq/zmqpublishnotifier.cpp b/src/zmq/zmqpublishnotifier.cpp index ba89d1401..233a45d19 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 5aea10fbc..89c55f31f 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 3c8064ea2..55b30afde 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 4aeb39325..455e89e31 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()