Skip to content

Commit 40ede99

Browse files
author
Antoine Riard
committed
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.
1 parent 7e89994 commit 40ede99

File tree

2 files changed

+26
-2
lines changed

2 files changed

+26
-2
lines changed

src/wallet/wallet.cpp

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1172,8 +1172,19 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose)
11721172
return true;
11731173
}
11741174

1175-
void CWallet::LoadToWallet(const CWalletTx& wtxIn)
1175+
void CWallet::LoadToWallet(CWalletTx& wtxIn)
11761176
{
1177+
// If wallet doesn't have a chain (e.g wallet-tool), lock can't be taken.
1178+
auto locked_chain = LockChain();
1179+
// If tx hasn't been reorged out of chain while wallet being shutdown
1180+
// change tx status to UNCONFIRMED and reset hashBlock/nIndex.
1181+
if (!wtxIn.m_confirm.hashBlock.IsNull()) {
1182+
if (locked_chain && !locked_chain->getBlockHeight(wtxIn.m_confirm.hashBlock)) {
1183+
wtxIn.setUnconfirmed();
1184+
wtxIn.m_confirm.hashBlock = uint256();
1185+
wtxIn.m_confirm.nIndex = 0;
1186+
}
1187+
}
11771188
uint256 hash = wtxIn.GetHash();
11781189
const auto& ins = mapWallet.emplace(hash, wtxIn);
11791190
CWalletTx& wtx = ins.first->second;
@@ -3330,6 +3341,11 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve
33303341

33313342
DBErrors CWallet::LoadWallet(bool& fFirstRunRet)
33323343
{
3344+
// Even if we don't use this lock in this function, we want to preserve
3345+
// lock order in LoadToWallet if query of chain state is needed to know
3346+
// tx status. If lock can't be taken (e.g wallet-tool), tx confirmation
3347+
// status may be not reliable.
3348+
auto locked_chain = LockChain();
33333349
LOCK(cs_wallet);
33343350

33353351
fFirstRunRet = false;
@@ -4231,6 +4247,11 @@ bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, b
42314247
// Recover readable keypairs:
42324248
CWallet dummyWallet(&chain, WalletLocation(), WalletDatabase::CreateDummy());
42334249
std::string backup_filename;
4250+
// Even if we don't use this lock in this function, we want to preserve
4251+
// lock order in LoadToWallet if query of chain state is needed to know
4252+
// tx status. If lock can't be taken, tx confirmation status may be not
4253+
// reliable.
4254+
auto locked_chain = dummyWallet.LockChain();
42344255
if (!WalletBatch::Recover(wallet_path, (void *)&dummyWallet, WalletBatch::RecoverKeysOnlyFilter, backup_filename)) {
42354256
return false;
42364257
}

src/wallet/wallet.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -946,6 +946,9 @@ class CWallet final : public FillableSigningProvider, private interfaces::Chain:
946946
bool IsLocked() const;
947947
bool Lock();
948948

949+
/** Interface to assert chain access and if successful lock it */
950+
std::unique_ptr<interfaces::Chain::Lock> LockChain() { return m_chain ? m_chain->lock() : nullptr; }
951+
949952
std::map<uint256, CWalletTx> mapWallet GUARDED_BY(cs_wallet);
950953

951954
typedef std::multimap<int64_t, CWalletTx*> TxItems;
@@ -1091,7 +1094,7 @@ class CWallet final : public FillableSigningProvider, private interfaces::Chain:
10911094

10921095
void MarkDirty();
10931096
bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose=true);
1094-
void LoadToWallet(const CWalletTx& wtxIn) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
1097+
void LoadToWallet(CWalletTx& wtxIn) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
10951098
void TransactionAddedToMempool(const CTransactionRef& tx) override;
10961099
void BlockConnected(const CBlock& block, const std::vector<CTransactionRef>& vtxConflicted) override;
10971100
void BlockDisconnected(const CBlock& block) override;

0 commit comments

Comments
 (0)