From 70ee1f8709a54a9aeac004d8589faa08f665587a Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Fri, 26 Oct 2018 15:26:16 -0700 Subject: New PartiallySignedTransaction constructor from CTransction New constructor that creates a PartiallySignedTransaction from a CTransaction, automatically sizing the inputs and outputs vectors for convenience. Github-Pull: #14588 Rebased-From: 65166d4cf828909dc4bc49dd68a58103d015f1fd --- src/script/sign.cpp | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src/script/sign.cpp') diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 1ab5051ff..190c0bf68 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -491,6 +491,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(); -- cgit v1.2.3 From 39ece4fc2807b3442759d879985146ac0de9742d Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Fri, 26 Oct 2018 15:28:48 -0700 Subject: Add bool PSBTInputSigned Refactor out a "PSBTInputSigned" function to check if a PSBT is signed, for use in subsequent commits. Also improve a related comment. GitHub-Pull: #14588 Rebased-From: 53e6fffb8f5b10f94708d33d667a67cb91c2d09d --- src/script/sign.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'src/script/sign.cpp') diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 190c0bf68..945e16235 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -233,10 +233,14 @@ bool ProduceSignature(const SigningProvider& provider, const BaseSignatureCreato return sigdata.complete; } +bool PSBTInputSigned(PSBTInput& input) +{ + return !input.final_script_sig.empty() || !input.final_script_witness.IsNull(); +} + bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& tx, PSBTInput& input, SignatureData& sigdata, int index, int sighash) { - // 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()) { + if (PSBTInputSigned(input)) { return true; } -- cgit v1.2.3 From ad94165db91c0416634459e18f173c5cd063dc55 Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Fri, 26 Oct 2018 15:30:50 -0700 Subject: Simplify arguments to SignPSBTInput Remove redundant arguments to SignPSBTInput -- since it needs several bits of the PartiallySignedTransaction, pass in a reference instead of doing it piecemeal. This saves us having to pass in both a PSBTInput and its index, as well as having to pass in the CTransaction. Also avoid redundantly passing the sighash_type, which is contained in the PSBTInput already. Github-Pull: #14588 Rebased-From: 0f5bda2bd941686620ef0eb90bd7ed973cc7ef73 --- src/script/sign.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'src/script/sign.cpp') diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 945e16235..8bc142f6e 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -238,8 +238,11 @@ bool PSBTInputSigned(PSBTInput& input) return !input.final_script_sig.empty() || !input.final_script_witness.IsNull(); } -bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& tx, PSBTInput& input, SignatureData& sigdata, int index, int sighash) +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; } -- cgit v1.2.3 From db445d4e5a25ff13de014dbbf43bb99cde8b8769 Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Fri, 26 Oct 2018 15:31:41 -0700 Subject: Refactor PSBTInput signing to enforce invariant Refactor the process of PSBTInput signing to enforce the invariant that a PSBTInput always has _either_ a witness_utxo or a non_witness_utxo, never both. This simplifies the logic of SignPSBTInput slightly, since it no longer has to deal with the "both" case. When calling it, we now give it, in order of preference: (1) whichever of the utxo fields was already present in the PSBT we received, or (2) if neither, the non_witness_utxo field, which is just a copy of the input transaction, which we get from the wallet. SignPSBTInput no longer has to remove one of the two fields; instead, it will check if we have a witness signature, and if so, it will replace the non_witness_utxo with the witness_utxo (which is smaller, as it is just a copy of the output being spent.) Add PSBTInput::IsSane checks in two more places, which checks for both utxo fields being present; we will now give an RPC error early on if we are supplied such a malformed PSBT to fill in. Also add a check to FillPSBT, to avoid touching any input that is already signed. (This is now redundant, since we should no longer potentially harm an already-signed input, but it's harmless.) fixes #14473 Github-Pull: #14588 --- src/script/sign.cpp | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) (limited to 'src/script/sign.cpp') diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 8bc142f6e..f97d6a253 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -253,15 +253,19 @@ bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& // 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 @@ -279,6 +283,13 @@ bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& // 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; } -- cgit v1.2.3