aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/rpc/rawtransaction.cpp13
-rw-r--r--src/script/sign.cpp49
-rw-r--r--src/script/sign.h6
-rw-r--r--src/wallet/rpcwallet.cpp65
-rw-r--r--src/wallet/rpcwallet.h2
-rw-r--r--src/wallet/test/psbt_wallet_tests.cpp6
-rwxr-xr-xtest/functional/rpc_psbt.py7
7 files changed, 80 insertions, 68 deletions
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index 51b4e44aed..94a9ea20da 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -1538,12 +1538,13 @@ 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);
-
- complete &= SignPSBTInput(DUMMY_SIGNING_PROVIDER, *psbtx.tx, input, i, 1);
+ complete &= SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, SIGHASH_ALL);
}
UniValue result(UniValue::VOBJ);
@@ -1556,10 +1557,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 89cc7c808c..69ee08ffd7 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -239,10 +239,17 @@ bool ProduceSignature(const SigningProvider& provider, const BaseSignatureCreato
return sigdata.complete;
}
-bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& tx, PSBTInput& input, 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, int index, int sighash)
+{
+ PSBTInput& input = psbt.inputs.at(index);
+ const CMutableTransaction& tx = *psbt.tx;
+
+ if (PSBTInputSigned(input)) {
return true;
}
@@ -253,15 +260,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
@@ -280,18 +291,10 @@ bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& t
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) {
- assert(!utxo.IsNull());
input.witness_utxo = utxo;
- }
-
- // 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();
- }
+ input.non_witness_utxo = nullptr;
}
return sig_complete;
@@ -513,6 +516,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 d47aada17d..e71f43f96d 100644
--- a/src/script/sign.h
+++ b/src/script/sign.h
@@ -566,6 +566,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)
@@ -729,8 +730,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, int index, int sighash = SIGHASH_ALL);
+bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, 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 5a89448e02..bfac990639 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -3772,24 +3772,34 @@ void AddKeypathToMap(const CWallet* pwallet, const CKeyID& keyID, std::map<CPubK
hd_keypaths.emplace(vchPubKey, std::move(info));
}
-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
@@ -3797,12 +3807,12 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, const C
throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Specified Sighash and sighash in PSBT do not match.");
}
- complete &= SignPSBTInput(HidingSigningProvider(pwallet, !sign, !bip32derivs), *psbtx.tx, input, i, sighash_type);
+ complete &= SignPSBTInput(HidingSigningProvider(pwallet, !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 < 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);
// Fill a SignatureData with output info
@@ -3867,19 +3877,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;
@@ -3971,29 +3977,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 cb1ad25461..9918eeb89f 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);
diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py
index fca910bf64..86b043176c 100755
--- a/test/functional/rpc_psbt.py
+++ b/test/functional/rpc_psbt.py
@@ -207,6 +207,13 @@ class PSBTTest(BitcoinTestFramework):
assert tx_in["sequence"] > MAX_BIP125_RBF_SEQUENCE
assert_equal(decoded_psbt["tx"]["locktime"], 0)
+ # Regression test for 14473 (mishandling of already-signed witness transaction):
+ psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}])
+ complete_psbt = self.nodes[0].walletprocesspsbt(psbtx_info["psbt"])
+ double_processed_psbt = self.nodes[0].walletprocesspsbt(complete_psbt["psbt"])
+ assert_equal(complete_psbt, double_processed_psbt)
+ # We don't care about the decode result, but decoding must succeed.
+ self.nodes[0].decodepsbt(double_processed_psbt["psbt"])
# BIP 174 Test Vectors
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-06 02:13:45 +0000