Skip to content

Commit aa39ca7

Browse files
laanwjlaanwj
committed
Merge #13723: PSBT key path cleanups
917353c Make SignPSBTInput operate on a private SignatureData object (Pieter Wuille) cad5dd2 Pass HD path data through SignatureData (Pieter Wuille) 03a9958 Implement key origin lookup in CWallet (Pieter Wuille) 3b01efa [MOVEONLY] Move ParseHDKeypath to utilstrencodings (Pieter Wuille) 81e1dd5 Generalize PublicOnlySigningProvider into HidingSigningProvider (Pieter Wuille) 84f1f1b Make SigningProvider expose key origin information (Pieter Wuille) 611ab30 Introduce KeyOriginInfo for fingerprint + path (Pieter Wuille) Pull request description: This PR adds "key origin" (master fingeprint + key path) information to what is exposed from `SigningProvider`s, allowing this information to be used by the generic PSBT code instead of having the RPC pull it directly from the wallet. This is also a preparation to having PSBT interact with output descriptors, which can then directly expose key origin information for the scripts they generate. Tree-SHA512: c718382ba8ba2d6fc9a32c062bd4cff08b6f39b133838aa03115c39aeca0f654c7cc3ec72d87005bf8306e550824cd8eb9d60f0bd41784a3e22e17b2afcfe833
2 parents ee9e6e7 + 917353c commit aa39ca7

File tree

9 files changed

+160
-134
lines changed

9 files changed

+160
-134
lines changed

‎src/rpc/rawtransaction.cpp‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1458,11 +1458,8 @@ UniValue decodepsbt(const JSONRPCRequest& request)
14581458
UniValue keypath(UniValue::VOBJ);
14591459
keypath.pushKV("pubkey", HexStr(entry.first));
14601460

1461-
uint32_t fingerprint = entry.second.at(0);
1462-
keypath.pushKV("master_fingerprint", strprintf("%08x", bswap_32(fingerprint)));
1463-
1464-
entry.second.erase(entry.second.begin());
1465-
keypath.pushKV("path", WriteHDKeypath(entry.second));
1461+
keypath.pushKV("master_fingerprint", strprintf("%08x", ReadBE32(entry.second.fingerprint)));
1462+
keypath.pushKV("path", WriteHDKeypath(entry.second.path));
14661463
keypaths.push_back(keypath);
14671464
}
14681465
in.pushKV("bip32_derivs", keypaths);
@@ -1520,12 +1517,8 @@ UniValue decodepsbt(const JSONRPCRequest& request)
15201517
for (auto entry : output.hd_keypaths) {
15211518
UniValue keypath(UniValue::VOBJ);
15221519
keypath.pushKV("pubkey", HexStr(entry.first));
1523-
1524-
uint32_t fingerprint = entry.second.at(0);
1525-
keypath.pushKV("master_fingerprint", strprintf("%08x", bswap_32(fingerprint)));
1526-
1527-
entry.second.erase(entry.second.begin());
1528-
keypath.pushKV("path", WriteHDKeypath(entry.second));
1520+
keypath.pushKV("master_fingerprint", strprintf("%08x", ReadBE32(entry.second.fingerprint)));
1521+
keypath.pushKV("path", WriteHDKeypath(entry.second.path));
15291522
keypaths.push_back(keypath);
15301523
}
15311524
out.pushKV("bip32_derivs", keypaths);
@@ -1646,8 +1639,7 @@ UniValue finalizepsbt(const JSONRPCRequest& request)
16461639
for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) {
16471640
PSBTInput& input = psbtx.inputs.at(i);
16481641

1649-
SignatureData sigdata;
1650-
complete &= SignPSBTInput(DUMMY_SIGNING_PROVIDER, *psbtx.tx, input, sigdata, i, 1);
1642+
complete &= SignPSBTInput(DUMMY_SIGNING_PROVIDER, *psbtx.tx, input, i, 1);
16511643
}
16521644

16531645
UniValue result(UniValue::VOBJ);

‎src/script/sign.cpp‎

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,6 @@ static bool GetCScript(const SigningProvider& provider, const SignatureData& sig
5050

5151
static bool GetPubKey(const SigningProvider& provider, SignatureData& sigdata, const CKeyID& address, CPubKey& pubkey)
5252
{
53-
if (provider.GetPubKey(address, pubkey)) {
54-
sigdata.misc_pubkeys.emplace(pubkey.GetID(), pubkey);
55-
return true;
56-
}
5753
// Look for pubkey in all partial sigs
5854
const auto it = sigdata.signatures.find(address);
5955
if (it != sigdata.signatures.end()) {
@@ -63,7 +59,15 @@ static bool GetPubKey(const SigningProvider& provider, SignatureData& sigdata, c
6359
// Look for pubkey in pubkey list
6460
const auto& pk_it = sigdata.misc_pubkeys.find(address);
6561
if (pk_it != sigdata.misc_pubkeys.end()) {
66-
pubkey = pk_it->second;
62+
pubkey = pk_it->second.first;
63+
return true;
64+
}
65+
// Query the underlying provider
66+
if (provider.GetPubKey(address, pubkey)) {
67+
KeyOriginInfo info;
68+
if (provider.GetKeyOrigin(address, info)) {
69+
sigdata.misc_pubkeys.emplace(address, std::make_pair(pubkey, std::move(info)));
70+
}
6771
return true;
6872
}
6973
return false;
@@ -232,14 +236,15 @@ bool ProduceSignature(const SigningProvider& provider, const BaseSignatureCreato
232236
return sigdata.complete;
233237
}
234238

235-
bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& tx, PSBTInput& input, SignatureData& sigdata, int index, int sighash)
239+
bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& tx, PSBTInput& input, int index, int sighash)
236240
{
237241
// if this input has a final scriptsig or scriptwitness, don't do anything with it
238242
if (!input.final_script_sig.empty() || !input.final_script_witness.IsNull()) {
239243
return true;
240244
}
241245

242246
// Fill SignatureData with input info
247+
SignatureData sigdata;
243248
input.FillSignatureData(sigdata);
244249

245250
// Get UTXO
@@ -271,6 +276,16 @@ bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& t
271276
// Verify that a witness signature was produced in case one was required.
272277
if (require_witness_sig && !sigdata.witness) return false;
273278
input.FromSignatureData(sigdata);
279+
280+
// If both UTXO types are present, drop the unnecessary one.
281+
if (input.non_witness_utxo && !input.witness_utxo.IsNull()) {
282+
if (sigdata.witness) {
283+
input.non_witness_utxo = nullptr;
284+
} else {
285+
input.witness_utxo.SetNull();
286+
}
287+
}
288+
274289
return sig_complete;
275290
}
276291

@@ -541,7 +556,7 @@ void PSBTInput::FillSignatureData(SignatureData& sigdata) const
541556
sigdata.witness_script = witness_script;
542557
}
543558
for (const auto& key_pair : hd_keypaths) {
544-
sigdata.misc_pubkeys.emplace(key_pair.first.GetID(), key_pair.first);
559+
sigdata.misc_pubkeys.emplace(key_pair.first.GetID(), key_pair);
545560
}
546561
}
547562

@@ -569,6 +584,9 @@ void PSBTInput::FromSignatureData(const SignatureData& sigdata)
569584
if (witness_script.empty() && !sigdata.witness_script.empty()) {
570585
witness_script = sigdata.witness_script;
571586
}
587+
for (const auto& entry : sigdata.misc_pubkeys) {
588+
hd_keypaths.emplace(entry.second);
589+
}
572590
}
573591

574592
void PSBTInput::Merge(const PSBTInput& input)
@@ -610,7 +628,7 @@ void PSBTOutput::FillSignatureData(SignatureData& sigdata) const
610628
sigdata.witness_script = witness_script;
611629
}
612630
for (const auto& key_pair : hd_keypaths) {
613-
sigdata.misc_pubkeys.emplace(key_pair.first.GetID(), key_pair.first);
631+
sigdata.misc_pubkeys.emplace(key_pair.first.GetID(), key_pair);
614632
}
615633
}
616634

@@ -622,6 +640,9 @@ void PSBTOutput::FromSignatureData(const SignatureData& sigdata)
622640
if (witness_script.empty() && !sigdata.witness_script.empty()) {
623641
witness_script = sigdata.witness_script;
624642
}
643+
for (const auto& entry : sigdata.misc_pubkeys) {
644+
hd_keypaths.emplace(entry.second);
645+
}
625646
}
626647

627648
bool PSBTOutput::IsNull() const
@@ -638,14 +659,26 @@ void PSBTOutput::Merge(const PSBTOutput& output)
638659
if (witness_script.empty() && !output.witness_script.empty()) witness_script = output.witness_script;
639660
}
640661

641-
bool PublicOnlySigningProvider::GetCScript(const CScriptID &scriptid, CScript& script) const
662+
bool HidingSigningProvider::GetCScript(const CScriptID& scriptid, CScript& script) const
642663
{
643664
return m_provider->GetCScript(scriptid, script);
644665
}
645666

646-
bool PublicOnlySigningProvider::GetPubKey(const CKeyID &address, CPubKey& pubkey) const
667+
bool HidingSigningProvider::GetPubKey(const CKeyID& keyid, CPubKey& pubkey) const
668+
{
669+
return m_provider->GetPubKey(keyid, pubkey);
670+
}
671+
672+
bool HidingSigningProvider::GetKey(const CKeyID& keyid, CKey& key) const
673+
{
674+
if (m_hide_secret) return false;
675+
return m_provider->GetKey(keyid, key);
676+
}
677+
678+
bool HidingSigningProvider::GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const
647679
{
648-
return m_provider->GetPubKey(address, pubkey);
680+
if (m_hide_origin) return false;
681+
return m_provider->GetKeyOrigin(keyid, info);
649682
}
650683

651684
bool FlatSigningProvider::GetCScript(const CScriptID& scriptid, CScript& script) const { return LookupHelper(scripts, scriptid, script); }

‎src/script/sign.h‎

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@ class CTransaction;
2020

2121
struct CMutableTransaction;
2222

23+
struct KeyOriginInfo
24+
{
25+
unsigned char fingerprint[4];
26+
std::vector<uint32_t> path;
27+
};
28+
2329
/** An interface to be implemented by keystores that support signing. */
2430
class SigningProvider
2531
{
@@ -28,19 +34,24 @@ class SigningProvider
2834
virtual bool GetCScript(const CScriptID &scriptid, CScript& script) const { return false; }
2935
virtual bool GetPubKey(const CKeyID &address, CPubKey& pubkey) const { return false; }
3036
virtual bool GetKey(const CKeyID &address, CKey& key) const { return false; }
37+
virtual bool GetKeyOrigin(const CKeyID& id, KeyOriginInfo& info) const { return false; }
3138
};
3239

3340
extern const SigningProvider& DUMMY_SIGNING_PROVIDER;
3441

35-
class PublicOnlySigningProvider : public SigningProvider
42+
class HidingSigningProvider : public SigningProvider
3643
{
3744
private:
45+
const bool m_hide_secret;
46+
const bool m_hide_origin;
3847
const SigningProvider* m_provider;
3948

4049
public:
41-
PublicOnlySigningProvider(const SigningProvider* provider) : m_provider(provider) {}
42-
bool GetCScript(const CScriptID &scriptid, CScript& script) const;
43-
bool GetPubKey(const CKeyID &address, CPubKey& pubkey) const;
50+
HidingSigningProvider(const SigningProvider* provider, bool hide_secret, bool hide_origin) : m_hide_secret(hide_secret), m_hide_origin(hide_origin), m_provider(provider) {}
51+
bool GetCScript(const CScriptID& scriptid, CScript& script) const override;
52+
bool GetPubKey(const CKeyID& keyid, CPubKey& pubkey) const override;
53+
bool GetKey(const CKeyID& keyid, CKey& key) const override;
54+
bool GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const override;
4455
};
4556

4657
struct FlatSigningProvider final : public SigningProvider
@@ -98,7 +109,7 @@ struct SignatureData {
98109
CScript witness_script; ///< The witnessScript (if any) for the input. witnessScripts are used in P2WSH outputs.
99110
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.
100111
std::map<CKeyID, SigPair> signatures; ///< BIP 174 style partial signatures for the input. May contain all signatures necessary for producing a final scriptSig or scriptWitness.
101-
std::map<CKeyID, CPubKey> misc_pubkeys;
112+
std::map<CKeyID, std::pair<CPubKey, KeyOriginInfo>> misc_pubkeys;
102113

103114
SignatureData() {}
104115
explicit SignatureData(const CScript& script) : scriptSig(script) {}
@@ -155,7 +166,7 @@ void UnserializeFromVector(Stream& s, X&... args)
155166

156167
// Deserialize HD keypaths into a map
157168
template<typename Stream>
158-
void DeserializeHDKeypaths(Stream& s, const std::vector<unsigned char>& key, std::map<CPubKey, std::vector<uint32_t>>& hd_keypaths)
169+
void DeserializeHDKeypaths(Stream& s, const std::vector<unsigned char>& key, std::map<CPubKey, KeyOriginInfo>& hd_keypaths)
159170
{
160171
// Make sure that the key is the size of pubkey + 1
161172
if (key.size() != CPubKey::PUBLIC_KEY_SIZE + 1 && key.size() != CPubKey::COMPRESSED_PUBLIC_KEY_SIZE + 1) {
@@ -172,25 +183,31 @@ void DeserializeHDKeypaths(Stream& s, const std::vector<unsigned char>& key, std
172183

173184
// Read in key path
174185
uint64_t value_len = ReadCompactSize(s);
175-
std::vector<uint32_t> keypath;
176-
for (unsigned int i = 0; i < value_len; i += sizeof(uint32_t)) {
186+
if (value_len % 4 || value_len == 0) {
187+
throw std::ios_base::failure("Invalid length for HD key path");
188+
}
189+
190+
KeyOriginInfo keypath;
191+
s >> keypath.fingerprint;
192+
for (unsigned int i = 4; i < value_len; i += sizeof(uint32_t)) {
177193
uint32_t index;
178194
s >> index;
179-
keypath.push_back(index);
195+
keypath.path.push_back(index);
180196
}
181197

182198
// Add to map
183-
hd_keypaths.emplace(pubkey, keypath);
199+
hd_keypaths.emplace(pubkey, std::move(keypath));
184200
}
185201

186202
// Serialize HD keypaths to a stream from a map
187203
template<typename Stream>
188-
void SerializeHDKeypaths(Stream& s, const std::map<CPubKey, std::vector<uint32_t>>& hd_keypaths, uint8_t type)
204+
void SerializeHDKeypaths(Stream& s, const std::map<CPubKey, KeyOriginInfo>& hd_keypaths, uint8_t type)
189205
{
190206
for (auto keypath_pair : hd_keypaths) {
191207
SerializeToVector(s, type, MakeSpan(keypath_pair.first));
192-
WriteCompactSize(s, keypath_pair.second.size() * sizeof(uint32_t));
193-
for (auto& path : keypath_pair.second) {
208+
WriteCompactSize(s, (keypath_pair.second.path.size() + 1) * sizeof(uint32_t));
209+
s << keypath_pair.second.fingerprint;
210+
for (const auto& path : keypath_pair.second.path) {
194211
s << path;
195212
}
196213
}
@@ -205,7 +222,7 @@ struct PSBTInput
205222
CScript witness_script;
206223
CScript final_script_sig;
207224
CScriptWitness final_script_witness;
208-
std::map<CPubKey, std::vector<uint32_t>> hd_keypaths;
225+
std::map<CPubKey, KeyOriginInfo> hd_keypaths;
209226
std::map<CKeyID, SigPair> partial_sigs;
210227
std::map<std::vector<unsigned char>, std::vector<unsigned char>> unknown;
211228
int sighash_type = 0;
@@ -418,7 +435,7 @@ struct PSBTOutput
418435
{
419436
CScript redeem_script;
420437
CScript witness_script;
421-
std::map<CPubKey, std::vector<uint32_t>> hd_keypaths;
438+
std::map<CPubKey, KeyOriginInfo> hd_keypaths;
422439
std::map<std::vector<unsigned char>, std::vector<unsigned char>> unknown;
423440

424441
bool IsNull() const;
@@ -687,7 +704,7 @@ bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, C
687704
bool SignSignature(const SigningProvider &provider, const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType);
688705

689706
/** Signs a PSBTInput, verifying that all provided data matches what is being signed. */
690-
bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& tx, PSBTInput& input, SignatureData& sigdata, int index, int sighash = 1);
707+
bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& tx, PSBTInput& input, int index, int sighash = SIGHASH_ALL);
691708

692709
/** Extract signature data from a transaction input, and insert it. */
693710
SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn, const CTxOut& txout);

‎src/utilstrencodings.cpp‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,3 +544,43 @@ bool ParseFixedPoint(const std::string &val, int decimals, int64_t *amount_out)
544544
return true;
545545
}
546546

547+
bool ParseHDKeypath(const std::string& keypath_str, std::vector<uint32_t>& keypath)
548+
{
549+
std::stringstream ss(keypath_str);
550+
std::string item;
551+
bool first = true;
552+
while (std::getline(ss, item, '/')) {
553+
if (item.compare("m") == 0) {
554+
if (first) {
555+
first = false;
556+
continue;
557+
}
558+
return false;
559+
}
560+
// Finds whether it is hardened
561+
uint32_t path = 0;
562+
size_t pos = item.find("'");
563+
if (pos != std::string::npos) {
564+
// The hardened tick can only be in the last index of the string
565+
if (pos != item.size() - 1) {
566+
return false;
567+
}
568+
path |= 0x80000000;
569+
item = item.substr(0, item.size() - 1); // Drop the last character which is the hardened tick
570+
}
571+
572+
// Ensure this is only numbers
573+
if (item.find_first_not_of( "0123456789" ) != std::string::npos) {
574+
return false;
575+
}
576+
uint32_t number;
577+
if (!ParseUInt32(item, &number)) {
578+
return false;
579+
}
580+
path |= number;
581+
582+
keypath.push_back(path);
583+
first = false;
584+
}
585+
return true;
586+
}

‎src/utilstrencodings.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,4 +183,7 @@ bool ConvertBits(const O& outfn, I it, I end) {
183183
return true;
184184
}
185185

186+
/** Parse an HD keypaths like "m/7/0'/2000". */
187+
bool ParseHDKeypath(const std::string& keypath_str, std::vector<uint32_t>& keypath);
188+
186189
#endif // BITCOIN_UTILSTRENCODINGS_H

0 commit comments

Comments
 (0)