Merge #17371: Refactor: Require scriptPubKey to get wallet SigningProvider

d0dab897af Refactor: Require scriptPubKey to get wallet SigningProvider (Andrew Chow)
4b0c718f8f Accumulate result UniValue in SignTransaction (Andrew Chow)

Pull request description:

  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.

  Split from #17261

ACKs for top commit:
  instagibbs:
    utACK d0dab897af
  ryanofsky:
    Code review ACK d0dab897af. Thanks for the SignTransaction update. No other changes since last review
  Sjors:
    Code review ACK d0dab897af
  promag:
    Code review ACK d0dab897af.
  meshcollider:
    Code review ACK d0dab897af

Tree-SHA512: c3f52df20fd9d6b3b5aa65562cf5f7dce7b7f44c148b0f988f8b578fce2a28e9b7bf010f5f04bb5bf60f5272b2899f1dbbfb8aee81579c21c9cba559d1d2bb70
This commit is contained in:
Samuel Dobson 2019-11-23 08:33:38 +13:00
commit 8aac85d71e
No known key found for this signature in database
GPG key ID: D300116E1C875A3D
11 changed files with 137 additions and 57 deletions

View file

@ -116,8 +116,22 @@ public:
std::string error;
return m_wallet->GetNewDestination(type, label, dest, error);
}
bool getPubKey(const CKeyID& address, CPubKey& pub_key) override { return m_wallet->GetLegacyScriptPubKeyMan()->GetPubKey(address, pub_key); }
bool getPrivKey(const CKeyID& address, CKey& key) override { return m_wallet->GetLegacyScriptPubKeyMan()->GetKey(address, key); }
bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) override
{
const SigningProvider* provider = m_wallet->GetSigningProvider(script);
if (provider) {
return provider->GetPubKey(address, pub_key);
}
return false;
}
bool getPrivKey(const CScript& script, const CKeyID& address, CKey& key) override
{
const SigningProvider* provider = m_wallet->GetSigningProvider(script);
if (provider) {
return provider->GetKey(address, key);
}
return false;
}
bool isSpendable(const CTxDestination& dest) override { return m_wallet->IsMine(dest) & ISMINE_SPENDABLE; }
bool haveWatchOnly() override
{

View file

@ -81,10 +81,10 @@ public:
virtual bool getNewDestination(const OutputType type, const std::string label, CTxDestination& dest) = 0;
//! Get public key.
virtual bool getPubKey(const CKeyID& address, CPubKey& pub_key) = 0;
virtual bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) = 0;
//! Get private key.
virtual bool getPrivKey(const CKeyID& address, CKey& key) = 0;
virtual bool getPrivKey(const CScript& script, const CKeyID& address, CKey& key) = 0;
//! Return whether wallet has private key.
virtual bool isSpendable(const CTxDestination& dest) = 0;

View file

@ -468,7 +468,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog)
{
CPubKey pubkey;
PKHash *pkhash = boost::get<PKHash>(&address);
if (pkhash && model->wallet().getPubKey(CKeyID(*pkhash), pubkey))
if (pkhash && model->wallet().getPubKey(out.txout.scriptPubKey, CKeyID(*pkhash), pubkey))
{
nBytesInputs += (pubkey.IsCompressed() ? 148 : 180);
}

View file

@ -136,7 +136,7 @@ void SignVerifyMessageDialog::on_signMessageButton_SM_clicked()
}
CKey key;
if (!model->wallet().getPrivKey(CKeyID(*pkhash), key))
if (!model->wallet().getPrivKey(GetScriptForDestination(destination), CKeyID(*pkhash), key))
{
ui->statusLabel_SM->setStyleSheet("QLabel { color: red; }");
ui->statusLabel_SM->setText(tr("Private key for the entered address is not available."));

View file

@ -763,7 +763,9 @@ static UniValue signrawtransactionwithkey(const JSONRPCRequest& request)
// Parse the prevtxs array
ParsePrevouts(request.params[2], &keystore, coins);
return SignTransaction(mtx, &keystore, coins, request.params[3]);
UniValue result(UniValue::VOBJ);
SignTransaction(mtx, &keystore, coins, request.params[3], result);
return result;
}
static UniValue sendrawtransaction(const JSONRPCRequest& request)

View file

@ -268,7 +268,7 @@ void ParsePrevouts(const UniValue& prevTxsUnival, FillableSigningProvider* keyst
}
}
UniValue SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map<COutPoint, Coin>& coins, const UniValue& hashType)
void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map<COutPoint, Coin>& coins, const UniValue& hashType, UniValue& result)
{
int nHashType = ParseSighashString(hashType);
@ -319,12 +319,12 @@ UniValue SignTransaction(CMutableTransaction& mtx, const SigningProvider* keysto
}
bool fComplete = vErrors.empty();
UniValue result(UniValue::VOBJ);
result.pushKV("hex", EncodeHexTx(CTransaction(mtx)));
result.pushKV("complete", fComplete);
if (!vErrors.empty()) {
if (result.exists("errors")) {
vErrors.push_backV(result["errors"].getValues());
}
result.pushKV("errors", vErrors);
}
return result;
}

View file

@ -21,9 +21,9 @@ class SigningProvider;
* @param keystore Temporary keystore containing signing keys
* @param coins Map of unspent outputs
* @param hashType The signature hash type
* @returns JSON object with details of signed transaction
* @param result JSON object where signed transaction results accumulate
*/
UniValue SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map<COutPoint, Coin>& coins, const UniValue& hashType);
void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map<COutPoint, Coin>& coins, const UniValue& hashType, UniValue& result);
/**
* Parse a prevtxs UniValue array and get the map of coins from it

View file

@ -39,12 +39,35 @@ TransactionError FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& ps
return TransactionError::SIGHASH_MISMATCH;
}
complete &= SignPSBTInput(HidingSigningProvider(pwallet->GetSigningProvider(), !sign, !bip32derivs), psbtx, i, sighash_type);
// Get the scriptPubKey to know which SigningProvider to use
CScript script;
if (!input.witness_utxo.IsNull()) {
script = input.witness_utxo.scriptPubKey;
} else if (input.non_witness_utxo) {
script = input.non_witness_utxo->vout[txin.prevout.n].scriptPubKey;
} else {
// There's no UTXO so we can just skip this now
complete = false;
continue;
}
SignatureData sigdata;
input.FillSignatureData(sigdata);
const SigningProvider* provider = pwallet->GetSigningProvider(script, sigdata);
if (!provider) {
complete = false;
continue;
}
complete &= SignPSBTInput(HidingSigningProvider(provider, !sign, !bip32derivs), psbtx, i, sighash_type);
}
// Fill in the bip32 keypaths and redeemscripts for the outputs so that hardware wallets can identify change
for (unsigned int i = 0; i < psbtx.tx->vout.size(); ++i) {
UpdatePSBTOutput(HidingSigningProvider(pwallet->GetSigningProvider(), true, !bip32derivs), psbtx, i);
const CTxOut& out = psbtx.tx->vout.at(i);
const SigningProvider* provider = pwallet->GetSigningProvider(out.scriptPubKey);
if (provider) {
UpdatePSBTOutput(HidingSigningProvider(provider, true, !bip32derivs), psbtx, i);
}
}
return TransactionError::OK;

View file

@ -560,7 +560,11 @@ static UniValue signmessage(const JSONRPCRequest& request)
throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to key");
}
const SigningProvider* provider = pwallet->GetSigningProvider();
CScript script_pub_key = GetScriptForDestination(*pkhash);
const SigningProvider* provider = pwallet->GetSigningProvider(script_pub_key);
if (!provider) {
throw JSONRPCError(RPC_WALLET_ERROR, "Private key not available");
}
CKey key;
CKeyID keyID(*pkhash);
@ -2940,34 +2944,36 @@ static UniValue listunspent(const JSONRPCRequest& request)
entry.pushKV("label", i->second.name);
}
const SigningProvider* provider = pwallet->GetSigningProvider();
if (scriptPubKey.IsPayToScriptHash()) {
const CScriptID& hash = CScriptID(boost::get<ScriptHash>(address));
CScript redeemScript;
if (provider->GetCScript(hash, redeemScript)) {
entry.pushKV("redeemScript", HexStr(redeemScript.begin(), redeemScript.end()));
// Now check if the redeemScript is actually a P2WSH script
CTxDestination witness_destination;
if (redeemScript.IsPayToWitnessScriptHash()) {
bool extracted = ExtractDestination(redeemScript, witness_destination);
CHECK_NONFATAL(extracted);
// Also return the witness script
const WitnessV0ScriptHash& whash = boost::get<WitnessV0ScriptHash>(witness_destination);
CScriptID id;
CRIPEMD160().Write(whash.begin(), whash.size()).Finalize(id.begin());
CScript witnessScript;
if (provider->GetCScript(id, witnessScript)) {
entry.pushKV("witnessScript", HexStr(witnessScript.begin(), witnessScript.end()));
const SigningProvider* provider = pwallet->GetSigningProvider(scriptPubKey);
if (provider) {
if (scriptPubKey.IsPayToScriptHash()) {
const CScriptID& hash = CScriptID(boost::get<ScriptHash>(address));
CScript redeemScript;
if (provider->GetCScript(hash, redeemScript)) {
entry.pushKV("redeemScript", HexStr(redeemScript.begin(), redeemScript.end()));
// Now check if the redeemScript is actually a P2WSH script
CTxDestination witness_destination;
if (redeemScript.IsPayToWitnessScriptHash()) {
bool extracted = ExtractDestination(redeemScript, witness_destination);
CHECK_NONFATAL(extracted);
// Also return the witness script
const WitnessV0ScriptHash& whash = boost::get<WitnessV0ScriptHash>(witness_destination);
CScriptID id;
CRIPEMD160().Write(whash.begin(), whash.size()).Finalize(id.begin());
CScript witnessScript;
if (provider->GetCScript(id, witnessScript)) {
entry.pushKV("witnessScript", HexStr(witnessScript.begin(), witnessScript.end()));
}
}
}
}
} else if (scriptPubKey.IsPayToWitnessScriptHash()) {
const WitnessV0ScriptHash& whash = boost::get<WitnessV0ScriptHash>(address);
CScriptID id;
CRIPEMD160().Write(whash.begin(), whash.size()).Finalize(id.begin());
CScript witnessScript;
if (provider->GetCScript(id, witnessScript)) {
entry.pushKV("witnessScript", HexStr(witnessScript.begin(), witnessScript.end()));
} else if (scriptPubKey.IsPayToWitnessScriptHash()) {
const WitnessV0ScriptHash& whash = boost::get<WitnessV0ScriptHash>(address);
CScriptID id;
CRIPEMD160().Write(whash.begin(), whash.size()).Finalize(id.begin());
CScript witnessScript;
if (provider->GetCScript(id, witnessScript)) {
entry.pushKV("witnessScript", HexStr(witnessScript.begin(), witnessScript.end()));
}
}
}
}
@ -2978,8 +2984,11 @@ static UniValue listunspent(const JSONRPCRequest& request)
entry.pushKV("spendable", out.fSpendable);
entry.pushKV("solvable", out.fSolvable);
if (out.fSolvable) {
auto descriptor = InferDescriptor(scriptPubKey, *pwallet->GetLegacyScriptPubKeyMan());
entry.pushKV("desc", descriptor->ToString());
const SigningProvider* provider = pwallet->GetSigningProvider(scriptPubKey);
if (provider) {
auto descriptor = InferDescriptor(scriptPubKey, *provider);
entry.pushKV("desc", descriptor->ToString());
}
}
if (avoid_reuse) entry.pushKV("reused", reused);
entry.pushKV("safe", out.fSafe);
@ -3288,7 +3297,23 @@ UniValue signrawtransactionwithwallet(const JSONRPCRequest& request)
// Parse the prevtxs array
ParsePrevouts(request.params[1], nullptr, coins);
return SignTransaction(mtx, &*pwallet->GetLegacyScriptPubKeyMan(), coins, request.params[2]);
std::set<const SigningProvider*> providers;
for (const std::pair<COutPoint, Coin> coin_pair : coins) {
const SigningProvider* provider = pwallet->GetSigningProvider(coin_pair.second.out.scriptPubKey);
if (provider) {
providers.insert(std::move(provider));
}
}
if (providers.size() == 0) {
// When there are no available providers, use DUMMY_SIGNING_PROVIDER so we can check if the tx is complete
providers.insert(&DUMMY_SIGNING_PROVIDER);
}
UniValue result(UniValue::VOBJ);
for (const SigningProvider* provider : providers) {
SignTransaction(mtx, provider, coins, request.params[2], result);
}
return result;
}
static UniValue bumpfee(const JSONRPCRequest& request)
@ -3653,9 +3678,10 @@ static UniValue DescribeWalletAddress(CWallet* pwallet, const CTxDestination& de
{
UniValue ret(UniValue::VOBJ);
UniValue detail = DescribeAddress(dest);
CScript script = GetScriptForDestination(dest);
const SigningProvider* provider = nullptr;
if (pwallet) {
provider = pwallet->GetSigningProvider();
provider = pwallet->GetSigningProvider(script);
}
ret.pushKVs(detail);
ret.pushKVs(boost::apply_visitor(DescribeWalletAddressVisitor(provider), dest));
@ -3747,11 +3773,11 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
CScript scriptPubKey = GetScriptForDestination(dest);
ret.pushKV("scriptPubKey", HexStr(scriptPubKey.begin(), scriptPubKey.end()));
const SigningProvider* provider = pwallet->GetSigningProvider();
const SigningProvider* provider = pwallet->GetSigningProvider(scriptPubKey);
isminetype mine = pwallet->IsMine(dest);
ret.pushKV("ismine", bool(mine & ISMINE_SPENDABLE));
bool solvable = IsSolvable(*provider, scriptPubKey);
bool solvable = provider && IsSolvable(*provider, scriptPubKey);
ret.pushKV("solvable", solvable);
if (solvable) {
ret.pushKV("desc", InferDescriptor(scriptPubKey, *provider)->ToString());
@ -3764,7 +3790,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
}
ret.pushKV("ischange", pwallet->IsChange(scriptPubKey));
ScriptPubKeyMan* spk_man = pwallet->GetScriptPubKeyMan();
ScriptPubKeyMan* spk_man = pwallet->GetScriptPubKeyMan(scriptPubKey);
if (spk_man) {
if (const CKeyMetadata* meta = spk_man->GetMetadata(dest)) {
ret.pushKV("timestamp", meta->nCreateTime);

View file

@ -1363,7 +1363,11 @@ bool CWallet::DummySignInput(CTxIn &tx_in, const CTxOut &txout, bool use_max_sig
const CScript& scriptPubKey = txout.scriptPubKey;
SignatureData sigdata;
const SigningProvider* provider = GetSigningProvider();
const SigningProvider* provider = GetSigningProvider(scriptPubKey);
if (!provider) {
// We don't know about this scriptpbuKey;
return false;
}
if (!ProduceSignature(*provider, use_max_sig ? DUMMY_MAXIMUM_SIGNATURE_CREATOR : DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigdata)) {
return false;
@ -2125,7 +2129,7 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector<
continue;
}
const SigningProvider* provider = GetSigningProvider();
const SigningProvider* provider = GetSigningProvider(wtx.tx->vout[i].scriptPubKey);
bool solvable = provider ? IsSolvable(*provider, wtx.tx->vout[i].scriptPubKey) : false;
bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable));
@ -2378,8 +2382,9 @@ bool CWallet::SignTransaction(CMutableTransaction& tx)
const CAmount& amount = mi->second.tx->vout[input.prevout.n].nValue;
SignatureData sigdata;
const SigningProvider* provider = GetSigningProvider();
const SigningProvider* provider = GetSigningProvider(scriptPubKey);
if (!provider) {
// We don't know about this scriptpbuKey;
return false;
}
@ -2846,7 +2851,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
const CScript& scriptPubKey = coin.txout.scriptPubKey;
SignatureData sigdata;
const SigningProvider* provider = GetSigningProvider();
const SigningProvider* provider = GetSigningProvider(scriptPubKey);
if (!provider || !ProduceSignature(*provider, MutableTransactionSignatureCreator(&txNew, nIn, coin.txout.nValue, SIGHASH_ALL), scriptPubKey, sigdata))
{
strFailReason = _("Signing transaction failed").translated;
@ -4043,12 +4048,17 @@ bool CWallet::Lock()
return true;
}
ScriptPubKeyMan* CWallet::GetScriptPubKeyMan() const
ScriptPubKeyMan* CWallet::GetScriptPubKeyMan(const CScript& script) const
{
return m_spk_man.get();
}
const SigningProvider* CWallet::GetSigningProvider() const
const SigningProvider* CWallet::GetSigningProvider(const CScript& script) const
{
return m_spk_man.get();
}
const SigningProvider* CWallet::GetSigningProvider(const CScript& script, SignatureData& sigdata) const
{
return m_spk_man.get();
}

View file

@ -1130,8 +1130,13 @@ public:
LogPrintf(("%s " + fmt).c_str(), GetDisplayName(), parameters...);
};
ScriptPubKeyMan* GetScriptPubKeyMan() const;
const SigningProvider* GetSigningProvider() const;
//! Get the ScriptPubKeyMan for a script
ScriptPubKeyMan* GetScriptPubKeyMan(const CScript& script) const;
//! Get the SigningProvider for a script
const SigningProvider* GetSigningProvider(const CScript& script) const;
const SigningProvider* GetSigningProvider(const CScript& script, SignatureData& sigdata) const;
LegacyScriptPubKeyMan* GetLegacyScriptPubKeyMan() const;
// Temporary LegacyScriptPubKeyMan accessors and aliases.