serde: remove impls for PSET and Transaction

Leave the impl for BlockHeader (which is used by the functionary in a
couple of places) (though I'd also like to delete this), the impls for
the hash types (which are manually implemented and have regression
tests), and the confidential stuff (which are also manually implemented
and fairly straightforward).

But for Transaction and PSET these impls are untenable. They are:

* based on serde-derives (though 'manually' through the serde_struct_impl
  macro) of extremely complicated and fragile types, exposing implementation
  details and probably bypassing invariants on deserialization
* not specified or documented anywhere
* have very poor error messages on deserialization
* not tested anywhere (not a single test broke with this change!!)

If people are depending on these, we should provide some sort of compat
crate/module. We can wait until they are trying to upgrade rust-elements
and file complaints to do this. I'm skeptical that anybody *is*, given
the above problems, at least not on purpose.

If people want serde impls but don't need backward compatibility, we
can provide that via the bitcoin-consensus-encoding crate, which will
have general-purpose "serde-encode consensus objects in hex/bytes"
adaptors in an upcoming version.

Author: apoelstra

src/block.rs

@@ -360,7 +360,6 @@ pub struct Block {
     /// Complete list of transaction in the block
     pub txdata: Vec<Transaction>,
 }
-serde_struct_impl!(Block, header, txdata);
 impl_consensus_encoding!(Block, header, txdata);
 
 impl Block {

src/lib.rs

@@ -69,8 +69,6 @@ mod parse;
 pub mod pset;
 pub mod schnorr;
 pub mod script;
-#[cfg(feature = "serde")]
-mod serde_utils;
 pub mod sighash;
 pub mod taproot;
 mod transaction;

src/locktime.rs

@@ -66,7 +66,6 @@ pub const LOCK_TIME_THRESHOLD: u32 = 500_000_000;
 /// ```
 #[allow(clippy::derive_ord_xor_partial_ord)]
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub enum LockTime {
     /// A block height lock time value.
     ///
@@ -292,7 +291,6 @@ impl Decodable for LockTime {
 
 /// An absolute block height, guaranteed to always contain a valid height value.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
-#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct Height(u32);
 
 impl Height {
@@ -377,7 +375,6 @@ impl TryFrom<String> for Height {
 /// `to_consensus_u32()`. Said another way, `Time(x)` means 'x seconds since epoch' _not_ '(x -
 /// threshold) seconds since epoch'.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
-#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct Time(u32);
 
 impl Time {

src/opcodes.rs

@@ -20,8 +20,6 @@
 
 #![allow(non_camel_case_types)]
 
-#[cfg(feature = "serde")] use serde;
-
 use std::fmt;
 
 // Note: I am deliberately not implementing PartialOrd or Ord on the
@@ -862,16 +860,6 @@ impl fmt::Display for All {
     }
 }
 
-#[cfg(feature = "serde")]
-impl serde::Serialize for All {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: serde::Serializer,
-    {
-        serializer.serialize_str(&self.to_string())
-    }
-}
-
 /// Empty stack is also FALSE
 pub static OP_FALSE: All = all::OP_PUSHBYTES_0;
 /// Number 1 is also TRUE

src/pset/map/global.rs

@@ -58,7 +58,6 @@ const PSBT_ELEMENTS_GLOBAL_TX_MODIFIABLE: u8 = 0x01;
 
 /// Global transaction data
 #[derive(Debug, Clone, PartialEq)]
-#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct TxData {
     /// Transaction version. Must be 2.
     pub version: u32,
@@ -93,10 +92,8 @@ impl Default for TxData {
 
 /// A key-value map for global data.
 #[derive(Clone, Debug, PartialEq)]
-#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct Global {
     /// Global transaction data
-    #[cfg_attr(feature = "serde", serde(flatten))]
     pub tx_data: TxData,
     /// The version number of this PSET. Must be present.
     pub version: u32,
@@ -109,16 +106,8 @@ pub struct Global {
     /// Elements tx modifiable flag
     pub elements_tx_modifiable_flag: Option<u8>,
     /// Other Proprietary fields
-    #[cfg_attr(
-        feature = "serde",
-        serde(with = "crate::serde_utils::btreemap_as_seq_byte_values")
-    )]
     pub proprietary: BTreeMap<raw::ProprietaryKey, Vec<u8>>,
     /// Unknown global key-value pairs.
-    #[cfg_attr(
-        feature = "serde",
-        serde(with = "crate::serde_utils::btreemap_as_seq_byte_values")
-    )]
     pub unknown: BTreeMap<raw::Key, Vec<u8>>,
 }
 

src/pset/map/input.rs

@@ -172,7 +172,6 @@ const PSBT_ELEMENTS_IN_BLINDED_ISSUANCE: u8 = 0x15;
 /// A key-value map for an input of the corresponding index in the unsigned
 /// transaction.
 #[derive(Clone, Debug, PartialEq)]
-#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct Input {
     /// The non-witness transaction this input spends from. Should only be
     /// [`std::option::Option::Some`] for inputs which spend non-segwit outputs or
@@ -184,10 +183,6 @@ pub struct Input {
     pub witness_utxo: Option<TxOut>,
     /// A map from public keys to their corresponding signature as would be
     /// pushed to the stack from a scriptSig or witness.
-    #[cfg_attr(
-        feature = "serde",
-        serde(with = "crate::serde_utils::btreemap_byte_values")
-    )]
     pub partial_sigs: BTreeMap<PublicKey, Vec<u8>>,
     /// The sighash type to be used for this input. Signatures for this input
     /// must use the sighash type.
@@ -198,7 +193,6 @@ pub struct Input {
     pub witness_script: Option<Script>,
     /// A map from public keys needed to sign this input to their corresponding
     /// master key fingerprints and derivation paths.
-    #[cfg_attr(feature = "serde", serde(with = "crate::serde_utils::btreemap_as_seq"))]
     pub bip32_derivation: BTreeMap<PublicKey, KeySource>,
     /// The finalized, fully-constructed scriptSig with signatures and any other
     /// scripts necessary for this input to pass validation.
@@ -208,28 +202,12 @@ pub struct Input {
     pub final_script_witness: Option<Vec<Vec<u8>>>,
     /// TODO: Proof of reserves commitment
     /// RIPEMD160 hash to preimage map
-    #[cfg_attr(
-        feature = "serde",
-        serde(with = "crate::serde_utils::btreemap_byte_values")
-    )]
     pub ripemd160_preimages: BTreeMap<ripemd160::Hash, Vec<u8>>,
     /// SHA256 hash to preimage map
-    #[cfg_attr(
-        feature = "serde",
-        serde(with = "crate::serde_utils::btreemap_byte_values")
-    )]
     pub sha256_preimages: BTreeMap<sha256::Hash, Vec<u8>>,
     /// HSAH160 hash to preimage map
-    #[cfg_attr(
-        feature = "serde",
-        serde(with = "crate::serde_utils::btreemap_byte_values")
-    )]
     pub hash160_preimages: BTreeMap<hash160::Hash, Vec<u8>>,
     /// HAS256 hash to preimage map
-    #[cfg_attr(
-        feature = "serde",
-        serde(with = "crate::serde_utils::btreemap_byte_values")
-    )]
     pub hash256_preimages: BTreeMap<sha256d::Hash, Vec<u8>>,
     /// (PSET) Prevout TXID of the input
     pub previous_txid: Txid,
@@ -244,13 +222,10 @@ pub struct Input {
     /// Serialized schnorr signature with sighash type for key spend
     pub tap_key_sig: Option<schnorr::SchnorrSig>,
     /// Map of `<xonlypubkey>|<leafhash>` with signature
-    #[cfg_attr(feature = "serde", serde(with = "crate::serde_utils::btreemap_as_seq"))]
     pub tap_script_sigs: BTreeMap<(XOnlyPublicKey, TapLeafHash), schnorr::SchnorrSig>,
     /// Map of Control blocks to Script version pair
-    #[cfg_attr(feature = "serde", serde(with = "crate::serde_utils::btreemap_as_seq"))]
     pub tap_scripts: BTreeMap<ControlBlock, (Script, LeafVersion)>,
     /// Map of tap root x only keys to origin info and leaf hashes contained in it
-    #[cfg_attr(feature = "serde", serde(with = "crate::serde_utils::btreemap_as_seq"))]
     pub tap_key_origins: BTreeMap<XOnlyPublicKey, (Vec<TapLeafHash>, KeySource)>,
     /// Taproot Internal key
     pub tap_internal_key: Option<XOnlyPublicKey>,
@@ -303,16 +278,8 @@ pub struct Input {
     /// Whether the issuance is blinded
     pub blinded_issuance: Option<u8>,
     /// Other fields
-    #[cfg_attr(
-        feature = "serde",
-        serde(with = "crate::serde_utils::btreemap_as_seq_byte_values")
-    )]
     pub proprietary: BTreeMap<raw::ProprietaryKey, Vec<u8>>,
     /// Unknown key-value pairs for this input.
-    #[cfg_attr(
-        feature = "serde",
-        serde(with = "crate::serde_utils::btreemap_as_seq_byte_values")
-    )]
     pub unknown: BTreeMap<raw::Key, Vec<u8>>,
 }
 

src/pset/map/output.rs

@@ -87,22 +87,19 @@ const PSBT_ELEMENTS_OUT_BLIND_ASSET_PROOF: u8 = 0x0a;
 /// A key-value map for an output of the corresponding index in the unsigned
 /// transaction.
 #[derive(Clone, Default, Debug, PartialEq)]
-#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct Output {
     /// The redeem script for this output.
     pub redeem_script: Option<Script>,
     /// The witness script for this output.
     pub witness_script: Option<Script>,
     /// A map from public keys needed to spend this output to their
     /// corresponding master key fingerprints and derivation paths.
-    #[cfg_attr(feature = "serde", serde(with = "crate::serde_utils::btreemap_as_seq"))]
     pub bip32_derivation: BTreeMap<PublicKey, KeySource>,
     /// The internal pubkey
     pub tap_internal_key: Option<XOnlyPublicKey>,
     /// Taproot Output tree
     pub tap_tree: Option<TapTree>,
     /// Map of tap root x only keys to origin info and leaf hashes contained in it
-    #[cfg_attr(feature = "serde", serde(with = "crate::serde_utils::btreemap_as_seq"))]
     pub tap_key_origins: BTreeMap<XOnlyPublicKey, (Vec<TapLeafHash>, KeySource)>,
     /// (PSET) The explicit amount of the output
     pub amount: Option<u64>,
@@ -131,22 +128,13 @@ pub struct Output {
     pub blind_asset_proof: Option<SurjectionProof>,
     /// Pset
     /// Other fields
-    #[cfg_attr(
-        feature = "serde",
-        serde(with = "crate::serde_utils::btreemap_as_seq_byte_values")
-    )]
     pub proprietary: BTreeMap<raw::ProprietaryKey, Vec<u8>>,
     /// Unknown key-value pairs for this output.
-    #[cfg_attr(
-        feature = "serde",
-        serde(with = "crate::serde_utils::btreemap_as_seq_byte_values")
-    )]
     pub unknown: BTreeMap<raw::Key, Vec<u8>>,
 }
 
 /// Taproot Tree representing a finalized [`TaprootBuilder`] (a complete binary tree)
 #[derive(Clone, Debug)]
-#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct TapTree(pub(crate) TaprootBuilder);
 
 impl PartialEq for TapTree {

src/pset/mod.rs

@@ -58,7 +58,6 @@ pub use self::map::{Global, GlobalTxData, Input, Output, PsbtSighashType, TapTre
 
 /// A Partially Signed Transaction.
 #[derive(Debug, Clone, PartialEq)]
-#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct PartiallySignedTransaction {
     /// The key-value pairs for all global data.
     pub global: Global,

src/pset/raw.rs

@@ -24,15 +24,10 @@ use crate::encode::{self, deserialize, serialize, Decodable, Encodable, VarInt,
 use hex::DisplayHex as _;
 /// A PSET key in its raw byte form.
 #[derive(Debug, PartialEq, Hash, Eq, Clone, Ord, PartialOrd)]
-#[cfg_attr(
-    feature = "serde",
-    derive(serde::Serialize, serde::Deserialize),
-)]
 pub struct Key {
     /// The type of this PSET key.
     pub type_value: u8,
     /// The key itself in raw byte form.
-    #[cfg_attr(feature = "serde", serde(with = "crate::serde_utils::hex_bytes"))]
     pub key: Vec<u8>,
 }
 
@@ -50,15 +45,10 @@ impl Key {
 
 /// A PSET key-value pair in its raw byte form.
 #[derive(Debug, PartialEq)]
-#[cfg_attr(
-    feature = "serde",
-    derive(serde::Serialize, serde::Deserialize),
-)]
 pub struct Pair {
     /// The key of this key-value pair.
     pub key: Key,
     /// The value of this key-value pair in raw byte form.
-    #[cfg_attr(feature = "serde", serde(with = "crate::serde_utils::hex_bytes"))]
     pub value: Vec<u8>,
 }
 
@@ -67,23 +57,17 @@ pub type ProprietaryType = u8;
 
 /// Proprietary keys (i.e. keys starting with 0xFC byte) with their internal
 /// structure according to BIP 174.
-#[cfg_attr(
-    feature = "serde",
-    derive(serde::Serialize, serde::Deserialize),
-)]
 #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
 pub struct ProprietaryKey<Subtype = ProprietaryType>
 where
     Subtype: Copy + From<u8> + Into<u8>,
 {
     /// Proprietary type prefix used for grouping together keys under some
     /// application and avoid namespace collision
-    #[cfg_attr(feature = "serde", serde(with = "crate::serde_utils::hex_bytes"))]
     pub prefix: Vec<u8>,
     /// Custom proprietary subtype
     pub subtype: Subtype,
     /// Additional key bytes (like serialized public key data etc)
-    #[cfg_attr(feature = "serde", serde(with = "crate::serde_utils::hex_bytes"))]
     pub key: Vec<u8>,
 }