Skip to content

Commit d0dab89

Browse files
achow101achow101
committed
Refactor: Require scriptPubKey to get wallet SigningProvider
Easier to review ignoring whitespace: git log -p -n1 -w This commit does not change behavior. It passes new CScript arguments to signing functions, but the arguments aren't currently used.
1 parent 4b0c718 commit d0dab89

File tree

8 files changed

+127
-51
lines changed

8 files changed

+127
-51
lines changed

‎src/interfaces/wallet.cpp

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,22 @@ class WalletImpl : public Wallet
117117
std::string error;
118118
return m_wallet->GetNewDestination(type, label, dest, error);
119119
}
120-
bool getPubKey(const CKeyID& address, CPubKey& pub_key) override { return m_wallet->GetLegacyScriptPubKeyMan()->GetPubKey(address, pub_key); }
121-
bool getPrivKey(const CKeyID& address, CKey& key) override { return m_wallet->GetLegacyScriptPubKeyMan()->GetKey(address, key); }
120+
bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) override
121+
{
122+
const SigningProvider* provider = m_wallet->GetSigningProvider(script);
123+
if (provider) {
124+
return provider->GetPubKey(address, pub_key);
125+
}
126+
return false;
127+
}
128+
bool getPrivKey(const CScript& script, const CKeyID& address, CKey& key) override
129+
{
130+
const SigningProvider* provider = m_wallet->GetSigningProvider(script);
131+
if (provider) {
132+
return provider->GetKey(address, key);
133+
}
134+
return false;
135+
}
122136
bool isSpendable(const CTxDestination& dest) override { return m_wallet->IsMine(dest) & ISMINE_SPENDABLE; }
123137
bool haveWatchOnly() override
124138
{

‎src/interfaces/wallet.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,10 @@ class Wallet
8181
virtual bool getNewDestination(const OutputType type, const std::string label, CTxDestination& dest) = 0;
8282

8383
//! Get public key.
84-
virtual bool getPubKey(const CKeyID& address, CPubKey& pub_key) = 0;
84+
virtual bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) = 0;
8585

8686
//! Get private key.
87-
virtual bool getPrivKey(const CKeyID& address, CKey& key) = 0;
87+
virtual bool getPrivKey(const CScript& script, const CKeyID& address, CKey& key) = 0;
8888

8989
//! Return whether wallet has private key.
9090
virtual bool isSpendable(const CTxDestination& dest) = 0;

‎src/qt/coincontroldialog.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog)
468468
{
469469
CPubKey pubkey;
470470
PKHash *pkhash = boost::get<PKHash>(&address);
471-
if (pkhash && model->wallet().getPubKey(CKeyID(*pkhash), pubkey))
471+
if (pkhash && model->wallet().getPubKey(out.txout.scriptPubKey, CKeyID(*pkhash), pubkey))
472472
{
473473
nBytesInputs += (pubkey.IsCompressed() ? 148 : 180);
474474
}

‎src/qt/signverifymessagedialog.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ void SignVerifyMessageDialog::on_signMessageButton_SM_clicked()
136136
}
137137

138138
CKey key;
139-
if (!model->wallet().getPrivKey(CKeyID(*pkhash), key))
139+
if (!model->wallet().getPrivKey(GetScriptForDestination(destination), CKeyID(*pkhash), key))
140140
{
141141
ui->statusLabel_SM->setStyleSheet("QLabel { color: red; }");
142142
ui->statusLabel_SM->setText(tr("Private key for the entered address is not available."));

‎src/wallet/psbtwallet.cpp

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,35 @@ TransactionError FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& ps
3939
return TransactionError::SIGHASH_MISMATCH;
4040
}
4141

42-
complete &= SignPSBTInput(HidingSigningProvider(pwallet->GetSigningProvider(), !sign, !bip32derivs), psbtx, i, sighash_type);
42+
// Get the scriptPubKey to know which SigningProvider to use
43+
CScript script;
44+
if (!input.witness_utxo.IsNull()) {
45+
script = input.witness_utxo.scriptPubKey;
46+
} else if (input.non_witness_utxo) {
47+
script = input.non_witness_utxo->vout[txin.prevout.n].scriptPubKey;
48+
} else {
49+
// There's no UTXO so we can just skip this now
50+
complete = false;
51+
continue;
52+
}
53+
SignatureData sigdata;
54+
input.FillSignatureData(sigdata);
55+
const SigningProvider* provider = pwallet->GetSigningProvider(script, sigdata);
56+
if (!provider) {
57+
complete = false;
58+
continue;
59+
}
60+
61+
complete &= SignPSBTInput(HidingSigningProvider(provider, !sign, !bip32derivs), psbtx, i, sighash_type);
4362
}
4463

4564
// Fill in the bip32 keypaths and redeemscripts for the outputs so that hardware wallets can identify change
4665
for (unsigned int i = 0; i < psbtx.tx->vout.size(); ++i) {
47-
UpdatePSBTOutput(HidingSigningProvider(pwallet->GetSigningProvider(), true, !bip32derivs), psbtx, i);
66+
const CTxOut& out = psbtx.tx->vout.at(i);
67+
const SigningProvider* provider = pwallet->GetSigningProvider(out.scriptPubKey);
68+
if (provider) {
69+
UpdatePSBTOutput(HidingSigningProvider(provider, true, !bip32derivs), psbtx, i);
70+
}
4871
}
4972

5073
return TransactionError::OK;

‎src/wallet/rpcwallet.cpp

Lines changed: 59 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,11 @@ static UniValue signmessage(const JSONRPCRequest& request)
550550
throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to key");
551551
}
552552

553-
const SigningProvider* provider = pwallet->GetSigningProvider();
553+
CScript script_pub_key = GetScriptForDestination(*pkhash);
554+
const SigningProvider* provider = pwallet->GetSigningProvider(script_pub_key);
555+
if (!provider) {
556+
throw JSONRPCError(RPC_WALLET_ERROR, "Private key not available");
557+
}
554558

555559
CKey key;
556560
CKeyID keyID(*pkhash);
@@ -2933,34 +2937,36 @@ static UniValue listunspent(const JSONRPCRequest& request)
29332937
entry.pushKV("label", i->second.name);
29342938
}
29352939

2936-
const SigningProvider* provider = pwallet->GetSigningProvider();
2937-
if (scriptPubKey.IsPayToScriptHash()) {
2938-
const CScriptID& hash = CScriptID(boost::get<ScriptHash>(address));
2939-
CScript redeemScript;
2940-
if (provider->GetCScript(hash, redeemScript)) {
2941-
entry.pushKV("redeemScript", HexStr(redeemScript.begin(), redeemScript.end()));
2942-
// Now check if the redeemScript is actually a P2WSH script
2943-
CTxDestination witness_destination;
2944-
if (redeemScript.IsPayToWitnessScriptHash()) {
2945-
bool extracted = ExtractDestination(redeemScript, witness_destination);
2946-
CHECK_NONFATAL(extracted);
2947-
// Also return the witness script
2948-
const WitnessV0ScriptHash& whash = boost::get<WitnessV0ScriptHash>(witness_destination);
2949-
CScriptID id;
2950-
CRIPEMD160().Write(whash.begin(), whash.size()).Finalize(id.begin());
2951-
CScript witnessScript;
2952-
if (provider->GetCScript(id, witnessScript)) {
2953-
entry.pushKV("witnessScript", HexStr(witnessScript.begin(), witnessScript.end()));
2940+
const SigningProvider* provider = pwallet->GetSigningProvider(scriptPubKey);
2941+
if (provider) {
2942+
if (scriptPubKey.IsPayToScriptHash()) {
2943+
const CScriptID& hash = CScriptID(boost::get<ScriptHash>(address));
2944+
CScript redeemScript;
2945+
if (provider->GetCScript(hash, redeemScript)) {
2946+
entry.pushKV("redeemScript", HexStr(redeemScript.begin(), redeemScript.end()));
2947+
// Now check if the redeemScript is actually a P2WSH script
2948+
CTxDestination witness_destination;
2949+
if (redeemScript.IsPayToWitnessScriptHash()) {
2950+
bool extracted = ExtractDestination(redeemScript, witness_destination);
2951+
CHECK_NONFATAL(extracted);
2952+
// Also return the witness script
2953+
const WitnessV0ScriptHash& whash = boost::get<WitnessV0ScriptHash>(witness_destination);
2954+
CScriptID id;
2955+
CRIPEMD160().Write(whash.begin(), whash.size()).Finalize(id.begin());
2956+
CScript witnessScript;
2957+
if (provider->GetCScript(id, witnessScript)) {
2958+
entry.pushKV("witnessScript", HexStr(witnessScript.begin(), witnessScript.end()));
2959+
}
29542960
}
29552961
}
2956-
}
2957-
} else if (scriptPubKey.IsPayToWitnessScriptHash()) {
2958-
const WitnessV0ScriptHash& whash = boost::get<WitnessV0ScriptHash>(address);
2959-
CScriptID id;
2960-
CRIPEMD160().Write(whash.begin(), whash.size()).Finalize(id.begin());
2961-
CScript witnessScript;
2962-
if (provider->GetCScript(id, witnessScript)) {
2963-
entry.pushKV("witnessScript", HexStr(witnessScript.begin(), witnessScript.end()));
2962+
} else if (scriptPubKey.IsPayToWitnessScriptHash()) {
2963+
const WitnessV0ScriptHash& whash = boost::get<WitnessV0ScriptHash>(address);
2964+
CScriptID id;
2965+
CRIPEMD160().Write(whash.begin(), whash.size()).Finalize(id.begin());
2966+
CScript witnessScript;
2967+
if (provider->GetCScript(id, witnessScript)) {
2968+
entry.pushKV("witnessScript", HexStr(witnessScript.begin(), witnessScript.end()));
2969+
}
29642970
}
29652971
}
29662972
}
@@ -2971,8 +2977,11 @@ static UniValue listunspent(const JSONRPCRequest& request)
29712977
entry.pushKV("spendable", out.fSpendable);
29722978
entry.pushKV("solvable", out.fSolvable);
29732979
if (out.fSolvable) {
2974-
auto descriptor = InferDescriptor(scriptPubKey, *pwallet->GetLegacyScriptPubKeyMan());
2975-
entry.pushKV("desc", descriptor->ToString());
2980+
const SigningProvider* provider = pwallet->GetSigningProvider(scriptPubKey);
2981+
if (provider) {
2982+
auto descriptor = InferDescriptor(scriptPubKey, *provider);
2983+
entry.pushKV("desc", descriptor->ToString());
2984+
}
29762985
}
29772986
if (avoid_reuse) entry.pushKV("reused", reused);
29782987
entry.pushKV("safe", out.fSafe);
@@ -3281,9 +3290,23 @@ UniValue signrawtransactionwithwallet(const JSONRPCRequest& request)
32813290
// Parse the prevtxs array
32823291
ParsePrevouts(request.params[1], nullptr, coins);
32833292

3293+
std::set<const SigningProvider*> providers;
3294+
for (const std::pair<COutPoint, Coin> coin_pair : coins) {
3295+
const SigningProvider* provider = pwallet->GetSigningProvider(coin_pair.second.out.scriptPubKey);
3296+
if (provider) {
3297+
providers.insert(std::move(provider));
3298+
}
3299+
}
3300+
if (providers.size() == 0) {
3301+
// When there are no available providers, use DUMMY_SIGNING_PROVIDER so we can check if the tx is complete
3302+
providers.insert(&DUMMY_SIGNING_PROVIDER);
3303+
}
3304+
32843305
UniValue result(UniValue::VOBJ);
3285-
SignTransaction(mtx, &*pwallet->GetLegacyScriptPubKeyMan(), coins, request.params[2], result);
3286-
return result;
3306+
for (const SigningProvider* provider : providers) {
3307+
SignTransaction(mtx, provider, coins, request.params[2], result);
3308+
}
3309+
return result;
32873310
}
32883311

32893312
static UniValue bumpfee(const JSONRPCRequest& request)
@@ -3648,9 +3671,10 @@ static UniValue DescribeWalletAddress(CWallet* pwallet, const CTxDestination& de
36483671
{
36493672
UniValue ret(UniValue::VOBJ);
36503673
UniValue detail = DescribeAddress(dest);
3674+
CScript script = GetScriptForDestination(dest);
36513675
const SigningProvider* provider = nullptr;
36523676
if (pwallet) {
3653-
provider = pwallet->GetSigningProvider();
3677+
provider = pwallet->GetSigningProvider(script);
36543678
}
36553679
ret.pushKVs(detail);
36563680
ret.pushKVs(boost::apply_visitor(DescribeWalletAddressVisitor(provider), dest));
@@ -3742,11 +3766,11 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
37423766

37433767
CScript scriptPubKey = GetScriptForDestination(dest);
37443768
ret.pushKV("scriptPubKey", HexStr(scriptPubKey.begin(), scriptPubKey.end()));
3745-
const SigningProvider* provider = pwallet->GetSigningProvider();
3769+
const SigningProvider* provider = pwallet->GetSigningProvider(scriptPubKey);
37463770

37473771
isminetype mine = pwallet->IsMine(dest);
37483772
ret.pushKV("ismine", bool(mine & ISMINE_SPENDABLE));
3749-
bool solvable = IsSolvable(*provider, scriptPubKey);
3773+
bool solvable = provider && IsSolvable(*provider, scriptPubKey);
37503774
ret.pushKV("solvable", solvable);
37513775
if (solvable) {
37523776
ret.pushKV("desc", InferDescriptor(scriptPubKey, *provider)->ToString());
@@ -3759,7 +3783,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
37593783
}
37603784
ret.pushKV("ischange", pwallet->IsChange(scriptPubKey));
37613785

3762-
ScriptPubKeyMan* spk_man = pwallet->GetScriptPubKeyMan();
3786+
ScriptPubKeyMan* spk_man = pwallet->GetScriptPubKeyMan(scriptPubKey);
37633787
if (spk_man) {
37643788
CKeyID key_id = GetKeyForDestination(*provider, dest);
37653789
const CKeyMetadata* meta = nullptr;

‎src/wallet/wallet.cpp

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1342,7 +1342,11 @@ bool CWallet::DummySignInput(CTxIn &tx_in, const CTxOut &txout, bool use_max_sig
13421342
const CScript& scriptPubKey = txout.scriptPubKey;
13431343
SignatureData sigdata;
13441344

1345-
const SigningProvider* provider = GetSigningProvider();
1345+
const SigningProvider* provider = GetSigningProvider(scriptPubKey);
1346+
if (!provider) {
1347+
// We don't know about this scriptpbuKey;
1348+
return false;
1349+
}
13461350

13471351
if (!ProduceSignature(*provider, use_max_sig ? DUMMY_MAXIMUM_SIGNATURE_CREATOR : DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigdata)) {
13481352
return false;
@@ -2096,7 +2100,7 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector<
20962100
continue;
20972101
}
20982102

2099-
const SigningProvider* provider = GetSigningProvider();
2103+
const SigningProvider* provider = GetSigningProvider(wtx.tx->vout[i].scriptPubKey);
21002104

21012105
bool solvable = provider ? IsSolvable(*provider, wtx.tx->vout[i].scriptPubKey) : false;
21022106
bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable));
@@ -2333,8 +2337,9 @@ bool CWallet::SignTransaction(CMutableTransaction& tx)
23332337
const CAmount& amount = mi->second.tx->vout[input.prevout.n].nValue;
23342338
SignatureData sigdata;
23352339

2336-
const SigningProvider* provider = GetSigningProvider();
2340+
const SigningProvider* provider = GetSigningProvider(scriptPubKey);
23372341
if (!provider) {
2342+
// We don't know about this scriptpbuKey;
23382343
return false;
23392344
}
23402345

@@ -2796,7 +2801,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
27962801
const CScript& scriptPubKey = coin.txout.scriptPubKey;
27972802
SignatureData sigdata;
27982803

2799-
const SigningProvider* provider = GetSigningProvider();
2804+
const SigningProvider* provider = GetSigningProvider(scriptPubKey);
28002805
if (!provider || !ProduceSignature(*provider, MutableTransactionSignatureCreator(&txNew, nIn, coin.txout.nValue, SIGHASH_ALL), scriptPubKey, sigdata))
28012806
{
28022807
strFailReason = _("Signing transaction failed").translated;
@@ -4002,12 +4007,17 @@ bool CWallet::Lock()
40024007
return true;
40034008
}
40044009

4005-
ScriptPubKeyMan* CWallet::GetScriptPubKeyMan() const
4010+
ScriptPubKeyMan* CWallet::GetScriptPubKeyMan(const CScript& script) const
4011+
{
4012+
return m_spk_man.get();
4013+
}
4014+
4015+
const SigningProvider* CWallet::GetSigningProvider(const CScript& script) const
40064016
{
40074017
return m_spk_man.get();
40084018
}
40094019

4010-
const SigningProvider* CWallet::GetSigningProvider() const
4020+
const SigningProvider* CWallet::GetSigningProvider(const CScript& script, SignatureData& sigdata) const
40114021
{
40124022
return m_spk_man.get();
40134023
}

‎src/wallet/wallet.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,8 +1113,13 @@ class CWallet final : public WalletStorage, private interfaces::Chain::Notificat
11131113
LogPrintf(("%s " + fmt).c_str(), GetDisplayName(), parameters...);
11141114
};
11151115

1116-
ScriptPubKeyMan* GetScriptPubKeyMan() const;
1117-
const SigningProvider* GetSigningProvider() const;
1116+
//! Get the ScriptPubKeyMan for a script
1117+
ScriptPubKeyMan* GetScriptPubKeyMan(const CScript& script) const;
1118+
1119+
//! Get the SigningProvider for a script
1120+
const SigningProvider* GetSigningProvider(const CScript& script) const;
1121+
const SigningProvider* GetSigningProvider(const CScript& script, SignatureData& sigdata) const;
1122+
11181123
LegacyScriptPubKeyMan* GetLegacyScriptPubKeyMan() const;
11191124

11201125
// Temporary LegacyScriptPubKeyMan accessors and aliases.

0 commit comments

Comments
 (0)