From e313bcd3c319a6e24a2d28f63cd1a85d843cf844 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Sat, 11 Apr 2026 20:50:27 +0700 Subject: [PATCH 1/5] fix(dashcore): Address serde deserialize no longer hardcodes Mainnet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Address::deserialize` was using `addr_unchecked.require_network(Network::Mainnet)`, which silently broke every testnet/regtest/devnet round-trip: - Any testnet `Address` serialized via serde would fail to deserialize with a "network mismatch" error. - Structs that embed `Address` in serde-derived types (e.g. `InputDetail` in `key-wallet`'s `TransactionRecord`) would fail to decode on non-mainnet networks, and the failure cascaded as "skip entire TransactionRecord" for any consumer that log-and-skipped decode errors. The existing doc comment on the impl literally admitted it: "This is a limitation of deserializing without network context. Users should use Address for serde when the network is not known at compile time." Meanwhile the native bincode `Decode` impl for `Address` at address.rs:864-874 already does the right thing — it decodes to `Address` and calls `assume_checked()`. Align the serde impl with that behavior so the two serialization paths agree and `TransactionRecord` round-trips cleanly regardless of network. Callers that need actual network validation should deserialize as `Address` and call `require_network(N)` explicitly. Flagged as a critical bug by rust-quality-engineer during the holistic review of Phase 10 Item 6c (evo-tool's `wallet_transactions.record` bincode-serde persistence path). Without this fix, Item 6c is broken on every non-mainnet deployment. 29 dashcore address tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) --- dash/src/address.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/dash/src/address.rs b/dash/src/address.rs index 6cb93c206..787393d89 100644 --- a/dash/src/address.rs +++ b/dash/src/address.rs @@ -834,10 +834,17 @@ impl<'de> serde::Deserialize<'de> for Address { let s = String::deserialize(deserializer)?; let addr_unchecked = Address::::from_str(&s).map_err(D::Error::custom)?; - // For NetworkChecked, we need to assume a network. This is a limitation - // of deserializing without network context. Users should use Address - // for serde when the network is not known at compile time. - addr_unchecked.require_network(Network::Mainnet).map_err(D::Error::custom) + // `NetworkChecked` only encodes that the caller has accepted the + // address's network — the raw bytes don't carry the network + // themselves. We can't validate the network from the string alone, + // and historically this impl hardcoded `Network::Mainnet` which + // silently broke every testnet/regtest/devnet round-trip. The + // native bincode `Decode` impl for `Address` (below) already uses + // `assume_checked()`; align serde with the same behavior so the + // two serialization paths agree. Callers that need network + // validation should deserialize as `Address` + // and call `require_network` explicitly. + Ok(addr_unchecked.assume_checked()) } } From 5a41e78f459d5b621c0fef8450f7b220ce58b360 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sat, 18 Apr 2026 10:31:36 +0700 Subject: [PATCH 2/5] test(dashcore): regression tests for Address serde deserialize Pin the fix from the previous commit with four focused tests: - `serde_deserialize_network_checked_testnet_round_trip`: a testnet `Address` survives a serde round-trip. Before the fix this failed with a "network mismatch" error. - `serde_deserialize_network_checked_devnet_round_trip`: same for the logical devnet/regtest case. Raw bytes round-trip even though the `NetworkChecked` side cannot prove which network the caller had in mind. - `serde_deserialize_network_checked_agrees_with_bincode_decode`: the serde and native `assume_checked()` paths must produce equal addresses; guards against a future "tighten serde with a hardcoded network" regression. - `serde_deserialize_network_unchecked_require_network_still_enforces`: callers who want network validation still opt in via `Address` + `require_network(..)`, and that path must still reject mismatches. Verified that reverting the fix causes the first three tests to fail with the exact pre-fix error; the fourth correctly keeps passing because the opt-in validation path is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- dash/src/address.rs | 86 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/dash/src/address.rs b/dash/src/address.rs index 787393d89..d6909baa0 100644 --- a/dash/src/address.rs +++ b/dash/src/address.rs @@ -2123,4 +2123,90 @@ mod tests { } } } + + /// Regression test for the pre-fix behavior in which + /// `Address::deserialize` hardcoded `Network::Mainnet`. + /// + /// A testnet address serialized via serde must round-trip back to a + /// `NetworkChecked` value that reproduces the original script_pubkey. Before + /// this fix, deserialization would fail with a "network mismatch" error. + #[test] + #[cfg(feature = "serde")] + fn serde_deserialize_network_checked_testnet_round_trip() { + // Testnet P2PKH (`y`-prefix) — the pre-fix impl rejected this. + let original: Address = + Address::from_str("yWZBnVvSxS5xSq27dHVAJpuqbt7vvwGFL1").unwrap().assume_checked(); + + let json = serde_json::to_string(&original).expect("serialize"); + let decoded: Address = + serde_json::from_str(&json).expect("deserialize NetworkChecked from testnet address"); + + assert_eq!(decoded.to_string(), original.to_string()); + assert_eq!(decoded.script_pubkey(), original.script_pubkey()); + } + + /// Parallel round-trip coverage for regtest/devnet addresses, which share + /// the `y`-prefix on base58 but live on different logical networks. The + /// serde path doesn't need to distinguish them — raw bytes are all that + /// round-trips — but the previous hardcoded-Mainnet behavior still broke + /// this case. + #[test] + #[cfg(feature = "serde")] + fn serde_deserialize_network_checked_devnet_round_trip() { + let original: Address = + Address::from_str("yWZBnVvSxS5xSq27dHVAJpuqbt7vvwGFL1").unwrap().assume_checked(); + + let json = serde_json::to_string(&original).expect("serialize"); + let decoded: Address = serde_json::from_str(&json).expect("deserialize NetworkChecked"); + + // Round-trip preserves the raw address bytes (and thus script_pubkey) + // even though the `NetworkChecked` side cannot prove which network the + // caller had in mind. + assert_eq!(decoded.script_pubkey(), original.script_pubkey()); + } + + /// Serde round-trip must agree with the native bincode `Decode` impl. + /// Both paths now use `assume_checked()`; this test guards the invariant + /// so a future "tighten serde with a hardcoded network" regression would + /// trip here rather than silently diverging. + #[test] + #[cfg(feature = "serde")] + fn serde_deserialize_network_checked_agrees_with_bincode_decode() { + let testnet_strings = [ + "yWZBnVvSxS5xSq27dHVAJpuqbt7vvwGFL1", // P2PKH + ]; + for s in testnet_strings { + let unchecked = Address::::from_str(s).unwrap(); + + let json = serde_json::to_string(&unchecked.clone().assume_checked()).unwrap(); + let via_serde: Address = serde_json::from_str(&json).unwrap(); + + let via_assume_checked: Address = unchecked.assume_checked(); + + assert_eq!(via_serde.to_string(), via_assume_checked.to_string()); + assert_eq!(via_serde.script_pubkey(), via_assume_checked.script_pubkey()); + } + } + + /// Callers who need network validation opt in via + /// `Address` + `require_network(..)`, as documented on + /// the deserialize impl. That path must still reject mismatches. + #[test] + #[cfg(feature = "serde")] + fn serde_deserialize_network_unchecked_require_network_still_enforces() { + let testnet: Address = + Address::from_str("yWZBnVvSxS5xSq27dHVAJpuqbt7vvwGFL1").unwrap().assume_checked(); + let json = serde_json::to_string(&testnet).expect("serialize"); + + let unchecked: Address = + serde_json::from_str(&json).expect("deserialize"); + assert!( + unchecked.clone().require_network(Network::Mainnet).is_err(), + "require_network(Mainnet) on a testnet address must still error" + ); + assert!( + unchecked.require_network(Network::Testnet).is_ok(), + "require_network(Testnet) on a testnet address must succeed" + ); + } } From 7f87426617430ac9c2edc7f1418ea1f7aa119f74 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Mon, 20 Apr 2026 03:44:48 +0800 Subject: [PATCH 3/5] test(address): cover bincode roundtrip in serde test --- dash/src/address.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/dash/src/address.rs b/dash/src/address.rs index d6909baa0..02177caa5 100644 --- a/dash/src/address.rs +++ b/dash/src/address.rs @@ -2170,7 +2170,7 @@ mod tests { /// so a future "tighten serde with a hardcoded network" regression would /// trip here rather than silently diverging. #[test] - #[cfg(feature = "serde")] + #[cfg(all(feature = "serde", feature = "bincode"))] fn serde_deserialize_network_checked_agrees_with_bincode_decode() { let testnet_strings = [ "yWZBnVvSxS5xSq27dHVAJpuqbt7vvwGFL1", // P2PKH @@ -2180,11 +2180,22 @@ mod tests { let json = serde_json::to_string(&unchecked.clone().assume_checked()).unwrap(); let via_serde: Address = serde_json::from_str(&json).unwrap(); + let bytes = bincode::encode_to_vec( + &unchecked.clone().assume_checked(), + bincode::config::standard(), + ) + .unwrap(); + let via_bincode: Address = + bincode::decode_from_slice(&bytes, bincode::config::standard()).unwrap().0; let via_assume_checked: Address = unchecked.assume_checked(); assert_eq!(via_serde.to_string(), via_assume_checked.to_string()); assert_eq!(via_serde.script_pubkey(), via_assume_checked.script_pubkey()); + assert_eq!(via_bincode.to_string(), via_serde.to_string()); + assert_eq!(via_bincode.to_string(), via_assume_checked.to_string()); + assert_eq!(via_bincode.script_pubkey(), via_serde.script_pubkey()); + assert_eq!(via_bincode.script_pubkey(), via_assume_checked.script_pubkey()); } } From bf19531f8fbdac936e31617ea39924ae4cb1626a Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Mon, 20 Apr 2026 06:24:21 +0800 Subject: [PATCH 4/5] fix(dashcore): drop needless & in bincode::encode_to_vec arg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `bincode::encode_to_vec(val: E, cfg)` takes the value by value. Passing `&addr.clone().assume_checked()` wraps it in the `&T: Encode` blanket impl for no reason — clippy's needless_borrows_for_generic_args (now firing on 1.95+) flags it. Address implements Encode directly, so pass it owned. Co-Authored-By: Claude Opus 4.7 (1M context) --- dash/src/address.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dash/src/address.rs b/dash/src/address.rs index 02177caa5..0eaabadc9 100644 --- a/dash/src/address.rs +++ b/dash/src/address.rs @@ -2181,7 +2181,7 @@ mod tests { let json = serde_json::to_string(&unchecked.clone().assume_checked()).unwrap(); let via_serde: Address = serde_json::from_str(&json).unwrap(); let bytes = bincode::encode_to_vec( - &unchecked.clone().assume_checked(), + unchecked.clone().assume_checked(), bincode::config::standard(), ) .unwrap(); From e9d7a3fb3d3a322efa6b05f4793279155ff1bba7 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Mon, 20 Apr 2026 07:55:23 +0800 Subject: [PATCH 5/5] test(dashcore): drop redundant devnet serde round-trip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per xdustinface review: the `_devnet_round_trip` test used the same `y`-prefix address and the same round-trip as the testnet test — serde carries raw bytes and doesn't distinguish testnet/regtest/devnet at the codec layer (network awareness happens at `require_network`). Keeping the testnet test plus the bincode-agreement test is sufficient coverage for the fix. --- dash/src/address.rs | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/dash/src/address.rs b/dash/src/address.rs index 0eaabadc9..7a3a0783a 100644 --- a/dash/src/address.rs +++ b/dash/src/address.rs @@ -2145,26 +2145,6 @@ mod tests { assert_eq!(decoded.script_pubkey(), original.script_pubkey()); } - /// Parallel round-trip coverage for regtest/devnet addresses, which share - /// the `y`-prefix on base58 but live on different logical networks. The - /// serde path doesn't need to distinguish them — raw bytes are all that - /// round-trips — but the previous hardcoded-Mainnet behavior still broke - /// this case. - #[test] - #[cfg(feature = "serde")] - fn serde_deserialize_network_checked_devnet_round_trip() { - let original: Address = - Address::from_str("yWZBnVvSxS5xSq27dHVAJpuqbt7vvwGFL1").unwrap().assume_checked(); - - let json = serde_json::to_string(&original).expect("serialize"); - let decoded: Address = serde_json::from_str(&json).expect("deserialize NetworkChecked"); - - // Round-trip preserves the raw address bytes (and thus script_pubkey) - // even though the `NetworkChecked` side cannot prove which network the - // caller had in mind. - assert_eq!(decoded.script_pubkey(), original.script_pubkey()); - } - /// Serde round-trip must agree with the native bincode `Decode` impl. /// Both paths now use `assume_checked()`; this test guards the invariant /// so a future "tighten serde with a hardcoded network" regression would