highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-04-14 01:27:24 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge #18115: wallet: Pass in transactions and messages for signing instead of exporting the private keys

Browse Source 
d2774c09cf Clear any input_errors for an input after it is signed (Andrew Chow)
dc174881ad Replace GetSigningProvider with GetSolvingProvider (Andrew Chow)
6a9c429084 Move direct calls to MessageSign into new SignMessage functions in CWallet and ScriptPubKeyMan (Andrew Chow)
82a30fade7 Move key and script filling and signing from CWallet::FillPSBT to ScriptPubKeyMan::FillPSBT (Andrew Chow)
3d70dd99f9 Move FillPSBT to be a member of CWallet (Andrew Chow)
a4af324d15 Use CWallet::SignTransaction in CreateTransaction and signrawtransactionwithwallet (Andrew Chow)
f37de92744 Implement CWallet::SignTransaction using ScriptPubKeyMan::SignTransaction (Andrew Chow)
d999dd588c Add SignTransaction function to ScriptPubKeyMan and LegacyScriptPubKeyMan (Andrew Chow)
2c52b59d0a Refactor rawtransaction's SignTransaction into generic SignTransaction function (Andrew Chow)

Pull request description:

  Following #17261, the way to sign transactions, PSBTs, and messages was to use `GetSigningProvider()` and get a `SigningProvider` containing the private keys. However this may not be feasible for future `ScriptPubKeyMan`s, such as for hardware wallets. Instead of exporting a `SigningProvider` containing private keys, we need to pass these things into the `ScriptPubKeyMan` (via `CWallet`) so that they can do whatever is needed internally to sign them. This is largely a refactor as the logic of processing transactions, PSBTs, and messages for is moved into `LegacyScriptPubKeyMan` and `CWallet` instead of being handled by the caller (e.g. `signrawtransaction`).

  To help with this, I've refactored the 3(!) implementations of a `SignTransaction()` function into one generic one. This function will be called by `signrawtransactionwithkey` and `LegacyScriptPubKeyMan::SignTransaction()`. `CWallet::CreateTransaction()` is changed to call `CWallet::SignTransaction()` which in turn, calls `LegacyScriptPubKeyMan::SignTransaction()`. Other `ScriptPubKeyMan`s may implement `SignTransaction()` differently.

  `FillPSBT()` is moved to be a member function of `CWallet` and the `psbtwallet.cpp/h` files removed. It is further split so that `CWallet` handles filling the UTXOs while the `ScriptPubKeyMan` handles adding keys, derivation paths, scripts, and signatures. In the end `LegacyScriptPubKeyMan::FillPSBT` still calls `SignPSBTInput`, but the `SigningProvider` is internal to `LegacyScriptPubKeyMan`. Other `ScriptPubKeyMan`s may do something different.

  A new `SignMessage()` function is added to both `CWallet` and `ScriptPubKeyMan`. Instead of having the caller (i.e. `signmessage` or the sign message dialog) get the private key, hash the message, and sign, `ScriptPubKeyMan` will now handle that (`CWallet` passes through to the `ScriptPubKeyMan`s as it does for many functions). This signing code is thus consolidated into `LegacyScriptPubKeyMan::SignMessage()`, though other `ScriptPubKeyMan`s may implement it differently. Additionally, a `SigningError` enum is introduced for the different errors that we expect to see from `SignMessage()`.

  Lastly, `GetSigningProvider()` is renamed to `GetPublicSigningProvider()`. It will now only provide pubkeys, key origins, and scripts. `LegacySigningProvider` has it's `GetKey` and `HaveKey` functions changed to only return false. Future implementations should return `HidingSigningProvider`s where private keys are hidden.

  Other things like `dumpprivkey` and `dumpwallet` are not changed because they directly need and access the `LegacyScriptPubKeyMan` so are not relevant to future changes.

ACKs for top commit:
  instagibbs:
    reACK d2774c09cf
  Sjors:
    re-utACK d2774c09cf
  meshcollider:
    re-utACK d2774c09cf

Tree-SHA512: 89c83e7e7e9315e283fae145a2264648a9d7f7ace8f3281cb3f44f0b013c988d67ba4fa9726e50c643c0ed921bdd269adaec984840d11acf4a681f3e8a582cc1
This commit is contained in:
Samuel Dobson
2020-03-10 08:56:38 +13:00
parent ffdde475b0 d2774c09cf
commit dcf2ccbfde
19 changed files with 433 additions and 276 deletions
Download Patch File Download Diff File Expand all files Collapse all files

57
src/wallet/rpcwallet.cpp
View File

@@ -27,7 +27,6 @@
#include <util/vector.h>
#include <wallet/coincontrol.h>
#include <wallet/feebumper.h>
#include <wallet/psbtwallet.h>
#include <wallet/rpcwallet.h>
#include <wallet/wallet.h>
#include <wallet/walletdb.h>
@@ -566,22 +565,12 @@ static UniValue signmessage(const JSONRPCRequest& request)
throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to key");
}
CScript script_pub_key = GetScriptForDestination(*pkhash);
std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(script_pub_key);
if (!provider) {
throw JSONRPCError(RPC_WALLET_ERROR, "Private key not available");
}
CKey key;
CKeyID keyID(*pkhash);
if (!provider->GetKey(keyID, key)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Private key not available");
}
std::string signature;
if (!MessageSign(key, strMessage, signature)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Sign failed");
SigningResult err = pwallet->SignMessage(strMessage, *pkhash, signature);
if (err == SigningResult::SIGNING_FAILED) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, SigningResultString(err));
} else if (err != SigningResult::OK){
throw JSONRPCError(RPC_WALLET_ERROR, SigningResultString(err));
}
return signature;
@@ -2973,7 +2962,7 @@ static UniValue listunspent(const JSONRPCRequest& request)
entry.pushKV("label", i->second.name);
}
std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(scriptPubKey);
std::unique_ptr<SigningProvider> provider = pwallet->GetSolvingProvider(scriptPubKey);
if (provider) {
if (scriptPubKey.IsPayToScriptHash()) {
const CScriptID& hash = CScriptID(boost::get<ScriptHash>(address));
@@ -3013,7 +3002,7 @@ static UniValue listunspent(const JSONRPCRequest& request)
entry.pushKV("spendable", out.fSpendable);
entry.pushKV("solvable", out.fSolvable);
if (out.fSolvable) {
std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(scriptPubKey);
std::unique_ptr<SigningProvider> provider = pwallet->GetSolvingProvider(scriptPubKey);
if (provider) {
auto descriptor = InferDescriptor(scriptPubKey, *provider);
entry.pushKV("desc", descriptor->ToString());
@@ -3329,23 +3318,15 @@ UniValue signrawtransactionwithwallet(const JSONRPCRequest& request)
// Parse the prevtxs array
ParsePrevouts(request.params[1], nullptr, coins);
std::set<std::shared_ptr<SigningProvider>> providers;
for (const std::pair<COutPoint, Coin> coin_pair : coins) {
std::unique_ptr<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 a dummy SigningProvider so we can check if the tx is complete
providers.insert(std::make_shared<SigningProvider>());
}
int nHashType = ParseSighashString(request.params[2]);
// Script verification errors
std::map<int, std::string> input_errors;
bool complete = pwallet->SignTransaction(mtx, coins, nHashType, input_errors);
UniValue result(UniValue::VOBJ);
for (std::shared_ptr<SigningProvider> provider : providers) {
SignTransaction(mtx, provider.get(), coins, request.params[2], result);
}
return result;
SignTransactionResultToJSON(mtx, complete, coins, input_errors, result);
return result;
}
static UniValue bumpfee(const JSONRPCRequest& request)
@@ -3524,7 +3505,7 @@ static UniValue bumpfee(const JSONRPCRequest& request)
} else {
PartiallySignedTransaction psbtx(mtx);
bool complete = false;
const TransactionError err = FillPSBT(pwallet, psbtx, complete, SIGHASH_ALL, false /* sign */, true /* bip32derivs */);
const TransactionError err = pwallet->FillPSBT(psbtx, complete, SIGHASH_ALL, false /* sign */, true /* bip32derivs */);
CHECK_NONFATAL(err == TransactionError::OK);
CHECK_NONFATAL(!complete);
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
@@ -3735,7 +3716,7 @@ static UniValue DescribeWalletAddress(const CWallet* const pwallet, const CTxDes
CScript script = GetScriptForDestination(dest);
std::unique_ptr<SigningProvider> provider = nullptr;
if (pwallet) {
provider = pwallet->GetSigningProvider(script);
provider = pwallet->GetSolvingProvider(script);
}
ret.pushKVs(detail);
ret.pushKVs(boost::apply_visitor(DescribeWalletAddressVisitor(provider.get()), dest));
@@ -3837,7 +3818,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
CScript scriptPubKey = GetScriptForDestination(dest);
ret.pushKV("scriptPubKey", HexStr(scriptPubKey.begin(), scriptPubKey.end()));
std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(scriptPubKey);
std::unique_ptr<SigningProvider> provider = pwallet->GetSolvingProvider(scriptPubKey);
isminetype mine = pwallet->IsMine(dest);
ret.pushKV("ismine", bool(mine & ISMINE_SPENDABLE));
@@ -4141,7 +4122,7 @@ UniValue walletprocesspsbt(const JSONRPCRequest& request)
bool sign = request.params[1].isNull() ? true : request.params[1].get_bool();
bool bip32derivs = request.params[3].isNull() ? true : request.params[3].get_bool();
bool complete = true;
const TransactionError err = FillPSBT(pwallet, psbtx, complete, nHashType, sign, bip32derivs);
const TransactionError err = pwallet->FillPSBT(psbtx, complete, nHashType, sign, bip32derivs);
if (err != TransactionError::OK) {
throw JSONRPCTransactionError(err);
}
@@ -4264,7 +4245,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request)
// Fill transaction with out data but don't sign
bool bip32derivs = request.params[4].isNull() ? true : request.params[4].get_bool();
bool complete = true;
const TransactionError err = FillPSBT(pwallet, psbtx, complete, 1, false, bip32derivs);
const TransactionError err = pwallet->FillPSBT(psbtx, complete, 1, false, bip32derivs);
if (err != TransactionError::OK) {
throw JSONRPCTransactionError(err);
}