diff --git a/qa/rpc-tests/importmulti.py b/qa/rpc-tests/importmulti.py index 52e40d6c1c..02a932e737 100755 --- a/qa/rpc-tests/importmulti.py +++ b/qa/rpc-tests/importmulti.py @@ -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") diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 25fad3c2e3..6fd50127bd 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -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()) { diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index f358d720cd..0a3225937e 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -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 mapKeyBirth; + std::map mapKeyBirth; std::set setKeyPool; pwalletMain->GetKeyBirthTimes(mapKeyBirth); pwalletMain->GetAllReserveKeys(setKeyPool); // sort time/key pairs std::vector > vKeyBirth; - for (std::map::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(&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"); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 192af22cb8..f8f5a9306d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -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 &mapKeyBirth) const { +void CWallet::GetKeyBirthTimes(std::map &mapKeyBirth) const { AssertLockHeld(cs_wallet); // mapKeyMetadata mapKeyBirth.clear(); // get birth times for keys with metadata - for (std::map::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 diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 8d3ee9dddc..990c3bdf41 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -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 mapKeyMetadata; + // Map from Key ID (for regular keys) or Script ID (for watch-only keys) to + // key metadata. + std::map mapKeyMetadata; typedef std::map 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 &mapKeyBirth) const; + void GetKeyBirthTimes(std::map &mapKeyBirth) const; /** * Increment the next transaction order id diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 15e1e4c45f..106a59d562 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -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 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) diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 8437a95ba7..c7c65465df 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -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);