fix(dashcore): make OutPoint serde tolerant of ContentDeserializer's HR-quirk

dash/Cargo.toml

@@ -67,7 +67,7 @@ serde_json = "1.0.140"
 serde_test = "1.0.177"
 serde_derive = "1.0.219"
 secp256k1 = { features = [ "recovery", "rand", "hashes" ], version="0.30.0" }
-bincode = { version = "2.0.1" }
+bincode = { version = "2.0.1", features = ["serde"] }
 assert_matches = "1.5.0"
 dashcore = { path = ".", features = ["core-block-hash-use-x11", "message_verification", "quorum_validation", "signer"] }
 criterion = "0.5"

dash/src/blockdata/transaction/outpoint.rs

@@ -337,6 +337,113 @@ mod tests {
         assert!(tx.out_point_buffer(1).is_none());
     }
 
+    /// Regression test for the bug where `OutPoint::deserialize` errored with
+    /// "invalid type: map, expected an OutPoint" when an OutPoint-bearing
+    /// struct was wrapped by an internally-tagged enum and round-tripped
+    /// through serde's intermediate `ContentDeserializer`. `ContentDeserializer`
+    /// always reports `is_human_readable() == true`, so a value originally
+    /// produced by a non-human-readable encoder ends up replayed into the HR
+    /// branch as a map — which the previous string-only HR visitor rejected.
+    #[cfg(feature = "serde")]
+    #[test]
+    fn serde_round_trip_through_internally_tagged_enum() {
+        use serde_derive::{Deserialize, Serialize};
+
+        #[derive(Debug, PartialEq, Serialize, Deserialize)]
+        struct WithOutPoint {
+            out_point: OutPoint,
+        }
+
+        #[derive(Debug, PartialEq, Serialize, Deserialize)]
+        #[serde(tag = "type")]
+        enum Tagged {
+            A(WithOutPoint),
+        }
+
+        let original = Tagged::A(WithOutPoint {
+            out_point: OutPoint {
+                txid: "5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c9456"
+                    .parse()
+                    .unwrap(),
+                vout: 7,
+            },
+        });
+
+        // Round-trip through serde_json::Value forces serde to buffer the value
+        // into a Content tree, then replay it through ContentDeserializer when
+        // resolving the internally-tagged enum variant. The canonical HR form
+        // serializes the OutPoint as a string, so this exercises the visit_str
+        // path through ContentDeserializer.
+        let value = serde_json::to_value(&original).unwrap();
+        let restored: Tagged = serde_json::from_value(value).unwrap();
+        assert_eq!(original, restored);
+
+        // Hand-build the struct/map form of the OutPoint inside the tagged
+        // enum and deserialize from it. This is the exact shape that triggered
+        // the original bug downstream (`platform_value::Value` produces struct
+        // shapes for OutPoint because it is non-human-readable, and the tagged
+        // enum then replays those through `ContentDeserializer` with
+        // `is_human_readable() == true`). Before the fix this failed with
+        // `invalid type: map, expected an OutPoint`.
+        let map_form = serde_json::json!({
+            "type": "A",
+            "out_point": {
+                "txid": "5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c9456",
+                "vout": 7,
+            },
+        });
+        let from_map: Tagged = serde_json::from_value(map_form).unwrap();
+        assert_eq!(original, from_map);
+
+        // The canonical HR string form must still deserialize, so existing
+        // JSON producers do not break.
+        let from_string: OutPoint = serde_json::from_str(
+            "\"5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c9456:7\"",
+        )
+        .unwrap();
+        assert_eq!(
+            from_string,
+            OutPoint {
+                txid: "5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c9456"
+                    .parse()
+                    .unwrap(),
+                vout: 7,
+            },
+        );
+
+        // Plain bincode (non-human-readable) round-trip of an `OutPoint`
+        // must still succeed via the struct-shape branch — guards against
+        // breaking the `visit_seq` path. (Note: a bincode round-trip of the
+        // tagged enum above is *not* possible in serde at all — internally-
+        // tagged enum dispatch requires `deserialize_any` on the upstream
+        // deserializer, and bincode is not self-describing. That's a serde
+        // limitation orthogonal to this bug.)
+        let raw = OutPoint {
+            txid: "5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c9456"
+                .parse()
+                .unwrap(),
+            vout: 7,
+        };
+        let cfg = bincode::config::standard();
+        let bytes = bincode::serde::encode_to_vec(raw, cfg).unwrap();
+        let (decoded, _): (OutPoint, _) = bincode::serde::decode_from_slice(&bytes, cfg).unwrap();
+        assert_eq!(raw, decoded);
+
+        // Duplicate field in the map form must error with `duplicate field`,
+        // not silently keep the last value. Parse from a JSON string (rather
+        // than `serde_json::json!`) because `Value::Object` deduplicates keys
+        // on construction — the duplicate must reach the visitor as separate
+        // map entries, which only happens during streaming parse.
+        let err = serde_json::from_str::<OutPoint>(
+            r#"{"txid":"5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c9456","txid":"0000000000000000000000000000000000000000000000000000000000000000","vout":7}"#,
+        )
+        .unwrap_err();
+        assert!(
+            err.to_string().contains("duplicate field"),
+            "expected duplicate-field error, got: {err}",
+        );
+    }
+
     // #[test]
     // fn out_point_parse() {
     //     let mut tx = Transaction {

dash/src/serde_utils.rs

@@ -358,143 +358,155 @@ pub(crate) use {serde_string_deserialize_impl, serde_string_impl, serde_string_s
 
 /// A combination macro where the human-readable serialization is done like
 /// serde_string_impl and the non-human-readable impl is done as a struct.
+///
+/// The deserialize impl uses a *single* visitor that handles all three shapes
+/// (`visit_str`, `visit_seq`, `visit_map`) — required to interoperate with
+/// serde's `ContentDeserializer`, the format-agnostic intermediate buffer
+/// serde uses to dispatch internally-tagged enums (`#[serde(tag = "...")]`),
+/// `flatten`, and untagged enums. `ContentDeserializer` always reports
+/// `is_human_readable() == true` regardless of the upstream format (this is
+/// intentional in serde's source: see long-standing upstream issues — the
+/// recommended pattern is "don't branch on `is_human_readable()` for shape
+/// dispatch — accept any shape"). A value originally written by a
+/// non-human-readable encoder can therefore be replayed into the
+/// human-readable branch as a map and must be accepted there. See the
+/// regression test
+/// `outpoint::tests::serde_round_trip_through_internally_tagged_enum`.
 macro_rules! serde_struct_human_string_impl {
     ($name:ident, $expecting:literal, $($fe:ident),*) => (
         impl<'de> $crate::serde::Deserialize<'de> for $name {
             fn deserialize<D>(deserializer: D) -> Result<$name, D::Error>
             where
                 D: $crate::serde::de::Deserializer<'de>,
             {
-                if deserializer.is_human_readable() {
-                    use core::fmt::{self, Formatter};
-                    use core::str::FromStr;
-
-                    struct Visitor;
-                    impl<'de> $crate::serde::de::Visitor<'de> for Visitor {
-                        type Value = $name;
+                use core::fmt::{self, Formatter};
+                use core::str::FromStr;
+                use $crate::serde::de::IgnoredAny;
 
-                        fn expecting(&self, f: &mut Formatter) -> fmt::Result {
-                            f.write_str($expecting)
-                        }
+                #[allow(non_camel_case_types)]
+                enum Enum { Unknown__Field, $($fe),* }
 
-                        fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
-                        where
-                            E: $crate::serde::de::Error,
-                        {
-                            $name::from_str(v).map_err(E::custom)
-                        }
+                struct EnumVisitor;
+                impl<'de> $crate::serde::de::Visitor<'de> for EnumVisitor {
+                    type Value = Enum;
 
+                    fn expecting(&self, f: &mut Formatter) -> fmt::Result {
+                        f.write_str("a field name")
                     }
 
-                    deserializer.deserialize_str(Visitor)
-                } else {
-                    use core::fmt::{self, Formatter};
-                    use $crate::serde::de::IgnoredAny;
-
-                    #[allow(non_camel_case_types)]
-                    enum Enum { Unknown__Field, $($fe),* }
-
-                    struct EnumVisitor;
-                    impl<'de> $crate::serde::de::Visitor<'de> for EnumVisitor {
-                        type Value = Enum;
-
-                        fn expecting(&self, f: &mut Formatter) -> fmt::Result {
-                            f.write_str("a field name")
-                        }
-
-                        fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
-                        where
-                            E: $crate::serde::de::Error,
-                        {
-                            match v {
-                                $(
-                                stringify!($fe) => Ok(Enum::$fe)
-                                ),*,
-                                _ => Ok(Enum::Unknown__Field)
-                            }
+                    fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
+                    where
+                        E: $crate::serde::de::Error,
+                    {
+                        match v {
+                            $(
+                            stringify!($fe) => Ok(Enum::$fe)
+                            ),*,
+                            _ => Ok(Enum::Unknown__Field)
                         }
                     }
+                }
 
-                    impl<'de> $crate::serde::Deserialize<'de> for Enum {
-                        fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-                        where
-                            D: $crate::serde::de::Deserializer<'de>,
-                        {
-                            deserializer.deserialize_str(EnumVisitor)
-                        }
+                impl<'de> $crate::serde::Deserialize<'de> for Enum {
+                    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+                    where
+                        D: $crate::serde::de::Deserializer<'de>,
+                    {
+                        deserializer.deserialize_str(EnumVisitor)
                     }
+                }
 
-                    struct Visitor;
-
-                    impl<'de> $crate::serde::de::Visitor<'de> for Visitor {
-                        type Value = $name;
+                struct Visitor;
 
-                        fn expecting(&self, f: &mut Formatter) -> fmt::Result {
-                            f.write_str("a struct")
-                        }
+                impl<'de> $crate::serde::de::Visitor<'de> for Visitor {
+                    type Value = $name;
 
-                        fn visit_seq<V>(self, mut seq: V) -> Result<Self::Value, V::Error>
-                        where
-                            V: $crate::serde::de::SeqAccess<'de>,
-                        {
-                            use $crate::serde::de::Error;
+                    fn expecting(&self, f: &mut Formatter) -> fmt::Result {
+                        f.write_str($expecting)
+                    }
 
-                            let length = 0;
-                            $(
-                                let $fe = seq.next_element()?.ok_or_else(|| {
-                                    Error::invalid_length(length, &self)
-                                })?;
-                                #[allow(unused_variables)]
-                                let length = length + 1;
-                            )*
-
-                            let ret = $name {
-                                $($fe),*
-                            };
+                    fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
+                    where
+                        E: $crate::serde::de::Error,
+                    {
+                        $name::from_str(v).map_err(E::custom)
+                    }
 
-                            Ok(ret)
-                        }
+                    fn visit_seq<V>(self, mut seq: V) -> Result<Self::Value, V::Error>
+                    where
+                        V: $crate::serde::de::SeqAccess<'de>,
+                    {
+                        use $crate::serde::de::Error;
+
+                        let length = 0;
+                        $(
+                            let $fe = seq.next_element()?.ok_or_else(|| {
+                                Error::invalid_length(length, &self)
+                            })?;
+                            #[allow(unused_variables)]
+                            let length = length + 1;
+                        )*
+
+                        let ret = $name {
+                            $($fe),*
+                        };
+
+                        Ok(ret)
+                    }
 
-                        fn visit_map<A>(self, mut map: A) -> Result<Self::Value, A::Error>
-                        where
-                            A: $crate::serde::de::MapAccess<'de>,
-                        {
-                            use $crate::serde::de::Error;
+                    fn visit_map<A>(self, mut map: A) -> Result<Self::Value, A::Error>
+                    where
+                        A: $crate::serde::de::MapAccess<'de>,
+                    {
+                        use $crate::serde::de::Error;
 
-                            $(let mut $fe = None;)*
+                        $(let mut $fe = None;)*
 
-                            loop {
-                                match map.next_key::<Enum>()? {
-                                    Some(Enum::Unknown__Field) => {
-                                        map.next_value::<IgnoredAny>()?;
-                                    }
-                                    $(
-                                        Some(Enum::$fe) => {
-                                            $fe = Some(map.next_value()?);
-                                        }
-                                    )*
-                                    None => { break; }
+                        loop {
+                            match map.next_key::<Enum>()? {
+                                Some(Enum::Unknown__Field) => {
+                                    map.next_value::<IgnoredAny>()?;
                                 }
+                                $(
+                                    Some(Enum::$fe) => {
+                                        if $fe.is_some() {
+                                            return Err(A::Error::duplicate_field(stringify!($fe)));
+                                        }
+                                        $fe = Some(map.next_value()?);
+                                    }
+                                )*
+                                None => { break; }
                             }
+                        }
 
-                            $(
-                                let $fe = match $fe {
-                                    Some(x) => x,
-                                    None => return Err(A::Error::missing_field(stringify!($fe))),
-                                };
-                            )*
-
-                            let ret = $name {
-                                $($fe),*
+                        $(
+                            let $fe = match $fe {
+                                Some(x) => x,
+                                None => return Err(A::Error::missing_field(stringify!($fe))),
                             };
+                        )*
 
-                            Ok(ret)
-                        }
+                        let ret = $name {
+                            $($fe),*
+                        };
+
+                        Ok(ret)
                     }
-                    // end type defs
+                }
+                // end type defs
 
-                    static FIELDS: &'static [&'static str] = &[$(stringify!($fe)),*];
+                static FIELDS: &'static [&'static str] = &[$(stringify!($fe)),*];
 
+                if deserializer.is_human_readable() {
+                    // Self-describing format (raw JSON, or a `ContentDeserializer`
+                    // replaying buffered content for an internally-tagged enum).
+                    // `deserialize_any` lets the deserializer dispatch to whichever
+                    // visit_* method matches the actual data shape — so we accept
+                    // the canonical "txid:vout" string form, plus the struct/seq
+                    // forms that show up when a non-human-readable struct is
+                    // re-deserialized via `ContentDeserializer`.
+                    deserializer.deserialize_any(Visitor)
+                } else {
                     deserializer.deserialize_struct(stringify!($name), FIELDS, Visitor)
                 }
             }