From 611ab307fbd8b6f8f7ffc1d569bb86d1f9cb4e92 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 19 Jul 2018 23:15:53 -0700 Subject: [PATCH 1/7] Introduce KeyOriginInfo for fingerprint + path --- src/rpc/rawtransaction.cpp | 15 ++++----------- src/script/sign.h | 32 ++++++++++++++++++++++---------- src/wallet/rpcwallet.cpp | 15 ++++++++------- 3 files changed, 34 insertions(+), 28 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 608a1b5da27..8d96add13b5 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1460,11 +1460,8 @@ UniValue decodepsbt(const JSONRPCRequest& request) UniValue keypath(UniValue::VOBJ); keypath.pushKV("pubkey", HexStr(entry.first)); - uint32_t fingerprint = entry.second.at(0); - keypath.pushKV("master_fingerprint", strprintf("%08x", bswap_32(fingerprint))); - - entry.second.erase(entry.second.begin()); - keypath.pushKV("path", WriteHDKeypath(entry.second)); + keypath.pushKV("master_fingerprint", strprintf("%08x", ReadBE32(entry.second.fingerprint))); + keypath.pushKV("path", WriteHDKeypath(entry.second.path)); keypaths.push_back(keypath); } in.pushKV("bip32_derivs", keypaths); @@ -1522,12 +1519,8 @@ UniValue decodepsbt(const JSONRPCRequest& request) for (auto entry : output.hd_keypaths) { UniValue keypath(UniValue::VOBJ); keypath.pushKV("pubkey", HexStr(entry.first)); - - uint32_t fingerprint = entry.second.at(0); - keypath.pushKV("master_fingerprint", strprintf("%08x", bswap_32(fingerprint))); - - entry.second.erase(entry.second.begin()); - keypath.pushKV("path", WriteHDKeypath(entry.second)); + keypath.pushKV("master_fingerprint", strprintf("%08x", ReadBE32(entry.second.fingerprint))); + keypath.pushKV("path", WriteHDKeypath(entry.second.path)); keypaths.push_back(keypath); } out.pushKV("bip32_derivs", keypaths); diff --git a/src/script/sign.h b/src/script/sign.h index d2033af7316..e0668f8c58e 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -20,6 +20,12 @@ class CTransaction; struct CMutableTransaction; +struct KeyOriginInfo +{ + unsigned char fingerprint[4]; + std::vector path; +}; + /** An interface to be implemented by keystores that support signing. */ class SigningProvider { @@ -155,7 +161,7 @@ void UnserializeFromVector(Stream& s, X&... args) // Deserialize HD keypaths into a map template -void DeserializeHDKeypaths(Stream& s, const std::vector& key, std::map>& hd_keypaths) +void DeserializeHDKeypaths(Stream& s, const std::vector& key, std::map& hd_keypaths) { // Make sure that the key is the size of pubkey + 1 if (key.size() != CPubKey::PUBLIC_KEY_SIZE + 1 && key.size() != CPubKey::COMPRESSED_PUBLIC_KEY_SIZE + 1) { @@ -172,25 +178,31 @@ void DeserializeHDKeypaths(Stream& s, const std::vector& key, std // Read in key path uint64_t value_len = ReadCompactSize(s); - std::vector keypath; - for (unsigned int i = 0; i < value_len; i += sizeof(uint32_t)) { + if (value_len % 4 || value_len == 0) { + throw std::ios_base::failure("Invalid length for HD key path"); + } + + KeyOriginInfo keypath; + s >> keypath.fingerprint; + for (unsigned int i = 4; i < value_len; i += sizeof(uint32_t)) { uint32_t index; s >> index; - keypath.push_back(index); + keypath.path.push_back(index); } // Add to map - hd_keypaths.emplace(pubkey, keypath); + hd_keypaths.emplace(pubkey, std::move(keypath)); } // Serialize HD keypaths to a stream from a map template -void SerializeHDKeypaths(Stream& s, const std::map>& hd_keypaths, uint8_t type) +void SerializeHDKeypaths(Stream& s, const std::map& hd_keypaths, uint8_t type) { for (auto keypath_pair : hd_keypaths) { SerializeToVector(s, type, MakeSpan(keypath_pair.first)); - WriteCompactSize(s, keypath_pair.second.size() * sizeof(uint32_t)); - for (auto& path : keypath_pair.second) { + WriteCompactSize(s, (keypath_pair.second.path.size() + 1) * sizeof(uint32_t)); + s << keypath_pair.second.fingerprint; + for (const auto& path : keypath_pair.second.path) { s << path; } } @@ -205,7 +217,7 @@ struct PSBTInput CScript witness_script; CScript final_script_sig; CScriptWitness final_script_witness; - std::map> hd_keypaths; + std::map hd_keypaths; std::map partial_sigs; std::map, std::vector> unknown; int sighash_type = 0; @@ -413,7 +425,7 @@ struct PSBTOutput { CScript redeem_script; CScript witness_script; - std::map> hd_keypaths; + std::map hd_keypaths; std::map, std::vector> unknown; bool IsNull() const; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 4aed097364e..fd3b82d9ab4 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4464,7 +4464,7 @@ bool ParseHDKeypath(std::string keypath_str, std::vector& keypath) return true; } -void AddKeypathToMap(const CWallet* pwallet, const CKeyID& keyID, std::map>& hd_keypaths) +void AddKeypathToMap(const CWallet* pwallet, const CKeyID& keyID, std::map& hd_keypaths) { CPubKey vchPubKey; if (!pwallet->GetPubKey(keyID, vchPubKey)) { @@ -4475,9 +4475,9 @@ void AddKeypathToMap(const CWallet* pwallet, const CKeyID& keyID, std::mapmapKeyMetadata.end()) { meta = it->second; } - std::vector keypath; + KeyOriginInfo info; if (!meta.hdKeypath.empty()) { - if (!ParseHDKeypath(meta.hdKeypath, keypath)) { + if (!ParseHDKeypath(meta.hdKeypath, info.path)) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Internal keypath is broken"); } // Get the proper master key id @@ -4485,12 +4485,13 @@ void AddKeypathToMap(const CWallet* pwallet, const CKeyID& keyID, std::mapGetKey(meta.hd_seed_id, key); CExtKey masterKey; masterKey.SetSeed(key.begin(), key.size()); - // Add to map - keypath.insert(keypath.begin(), ReadLE32(masterKey.key.GetPubKey().GetID().begin())); + // Compute identifier + CKeyID masterid = masterKey.key.GetPubKey().GetID(); + std::copy(masterid.begin(), masterid.begin() + 4, info.fingerprint); } else { // Single pubkeys get the master fingerprint of themselves - keypath.insert(keypath.begin(), ReadLE32(vchPubKey.GetID().begin())); + std::copy(keyID.begin(), keyID.begin() + 4, info.fingerprint); } - hd_keypaths.emplace(vchPubKey, keypath); + hd_keypaths.emplace(vchPubKey, std::move(info)); } bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, const CTransaction* txConst, int sighash_type, bool sign, bool bip32derivs) From 84f1f1bfdf900cd28099e428441aa42f9d11a0ed Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 19 Jul 2018 17:25:21 -0700 Subject: [PATCH 2/7] Make SigningProvider expose key origin information --- src/script/sign.cpp | 9 +++++++-- src/script/sign.h | 2 ++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 1982e8a832d..47931e21d92 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -645,9 +645,14 @@ bool PublicOnlySigningProvider::GetCScript(const CScriptID &scriptid, CScript& s return m_provider->GetCScript(scriptid, script); } -bool PublicOnlySigningProvider::GetPubKey(const CKeyID &address, CPubKey& pubkey) const +bool PublicOnlySigningProvider::GetPubKey(const CKeyID& keyid, CPubKey& pubkey) const { - return m_provider->GetPubKey(address, pubkey); + return m_provider->GetPubKey(keyid, pubkey); +} + +bool PublicOnlySigningProvider::GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const +{ + return m_provider->GetKeyOrigin(keyid, info); } bool FlatSigningProvider::GetCScript(const CScriptID& scriptid, CScript& script) const { return LookupHelper(scripts, scriptid, script); } diff --git a/src/script/sign.h b/src/script/sign.h index e0668f8c58e..323fe70f347 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -34,6 +34,7 @@ public: virtual bool GetCScript(const CScriptID &scriptid, CScript& script) const { return false; } virtual bool GetPubKey(const CKeyID &address, CPubKey& pubkey) const { return false; } virtual bool GetKey(const CKeyID &address, CKey& key) const { return false; } + virtual bool GetKeyOrigin(const CKeyID& id, KeyOriginInfo& info) const { return false; } }; extern const SigningProvider& DUMMY_SIGNING_PROVIDER; @@ -47,6 +48,7 @@ public: PublicOnlySigningProvider(const SigningProvider* provider) : m_provider(provider) {} bool GetCScript(const CScriptID &scriptid, CScript& script) const; bool GetPubKey(const CKeyID &address, CPubKey& pubkey) const; + bool GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const; }; struct FlatSigningProvider final : public SigningProvider From 81e1dd5ce1a32114a38691ec6b55e72ab04dbbb1 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 20 Jul 2018 00:04:02 -0700 Subject: [PATCH 3/7] Generalize PublicOnlySigningProvider into HidingSigningProvider --- src/script/sign.cpp | 13 ++++++++++--- src/script/sign.h | 13 ++++++++----- src/wallet/rpcwallet.cpp | 6 +----- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 47931e21d92..ae29f72b054 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -640,18 +640,25 @@ void PSBTOutput::Merge(const PSBTOutput& output) if (witness_script.empty() && !output.witness_script.empty()) witness_script = output.witness_script; } -bool PublicOnlySigningProvider::GetCScript(const CScriptID &scriptid, CScript& script) const +bool HidingSigningProvider::GetCScript(const CScriptID& scriptid, CScript& script) const { return m_provider->GetCScript(scriptid, script); } -bool PublicOnlySigningProvider::GetPubKey(const CKeyID& keyid, CPubKey& pubkey) const +bool HidingSigningProvider::GetPubKey(const CKeyID& keyid, CPubKey& pubkey) const { return m_provider->GetPubKey(keyid, pubkey); } -bool PublicOnlySigningProvider::GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const +bool HidingSigningProvider::GetKey(const CKeyID& keyid, CKey& key) const { + if (m_hide_secret) return false; + return m_provider->GetKey(keyid, key); +} + +bool HidingSigningProvider::GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const +{ + if (m_hide_origin) return false; return m_provider->GetKeyOrigin(keyid, info); } diff --git a/src/script/sign.h b/src/script/sign.h index 323fe70f347..d8334f2ea27 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -39,16 +39,19 @@ public: extern const SigningProvider& DUMMY_SIGNING_PROVIDER; -class PublicOnlySigningProvider : public SigningProvider +class HidingSigningProvider : public SigningProvider { private: + const bool m_hide_secret; + const bool m_hide_origin; const SigningProvider* m_provider; public: - PublicOnlySigningProvider(const SigningProvider* provider) : m_provider(provider) {} - bool GetCScript(const CScriptID &scriptid, CScript& script) const; - bool GetPubKey(const CKeyID &address, CPubKey& pubkey) const; - bool GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const; + HidingSigningProvider(const SigningProvider* provider, bool hide_secret, bool hide_origin) : m_hide_secret(hide_secret), m_hide_origin(hide_origin), m_provider(provider) {} + bool GetCScript(const CScriptID& scriptid, CScript& script) const override; + bool GetPubKey(const CKeyID& keyid, CPubKey& pubkey) const override; + bool GetKey(const CKeyID& keyid, CKey& key) const override; + bool GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const override; }; struct FlatSigningProvider final : public SigningProvider diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index fd3b82d9ab4..a719e883c76 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4520,11 +4520,7 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, const C } SignatureData sigdata; - if (sign) { - complete &= SignPSBTInput(*pwallet, *psbtx.tx, input, sigdata, i, sighash_type); - } else { - complete &= SignPSBTInput(PublicOnlySigningProvider(pwallet), *psbtx.tx, input, sigdata, i, sighash_type); - } + complete &= SignPSBTInput(HidingSigningProvider(pwallet, !sign, false), *psbtx.tx, input, sigdata, i, sighash_type); if (it != pwallet->mapWallet.end()) { // Drop the unnecessary UTXO if we added both from the wallet. From 3b01efa0d1bf3d23d1b7b7e518849f1fc26314f9 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 19 Jul 2018 17:37:19 -0700 Subject: [PATCH 4/7] [MOVEONLY] Move ParseHDKeypath to utilstrencodings --- src/utilstrencodings.cpp | 40 ++++++++++++++++++++++++++ src/utilstrencodings.h | 3 ++ src/wallet/rpcwallet.cpp | 41 --------------------------- src/wallet/test/psbt_wallet_tests.cpp | 2 -- 4 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/utilstrencodings.cpp b/src/utilstrencodings.cpp index a06d88cb199..e66e2c238c5 100644 --- a/src/utilstrencodings.cpp +++ b/src/utilstrencodings.cpp @@ -544,3 +544,43 @@ bool ParseFixedPoint(const std::string &val, int decimals, int64_t *amount_out) return true; } +bool ParseHDKeypath(const std::string& keypath_str, std::vector& 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; +} diff --git a/src/utilstrencodings.h b/src/utilstrencodings.h index 5f2211b5dc9..8f29f8f322c 100644 --- a/src/utilstrencodings.h +++ b/src/utilstrencodings.h @@ -183,4 +183,7 @@ bool ConvertBits(const O& outfn, I it, I end) { return true; } +/** Parse an HD keypaths like "m/7/0'/2000". */ +bool ParseHDKeypath(const std::string& keypath_str, std::vector& keypath); + #endif // BITCOIN_UTILSTRENCODINGS_H diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index a719e883c76..aa7f5312b41 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4423,47 +4423,6 @@ UniValue sethdseed(const JSONRPCRequest& request) return NullUniValue; } -bool ParseHDKeypath(std::string keypath_str, std::vector& 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 AddKeypathToMap(const CWallet* pwallet, const CKeyID& keyID, std::map& hd_keypaths) { CPubKey vchPubKey; diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index 61c3fa94a6a..526f2d983fc 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -13,8 +13,6 @@ #include #include -extern bool ParseHDKeypath(std::string keypath_str, std::vector& keypath); - BOOST_FIXTURE_TEST_SUITE(psbt_wallet_tests, WalletTestingSetup) BOOST_AUTO_TEST_CASE(psbt_updater_test) From 03a99586a398ee38f40c3b72d24c6a2ba4b88579 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 19 Jul 2018 17:56:52 -0700 Subject: [PATCH 5/7] Implement key origin lookup in CWallet --- src/wallet/rpcwallet.cpp | 21 ++------------------- src/wallet/wallet.cpp | 26 ++++++++++++++++++++++++++ src/wallet/wallet.h | 2 ++ 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index aa7f5312b41..0fcdd780a7d 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4429,26 +4429,9 @@ void AddKeypathToMap(const CWallet* pwallet, const CKeyID& keyID, std::mapGetPubKey(keyID, vchPubKey)) { return; } - CKeyMetadata meta; - auto it = pwallet->mapKeyMetadata.find(keyID); - if (it != pwallet->mapKeyMetadata.end()) { - meta = it->second; - } KeyOriginInfo info; - if (!meta.hdKeypath.empty()) { - if (!ParseHDKeypath(meta.hdKeypath, info.path)) { - throw JSONRPCError(RPC_INTERNAL_ERROR, "Internal keypath is broken"); - } - // Get the proper master key id - CKey key; - pwallet->GetKey(meta.hd_seed_id, key); - CExtKey masterKey; - masterKey.SetSeed(key.begin(), key.size()); - // Compute identifier - CKeyID masterid = masterKey.key.GetPubKey().GetID(); - std::copy(masterid.begin(), masterid.begin() + 4, info.fingerprint); - } else { // Single pubkeys get the master fingerprint of themselves - std::copy(keyID.begin(), keyID.begin() + 4, info.fingerprint); + if (!pwallet->GetKeyOrigin(keyID, info)) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "Internal keypath is broken"); } hd_keypaths.emplace(vchPubKey, std::move(info)); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 5a7fdf9a855..92ffc3c1084 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4466,3 +4466,29 @@ std::vector CWallet::GroupOutputs(const std::vector& outpu } return groups; } + +bool CWallet::GetKeyOrigin(const CKeyID& keyID, KeyOriginInfo& info) const +{ + CKeyMetadata meta; + { + LOCK(cs_wallet); + auto it = mapKeyMetadata.find(keyID); + if (it != mapKeyMetadata.end()) { + meta = it->second; + } + } + if (!meta.hdKeypath.empty()) { + if (!ParseHDKeypath(meta.hdKeypath, info.path)) return false; + // Get the proper master key id + CKey key; + GetKey(meta.hd_seed_id, key); + CExtKey masterKey; + masterKey.SetSeed(key.begin(), key.size()); + // Compute identifier + CKeyID masterid = masterKey.key.GetPubKey().GetID(); + std::copy(masterid.begin(), masterid.begin() + 4, info.fingerprint); + } else { // Single pubkeys get the master fingerprint of themselves + std::copy(keyID.begin(), keyID.begin() + 4, info.fingerprint); + } + return true; +} diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 57b22c0e49f..e9f6a452436 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1211,6 +1211,8 @@ public: LogPrintf(("%s " + fmt).c_str(), GetDisplayName(), parameters...); }; + /** Implement lookup of key origin information through wallet key metadata. */ + bool GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const override; }; /** A key allocated from the key pool. */ From cad5dd2368109ec398a3b79c8b9e94dfd23f0845 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 19 Jul 2018 18:47:24 -0700 Subject: [PATCH 6/7] Pass HD path data through SignatureData --- src/script/sign.cpp | 24 +++++++++++++++++------- src/script/sign.h | 2 +- src/wallet/rpcwallet.cpp | 18 ++---------------- 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/script/sign.cpp b/src/script/sign.cpp index ae29f72b054..4060111ff22 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -50,10 +50,6 @@ static bool GetCScript(const SigningProvider& provider, const SignatureData& sig static bool GetPubKey(const SigningProvider& provider, SignatureData& sigdata, const CKeyID& address, CPubKey& pubkey) { - if (provider.GetPubKey(address, pubkey)) { - sigdata.misc_pubkeys.emplace(pubkey.GetID(), pubkey); - return true; - } // Look for pubkey in all partial sigs const auto it = sigdata.signatures.find(address); if (it != sigdata.signatures.end()) { @@ -63,7 +59,15 @@ static bool GetPubKey(const SigningProvider& provider, SignatureData& sigdata, c // Look for pubkey in pubkey list const auto& pk_it = sigdata.misc_pubkeys.find(address); if (pk_it != sigdata.misc_pubkeys.end()) { - pubkey = pk_it->second; + pubkey = pk_it->second.first; + return true; + } + // Query the underlying provider + if (provider.GetPubKey(address, pubkey)) { + KeyOriginInfo info; + if (provider.GetKeyOrigin(address, info)) { + sigdata.misc_pubkeys.emplace(address, std::make_pair(pubkey, std::move(info))); + } return true; } return false; @@ -543,7 +547,7 @@ void PSBTInput::FillSignatureData(SignatureData& sigdata) const sigdata.witness_script = witness_script; } for (const auto& key_pair : hd_keypaths) { - sigdata.misc_pubkeys.emplace(key_pair.first.GetID(), key_pair.first); + sigdata.misc_pubkeys.emplace(key_pair.first.GetID(), key_pair); } } @@ -571,6 +575,9 @@ void PSBTInput::FromSignatureData(const SignatureData& sigdata) if (witness_script.empty() && !sigdata.witness_script.empty()) { witness_script = sigdata.witness_script; } + for (const auto& entry : sigdata.misc_pubkeys) { + hd_keypaths.emplace(entry.second); + } } void PSBTInput::Merge(const PSBTInput& input) @@ -612,7 +619,7 @@ void PSBTOutput::FillSignatureData(SignatureData& sigdata) const sigdata.witness_script = witness_script; } for (const auto& key_pair : hd_keypaths) { - sigdata.misc_pubkeys.emplace(key_pair.first.GetID(), key_pair.first); + sigdata.misc_pubkeys.emplace(key_pair.first.GetID(), key_pair); } } @@ -624,6 +631,9 @@ void PSBTOutput::FromSignatureData(const SignatureData& sigdata) if (witness_script.empty() && !sigdata.witness_script.empty()) { witness_script = sigdata.witness_script; } + for (const auto& entry : sigdata.misc_pubkeys) { + hd_keypaths.emplace(entry.second); + } } bool PSBTOutput::IsNull() const diff --git a/src/script/sign.h b/src/script/sign.h index d8334f2ea27..a4e1eac403e 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -109,7 +109,7 @@ struct SignatureData { CScript witness_script; ///< The witnessScript (if any) for the input. witnessScripts are used in P2WSH outputs. CScriptWitness scriptWitness; ///< The scriptWitness of an input. Contains complete signatures or the traditional partial signatures format. scriptWitness is part of a transaction input per BIP 144. std::map signatures; ///< BIP 174 style partial signatures for the input. May contain all signatures necessary for producing a final scriptSig or scriptWitness. - std::map misc_pubkeys; + std::map> misc_pubkeys; SignatureData() {} explicit SignatureData(const CScript& script) : scriptSig(script) {} diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 0fcdd780a7d..c2cd4ea2a04 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4462,7 +4462,7 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, const C } SignatureData sigdata; - complete &= SignPSBTInput(HidingSigningProvider(pwallet, !sign, false), *psbtx.tx, input, sigdata, i, sighash_type); + complete &= SignPSBTInput(HidingSigningProvider(pwallet, !sign, !bip32derivs), *psbtx.tx, input, sigdata, i, sighash_type); if (it != pwallet->mapWallet.end()) { // Drop the unnecessary UTXO if we added both from the wallet. @@ -4472,13 +4472,6 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, const C input.witness_utxo.SetNull(); } } - - // Get public key paths - if (bip32derivs) { - for (const auto& pubkey_it : sigdata.misc_pubkeys) { - AddKeypathToMap(pwallet, pubkey_it.first, input.hd_keypaths); - } - } } // Fill in the bip32 keypaths and redeemscripts for the outputs so that hardware wallets can identify change @@ -4496,15 +4489,8 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, const C psbt_out.FillSignatureData(sigdata); MutableTransactionSignatureCreator creator(psbtx.tx.get_ptr(), 0, out.nValue, 1); - ProduceSignature(*pwallet, creator, out.scriptPubKey, sigdata); + ProduceSignature(HidingSigningProvider(pwallet, true, !bip32derivs), creator, out.scriptPubKey, sigdata); psbt_out.FromSignatureData(sigdata); - - // Get public key paths - if (bip32derivs) { - for (const auto& pubkey_it : sigdata.misc_pubkeys) { - AddKeypathToMap(pwallet, pubkey_it.first, psbt_out.hd_keypaths); - } - } } return complete; } From 917353c8b0eff4cd95f9a5f7719f6756bb8338b1 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 20 Jul 2018 00:23:32 -0700 Subject: [PATCH 7/7] Make SignPSBTInput operate on a private SignatureData object --- src/rpc/rawtransaction.cpp | 3 +-- src/script/sign.cpp | 13 ++++++++++++- src/script/sign.h | 2 +- src/wallet/rpcwallet.cpp | 12 +----------- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 8d96add13b5..4f9b399a913 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1641,8 +1641,7 @@ UniValue finalizepsbt(const JSONRPCRequest& request) for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { PSBTInput& input = psbtx.inputs.at(i); - SignatureData sigdata; - complete &= SignPSBTInput(DUMMY_SIGNING_PROVIDER, *psbtx.tx, input, sigdata, i, 1); + complete &= SignPSBTInput(DUMMY_SIGNING_PROVIDER, *psbtx.tx, input, i, 1); } UniValue result(UniValue::VOBJ); diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 4060111ff22..ea14abce4ca 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -237,7 +237,7 @@ bool ProduceSignature(const SigningProvider& provider, const BaseSignatureCreato return sigdata.complete; } -bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& tx, PSBTInput& input, SignatureData& sigdata, int index, int sighash) +bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& tx, PSBTInput& input, int index, int sighash) { // if this input has a final scriptsig or scriptwitness, don't do anything with it if (!input.final_script_sig.empty() || !input.final_script_witness.IsNull()) { @@ -245,6 +245,7 @@ bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& t } // Fill SignatureData with input info + SignatureData sigdata; input.FillSignatureData(sigdata); // Get UTXO @@ -276,6 +277,16 @@ bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& t // Verify that a witness signature was produced in case one was required. if (require_witness_sig && !sigdata.witness) return false; input.FromSignatureData(sigdata); + + // If both UTXO types are present, drop the unnecessary one. + if (input.non_witness_utxo && !input.witness_utxo.IsNull()) { + if (sigdata.witness) { + input.non_witness_utxo = nullptr; + } else { + input.witness_utxo.SetNull(); + } + } + return sig_complete; } diff --git a/src/script/sign.h b/src/script/sign.h index a4e1eac403e..160b3be75b7 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -696,7 +696,7 @@ bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, C bool SignSignature(const SigningProvider &provider, const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType); /** Signs a PSBTInput, verifying that all provided data matches what is being signed. */ -bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& tx, PSBTInput& input, SignatureData& sigdata, int index, int sighash = 1); +bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& tx, PSBTInput& input, int index, int sighash = SIGHASH_ALL); /** Extract signature data from a transaction input, and insert it. */ SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn, const CTxOut& txout); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index c2cd4ea2a04..b8e7b7fe0cb 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4461,17 +4461,7 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, const C throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Specified Sighash and sighash in PSBT do not match."); } - SignatureData sigdata; - complete &= SignPSBTInput(HidingSigningProvider(pwallet, !sign, !bip32derivs), *psbtx.tx, input, sigdata, i, sighash_type); - - if (it != pwallet->mapWallet.end()) { - // Drop the unnecessary UTXO if we added both from the wallet. - if (sigdata.witness) { - input.non_witness_utxo = nullptr; - } else { - input.witness_utxo.SetNull(); - } - } + complete &= SignPSBTInput(HidingSigningProvider(pwallet, !sign, !bip32derivs), *psbtx.tx, input, i, sighash_type); } // Fill in the bip32 keypaths and redeemscripts for the outputs so that hardware wallets can identify change