Skip to content
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

Jump to bottom

Add support for watch-only addresses #2861

Closed
wants to merge 1 commit into from
Closed

Add support for watch-only addresses #2861

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
2 changes: 2 additions & 0 deletions src/bitcoinrpc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ static const CRPCCommand vRPCCommands[] =
{ "dumpwallet", &dumpwallet, true, false, true },
{ "importprivkey", &importprivkey, false, false, true },
{ "importwallet", &importwallet, false, false, true },
{ "importaddress", &importaddress, false, false, true },
{ "listunspent", &listunspent, false, false, true },
{ "getrawtransaction", &getrawtransaction, false, false, false },
{ "createrawtransaction", &createrawtransaction, false, false, false },
Expand Down Expand Up @@ -1245,6 +1246,7 @@ Array RPCConvertValues(const std::string &strMethod, const std::vector<std::stri
if (strMethod == "lockunspent" && n > 0) ConvertTo<bool>(params[0]);
if (strMethod == "lockunspent" && n > 1) ConvertTo<Array>(params[1]);
if (strMethod == "importprivkey" && n > 2) ConvertTo<bool>(params[2]);
if (strMethod == "importaddress" && n > 2) ConvertTo<bool>(params[2]);
if (strMethod == "verifychain" && n > 0) ConvertTo<boost::int64_t>(params[0]);
if (strMethod == "verifychain" && n > 1) ConvertTo<boost::int64_t>(params[1]);
if (strMethod == "keypoolrefill" && n > 0) ConvertTo<boost::int64_t>(params[0]);
Expand Down
1 change: 1 addition & 0 deletions src/bitcoinrpc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ extern json_spirit::Value getnettotals(const json_spirit::Array& params, bool fH

extern json_spirit::Value dumpprivkey(const json_spirit::Array& params, bool fHelp); // in rpcdump.cpp
extern json_spirit::Value importprivkey(const json_spirit::Array& params, bool fHelp);
extern json_spirit::Value importaddress(const json_spirit::Array& params, bool fHelp);
extern json_spirit::Value dumpwallet(const json_spirit::Array& params, bool fHelp);
extern json_spirit::Value importwallet(const json_spirit::Array& params, bool fHelp);

Expand Down
13 changes: 13 additions & 0 deletions src/keystore.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,19 @@ bool CBasicKeyStore::GetCScript(const CScriptID &hash, CScript& redeemScriptOut)
return false;
}

bool CBasicKeyStore::AddWatchOnly(const CTxDestination &dest)
{
LOCK(cs_KeyStore);
setWatchOnly.insert(dest);
return true;
}

bool CBasicKeyStore::HaveWatchOnly(const CTxDestination &dest) const
{
LOCK(cs_KeyStore);
return setWatchOnly.count(dest) > 0;
}

bool CCryptoKeyStore::SetCrypted()
{
LOCK(cs_KeyStore);
Expand Down
24 changes: 24 additions & 0 deletions src/keystore.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,24 @@
#include "crypter.h"
#include "sync.h"
#include <boost/signals2/signal.hpp>
#include <boost/variant.hpp>

class CScript;

class CNoDestination {
public:
friend bool operator==(const CNoDestination &a, const CNoDestination &b) { return true; }
friend bool operator<(const CNoDestination &a, const CNoDestination &b) { return true; }
};

/** A txout script template with a specific destination. It is either:
* * CNoDestination: no destination set
* * CKeyID: TX_PUBKEYHASH destination
* * CScriptID: TX_SCRIPTHASH destination
* A CTxDestination is the internal data type encoded in a CBitcoinAddress
*/
typedef boost::variant<CNoDestination, CKeyID, CScriptID> CTxDestination;

/** A virtual base class for key stores */
class CKeyStore
{
Expand All @@ -34,17 +49,23 @@ class CKeyStore
virtual bool AddCScript(const CScript& redeemScript) =0;
virtual bool HaveCScript(const CScriptID &hash) const =0;
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;
};

typedef std::map<CKeyID, CKey> KeyMap;
typedef std::map<CScriptID, CScript > ScriptMap;
typedef std::set<CTxDestination> WatchOnlySet;

/** Basic key store, that keeps keys in an address->secret map */
class CBasicKeyStore : public CKeyStore
{
protected:
KeyMap mapKeys;
ScriptMap mapScripts;
WatchOnlySet setWatchOnly;

public:
bool AddKeyPubKey(const CKey& key, const CPubKey &pubkey);
Expand Down Expand Up @@ -86,6 +107,9 @@ class CBasicKeyStore : public CKeyStore
virtual bool AddCScript(const CScript& redeemScript);
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;
};

typedef std::map<CKeyID, std::pair<CPubKey, std::vector<unsigned char> > > CryptedKeyMap;
Expand Down
45 changes: 45 additions & 0 deletions src/rpcdump.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,51 @@ Value importprivkey(const Array& params, bool fHelp)
return Value::null;
}

Value importaddress(const Array& params, bool fHelp)
{
if (fHelp || params.size() < 1 || params.size() > 3)
throw runtime_error(
"importaddress <address> [label] [rescan=true]\n"
"Adds an address that can be watched as if it were in your wallet but cannot be used to spend.");
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm in favor of keeping importaddress as it is, but if we do allow the wallet to contain redeemScripts that you don't have the keys to spend it'd make sense to also add my "addredeemscript" patch, but more importantly make sure addmultisigaddress automatically creates a watch-only address if you add a multi-sig address to your wallet for which you don't have all the private keys.

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason for not automatically causing ownership when a multisig address is created, is because the software still deals very badly with double spending, so you don't want multiple instances to be able to spend the same coins simultaneously. With watch-only, that concern doesn't exist, and I think the best solution is consider anything related to the imported address 'mine'.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, you're saying that when adding a multisig script for which you don't have all keys, it should automatically become watch-only. That seems reasonable, but maybe the semantics will be too confusing in that case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, scrap that idea then.

I see how it could cause confusion, especially for someone debugging a RPC-using thing who is assuming they can spend every addr.


CBitcoinAddress address(params[0].get_str());
if (!address.IsValid())
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address");
CTxDestination dest;
dest = address.Get();

string strLabel = "";
if (params.size() > 1)
strLabel = params[1].get_str();

// Whether to perform rescan after import
bool fRescan = true;
if (params.size() > 2)
fRescan = params[2].get_bool();

{
LOCK2(cs_main, pwalletMain->cs_wallet);

// Don't throw error in case an address is already there
if (pwalletMain->HaveWatchOnly(dest))
return Value::null;

pwalletMain->MarkDirty();
pwalletMain->SetAddressBook(dest, strLabel, "receive");

if (!pwalletMain->AddWatchOnly(dest))
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");

if (fRescan)
{
pwalletMain->ScanForWalletTransactions(chainActive.Genesis(), true);
pwalletMain->ReacceptWalletTransactions();
}
}

return Value::null;
}

Value importwallet(const Array& params, bool fHelp)
{
if (fHelp || params.size() != 1)
Expand Down
1 change: 1 addition & 0 deletions src/rpcrawtransaction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ Value listunspent(const Array& params, bool fHelp)
}
entry.push_back(Pair("amount",ValueFromAmount(nValue)));
entry.push_back(Pair("confirmations",out.nDepth));
entry.push_back(Pair("spendable", out.fSpendable));
results.push_back(entry);
}

Expand Down
52 changes: 31 additions & 21 deletions src/rpcwallet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1424,36 +1424,45 @@ Value encryptwallet(const Array& params, bool fHelp)

class DescribeAddressVisitor : public boost::static_visitor<Object>
{
private:
isminetype mine;

public:
DescribeAddressVisitor(isminetype mineIn) : mine(mineIn) {}

Object operator()(const CNoDestination &dest) const { return Object(); }

Object operator()(const CKeyID &keyID) const {
Object obj;
CPubKey vchPubKey;
pwalletMain->GetPubKey(keyID, vchPubKey);
obj.push_back(Pair("isscript", false));
obj.push_back(Pair("pubkey", HexStr(vchPubKey)));
obj.push_back(Pair("iscompressed", vchPubKey.IsCompressed()));
if (mine == MINE_SPENDABLE) {
pwalletMain->GetPubKey(keyID, vchPubKey);
obj.push_back(Pair("pubkey", HexStr(vchPubKey)));
obj.push_back(Pair("iscompressed", vchPubKey.IsCompressed()));
}
return obj;
}

Object operator()(const CScriptID &scriptID) const {
Object obj;
obj.push_back(Pair("isscript", true));
CScript subscript;
pwalletMain->GetCScript(scriptID, subscript);
std::vector<CTxDestination> addresses;
txnouttype whichType;
int nRequired;
ExtractDestinations(subscript, whichType, addresses, nRequired);
obj.push_back(Pair("script", GetTxnOutputType(whichType)));
obj.push_back(Pair("hex", HexStr(subscript.begin(), subscript.end())));
Array a;
BOOST_FOREACH(const CTxDestination& addr, addresses)
a.push_back(CBitcoinAddress(addr).ToString());
obj.push_back(Pair("addresses", a));
if (whichType == TX_MULTISIG)
obj.push_back(Pair("sigsrequired", nRequired));
if (mine == MINE_SPENDABLE) {
CScript subscript;
pwalletMain->GetCScript(scriptID, subscript);
std::vector<CTxDestination> addresses;
txnouttype whichType;
int nRequired;
ExtractDestinations(subscript, whichType, addresses, nRequired);
obj.push_back(Pair("script", GetTxnOutputType(whichType)));
obj.push_back(Pair("hex", HexStr(subscript.begin(), subscript.end())));
Array a;
BOOST_FOREACH(const CTxDestination& addr, addresses)
a.push_back(CBitcoinAddress(addr).ToString());
obj.push_back(Pair("addresses", a));
if (whichType == TX_MULTISIG)
obj.push_back(Pair("sigsrequired", nRequired));
}
return obj;
}
};
Expand All @@ -1475,10 +1484,11 @@ Value validateaddress(const Array& params, bool fHelp)
CTxDestination dest = address.Get();
string currentAddress = address.ToString();
ret.push_back(Pair("address", currentAddress));
bool fMine = pwalletMain ? IsMine(*pwalletMain, dest) : false;
ret.push_back(Pair("ismine", fMine));
if (fMine) {
Object detail = boost::apply_visitor(DescribeAddressVisitor(), dest);
isminetype mine = pwalletMain ? IsMine(*pwalletMain, dest) : MINE_NO;
ret.push_back(Pair("ismine", mine != MINE_NO));
if (mine != MINE_NO) {
ret.push_back(Pair("watchonly", mine == MINE_WATCH_ONLY));
Object detail = boost::apply_visitor(DescribeAddressVisitor(mine), dest);
ret.insert(ret.end(), detail.begin(), detail.end());
}
if (pwalletMain && pwalletMain->mapAddressBook.count(dest))
Expand Down
52 changes: 39 additions & 13 deletions src/script.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1454,36 +1454,57 @@ class CKeyStoreIsMineVisitor : public boost::static_visitor<bool>
bool operator()(const CScriptID &scriptID) const { return keystore->HaveCScript(scriptID); }
};

bool IsMine(const CKeyStore &keystore, const CTxDestination &dest)
isminetype IsMine(const CKeyStore &keystore, const CTxDestination &dest)
{
return boost::apply_visitor(CKeyStoreIsMineVisitor(&keystore), dest);
if (boost::apply_visitor(CKeyStoreIsMineVisitor(&keystore), dest))
return MINE_SPENDABLE;
if (keystore.HaveWatchOnly(dest))
return MINE_WATCH_ONLY;
return MINE_NO;
}

bool IsMine(const CKeyStore &keystore, const CScript& scriptPubKey)
isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey)
{
vector<valtype> vSolutions;
txnouttype whichType;
if (!Solver(scriptPubKey, whichType, vSolutions))
return false;
if (!Solver(scriptPubKey, whichType, vSolutions)) {
if (keystore.HaveWatchOnly(scriptPubKey.GetID()))
return MINE_WATCH_ONLY;
return MINE_NO;
}

CKeyID keyID;
switch (whichType)
{
case TX_NONSTANDARD:
case TX_NULL_DATA:
return false;
break;
case TX_PUBKEY:
keyID = CPubKey(vSolutions[0]).GetID();
return keystore.HaveKey(keyID);
if (keystore.HaveKey(keyID))
return MINE_SPENDABLE;
if (keystore.HaveWatchOnly(keyID))
return MINE_WATCH_ONLY;
break;
case TX_PUBKEYHASH:
keyID = CKeyID(uint160(vSolutions[0]));
return keystore.HaveKey(keyID);
if (keystore.HaveKey(keyID))
return MINE_SPENDABLE;
if (keystore.HaveWatchOnly(keyID))
return MINE_WATCH_ONLY;
break;
case TX_SCRIPTHASH:
{
CScriptID scriptID = CScriptID(uint160(vSolutions[0]));
CScript subscript;
if (!keystore.GetCScript(CScriptID(uint160(vSolutions[0])), subscript))
return false;
return IsMine(keystore, subscript);
if (keystore.GetCScript(scriptID, subscript)) {
isminetype ret = IsMine(keystore, subscript);
if (ret)
return ret;
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems inconsistent with the way DescribeAddressVisitor is written: it allows for the case where the wallet has a redeemScript in it that can't be spent, yet the visitor will only show information on the redeemScript if it can be spent. I think it'd make more sense for it to be possible to have a redeemScript in your wallet that you can't spend, and for DescribeAddressVisitor to still show all the information known about the address in that case.

}
if (keystore.HaveWatchOnly(scriptID))
return MINE_WATCH_ONLY;
break;
}
case TX_MULTISIG:
{
Expand All @@ -1493,10 +1514,15 @@ bool IsMine(const CKeyStore &keystore, const CScript& scriptPubKey)
// them) enable spend-out-from-under-you attacks, especially
// in shared-wallet situations.
vector<valtype> keys(vSolutions.begin()+1, vSolutions.begin()+vSolutions.size()-1);
return HaveKeys(keys, keystore) == keys.size();
if (HaveKeys(keys, keystore) == keys.size())
return MINE_SPENDABLE;
break;
}
}
return false;

if (keystore.HaveWatchOnly(scriptPubKey.GetID()))
return MINE_WATCH_ONLY;
return MINE_NO;
}

bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet)
Expand Down
27 changes: 10 additions & 17 deletions src/script.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include <vector>

#include <boost/foreach.hpp>
#include <boost/variant.hpp>

#include "keystore.h"
#include "bignum.h"
Expand Down Expand Up @@ -38,6 +37,14 @@ enum
SCRIPT_VERIFY_NOCACHE = (1U << 3), // do not store results in signature cache (but do query it)
};

/** IsMine() return codes */
enum isminetype
{
MINE_NO = 0,
MINE_WATCH_ONLY = 1,
MINE_SPENDABLE = 2,
};

enum txnouttype
{
TX_NONSTANDARD,
Expand All @@ -49,20 +56,6 @@ enum txnouttype
TX_NULL_DATA,
};

class CNoDestination {
public:
friend bool operator==(const CNoDestination &a, const CNoDestination &b) { return true; }
friend bool operator<(const CNoDestination &a, const CNoDestination &b) { return true; }
};

/** A txout script template with a specific destination. It is either:
* * CNoDestination: no destination set
* * CKeyID: TX_PUBKEYHASH destination
* * CScriptID: TX_SCRIPTHASH destination
* A CTxDestination is the internal data type encoded in a CBitcoinAddress
*/
typedef boost::variant<CNoDestination, CKeyID, CScriptID> CTxDestination;

const char* GetTxnOutputType(txnouttype t);

/** Script opcodes */
Expand Down Expand Up @@ -686,8 +679,8 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
bool Solver(const CScript& scriptPubKey, txnouttype& typeRet, std::vector<std::vector<unsigned char> >& vSolutionsRet);
int ScriptSigArgsExpected(txnouttype t, const std::vector<std::vector<unsigned char> >& vSolutions);
bool IsStandard(const CScript& scriptPubKey, txnouttype& whichType);
bool IsMine(const CKeyStore& keystore, const CScript& scriptPubKey);
bool IsMine(const CKeyStore& keystore, const CTxDestination &dest);
isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey);
isminetype IsMine(const CKeyStore& keystore, const CTxDestination &dest);
void ExtractAffectedKeys(const CKeyStore &keystore, const CScript& scriptPubKey, std::vector<CKeyID> &vKeys);
bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet);
bool ExtractDestinations(const CScript& scriptPubKey, txnouttype& typeRet, std::vector<CTxDestination>& addressRet, int& nRequiredRet);
Expand Down
2 changes: 1 addition & 1 deletion src/test/wallet_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ static void add_coin(int64 nValue, int nAge = 6*24, bool fIsFromMe = false, int
wtx->fDebitCached = true;
wtx->nDebitCached = 1;
}
COutput output(wtx, nInput, nAge);
COutput output(wtx, nInput, nAge, true);
vCoins.push_back(output);
}

Expand Down
Loading