From 43811e63380d803e037de69dc0567aae590fa109 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 13 Aug 2018 14:59:31 -0700 Subject: Fix PSBT deserialization of 0-input transactions 0-input transactions can be ambiguously deserialized as being witness transactions. Since the unsigned transaction is never serialized as a witness transaction as it has no witnesses, we should always deserialize it as a non-witness transaction and set the serialization flags as such. Also always serialize the unsigned transaction as a non-witness transaction. --- src/script/sign.h | 7 +++++-- src/streams.h | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/script/sign.h b/src/script/sign.h index 24cddda51b..86188cbb38 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -547,7 +547,8 @@ struct PartiallySignedTransaction SerializeToVector(s, PSBT_GLOBAL_UNSIGNED_TX); // Write serialized tx to a stream - SerializeToVector(s, *tx); + OverrideStream os(&s, s.GetType(), s.GetVersion() | SERIALIZE_TRANSACTION_NO_WITNESS); + SerializeToVector(os, *tx); // Write the unknown things for (auto& entry : unknown) { @@ -601,7 +602,9 @@ struct PartiallySignedTransaction throw std::ios_base::failure("Global unsigned tx key is more than one byte type"); } CMutableTransaction mtx; - UnserializeFromVector(s, mtx); + // Set the stream to serialize with non-witness since this should always be non-witness + OverrideStream os(&s, s.GetType(), s.GetVersion() | SERIALIZE_TRANSACTION_NO_WITNESS); + UnserializeFromVector(os, mtx); tx = std::move(mtx); // Make sure that all scriptSigs and scriptWitnesses are empty for (const CTxIn& txin : tx->vin) { diff --git a/src/streams.h b/src/streams.h index 2dcca6646d..096ebfc9c2 100644 --- a/src/streams.h +++ b/src/streams.h @@ -61,6 +61,7 @@ public: int GetVersion() const { return nVersion; } int GetType() const { return nType; } + size_t size() const { return stream->size(); } }; template -- cgit v1.2.3 From bd19cc78cfc455cf06e120adb0d12c2f96ba8fca Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 13 Aug 2018 15:00:06 -0700 Subject: Serialize non-witness utxo as a non-witness tx but always deserialize as witness Strip out the witnesses when serializing the non-witness utxo. However witness serializations are allowed, so make sure we always deserialize as witness. --- src/script/sign.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/script/sign.h b/src/script/sign.h index 86188cbb38..80fda617e9 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -223,7 +223,8 @@ struct PSBTInput // If there is a non-witness utxo, then don't add the witness one. if (non_witness_utxo) { SerializeToVector(s, PSBT_IN_NON_WITNESS_UTXO); - SerializeToVector(s, non_witness_utxo); + OverrideStream os(&s, s.GetType(), s.GetVersion() | SERIALIZE_TRANSACTION_NO_WITNESS); + SerializeToVector(os, non_witness_utxo); } else if (!witness_utxo.IsNull()) { SerializeToVector(s, PSBT_IN_WITNESS_UTXO); SerializeToVector(s, witness_utxo); @@ -297,13 +298,17 @@ struct PSBTInput // Do stuff based on type switch(type) { case PSBT_IN_NON_WITNESS_UTXO: + { if (non_witness_utxo) { throw std::ios_base::failure("Duplicate Key, input non-witness utxo already provided"); } else if (key.size() != 1) { throw std::ios_base::failure("Non-witness utxo key is more than one byte type"); } - UnserializeFromVector(s, non_witness_utxo); + // Set the stream to unserialize with witness since this is always a valid network transaction + OverrideStream os(&s, s.GetType(), s.GetVersion() & ~SERIALIZE_TRANSACTION_NO_WITNESS); + UnserializeFromVector(os, non_witness_utxo); break; + } case PSBT_IN_WITNESS_UTXO: if (!witness_utxo.IsNull()) { throw std::ios_base::failure("Duplicate Key, input witness utxo already provided"); -- cgit v1.2.3