From 5397cef9b00ae10b0fa2d26167493591442e28fc Mon Sep 17 00:00:00 2001 From: jwinterm Date: Sun, 26 Jul 2026 13:51:36 -0400 Subject: [PATCH] Slate: validate the declared version when deserializing VersionedSlate and VersionedBinSlate are untagged and try V5 first, so without a version check a V4 slate parses as V5. Only visible without a payment proof; with one the V4 and V5 proof shapes differ enough for serde to pick the right variant by chance. Reject a mismatched version in the V4 and V5 deserializers, for both the JSON and binary encodings, and set the declared version from the conversion so it always matches the structure emitted. Add V4/V5 round trip tests over both encodings, with and without a proof. The V3 API examples showed V4 slates declaring version 5; they now declare 4. --- api/src/foreign_rpc.rs | 4 +- api/src/owner_rpc.rs | 6 +-- libwallet/src/slate.rs | 20 ++++----- libwallet/src/slate_versions/mod.rs | 62 +++++++++++++++++++++++++- libwallet/src/slate_versions/ser.rs | 20 ++++++++- libwallet/src/slate_versions/v4_bin.rs | 11 ++++- libwallet/src/slate_versions/v5_bin.rs | 11 ++++- libwallet/src/slatepack/packer.rs | 12 ++--- 8 files changed, 118 insertions(+), 28 deletions(-) diff --git a/api/src/foreign_rpc.rs b/api/src/foreign_rpc.rs index 028a69e..6b67b6e 100644 --- a/api/src/foreign_rpc.rs +++ b/api/src/foreign_rpc.rs @@ -144,7 +144,7 @@ pub trait ForeignRpc { } ], "sta": "S1", - "ver": "5:2" + "ver": "4:2" }, null, null @@ -179,7 +179,7 @@ pub trait ForeignRpc { } ], "sta": "S2", - "ver": "5:2" + "ver": "4:2" } } } diff --git a/api/src/owner_rpc.rs b/api/src/owner_rpc.rs index 8b8a2bf..6318071 100644 --- a/api/src/owner_rpc.rs +++ b/api/src/owner_rpc.rs @@ -511,7 +511,7 @@ pub trait OwnerRpc { } ], "sta": "S1", - "ver": "5:2" + "ver": "4:2" } } } @@ -589,7 +589,7 @@ pub trait OwnerRpc { } ], "sta": "I1", - "ver": "5:2" + "ver": "4:2" } } } @@ -960,7 +960,7 @@ pub trait OwnerRpc { "id": "0436430c-2b02-624c-2032-570501212b00", "sigs": [], "sta": "S3", - "ver": "5:3" + "ver": "4:3" } } } diff --git a/libwallet/src/slate.rs b/libwallet/src/slate.rs index 07d59dd..025e7e2 100644 --- a/libwallet/src/slate.rs +++ b/libwallet/src/slate.rs @@ -914,14 +914,15 @@ impl From<&KernelFeaturesArgs> for KernelFeaturesArgsV5 { impl From<&VersionCompatInfo> for VersionCompatInfoV5 { fn from(data: &VersionCompatInfo) -> VersionCompatInfoV5 { let VersionCompatInfo { - version, + version: _, block_header_version, } = data; - let version = *version; - let block_header_version = *block_header_version; + // The declared version describes the structure we are producing, so it is fixed + // here rather than carried over from the internal slate. Readers rely on it to + // select the matching variant. VersionCompatInfoV5 { - version, - block_header_version, + version: 5, + block_header_version: *block_header_version, } } } @@ -1360,14 +1361,13 @@ impl From<&KernelFeaturesArgs> for KernelFeaturesArgsV4 { impl From<&VersionCompatInfo> for VersionCompatInfoV4 { fn from(data: &VersionCompatInfo) -> VersionCompatInfoV4 { let VersionCompatInfo { - version, + version: _, block_header_version, } = data; - let version = *version; - let block_header_version = *block_header_version; + // See VersionCompatInfoV5: the declared version always matches the structure. VersionCompatInfoV4 { - version, - block_header_version, + version: 4, + block_header_version: *block_header_version, } } } diff --git a/libwallet/src/slate_versions/mod.rs b/libwallet/src/slate_versions/mod.rs index 5249a38..1b94c45 100644 --- a/libwallet/src/slate_versions/mod.rs +++ b/libwallet/src/slate_versions/mod.rs @@ -153,13 +153,15 @@ pub mod tests { use crate::slate::{KernelFeaturesArgs, ParticipantData, PaymentInfo, PaymentMemo}; use crate::slate_versions::v5::{CommitsV5, SlateV5}; use crate::{ - slate, Error, Slate, Slatepacker, SlatepackerArgs, VersionedBinSlate, VersionedSlate, + slate, Error, Slate, SlateVersion, Slatepacker, SlatepackerArgs, VersionedBinSlate, + VersionedSlate, }; use chrono::{DateTime, Utc}; use ed25519_dalek::Signature as DalekSignature; use ed25519_dalek::VerifyingKey as DalekPublicKey; use grin_core::global::{set_local_chain_type, ChainTypes}; use grin_keychain::{ExtKeychain, Keychain, SwitchCommitmentType}; + use grin_wallet_util::byte_ser; use std::convert::TryInto; /// Populate a test internal slate with all fields to test conversions @@ -323,6 +325,64 @@ pub mod tests { Ok(()) } + // A slate must deserialize back into the version it declares. VersionedSlate is + // untagged and tries V5 first, so without a version check in each deserializer a + // V4 slate is silently parsed (and reported) as V5. + #[test] + fn versioned_slate_json_round_trip() -> Result<(), Error> { + set_local_chain_type(ChainTypes::Mainnet); + let slate_internal = populate_test_slate()?; + + // A slate without a payment proof is the case that matters: the V4 and V5 proof + // shapes differ, so a proof makes the variants distinguishable by accident. + for with_proof in [true, false] { + for version in [SlateVersion::V4, SlateVersion::V5] { + let mut slate = slate_internal.clone(); + if !with_proof { + slate.payment_proof = None; + } + slate.version_info.version = match version { + SlateVersion::V4 => 4, + SlateVersion::V5 => 5, + }; + let versioned = VersionedSlate::into_version(slate, version.clone())?; + let json = serde_json::to_string(&versioned).unwrap(); + let recovered: VersionedSlate = serde_json::from_str(&json).unwrap(); + assert_eq!(recovered.version(), version, "json round trip: {}", json); + } + } + + Ok(()) + } + + #[test] + fn versioned_bin_slate_round_trip() -> Result<(), Error> { + set_local_chain_type(ChainTypes::Mainnet); + let slate_internal = populate_test_slate()?; + + for with_proof in [true, false] { + for version in [SlateVersion::V4, SlateVersion::V5] { + let mut slate = slate_internal.clone(); + if !with_proof { + slate.payment_proof = None; + } + slate.version_info.version = match version { + SlateVersion::V4 => 4, + SlateVersion::V5 => 5, + }; + let versioned = VersionedSlate::into_version(slate, version.clone())?; + let bin: VersionedBinSlate = versioned.try_into().unwrap(); + let bytes = byte_ser::to_bytes(&bin).unwrap(); + let recovered: VersionedSlate = byte_ser::from_bytes::(&bytes) + .unwrap() + .into(); + assert_eq!(recovered.version(), version); + } + } + + Ok(()) + } + #[test] fn slatepack_version_v4_v5_bin() -> Result<(), Error> { set_local_chain_type(ChainTypes::Mainnet); diff --git a/libwallet/src/slate_versions/ser.rs b/libwallet/src/slate_versions/ser.rs index 845c351..7a7a431 100644 --- a/libwallet/src/slate_versions/ser.rs +++ b/libwallet/src/slate_versions/ser.rs @@ -526,7 +526,15 @@ pub mod version_info_v4 { return Err(Error::custom("Cannot parse version")); } match u16::from_str_radix(v[0], 10) { - Ok(u) => retval.version = u, + // VersionedSlate is untagged, so the declared version must be checked here: + // otherwise a slate is silently parsed as the wrong variant. + Ok(4) => retval.version = 4, + Ok(u) => { + return Err(Error::custom(format!( + "Expected a V4 slate, found version {}", + u + ))) + } Err(e) => return Err(Error::custom(format!("Cannot parse version: {}", e))), } match u16::from_str_radix(v[1], 10) { @@ -568,7 +576,15 @@ pub mod version_info_v5 { return Err(Error::custom("Cannot parse version")); } match u16::from_str_radix(v[0], 10) { - Ok(u) => retval.version = u, + // See the V4 deserializer: the declared version disambiguates the + // untagged VersionedSlate variants. + Ok(5) => retval.version = 5, + Ok(u) => { + return Err(Error::custom(format!( + "Expected a V5 slate, found version {}", + u + ))) + } Err(e) => return Err(Error::custom(format!("Cannot parse version: {}", e))), } match u16::from_str_radix(v[1], 10) { diff --git a/libwallet/src/slate_versions/v4_bin.rs b/libwallet/src/slate_versions/v4_bin.rs index dfb073c..8a66a31 100644 --- a/libwallet/src/slate_versions/v4_bin.rs +++ b/libwallet/src/slate_versions/v4_bin.rs @@ -463,8 +463,17 @@ impl Writeable for SlateV4Bin { impl Readable for SlateV4Bin { fn read(reader: &mut R) -> Result { + // VersionedBinSlate is untagged, so reject a slate that does not declare V4 here; + // otherwise it can be parsed as the wrong variant. + let version = reader.read_u16()?; + if version != 4 { + return Err(grin_ser::Error::UnexpectedData { + expected: 4u16.to_be_bytes().to_vec(), + received: version.to_be_bytes().to_vec(), + }); + } let ver = VersionCompatInfoV4 { - version: reader.read_u16()?, + version, block_header_version: reader.read_u16()?, }; let id = UuidWrap::read(reader)?.0; diff --git a/libwallet/src/slate_versions/v5_bin.rs b/libwallet/src/slate_versions/v5_bin.rs index 43ed3ef..7af92f7 100644 --- a/libwallet/src/slate_versions/v5_bin.rs +++ b/libwallet/src/slate_versions/v5_bin.rs @@ -382,8 +382,17 @@ impl Writeable for SlateV5Bin { impl Readable for SlateV5Bin { fn read(reader: &mut R) -> Result { + // See SlateV4Bin: the declared version disambiguates the untagged + // VersionedBinSlate variants. + let version = reader.read_u16()?; + if version != 5 { + return Err(grin_ser::Error::UnexpectedData { + expected: 5u16.to_be_bytes().to_vec(), + received: version.to_be_bytes().to_vec(), + }); + } let ver = VersionCompatInfoV5 { - version: reader.read_u16()?, + version, block_header_version: reader.read_u16()?, }; let id = UuidWrap::read(reader)?.0; diff --git a/libwallet/src/slatepack/packer.rs b/libwallet/src/slatepack/packer.rs index a19de72..d4324ad 100644 --- a/libwallet/src/slatepack/packer.rs +++ b/libwallet/src/slatepack/packer.rs @@ -97,16 +97,12 @@ impl<'a> Slatepacker<'a> { // Emit the lowest version that can represent this slate, for maximum // interoperability. Only the (opt-in) payment proof needs V5 fields // (timestamp/memo) that V4 cannot carry; everything else stays V4. - let (version, version_num) = if slate.payment_proof.is_some() { - (SlateVersion::V5, 5) + let version = if slate.payment_proof.is_some() { + SlateVersion::V5 } else { - (SlateVersion::V4, 4) + SlateVersion::V4 }; - // Keep the declared version consistent with the emitted structure so readers - // select the right variant. - let mut slate = slate.clone(); - slate.version_info.version = version_num; - let out_slate = VersionedSlate::into_version(slate, version)?; + let out_slate = VersionedSlate::into_version(slate.clone(), version)?; let bin_slate = VersionedBinSlate::try_from(out_slate).map_err(|_| Error::SlatepackSer)?; let mut slatepack = Slatepack::default(); slatepack.payload = byte_ser::to_bytes(&bin_slate).map_err(|_| Error::SlatepackSer)?;