From d5087d1ba08142bdf135333a0da08ef0f5fc7ef0 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 9 Jun 2014 21:11:59 +0200 Subject: [PATCH] Use script matching rather than destination matching for watch-only. This changes the keystore data format, wallet format and IsMine logic to detect watch-only outputs based on direct script matching rather than first trying to convert outputs to destinations (addresses). The reason is that we don't know how the software that has the spending keys works. It may support the same types of scripts as us, but that is not guaranteed. Furthermore, it removes the ambiguity between addresses used as identifiers for output scripts or identifiers for public keys. One practical implication is that adding a normal pay-to-pubkey-hash address via importaddress will not cause payments to the corresponding full public key to be detected as IsMine. If that is wanted, add those scripts directly (importaddress now also accepts any hex-encoded script). Conflicts: src/wallet.cpp --- src/keystore.cpp | 4 ++-- src/keystore.h | 10 +++++----- src/rpcdump.cpp | 23 +++++++++++++++-------- src/script.cpp | 22 +++++++--------------- src/script.h | 2 +- src/wallet.cpp | 15 ++++++++------- src/wallet.h | 4 ++-- src/walletdb.cpp | 12 ++++++------ src/walletdb.h | 2 +- 9 files changed, 47 insertions(+), 47 deletions(-) diff --git a/src/keystore.cpp b/src/keystore.cpp index c2ea1ce5a..2a4c88d56 100644 --- a/src/keystore.cpp +++ b/src/keystore.cpp @@ -59,14 +59,14 @@ bool CBasicKeyStore::GetCScript(const CScriptID &hash, CScript& redeemScriptOut) return false; } -bool CBasicKeyStore::AddWatchOnly(const CTxDestination &dest) +bool CBasicKeyStore::AddWatchOnly(const CScript &dest) { LOCK(cs_KeyStore); setWatchOnly.insert(dest); return true; } -bool CBasicKeyStore::HaveWatchOnly(const CTxDestination &dest) const +bool CBasicKeyStore::HaveWatchOnly(const CScript &dest) const { LOCK(cs_KeyStore); return setWatchOnly.count(dest) > 0; diff --git a/src/keystore.h b/src/keystore.h index 90fc3a4c7..72411a138 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -48,13 +48,13 @@ public: virtual bool GetCScript(const CScriptID &hash, CScript& redeemScriptOut) const =0; // Support for Watch-only addresses - virtual bool AddWatchOnly(const CTxDestination &dest) =0; - virtual bool HaveWatchOnly(const CTxDestination &dest) const =0; + virtual bool AddWatchOnly(const CScript &dest) =0; + virtual bool HaveWatchOnly(const CScript &dest) const =0; }; typedef std::map KeyMap; typedef std::map ScriptMap; -typedef std::set WatchOnlySet; +typedef std::set WatchOnlySet; /** Basic key store, that keeps keys in an address->secret map */ class CBasicKeyStore : public CKeyStore @@ -105,8 +105,8 @@ public: virtual bool HaveCScript(const CScriptID &hash) const; virtual bool GetCScript(const CScriptID &hash, CScript& redeemScriptOut) const; - virtual bool AddWatchOnly(const CTxDestination &dest); - virtual bool HaveWatchOnly(const CTxDestination &dest) const; + virtual bool AddWatchOnly(const CScript &dest); + virtual bool HaveWatchOnly(const CScript &dest) const; }; typedef std::vector > CKeyingMaterial; diff --git a/src/rpcdump.cpp b/src/rpcdump.cpp index 98af4695d..e505d84b4 100644 --- a/src/rpcdump.cpp +++ b/src/rpcdump.cpp @@ -138,13 +138,19 @@ Value importaddress(const Array& params, bool fHelp) if (fHelp || params.size() < 1 || params.size() > 3) throw runtime_error( "importaddress
[label] [rescan=true]\n" - "Adds an address that can be watched as if it were in your wallet but cannot be used to spend."); + "Adds an address or script (in hex) that can be watched as if it were in your wallet but cannot be used to spend."); + + CScript script; CBitcoinAddress address(params[0].get_str()); - if (!address.IsValid()) - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address"); - CTxDestination dest; - dest = address.Get(); + if (address.IsValid()) { + script.SetDestination(address.Get()); + } else if (IsHex(params[0].get_str())) { + std::vector data(ParseHex(params[0].get_str())); + script = CScript(data.begin(), data.end()); + } else { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address or script"); + } string strLabel = ""; if (params.size() > 1) @@ -159,15 +165,16 @@ Value importaddress(const Array& params, bool fHelp) LOCK2(cs_main, pwalletMain->cs_wallet); // add to address book or update label - pwalletMain->SetAddressBook(dest, strLabel, "receive"); + if (address.IsValid()) + pwalletMain->SetAddressBook(address.Get(), strLabel, "receive"); // Don't throw error in case an address is already there - if (pwalletMain->HaveWatchOnly(dest)) + if (pwalletMain->HaveWatchOnly(script)) return Value::null; pwalletMain->MarkDirty(); - if (!pwalletMain->AddWatchOnly(dest)) + if (!pwalletMain->AddWatchOnly(script)) throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); if (fRescan) diff --git a/src/script.cpp b/src/script.cpp index 0ef012625..89f752bd1 100644 --- a/src/script.cpp +++ b/src/script.cpp @@ -1456,13 +1456,11 @@ public: bool operator()(const CScriptID &scriptID) const { return keystore->HaveCScript(scriptID); } }; -isminetype IsMine(const CKeyStore &keystore, const CTxDestination &dest) +isminetype IsMine(const CKeyStore &keystore, const CTxDestination& dest) { - if (boost::apply_visitor(CKeyStoreIsMineVisitor(&keystore), dest)) - return MINE_SPENDABLE; - if (keystore.HaveWatchOnly(dest)) - return MINE_WATCH_ONLY; - return MINE_NO; + CScript script; + script.SetDestination(dest); + return IsMine(keystore, script); } isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey) @@ -1470,7 +1468,7 @@ isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey) vector vSolutions; txnouttype whichType; if (!Solver(scriptPubKey, whichType, vSolutions)) { - if (keystore.HaveWatchOnly(scriptPubKey.GetID())) + if (keystore.HaveWatchOnly(scriptPubKey)) return MINE_WATCH_ONLY; return MINE_NO; } @@ -1485,15 +1483,11 @@ isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey) keyID = CPubKey(vSolutions[0]).GetID(); if (keystore.HaveKey(keyID)) return MINE_SPENDABLE; - if (keystore.HaveWatchOnly(keyID)) - return MINE_WATCH_ONLY; break; case TX_PUBKEYHASH: keyID = CKeyID(uint160(vSolutions[0])); if (keystore.HaveKey(keyID)) return MINE_SPENDABLE; - if (keystore.HaveWatchOnly(keyID)) - return MINE_WATCH_ONLY; break; case TX_SCRIPTHASH: { @@ -1501,11 +1495,9 @@ isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey) CScript subscript; if (keystore.GetCScript(scriptID, subscript)) { isminetype ret = IsMine(keystore, subscript); - if (ret) + if (ret == MINE_SPENDABLE) return ret; } - if (keystore.HaveWatchOnly(scriptID)) - return MINE_WATCH_ONLY; break; } case TX_MULTISIG: @@ -1522,7 +1514,7 @@ isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey) } } - if (keystore.HaveWatchOnly(scriptPubKey.GetID())) + if (keystore.HaveWatchOnly(scriptPubKey)) return MINE_WATCH_ONLY; return MINE_NO; } diff --git a/src/script.h b/src/script.h index 56c7d01d4..edba5757e 100644 --- a/src/script.h +++ b/src/script.h @@ -812,7 +812,7 @@ bool Solver(const CScript& scriptPubKey, txnouttype& typeRet, std::vector >& vSolutions); bool IsStandard(const CScript& scriptPubKey, txnouttype& whichType); isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey); -isminetype IsMine(const CKeyStore& keystore, const CTxDestination &dest); +isminetype IsMine(const CKeyStore& keystore, const CTxDestination& dest); void ExtractAffectedKeys(const CKeyStore &keystore, const CScript& scriptPubKey, std::vector &vKeys); bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet); bool ExtractDestinations(const CScript& scriptPubKey, txnouttype& typeRet, std::vector& addressRet, int& nRequiredRet); diff --git a/src/wallet.cpp b/src/wallet.cpp index 4b480321a..445d98b90 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -145,7 +145,7 @@ bool CWallet::LoadCScript(const CScript& redeemScript) return CCryptoKeyStore::AddCScript(redeemScript); } -bool CWallet::AddWatchOnly(const CTxDestination &dest) +bool CWallet::AddWatchOnly(const CScript &dest) { if (!CCryptoKeyStore::AddWatchOnly(dest)) return false; @@ -155,9 +155,8 @@ bool CWallet::AddWatchOnly(const CTxDestination &dest) return CWalletDB(strWalletFile).WriteWatchOnly(dest); } -bool CWallet::LoadWatchOnly(const CTxDestination &dest) +bool CWallet::LoadWatchOnly(const CScript &dest) { - LogPrintf("Loaded %s!\n", CBitcoinAddress(dest).ToString().c_str()); return CCryptoKeyStore::AddWatchOnly(dest); } @@ -729,17 +728,19 @@ int64_t CWallet::GetDebit(const CTxIn &txin, const isminefilter& filter) const bool CWallet::IsChange(const CTxOut& txout) const { - CTxDestination address; - // TODO: fix handling of 'change' outputs. The assumption is that any - // payment to a TX_PUBKEYHASH that is mine but isn't in the address book + // payment to a script that is ours, but is not in the address book // is change. That assumption is likely to break when we implement multisignature // wallets that return change back into a multi-signature-protected address; // a better way of identifying which outputs are 'the send' and which are // 'the change' will need to be implemented (maybe extend CWalletTx to remember // which output, if any, was change). - if (ExtractDestination(txout.scriptPubKey, address) && ::IsMine(*this, address) == MINE_SPENDABLE) + if (::IsMine(*this, txout.scriptPubKey)) { + CTxDestination address; + if (!ExtractDestination(txout.scriptPubKey, address)) + return true; + LOCK(cs_wallet); if (!mapAddressBook.count(address)) return true; diff --git a/src/wallet.h b/src/wallet.h index 3453d23d9..b44b73568 100644 --- a/src/wallet.h +++ b/src/wallet.h @@ -227,9 +227,9 @@ 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 CTxDestination &dest); + bool AddWatchOnly(const CScript &dest); // Adds a watch-only address to the store, without saving it to disk (used by LoadWallet) - bool LoadWatchOnly(const CTxDestination &dest); + bool LoadWatchOnly(const CScript &dest); bool Unlock(const SecureString& strWalletPassphrase); bool ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase); diff --git a/src/walletdb.cpp b/src/walletdb.cpp index 9338d5709..a95baf83d 100644 --- a/src/walletdb.cpp +++ b/src/walletdb.cpp @@ -112,10 +112,10 @@ bool CWalletDB::WriteCScript(const uint160& hash, const CScript& redeemScript) return Write(std::make_pair(std::string("cscript"), hash), redeemScript, false); } -bool CWalletDB::WriteWatchOnly(const CTxDestination &dest) +bool CWalletDB::WriteWatchOnly(const CScript &dest) { nWalletDBUpdated++; - return Write(std::make_pair(std::string("watch"), CBitcoinAddress(dest).ToString()), '1'); + return Write(std::make_pair(std::string("watchs"), dest), '1'); } bool CWalletDB::WriteBestBlock(const CBlockLocator& locator) @@ -410,14 +410,14 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, wss.fAnyUnordered = true; } } - else if (strType == "watch") + else if (strType == "watchs") { - std::string strAddress; - ssKey >> strAddress; + CScript script; + ssKey >> script; char fYes; ssValue >> fYes; if (fYes == '1') - pwallet->LoadWatchOnly(CBitcoinAddress(strAddress).Get()); + pwallet->LoadWatchOnly(script); // Watch-only addresses have no birthday information for now, // so set the wallet birthday to the beginning of time. diff --git a/src/walletdb.h b/src/walletdb.h index 3a6cb152a..58b4571b1 100644 --- a/src/walletdb.h +++ b/src/walletdb.h @@ -94,7 +94,7 @@ public: bool WriteCScript(const uint160& hash, const CScript& redeemScript); - bool WriteWatchOnly(const CTxDestination &dest); + bool WriteWatchOnly(const CScript &script); bool WriteBestBlock(const CBlockLocator& locator); bool ReadBestBlock(CBlockLocator& locator);