Serialize Psbt fields, don't consensus_encode them

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/src/psbt/macros.rs

@@ -40,14 +40,11 @@ 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)
             }
         }
     };
@@ -73,13 +70,6 @@ macro_rules! impl_psbtmap_consensus_decoding {
     };
 }
 
-macro_rules! impl_psbtmap_consensus_enc_dec_oding {
-    ($thing:ty) => {
-        impl_psbtmap_consensus_decoding!($thing);
-        impl_psbtmap_consensus_encoding!($thing);
-    };
-}
-
 #[rustfmt::skip]
 macro_rules! impl_psbt_insert_pair {
     ($slf:ident.$unkeyed_name:ident <= <$raw_key:ident: _>|<$raw_value:ident: $unkeyed_value_type:ty>) => {

bitcoin/src/psbt/map/global.rs

@@ -7,7 +7,7 @@ use crate::prelude::*;
 use crate::io::{self, Cursor, Read};
 
 use crate::blockdata::transaction::Transaction;
-use crate::consensus::{encode, Encodable, Decodable};
+use crate::consensus::{encode, Decodable};
 use crate::consensus::encode::MAX_VEC_SIZE;
 use crate::psbt::map::Map;
 use crate::psbt::{raw, Error, PartiallySignedTransaction};
@@ -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,7 +83,7 @@ impl Map for PartiallySignedTransaction {
             });
         }
 
-        Ok(rv)
+        rv
     }
 }
 

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,12 @@ impl Map for Input {
             });
         }
 
-        Ok(rv)
+        rv
     }
 }
 
-impl_psbtmap_consensus_enc_dec_oding!(Input);
+impl_psbtmap_serialize!(Input);
+impl_psbtmap_consensus_decoding!(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,28 +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>;
 
-    fn consensus_encode_map<W: io::Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
+    /// Serialize Psbt binary map data according to BIP-174 specification.
-        let mut len = 0;
+    ///
-        for pair in Map::get_pairs(self)? {
+    /// <map> := <keypair>* 0x00
-            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
-    }
-
-    /// Encodes map data with bitcoin consensus encoding.
-    /// Serialize Psbt map data according to BIP-174 specification.
-    fn serialize_map<W: io::Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
-        let mut len = 0;
-        for pair in Map::get_pairs(self)? {
-            len += encode::Encodable::consensus_encode(&pair, w)?;
-        }
-
-        Ok(len + encode::Encodable::consensus_encode(&0x00_u8, w)?)
     }
 }

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,9 @@ impl Map for Output {
             });
         }
 
-        Ok(rv)
+        rv
     }
 }
 
-impl_psbtmap_consensus_enc_dec_oding!(Output);
+impl_psbtmap_serialize!(Output);
+impl_psbtmap_consensus_decoding!(Output);

bitcoin/src/psbt/mod.rs

@@ -816,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;
 
     use secp256k1::{Secp256k1, self};
     #[cfg(feature = "rand")]
@@ -824,7 +825,7 @@ 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};
+    use crate::consensus::encode::deserialize;
     use crate::bip32::{ChildNumber, ExtendedPrivKey, ExtendedPubKey, KeySource};
     use crate::psbt::map::{Output, Input};
     use crate::psbt::raw;
@@ -898,7 +899,7 @@ mod tests {
             ..Default::default()
         };
 
-        let actual: Output = deserialize(&serialize(&expected)).unwrap();
+        let actual: Output = deserialize(&expected.serialize()).unwrap();
 
         assert_eq!(expected, actual);
     }
@@ -943,7 +944,7 @@ mod tests {
             outputs: vec![Output::default(), Output::default()],
         };
 
-        let actual: Psbt = Psbt::deserialize(&Psbt::serialize(&expected)).unwrap();
+        let actual: Psbt = Psbt::deserialize(&expected.serialize()).unwrap();
         assert_eq!(expected, actual);
     }
 
@@ -957,7 +958,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);
     }
@@ -1282,7 +1283,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)
             }
         }
 
@@ -1328,7 +1329,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())
             }
         }
 

bitcoin/src/psbt/raw.rs

@@ -15,6 +15,8 @@ use crate::consensus::encode::{self, ReadExt, WriteExt, Decodable, Encodable, Va
 use crate::hashes::hex;
 use crate::psbt::Error;
 
+use super::serialize::Serialize;
+
 /// 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>,
 }
@@ -94,25 +99,28 @@ 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
     }
 }
 

bitcoin/src/psbt/serialize.rs

@@ -23,11 +23,12 @@ use crate::taproot::{TapBranchHash, TapLeafHash, ControlBlock, LeafVersion};
 use crate::crypto::key::PublicKey;
 
 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 maps.
-pub trait Serialize {
+pub(crate) trait Serialize {
     /// Serialize a value as raw data.
     fn serialize(&self) -> Vec<u8>;
 }
@@ -38,28 +39,36 @@ pub trait Deserialize: Sized {
     fn deserialize(bytes: &[u8]) -> Result<Self, encode::Error>;
 }
 
+impl Serialize for Psbt {
+    /// Serialize a value as raw binary data.
+   fn serialize(&self) -> Vec<u8> {
+        self.serialize()
+    }
+}
+
 impl PartiallySignedTransaction {
-     /// Serialize a value as bytes in hex.
+    /// Serialize a value as bytes in hex.
-     pub fn serialize_hex(&self) -> String {
+    pub fn serialize_hex(&self) -> String {
         bitcoin_hashes::hex::ToHex::to_hex(&self.serialize()[..])
     }
 
-    /// Serialize a value as raw binary data.
+    /// 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);
 
-        self.serialize_map(&mut buf).unwrap();
+        buf.extend(self.serialize_map());
 
         for i in &self.inputs {
-            i.consensus_encode(&mut buf).unwrap();
+            buf.extend(i.serialize_map());
         }
 
         for i in &self.outputs {
-            i.consensus_encode(&mut buf).unwrap();
+            buf.extend(i.serialize_map());
         }
 
         buf

bitcoin/tests/psbt.rs

@@ -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
     };