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.

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

Import key origin data through descriptors in importmulti #14021

Merged
merged 9 commits into from
Feb 14, 2019
1 change: 1 addition & 0 deletions src/rpc/rawtransaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <script/sign.h>
#include <script/standard.h>
#include <uint256.h>
#include <util/bip32.h>
#include <util/strencodings.h>
#include <validation.h>
#include <validationinterface.h>
Expand Down
17 changes: 4 additions & 13 deletions src/script/descriptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <script/standard.h>

#include <span.h>
#include <util/bip32.h>
#include <util/system.h>
#include <util/strencodings.h>

Expand All @@ -25,16 +26,6 @@ namespace {

typedef std::vector<uint32_t> KeyPath;

std::string FormatKeyPath(const KeyPath& path)
{
std::string ret;
for (auto i : path) {
ret += strprintf("/%i", (i << 1) >> 1);
if (i >> 31) ret += '\'';
}
return ret;
}

/** Interface for public key objects in descriptors. */
struct PubkeyProvider
{
Expand Down Expand Up @@ -63,7 +54,7 @@ class OriginPubkeyProvider final : public PubkeyProvider

std::string OriginString() const
{
return HexStr(std::begin(m_origin.fingerprint), std::end(m_origin.fingerprint)) + FormatKeyPath(m_origin.path);
return HexStr(std::begin(m_origin.fingerprint), std::end(m_origin.fingerprint)) + FormatHDKeypath(m_origin.path);
}

public:
Expand Down Expand Up @@ -184,7 +175,7 @@ class BIP32PubkeyProvider final : public PubkeyProvider
}
std::string ToString() const override
{
std::string ret = EncodeExtPubKey(m_extkey) + FormatKeyPath(m_path);
std::string ret = EncodeExtPubKey(m_extkey) + FormatHDKeypath(m_path);
if (IsRange()) {
ret += "/*";
if (m_derive == DeriveType::HARDENED) ret += '\'';
Expand All @@ -195,7 +186,7 @@ class BIP32PubkeyProvider final : public PubkeyProvider
{
CExtKey key;
if (!GetExtKey(arg, key)) return false;
out = EncodeExtKey(key) + FormatKeyPath(m_path);
out = EncodeExtKey(key) + FormatHDKeypath(m_path);
if (IsRange()) {
out += "/*";
if (m_derive == DeriveType::HARDENED) out += '\'';
Expand Down
66 changes: 66 additions & 0 deletions src/util/bip32.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright (c) 2019 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <sstream>
#include <stdio.h>
#include <tinyformat.h>
#include <util/bip32.h>
#include <util/strencodings.h>


bool ParseHDKeypath(const std::string& keypath_str, std::vector<uint32_t>& keypath)
{
std::stringstream ss(keypath_str);
std::string item;
bool first = true;
while (std::getline(ss, item, '/')) {
if (item.compare("m") == 0) {
if (first) {
first = false;
continue;
}
return false;
}
// Finds whether it is hardened
uint32_t path = 0;
size_t pos = item.find("'");
if (pos != std::string::npos) {
// The hardened tick can only be in the last index of the string
if (pos != item.size() - 1) {
return false;
}
path |= 0x80000000;
item = item.substr(0, item.size() - 1); // Drop the last character which is the hardened tick
}

// Ensure this is only numbers
if (item.find_first_not_of( "0123456789" ) != std::string::npos) {
return false;
}
uint32_t number;
if (!ParseUInt32(item, &number)) {
return false;
}
path |= number;

keypath.push_back(path);
first = false;
}
return true;
}

std::string FormatHDKeypath(const std::vector<uint32_t>& path)
{
std::string ret;
for (auto i : path) {
ret += strprintf("/%i", (i << 1) >> 1);
if (i >> 31) ret += '\'';
}
return ret;
}

std::string WriteHDKeypath(const std::vector<uint32_t>& keypath)
{
return "m" + FormatHDKeypath(keypath);
}
41 changes: 0 additions & 41 deletions src/util/strencodings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -546,47 +546,6 @@ bool ParseFixedPoint(const std::string &val, int decimals, int64_t *amount_out)
return true;
}

bool ParseHDKeypath(const std::string& keypath_str, std::vector<uint32_t>& keypath)
{
std::stringstream ss(keypath_str);
std::string item;
bool first = true;
while (std::getline(ss, item, '/')) {
if (item.compare("m") == 0) {
if (first) {
first = false;
continue;
}
return false;
}
// Finds whether it is hardened
uint32_t path = 0;
size_t pos = item.find("'");
if (pos != std::string::npos) {
// The hardened tick can only be in the last index of the string
if (pos != item.size() - 1) {
return false;
}
path |= 0x80000000;
item = item.substr(0, item.size() - 1); // Drop the last character which is the hardened tick
}

// Ensure this is only numbers
if (item.find_first_not_of( "0123456789" ) != std::string::npos) {
return false;
}
uint32_t number;
if (!ParseUInt32(item, &number)) {
return false;
}
path |= number;

keypath.push_back(path);
first = false;
}
return true;
}

void Downcase(std::string& str)
{
std::transform(str.begin(), str.end(), str.begin(), [](char c){return ToLower(c);});
Expand Down
11 changes: 9 additions & 2 deletions src/wallet/rpcdump.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <script/script.h>
#include <script/standard.h>
#include <sync.h>
#include <util/bip32.h>
#include <util/system.h>
#include <util/time.h>
#include <validation.h>
Expand Down Expand Up @@ -850,7 +851,7 @@ UniValue dumpwallet(const JSONRPCRequest& request)
} else {
file << "change=1";
}
file << strprintf(" # addr=%s%s\n", strAddr, (pwallet->mapKeyMetadata[keyid].hdKeypath.size() > 0 ? " hdkeypath="+pwallet->mapKeyMetadata[keyid].hdKeypath : ""));
file << strprintf(" # addr=%s%s\n", strAddr, (pwallet->mapKeyMetadata[keyid].has_key_origin ? " hdkeypath="+WriteHDKeypath(pwallet->mapKeyMetadata[keyid].key_origin.path) : ""));
}
}
file << "\n";
Expand Down Expand Up @@ -887,6 +888,7 @@ struct ImportData
// Output data
std::set<CScript> import_scripts;
std::map<CKeyID, bool> used_keys; //!< Import these private keys if available (the value indicates whether if the key is required for solvability)
std::map<CKeyID, KeyOriginInfo> key_origins;
};

enum class ScriptContext
Expand Down Expand Up @@ -1157,7 +1159,7 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID
}

std::copy(out_keys.pubkeys.begin(), out_keys.pubkeys.end(), std::inserter(pubkey_map, pubkey_map.end()));

import_data.key_origins.insert(out_keys.origins.begin(), out_keys.origins.end());
for (size_t i = 0; i < priv_keys.size(); ++i) {
const auto& str = priv_keys[i].get_str();
CKey key = DecodeSecret(str);
Expand Down Expand Up @@ -1260,6 +1262,11 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
if (!pwallet->GetPubKey(id, temp) && !pwallet->AddWatchOnly(GetScriptForRawPubKey(pubkey), timestamp)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
}
const auto& key_orig_it = import_data.key_origins.find(id);
if (key_orig_it != import_data.key_origins.end()) {
pwallet->AddKeyOrigin(pubkey, key_orig_it->second);
}
pwallet->mapKeyMetadata[id].nCreateTime = timestamp;
Copy link
Member

Choose a reason for hiding this comment

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

Didn't the imported keys already have a metadata entry? If so, then shouldn't you only replace the timestamp if absent?

Copy link
Member Author

Choose a reason for hiding this comment

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

Only new things can be imported, so there shouldn't already be an existing metadata entry.

}

for (const CScript& script : script_pub_keys) {
Expand Down
11 changes: 5 additions & 6 deletions src/wallet/rpcwallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <script/sign.h>
#include <shutdown.h>
#include <timedata.h>
#include <util/bip32.h>
#include <util/system.h>
#include <util/moneystr.h>
#include <wallet/coincontrol.h>
Expand Down Expand Up @@ -2418,7 +2419,6 @@ static UniValue getwalletinfo(const JSONRPCRequest& request)
" \"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"
" \"hdseedid\": \"<hash160>\" (string, optional) the Hash160 of the HD seed (only present when HD is enabled)\n"
" \"hdmasterkeyid\": \"<hash160>\" (string, optional) alias for hdseedid retained for backwards-compatibility. Will be removed in V0.18.\n"
Copy link
Member

Choose a reason for hiding this comment

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

all instances(aside from notes) indeed appear to be deleted

" \"private_keys_enabled\": true|false (boolean) false if privatekeys are disabled for this wallet (enforced watch-only wallet)\n"
"}\n"
},
Expand Down Expand Up @@ -2456,7 +2456,6 @@ static UniValue getwalletinfo(const JSONRPCRequest& request)
obj.pushKV("paytxfee", ValueFromAmount(pwallet->m_pay_tx_fee.GetFeePerK()));
if (!seed_id.IsNull()) {
obj.pushKV("hdseedid", seed_id.GetHex());
obj.pushKV("hdmasterkeyid", seed_id.GetHex());
}
obj.pushKV("private_keys_enabled", !pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS));
return obj;
Expand Down Expand Up @@ -3660,7 +3659,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
" \"timestamp\" : timestamp, (number, optional) The creation time of the key if available in seconds since epoch (Jan 1 1970 GMT)\n"
" \"hdkeypath\" : \"keypath\" (string, optional) The HD keypath if the key is HD and available\n"
" \"hdseedid\" : \"<hash160>\" (string, optional) The Hash160 of the HD seed\n"
" \"hdmasterkeyid\" : \"<hash160>\" (string, optional) alias for hdseedid maintained for backwards compatibility. Will be removed in V0.18.\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

This PR removes some but not all of the hdmasterkeyid fields, suggest having a separate commit that removes all of them at once.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

" \"hdmasterfingerprint\" : \"<hash160>\" (string, optional) The fingperint of the master key.\n"
" \"labels\" (object) Array of labels associated with the address.\n"
" [\n"
" { (json object of label data)\n"
Expand Down Expand Up @@ -3723,10 +3722,10 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
}
if (meta) {
ret.pushKV("timestamp", meta->nCreateTime);
if (!meta->hdKeypath.empty()) {
ret.pushKV("hdkeypath", meta->hdKeypath);
if (meta->has_key_origin) {
ret.pushKV("hdkeypath", WriteHDKeypath(meta->key_origin.path));
ret.pushKV("hdseedid", meta->hd_seed_id.GetHex());
ret.pushKV("hdmasterkeyid", meta->hd_seed_id.GetHex());
ret.pushKV("hdmasterfingerprint", HexStr(meta->key_origin.fingerprint, meta->key_origin.fingerprint + 4));
Copy link
Member

Choose a reason for hiding this comment

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

Nit: key_origin should have a ToString() method (you're adding something like this in a later commit, though that's for the whole origin)

Copy link
Member Author

Choose a reason for hiding this comment

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

A key_origin ToString() method would print both the fingerprint and path. This only needs the fingerprint.

}
}

Expand Down
1 change: 1 addition & 0 deletions src/wallet/test/psbt_wallet_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include <key_io.h>
#include <script/sign.h>
#include <util/bip32.h>
#include <util/strencodings.h>
#include <wallet/psbtwallet.h>
#include <wallet/rpcwallet.h>
Expand Down
Loading