Skip to content
Permalink
Browse files

Merge #9108: Use importmulti timestamp when importing watch only keys…

… (on top of #9682)

a80f98b Use importmulti timestamp when importing watch only keys (Russell Yanofsky)
a58370e Dedup nTimeFirstKey update logic (Russell Yanofsky)
  • Loading branch information...
laanwj committed Feb 15, 2017
2 parents 4c69d68 + a80f98b commit d8e8b06bd0659b35e51effe35400408ec15bb09b
Showing with 130 additions and 57 deletions.
  1. +32 −1 qa/rpc-tests/importmulti.py
  2. +3 −0 src/rpc/misc.cpp
  3. +15 −18 src/wallet/rpcdump.cpp
  4. +32 −14 src/wallet/wallet.cpp
  5. +21 −6 src/wallet/wallet.h
  6. +26 −17 src/wallet/walletdb.cpp
  7. +1 −1 src/wallet/walletdb.h
@@ -20,6 +20,7 @@ def run_test (self):
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 @@ def run_test (self):
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 @@ def run_test (self):
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 @@ def run_test (self):
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 @@ def run_test (self):
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 @@ def run_test (self):
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 @@ def run_test (self):
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 @@ def run_test (self):
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 @@ def run_test (self):
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 @@ def run_test (self):
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 @@ def run_test (self):
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 @@ def run_test (self):
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 @@ def run_test (self):
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 @@ def run_test (self):
"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 @@ def run_test (self):
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 @@ def run_test (self):
"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 @@ def run_test (self):
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 @@ def run_test (self):
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 @@ def run_test (self):
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 @@ def run_test (self):
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")
@@ -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()) {
@@ -143,7 +143,7 @@ UniValue importprivkey(const JSONRPCRequest& request)
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet");

// whenever a key is imported, we need to scan the whole chain
pwalletMain->nTimeFirstKey = 1; // 0 would be considered 'no value'
pwalletMain->UpdateTimeFirstKey(1);

if (fRescan) {
pwalletMain->ScanForWalletTransactions(chainActive.Genesis(), true);
@@ -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) {
@@ -500,8 +500,7 @@ UniValue importwallet(const JSONRPCRequest& request)
while (pindex && pindex->pprev && pindex->GetBlockTime() > nTimeBegin - 7200)
pindex = pindex->pprev;

if (!pwalletMain->nTimeFirstKey || nTimeBegin < pwalletMain->nTimeFirstKey)
pwalletMain->nTimeFirstKey = nTimeBegin;
pwalletMain->UpdateTimeFirstKey(nTimeBegin);

LogPrintf("Rescanning last %i blocks\n", chainActive.Height() - pindex->nHeight + 1);
pwalletMain->ScanForWalletTransactions(pindex);
@@ -576,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());
@@ -721,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");
}

@@ -738,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");
}

@@ -782,9 +783,7 @@ UniValue ProcessImport(const UniValue& data, const int64_t timestamp)
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet");
}

if (timestamp < pwalletMain->nTimeFirstKey) {
pwalletMain->nTimeFirstKey = timestamp;
}
pwalletMain->UpdateTimeFirstKey(timestamp);
}
}

@@ -833,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");
}

@@ -851,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");
}

@@ -912,9 +911,7 @@ UniValue ProcessImport(const UniValue& data, const int64_t timestamp)
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet");
}

if (timestamp < pwalletMain->nTimeFirstKey) {
pwalletMain->nTimeFirstKey = timestamp;
}
pwalletMain->UpdateTimeFirstKey(timestamp);

success = true;
}
@@ -927,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");
}

@@ -113,8 +113,7 @@ CPubKey CWallet::GenerateNewKey()
assert(secret.VerifyPubKey(pubkey));

mapKeyMetadata[pubkey.GetID()] = metadata;
if (!nTimeFirstKey || nCreationTime < nTimeFirstKey)
nTimeFirstKey = nCreationTime;
UpdateTimeFirstKey(nCreationTime);

if (!AddKeyPubKey(secret, pubkey))
throw std::runtime_error(std::string(__func__) + ": AddKey failed");
@@ -207,13 +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
if (meta.nCreateTime && (!nTimeFirstKey || meta.nCreateTime < nTimeFirstKey))
nTimeFirstKey = meta.nCreateTime;

mapKeyMetadata[pubkey.GetID()] = meta;
UpdateTimeFirstKey(meta.nCreateTime);
mapKeyMetadata[keyID] = meta;
return true;
}

@@ -222,6 +219,18 @@ bool CWallet::LoadCryptedKey(const CPubKey &vchPubKey, const std::vector<unsigne
return CCryptoKeyStore::AddCryptedKey(vchPubKey, vchCryptedSecret);
}

void CWallet::UpdateTimeFirstKey(int64_t nCreateTime)
{
AssertLockHeld(cs_wallet);
if (nCreateTime <= 1) {
// Cannot determine birthday information, so set the wallet birthday to
// the beginning of time.
nTimeFirstKey = 1;
} else if (!nTimeFirstKey || nCreateTime < nTimeFirstKey) {
nTimeFirstKey = nCreateTime;
}
}

bool CWallet::AddCScript(const CScript& redeemScript)
{
if (!CCryptoKeyStore::AddCScript(redeemScript))
@@ -247,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)
@@ -3416,14 +3432,16 @@ class CAffectedKeysVisitor : public boost::static_visitor<void> {
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

0 comments on commit d8e8b06

Please sign in to comment.
You can’t perform that action at this time.