Can create Watch Only HD wallet with -hdwatchonly #9728

Open
wants to merge 6 commits into
from
View
@@ -146,7 +146,8 @@ template<typename K, int Size, CChainParams::Base58Type Type> class CBitcoinExtK
K GetKey() {
K ret;
- if (vchData.size() == Size) {
+ const bool fCorrectVersion = vchVersion == Params().Base58Prefix(Type);
+ if (vchData.size() == Size && fCorrectVersion) {
// If base58 encoded data does not hold an ext key, return a !IsValid() key
ret.Decode(&vchData[0]);
}
View
@@ -106,6 +106,16 @@ bool CBasicKeyStore::HaveWatchOnly(const CScript &dest) const
return setWatchOnly.count(dest) > 0;
}
+bool CBasicKeyStore::HaveWatchOnly(const CKeyID &keyId) const
+{
+ LOCK(cs_KeyStore);
+ WatchKeyMap::const_iterator it = mapWatchKeys.find(keyId);
+ if (it != mapWatchKeys.end()) {
+ return true;
+ }
+ return false;
+}
+
bool CBasicKeyStore::HaveWatchOnly() const
{
LOCK(cs_KeyStore);
View
@@ -104,6 +104,7 @@ class CBasicKeyStore : public CKeyStore
virtual bool AddWatchOnly(const CScript &dest);
virtual bool RemoveWatchOnly(const CScript &dest);
virtual bool HaveWatchOnly(const CScript &dest) const;
+ virtual bool HaveWatchOnly(const CKeyID &keyId) const;
virtual bool HaveWatchOnly() const;
};
View
@@ -231,6 +231,11 @@ UniValue validateaddress(const JSONRPCRequest& request)
ret.push_back(Pair("hdmasterkeyid", it->second.hdMasterKeyID.GetHex()));
}
}
+
+ if (pwallet->IsHDWatchOnly()) {
+ CBitcoinExtPubKey watchOnly(pwallet->GetHDChain().masterPubKey);
+ ret.push_back(Pair("hdwatchonlykey", watchOnly.ToString()));
+ }
}
#endif
}
View
@@ -2399,7 +2399,8 @@ UniValue getwalletinfo(const JSONRPCRequest& request)
" \"keypoolsize_hd_internal\": xxxx, (numeric) how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)\n"
" \"unlocked_until\": ttt, (numeric) the timestamp in seconds since epoch (midnight Jan 1 1970 GMT) that the wallet is unlocked for transfers, or 0 if the wallet is locked\n"
" \"paytxfee\": x.xxxx, (numeric) the transaction fee configuration, set in " + CURRENCY_UNIT + "/kB\n"
- " \"hdmasterkeyid\": \"<hash160>\" (string) the Hash160 of the HD master pubkey\n"
+ " \"hdmasterkeyid\": \"<hash160>\" (string) the Hash160 of the HD master pubkey\n" +
+ " \"hdwatchonlykey\": \"<hdpubkey>\" (string) the extended pubkey used for key derivation in hdwatchonly mode\n"
"}\n"
"\nExamples:\n"
+ HelpExampleCli("getwalletinfo", "")
@@ -2425,6 +2426,10 @@ UniValue getwalletinfo(const JSONRPCRequest& request)
if (pwallet->IsCrypted()) {
obj.push_back(Pair("unlocked_until", pwallet->nRelockTime));
}
+ if (pwallet->IsHDWatchOnly()) {
+ CBitcoinExtPubKey watchOnly(pwallet->GetHDChain().masterPubKey);
+ obj.push_back(Pair("hdwatchonlykey", watchOnly.ToString()));
+ }
obj.push_back(Pair("paytxfee", ValueFromAmount(payTxFee.GetFeePerK())));
if (!masterKeyID.IsNull())
obj.push_back(Pair("hdmasterkeyid", masterKeyID.GetHex()));
View
@@ -93,77 +93,114 @@ CPubKey CWallet::GenerateNewKey(bool internal)
bool fCompressed = CanSupportFeature(FEATURE_COMPRPUBKEY); // default to compressed public keys if we want 0.6.0 wallets
CKey secret;
-
+ CPubKey pubkey;
// Create new metadata
int64_t nCreationTime = GetTime();
CKeyMetadata metadata(nCreationTime);
// use HD key derivation if HD was enabled during wallet creation
if (IsHDEnabled()) {
- DeriveNewChildKey(metadata, secret, (CanSupportFeature(FEATURE_HD_SPLIT) ? internal : false));
+ DeriveNewChildKey(metadata, secret, pubkey, (CanSupportFeature(FEATURE_HD_SPLIT) ? internal : false));
} else {
secret.MakeNewKey(fCompressed);
+ pubkey = secret.GetPubKey();
+ assert(secret.VerifyPubKey(pubkey));
}
// Compressed public keys were introduced in version 0.6.0
if (fCompressed)
SetMinVersion(FEATURE_COMPRPUBKEY);
- CPubKey pubkey = secret.GetPubKey();
- assert(secret.VerifyPubKey(pubkey));
-
mapKeyMetadata[pubkey.GetID()] = metadata;
UpdateTimeFirstKey(nCreationTime);
- if (!AddKeyPubKey(secret, pubkey))
- throw std::runtime_error(std::string(__func__) + ": AddKey failed");
+ if(!IsHDWatchOnly()) {
+ if (!AddKeyPubKey(secret, pubkey))
+ throw std::runtime_error(std::string(__func__) + ": AddKey failed");
+ }
+ else {
+ if (!AddWatchOnly(pubkey, metadata, nCreationTime))
+ throw std::runtime_error(std::string(__func__) + ": AddWatchOnly failed");
+ }
return pubkey;
}
-void CWallet::DeriveNewChildKey(CKeyMetadata& metadata, CKey& secret, bool internal)
+void CWallet::DeriveNewChildKey(CKeyMetadata& metadata, CKey& secret, CPubKey& pubkey, bool internal)
{
- // for now we use a fixed keypath scheme of m/0'/0'/k
CKey key; //master key seed (256bit)
- CExtKey masterKey; //hd master key
- CExtKey accountKey; //key at m/0'
- CExtKey chainChildKey; //key at m/0'/0' (external) or m/0'/1' (internal)
- CExtKey childKey; //key at m/0'/0'/<n>'
-
// try to get the master key
- if (!GetKey(hdChain.masterKeyID, key))
+ if (GetKey(hdChain.masterKeyID, key))
+ {
+ // for now we use a fixed keypath scheme of m/0'/0'/k
@jtimon

jtimon Jul 13, 2017

Member

it seems like you are repeating yourself a little bit here. Perhaps a common function can be extracted to avoid some code duplication between this if and the next one?

@NicolasDorier

NicolasDorier Jul 15, 2017

Member

I tried to think about this, but this was not as easy. I preferred keeping this way as it makes review easier: I did not touch what was already working.

+ CExtKey masterKey; //hd master key
+ CExtKey accountKey; //key at m/0'
+ CExtKey chainChildKey; //key at m/0'/0' (external) or m/0'/1' (internal)
+ CExtKey childKey; //key at m/0'/0'/<n>'
+
+ masterKey.SetMaster(key.begin(), key.size());
+
+ // derive m/0'
+ // use hardened derivation (child keys >= 0x80000000 are hardened after bip32)
+ masterKey.Derive(accountKey, BIP32_HARDENED_KEY_LIMIT);
+
+ // derive m/0'/0' (external chain) OR m/0'/1' (internal chain)
+ assert(internal ? CanSupportFeature(FEATURE_HD_SPLIT) : true);
+ accountKey.Derive(chainChildKey, BIP32_HARDENED_KEY_LIMIT + (internal ? 1 : 0));
+
+ // derive child key at next index, skip keys already known to the wallet
+ do {
+ // always derive hardened keys
+ // childIndex | BIP32_HARDENED_KEY_LIMIT = derive childIndex in hardened child-index-range
+ // example: 1 | BIP32_HARDENED_KEY_LIMIT == 0x80000001 == 2147483649
+ if (internal) {
+ chainChildKey.Derive(childKey, hdChain.nInternalChainCounter | BIP32_HARDENED_KEY_LIMIT);
+ metadata.hdKeypath = "m/0'/1'/" + std::to_string(hdChain.nInternalChainCounter) + "'";
+ hdChain.nInternalChainCounter++;
+ }
+ else {
+ chainChildKey.Derive(childKey, hdChain.nExternalChainCounter | BIP32_HARDENED_KEY_LIMIT);
+ metadata.hdKeypath = "m/0'/0'/" + std::to_string(hdChain.nExternalChainCounter) + "'";
+ hdChain.nExternalChainCounter++;
+ }
+ } while (HaveKey(childKey.key.GetPubKey().GetID()));
+ secret = childKey.key;
+ pubkey = childKey.key.GetPubKey();
+ assert(secret.VerifyPubKey(pubkey));
+ metadata.hdMasterKeyID = hdChain.masterKeyID;
+ // update the chain model in the database
+ if (!CWalletDB(*dbw).WriteHDChain(hdChain))
+ throw std::runtime_error(std::string(__func__) + ": Writing HD chain model failed");
+ }
+ else if (IsHDWatchOnly()) {
+ CExtPubKey& masterKey = hdChain.masterPubKey; //hd master key
+ CExtPubKey chainChildKey; //key at m/0 (external) or m/1 (internal)
+ CExtPubKey childKey; //key at m/0/<n>
+
+ // derive m/x
+ masterKey.Derive(chainChildKey, internal ? 1 : 0);
+
+ // derive child key at next index, skip keys already known to the wallet
+ do {
+ if (internal) {
+ chainChildKey.Derive(childKey, hdChain.nInternalChainCounter);
+ metadata.hdKeypath = "m/1/" + std::to_string(hdChain.nInternalChainCounter);
+ hdChain.nInternalChainCounter++;
+ }
+ else {
+ chainChildKey.Derive(childKey, hdChain.nExternalChainCounter);
+ metadata.hdKeypath = "m/0/" + std::to_string(hdChain.nExternalChainCounter);
+ hdChain.nExternalChainCounter++;
+ }
+ metadata.hdMasterKeyID = hdChain.masterKeyID;
+ } while (HaveWatchOnly(childKey.pubkey.GetID()));
+ pubkey = childKey.pubkey;
+ // update the chain model in the database
+ if (!CWalletDB(*dbw).WriteHDChain(hdChain))
+ throw std::runtime_error(std::string(__func__) + ": Writing HD chain model failed");
+ }
+ else {
throw std::runtime_error(std::string(__func__) + ": Master key not found");
-
- masterKey.SetMaster(key.begin(), key.size());
-
- // derive m/0'
- // use hardened derivation (child keys >= 0x80000000 are hardened after bip32)
- masterKey.Derive(accountKey, BIP32_HARDENED_KEY_LIMIT);
-
- // derive m/0'/0' (external chain) OR m/0'/1' (internal chain)
- assert(internal ? CanSupportFeature(FEATURE_HD_SPLIT) : true);
- accountKey.Derive(chainChildKey, BIP32_HARDENED_KEY_LIMIT+(internal ? 1 : 0));
-
- // derive child key at next index, skip keys already known to the wallet
- do {
- // always derive hardened keys
- // childIndex | BIP32_HARDENED_KEY_LIMIT = derive childIndex in hardened child-index-range
- // example: 1 | BIP32_HARDENED_KEY_LIMIT == 0x80000001 == 2147483649
- if (internal) {
- chainChildKey.Derive(childKey, hdChain.nInternalChainCounter | BIP32_HARDENED_KEY_LIMIT);
- metadata.hdKeypath = "m/0'/1'/" + std::to_string(hdChain.nInternalChainCounter) + "'";
- hdChain.nInternalChainCounter++;
- }
- else {
- chainChildKey.Derive(childKey, hdChain.nExternalChainCounter | BIP32_HARDENED_KEY_LIMIT);
- metadata.hdKeypath = "m/0'/0'/" + std::to_string(hdChain.nExternalChainCounter) + "'";
- hdChain.nExternalChainCounter++;
- }
- } while (HaveKey(childKey.key.GetPubKey().GetID()));
- secret = childKey.key;
- metadata.hdMasterKeyID = hdChain.masterKeyID;
- // update the chain model in the database
- if (!CWalletDB(*dbw).WriteHDChain(hdChain))
- throw std::runtime_error(std::string(__func__) + ": Writing HD chain model failed");
+ }
}
bool CWallet::AddKeyPubKey(const CKey& secret, const CPubKey &pubkey)
@@ -275,6 +312,29 @@ bool CWallet::AddWatchOnly(const CScript& dest, int64_t nCreateTime)
return AddWatchOnly(dest);
}
+bool CWallet::AddWatchOnly(const CPubKey &pubkey, const CKeyMetadata& meta, int64_t nCreateTime)
+{
+ auto script = GetScriptForDestination(pubkey.GetID());
+ if (!HaveWatchOnly(script))
+ {
+ mapKeyMetadata[CScriptID(script)] = meta;
+ if (!AddWatchOnly(script, nCreateTime))
+ return false;
+ }
+
+ if (!CWalletDB(*dbw).WriteKeyMeta(pubkey, meta))
+ return false;
+
+ script = GetScriptForRawPubKey(pubkey);
+ if (!HaveWatchOnly(script))
+ {
+ mapKeyMetadata[CScriptID(script)] = meta;
+ if (!AddWatchOnly(script, nCreateTime))
+ return false;
+ }
+ return true;
+}
+
bool CWallet::RemoveWatchOnly(const CScript &dest)
{
AssertLockHeld(cs_wallet);
@@ -1355,6 +1415,25 @@ bool CWallet::SetHDMasterKey(const CPubKey& pubkey)
return true;
}
+bool CWallet::SetHDMasterKey(const CExtPubKey& extPubKey)
+{
+ LOCK(cs_wallet);
+
+ // ensure this wallet.dat can only be opened by clients supporting HD
+ SetMinVersion(FEATURE_WATCHONLY_HD);
+
+ // store the keyid (hash160) together with
+ // the child index counter in the database
+ // as a hdchain object
+ CHDChain newHdChain;
+ newHdChain.masterKeyID = extPubKey.pubkey.GetID();
+ newHdChain.isWatchOnly = true;
+ newHdChain.masterPubKey = extPubKey;
+ SetHDChain(newHdChain, false);
+
+ return true;
+}
+
bool CWallet::SetHDChain(const CHDChain& chain, bool memonly)
{
LOCK(cs_wallet);
@@ -1370,6 +1449,11 @@ bool CWallet::IsHDEnabled() const
return !hdChain.masterKeyID.IsNull();
}
+bool CWallet::IsHDWatchOnly() const
+{
+ return IsHDEnabled() && hdChain.isWatchOnly;
+}
+
int64_t CWalletTx::GetTxTime() const
{
int64_t n = nTimeSmart;
@@ -1810,8 +1894,18 @@ bool CWalletTx::IsTrusted() const
if (parent == NULL)
return false;
const CTxOut& parentOut = parent->tx->vout[txin.prevout.n];
- if (pwallet->IsMine(parentOut) != ISMINE_SPENDABLE)
- return false;
+ const auto& isMine = pwallet->IsMine(parentOut);
+ if (isMine != ISMINE_SPENDABLE)
+ {
+ // If the wallet is hdwatchonly, check if it is a key we generated
+ if (!pwallet->IsHDWatchOnly() || isMine != ISMINE_WATCH_SOLVABLE)
+ return false;
+ const auto& meta = pwallet->mapKeyMetadata;
+ auto it = meta.find(CScriptID(parentOut.scriptPubKey));
+ if (it == meta.end() ||
+ it->second.hdKeypath.empty())
@instagibbs

instagibbs Jun 27, 2017 edited

Member

I think we may need to generalize this check a bit more. If the script here is a multisig in which the watch-only wallet owns all the keys, this will return false since there is no hdKeypath.

Practically this means that spending 0-conf p2sh funds is a dicey experience that results in "Insufficient funds" responses sometimes.

Perhaps something like pwalletMain->IsHDWatchOnly(CScriptID& scriptID) to make this check, which accounts for this situation?

@instagibbs

instagibbs Jun 27, 2017

Member

Since we're not even required to be backwards compatible, I think adding a new piece of key metadata fIsHDWatchOnly, and adding that during importaddress(or some new call) when all keys are hdwatchonly keys(or a mixture of watchonly and stored private keys?).

@NicolasDorier

NicolasDorier Jun 28, 2017 edited

Member

Does IsTrusted() returns ISMINE_SPENDABLE when for a multi sig output where we know all the keys when not hdwatchonly?

@instagibbs

instagibbs Jun 28, 2017

Member

If you have the private keys, yes, because of check a few lines above.

@NicolasDorier

NicolasDorier Jun 29, 2017

Member

So your goal is that importaddress scriptPubKeys should be considered trusted if we are in hdwatchonly mode ?

@instagibbs

instagibbs Jun 29, 2017

Member

If we completely those addresses via hdwatchonly keys and/or privkeys, yes.

@instagibbs

instagibbs Jun 29, 2017

Member

Currently in this mode you can import a privkey and spend unconfirmed outputs. Likewise you can spend unconfirmed hdwatchonly outputs. You cannot however spend a mixture of the two in a multisig, or pure hdwatchonly p2sh.

+ return false;
+ }
}
return true;
}
@@ -3185,8 +3279,11 @@ void CWallet::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool int
CKeyPool tmpKeypool;
if (!walletdb.ReadPool(id, tmpKeypool))
throw std::runtime_error(std::string(__func__) + ": read failed");
- if (!HaveKey(tmpKeypool.vchPubKey.GetID()))
+ if (!IsHDWatchOnly() && !HaveKey(tmpKeypool.vchPubKey.GetID()))
throw std::runtime_error(std::string(__func__) + ": unknown key in key pool");
+ if (IsHDWatchOnly())
+ if (!HaveWatchOnly(tmpKeypool.vchPubKey.GetID()))
+ throw std::runtime_error(std::string(__func__) + ": unknown key in key pool");
if (!IsHDEnabled() || !CanSupportFeature(FEATURE_HD_SPLIT) || tmpKeypool.fInternal == internal)
{
nIndex = id;
@@ -3747,6 +3844,7 @@ std::string CWallet::GetWalletHelpString(bool showDebug)
strUsage += HelpMessageOpt("-spendzeroconfchange", strprintf(_("Spend unconfirmed change when sending transactions (default: %u)"), DEFAULT_SPEND_ZEROCONF_CHANGE));
strUsage += HelpMessageOpt("-txconfirmtarget=<n>", strprintf(_("If paytxfee is not set, include enough fee so transactions begin confirmation on average within n blocks (default: %u)"), DEFAULT_TX_CONFIRM_TARGET));
strUsage += HelpMessageOpt("-usehd", _("Use hierarchical deterministic key generation (HD) after BIP32. Only has effect during wallet creation/first start") + " " + strprintf(_("(default: %u)"), DEFAULT_USE_HD_WALLET));
+ strUsage += HelpMessageOpt("-hdwatchonly", _("Create a new watch-only HD wallet from a BIP32 HD public key"));
strUsage += HelpMessageOpt("-walletrbf", strprintf(_("Send transactions with full-RBF opt-in enabled (default: %u)"), DEFAULT_WALLET_RBF));
strUsage += HelpMessageOpt("-upgradewallet", _("Upgrade wallet to latest format on startup"));
strUsage += HelpMessageOpt("-wallet=<file>", _("Specify wallet file (within data directory)") + " " + strprintf(_("(default: %s)"), DEFAULT_WALLET_DAT));
@@ -3841,6 +3939,26 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile)
walletInstance->SetMaxVersion(nMaxVersion);
}
+
+ std::string hdWatchOnly = GetArg("-hdwatchonly", "");
+ CExtPubKey extPubKey;
+ if (!hdWatchOnly.empty()) {
+ CBitcoinExtPubKey bitcoinExtPubKey(hdWatchOnly);
+ extPubKey = bitcoinExtPubKey.GetKey();
+ if (!extPubKey.pubkey.IsFullyValid()) {
+ InitError(_("Invalid ExtPubKey format"));
+ return NULL;
+ }
+
+ if (!fFirstRun) {
+ if (!walletInstance->IsHDWatchOnly() ||
+ walletInstance->GetHDChain().masterKeyID != extPubKey.pubkey.GetID()) {
+ InitError(_("Cannot specify new hdwatchonly on an already existing wallet"));
+ return NULL;
+ }
+ }
+ }
+
if (fFirstRun)
{
// Create new keyUser and set as default key
@@ -3850,9 +3968,15 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile)
walletInstance->SetMinVersion(FEATURE_HD_SPLIT);
// generate a new master key
- CPubKey masterPubKey = walletInstance->GenerateNewHDMasterKey();
- if (!walletInstance->SetHDMasterKey(masterPubKey))
- throw std::runtime_error(std::string(__func__) + ": Storing master key failed");
+ if(hdWatchOnly.empty()) {
+ CPubKey masterPubKey = walletInstance->GenerateNewHDMasterKey();
+ if (!walletInstance->SetHDMasterKey(masterPubKey))
+ throw std::runtime_error(std::string(__func__) + ": Storing master key failed");
+ }
+ else {
+ if (!walletInstance->SetHDMasterKey(extPubKey))
+ throw std::runtime_error(std::string(__func__) + ": Storing master key failed");
+ }
}
CPubKey newDefaultKey;
if (walletInstance->GetKeyFromPool(newDefaultKey, false)) {
Oops, something went wrong.