Skip to content

Use boost::asio::deadline_timer for walletpassphrase timeout #2625

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 1 commit into from
May 30, 2013
Merged

Use boost::asio::deadline_timer for walletpassphrase timeout #2625

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 25 additions & 3 deletions src/bitcoinrpc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,17 @@
#include "bitcoinrpc.h"
#include "db.h"

#include <boost/algorithm/string.hpp>
#include <boost/asio.hpp>
#include <boost/asio/ip/v6_only.hpp>
#include <boost/asio/ssl.hpp>
#include <boost/bind.hpp>
#include <boost/filesystem.hpp>
#include <boost/filesystem/fstream.hpp>
#include <boost/foreach.hpp>
#include <boost/iostreams/concepts.hpp>
#include <boost/iostreams/stream.hpp>
#include <boost/algorithm/string.hpp>
#include <boost/lexical_cast.hpp>
#include <boost/asio/ssl.hpp>
#include <boost/filesystem/fstream.hpp>
#include <boost/shared_ptr.hpp>
#include <list>

Expand All @@ -34,6 +34,7 @@ static std::string strRPCUserColonPass;

// These are created by StartRPCThreads, destroyed in StopRPCThreads
static asio::io_service* rpc_io_service = NULL;
static map<string, boost::shared_ptr<deadline_timer> > deadlineTimers;
static ssl::context* rpc_ssl_context = NULL;
static boost::thread_group* rpc_worker_group = NULL;

Expand Down Expand Up @@ -843,13 +844,34 @@ void StopRPCThreads()
{
if (rpc_io_service == NULL) return;

deadlineTimers.clear();
rpc_io_service->stop();
rpc_worker_group->join_all();
delete rpc_worker_group; rpc_worker_group = NULL;
delete rpc_ssl_context; rpc_ssl_context = NULL;
delete rpc_io_service; rpc_io_service = NULL;
}

void RPCRunHandler(const boost::system::error_code& err, boost::function<void(void)> func)
{
if (!err)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a comment that err will have a nonzero value when the timer was cancelled, I had to look this up as it was unclear to me why a timer handler would get an error passed in.

func();
}

void RPCRunLater(const std::string& name, boost::function<void(void)> func, int64 nSeconds)
{
assert(rpc_io_service != NULL);

if (deadlineTimers.count(name) == 0)
{
deadlineTimers.insert(make_pair(name,
boost::shared_ptr<deadline_timer>(new deadline_timer(*rpc_io_service))));
}
deadlineTimers[name]->expires_from_now(posix_time::seconds(nSeconds));
deadlineTimers[name]->async_wait(boost::bind(RPCRunHandler, _1, func));
}


class JSONRequest
{
public:
Expand Down
6 changes: 6 additions & 0 deletions src/bitcoinrpc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ void RPCTypeCheck(const json_spirit::Array& params,
void RPCTypeCheck(const json_spirit::Object& o,
const std::map<std::string, json_spirit::Value_type>& typesExpected, bool fAllowNull=false);

/*
Run func nSeconds from now. Uses boost deadline timers.
Overrides previous timer <name> (if any).
*/
void RPCRunLater(const std::string& name, boost::function<void(void)> func, int64 nSeconds);

typedef json_spirit::Value(*rpcfn_type)(const json_spirit::Array& params, bool fHelp);

class CRPCCommand
Expand Down
67 changes: 11 additions & 56 deletions src/rpcwallet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ Value getinfo(const Array& params, bool fHelp)
obj.push_back(Pair("keypoolsize", pwalletMain->GetKeyPoolSize()));
obj.push_back(Pair("paytxfee", ValueFromAmount(nTransactionFee)));
if (pwalletMain->IsCrypted())
obj.push_back(Pair("unlocked_until", (boost::int64_t)nWalletUnlockTime / 1000));
obj.push_back(Pair("unlocked_until", (boost::int64_t)nWalletUnlockTime));
obj.push_back(Pair("errors", GetWarnings("statusbar")));
return obj;
}
Expand Down Expand Up @@ -1256,56 +1256,11 @@ Value keypoolrefill(const Array& params, bool fHelp)
}


void ThreadTopUpKeyPool(void* parg)
static void LockWallet(CWallet* pWallet)
{
// Make this thread recognisable as the key-topping-up thread
RenameThread("bitcoin-key-top");

pwalletMain->TopUpKeyPool();
}

void ThreadCleanWalletPassphrase(void* parg)
{
// Make this thread recognisable as the wallet relocking thread
RenameThread("bitcoin-lock-wa");

int64 nMyWakeTime = GetTimeMillis() + *((int64*)parg) * 1000;

ENTER_CRITICAL_SECTION(cs_nWalletUnlockTime);

if (nWalletUnlockTime == 0)
{
nWalletUnlockTime = nMyWakeTime;

do
{
if (nWalletUnlockTime==0)
break;
int64 nToSleep = nWalletUnlockTime - GetTimeMillis();
if (nToSleep <= 0)
break;

LEAVE_CRITICAL_SECTION(cs_nWalletUnlockTime);
MilliSleep(nToSleep);
ENTER_CRITICAL_SECTION(cs_nWalletUnlockTime);

} while(1);

if (nWalletUnlockTime)
{
nWalletUnlockTime = 0;
pwalletMain->Lock();
}
}
else
{
if (nWalletUnlockTime < nMyWakeTime)
nWalletUnlockTime = nMyWakeTime;
}

LEAVE_CRITICAL_SECTION(cs_nWalletUnlockTime);

delete (int64*)parg;
LOCK(cs_nWalletUnlockTime);
nWalletUnlockTime = 0;
pWallet->Lock();
}

Value walletpassphrase(const Array& params, bool fHelp)
Expand All @@ -1319,9 +1274,6 @@ Value walletpassphrase(const Array& params, bool fHelp)
if (!pwalletMain->IsCrypted())
throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrase was called.");

if (!pwalletMain->IsLocked())
throw JSONRPCError(RPC_WALLET_ALREADY_UNLOCKED, "Error: Wallet is already unlocked.");

// Note that the walletpassphrase is stored in params[0] which is not mlock()ed
SecureString strWalletPass;
strWalletPass.reserve(100);
Expand All @@ -1339,9 +1291,12 @@ Value walletpassphrase(const Array& params, bool fHelp)
"walletpassphrase <passphrase> <timeout>\n"
"Stores the wallet decryption key in memory for <timeout> seconds.");

NewThread(ThreadTopUpKeyPool, NULL);
int64* pnSleepTime = new int64(params[1].get_int64());
NewThread(ThreadCleanWalletPassphrase, pnSleepTime);
pwalletMain->TopUpKeyPool();
Copy link
Contributor

Choose a reason for hiding this comment

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

This used to be the source of quite some lag when calling unlock, not sure if it matters, but thats why it was in a separate thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Topping up the keypool in a separate thread adds indeterminism which scares me. I'd rather the state of the wallet be known when walletpassphrase returns ("full keypool, unlocked").

E.g. if the keypool is completely exhausted, and I do 'walletpassphrase' immediately followed by a 'send', will I always get a fresh 'change' key or not? Separate thread == maybe, maybe not....

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe using anything from keypool tries to top up keypool before it uses any keys anyway. Also, the topup keypool thread locks wallet first thing anyway, so unless your machine is under heavy load (or a shitty VPS), it should be pretty constant. Anyway, if you dont care about the performance, it doesnt matter much.


int64 nSleepTime = params[1].get_int64();
LOCK(cs_nWalletUnlockTime);
nWalletUnlockTime = GetTime() + nSleepTime;
RPCRunLater("lockwallet", boost::bind(LockWallet, pwalletMain), nSleepTime);

return Value::null;
}
Expand Down
5 changes: 1 addition & 4 deletions src/wallet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,6 @@ bool CWallet::AddCScript(const CScript& redeemScript)

bool CWallet::Unlock(const SecureString& strWalletPassphrase)
{
if (!IsLocked())
return false;

CCrypter crypter;
CKeyingMaterial vMasterKey;

Expand All @@ -100,7 +97,7 @@ bool CWallet::Unlock(const SecureString& strWalletPassphrase)
if(!crypter.SetKeyFromPassphrase(strWalletPassphrase, pMasterKey.second.vchSalt, pMasterKey.second.nDeriveIterations, pMasterKey.second.nDerivationMethod))
return false;
if (!crypter.Decrypt(pMasterKey.second.vchCryptedKey, vMasterKey))
return false;
continue; // try another master key
if (CCryptoKeyStore::Unlock(vMasterKey))
return true;
}
Expand Down