From 10b4729e33f76092bd8cfa06d1a5e0a066436f76 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Wed, 24 Jul 2019 15:41:41 -0400 Subject: [PATCH 1/9] Pass block height in Chain::BlockConnected/Chain::BlockDisconnected To do so we update CValidationInterface::BlockDisconnect to take a CBlockIndex pointing to the block being disconnected. This new parameter will be use in the following commit to establish wallet height. --- src/interfaces/chain.cpp | 6 +++--- src/interfaces/chain.h | 4 ++-- src/test/validation_block_tests.cpp | 3 ++- src/validation.cpp | 2 +- src/validationinterface.cpp | 11 ++++++----- src/validationinterface.h | 4 ++-- src/wallet/wallet.cpp | 6 ++++-- src/wallet/wallet.h | 4 ++-- src/zmq/zmqnotificationinterface.cpp | 2 +- src/zmq/zmqnotificationinterface.h | 2 +- 10 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 23099a7799a..c2a8faf8c3f 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -182,11 +182,11 @@ public: const CBlockIndex* index, const std::vector& tx_conflicted) override { - m_notifications->BlockConnected(*block, tx_conflicted); + m_notifications->BlockConnected(*block, tx_conflicted, index->nHeight); } - void BlockDisconnected(const std::shared_ptr& block) override + void BlockDisconnected(const std::shared_ptr& block, const CBlockIndex* index) override { - m_notifications->BlockDisconnected(*block); + m_notifications->BlockDisconnected(*block, index->nHeight); } void UpdatedBlockTip(const CBlockIndex* index, const CBlockIndex* fork_index, bool is_ibd) override { diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 82eeba11605..4cb2aba2c99 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -226,8 +226,8 @@ public: virtual ~Notifications() {} virtual void TransactionAddedToMempool(const CTransactionRef& tx) {} virtual void TransactionRemovedFromMempool(const CTransactionRef& ptx) {} - virtual void BlockConnected(const CBlock& block, const std::vector& tx_conflicted) {} - virtual void BlockDisconnected(const CBlock& block) {} + virtual void BlockConnected(const CBlock& block, const std::vector& tx_conflicted, int height) {} + virtual void BlockDisconnected(const CBlock& block, int height) {} virtual void UpdatedBlockTip() {} virtual void ChainStateFlushed(const CBlockLocator& locator) {} }; diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index ae998e92a57..26ca9a31627 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -40,9 +40,10 @@ struct TestSubscriber : public CValidationInterface { m_expected_tip = block->GetHash(); } - void BlockDisconnected(const std::shared_ptr& block) override + void BlockDisconnected(const std::shared_ptr& block, const CBlockIndex* pindex) override { BOOST_CHECK_EQUAL(m_expected_tip, block->GetHash()); + BOOST_CHECK_EQUAL(m_expected_tip, pindex->GetBlockHash()); m_expected_tip = block->hashPrevBlock; } diff --git a/src/validation.cpp b/src/validation.cpp index 11072b6038d..7be97df78a6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2435,7 +2435,7 @@ bool CChainState::DisconnectTip(BlockValidationState& state, const CChainParams& UpdateTip(pindexDelete->pprev, chainparams); // Let wallets know transactions went from 1-confirmed to // 0-confirmed or conflicted: - GetMainSignals().BlockDisconnected(pblock); + GetMainSignals().BlockDisconnected(pblock, pindexDelete); return true; } diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index a46b4003f1d..663308bae95 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -29,7 +29,7 @@ struct MainSignalsInstance { boost::signals2::signal UpdatedBlockTip; boost::signals2::signal TransactionAddedToMempool; boost::signals2::signal &, const CBlockIndex *pindex, const std::vector&)> BlockConnected; - boost::signals2::signal &)> BlockDisconnected; + boost::signals2::signal&, const CBlockIndex* pindex)> BlockDisconnected; boost::signals2::signal TransactionRemovedFromMempool; boost::signals2::signal ChainStateFlushed; boost::signals2::signal BlockChecked; @@ -92,7 +92,7 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) { conns.UpdatedBlockTip = g_signals.m_internals->UpdatedBlockTip.connect(std::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); conns.TransactionAddedToMempool = g_signals.m_internals->TransactionAddedToMempool.connect(std::bind(&CValidationInterface::TransactionAddedToMempool, pwalletIn, std::placeholders::_1)); conns.BlockConnected = g_signals.m_internals->BlockConnected.connect(std::bind(&CValidationInterface::BlockConnected, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); - conns.BlockDisconnected = g_signals.m_internals->BlockDisconnected.connect(std::bind(&CValidationInterface::BlockDisconnected, pwalletIn, std::placeholders::_1)); + conns.BlockDisconnected = g_signals.m_internals->BlockDisconnected.connect(std::bind(&CValidationInterface::BlockDisconnected, pwalletIn, std::placeholders::_1, std::placeholders::_2)); conns.TransactionRemovedFromMempool = g_signals.m_internals->TransactionRemovedFromMempool.connect(std::bind(&CValidationInterface::TransactionRemovedFromMempool, pwalletIn, std::placeholders::_1)); conns.ChainStateFlushed = g_signals.m_internals->ChainStateFlushed.connect(std::bind(&CValidationInterface::ChainStateFlushed, pwalletIn, std::placeholders::_1)); conns.BlockChecked = g_signals.m_internals->BlockChecked.connect(std::bind(&CValidationInterface::BlockChecked, pwalletIn, std::placeholders::_1, std::placeholders::_2)); @@ -156,9 +156,10 @@ void CMainSignals::BlockConnected(const std::shared_ptr &pblock, c }); } -void CMainSignals::BlockDisconnected(const std::shared_ptr &pblock) { - m_internals->m_schedulerClient.AddToProcessQueue([pblock, this] { - m_internals->BlockDisconnected(pblock); +void CMainSignals::BlockDisconnected(const std::shared_ptr& pblock, const CBlockIndex* pindex) +{ + m_internals->m_schedulerClient.AddToProcessQueue([pblock, pindex, this] { + m_internals->BlockDisconnected(pblock, pindex); }); } diff --git a/src/validationinterface.h b/src/validationinterface.h index dc8425869b2..6a8059a6a01 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -114,7 +114,7 @@ protected: * * Called on a background thread. */ - virtual void BlockDisconnected(const std::shared_ptr &block) {} + virtual void BlockDisconnected(const std::shared_ptr &block, const CBlockIndex* pindex) {} /** * Notifies listeners of the new active block chain on-disk. * @@ -178,7 +178,7 @@ public: void UpdatedBlockTip(const CBlockIndex *, const CBlockIndex *, bool fInitialDownload); void TransactionAddedToMempool(const CTransactionRef &); void BlockConnected(const std::shared_ptr &, const CBlockIndex *pindex, const std::shared_ptr> &); - void BlockDisconnected(const std::shared_ptr &); + void BlockDisconnected(const std::shared_ptr &, const CBlockIndex* pindex); void ChainStateFlushed(const CBlockLocator &); void BlockChecked(const CBlock&, const BlockValidationState&); void NewPoWValidBlock(const CBlockIndex *, const std::shared_ptr&); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0b7dc256adb..365a296e6f1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1052,7 +1052,8 @@ void CWallet::TransactionRemovedFromMempool(const CTransactionRef &ptx) { } } -void CWallet::BlockConnected(const CBlock& block, const std::vector& vtxConflicted) { +void CWallet::BlockConnected(const CBlock& block, const std::vector& vtxConflicted, int height) +{ const uint256& block_hash = block.GetHash(); auto locked_chain = chain().lock(); LOCK(cs_wallet); @@ -1068,7 +1069,8 @@ void CWallet::BlockConnected(const CBlock& block, const std::vector& vtxConflicted) override; - void BlockDisconnected(const CBlock& block) override; + void BlockConnected(const CBlock& block, const std::vector& vtxConflicted, int height) override; + void BlockDisconnected(const CBlock& block, int height) override; void UpdatedBlockTip() override; int64_t RescanFromTime(int64_t startTime, const WalletRescanReserver& reserver, bool update); diff --git a/src/zmq/zmqnotificationinterface.cpp b/src/zmq/zmqnotificationinterface.cpp index ebbaf8683d5..b3b97b6a2ae 100644 --- a/src/zmq/zmqnotificationinterface.cpp +++ b/src/zmq/zmqnotificationinterface.cpp @@ -185,7 +185,7 @@ void CZMQNotificationInterface::BlockConnected(const std::shared_ptr& pblock) +void CZMQNotificationInterface::BlockDisconnected(const std::shared_ptr& pblock, const CBlockIndex* pindexDisconnected) { for (const CTransactionRef& ptx : pblock->vtx) { // Do a normal notify for each transaction removed in block disconnection diff --git a/src/zmq/zmqnotificationinterface.h b/src/zmq/zmqnotificationinterface.h index 6be0554a656..8bf9b0ba470 100644 --- a/src/zmq/zmqnotificationinterface.h +++ b/src/zmq/zmqnotificationinterface.h @@ -27,7 +27,7 @@ protected: // CValidationInterface void TransactionAddedToMempool(const CTransactionRef& tx) override; void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected, const std::vector& vtxConflicted) override; - void BlockDisconnected(const std::shared_ptr& pblock) override; + void BlockDisconnected(const std::shared_ptr& pblock, const CBlockIndex* pindexDisconnected) override; void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override; private: From 5aacc3eff15b9b5bdc951f1e274f00d581f63bce Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Sat, 20 Apr 2019 12:02:52 -0400 Subject: [PATCH 2/9] Add m_last_block_processed_height field in CWallet At BlockConnected/BlockDisconnected, we rely on height of block itself to know current height of wallet --- src/qt/test/wallettests.cpp | 2 ++ src/wallet/test/wallet_tests.cpp | 22 ++++++++++++++++++++++ src/wallet/wallet.cpp | 8 ++++++-- src/wallet/wallet.h | 27 ++++++++++++++++++++++++--- 4 files changed, 54 insertions(+), 5 deletions(-) diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 881653cdac0..6e009145a24 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -139,10 +139,12 @@ void TestGUI(interfaces::Node& node) wallet->LoadWallet(firstRun); { auto spk_man = wallet->GetLegacyScriptPubKeyMan(); + auto locked_chain = wallet->chain().lock(); LOCK(wallet->cs_wallet); AssertLockHeld(spk_man->cs_wallet); wallet->SetAddressBook(GetDestinationForKey(test.coinbaseKey.GetPubKey(), wallet->m_default_address_type), "", "receive"); spk_man->AddKeyPubKey(test.coinbaseKey, test.coinbaseKey.GetPubKey()); + wallet->SetLastBlockProcessed(105, ::ChainActive().Tip()->GetBlockHash()); } { auto locked_chain = wallet->chain().lock(); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 72e1b4e83ba..b9328634e55 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -50,6 +50,10 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Verify ScanForWalletTransactions accommodates a null start block. { CWallet wallet(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + { + LOCK(wallet.cs_wallet); + wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); + } AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(&wallet); reserver.reserve(); @@ -65,6 +69,10 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // and new block files. { CWallet wallet(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + { + LOCK(wallet.cs_wallet); + wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); + } AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(&wallet); reserver.reserve(); @@ -84,6 +92,10 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // file. { CWallet wallet(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + { + LOCK(wallet.cs_wallet); + wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); + } AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(&wallet); reserver.reserve(); @@ -102,6 +114,10 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Verify ScanForWalletTransactions scans no blocks. { CWallet wallet(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + { + LOCK(wallet.cs_wallet); + wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); + } AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(&wallet); reserver.reserve(); @@ -258,6 +274,7 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) LockAssertion lock(::cs_main); LOCK(wallet.cs_wallet); AssertLockHeld(spk_man->cs_wallet); + wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); wtx.SetConf(CWalletTx::Status::CONFIRMED, ::ChainActive().Tip()->GetBlockHash(), 0); @@ -435,6 +452,10 @@ public: { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); wallet = MakeUnique(m_chain.get(), WalletLocation(), WalletDatabase::CreateMock()); + { + LOCK(wallet->cs_wallet); + wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); + } bool firstRun; wallet->LoadWallet(firstRun); AddKey(*wallet, coinbaseKey); @@ -473,6 +494,7 @@ public: LOCK(cs_main); LOCK(wallet->cs_wallet); + wallet->SetLastBlockProcessed(wallet->GetLastBlockHeight() + 1, ::ChainActive().Tip()->GetBlockHash()); auto it = wallet->mapWallet.find(tx->GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); it->second.SetConf(CWalletTx::Status::CONFIRMED, ::ChainActive().Tip()->GetBlockHash(), 1); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 365a296e6f1..6c5754aa1fe 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1058,6 +1058,8 @@ void CWallet::BlockConnected(const CBlock& block, const std::vector CWallet::CreateWalletFromFile(interfaces::Chain& chain, const Optional tip_height = locked_chain->getHeight(); if (tip_height) { walletInstance->m_last_block_processed = locked_chain->getBlockHash(*tip_height); + walletInstance->m_last_block_processed_height = *tip_height; } else { walletInstance->m_last_block_processed.SetNull(); + walletInstance->m_last_block_processed_height = -1; } if (tip_height && *tip_height != rescan_height) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index dba8a8cc08d..7c22f06e2cc 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -679,12 +679,18 @@ private: * The following is used to keep track of how far behind the wallet is * from the chain sync, and to allow clients to block on us being caught up. * - * Note that this is *not* how far we've processed, we may need some rescan - * to have seen all transactions in the chain, but is only used to track - * live BlockConnected callbacks. + * Processed hash is a pointer on node's tip and doesn't imply that the wallet + * has scanned sequentially all blocks up to this one. */ uint256 m_last_block_processed GUARDED_BY(cs_wallet); + /* Height of last block processed is used by wallet to know depth of transactions + * without relying on Chain interface beyond asynchronous updates. For safety, we + * initialize it to -1. Height is a pointer on node's tip and doesn't imply + * that the wallet has scanned sequentially all blocks up to this one. + */ + int m_last_block_processed_height GUARDED_BY(cs_wallet) = -1; + public: /* * Main wallet lock. @@ -1129,6 +1135,21 @@ public: LegacyScriptPubKeyMan::WatchKeyMap& mapWatchKeys GUARDED_BY(cs_KeyStore) = m_spk_man->mapWatchKeys; WalletBatch*& encrypted_batch GUARDED_BY(cs_wallet) = m_spk_man->encrypted_batch; using CryptedKeyMap = LegacyScriptPubKeyMan::CryptedKeyMap; + + /** Get last block processed height */ + int GetLastBlockHeight() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) + { + AssertLockHeld(cs_wallet); + assert(m_last_block_processed_height >= 0); + return m_last_block_processed_height; + }; + /** Set last block processed height, currently only use in unit test */ + void SetLastBlockProcessed(int block_height, uint256 block_hash) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) + { + AssertLockHeld(cs_wallet); + m_last_block_processed_height = block_height; + m_last_block_processed = block_hash; + }; }; /** From 9700fcb47feca9d78e005b8d18b41148c8f6b25f Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Thu, 24 Oct 2019 12:53:57 -0400 Subject: [PATCH 3/9] Replace CWalletTx::SetConf by Confirmation initialization list --- src/wallet/rpcdump.cpp | 3 ++- src/wallet/test/wallet_tests.cpp | 9 +++++--- src/wallet/wallet.cpp | 38 +++++++++++++------------------- src/wallet/wallet.h | 13 +++++------ 4 files changed, 29 insertions(+), 34 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index da4da4d9e0f..bc6df1cc99e 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -382,7 +382,8 @@ UniValue importprunedfunds(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Something wrong with merkleblock"); } - wtx.SetConf(CWalletTx::Status::CONFIRMED, merkleBlock.header.GetHash(), txnIndex); + CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, merkleBlock.header.GetHash(), txnIndex); + wtx.m_confirm = confirm; auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index b9328634e55..b6acaf4d999 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -276,7 +276,8 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) AssertLockHeld(spk_man->cs_wallet); wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); - wtx.SetConf(CWalletTx::Status::CONFIRMED, ::ChainActive().Tip()->GetBlockHash(), 0); + CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, ::ChainActive().Tip()->GetBlockHash(), 0); + wtx.m_confirm = confirm; // Call GetImmatureCredit() once before adding the key to the wallet to // cache the current immature credit amount, which is 0. @@ -317,7 +318,8 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64 wallet.AddToWallet(wtx); } if (block) { - wtx.SetConf(CWalletTx::Status::CONFIRMED, block->GetBlockHash(), 0); + CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, block->GetBlockHash(), 0); + wtx.m_confirm = confirm; } wallet.AddToWallet(wtx); return wallet.mapWallet.at(wtx.GetHash()).nTimeSmart; @@ -497,7 +499,8 @@ public: wallet->SetLastBlockProcessed(wallet->GetLastBlockHeight() + 1, ::ChainActive().Tip()->GetBlockHash()); auto it = wallet->mapWallet.find(tx->GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); - it->second.SetConf(CWalletTx::Status::CONFIRMED, ::ChainActive().Tip()->GetBlockHash(), 1); + CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, ::ChainActive().Tip()->GetBlockHash(), 1); + it->second.m_confirm = confirm; return it->second; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6c5754aa1fe..2ee1d001b91 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -848,19 +848,19 @@ void CWallet::LoadToWallet(CWalletTx& wtxIn) } } -bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Status status, const uint256& block_hash, int posInBlock, bool fUpdate) +bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, bool fUpdate) { const CTransaction& tx = *ptx; { AssertLockHeld(cs_wallet); - if (!block_hash.IsNull()) { + if (!confirm.hashBlock.IsNull()) { for (const CTxIn& txin : tx.vin) { std::pair range = mapTxSpends.equal_range(txin.prevout); while (range.first != range.second) { if (range.first->second != tx.GetHash()) { - WalletLogPrintf("Transaction %s (in block %s) conflicts with wallet transaction %s (both spend %s:%i)\n", tx.GetHash().ToString(), block_hash.ToString(), range.first->second.ToString(), range.first->first.hash.ToString(), range.first->first.n); - MarkConflicted(block_hash, range.first->second); + WalletLogPrintf("Transaction %s (in block %s) conflicts with wallet transaction %s (both spend %s:%i)\n", tx.GetHash().ToString(), confirm.hashBlock.ToString(), range.first->second.ToString(), range.first->first.hash.ToString(), range.first->first.n); + MarkConflicted(confirm.hashBlock, range.first->second); } range.first++; } @@ -888,7 +888,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::St // Block disconnection override an abandoned tx as unconfirmed // which means user may have to call abandontransaction again - wtx.SetConf(status, block_hash, posInBlock); + wtx.m_confirm = confirm; return AddToWallet(wtx, false); } @@ -1022,9 +1022,9 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) } } -void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Status status, const uint256& block_hash, int posInBlock, bool update_tx) +void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, bool update_tx) { - if (!AddToWalletIfInvolvingMe(ptx, status, block_hash, posInBlock, update_tx)) + if (!AddToWalletIfInvolvingMe(ptx, confirm, update_tx)) return; // Not one of ours // If a transaction changes 'conflicted' state, that changes the balance @@ -1036,7 +1036,8 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Status stat void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx) { auto locked_chain = chain().lock(); LOCK(cs_wallet); - SyncTransaction(ptx, CWalletTx::Status::UNCONFIRMED, {} /* block hash */, 0 /* position in block */); + CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, {}, 0); + SyncTransaction(ptx, confirm); auto it = mapWallet.find(ptx->GetHash()); if (it != mapWallet.end()) { @@ -1061,7 +1062,8 @@ void CWallet::BlockConnected(const CBlock& block, const std::vector GetConflicts() const NO_THREAD_SAFETY_ANALYSIS; - void SetConf(Status status, const uint256& block_hash, int posInBlock); - /** * Return depth of transaction in blockchain: * <0 : conflicts with a transaction this deep in the blockchain @@ -642,7 +641,7 @@ private: * Abandoned state should probably be more carefully tracked via different * posInBlock signals or by checking mempool presence when necessary. */ - bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, CWalletTx::Status status, const uint256& block_hash, int posInBlock, bool fUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, CWalletTx::Confirmation confirm, bool fUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /* Mark a transaction (and its in-wallet descendants) as conflicting with a particular block. */ void MarkConflicted(const uint256& hashBlock, const uint256& hashTx); @@ -654,7 +653,7 @@ private: /* Used by TransactionAddedToMemorypool/BlockConnected/Disconnected/ScanForWalletTransactions. * Should be called with non-zero block_hash and posInBlock if this is for a transaction that is included in a block. */ - void SyncTransaction(const CTransactionRef& tx, CWalletTx::Status status, const uint256& block_hash, int posInBlock = 0, bool update_tx = true) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void SyncTransaction(const CTransactionRef& tx, CWalletTx::Confirmation confirm, bool update_tx = true) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); std::atomic m_wallet_flags{0}; From 5971d3848e09abf571e5308185275296127efca4 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Sat, 20 Apr 2019 11:22:59 -0400 Subject: [PATCH 4/9] Add block_height field in struct Confirmation At wallet loading, we rely on chain state querying to retrieve height of txn, to do so we ensure that lock order is respected between cs_main and cs_wallet. If wallet loaded is the wallet-tool one, all wallet txn will show up with a height of zero. It doesn't matter as confirmation height is not used by wallet-tool. Reorder arguments and document Confirmation calls to avoid ambiguity. Fixes nits left from #16624 --- src/wallet/rpcdump.cpp | 6 +++-- src/wallet/test/wallet_tests.cpp | 6 ++--- src/wallet/wallet.cpp | 42 ++++++++++++++++++++------------ src/wallet/wallet.h | 15 +++++++----- 4 files changed, 43 insertions(+), 26 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index bc6df1cc99e..ddb8bfaf179 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -364,10 +364,12 @@ UniValue importprunedfunds(const JSONRPCRequest& request) std::vector vMatch; std::vector vIndex; unsigned int txnIndex = 0; + Optional height; if (merkleBlock.txn.ExtractMatches(vMatch, vIndex) == merkleBlock.header.hashMerkleRoot) { auto locked_chain = pwallet->chain().lock(); - if (locked_chain->getBlockHeight(merkleBlock.header.GetHash()) == nullopt) { + height = locked_chain->getBlockHeight(merkleBlock.header.GetHash()); + if (height == nullopt) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found in chain"); } @@ -382,7 +384,7 @@ UniValue importprunedfunds(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Something wrong with merkleblock"); } - CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, merkleBlock.header.GetHash(), txnIndex); + CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, *height, merkleBlock.header.GetHash(), txnIndex); wtx.m_confirm = confirm; auto locked_chain = pwallet->chain().lock(); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index b6acaf4d999..3f0e40149cf 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -276,7 +276,7 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) AssertLockHeld(spk_man->cs_wallet); wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); - CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, ::ChainActive().Tip()->GetBlockHash(), 0); + CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, ::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash(), 0); wtx.m_confirm = confirm; // Call GetImmatureCredit() once before adding the key to the wallet to @@ -318,7 +318,7 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64 wallet.AddToWallet(wtx); } if (block) { - CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, block->GetBlockHash(), 0); + CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, block->nHeight, block->GetBlockHash(), 0); wtx.m_confirm = confirm; } wallet.AddToWallet(wtx); @@ -499,7 +499,7 @@ public: wallet->SetLastBlockProcessed(wallet->GetLastBlockHeight() + 1, ::ChainActive().Tip()->GetBlockHash()); auto it = wallet->mapWallet.find(tx->GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); - CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, ::ChainActive().Tip()->GetBlockHash(), 1); + CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, ::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash(), 1); it->second.m_confirm = confirm; return it->second; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2ee1d001b91..512273aa822 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -766,10 +766,12 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) wtx.m_confirm.status = wtxIn.m_confirm.status; wtx.m_confirm.nIndex = wtxIn.m_confirm.nIndex; wtx.m_confirm.hashBlock = wtxIn.m_confirm.hashBlock; + wtx.m_confirm.block_height = wtxIn.m_confirm.block_height; fUpdated = true; } else { assert(wtx.m_confirm.nIndex == wtxIn.m_confirm.nIndex); assert(wtx.m_confirm.hashBlock == wtxIn.m_confirm.hashBlock); + assert(wtx.m_confirm.block_height == wtxIn.m_confirm.block_height); } if (wtxIn.fFromMe && wtxIn.fFromMe != wtx.fFromMe) { @@ -820,12 +822,22 @@ void CWallet::LoadToWallet(CWalletTx& wtxIn) { // If wallet doesn't have a chain (e.g wallet-tool), lock can't be taken. auto locked_chain = LockChain(); - // If tx hasn't been reorged out of chain while wallet being shutdown - // change tx status to UNCONFIRMED and reset hashBlock/nIndex. - if (!wtxIn.m_confirm.hashBlock.IsNull()) { - if (locked_chain && !locked_chain->getBlockHeight(wtxIn.m_confirm.hashBlock)) { + if (locked_chain) { + Optional block_height = locked_chain->getBlockHeight(wtxIn.m_confirm.hashBlock); + if (block_height) { + // Update cached block height variable since it not stored in the + // serialized transaction. + wtxIn.m_confirm.block_height = *block_height; + } else if (wtxIn.isConflicted() || wtxIn.isConfirmed()) { + // If tx block (or conflicting block) was reorged out of chain + // while the wallet was shutdown, change tx status to UNCONFIRMED + // and reset block height, hash, and index. ABANDONED tx don't have + // associated blocks and don't need to be updated. The case where a + // transaction was reorged out while online and then reconfirmed + // while offline is covered by the rescan logic. wtxIn.setUnconfirmed(); wtxIn.m_confirm.hashBlock = uint256(); + wtxIn.m_confirm.block_height = 0; wtxIn.m_confirm.nIndex = 0; } } @@ -842,7 +854,7 @@ void CWallet::LoadToWallet(CWalletTx& wtxIn) if (it != mapWallet.end()) { CWalletTx& prevtx = it->second; if (prevtx.isConflicted()) { - MarkConflicted(prevtx.m_confirm.hashBlock, wtx.GetHash()); + MarkConflicted(prevtx.m_confirm.hashBlock, prevtx.m_confirm.block_height, wtx.GetHash()); } } } @@ -860,7 +872,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Co while (range.first != range.second) { if (range.first->second != tx.GetHash()) { WalletLogPrintf("Transaction %s (in block %s) conflicts with wallet transaction %s (both spend %s:%i)\n", tx.GetHash().ToString(), confirm.hashBlock.ToString(), range.first->second.ToString(), range.first->first.hash.ToString(), range.first->first.n); - MarkConflicted(confirm.hashBlock, range.first->second); + MarkConflicted(confirm.hashBlock, confirm.block_height, range.first->second); } range.first++; } @@ -948,7 +960,6 @@ bool CWallet::AbandonTransaction(interfaces::Chain::Lock& locked_chain, const ui if (currentconfirm == 0 && !wtx.isAbandoned()) { // If the orig tx was not in block/mempool, none of its spends can be in mempool assert(!wtx.InMempool()); - wtx.m_confirm.nIndex = 0; wtx.setAbandoned(); wtx.MarkDirty(); batch.WriteTx(wtx); @@ -970,7 +981,7 @@ bool CWallet::AbandonTransaction(interfaces::Chain::Lock& locked_chain, const ui return true; } -void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) +void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx) { auto locked_chain = chain().lock(); LOCK(cs_wallet); @@ -1004,6 +1015,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) // Mark transaction as conflicted with this block. wtx.m_confirm.nIndex = 0; wtx.m_confirm.hashBlock = hashBlock; + wtx.m_confirm.block_height = conflicting_height; wtx.setConflicted(); wtx.MarkDirty(); batch.WriteTx(wtx); @@ -1036,7 +1048,7 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmatio void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx) { auto locked_chain = chain().lock(); LOCK(cs_wallet); - CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, {}, 0); + CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0); SyncTransaction(ptx, confirm); auto it = mapWallet.find(ptx->GetHash()); @@ -1061,10 +1073,10 @@ void CWallet::BlockConnected(const CBlock& block, const std::vectorGetHash(); } bool IsCoinBase() const { return tx->IsCoinBase(); } @@ -644,7 +647,7 @@ private: bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, CWalletTx::Confirmation confirm, bool fUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /* Mark a transaction (and its in-wallet descendants) as conflicting with a particular block. */ - void MarkConflicted(const uint256& hashBlock, const uint256& hashTx); + void MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx); /* Mark a transaction's inputs dirty, thus forcing the outputs to be recomputed */ void MarkInputsDirty(const CTransactionRef& tx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); From 769ff05e48fb53d4b62c59060424a0fea71d0aab Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Wed, 6 Nov 2019 12:00:39 -0500 Subject: [PATCH 5/9] Refactor some importprunedfunds checks with guard clause Credit to jkczyz --- src/wallet/rpcdump.cpp | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index ddb8bfaf179..d70f68f945c 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -363,31 +363,26 @@ UniValue importprunedfunds(const JSONRPCRequest& request) //Search partial merkle tree in proof for our transaction and index in valid block std::vector vMatch; std::vector vIndex; - unsigned int txnIndex = 0; - Optional height; - if (merkleBlock.txn.ExtractMatches(vMatch, vIndex) == merkleBlock.header.hashMerkleRoot) { - - auto locked_chain = pwallet->chain().lock(); - height = locked_chain->getBlockHeight(merkleBlock.header.GetHash()); - if (height == nullopt) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found in chain"); - } - - std::vector::const_iterator it; - if ((it = std::find(vMatch.begin(), vMatch.end(), hashTx))==vMatch.end()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction given doesn't exist in proof"); - } - - txnIndex = vIndex[it - vMatch.begin()]; - } - else { + if (merkleBlock.txn.ExtractMatches(vMatch, vIndex) != merkleBlock.header.hashMerkleRoot) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Something wrong with merkleblock"); } + auto locked_chain = pwallet->chain().lock(); + Optional height = locked_chain->getBlockHeight(merkleBlock.header.GetHash()); + if (height == nullopt) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found in chain"); + } + + std::vector::const_iterator it; + if ((it = std::find(vMatch.begin(), vMatch.end(), hashTx)) == vMatch.end()) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction given doesn't exist in proof"); + } + + unsigned int txnIndex = vIndex[it - vMatch.begin()]; + CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, *height, merkleBlock.header.GetHash(), txnIndex); wtx.m_confirm = confirm; - auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); if (pwallet->IsMine(*wtx.tx)) { From f77b1de16feee097a88e99d2ecdd4d84beb4f915 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Mon, 24 Jun 2019 19:07:09 -0400 Subject: [PATCH 6/9] Only return early from BlockUntilSyncedToCurrentChain if current tip is exact match In the next commit, we start using BlockConnected/BlockDisconnected callbacks to establish tx depth, rather than querying the chain directly. Currently, BlockUntilSyncedToCurrentChain will return early if the best block processed by the wallet is a descendant of the node'tip. That means that in the case of a re-org, it won't wait for the BlockDisconnected callbacks that have been enqueued during the re-org but have not yet been triggered in the wallet. Change BlockUntilSyncedToCurrentChain to only return early if the wallet's m_last_block_processed matches the tip exactly. This ensures that there are no BlockDisconnected or BlockConnected callbacks in-flight. --- src/interfaces/chain.cpp | 4 +--- src/interfaces/chain.h | 5 ++--- src/wallet/wallet.cpp | 2 +- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index c2a8faf8c3f..0635909cae9 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -353,13 +353,11 @@ public: { return MakeUnique(*this, notifications); } - void waitForNotificationsIfNewBlocksConnected(const uint256& old_tip) override + void waitForNotificationsIfTipChanged(const uint256& old_tip) override { if (!old_tip.IsNull()) { LOCK(::cs_main); if (old_tip == ::ChainActive().Tip()->GetBlockHash()) return; - CBlockIndex* block = LookupBlockIndex(old_tip); - if (block && block->GetAncestor(::ChainActive().Height()) == ::ChainActive().Tip()) return; } SyncWithValidationInterfaceQueue(); } diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 4cb2aba2c99..e07ec1b371a 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -236,9 +236,8 @@ public: virtual std::unique_ptr handleNotifications(Notifications& notifications) = 0; //! Wait for pending notifications to be processed unless block hash points to the current - //! chain tip, or to a possible descendant of the current chain tip that isn't currently - //! connected. - virtual void waitForNotificationsIfNewBlocksConnected(const uint256& old_tip) = 0; + //! chain tip. + virtual void waitForNotificationsIfTipChanged(const uint256& old_tip) = 0; //! Register handler for RPC. Command is not copied, so reference //! needs to remain valid until Handler is disconnected. diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 512273aa822..9fdb07ce84c 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1113,7 +1113,7 @@ void CWallet::BlockUntilSyncedToCurrentChain() { // for the queue to drain enough to execute it (indicating we are caught up // at least with the time we entered this function). uint256 last_block_hash = WITH_LOCK(cs_wallet, return m_last_block_processed); - chain().waitForNotificationsIfNewBlocksConnected(last_block_hash); + chain().waitForNotificationsIfTipChanged(last_block_hash); } From 0ff03871add000f8b4d8f82aeb168eed2fc9dc5f Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Mon, 22 Apr 2019 14:22:25 -0400 Subject: [PATCH 7/9] Use CWallet::m_last_block_processed_height in GetDepthInMainChain Avoid to lock chain to query state thanks to tracking last block height in CWallet. --- src/wallet/wallet.cpp | 4 +++- src/wallet/wallet.h | 8 +++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9fdb07ce84c..949977104b2 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3936,9 +3936,11 @@ CKeyPool::CKeyPool(const CPubKey& vchPubKeyIn, bool internalIn) int CWalletTx::GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const { + assert(pwallet != nullptr); + AssertLockHeld(pwallet->cs_wallet); if (isUnconfirmed() || isAbandoned()) return 0; - return locked_chain.getBlockDepth(m_confirm.hashBlock) * (isConflicted() ? -1 : 1); + return (pwallet->GetLastBlockHeight() - m_confirm.block_height + 1) * (isConflicted() ? -1 : 1); } int CWalletTx::GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index f3691a62187..e1dab8d37ea 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -499,7 +499,13 @@ public: * 0 : in memory pool, waiting to be included in a block * >=1 : this many blocks deep in the main chain */ - int GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const; + // TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct + // annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation + // "NO_THREAD_SAFETY_ANALYSIS" was temporarily added to avoid having to + // resolve the issue of member access into incomplete type CWallet. Note + // that we still have the runtime check "AssertLockHeld(pwallet->cs_wallet)" + // in place. + int GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const NO_THREAD_SAFETY_ANALYSIS; bool IsInMainChain(interfaces::Chain::Lock& locked_chain) const { return GetDepthInMainChain(locked_chain) > 0; } /** From b66c429c56c85fa16c309be0b2bca9c25fdd3e1a Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Mon, 29 Apr 2019 09:52:01 -0400 Subject: [PATCH 8/9] Remove locked_chain from GetDepthInMainChain and its callers We don't remove yet Chain locks as we need to preserve lock order with CWallet one until swapping at once to avoid deadlock failures (spotted by --enable-debug) --- src/interfaces/wallet.cpp | 29 ++++++----- src/wallet/feebumper.cpp | 12 ++--- src/wallet/rpcdump.cpp | 6 +-- src/wallet/rpcwallet.cpp | 23 ++++----- src/wallet/test/wallet_tests.cpp | 4 +- src/wallet/wallet.cpp | 86 ++++++++++++++++---------------- src/wallet/wallet.h | 24 ++++----- 7 files changed, 91 insertions(+), 93 deletions(-) diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index b6ede08b14f..899f04202ec 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -31,7 +31,7 @@ namespace interfaces { namespace { //! Construct wallet tx struct. -WalletTx MakeWalletTx(interfaces::Chain::Lock& locked_chain, CWallet& wallet, const CWalletTx& wtx) +WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) { WalletTx result; result.tx = wtx.tx; @@ -49,7 +49,7 @@ WalletTx MakeWalletTx(interfaces::Chain::Lock& locked_chain, CWallet& wallet, co wallet.IsMine(result.txout_address.back()) : ISMINE_NO); } - result.credit = wtx.GetCredit(locked_chain, ISMINE_ALL); + result.credit = wtx.GetCredit(ISMINE_ALL); result.debit = wtx.GetDebit(ISMINE_ALL); result.change = wtx.GetChange(); result.time = wtx.GetTxTime(); @@ -63,21 +63,20 @@ WalletTxStatus MakeWalletTxStatus(interfaces::Chain::Lock& locked_chain, const C { WalletTxStatus result; result.block_height = locked_chain.getBlockHeight(wtx.m_confirm.hashBlock).get_value_or(std::numeric_limits::max()); - result.blocks_to_maturity = wtx.GetBlocksToMaturity(locked_chain); - result.depth_in_main_chain = wtx.GetDepthInMainChain(locked_chain); + result.blocks_to_maturity = wtx.GetBlocksToMaturity(); + result.depth_in_main_chain = wtx.GetDepthInMainChain(); result.time_received = wtx.nTimeReceived; result.lock_time = wtx.tx->nLockTime; result.is_final = locked_chain.checkFinalTx(*wtx.tx); result.is_trusted = wtx.IsTrusted(locked_chain); result.is_abandoned = wtx.isAbandoned(); result.is_coinbase = wtx.IsCoinBase(); - result.is_in_main_chain = wtx.IsInMainChain(locked_chain); + result.is_in_main_chain = wtx.IsInMainChain(); return result; } //! Construct wallet TxOut struct. -WalletTxOut MakeWalletTxOut(interfaces::Chain::Lock& locked_chain, - CWallet& wallet, +WalletTxOut MakeWalletTxOut(CWallet& wallet, const CWalletTx& wtx, int n, int depth) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) @@ -86,7 +85,7 @@ WalletTxOut MakeWalletTxOut(interfaces::Chain::Lock& locked_chain, result.txout = wtx.tx->vout[n]; result.time = wtx.GetTxTime(); result.depth_in_main_chain = depth; - result.is_spent = wallet.IsSpent(locked_chain, wtx.GetHash(), n); + result.is_spent = wallet.IsSpent(wtx.GetHash(), n); return result; } @@ -235,7 +234,7 @@ public: { auto locked_chain = m_wallet->chain().lock(); LOCK(m_wallet->cs_wallet); - return m_wallet->AbandonTransaction(*locked_chain, txid); + return m_wallet->AbandonTransaction(txid); } bool transactionCanBeBumped(const uint256& txid) override { @@ -282,7 +281,7 @@ public: LOCK(m_wallet->cs_wallet); auto mi = m_wallet->mapWallet.find(txid); if (mi != m_wallet->mapWallet.end()) { - return MakeWalletTx(*locked_chain, *m_wallet, mi->second); + return MakeWalletTx(*m_wallet, mi->second); } return {}; } @@ -293,7 +292,7 @@ public: std::vector result; result.reserve(m_wallet->mapWallet.size()); for (const auto& entry : m_wallet->mapWallet) { - result.emplace_back(MakeWalletTx(*locked_chain, *m_wallet, entry.second)); + result.emplace_back(MakeWalletTx(*m_wallet, entry.second)); } return result; } @@ -338,7 +337,7 @@ public: in_mempool = mi->second.InMempool(); order_form = mi->second.vOrderForm; tx_status = MakeWalletTxStatus(*locked_chain, mi->second); - return MakeWalletTx(*locked_chain, *m_wallet, mi->second); + return MakeWalletTx(*m_wallet, mi->second); } return {}; } @@ -407,7 +406,7 @@ public: auto& group = result[entry.first]; for (const auto& coin : entry.second) { group.emplace_back(COutPoint(coin.tx->GetHash(), coin.i), - MakeWalletTxOut(*locked_chain, *m_wallet, *coin.tx, coin.i, coin.nDepth)); + MakeWalletTxOut(*m_wallet, *coin.tx, coin.i, coin.nDepth)); } } return result; @@ -422,9 +421,9 @@ public: result.emplace_back(); auto it = m_wallet->mapWallet.find(output.hash); if (it != m_wallet->mapWallet.end()) { - int depth = it->second.GetDepthInMainChain(*locked_chain); + int depth = it->second.GetDepthInMainChain(); if (depth >= 0) { - result.back() = MakeWalletTxOut(*locked_chain, *m_wallet, it->second, output.n, depth); + result.back() = MakeWalletTxOut(*m_wallet, it->second, output.n, depth); } } } diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 0a4bb3f396f..8f0b495ac4f 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -16,7 +16,7 @@ //! Check whether transaction has descendant in wallet or mempool, or has been //! mined, or conflicts with a mined transaction. Return a feebumper::Result. -static feebumper::Result PreconditionChecks(interfaces::Chain::Lock& locked_chain, const CWallet& wallet, const CWalletTx& wtx, std::vector& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) +static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWalletTx& wtx, std::vector& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { if (wallet.HasWalletSpend(wtx.GetHash())) { errors.push_back("Transaction has descendants in the wallet"); @@ -30,7 +30,7 @@ static feebumper::Result PreconditionChecks(interfaces::Chain::Lock& locked_chai } } - if (wtx.GetDepthInMainChain(locked_chain) != 0) { + if (wtx.GetDepthInMainChain() != 0) { errors.push_back("Transaction has been mined, or is conflicted with a mined transaction"); return feebumper::Result::WALLET_ERROR; } @@ -146,7 +146,7 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid) if (wtx == nullptr) return false; std::vector errors_dummy; - feebumper::Result res = PreconditionChecks(*locked_chain, wallet, *wtx, errors_dummy); + feebumper::Result res = PreconditionChecks(wallet, *wtx, errors_dummy); return res == feebumper::Result::OK; } @@ -165,7 +165,7 @@ Result CreateTotalBumpTransaction(const CWallet* wallet, const uint256& txid, co } const CWalletTx& wtx = it->second; - Result result = PreconditionChecks(*locked_chain, *wallet, wtx, errors); + Result result = PreconditionChecks(*wallet, wtx, errors); if (result != Result::OK) { return result; } @@ -291,7 +291,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo } const CWalletTx& wtx = it->second; - Result result = PreconditionChecks(*locked_chain, wallet, wtx, errors); + Result result = PreconditionChecks(wallet, wtx, errors); if (result != Result::OK) { return result; } @@ -382,7 +382,7 @@ Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransacti CWalletTx& oldWtx = it->second; // make sure the transaction still has no descendants and hasn't been mined in the meantime - Result result = PreconditionChecks(*locked_chain, wallet, oldWtx, errors); + Result result = PreconditionChecks(wallet, oldWtx, errors); if (result != Result::OK) { return result; } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index d70f68f945c..0d14d86d026 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -325,7 +325,7 @@ UniValue importaddress(const JSONRPCRequest& request) { auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); - pwallet->ReacceptWalletTransactions(*locked_chain); + pwallet->ReacceptWalletTransactions(); } } @@ -514,7 +514,7 @@ UniValue importpubkey(const JSONRPCRequest& request) { auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); - pwallet->ReacceptWalletTransactions(*locked_chain); + pwallet->ReacceptWalletTransactions(); } } @@ -1413,7 +1413,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) { auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); - pwallet->ReacceptWalletTransactions(*locked_chain); + pwallet->ReacceptWalletTransactions(); } if (pwallet->IsAbortingRescan()) { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 7f998ab4501..f3bd223b981 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -126,7 +126,7 @@ void EnsureWalletIsUnlocked(const CWallet* pwallet) static void WalletTxToJSON(interfaces::Chain& chain, interfaces::Chain::Lock& locked_chain, const CWalletTx& wtx, UniValue& entry) { - int confirms = wtx.GetDepthInMainChain(locked_chain); + int confirms = wtx.GetDepthInMainChain(); entry.pushKV("confirmations", confirms); if (wtx.IsCoinBase()) entry.pushKV("generated", true); @@ -631,7 +631,7 @@ static UniValue getreceivedbyaddress(const JSONRPCRequest& request) for (const CTxOut& txout : wtx.tx->vout) if (txout.scriptPubKey == scriptPubKey) - if (wtx.GetDepthInMainChain(*locked_chain) >= nMinDepth) + if (wtx.GetDepthInMainChain() >= nMinDepth) nAmount += txout.nValue; } @@ -697,7 +697,7 @@ static UniValue getreceivedbylabel(const JSONRPCRequest& request) { CTxDestination address; if (ExtractDestination(txout.scriptPubKey, address) && pwallet->IsMine(address) && setAddress.count(address)) { - if (wtx.GetDepthInMainChain(*locked_chain) >= nMinDepth) + if (wtx.GetDepthInMainChain() >= nMinDepth) nAmount += txout.nValue; } } @@ -1057,7 +1057,7 @@ static UniValue ListReceived(interfaces::Chain::Lock& locked_chain, CWallet * co continue; } - int nDepth = wtx.GetDepthInMainChain(locked_chain); + int nDepth = wtx.GetDepthInMainChain(); if (nDepth < nMinDepth) continue; @@ -1314,8 +1314,7 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, CWallet* con } // Received - if (listReceived.size() > 0 && wtx.GetDepthInMainChain(locked_chain) >= nMinDepth) - { + if (listReceived.size() > 0 && wtx.GetDepthInMainChain() >= nMinDepth) { for (const COutputEntry& r : listReceived) { std::string label; @@ -1332,9 +1331,9 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, CWallet* con MaybePushAddress(entry, r.destination); if (wtx.IsCoinBase()) { - if (wtx.GetDepthInMainChain(locked_chain) < 1) + if (wtx.GetDepthInMainChain() < 1) entry.pushKV("category", "orphan"); - else if (wtx.IsImmatureCoinBase(locked_chain)) + else if (wtx.IsImmatureCoinBase()) entry.pushKV("category", "immature"); else entry.pushKV("category", "generate"); @@ -1598,7 +1597,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request) for (const std::pair& pairWtx : pwallet->mapWallet) { CWalletTx tx = pairWtx.second; - if (depth == -1 || abs(tx.GetDepthInMainChain(*locked_chain)) < depth) { + if (depth == -1 || abs(tx.GetDepthInMainChain()) < depth) { ListTransactions(*locked_chain, pwallet, tx, 0, true, transactions, filter, nullptr /* filter_label */); } } @@ -1715,7 +1714,7 @@ static UniValue gettransaction(const JSONRPCRequest& request) } const CWalletTx& wtx = it->second; - CAmount nCredit = wtx.GetCredit(*locked_chain, filter); + CAmount nCredit = wtx.GetCredit(filter); CAmount nDebit = wtx.GetDebit(filter); CAmount nNet = nCredit - nDebit; CAmount nFee = (wtx.IsFromMe(filter) ? wtx.tx->GetValueOut() - nDebit : 0); @@ -1779,7 +1778,7 @@ static UniValue abandontransaction(const JSONRPCRequest& request) if (!pwallet->mapWallet.count(hash)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid or non-wallet transaction id"); } - if (!pwallet->AbandonTransaction(*locked_chain, hash)) { + if (!pwallet->AbandonTransaction(hash)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not eligible for abandonment"); } @@ -2210,7 +2209,7 @@ static UniValue lockunspent(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, vout index out of bounds"); } - if (pwallet->IsSpent(*locked_chain, outpt.hash, outpt.n)) { + if (pwallet->IsSpent(outpt.hash, outpt.n)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expected unspent output"); } diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 3f0e40149cf..93d2a716a3c 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -281,13 +281,13 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) // Call GetImmatureCredit() once before adding the key to the wallet to // cache the current immature credit amount, which is 0. - BOOST_CHECK_EQUAL(wtx.GetImmatureCredit(*locked_chain), 0); + BOOST_CHECK_EQUAL(wtx.GetImmatureCredit(), 0); // Invalidate the cached vanue, add the key, and make sure a new immature // credit amount is calculated. wtx.MarkDirty(); BOOST_CHECK(spk_man->AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey())); - BOOST_CHECK_EQUAL(wtx.GetImmatureCredit(*locked_chain), 50*COIN); + BOOST_CHECK_EQUAL(wtx.GetImmatureCredit(), 50*COIN); } static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64_t blockTime) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 949977104b2..4936e3851b3 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -452,7 +452,7 @@ void CWallet::SyncMetaData(std::pair ran * Outpoint is spent if any non-conflicted transaction * spends it: */ -bool CWallet::IsSpent(interfaces::Chain::Lock& locked_chain, const uint256& hash, unsigned int n) const +bool CWallet::IsSpent(const uint256& hash, unsigned int n) const { const COutPoint outpoint(hash, n); std::pair range; @@ -463,7 +463,7 @@ bool CWallet::IsSpent(interfaces::Chain::Lock& locked_chain, const uint256& hash const uint256& wtxid = it->second; std::map::const_iterator mit = mapWallet.find(wtxid); if (mit != mapWallet.end()) { - int depth = mit->second.GetDepthInMainChain(locked_chain); + int depth = mit->second.GetDepthInMainChain(); if (depth > 0 || (depth == 0 && !mit->second.isAbandoned())) return true; // Spent } @@ -913,7 +913,7 @@ bool CWallet::TransactionCanBeAbandoned(const uint256& hashTx) const auto locked_chain = chain().lock(); LOCK(cs_wallet); const CWalletTx* wtx = GetWalletTx(hashTx); - return wtx && !wtx->isAbandoned() && wtx->GetDepthInMainChain(*locked_chain) == 0 && !wtx->InMempool(); + return wtx && !wtx->isAbandoned() && wtx->GetDepthInMainChain() == 0 && !wtx->InMempool(); } void CWallet::MarkInputsDirty(const CTransactionRef& tx) @@ -926,9 +926,9 @@ void CWallet::MarkInputsDirty(const CTransactionRef& tx) } } -bool CWallet::AbandonTransaction(interfaces::Chain::Lock& locked_chain, const uint256& hashTx) +bool CWallet::AbandonTransaction(const uint256& hashTx) { - auto locked_chain_recursive = chain().lock(); // Temporary. Removed in upcoming lock cleanup + auto locked_chain = chain().lock(); // Temporary. Removed in upcoming lock cleanup LOCK(cs_wallet); WalletBatch batch(*database, "r+"); @@ -940,7 +940,7 @@ bool CWallet::AbandonTransaction(interfaces::Chain::Lock& locked_chain, const ui auto it = mapWallet.find(hashTx); assert(it != mapWallet.end()); CWalletTx& origtx = it->second; - if (origtx.GetDepthInMainChain(locked_chain) != 0 || origtx.InMempool()) { + if (origtx.GetDepthInMainChain() != 0 || origtx.InMempool()) { return false; } @@ -953,7 +953,7 @@ bool CWallet::AbandonTransaction(interfaces::Chain::Lock& locked_chain, const ui auto it = mapWallet.find(now); assert(it != mapWallet.end()); CWalletTx& wtx = it->second; - int currentconfirm = wtx.GetDepthInMainChain(locked_chain); + int currentconfirm = wtx.GetDepthInMainChain(); // If the orig tx was not in block, none of its spends can be assert(currentconfirm <= 0); // if (currentconfirm < 0) {Tx and spends are already conflicted, no need to abandon} @@ -1009,7 +1009,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c auto it = mapWallet.find(now); assert(it != mapWallet.end()); CWalletTx& wtx = it->second; - int currentconfirm = wtx.GetDepthInMainChain(*locked_chain); + int currentconfirm = wtx.GetDepthInMainChain(); if (conflictconfirms < currentconfirm) { // Block is 'more conflicted' than current confirm; update. // Mark transaction as conflicted with this block. @@ -1691,7 +1691,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc return result; } -void CWallet::ReacceptWalletTransactions(interfaces::Chain::Lock& locked_chain) +void CWallet::ReacceptWalletTransactions() { // If transactions aren't being broadcasted, don't let them into local mempool either if (!fBroadcastTransactions) @@ -1704,7 +1704,7 @@ void CWallet::ReacceptWalletTransactions(interfaces::Chain::Lock& locked_chain) CWalletTx& wtx = item.second; assert(wtx.GetHash() == wtxid); - int nDepth = wtx.GetDepthInMainChain(locked_chain); + int nDepth = wtx.GetDepthInMainChain(); if (!wtx.IsCoinBase() && (nDepth == 0 && !wtx.isAbandoned())) { mapSorted.insert(std::make_pair(wtx.nOrderPos, &wtx)); @@ -1715,11 +1715,11 @@ void CWallet::ReacceptWalletTransactions(interfaces::Chain::Lock& locked_chain) for (const std::pair& item : mapSorted) { CWalletTx& wtx = *(item.second); std::string unused_err_string; - wtx.SubmitMemoryPoolAndRelay(unused_err_string, false, locked_chain); + wtx.SubmitMemoryPoolAndRelay(unused_err_string, false); } } -bool CWalletTx::SubmitMemoryPoolAndRelay(std::string& err_string, bool relay, interfaces::Chain::Lock& locked_chain) +bool CWalletTx::SubmitMemoryPoolAndRelay(std::string& err_string, bool relay) { // Can't relay if wallet is not broadcasting if (!pwallet->GetBroadcastTransactions()) return false; @@ -1729,7 +1729,7 @@ bool CWalletTx::SubmitMemoryPoolAndRelay(std::string& err_string, bool relay, in // cause log spam. if (IsCoinBase()) return false; // Don't try to submit conflicted or confirmed transactions. - if (GetDepthInMainChain(locked_chain) != 0) return false; + if (GetDepthInMainChain() != 0) return false; // Submit transaction to mempool for relay pwallet->WalletLogPrintf("Submitting wtx %s to mempool for relay\n", GetHash().ToString()); @@ -1783,10 +1783,10 @@ CAmount CWalletTx::GetDebit(const isminefilter& filter) const return debit; } -CAmount CWalletTx::GetCredit(interfaces::Chain::Lock& locked_chain, const isminefilter& filter) const +CAmount CWalletTx::GetCredit(const isminefilter& filter) const { // Must wait until coinbase is safely deep enough in the chain before valuing it - if (IsImmatureCoinBase(locked_chain)) + if (IsImmatureCoinBase()) return 0; CAmount credit = 0; @@ -1800,16 +1800,16 @@ CAmount CWalletTx::GetCredit(interfaces::Chain::Lock& locked_chain, const ismine return credit; } -CAmount CWalletTx::GetImmatureCredit(interfaces::Chain::Lock& locked_chain, bool fUseCache) const +CAmount CWalletTx::GetImmatureCredit(bool fUseCache) const { - if (IsImmatureCoinBase(locked_chain) && IsInMainChain(locked_chain)) { + if (IsImmatureCoinBase() && IsInMainChain()) { return GetCachableAmount(IMMATURE_CREDIT, ISMINE_SPENDABLE, !fUseCache); } return 0; } -CAmount CWalletTx::GetAvailableCredit(interfaces::Chain::Lock& locked_chain, bool fUseCache, const isminefilter& filter) const +CAmount CWalletTx::GetAvailableCredit(bool fUseCache, const isminefilter& filter) const { if (pwallet == nullptr) return 0; @@ -1818,7 +1818,7 @@ CAmount CWalletTx::GetAvailableCredit(interfaces::Chain::Lock& locked_chain, boo bool allow_cache = (filter & ISMINE_ALL) && (filter & ISMINE_ALL) != ISMINE_ALL; // Must wait until coinbase is safely deep enough in the chain before valuing it - if (IsImmatureCoinBase(locked_chain)) + if (IsImmatureCoinBase()) return 0; if (fUseCache && allow_cache && m_amounts[AVAILABLE_CREDIT].m_cached[filter]) { @@ -1830,7 +1830,7 @@ CAmount CWalletTx::GetAvailableCredit(interfaces::Chain::Lock& locked_chain, boo uint256 hashTx = GetHash(); for (unsigned int i = 0; i < tx->vout.size(); i++) { - if (!pwallet->IsSpent(locked_chain, hashTx, i) && (allow_used_addresses || !pwallet->IsUsedDestination(hashTx, i))) { + if (!pwallet->IsSpent(hashTx, i) && (allow_used_addresses || !pwallet->IsUsedDestination(hashTx, i))) { const CTxOut &txout = tx->vout[i]; nCredit += pwallet->GetCredit(txout, filter); if (!MoneyRange(nCredit)) @@ -1845,9 +1845,9 @@ CAmount CWalletTx::GetAvailableCredit(interfaces::Chain::Lock& locked_chain, boo return nCredit; } -CAmount CWalletTx::GetImmatureWatchOnlyCredit(interfaces::Chain::Lock& locked_chain, const bool fUseCache) const +CAmount CWalletTx::GetImmatureWatchOnlyCredit(const bool fUseCache) const { - if (IsImmatureCoinBase(locked_chain) && IsInMainChain(locked_chain)) { + if (IsImmatureCoinBase() && IsInMainChain()) { return GetCachableAmount(IMMATURE_CREDIT, ISMINE_WATCH_ONLY, !fUseCache); } @@ -1878,7 +1878,7 @@ bool CWalletTx::IsTrusted(interfaces::Chain::Lock& locked_chain, std::set= 1) return true; if (nDepth < 0) return false; // using wtx's cached debit @@ -1954,7 +1954,7 @@ void CWallet::ResendWalletTransactions() // any confirmed or conflicting txs. if (wtx.nTimeReceived > m_best_block_time - 5 * 60) continue; std::string unused_err_string; - if (wtx.SubmitMemoryPoolAndRelay(unused_err_string, true, *locked_chain)) ++submitted_tx_count; + if (wtx.SubmitMemoryPoolAndRelay(unused_err_string, true)) ++submitted_tx_count; } } // locked_chain and cs_wallet @@ -1991,9 +1991,9 @@ CWallet::Balance CWallet::GetBalance(const int min_depth, bool avoid_reuse) cons { const CWalletTx& wtx = entry.second; const bool is_trusted{wtx.IsTrusted(*locked_chain, trusted_parents)}; - const int tx_depth{wtx.GetDepthInMainChain(*locked_chain)}; - const CAmount tx_credit_mine{wtx.GetAvailableCredit(*locked_chain, /* fUseCache */ true, ISMINE_SPENDABLE | reuse_filter)}; - const CAmount tx_credit_watchonly{wtx.GetAvailableCredit(*locked_chain, /* fUseCache */ true, ISMINE_WATCH_ONLY | reuse_filter)}; + const int tx_depth{wtx.GetDepthInMainChain()}; + const CAmount tx_credit_mine{wtx.GetAvailableCredit(/* fUseCache */ true, ISMINE_SPENDABLE | reuse_filter)}; + const CAmount tx_credit_watchonly{wtx.GetAvailableCredit(/* fUseCache */ true, ISMINE_WATCH_ONLY | reuse_filter)}; if (is_trusted && tx_depth >= min_depth) { ret.m_mine_trusted += tx_credit_mine; ret.m_watchonly_trusted += tx_credit_watchonly; @@ -2002,8 +2002,8 @@ CWallet::Balance CWallet::GetBalance(const int min_depth, bool avoid_reuse) cons ret.m_mine_untrusted_pending += tx_credit_mine; ret.m_watchonly_untrusted_pending += tx_credit_watchonly; } - ret.m_mine_immature += wtx.GetImmatureCredit(*locked_chain); - ret.m_watchonly_immature += wtx.GetImmatureWatchOnlyCredit(*locked_chain); + ret.m_mine_immature += wtx.GetImmatureCredit(); + ret.m_watchonly_immature += wtx.GetImmatureWatchOnlyCredit(); } } return ret; @@ -2047,10 +2047,10 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector< continue; } - if (wtx.IsImmatureCoinBase(locked_chain)) + if (wtx.IsImmatureCoinBase()) continue; - int nDepth = wtx.GetDepthInMainChain(locked_chain); + int nDepth = wtx.GetDepthInMainChain(); if (nDepth < 0) continue; @@ -2110,7 +2110,7 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector< if (IsLockedCoin(entry.first, i)) continue; - if (IsSpent(locked_chain, wtxid, i)) + if (IsSpent(wtxid, i)) continue; isminetype mine = IsMine(wtx.tx->vout[i]); @@ -2169,7 +2169,7 @@ std::map> CWallet::ListCoins(interfaces::Ch for (const COutPoint& output : lockedCoins) { auto it = mapWallet.find(output.hash); if (it != mapWallet.end()) { - int depth = it->second.GetDepthInMainChain(locked_chain); + int depth = it->second.GetDepthInMainChain(); if (depth >= 0 && output.n < it->second.tx->vout.size() && IsMine(it->second.tx->vout[output.n]) == ISMINE_SPENDABLE) { CTxDestination address; @@ -2909,7 +2909,7 @@ void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve } std::string err_string; - if (!wtx.SubmitMemoryPoolAndRelay(err_string, true, *locked_chain)) { + if (!wtx.SubmitMemoryPoolAndRelay(err_string, true)) { WalletLogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", err_string); // TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure. } @@ -3129,10 +3129,10 @@ std::map CWallet::GetAddressBalances(interfaces::Chain: if (!wtx.IsTrusted(locked_chain, trusted_parents)) continue; - if (wtx.IsImmatureCoinBase(locked_chain)) + if (wtx.IsImmatureCoinBase()) continue; - int nDepth = wtx.GetDepthInMainChain(locked_chain); + int nDepth = wtx.GetDepthInMainChain(); if (nDepth < (wtx.IsFromMe(ISMINE_ALL) ? 0 : 1)) continue; @@ -3144,7 +3144,7 @@ std::map CWallet::GetAddressBalances(interfaces::Chain: if(!ExtractDestination(wtx.tx->vout[i].scriptPubKey, addr)) continue; - CAmount n = IsSpent(locked_chain, walletEntry.first, i) ? 0 : wtx.tx->vout[i].nValue; + CAmount n = IsSpent(walletEntry.first, i) ? 0 : wtx.tx->vout[i].nValue; if (!balances.count(addr)) balances[addr] = 0; @@ -3908,7 +3908,7 @@ void CWallet::postInitProcess() // Add wallet transactions that aren't already in a block to mempool // Do this here as mempool requires genesis block to be loaded - ReacceptWalletTransactions(*locked_chain); + ReacceptWalletTransactions(); // Update wallet transactions with current mempool transactions. chain().requestMempoolTransactions(*this); @@ -3934,7 +3934,7 @@ CKeyPool::CKeyPool(const CPubKey& vchPubKeyIn, bool internalIn) m_pre_split = false; } -int CWalletTx::GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const +int CWalletTx::GetDepthInMainChain() const { assert(pwallet != nullptr); AssertLockHeld(pwallet->cs_wallet); @@ -3943,19 +3943,19 @@ int CWalletTx::GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const return (pwallet->GetLastBlockHeight() - m_confirm.block_height + 1) * (isConflicted() ? -1 : 1); } -int CWalletTx::GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const +int CWalletTx::GetBlocksToMaturity() const { if (!IsCoinBase()) return 0; - int chain_depth = GetDepthInMainChain(locked_chain); + int chain_depth = GetDepthInMainChain(); assert(chain_depth >= 0); // coinbase tx should not be conflicted return std::max(0, (COINBASE_MATURITY+1) - chain_depth); } -bool CWalletTx::IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const +bool CWalletTx::IsImmatureCoinBase() const { // note GetBlocksToMaturity is 0 for non-coinbase tx - return GetBlocksToMaturity(locked_chain) > 0; + return GetBlocksToMaturity() > 0; } std::vector CWallet::GroupOutputs(const std::vector& outputs, bool single_coin) const { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index e1dab8d37ea..081d644e1fc 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -449,14 +449,14 @@ public: //! filter decides which addresses will count towards the debit CAmount GetDebit(const isminefilter& filter) const; - CAmount GetCredit(interfaces::Chain::Lock& locked_chain, const isminefilter& filter) const; - CAmount GetImmatureCredit(interfaces::Chain::Lock& locked_chain, bool fUseCache=true) const; + CAmount GetCredit(const isminefilter& filter) const; + CAmount GetImmatureCredit(bool fUseCache = true) const; // TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct // annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The // annotation "NO_THREAD_SAFETY_ANALYSIS" was temporarily added to avoid // having to resolve the issue of member access into incomplete type CWallet. - CAmount GetAvailableCredit(interfaces::Chain::Lock& locked_chain, bool fUseCache=true, const isminefilter& filter=ISMINE_SPENDABLE) const NO_THREAD_SAFETY_ANALYSIS; - CAmount GetImmatureWatchOnlyCredit(interfaces::Chain::Lock& locked_chain, const bool fUseCache=true) const; + CAmount GetAvailableCredit(bool fUseCache = true, const isminefilter& filter = ISMINE_SPENDABLE) const NO_THREAD_SAFETY_ANALYSIS; + CAmount GetImmatureWatchOnlyCredit(const bool fUseCache = true) const; CAmount GetChange() const; // Get the marginal bytes if spending the specified output from this transaction @@ -483,7 +483,7 @@ public: int64_t GetTxTime() const; // Pass this transaction to node for mempool insertion and relay to peers if flag set to true - bool SubmitMemoryPoolAndRelay(std::string& err_string, bool relay, interfaces::Chain::Lock& locked_chain); + bool SubmitMemoryPoolAndRelay(std::string& err_string, bool relay); // TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct // annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation @@ -505,15 +505,15 @@ public: // resolve the issue of member access into incomplete type CWallet. Note // that we still have the runtime check "AssertLockHeld(pwallet->cs_wallet)" // in place. - int GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const NO_THREAD_SAFETY_ANALYSIS; - bool IsInMainChain(interfaces::Chain::Lock& locked_chain) const { return GetDepthInMainChain(locked_chain) > 0; } + int GetDepthInMainChain() const NO_THREAD_SAFETY_ANALYSIS; + bool IsInMainChain() const { return GetDepthInMainChain() > 0; } /** * @return number of blocks to maturity for this transaction: * 0 : is not a coinbase transaction, or is a mature coinbase transaction * >0 : is a coinbase transaction which matures in this many blocks */ - int GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const; + int GetBlocksToMaturity() const; bool isAbandoned() const { return m_confirm.status == CWalletTx::ABANDONED; } void setAbandoned() { @@ -530,7 +530,7 @@ public: void setConfirmed() { m_confirm.status = CWalletTx::CONFIRMED; } const uint256& GetHash() const { return tx->GetHash(); } bool IsCoinBase() const { return tx->IsCoinBase(); } - bool IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const; + bool IsImmatureCoinBase() const; }; class COutput @@ -808,7 +808,7 @@ public: bool SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector groups, std::set& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const; - bool IsSpent(interfaces::Chain::Lock& locked_chain, const uint256& hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool IsSpent(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; @@ -891,7 +891,7 @@ public: }; ScanResult ScanForWalletTransactions(const uint256& first_block, const uint256& last_block, const WalletRescanReserver& reserver, bool fUpdate); void TransactionRemovedFromMempool(const CTransactionRef &ptx) override; - void ReacceptWalletTransactions(interfaces::Chain::Lock& locked_chain) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void ReacceptWalletTransactions() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void ResendWalletTransactions(); struct Balance { CAmount m_mine_trusted{0}; //!< Trusted, at depth=GetBalance.min_depth or more @@ -1070,7 +1070,7 @@ public: bool TransactionCanBeAbandoned(const uint256& hashTx) const; /* Mark a transaction (and it in-wallet descendants) as abandoned so its inputs may be respent. */ - bool AbandonTransaction(interfaces::Chain::Lock& locked_chain, const uint256& hashTx); + bool AbandonTransaction(const uint256& hashTx); /** Mark a transaction as replaced by another transaction (e.g., BIP 125). */ bool MarkReplaced(const uint256& originalHash, const uint256& newHash); From 36b68de5b2938722911db900ca299f7008780d01 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Mon, 29 Apr 2019 10:18:50 -0400 Subject: [PATCH 9/9] Remove getBlockDepth method from Chain::interface Pass conflicting height in CWallet::MarkConflicted --- src/interfaces/chain.cpp | 6 ------ src/interfaces/chain.h | 4 ---- src/wallet/wallet.cpp | 2 +- 3 files changed, 1 insertion(+), 11 deletions(-) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 0635909cae9..26856a00d37 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -58,12 +58,6 @@ class LockImpl : public Chain::Lock, public UniqueLock } return nullopt; } - int getBlockDepth(const uint256& hash) override - { - const Optional tip_height = getHeight(); - const Optional height = getBlockHeight(hash); - return tip_height && height ? *tip_height - *height + 1 : 0; - } uint256 getBlockHash(int height) override { LockAssertion lock(::cs_main); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index e07ec1b371a..349af152d5c 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -76,10 +76,6 @@ public: //! included in the current chain. virtual Optional getBlockHeight(const uint256& hash) = 0; - //! Get block depth. Returns 1 for chain tip, 2 for preceding block, and - //! so on. Returns 0 for a block not included in the current chain. - virtual int getBlockDepth(const uint256& hash) = 0; - //! Get block hash. Height must be valid or this function will abort. virtual uint256 getBlockHash(int height) = 0; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 4936e3851b3..2bc39ea3d91 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -986,7 +986,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c auto locked_chain = chain().lock(); LOCK(cs_wallet); - int conflictconfirms = -locked_chain->getBlockDepth(hashBlock); + int conflictconfirms = (m_last_block_processed_height - conflicting_height + 1) * -1; // If number of conflict confirms cannot be determined, this means // that the block is still unknown or not yet part of the main chain, // for example when loading the wallet during a reindex. Do nothing in that