Skip to content

Commit 0cfbb17

Browse files
author
MacroFake
committed
Merge bitcoin/bitcoin#26130: Bugfix: Wallet: Lock cs_wallet for SignMessage
a60d9eb Bugfix: Wallet: Lock cs_wallet for SignMessage (Luke Dashjr) Pull request description: cs_desc_main is typically locked within scope of a cs_wallet lock, but: CWallet::IsLocked locks cs_wallet ...called from DescriptorScriptPubKeyMan::GetKeys ...called from DescriptorScriptPubKeyMan::GetSigningProvider which locks cs_desc_main first, but has no access to cs_wallet ...called from DescriptorScriptPubKeyMan::SignMessage ...called from CWallet::SignMessage which can access and lock cs_wallet Resolve the out of order locks by grabbing cs_wallet in CWallet::SignMessage first ------------- Note this is currently only an issue for the GUI (which lacks sufficient testing apparently), but can be reproduced by #26082 (CI fails as a result) ACKs for top commit: achow101: ACK a60d9eb w0xlt: ACK bitcoin/bitcoin@a60d9eb Tree-SHA512: 60f6959b0ceaf4d9339ba1a47154734034b637c41b1f9e26748a2dbbc3a2a95fc3696019103c55ae70c91d910ba8f3d7f4e27d263030eb60b689f290c4d82ea9
2 parents 100949a + a60d9eb commit 0cfbb17

File tree

1 file changed

+1
-0
lines changed

1 file changed

+1
-0
lines changed

src/wallet/wallet.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2110,6 +2110,7 @@ SigningResult CWallet::SignMessage(const std::string& message, const PKHash& pkh
21102110
CScript script_pub_key = GetScriptForDestination(pkhash);
21112111
for (const auto& spk_man_pair : m_spk_managers) {
21122112
if (spk_man_pair.second->CanProvide(script_pub_key, sigdata)) {
2113+
LOCK(cs_wallet); // DescriptorScriptPubKeyMan calls IsLocked which can lock cs_wallet in a deadlocking order
21132114
return spk_man_pair.second->SignMessage(message, pkhash, str_sig);
21142115
}
21152116
}

0 commit comments

Comments
 (0)