From ec54f45bb27b357e09fb19779c9f20c01c60c405 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 1 May 2026 18:27:47 +0700 Subject: [PATCH 1/6] fix(dashcore): make `OutPoint` serde survive ContentDeserializer replays MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `OutPoint`'s `Deserialize` impl branched on `is_human_readable()` and used two unrelated visitors — a string-only `StringVisitor` for HR and a struct-only `StructVisitor` for non-HR. Serde's `ContentDeserializer` (used internally for `#[serde(tag = "...")]`, `flatten`, and untagged enums) always reports `is_human_readable() == true` regardless of the upstream format, so a value originally produced as a non-HR struct could be replayed into the HR branch as a map and fail with `invalid type: map, expected an OutPoint`. Fix the `serde_struct_human_string_impl!` macro to use a single visitor that accepts all three input shapes (`visit_str`, `visit_seq`, `visit_map`) and use `deserialize_any` in the HR branch so the actual content shape is picked up regardless of what `is_human_readable` reports. The non-HR branch keeps `deserialize_struct` so bincode (which doesn't support `deserialize_any`) still works. The serialize side is unchanged. The trade-off: raw JSON now also accepts the struct/seq forms in addition to the canonical \`\"txid:vout\"\` string form, and tolerates unknown fields in the struct form. This is intentional and required — internally-tagged enums replay the entire buffered content (including the tag field) when resolving each variant, so \`visit_map\` must accept unknown fields. Canonical JSON output is unchanged because the serialize side still emits the string form. Add a regression test that round-trips an \`OutPoint\` through an internally-tagged enum via \`serde_json::Value\` (forcing the \`ContentDeserializer\` path) and through bincode. Co-Authored-By: Claude Opus 4.7 (1M context) --- dash/src/blockdata/transaction/outpoint.rs | 65 +++++++ dash/src/serde_utils.rs | 215 +++++++++++---------- 2 files changed, 175 insertions(+), 105 deletions(-) diff --git a/dash/src/blockdata/transaction/outpoint.rs b/dash/src/blockdata/transaction/outpoint.rs index d596a8fd6..2305c0b8a 100644 --- a/dash/src/blockdata/transaction/outpoint.rs +++ b/dash/src/blockdata/transaction/outpoint.rs @@ -337,6 +337,71 @@ 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. Before the fix this + // path failed with `invalid type: map, expected an OutPoint`. + let value = serde_json::to_value(&original).unwrap(); + let restored: Tagged = serde_json::from_value(value).unwrap(); + assert_eq!(original, restored); + + // 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, + }, + ); + + // Bincode (non-human-readable) round-trip must still succeed via the + // struct-shape branch. + let cfg = bincode::config::standard(); + let bytes = bincode::serde::encode_to_vec(&original, cfg).unwrap(); + let (decoded, _): (Tagged, _) = + bincode::serde::decode_from_slice(&bytes, cfg).unwrap(); + assert_eq!(original, decoded); + } + // #[test] // fn out_point_parse() { // let mut tx = Transaction { diff --git a/dash/src/serde_utils.rs b/dash/src/serde_utils.rs index 7110c9678..9a2c5310f 100644 --- a/dash/src/serde_utils.rs +++ b/dash/src/serde_utils.rs @@ -358,6 +358,16 @@ 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`) so it works correctly when serde +/// replays values through an intermediate buffer such as `ContentDeserializer` +/// — used by internally-tagged enums (`#[serde(tag = "...")]`), `flatten`, and +/// untagged enums. Those replays always report `is_human_readable() == true` +/// regardless of the upstream format, so a value that was originally written +/// as a non-human-readable struct can still be replayed into the human-readable +/// branch and must accept the map/seq shapes 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 { @@ -365,136 +375,131 @@ macro_rules! serde_struct_human_string_impl { where D: $crate::serde::de::Deserializer<'de>, { - if deserializer.is_human_readable() { - use core::fmt::{self, Formatter}; - use core::str::FromStr; + use core::fmt::{self, Formatter}; + use core::str::FromStr; + use $crate::serde::de::IgnoredAny; - struct Visitor; - impl<'de> $crate::serde::de::Visitor<'de> for Visitor { - type Value = $name; + #[allow(non_camel_case_types)] + enum Enum { Unknown__Field, $($fe),* } - fn expecting(&self, f: &mut Formatter) -> fmt::Result { - f.write_str($expecting) - } - - fn visit_str(self, v: &str) -> Result - 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(self, v: &str) -> Result - where - E: $crate::serde::de::Error, - { - match v { - $( - stringify!($fe) => Ok(Enum::$fe) - ),*, - _ => Ok(Enum::Unknown__Field) - } + fn visit_str(self, v: &str) -> Result + 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(deserializer: D) -> Result - where - D: $crate::serde::de::Deserializer<'de>, - { - deserializer.deserialize_str(EnumVisitor) - } + impl<'de> $crate::serde::Deserialize<'de> for Enum { + fn deserialize(deserializer: D) -> Result + 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(self, mut seq: V) -> Result - 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(self, v: &str) -> Result + where + E: $crate::serde::de::Error, + { + $name::from_str(v).map_err(E::custom) + } - Ok(ret) - } + fn visit_seq(self, mut seq: V) -> Result + 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(self, mut map: A) -> Result - where - A: $crate::serde::de::MapAccess<'de>, - { - use $crate::serde::de::Error; + fn visit_map(self, mut map: A) -> Result + 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::()? { - Some(Enum::Unknown__Field) => { - map.next_value::()?; - } - $( - Some(Enum::$fe) => { - $fe = Some(map.next_value()?); - } - )* - None => { break; } + loop { + match map.next_key::()? { + Some(Enum::Unknown__Field) => { + map.next_value::()?; } + $( + Some(Enum::$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) } } From 4dc2918525bd5c997459cf11412894d946ada3c9 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 1 May 2026 18:36:33 +0700 Subject: [PATCH 2/6] fix(test): drop bincode round-trip from OutPoint serde regression test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dev-dependency on `bincode` doesn't enable the `serde` feature, so `bincode::serde::encode_to_vec` is not in scope. The non-human-readable struct path is already exercised by the existing dashcore test suite which uses bincode extensively, so the regression test only needs to lock in the `ContentDeserializer` behavior — kept the JSON-via-tagged- enum and string-form assertions. Co-Authored-By: Claude Opus 4.7 (1M context) --- dash/src/blockdata/transaction/outpoint.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/dash/src/blockdata/transaction/outpoint.rs b/dash/src/blockdata/transaction/outpoint.rs index 2305c0b8a..8acc7cf8d 100644 --- a/dash/src/blockdata/transaction/outpoint.rs +++ b/dash/src/blockdata/transaction/outpoint.rs @@ -392,14 +392,6 @@ mod tests { vout: 7, }, ); - - // Bincode (non-human-readable) round-trip must still succeed via the - // struct-shape branch. - let cfg = bincode::config::standard(); - let bytes = bincode::serde::encode_to_vec(&original, cfg).unwrap(); - let (decoded, _): (Tagged, _) = - bincode::serde::decode_from_slice(&bytes, cfg).unwrap(); - assert_eq!(original, decoded); } // #[test] From aef57a40c0a9d429d499f4c8a6a5d10c97824a5f Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 1 May 2026 18:42:14 +0700 Subject: [PATCH 3/6] test(dashcore): restore bincode side of OutPoint serde tagged-enum test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding `features = ["serde"]` to the bincode dev-dep so the regression test can exercise the symmetric non-human-readable struct path through the tagged enum, not just the human-readable JSON path. Reword the macro doc comment so it frames the underlying issue as serde's `ContentDeserializer` HR-quirk rather than implying a bug specific to dashcore — the same sharp edge bites every custom `Deserialize` that uses `is_human_readable()` for shape dispatch and the upstream stance is "accept any shape, don't branch". Co-Authored-By: Claude Opus 4.7 (1M context) --- dash/Cargo.toml | 2 +- dash/src/blockdata/transaction/outpoint.rs | 8 ++++++++ dash/src/serde_utils.rs | 18 +++++++++++------- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/dash/Cargo.toml b/dash/Cargo.toml index a6574f853..22d697b9b 100644 --- a/dash/Cargo.toml +++ b/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" diff --git a/dash/src/blockdata/transaction/outpoint.rs b/dash/src/blockdata/transaction/outpoint.rs index 8acc7cf8d..71ec8faba 100644 --- a/dash/src/blockdata/transaction/outpoint.rs +++ b/dash/src/blockdata/transaction/outpoint.rs @@ -392,6 +392,14 @@ mod tests { vout: 7, }, ); + + // Bincode (non-human-readable) round-trip via the tagged enum must + // still succeed via the struct-shape branch — symmetric to the JSON + // case above. + let cfg = bincode::config::standard(); + let bytes = bincode::serde::encode_to_vec(&original, cfg).unwrap(); + let (decoded, _): (Tagged, _) = bincode::serde::decode_from_slice(&bytes, cfg).unwrap(); + assert_eq!(original, decoded); } // #[test] diff --git a/dash/src/serde_utils.rs b/dash/src/serde_utils.rs index 9a2c5310f..104f269e5 100644 --- a/dash/src/serde_utils.rs +++ b/dash/src/serde_utils.rs @@ -360,13 +360,17 @@ pub(crate) use {serde_string_deserialize_impl, serde_string_impl, serde_string_s /// 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`) so it works correctly when serde -/// replays values through an intermediate buffer such as `ContentDeserializer` -/// — used by internally-tagged enums (`#[serde(tag = "...")]`), `flatten`, and -/// untagged enums. Those replays always report `is_human_readable() == true` -/// regardless of the upstream format, so a value that was originally written -/// as a non-human-readable struct can still be replayed into the human-readable -/// branch and must accept the map/seq shapes there. See the regression test +/// (`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),*) => ( From 622484d4609606487e86b5df3f6932670498e5be Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 1 May 2026 18:51:51 +0700 Subject: [PATCH 4/6] test(dashcore): use plain OutPoint bincode round-trip instead of tagged MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bincode round-trip *via* a `#[serde(tag = ...)]` enum can't work — serde internally-tagged dispatch buffers content by calling `deserialize_any` on the upstream deserializer, and bincode is not self-describing (`Serde(AnyNotSupported)`). That limitation is orthogonal to the bug this PR fixes — the original failing case in dashpay/platform routes through `platform_value::Value`, which *does* support `deserialize_any`. Replace the bogus bincode-tagged-enum assertion with a plain `OutPoint` bincode round-trip, which still locks the non-human-readable struct path (`visit_seq`). Co-Authored-By: Claude Opus 4.7 (1M context) --- dash/src/blockdata/transaction/outpoint.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/dash/src/blockdata/transaction/outpoint.rs b/dash/src/blockdata/transaction/outpoint.rs index 71ec8faba..bd0ca4dbf 100644 --- a/dash/src/blockdata/transaction/outpoint.rs +++ b/dash/src/blockdata/transaction/outpoint.rs @@ -393,13 +393,23 @@ mod tests { }, ); - // Bincode (non-human-readable) round-trip via the tagged enum must - // still succeed via the struct-shape branch — symmetric to the JSON - // case above. + // 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(&original, cfg).unwrap(); - let (decoded, _): (Tagged, _) = bincode::serde::decode_from_slice(&bytes, cfg).unwrap(); - assert_eq!(original, decoded); + 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); } // #[test] From 863159c297fdc1adff61d7e23b439c16f0df0427 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 1 May 2026 18:56:04 +0700 Subject: [PATCH 5/6] fix(test): drop needless borrow flagged by clippy `bincode::serde::encode_to_vec` takes the value by `Serialize` bound and `OutPoint` is `Copy`, so the `&raw` borrow is redundant (`clippy::needless_borrows_for_generic_args`). Co-Authored-By: Claude Opus 4.7 (1M context) --- dash/src/blockdata/transaction/outpoint.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dash/src/blockdata/transaction/outpoint.rs b/dash/src/blockdata/transaction/outpoint.rs index bd0ca4dbf..1a6030a58 100644 --- a/dash/src/blockdata/transaction/outpoint.rs +++ b/dash/src/blockdata/transaction/outpoint.rs @@ -407,7 +407,7 @@ mod tests { vout: 7, }; let cfg = bincode::config::standard(); - let bytes = bincode::serde::encode_to_vec(&raw, cfg).unwrap(); + 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); } From be00e518df76a642cc44ba7d6729daac168326c4 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 1 May 2026 19:39:52 +0700 Subject: [PATCH 6/6] review: add map-form regression test and reject duplicate fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address CodeRabbit review feedback on the OutPoint serde fix: 1. Add a test that hand-builds the JSON object/map shape of `OutPoint` inside a `#[serde(tag = ...)]` enum and deserializes from it. This is the exact shape the original bug exhibited downstream — serde serialized as a map (because the upstream format was non-human- readable), then the tagged enum replayed through `ContentDeserializer` into the HR branch where the old string-only visitor errored. The `serde_json::to_value` round-trip was only exercising the visit_str path through `ContentDeserializer` because canonical OutPoint HR serialization is a string. 2. Reject duplicate field keys in `serde_struct_human_string_impl!`'s `visit_map`. Previously a duplicate silently kept the last value; now it errors with `duplicate field` like serde's derive. Add a test covering this — parse from a raw JSON string (not `json!`) because `Value::Object` deduplicates on construction. Co-Authored-By: Claude Opus 4.7 (1M context) --- dash/src/blockdata/transaction/outpoint.rs | 36 ++++++++++++++++++++-- dash/src/serde_utils.rs | 3 ++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/dash/src/blockdata/transaction/outpoint.rs b/dash/src/blockdata/transaction/outpoint.rs index 1a6030a58..46095882f 100644 --- a/dash/src/blockdata/transaction/outpoint.rs +++ b/dash/src/blockdata/transaction/outpoint.rs @@ -371,12 +371,30 @@ mod tests { // 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. Before the fix this - // path failed with `invalid type: map, expected an OutPoint`. + // 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( @@ -410,6 +428,20 @@ mod tests { 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::( + r#"{"txid":"5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c9456","txid":"0000000000000000000000000000000000000000000000000000000000000000","vout":7}"#, + ) + .unwrap_err(); + assert!( + err.to_string().contains("duplicate field"), + "expected duplicate-field error, got: {err}", + ); } // #[test] diff --git a/dash/src/serde_utils.rs b/dash/src/serde_utils.rs index 104f269e5..cf8048501 100644 --- a/dash/src/serde_utils.rs +++ b/dash/src/serde_utils.rs @@ -469,6 +469,9 @@ macro_rules! serde_struct_human_string_impl { } $( Some(Enum::$fe) => { + if $fe.is_some() { + return Err(A::Error::duplicate_field(stringify!($fe))); + } $fe = Some(map.next_value()?); } )*