maxmustermann/dogecoin
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Use importmulti timestamp when importing watch only keys

Browse source 
When importing a watch-only address over importmulti with a specific timestamp,
the wallet's nTimeFirstKey is currently set to 1. After this change, the
provided timestamp will be used and stored as metadata associated with
watch-only key. This can improve wallet performance because it can avoid the
need to scan the entire blockchain for watch only addresses when timestamps are
provided.

Also adds timestamp to validateaddress return value (needed for tests).

Fixes #9034.
This commit is contained in:
Russell Yanofsky 2016-11-08 16:55:02 -05:00
parent a58370e6a2
commit a80f98b1c7
7 changed files with 107 additions and 35 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
qa/rpc-tests/importmulti.py
View file

@ -20,6 +20,7 @@ class ImportMultiTest (BitcoinTestFramework):
print ("Mining blocks...")
self.nodes[0].generate(1)
self.nodes[1].generate(1)
timestamp = self.nodes[1].getblock(self.nodes[1].getbestblockhash())['mediantime']
# keyword definition
PRIV_KEY = 'privkey'
@ -59,6 +60,9 @@ class ImportMultiTest (BitcoinTestFramework):
address_assert = self.nodes[1].validateaddress(address['address'])
assert_equal(address_assert['iswatchonly'], True)
assert_equal(address_assert['ismine'], False)
assert_equal(address_assert['timestamp'], timestamp)
watchonly_address = address['address']
watchonly_timestamp = timestamp
# ScriptPubKey + internal
@ -73,6 +77,7 @@ class ImportMultiTest (BitcoinTestFramework):
address_assert = self.nodes[1].validateaddress(address['address'])
assert_equal(address_assert['iswatchonly'], True)
assert_equal(address_assert['ismine'], False)
assert_equal(address_assert['timestamp'], timestamp)
# ScriptPubKey + !internal
print("Should not import a scriptPubKey without internal flag")
@ -87,6 +92,7 @@ class ImportMultiTest (BitcoinTestFramework):
address_assert = self.nodes[1].validateaddress(address['address'])
assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False)
assert_equal('timestamp' in address_assert, False)
# Address + Public key + !Internal
@ -103,6 +109,7 @@ class ImportMultiTest (BitcoinTestFramework):
address_assert = self.nodes[1].validateaddress(address['address'])
assert_equal(address_assert['iswatchonly'], True)
assert_equal(address_assert['ismine'], False)
assert_equal(address_assert['timestamp'], timestamp)
# ScriptPubKey + Public key + internal
@ -119,6 +126,7 @@ class ImportMultiTest (BitcoinTestFramework):
address_assert = self.nodes[1].validateaddress(address['address'])
assert_equal(address_assert['iswatchonly'], True)
assert_equal(address_assert['ismine'], False)
assert_equal(address_assert['timestamp'], timestamp)
# ScriptPubKey + Public key + !internal
print("Should not import a scriptPubKey without internal and with public key")
@ -135,11 +143,11 @@ class ImportMultiTest (BitcoinTestFramework):
address_assert = self.nodes[1].validateaddress(address['address'])
assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False)
assert_equal('timestamp' in address_assert, False)
# Address + Private key + !watchonly
print("Should import an address with private key")
address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
timestamp = self.nodes[1].getblock(self.nodes[1].getbestblockhash())['mediantime']
result = self.nodes[1].importmulti([{
"scriptPubKey": {
"address": address['address']
@ -170,6 +178,7 @@ class ImportMultiTest (BitcoinTestFramework):
address_assert = self.nodes[1].validateaddress(address['address'])
assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False)
assert_equal('timestamp' in address_assert, False)
# ScriptPubKey + Private key + internal
print("Should import a scriptPubKey with internal and with private key")
@ -184,6 +193,7 @@ class ImportMultiTest (BitcoinTestFramework):
address_assert = self.nodes[1].validateaddress(address['address'])
assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], True)
assert_equal(address_assert['timestamp'], timestamp)
# ScriptPubKey + Private key + !internal
print("Should not import a scriptPubKey without internal and with private key")
@ -199,6 +209,7 @@ class ImportMultiTest (BitcoinTestFramework):
address_assert = self.nodes[1].validateaddress(address['address'])
assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False)
assert_equal('timestamp' in address_assert, False)
# P2SH address
@ -209,6 +220,7 @@ class ImportMultiTest (BitcoinTestFramework):
self.nodes[1].generate(100)
transactionid = self.nodes[1].sendtoaddress(multi_sig_script['address'], 10.00)
self.nodes[1].generate(1)
timestamp = self.nodes[1].getblock(self.nodes[1].getbestblockhash())['mediantime']
transaction = self.nodes[1].gettransaction(transactionid)
print("Should import a p2sh")
@ -222,6 +234,7 @@ class ImportMultiTest (BitcoinTestFramework):
address_assert = self.nodes[1].validateaddress(multi_sig_script['address'])
assert_equal(address_assert['isscript'], True)
assert_equal(address_assert['iswatchonly'], True)
assert_equal(address_assert['timestamp'], timestamp)
p2shunspent = self.nodes[1].listunspent(0,999999, [multi_sig_script['address']])[0]
assert_equal(p2shunspent['spendable'], False)
assert_equal(p2shunspent['solvable'], False)
@ -235,6 +248,7 @@ class ImportMultiTest (BitcoinTestFramework):
self.nodes[1].generate(100)
transactionid = self.nodes[1].sendtoaddress(multi_sig_script['address'], 10.00)
self.nodes[1].generate(1)
timestamp = self.nodes[1].getblock(self.nodes[1].getbestblockhash())['mediantime']
transaction = self.nodes[1].gettransaction(transactionid)
print("Should import a p2sh with respective redeem script")
@ -246,6 +260,8 @@ class ImportMultiTest (BitcoinTestFramework):
"redeemscript": multi_sig_script['redeemScript']
}])
assert_equal(result[0]['success'], True)
address_assert = self.nodes[1].validateaddress(multi_sig_script['address'])
assert_equal(address_assert['timestamp'], timestamp)
p2shunspent = self.nodes[1].listunspent(0,999999, [multi_sig_script['address']])[0]
assert_equal(p2shunspent['spendable'], False)
@ -260,6 +276,7 @@ class ImportMultiTest (BitcoinTestFramework):
self.nodes[1].generate(100)
transactionid = self.nodes[1].sendtoaddress(multi_sig_script['address'], 10.00)
self.nodes[1].generate(1)
timestamp = self.nodes[1].getblock(self.nodes[1].getbestblockhash())['mediantime']
transaction = self.nodes[1].gettransaction(transactionid)
print("Should import a p2sh with respective redeem script and private keys")
@ -272,6 +289,8 @@ class ImportMultiTest (BitcoinTestFramework):
"keys": [ self.nodes[0].dumpprivkey(sig_address_1['address']), self.nodes[0].dumpprivkey(sig_address_2['address'])]
}])
assert_equal(result[0]['success'], True)
address_assert = self.nodes[1].validateaddress(multi_sig_script['address'])
assert_equal(address_assert['timestamp'], timestamp)
p2shunspent = self.nodes[1].listunspent(0,999999, [multi_sig_script['address']])[0]
assert_equal(p2shunspent['spendable'], False)
@ -319,6 +338,7 @@ class ImportMultiTest (BitcoinTestFramework):
address_assert = self.nodes[1].validateaddress(address['address'])
assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False)
assert_equal('timestamp' in address_assert, False)
# ScriptPubKey + Public key + internal + Wrong pubkey
@ -338,6 +358,7 @@ class ImportMultiTest (BitcoinTestFramework):
address_assert = self.nodes[1].validateaddress(address['address'])
assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False)
assert_equal('timestamp' in address_assert, False)
# Address + Private key + !watchonly + Wrong private key
@ -357,6 +378,7 @@ class ImportMultiTest (BitcoinTestFramework):
address_assert = self.nodes[1].validateaddress(address['address'])
assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False)
assert_equal('timestamp' in address_assert, False)
# ScriptPubKey + Private key + internal + Wrong private key
@ -375,6 +397,15 @@ class ImportMultiTest (BitcoinTestFramework):
address_assert = self.nodes[1].validateaddress(address['address'])
assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False)
assert_equal('timestamp' in address_assert, False)
# restart nodes to check for proper serialization/deserialization of watch only address
stop_nodes(self.nodes)
self.nodes = start_nodes(2, self.options.tmpdir)
address_assert = self.nodes[1].validateaddress(watchonly_address)
assert_equal(address_assert['iswatchonly'], True)
assert_equal(address_assert['ismine'], False)
assert_equal(address_assert['timestamp'], watchonly_timestamp);
# Bad or missing timestamps
print("Should throw on invalid or missing timestamp values")

3
src/rpc/misc.cpp
View file

@ -208,6 +208,9 @@ UniValue validateaddress(const JSONRPCRequest& request)
if (pwalletMain) {
const auto& meta = pwalletMain->mapKeyMetadata;
auto it = address.GetKeyID(keyID) ? meta.find(keyID) : meta.end();
if (it == meta.end()) {
it = meta.find(CScriptID(scriptPubKey));
}
if (it != meta.end()) {
ret.push_back(Pair("timestamp", it->second.nCreateTime));
if (!it->second.hdKeypath.empty()) {

20
src/wallet/rpcdump.cpp
View file

@ -161,7 +161,7 @@ void ImportScript(const CScript& script, const string& strLabel, bool isRedeemSc
pwalletMain->MarkDirty();
if (!pwalletMain->HaveWatchOnly(script) && !pwalletMain->AddWatchOnly(script))
if (!pwalletMain->HaveWatchOnly(script) && !pwalletMain->AddWatchOnly(script, 0 /* nCreateTime */))
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
if (isRedeemScript) {
@ -575,15 +575,17 @@ UniValue dumpwallet(const JSONRPCRequest& request)
if (!file.is_open())
throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file");
std::map<CKeyID, int64_t> mapKeyBirth;
std::map<CTxDestination, int64_t> mapKeyBirth;
std::set<CKeyID> setKeyPool;
pwalletMain->GetKeyBirthTimes(mapKeyBirth);
pwalletMain->GetAllReserveKeys(setKeyPool);
// sort time/key pairs
std::vector<std::pair<int64_t, CKeyID> > vKeyBirth;
for (std::map<CKeyID, int64_t>::const_iterator it = mapKeyBirth.begin(); it != mapKeyBirth.end(); it++) {
vKeyBirth.push_back(std::make_pair(it->second, it->first));
for (const auto& entry : mapKeyBirth) {
if (const CKeyID* keyID = boost::get<CKeyID>(&entry.first)) { // set and test
vKeyBirth.push_back(std::make_pair(entry.second, *keyID));
}
}
mapKeyBirth.clear();
std::sort(vKeyBirth.begin(), vKeyBirth.end());
@ -720,7 +722,7 @@ UniValue ProcessImport(const UniValue& data, const int64_t timestamp)
pwalletMain->MarkDirty();
if (!pwalletMain->HaveWatchOnly(redeemScript) && !pwalletMain->AddWatchOnly(redeemScript)) {
if (!pwalletMain->HaveWatchOnly(redeemScript) && !pwalletMain->AddWatchOnly(redeemScript, timestamp)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
}
@ -737,7 +739,7 @@ UniValue ProcessImport(const UniValue& data, const int64_t timestamp)
pwalletMain->MarkDirty();
if (!pwalletMain->HaveWatchOnly(redeemDestination) && !pwalletMain->AddWatchOnly(redeemDestination)) {
if (!pwalletMain->HaveWatchOnly(redeemDestination) && !pwalletMain->AddWatchOnly(redeemDestination, timestamp)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
}
@ -830,7 +832,7 @@ UniValue ProcessImport(const UniValue& data, const int64_t timestamp)
pwalletMain->MarkDirty();
if (!pwalletMain->HaveWatchOnly(pubKeyScript) && !pwalletMain->AddWatchOnly(pubKeyScript)) {
if (!pwalletMain->HaveWatchOnly(pubKeyScript) && !pwalletMain->AddWatchOnly(pubKeyScript, timestamp)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
}
@ -848,7 +850,7 @@ UniValue ProcessImport(const UniValue& data, const int64_t timestamp)
pwalletMain->MarkDirty();
if (!pwalletMain->HaveWatchOnly(scriptRawPubKey) && !pwalletMain->AddWatchOnly(scriptRawPubKey)) {
if (!pwalletMain->HaveWatchOnly(scriptRawPubKey) && !pwalletMain->AddWatchOnly(scriptRawPubKey, timestamp)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
}
@ -922,7 +924,7 @@ UniValue ProcessImport(const UniValue& data, const int64_t timestamp)
pwalletMain->MarkDirty();
if (!pwalletMain->HaveWatchOnly(script) && !pwalletMain->AddWatchOnly(script)) {
if (!pwalletMain->HaveWatchOnly(script) && !pwalletMain->AddWatchOnly(script, timestamp)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
}

27
src/wallet/wallet.cpp
View file

@ -206,11 +206,11 @@ bool CWallet::AddCryptedKey(const CPubKey &vchPubKey,
return false;
}
bool CWallet::LoadKeyMetadata(const CPubKey &pubkey, const CKeyMetadata &meta)
bool CWallet::LoadKeyMetadata(const CTxDestination& keyID, const CKeyMetadata &meta)
{
AssertLockHeld(cs_wallet); // mapKeyMetadata
UpdateTimeFirstKey(meta.nCreateTime);
mapKeyMetadata[pubkey.GetID()] = meta;
mapKeyMetadata[keyID] = meta;
return true;
}
@ -256,15 +256,22 @@ bool CWallet::LoadCScript(const CScript& redeemScript)
return CCryptoKeyStore::AddCScript(redeemScript);
}
bool CWallet::AddWatchOnly(const CScript &dest)
bool CWallet::AddWatchOnly(const CScript& dest)
{
if (!CCryptoKeyStore::AddWatchOnly(dest))
return false;
nTimeFirstKey = 1; // No birthday information for watch-only keys.
const CKeyMetadata& meta = mapKeyMetadata[CScriptID(dest)];
UpdateTimeFirstKey(meta.nCreateTime);
NotifyWatchonlyChanged(true);
if (!fFileBacked)
return true;
return CWalletDB(strWalletFile).WriteWatchOnly(dest);
return CWalletDB(strWalletFile).WriteWatchOnly(dest, meta);
}
bool CWallet::AddWatchOnly(const CScript& dest, int64_t nCreateTime)
{
mapKeyMetadata[CScriptID(dest)].nCreateTime = nCreateTime;
return AddWatchOnly(dest);
}
bool CWallet::RemoveWatchOnly(const CScript &dest)
@ -3425,14 +3432,16 @@ public:
void operator()(const CNoDestination &none) {}
};
void CWallet::GetKeyBirthTimes(std::map<CKeyID, int64_t> &mapKeyBirth) const {
void CWallet::GetKeyBirthTimes(std::map<CTxDestination, int64_t> &mapKeyBirth) const {
AssertLockHeld(cs_wallet); // mapKeyMetadata
mapKeyBirth.clear();
// get birth times for keys with metadata
for (std::map<CKeyID, CKeyMetadata>::const_iterator it = mapKeyMetadata.begin(); it != mapKeyMetadata.end(); it++)
if (it->second.nCreateTime)
mapKeyBirth[it->first] = it->second.nCreateTime;
for (const auto& entry : mapKeyMetadata) {
if (entry.second.nCreateTime) {
mapKeyBirth[entry.first] = entry.second.nCreateTime;
}
}
// map in which we'll infer heights of other keys
CBlockIndex *pindexMax = chainActive[std::max(0, chainActive.Height() - 144)]; // the tip can be reorganized; use a 144-block safety margin

21
src/wallet/wallet.h
View file

@ -614,6 +614,17 @@ private:
int64_t nTimeFirstKey;
/**
* Private version of AddWatchOnly method which does not accept a
* timestamp, and which will reset the wallet's nTimeFirstKey value to 1 if
* the watch key did not previously have a timestamp associated with it.
* Because this is an inherited virtual method, it is accessible despite
* being marked private, but it is marked private anyway to encourage use
* of the other AddWatchOnly which accepts a timestamp and sets
* nTimeFirstKey more intelligently for more efficient rescans.
*/
bool AddWatchOnly(const CScript& dest) override;
public:
/*
* Main wallet lock.
@ -638,7 +649,9 @@ public:
mapKeyMetadata[keyid] = CKeyMetadata(keypool.nTime);
}
std::map<CKeyID, CKeyMetadata> mapKeyMetadata;
// Map from Key ID (for regular keys) or Script ID (for watch-only keys) to
// key metadata.
std::map<CTxDestination, CKeyMetadata> mapKeyMetadata;
typedef std::map<unsigned int, CMasterKey> MasterKeyMap;
MasterKeyMap mapMasterKeys;
@ -728,7 +741,7 @@ public:
//! Adds a key to the store, without saving it to disk (used by LoadWallet)
bool LoadKey(const CKey& key, const CPubKey &pubkey) { return CCryptoKeyStore::AddKeyPubKey(key, pubkey); }
//! Load metadata (used by LoadWallet)
bool LoadKeyMetadata(const CPubKey &pubkey, const CKeyMetadata &metadata);
bool LoadKeyMetadata(const CTxDestination& pubKey, const CKeyMetadata &metadata);
bool LoadMinVersion(int nVersion) { AssertLockHeld(cs_wallet); nWalletVersion = nVersion; nWalletMaxVersion = std::max(nWalletMaxVersion, nVersion); return true; }
void UpdateTimeFirstKey(int64_t nCreateTime);
@ -750,7 +763,7 @@ public:
bool GetDestData(const CTxDestination &dest, const std::string &key, std::string *value) const;
//! Adds a watch-only address to the store, and saves it to disk.
bool AddWatchOnly(const CScript &dest);
bool AddWatchOnly(const CScript& dest, int64_t nCreateTime);
bool RemoveWatchOnly(const CScript &dest);
//! Adds a watch-only address to the store, without saving it to disk (used by LoadWallet)
bool LoadWatchOnly(const CScript &dest);
@ -759,7 +772,7 @@ public:
bool ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase);
bool EncryptWallet(const SecureString& strWalletPassphrase);
void GetKeyBirthTimes(std::map<CKeyID, int64_t> &mapKeyBirth) const;
void GetKeyBirthTimes(std::map<CTxDestination, int64_t> &mapKeyBirth) const;
/**
* Increment the next transaction order id

36
src/wallet/walletdb.cpp
View file

@ -120,15 +120,19 @@ bool CWalletDB::WriteCScript(const uint160& hash, const CScript& redeemScript)
return Write(std::make_pair(std::string("cscript"), hash), *(const CScriptBase*)(&redeemScript), false);
}
bool CWalletDB::WriteWatchOnly(const CScript &dest)
bool CWalletDB::WriteWatchOnly(const CScript &dest, const CKeyMetadata& keyMeta)
{
nWalletDBUpdateCounter++;
if (!Write(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest)), keyMeta))
return false;
return Write(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest)), '1');
}
bool CWalletDB::EraseWatchOnly(const CScript &dest)
{
nWalletDBUpdateCounter++;
if (!Erase(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest))))
return false;
return Erase(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest)));
}
@ -259,6 +263,7 @@ class CWalletScanState {
public:
unsigned int nKeys;
unsigned int nCKeys;
unsigned int nWatchKeys;
unsigned int nKeyMeta;
bool fIsEncrypted;
bool fAnyUnordered;
@ -266,7 +271,7 @@ public:
vector<uint256> vWalletUpgrade;
CWalletScanState() {
nKeys = nCKeys = nKeyMeta = 0;
nKeys = nCKeys = nWatchKeys = nKeyMeta = 0;
fIsEncrypted = false;
fAnyUnordered = false;
nFileVersion = 0;
@ -348,16 +353,13 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
}
else if (strType == "watchs")
{
wss.nWatchKeys++;
CScript script;
ssKey >> *(CScriptBase*)(&script);
char fYes;
ssValue >> fYes;
if (fYes == '1')
pwallet->LoadWatchOnly(script);
// Watch-only addresses have no birthday information for now,
// so set the wallet birthday to the beginning of time.
pwallet->UpdateTimeFirstKey(1);
}
else if (strType == "key" || strType == "wkey")
{
@ -458,15 +460,27 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
}
wss.fIsEncrypted = true;
}
else if (strType == "keymeta")
else if (strType == "keymeta" || strType == "watchmeta")
{
CPubKey vchPubKey;
ssKey >> vchPubKey;
CTxDestination keyID;
if (strType == "keymeta")
{
CPubKey vchPubKey;
ssKey >> vchPubKey;
keyID = vchPubKey.GetID();
}
else if (strType == "watchmeta")
{
CScript script;
ssKey >> *(CScriptBase*)(&script);
keyID = CScriptID(script);
}
CKeyMetadata keyMeta;
ssValue >> keyMeta;
wss.nKeyMeta++;
pwallet->LoadKeyMetadata(vchPubKey, keyMeta);
pwallet->LoadKeyMetadata(keyID, keyMeta);
}
else if (strType == "defaultkey")
{
@ -620,7 +634,7 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet)
wss.nKeys, wss.nCKeys, wss.nKeyMeta, wss.nKeys + wss.nCKeys);
// nTimeFirstKey is only reliable if all keys have metadata
if ((wss.nKeys + wss.nCKeys) != wss.nKeyMeta)
if ((wss.nKeys + wss.nCKeys + wss.nWatchKeys) != wss.nKeyMeta)
pwallet->UpdateTimeFirstKey(1);
BOOST_FOREACH(uint256 hash, wss.vWalletUpgrade)

2
src/wallet/walletdb.h
View file

@ -135,7 +135,7 @@ public:
bool WriteCScript(const uint160& hash, const CScript& redeemScript);
bool WriteWatchOnly(const CScript &script);
bool WriteWatchOnly(const CScript &script, const CKeyMetadata &keymeta);
bool EraseWatchOnly(const CScript &script);
bool WriteBestBlock(const CBlockLocator& locator);