diff options
Diffstat (limited to 'src')
-rw-r--r-- | src/rpc/rawtransaction.cpp | 13 | ||||
-rw-r--r-- | src/script/sign.cpp | 44 | ||||
-rw-r--r-- | src/script/sign.h | 36 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 76 | ||||
-rw-r--r-- | src/wallet/rpcwallet.h | 2 | ||||
-rw-r--r-- | src/wallet/test/psbt_wallet_tests.cpp | 6 |
6 files changed, 106 insertions, 71 deletions
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 314184ab06..7de3638f4e 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1643,13 +1643,14 @@ UniValue finalizepsbt(const JSONRPCRequest& request) throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed %s", error)); } - // Get all of the previous transactions + // Finalize input signatures -- in case we have partial signatures that add up to a complete + // signature, but have not combined them yet (e.g. because the combiner that created this + // PartiallySignedTransaction did not understand them), this will combine them into a final + // script. bool complete = true; 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, sigdata, i, SIGHASH_ALL); } UniValue result(UniValue::VOBJ); @@ -1662,10 +1663,10 @@ UniValue finalizepsbt(const JSONRPCRequest& request) mtx.vin[i].scriptWitness = psbtx.inputs[i].final_script_witness; } ssTx << mtx; - result.pushKV("hex", HexStr(ssTx.begin(), ssTx.end())); + result.pushKV("hex", HexStr(ssTx.str())); } else { ssTx << psbtx; - result.pushKV("psbt", EncodeBase64((unsigned char*)ssTx.data(), ssTx.size())); + result.pushKV("psbt", EncodeBase64(ssTx.str())); } result.pushKV("complete", complete); diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 1ab5051ff7..f97d6a2533 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -233,10 +233,17 @@ 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 PSBTInputSigned(PSBTInput& input) { - // 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()) { + return !input.final_script_sig.empty() || !input.final_script_witness.IsNull(); +} + +bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, SignatureData& sigdata, int index, int sighash) +{ + PSBTInput& input = psbt.inputs.at(index); + const CMutableTransaction& tx = *psbt.tx; + + if (PSBTInputSigned(input)) { return true; } @@ -246,15 +253,19 @@ bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& t // Get UTXO bool require_witness_sig = false; CTxOut utxo; + + // Verify input sanity, which checks that at most one of witness or non-witness utxos is provided. + if (!input.IsSane()) { + return false; + } + if (input.non_witness_utxo) { // If we're taking our information from a non-witness UTXO, verify that it matches the prevout. - if (input.non_witness_utxo->GetHash() != tx.vin[index].prevout.hash) return false; - // If both witness and non-witness UTXO are provided, verify that they match. This check shouldn't - // matter, as the PSBT deserializer enforces only one of both is provided, and the only way both - // can be present is when they're added simultaneously by FillPSBT (in which case they always match). - // Still, check in order to not rely on callers to enforce this. - if (!input.witness_utxo.IsNull() && input.non_witness_utxo->vout[tx.vin[index].prevout.n] != input.witness_utxo) return false; - utxo = input.non_witness_utxo->vout[tx.vin[index].prevout.n]; + COutPoint prevout = tx.vin[index].prevout; + if (input.non_witness_utxo->GetHash() != prevout.hash) { + return false; + } + utxo = input.non_witness_utxo->vout[prevout.n]; } else if (!input.witness_utxo.IsNull()) { utxo = input.witness_utxo; // When we're taking our information from a witness UTXO, we can't verify it is actually data from @@ -272,6 +283,13 @@ 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 we have a witness signature, use the smaller witness UTXO. + if (sigdata.witness) { + input.witness_utxo = utxo; + input.non_witness_utxo = nullptr; + } + return sig_complete; } @@ -491,6 +509,12 @@ bool IsSolvable(const SigningProvider& provider, const CScript& script) return false; } +PartiallySignedTransaction::PartiallySignedTransaction(const CTransaction& tx) : tx(tx) +{ + inputs.resize(tx.vin.size()); + outputs.resize(tx.vout.size()); +} + bool PartiallySignedTransaction::IsNull() const { return !tx && inputs.empty() && outputs.empty() && unknown.empty(); diff --git a/src/script/sign.h b/src/script/sign.h index 245b15410f..f24d77cb31 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -286,6 +286,7 @@ struct PSBTInput template <typename Stream> inline void Unserialize(Stream& s) { // Read loop + bool found_sep = false; while(!s.empty()) { // Read std::vector<unsigned char> key; @@ -293,7 +294,10 @@ struct PSBTInput // the key is empty if that was actually a separator byte // This is a special case for key lengths 0 as those are not allowed (except for separator) - if (key.empty()) return; + if (key.empty()) { + found_sep = true; + break; + } // First byte of key is the type unsigned char type = key[0]; @@ -408,6 +412,10 @@ struct PSBTInput break; } } + + if (!found_sep) { + throw std::ios_base::failure("Separator is missing at the end of an input map"); + } } template <typename Stream> @@ -461,6 +469,7 @@ struct PSBTOutput template <typename Stream> inline void Unserialize(Stream& s) { // Read loop + bool found_sep = false; while(!s.empty()) { // Read std::vector<unsigned char> key; @@ -468,7 +477,10 @@ struct PSBTOutput // the key is empty if that was actually a separator byte // This is a special case for key lengths 0 as those are not allowed (except for separator) - if (key.empty()) return; + if (key.empty()) { + found_sep = true; + break; + } // First byte of key is the type unsigned char type = key[0]; @@ -513,6 +525,10 @@ struct PSBTOutput } } } + + if (!found_sep) { + throw std::ios_base::failure("Separator is missing at the end of an output map"); + } } template <typename Stream> @@ -534,6 +550,7 @@ struct PartiallySignedTransaction bool IsSane() const; PartiallySignedTransaction() {} PartiallySignedTransaction(const PartiallySignedTransaction& psbt_in) : tx(psbt_in.tx), inputs(psbt_in.inputs), outputs(psbt_in.outputs), unknown(psbt_in.unknown) {} + explicit PartiallySignedTransaction(const CTransaction& tx); // Only checks if they refer to the same transaction friend bool operator==(const PartiallySignedTransaction& a, const PartiallySignedTransaction &b) @@ -588,6 +605,7 @@ struct PartiallySignedTransaction } // Read global data + bool found_sep = false; while(!s.empty()) { // Read std::vector<unsigned char> key; @@ -595,7 +613,10 @@ struct PartiallySignedTransaction // the key is empty if that was actually a separator byte // This is a special case for key lengths 0 as those are not allowed (except for separator) - if (key.empty()) break; + if (key.empty()) { + found_sep = true; + break; + } // First byte of key is the type unsigned char type = key[0]; @@ -635,6 +656,10 @@ struct PartiallySignedTransaction } } + if (!found_sep) { + throw std::ios_base::failure("Separator is missing at the end of the global map"); + } + // Make sure that we got an unsigned tx if (!tx) { throw std::ios_base::failure("No unsigned transcation was provided"); @@ -689,8 +714,11 @@ bool ProduceSignature(const SigningProvider& provider, const BaseSignatureCreato bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType); bool SignSignature(const SigningProvider &provider, const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType); +/** Checks whether a PSBTInput is already signed. */ +bool PSBTInputSigned(PSBTInput& input); + /** 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, PartiallySignedTransaction& psbt, SignatureData& sigdata, 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 0a9242327b..6400b4470f 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4502,24 +4502,34 @@ void AddKeypathToMap(const CWallet* pwallet, const CKeyID& keyID, std::map<CPubK hd_keypaths.emplace(vchPubKey, keypath); } -bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, const CTransaction* txConst, int sighash_type, bool sign, bool bip32derivs) +bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, int sighash_type, bool sign, bool bip32derivs) { LOCK(pwallet->cs_wallet); // Get all of the previous transactions bool complete = true; - for (unsigned int i = 0; i < txConst->vin.size(); ++i) { - const CTxIn& txin = txConst->vin[i]; + for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { + const CTxIn& txin = psbtx.tx->vin[i]; PSBTInput& input = psbtx.inputs.at(i); - // If we don't know about this input, skip it and let someone else deal with it - const uint256& txhash = txin.prevout.hash; - const auto it = pwallet->mapWallet.find(txhash); - if (it != pwallet->mapWallet.end()) { - const CWalletTx& wtx = it->second; - CTxOut utxo = wtx.tx->vout[txin.prevout.n]; - // Update both UTXOs from the wallet. - input.non_witness_utxo = wtx.tx; - input.witness_utxo = utxo; + if (PSBTInputSigned(input)) { + continue; + } + + // Verify input looks sane. This will check that we have at most one uxto, witness or non-witness. + if (!input.IsSane()) { + throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "PSBT input is not sane."); + } + + // If we have no utxo, grab it from the wallet. + if (!input.non_witness_utxo && input.witness_utxo.IsNull()) { + const uint256& txhash = txin.prevout.hash; + const auto it = pwallet->mapWallet.find(txhash); + if (it != pwallet->mapWallet.end()) { + const CWalletTx& wtx = it->second; + // We only need the non_witness_utxo, which is a superset of the witness_utxo. + // The signing code will switch to the smaller witness_utxo if this is ok. + input.non_witness_utxo = wtx.tx; + } } // Get the Sighash type @@ -4529,9 +4539,9 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, const C SignatureData sigdata; if (sign) { - complete &= SignPSBTInput(*pwallet, *psbtx.tx, input, sigdata, i, sighash_type); + complete &= SignPSBTInput(*pwallet, psbtx, sigdata, i, sighash_type); } else { - complete &= SignPSBTInput(PublicOnlySigningProvider(pwallet), *psbtx.tx, input, sigdata, i, sighash_type); + complete &= SignPSBTInput(PublicOnlySigningProvider(pwallet), psbtx, sigdata, i, sighash_type); } if (sigdata.witness) { @@ -4541,15 +4551,6 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, const C } } - // 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(); - } - } - // Get public key paths if (bip32derivs) { for (const auto& pubkey_it : sigdata.misc_pubkeys) { @@ -4559,8 +4560,8 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, const C } // Fill in the bip32 keypaths and redeemscripts for the outputs so that hardware wallets can identify change - for (unsigned int i = 0; i < txConst->vout.size(); ++i) { - const CTxOut& out = txConst->vout.at(i); + for (unsigned int i = 0; i < psbtx.tx->vout.size(); ++i) { + const CTxOut& out = psbtx.tx->vout.at(i); PSBTOutput& psbt_out = psbtx.outputs.at(i); // Dummy tx so we can use ProduceSignature to get stuff out @@ -4637,19 +4638,15 @@ UniValue walletprocesspsbt(const JSONRPCRequest& request) // Get the sighash type int nHashType = ParseSighashString(request.params[2]); - // Use CTransaction for the constant parts of the - // transaction to avoid rehashing. - const CTransaction txConst(*psbtx.tx); - // Fill transaction with our data and also sign bool sign = request.params[1].isNull() ? true : request.params[1].get_bool(); bool bip32derivs = request.params[3].isNull() ? false : request.params[3].get_bool(); - bool complete = FillPSBT(pwallet, psbtx, &txConst, nHashType, sign, bip32derivs); + bool complete = FillPSBT(pwallet, psbtx, nHashType, sign, bip32derivs); UniValue result(UniValue::VOBJ); CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); ssTx << psbtx; - result.pushKV("psbt", EncodeBase64((unsigned char*)ssTx.data(), ssTx.size())); + result.pushKV("psbt", EncodeBase64(ssTx.str())); result.pushKV("complete", complete); return result; @@ -4741,29 +4738,18 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) FundTransaction(pwallet, rawTx, fee, change_position, request.params[3]); // Make a blank psbt - PartiallySignedTransaction psbtx; - psbtx.tx = rawTx; - for (unsigned int i = 0; i < rawTx.vin.size(); ++i) { - psbtx.inputs.push_back(PSBTInput()); - } - for (unsigned int i = 0; i < rawTx.vout.size(); ++i) { - psbtx.outputs.push_back(PSBTOutput()); - } - - // Use CTransaction for the constant parts of the - // transaction to avoid rehashing. - const CTransaction txConst(*psbtx.tx); + PartiallySignedTransaction psbtx(rawTx); // Fill transaction with out data but don't sign bool bip32derivs = request.params[4].isNull() ? false : request.params[4].get_bool(); - FillPSBT(pwallet, psbtx, &txConst, 1, false, bip32derivs); + FillPSBT(pwallet, psbtx, 1, false, bip32derivs); // Serialize the PSBT CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); ssTx << psbtx; UniValue result(UniValue::VOBJ); - result.pushKV("psbt", EncodeBase64((unsigned char*)ssTx.data(), ssTx.size())); + result.pushKV("psbt", EncodeBase64(ssTx.str())); result.pushKV("fee", ValueFromAmount(fee)); result.pushKV("changepos", change_position); return result; diff --git a/src/wallet/rpcwallet.h b/src/wallet/rpcwallet.h index 9b9a159b86..abd7750874 100644 --- a/src/wallet/rpcwallet.h +++ b/src/wallet/rpcwallet.h @@ -30,5 +30,5 @@ bool EnsureWalletIsAvailable(CWallet *, bool avoidException); UniValue getaddressinfo(const JSONRPCRequest& request); UniValue signrawtransactionwithwallet(const JSONRPCRequest& request); -bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, const CTransaction* txConst, int sighash_type = 1, bool sign = true, bool bip32derivs = false); +bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false); #endif //BITCOIN_WALLET_RPCWALLET_H diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index 61c3fa94a6..8d46718a7d 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -59,12 +59,8 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test) CDataStream ssData(ParseHex("70736274ff01009a020000000258e87a21b56daf0c23be8e7070456c336f7cbaa5c8757924f545887bb2abdd750000000000ffffffff838d0427d0ec650a68aa46bb0b098aea4422c071b2ca78352a077959d07cea1d0100000000ffffffff0270aaf00800000000160014d85c2b71d0060b09c9886aeb815e50991dda124d00e1f5050000000016001400aea9a2e5f0f876a588df5546e8742d1d87008f000000000000000000"), SER_NETWORK, PROTOCOL_VERSION); ssData >> psbtx; - // Use CTransaction for the constant parts of the - // transaction to avoid rehashing. - const CTransaction txConst(*psbtx.tx); - // Fill transaction with our data - FillPSBT(&m_wallet, psbtx, &txConst, 1, false, true); + FillPSBT(&m_wallet, psbtx, SIGHASH_ALL, false, true); // Get the final tx CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); |