Skip to content

rpc: Fix wallet unload during walletpassphrase timeout #14453

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 24, 2018
Merged

rpc: Fix wallet unload during walletpassphrase timeout #14453

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
321decf
rpc: Fix wallet unload during walletpassphrase timeout
promag Oct 9, 2018
8907df9
qa: Ensure wallet unload during walletpassphrase timeout
promag Oct 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions src/wallet/rpcwallet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1928,13 +1928,6 @@ static UniValue keypoolrefill(const JSONRPCRequest& request)
}


static void LockWallet(CWallet* pWallet)
{
LOCK(pWallet->cs_wallet);
pWallet->nRelockTime = 0;
pWallet->Lock();
}

static UniValue walletpassphrase(const JSONRPCRequest& request)
{
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
Expand Down Expand Up @@ -2004,7 +1997,18 @@ static UniValue walletpassphrase(const JSONRPCRequest& request)
pwallet->TopUpKeyPool();

pwallet->nRelockTime = GetTime() + nSleepTime;
RPCRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), std::bind(LockWallet, pwallet), nSleepTime);

// Keep a weak pointer to the wallet so that it is possible to unload the
// wallet before the following callback is called. If a valid shared pointer
// is acquired in the callback then the wallet is still loaded.
std::weak_ptr<CWallet> weak_wallet = wallet;
RPCRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), [weak_wallet] {
if (auto shared_wallet = weak_wallet.lock()) {
LOCK(shared_wallet->cs_wallet);
shared_wallet->Lock();
shared_wallet->nRelockTime = 0;
}
}, nSleepTime);

return NullUniValue;
}
Expand Down
5 changes: 5 additions & 0 deletions test/functional/wallet_multiwallet.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"""
import os
import shutil
import time

from test_framework.test_framework import BitcoinTestFramework
from test_framework.test_node import ErrorMatch
Expand Down Expand Up @@ -267,7 +268,11 @@ def wallet_file(name):
assert 'w1' not in self.nodes[0].listwallets()

# Successfully unload the wallet referenced by the request endpoint
# Also ensure unload works during walletpassphrase timeout
w2.encryptwallet('test')
w2.walletpassphrase('test', 1)
w2.unloadwallet()
time.sleep(1.1)
Copy link
Contributor

@ryanofsky ryanofsky Oct 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is a sleep needed between unloadwallet and listwallets? If it's not needed to make listwallets work, it might make the test clearer to move the sleep immediately before the code that actually does depends on it (loadwallet?) and add a comment describing what the sleep is for.

Copy link
Contributor Author

@promag promag Oct 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the sleep is not actually needed (there). However if the test terminates (and the node quits) then it's not guaranteed the timeout callback was executed? I can add a comment like "wait to ensure walletpassphrase callback is executed".

assert 'w2' not in self.nodes[0].listwallets()

# Successfully unload all wallets
Expand Down