Merge rust-bitcoin/rust-bitcoin#988: Replace consensus `Encodable`/`Decodable` Psbt Serialization

c4363e5ba1 Deserialize Psbt fields, don't consensus_encode (DanGould)
c1dd6ad8a2 Serialize Psbt fields, don't consensus_encode them (DanGould)
1b7b08aa5d De/serialize Psbt without consensus traits (DanGould)

Pull request description:

  fix https://github.com/rust-bitcoin/rust-bitcoin/issues/934

  Instead of using consensus {de,en}code, serialize high-level structures (PartiallySignedTransaciton, Output, Input) borrow the signature from `Serialize`, `Deserialize` traits and implement them on Psbt:

  ```rs
  impl Psbt {
      /// Serialize a value as raw data.
      fn serialize(&self) -> Vec<u8>;

      /// Deserialize a value from raw data.
      fn deserialize(bytes: &[u8]) -> Result<Self, encode::Error>;
  }
  ```

ACKs for top commit:
  apoelstra:
    ACK c4363e5ba1
  tcharding:
    ACK c4363e5ba1
  sanket1729:
    ACK c4363e5ba1. One small comment, but can be addressed in follow up if there is nothing else from other reviewers.
  Kixunil:
    ACK c4363e5ba1

Tree-SHA512: d8dd5ef1189b36fde08969b1ec36006a6fc55ae990f62ea744e6669e03a7f9e90e1d5907be7eac48ee1af23bc20a62aa7e60ff1ff78080d0e923bb9ccedcd432

bitcoin/examples/ecdsa-psbt.rs

@@ -236,16 +236,14 @@ impl WatchOnly {
     /// Finalizes the PSBT, in BIP174 parlance this is the 'Finalizer'.
     /// This is just an example. For a production-ready PSBT Finalizer, use [rust-miniscript](https://docs.rs/miniscript/latest/miniscript/psbt/trait.PsbtExt.html#tymethod.finalize)
     fn finalize_psbt(&self, mut psbt: Psbt) -> Result<Psbt> {
-        use bitcoin::psbt::serialize::Serialize;
-
         if psbt.inputs.is_empty() {
             return Err(psbt::SignError::MissingInputUtxo.into());
         }
 
         let sigs: Vec<_> = psbt.inputs[0].partial_sigs.values().collect();
         let mut script_witness: Witness = Witness::new();
-        script_witness.push(&sigs[0].serialize());
+        script_witness.push(&sigs[0].to_vec());
-        script_witness.push(self.input_xpub.to_pub().serialize());
+        script_witness.push(self.input_xpub.to_pub().to_bytes());
 
         psbt.inputs[0].final_script_witness = Some(script_witness);
 

bitcoin/examples/taproot-psbt.rs

@@ -85,7 +85,6 @@ use bitcoin::hashes::hex::FromHex;
 use bitcoin::hashes::Hash;
 use bitcoin::key::XOnlyPublicKey;
 use bitcoin::opcodes::all::{OP_CHECKSIG, OP_CLTV, OP_DROP};
-use bitcoin::psbt::serialize::Serialize;
 use bitcoin::psbt::{self, Input, Output, Psbt, PsbtSighashType};
 use bitcoin::schnorr::{self, TapTweak};
 use bitcoin::secp256k1::{Message, Secp256k1};
@@ -317,7 +316,7 @@ fn generate_bip86_key_spend_tx(
     // FINALIZER
     psbt.inputs.iter_mut().for_each(|input| {
         let mut script_witness: Witness = Witness::new();
-        script_witness.push(input.tap_key_sig.unwrap().serialize());
+        script_witness.push(input.tap_key_sig.unwrap().to_vec());
         input.final_script_witness = Some(script_witness);
 
         // Clear all the data fields as per the spec.
@@ -550,7 +549,7 @@ impl BenefactorWallet {
             // FINALIZER
             psbt.inputs.iter_mut().for_each(|input| {
                 let mut script_witness: Witness = Witness::new();
-                script_witness.push(input.tap_key_sig.unwrap().serialize());
+                script_witness.push(input.tap_key_sig.unwrap().to_vec());
                 input.final_script_witness = Some(script_witness);
 
                 // Clear all the data fields as per the spec.
@@ -686,10 +685,10 @@ impl BeneficiaryWallet {
         psbt.inputs.iter_mut().for_each(|input| {
             let mut script_witness: Witness = Witness::new();
             for (_, signature) in input.tap_script_sigs.iter() {
-                script_witness.push(signature.serialize());
+                script_witness.push(signature.to_vec());
             }
             for (control_block, (script, _)) in input.tap_scripts.iter() {
-                script_witness.push(script.serialize());
+                script_witness.push(script.to_bytes());
                 script_witness.push(control_block.serialize());
             }
             input.final_script_witness = Some(script_witness);

bitcoin/fuzz/fuzz_targets/deserialize_psbt.rs

@@ -1,14 +1,14 @@
 extern crate bitcoin;
 
 fn do_test(data: &[u8]) {
-    let psbt: Result<bitcoin::psbt::PartiallySignedTransaction, _> = bitcoin::consensus::encode::deserialize(data);
+    let psbt: Result<bitcoin::psbt::PartiallySignedTransaction, _> = bitcoin::psbt::Psbt::deserialize(data);
     match psbt {
         Err(_) => {},
         Ok(psbt) => {
-            let ser = bitcoin::consensus::encode::serialize(&psbt);
+            let ser = bitcoin::psbt::Psbt::serialize(&psbt);
-            let deser: bitcoin::psbt::PartiallySignedTransaction  = bitcoin::consensus::encode::deserialize(&ser).unwrap();
+            let deser = bitcoin::psbt::Psbt::deserialize(&ser).unwrap();
             // Since the fuzz data could order psbt fields differently, we compare to our deser/ser instead of data
-            assert_eq!(ser, bitcoin::consensus::encode::serialize(&deser));
+            assert_eq!(ser, bitcoin::psbt::Psbt::serialize(&deser));
         }
     }
 }

bitcoin/src/psbt/macros.rs

@@ -2,7 +2,7 @@
 
 #[allow(unused_macros)]
 macro_rules! hex_psbt {
-    ($s:expr) => { $crate::consensus::deserialize::<$crate::psbt::PartiallySignedTransaction>(&<$crate::prelude::Vec<u8> as $crate::hashes::hex::FromHex>::from_hex($s).unwrap()) };
+    ($s:expr) => { <$crate::psbt::PartiallySignedTransaction>::deserialize(&<$crate::prelude::Vec<u8> as $crate::hashes::hex::FromHex>::from_hex($s).unwrap()) };
 }
 
 macro_rules! combine {
@@ -40,29 +40,37 @@ macro_rules! impl_psbt_serialize {
     };
 }
 
-macro_rules! impl_psbtmap_consensus_encoding {
+macro_rules! impl_psbtmap_serialize {
     ($thing:ty) => {
-        impl $crate::consensus::Encodable for $thing {
+        impl $crate::psbt::serialize::Serialize for $thing {
-            fn consensus_encode<W: $crate::io::Write + ?Sized>(
+            fn serialize(&self) -> Vec<u8> {
-                &self,
+                self.serialize_map()
-                w: &mut W,
-            ) -> Result<usize, $crate::io::Error> {
-                self.consensus_encode_map(w)
             }
         }
     };
 }
 
-macro_rules! impl_psbtmap_consensus_decoding {
+macro_rules! impl_psbtmap_deserialize {
     ($thing:ty) => {
-        impl $crate::consensus::Decodable for $thing {
+        impl $crate::psbt::serialize::Deserialize for $thing {
-            fn consensus_decode<R: $crate::io::Read + ?Sized>(
+            fn deserialize(bytes: &[u8]) -> Result<Self, $crate::consensus::encode::Error> {
+                let mut decoder = crate::io::Cursor::new(bytes);
+                Self::decode(&mut decoder)
+            }
+        }
+    };
+}
+
+macro_rules! impl_psbtmap_decoding {
+    ($thing:ty) => {
+        impl $thing {
+            pub(crate) fn decode<R: $crate::io::Read + ?Sized>(
                 r: &mut R,
             ) -> Result<Self, $crate::consensus::encode::Error> {
                 let mut rv: Self = core::default::Default::default();
 
                 loop {
-                    match $crate::consensus::Decodable::consensus_decode(r) {
+                    match $crate::psbt::raw::Pair::decode(r) {
                         Ok(pair) => rv.insert_pair(pair)?,
                         Err($crate::consensus::encode::Error::Psbt($crate::psbt::Error::NoMorePairs)) => return Ok(rv),
                         Err(e) => return Err(e),
@@ -73,10 +81,11 @@ macro_rules! impl_psbtmap_consensus_decoding {
     };
 }
 
-macro_rules! impl_psbtmap_consensus_enc_dec_oding {
+macro_rules! impl_psbtmap_ser_de_serialize {
     ($thing:ty) => {
-        impl_psbtmap_consensus_decoding!($thing);
+        impl_psbtmap_decoding!($thing);
-        impl_psbtmap_consensus_encoding!($thing);
+        impl_psbtmap_serialize!($thing);
+        impl_psbtmap_deserialize!($thing);
     };
 }
 

bitcoin/src/psbt/map/global.rs

@@ -7,8 +7,8 @@ use crate::prelude::*;
 use crate::io::{self, Cursor, Read};
 
 use crate::blockdata::transaction::Transaction;
-use crate::consensus::{encode, Encodable, Decodable};
 use crate::consensus::encode::MAX_VEC_SIZE;
+use crate::consensus::{encode, Decodable};
 use crate::psbt::map::Map;
 use crate::psbt::{raw, Error, PartiallySignedTransaction};
 use crate::bip32::{ExtendedPubKey, Fingerprint, DerivationPath, ChildNumber};
@@ -23,7 +23,7 @@ const PSBT_GLOBAL_VERSION: u8 = 0xFB;
 const PSBT_GLOBAL_PROPRIETARY: u8 = 0xFC;
 
 impl Map for PartiallySignedTransaction {
-    fn get_pairs(&self) -> Result<Vec<raw::Pair>, io::Error> {
+    fn get_pairs(&self) -> Vec<raw::Pair> {
         let mut rv: Vec<raw::Pair> = Default::default();
 
         rv.push(raw::Pair {
@@ -35,10 +35,10 @@ impl Map for PartiallySignedTransaction {
                 // Manually serialized to ensure 0-input txs are serialized
                 // without witnesses.
                 let mut ret = Vec::new();
-                self.unsigned_tx.version.consensus_encode(&mut ret)?;
+                ret.extend(encode::serialize(&self.unsigned_tx.version));
-                self.unsigned_tx.input.consensus_encode(&mut ret)?;
+                ret.extend(encode::serialize(&self.unsigned_tx.input));
-                self.unsigned_tx.output.consensus_encode(&mut ret)?;
+                ret.extend(encode::serialize(&self.unsigned_tx.output));
-                self.unsigned_tx.lock_time.consensus_encode(&mut ret)?;
+                ret.extend(encode::serialize(&self.unsigned_tx.lock_time));
                 ret
             },
         });
@@ -83,12 +83,12 @@ impl Map for PartiallySignedTransaction {
             });
         }
 
-        Ok(rv)
+        rv
     }
 }
 
 impl PartiallySignedTransaction {
-    pub(crate) fn consensus_decode_global<R: io::Read + ?Sized>(r: &mut R) -> Result<Self, encode::Error> {
+    pub(crate) fn decode_global<R: io::Read + ?Sized>(r: &mut R) -> Result<Self, encode::Error> {
         let mut r = r.take(MAX_VEC_SIZE as u64);
         let mut tx: Option<Transaction> = None;
         let mut version: Option<u32> = None;
@@ -97,7 +97,7 @@ impl PartiallySignedTransaction {
         let mut proprietary: BTreeMap<raw::ProprietaryKey, Vec<u8>> = Default::default();
 
         loop {
-            match raw::Pair::consensus_decode(&mut r) {
+            match raw::Pair::decode(&mut r) {
                 Ok(pair) => {
                     match pair.key.type_value {
                         PSBT_GLOBAL_UNSIGNED_TX => {

bitcoin/src/psbt/map/input.rs

@@ -1,7 +1,6 @@
 // SPDX-License-Identifier: CC0-1.0
 
 use crate::prelude::*;
-use crate::io;
 
 use core::fmt;
 use core::str::FromStr;
@@ -394,7 +393,7 @@ impl Input {
 }
 
 impl Map for Input {
-    fn get_pairs(&self) -> Result<Vec<raw::Pair>, io::Error> {
+    fn get_pairs(&self) -> Vec<raw::Pair> {
         let mut rv: Vec<raw::Pair> = Default::default();
 
         impl_psbt_get_pair! {
@@ -486,11 +485,11 @@ impl Map for Input {
             });
         }
 
-        Ok(rv)
+        rv
     }
 }
 
-impl_psbtmap_consensus_enc_dec_oding!(Input);
+impl_psbtmap_ser_de_serialize!(Input);
 
 fn psbt_insert_hash_pair<H>(
     map: &mut BTreeMap<H, Vec<u8>>,

bitcoin/src/psbt/map/mod.rs

@@ -2,9 +2,6 @@
 
 use crate::prelude::*;
 
-use crate::io;
-
-use crate::consensus::encode;
 use crate::psbt::raw;
 
 mod global;
@@ -14,18 +11,26 @@ mod output;
 pub use self::input::{Input, PsbtSighashType};
 pub use self::output::{Output, TapTree, IncompleteTapTree};
 
+use super::serialize::Serialize;
+
 /// A trait that describes a PSBT key-value map.
 pub(super) trait Map {
     /// Attempt to get all key-value pairs.
-    fn get_pairs(&self) -> Result<Vec<raw::Pair>, io::Error>;
+    fn get_pairs(&self) -> Vec<raw::Pair>;
 
-    /// Encodes map data with bitcoin consensus encoding.
+    /// Serialize Psbt binary map data according to BIP-174 specification.
-    fn consensus_encode_map<W: io::Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
+    ///
-        let mut len = 0;
+    /// <map> := <keypair>* 0x00
-        for pair in Map::get_pairs(self)? {
+    ///
-            len += encode::Encodable::consensus_encode(&pair, w)?;
+    /// Why is the separator here 0x00 instead of 0xff? The separator here is used to distinguish between each chunk of data.
+    /// A separator of 0x00 would mean that the unserializer can read it as a key length of 0, which would never occur with
+    /// actual keys. It can thus be used as a separator and allow for easier unserializer implementation.
+    fn serialize_map(&self) -> Vec<u8> {
+        let mut buf = Vec::new();
+        for pair in Map::get_pairs(self) {
+            buf.extend(&pair.serialize());
         }
-
+        buf.push(0x00_u8);
-        Ok(len + encode::Encodable::consensus_encode(&0x00_u8, w)?)
+        buf
     }
 }

bitcoin/src/psbt/map/output.rs

@@ -4,8 +4,6 @@ use crate::prelude::*;
 use core;
 use core::convert::TryFrom;
 
-use crate::io;
-
 use crate::blockdata::script::ScriptBuf;
 use crate::consensus::encode;
 use secp256k1::XOnlyPublicKey;
@@ -279,7 +277,7 @@ impl Output {
 }
 
 impl Map for Output {
-    fn get_pairs(&self) -> Result<Vec<raw::Pair>, io::Error> {
+    fn get_pairs(&self) -> Vec<raw::Pair> {
         let mut rv: Vec<raw::Pair> = Default::default();
 
         impl_psbt_get_pair! {
@@ -320,8 +318,8 @@ impl Map for Output {
             });
         }
 
-        Ok(rv)
+        rv
     }
 }
 
-impl_psbtmap_consensus_enc_dec_oding!(Output);
+impl_psbtmap_ser_de_serialize!(Output);

bitcoin/src/psbt/mod.rs

@@ -17,11 +17,9 @@ use secp256k1::{Message, Secp256k1, Signing};
 use bitcoin_internals::write_err;
 
 use crate::{prelude::*, Amount};
-use crate::io;
 
 use crate::blockdata::script::ScriptBuf;
 use crate::blockdata::transaction::{Transaction, TxOut};
-use crate::consensus::{encode, Encodable, Decodable};
 use crate::bip32::{self, ExtendedPrivKey, ExtendedPubKey, KeySource};
 use crate::crypto::ecdsa;
 use crate::crypto::key::{PublicKey, PrivateKey};
@@ -39,7 +37,6 @@ pub use self::error::Error;
 
 mod map;
 pub use self::map::{Input, Output, TapTree, PsbtSighashType, IncompleteTapTree};
-use self::map::Map;
 
 /// Partially signed transaction, commonly referred to as a PSBT.
 pub type Psbt = PartiallySignedTransaction;
@@ -751,7 +748,7 @@ mod display_from_str {
     use super::PartiallySignedTransaction;
     use core::fmt::{Display, Formatter, self};
     use core::str::FromStr;
-    use crate::consensus::encode::{Error, self};
+    use crate::consensus::encode::Error;
     use base64::display::Base64Display;
     use bitcoin_internals::write_err;
 
@@ -793,7 +790,7 @@ mod display_from_str {
     #[cfg_attr(docsrs, doc(cfg(feature = "base64")))]
     impl Display for PartiallySignedTransaction {
         fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-            write!(f, "{}", Base64Display::with_config(&encode::serialize(self), base64::STANDARD))
+            write!(f, "{}", Base64Display::with_config(&self.serialize(), base64::STANDARD))
         }
     }
 
@@ -803,7 +800,7 @@ mod display_from_str {
 
         fn from_str(s: &str) -> Result<Self, Self::Err> {
             let data = base64::decode(s).map_err(PsbtParseError::Base64Encoding)?;
-            encode::deserialize(&data).map_err(PsbtParseError::PsbtEncoding)
+            PartiallySignedTransaction::deserialize(&data).map_err(PsbtParseError::PsbtEncoding)
         }
     }
 }
@@ -811,72 +808,6 @@ mod display_from_str {
 #[cfg_attr(docsrs, doc(cfg(feature = "base64")))]
 pub use self::display_from_str::PsbtParseError;
 
-impl Encodable for PartiallySignedTransaction {
-    fn consensus_encode<W: io::Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
-        let mut len = 0;
-        len += b"psbt".consensus_encode(w)?;
-
-        len += 0xff_u8.consensus_encode(w)?;
-
-        len += self.consensus_encode_map(w)?;
-
-        for i in &self.inputs {
-            len += i.consensus_encode(w)?;
-        }
-
-        for i in &self.outputs {
-            len += i.consensus_encode(w)?;
-        }
-
-        Ok(len)
-    }
-}
-
-impl Decodable for PartiallySignedTransaction {
-    fn consensus_decode_from_finite_reader<R: io::Read + ?Sized>(r: &mut R) -> Result<Self, encode::Error> {
-        let magic: [u8; 4] = Decodable::consensus_decode(r)?;
-
-        if *b"psbt" != magic {
-            return Err(Error::InvalidMagic.into());
-        }
-
-        if 0xff_u8 != u8::consensus_decode(r)? {
-            return Err(Error::InvalidSeparator.into());
-        }
-
-        let mut global = PartiallySignedTransaction::consensus_decode_global(r)?;
-        global.unsigned_tx_checks()?;
-
-        let inputs: Vec<Input> = {
-            let inputs_len: usize = global.unsigned_tx.input.len();
-
-            let mut inputs: Vec<Input> = Vec::with_capacity(inputs_len);
-
-            for _ in 0..inputs_len {
-                inputs.push(Decodable::consensus_decode(r)?);
-            }
-
-            inputs
-        };
-
-        let outputs: Vec<Output> = {
-            let outputs_len: usize = global.unsigned_tx.output.len();
-
-            let mut outputs: Vec<Output> = Vec::with_capacity(outputs_len);
-
-            for _ in 0..outputs_len {
-                outputs.push(Decodable::consensus_decode(r)?);
-            }
-
-            outputs
-        };
-
-        global.inputs = inputs;
-        global.outputs = outputs;
-        Ok(global)
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -885,6 +816,7 @@ mod tests {
     use crate::hashes::hex::FromHex;
     use crate::hashes::{sha256, hash160, Hash, ripemd160};
     use crate::hash_types::Txid;
+    use crate::psbt::serialize::{Serialize, Deserialize};
 
     use secp256k1::{Secp256k1, self};
     #[cfg(feature = "rand-std")]
@@ -893,7 +825,6 @@ mod tests {
     use crate::blockdata::script::ScriptBuf;
     use crate::blockdata::transaction::{Transaction, TxIn, TxOut, OutPoint, Sequence};
     use crate::network::constants::Network::Bitcoin;
-    use crate::consensus::encode::{deserialize, serialize, serialize_hex};
     use crate::bip32::{ChildNumber, ExtendedPrivKey, ExtendedPubKey, KeySource};
     use crate::psbt::map::{Output, Input};
     use crate::psbt::raw;
@@ -919,7 +850,7 @@ mod tests {
             inputs: vec![],
             outputs: vec![],
         };
-        assert_eq!(serialize_hex(&psbt), "70736274ff01000a0200000000000000000000");
+        assert_eq!(psbt.serialize_hex(), "70736274ff01000a0200000000000000000000");
     }
 
     #[test]
@@ -967,7 +898,7 @@ mod tests {
             ..Default::default()
         };
 
-        let actual: Output = deserialize(&serialize(&expected)).unwrap();
+        let actual = Output::deserialize(&expected.serialize()).unwrap();
 
         assert_eq!(expected, actual);
     }
@@ -1012,8 +943,7 @@ mod tests {
             outputs: vec![Output::default(), Output::default()],
         };
 
-        let actual: PartiallySignedTransaction = deserialize(&serialize(&expected)).unwrap();
+        let actual: Psbt = Psbt::deserialize(&expected.serialize()).unwrap();
-
         assert_eq!(expected, actual);
     }
 
@@ -1027,7 +957,7 @@ mod tests {
             value: vec![69u8, 42u8, 4u8],
         };
 
-        let actual: raw::Pair = deserialize(&serialize(&expected)).unwrap();
+        let actual = raw::Pair::deserialize(&expected.serialize()).unwrap();
 
         assert_eq!(expected, actual);
     }
@@ -1036,7 +966,7 @@ mod tests {
     fn deserialize_and_serialize_psbt_with_two_partial_sigs() {
         let hex = "70736274ff0100890200000001207ae985d787dfe6143d5c58fad79cc7105e0e799fcf033b7f2ba17e62d7b3200000000000ffffffff02563d03000000000022002019899534b9a011043c0dd57c3ff9a381c3522c5f27c6a42319085b56ca543a1d6adc020000000000220020618b47a07ebecca4e156edb1b9ea7c24bdee0139fc049237965ffdaf56d5ee73000000000001012b801a0600000000002200201148e93e9315e37dbed2121be5239257af35adc03ffdfc5d914b083afa44dab82202025fe7371376d53cf8a2783917c28bf30bd690b0a4d4a207690093ca2b920ee076473044022007e06b362e89912abd4661f47945430739b006a85d1b2a16c01dc1a4bd07acab022061576d7aa834988b7ab94ef21d8eebd996ea59ea20529a19b15f0c9cebe3d8ac01220202b3fe93530020a8294f0e527e33fbdff184f047eb6b5a1558a352f62c29972f8a473044022002787f926d6817504431ee281183b8119b6845bfaa6befae45e13b6d430c9d2f02202859f149a6cd26ae2f03a107e7f33c7d91730dade305fe077bae677b5d44952a01010547522102b3fe93530020a8294f0e527e33fbdff184f047eb6b5a1558a352f62c29972f8a21025fe7371376d53cf8a2783917c28bf30bd690b0a4d4a207690093ca2b920ee07652ae0001014752210283ef76537f2d58ae3aa3a4bd8ae41c3f230ccadffb1a0bd3ca504d871cff05e7210353d79cc0cb1396f4ce278d005f16d948e02a6aec9ed1109f13747ecb1507b37b52ae00010147522102b3937241777b6665e0d694e52f9c1b188433641df852da6fc42187b5d8a368a321034cdd474f01cc5aa7ff834ad8bcc882a87e854affc775486bc2a9f62e8f49bd7852ae00";
         let psbt: PartiallySignedTransaction = hex_psbt!(hex).unwrap();
-        assert_eq!(hex, serialize_hex(&psbt));
+        assert_eq!(hex, psbt.serialize_hex());
     }
 
     #[cfg(feature = "serde")]
@@ -1147,7 +1077,6 @@ mod tests {
 
         use crate::blockdata::script::ScriptBuf;
         use crate::blockdata::transaction::{Transaction, TxIn, TxOut, OutPoint, Sequence};
-        use crate::consensus::encode::serialize_hex;
         use crate::blockdata::locktime::absolute;
         use crate::psbt::map::{Map, Input, Output};
         use crate::psbt::{raw, PartiallySignedTransaction, Error};
@@ -1322,7 +1251,7 @@ mod tests {
 
             let base16str = "70736274ff0100750200000001268171371edff285e937adeea4b37b78000c0566cbb3ad64641713ca42171bf60000000000feffffff02d3dff505000000001976a914d0c59903c5bac2868760e90fd521a4665aa7652088ac00e1f5050000000017a9143545e6e33b832c47050f24d3eeb93c9c03948bc787b32e1300000100fda5010100000000010289a3c71eab4d20e0371bbba4cc698fa295c9463afa2e397f8533ccb62f9567e50100000017160014be18d152a9b012039daf3da7de4f53349eecb985ffffffff86f8aa43a71dff1448893a530a7237ef6b4608bbb2dd2d0171e63aec6a4890b40100000017160014fe3e9ef1a745e974d902c4355943abcb34bd5353ffffffff0200c2eb0b000000001976a91485cff1097fd9e008bb34af709c62197b38978a4888ac72fef84e2c00000017a914339725ba21efd62ac753a9bcd067d6c7a6a39d05870247304402202712be22e0270f394f568311dc7ca9a68970b8025fdd3b240229f07f8a5f3a240220018b38d7dcd314e734c9276bd6fb40f673325bc4baa144c800d2f2f02db2765c012103d2e15674941bad4a996372cb87e1856d3652606d98562fe39c5e9e7e413f210502483045022100d12b852d85dcd961d2f5f4ab660654df6eedcc794c0c33ce5cc309ffb5fce58d022067338a8e0e1725c197fb1a88af59f51e44e4255b20167c8684031c05d1f2592a01210223b72beef0965d10be0778efecd61fcac6f79a4ea169393380734464f84f2ab300000000000000";
 
-            assert_eq!(serialize_hex(&unserialized), base16str);
+            assert_eq!(unserialized.serialize_hex(), base16str);
             assert_eq!(unserialized, hex_psbt!(base16str).unwrap());
 
             #[cfg(feature = "base64")] {
@@ -1353,7 +1282,7 @@ mod tests {
             assert_eq!(redeem_script.to_p2sh(), expected_out);
 
             for output in psbt.outputs {
-                assert_eq!(output.get_pairs().unwrap().len(), 0)
+                assert_eq!(output.get_pairs().len(), 0)
             }
         }
 
@@ -1399,7 +1328,7 @@ mod tests {
             assert_eq!(redeem_script.to_p2sh(), expected_out);
 
             for output in psbt.outputs {
-                assert!(!output.get_pairs().unwrap().is_empty())
+                assert!(!output.get_pairs().is_empty())
             }
         }
 
@@ -1452,7 +1381,6 @@ mod tests {
 
     mod bip_371_vectors {
         use super::*;
-        use super::serialize;
 
         #[test]
         fn invalid_vectors() {
@@ -1482,8 +1410,8 @@ mod tests {
         }
 
         fn rtt_psbt(psbt: PartiallySignedTransaction) {
-            let enc = serialize(&psbt);
+            let enc = Psbt::serialize(&psbt);
-            let psbt2 = deserialize::<PartiallySignedTransaction>(&enc).unwrap();
+            let psbt2 = Psbt::deserialize(&enc).unwrap();
             assert_eq!(psbt, psbt2);
         }
 
@@ -1634,7 +1562,7 @@ mod tests {
         unserialized.inputs[0].hash160_preimages = hash160_preimages;
         unserialized.inputs[0].sha256_preimages = sha256_preimages;
 
-        let rtt: PartiallySignedTransaction = hex_psbt!(&serialize_hex(&unserialized)).unwrap();
+        let rtt: PartiallySignedTransaction = hex_psbt!(&unserialized.serialize_hex()).unwrap();
         assert_eq!(rtt, unserialized);
 
         // Now add an ripemd160 with incorrect preimage
@@ -1643,7 +1571,7 @@ mod tests {
         unserialized.inputs[0].ripemd160_preimages = ripemd160_preimages;
 
         // Now the roundtrip should fail as the preimage is incorrect.
-        let rtt: Result<PartiallySignedTransaction, _> = hex_psbt!(&serialize_hex(&unserialized));
+        let rtt: Result<PartiallySignedTransaction, _> = hex_psbt!(&unserialized.serialize_hex());
         assert!(rtt.is_err());
     }
 
@@ -1656,7 +1584,7 @@ mod tests {
             key: b"test".to_vec(),
         }, b"test".to_vec());
         assert!(!psbt.proprietary.is_empty());
-        let rtt: PartiallySignedTransaction = hex_psbt!(&serialize_hex(&psbt)).unwrap();
+        let rtt: PartiallySignedTransaction = hex_psbt!(&psbt.serialize_hex()).unwrap();
         assert!(!rtt.proprietary.is_empty());
     }
 

bitcoin/src/psbt/raw.rs

@@ -10,11 +10,13 @@ use crate::prelude::*;
 use core::fmt;
 use core::convert::TryFrom;
 
-use crate::io;
+use crate::io::{self, Cursor};
 use crate::consensus::encode::{self, ReadExt, WriteExt, Decodable, Encodable, VarInt, serialize, deserialize, MAX_VEC_SIZE};
 use crate::hashes::hex;
 use crate::psbt::Error;
 
+use super::serialize::{Serialize, Deserialize};
+
 /// A PSBT key in its raw byte form.
 #[derive(Debug, PartialEq, Hash, Eq, Clone, Ord, PartialOrd)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
@@ -23,18 +25,21 @@ pub struct Key {
     /// The type of this PSBT key.
     pub type_value: u8,
     /// The key itself in raw byte form.
+    /// `<key> := <keylen> <keytype> <keydata>`
     #[cfg_attr(feature = "serde", serde(with = "crate::serde_utils::hex_bytes"))]
     pub key: Vec<u8>,
 }
 
 /// A PSBT key-value pair in its raw byte form.
+/// `<keypair> := <key> <value>`
 #[derive(Debug, PartialEq, Eq)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "serde", serde(crate = "actual_serde"))]
 pub struct Pair {
     /// The key of this key-value pair.
     pub key: Key,
-    /// The value of this key-value pair in raw byte form.
+    /// The value data of this key-value pair in raw byte form.
+    /// `<value> := <valuelen> <valuedata>`
     #[cfg_attr(feature = "serde", serde(with = "crate::serde_utils::hex_bytes"))]
     pub value: Vec<u8>,
 }
@@ -66,8 +71,8 @@ impl fmt::Display for Key {
     }
 }
 
-impl Decodable for Key {
+impl Key {
-    fn consensus_decode<R: io::Read + ?Sized>(r: &mut R) -> Result<Self, encode::Error> {
+    pub(crate) fn decode<R: io::Read + ?Sized>(r: &mut R) -> Result<Self, encode::Error> {
         let VarInt(byte_size): VarInt = Decodable::consensus_decode(r)?;
 
         if byte_size == 0 {
@@ -94,32 +99,42 @@ impl Decodable for Key {
     }
 }
 
-impl Encodable for Key {
+impl Serialize for Key {
-    fn consensus_encode<W: io::Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
+    fn serialize(&self) -> Vec<u8> {
-        let mut len = 0;
+        let mut buf = Vec::new();
-        len += VarInt((self.key.len() + 1) as u64).consensus_encode(w)?;
+        VarInt((self.key.len() + 1) as u64).consensus_encode(&mut buf).expect("in-memory writers don't error");
 
-        len += self.type_value.consensus_encode(w)?;
+        self.type_value.consensus_encode(&mut buf).expect("in-memory writers don't error");
 
         for key in &self.key {
-            len += key.consensus_encode(w)?
+            key.consensus_encode(&mut buf).expect("in-memory writers don't error");
         }
 
-        Ok(len)
+        buf
     }
 }
 
-impl Encodable for Pair {
+impl Serialize for Pair {
-    fn consensus_encode<W: io::Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
+    fn serialize(&self) -> Vec<u8> {
-        let len = self.key.consensus_encode(w)?;
+        let mut buf = Vec::new();
-        Ok(len + self.value.consensus_encode(w)?)
+        buf.extend(self.key.serialize());
+        // <value> := <valuelen> <valuedata>
+        self.value.consensus_encode(&mut buf).unwrap();
+        buf
     }
 }
 
-impl Decodable for Pair {
+impl Deserialize for Pair {
-    fn consensus_decode<R: io::Read + ?Sized>(r: &mut R) -> Result<Self, encode::Error> {
+    fn deserialize(bytes: &[u8]) -> Result<Self, encode::Error> {
+        let mut decoder = Cursor::new(bytes);
+        Pair::decode(&mut decoder)
+    }
+}
+
+impl Pair {
+    pub(crate) fn decode<R: io::Read + ?Sized>(r: &mut R) -> Result<Self, encode::Error> {
         Ok(Pair {
-            key: Decodable::consensus_decode(r)?,
+            key: Key::decode(r)?,
             value: Decodable::consensus_decode(r)?,
         })
     }

bitcoin/src/psbt/serialize.rs

@@ -2,8 +2,8 @@
 
 //! PSBT serialization.
 //!
-//! Defines traits used for (de)serializing PSBT values into/from raw
+//! Traits to serialize PSBT values to and from raw bytes
-//! bytes from/as PSBT key-value pairs.
+//! according to the BIP-174 specification.
 //!
 
 use core::convert::TryInto;
@@ -20,26 +20,102 @@ use secp256k1::{self, XOnlyPublicKey};
 use crate::bip32::{ChildNumber, Fingerprint, KeySource};
 use crate::hashes::{hash160, ripemd160, sha256, sha256d, Hash};
 use crate::crypto::{ecdsa, schnorr};
-use crate::psbt;
+use crate::psbt::{self, Error, PartiallySignedTransaction};
 use crate::taproot::{TapBranchHash, TapLeafHash, ControlBlock, LeafVersion};
 use crate::crypto::key::PublicKey;
 
-use super::map::{TapTree, PsbtSighashType};
+use super::map::{Map, Input, Output, TapTree, PsbtSighashType};
+use super::Psbt;
 
 use crate::taproot::TaprootBuilder;
 /// A trait for serializing a value as raw data for insertion into PSBT
-/// key-value pairs.
+/// key-value maps.
-pub trait Serialize {
+pub(crate) trait Serialize {
     /// Serialize a value as raw data.
     fn serialize(&self) -> Vec<u8>;
 }
 
-/// A trait for deserializing a value from raw data in PSBT key-value pairs.
+/// A trait for deserializing a value from raw data in PSBT key-value maps.
-pub trait Deserialize: Sized {
+pub(crate) trait Deserialize: Sized {
     /// Deserialize a value from raw data.
     fn deserialize(bytes: &[u8]) -> Result<Self, encode::Error>;
 }
 
+impl PartiallySignedTransaction {
+    /// Serialize a value as bytes in hex.
+    pub fn serialize_hex(&self) -> String {
+        bitcoin_hashes::hex::ToHex::to_hex(&self.serialize()[..])
+    }
+
+    /// Serialize as raw binary data
+    pub fn serialize(&self) -> Vec<u8> {
+        let mut buf: Vec<u8> = Vec::new();
+
+        //  <magic>
+        buf.extend(b"psbt");
+
+        buf.push(0xff_u8);
+
+        buf.extend(self.serialize_map());
+
+        for i in &self.inputs {
+            buf.extend(i.serialize_map());
+        }
+
+        for i in &self.outputs {
+            buf.extend(i.serialize_map());
+        }
+
+        buf
+    }
+
+
+    /// Deserialize a value from raw binary data.
+    pub fn deserialize(bytes: &[u8]) -> Result<Self, encode::Error> {
+        const MAGIC_BYTES: &[u8] = b"psbt";
+        if bytes.get(0..MAGIC_BYTES.len()) != Some(MAGIC_BYTES) {
+            return Err(Error::InvalidMagic.into());
+        }
+
+        const PSBT_SERPARATOR: u8 = 0xff_u8;
+        if bytes.get(MAGIC_BYTES.len()) != Some(&PSBT_SERPARATOR) {
+            return Err(Error::InvalidSeparator.into());
+        }
+
+        let mut d = bytes.get(5..).ok_or(Error::NoMorePairs)?;
+
+        let mut global = Psbt::decode_global(&mut d)?;
+        global.unsigned_tx_checks()?;
+
+        let inputs: Vec<Input> = {
+            let inputs_len: usize = (global.unsigned_tx.input).len();
+
+            let mut inputs: Vec<Input> = Vec::with_capacity(inputs_len);
+
+            for _ in 0..inputs_len {
+                inputs.push(Input::decode(&mut d)?);
+            }
+
+            inputs
+        };
+
+        let outputs: Vec<Output> = {
+            let outputs_len: usize = (global.unsigned_tx.output).len();
+
+            let mut outputs: Vec<Output> = Vec::with_capacity(outputs_len);
+
+            for _ in 0..outputs_len {
+                outputs.push(Output::decode(&mut d)?);
+            }
+
+            outputs
+        };
+
+        global.inputs = inputs;
+        global.outputs = outputs;
+        Ok(global)
+    }
+}
 impl_psbt_de_serialize!(Transaction);
 impl_psbt_de_serialize!(TxOut);
 impl_psbt_de_serialize!(Witness);
@@ -402,4 +478,11 @@ mod tests {
         let sighash = PsbtSighashType::deserialize(&non_standard_sighash);
         assert!(sighash.is_ok())
     }
+
+    #[test]
+    #[should_panic(expected = "InvalidMagic")]
+    fn invalid_vector_1() {
+        let hex_psbt = b"0200000001268171371edff285e937adeea4b37b78000c0566cbb3ad64641713ca42171bf6000000006a473044022070b2245123e6bf474d60c5b50c043d4c691a5d2435f09a34a7662a9dc251790a022001329ca9dacf280bdf30740ec0390422422c81cb45839457aeb76fc12edd95b3012102657d118d3357b8e0f4c2cd46db7b39f6d9c38d9a70abcb9b2de5dc8dbfe4ce31feffffff02d3dff505000000001976a914d0c59903c5bac2868760e90fd521a4665aa7652088ac00e1f5050000000017a9143545e6e33b832c47050f24d3eeb93c9c03948bc787b32e1300";
+        PartiallySignedTransaction::deserialize(hex_psbt).unwrap();
+    }
 }

bitcoin/tests/psbt.rs

@@ -26,7 +26,7 @@ macro_rules! hex_script {
 
 macro_rules! hex_psbt {
     ($s:expr) => {
-        deserialize::<Psbt>(&<Vec<u8> as FromHex>::from_hex($s).unwrap())
+        Psbt::deserialize(&<Vec<u8> as FromHex>::from_hex($s).unwrap())
     };
 }
 
@@ -397,10 +397,10 @@ fn combine_lexicographically() {
     let expected_psbt = hex_psbt!(expected_psbt_hex).unwrap();
 
     let v = Vec::from_hex(psbt_1_hex).unwrap();
-    let mut psbt_1: Psbt = deserialize(&v).expect("failed to deserialize psbt 1");
+    let mut psbt_1 = Psbt::deserialize(&v).expect("failed to deserialize psbt 1");
 
     let v = Vec::from_hex(psbt_2_hex).unwrap();
-    let psbt_2: Psbt = deserialize(&v).expect("failed to deserialize psbt 2");
+    let psbt_2 = Psbt::deserialize(&v).expect("failed to deserialize psbt 2");
 
     psbt_1.combine(psbt_2).expect("failed to combine PSBTs");
 
@@ -417,16 +417,14 @@ fn sign(mut psbt: Psbt, keys: BTreeMap<bitcoin::PublicKey, PrivateKey>) -> Psbt
 /// Finalizes a PSBT accord to the Input Finalizer role described in BIP 174.
 /// This is just a test. For a production-ready PSBT Finalizer, use [rust-miniscript](https://docs.rs/miniscript/latest/miniscript/psbt/trait.PsbtExt.html#tymethod.finalize)
 fn finalize_psbt(mut psbt: Psbt) -> Psbt {
-    use bitcoin::psbt::serialize::Serialize;
-
     // Input 0: legacy UTXO
 
     let sigs: Vec<_> = psbt.inputs[0].partial_sigs.values().collect();
     let script_sig = script::Builder::new()
         .push_opcode(OP_0) // OP_CHECKMULTISIG bug pops +1 value when evaluating so push OP_0.
-        .push_slice(&sigs[0].serialize())
+        .push_slice(&sigs[0].to_vec())
-        .push_slice(&sigs[1].serialize())
+        .push_slice(&sigs[1].to_vec())
-        .push_slice(&psbt.inputs[0].redeem_script.clone().unwrap().serialize())
+        .push_slice(psbt.inputs[0].redeem_script.clone().unwrap().as_bytes())
         .into_script();
 
     psbt.inputs[0].final_script_sig = Some(script_sig);
@@ -439,7 +437,7 @@ fn finalize_psbt(mut psbt: Psbt) -> Psbt {
     // Input 1: SegWit UTXO
 
     let script_sig = script::Builder::new()
-        .push_slice(&psbt.inputs[1].redeem_script.clone().unwrap().serialize())
+        .push_slice(psbt.inputs[1].redeem_script.clone().unwrap().as_bytes())
         .into_script();
 
     psbt.inputs[1].final_script_sig = Some(script_sig);
@@ -448,9 +446,9 @@ fn finalize_psbt(mut psbt: Psbt) -> Psbt {
         let sigs: Vec<_> = psbt.inputs[1].partial_sigs.values().collect();
         let mut script_witness = Witness::new();
         script_witness.push([]); // Push 0x00 to the stack.
-        script_witness.push(&sigs[1].serialize());
+        script_witness.push(&sigs[1].to_vec());
-        script_witness.push(&sigs[0].serialize());
+        script_witness.push(&sigs[0].to_vec());
-        script_witness.push(&psbt.inputs[1].witness_script.clone().unwrap().serialize());
+        script_witness.push(psbt.inputs[1].witness_script.clone().unwrap().as_bytes());
 
         script_witness
     };