From 40ede992d97df38282919693dfe851c975c3b1d8 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Thu, 22 Aug 2019 13:16:40 -0400 Subject: [PATCH] Modify wallet tx status if has been reorged out Add a LockChain method to CWallet to know if we can lock or query chain state safely. At tx loading, we rely on chain to know if hashBlock of tx is still in main chain. If not, we set its status to unconfirmed and reset its hashBlock/nIndex. If wallet loaded is the wallet-tool one, all wallet txn will show up with a height of zero. It doesn't matter as status is not used by wallet-tool. We take lock prematurely in CWallet::LoadWallet and CWallet::Verify to ensure that lock order is respected between cs_main an cs_wallet. --- src/wallet/wallet.cpp | 23 ++++++++++++++++++++++- src/wallet/wallet.h | 5 ++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 8b636c41a..1c07f29c9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1172,8 +1172,19 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) return true; } -void CWallet::LoadToWallet(const CWalletTx& wtxIn) +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)) { + wtxIn.setUnconfirmed(); + wtxIn.m_confirm.hashBlock = uint256(); + wtxIn.m_confirm.nIndex = 0; + } + } uint256 hash = wtxIn.GetHash(); const auto& ins = mapWallet.emplace(hash, wtxIn); CWalletTx& wtx = ins.first->second; @@ -3330,6 +3341,11 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve DBErrors CWallet::LoadWallet(bool& fFirstRunRet) { + // Even if we don't use this lock in this function, we want to preserve + // lock order in LoadToWallet if query of chain state is needed to know + // tx status. If lock can't be taken (e.g wallet-tool), tx confirmation + // status may be not reliable. + auto locked_chain = LockChain(); LOCK(cs_wallet); fFirstRunRet = false; @@ -4231,6 +4247,11 @@ bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, b // Recover readable keypairs: CWallet dummyWallet(&chain, WalletLocation(), WalletDatabase::CreateDummy()); std::string backup_filename; + // Even if we don't use this lock in this function, we want to preserve + // lock order in LoadToWallet if query of chain state is needed to know + // tx status. If lock can't be taken, tx confirmation status may be not + // reliable. + auto locked_chain = dummyWallet.LockChain(); if (!WalletBatch::Recover(wallet_path, (void *)&dummyWallet, WalletBatch::RecoverKeysOnlyFilter, backup_filename)) { return false; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index d50ba2fb9..3428e8e00 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -946,6 +946,9 @@ public: bool IsLocked() const; bool Lock(); + /** Interface to assert chain access and if successful lock it */ + std::unique_ptr LockChain() { return m_chain ? m_chain->lock() : nullptr; } + std::map mapWallet GUARDED_BY(cs_wallet); typedef std::multimap TxItems; @@ -1091,7 +1094,7 @@ public: void MarkDirty(); bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose=true); - void LoadToWallet(const CWalletTx& wtxIn) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void LoadToWallet(CWalletTx& wtxIn) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void TransactionAddedToMempool(const CTransactionRef& tx) override; void BlockConnected(const CBlock& block, const std::vector& vtxConflicted) override; void BlockDisconnected(const CBlock& block) override;