fix(universaldb): align versionstamp operand encoding

Author: NathanFlurry

engine/packages/universaldb/src/versionstamp.rs

@@ -62,25 +62,26 @@ pub fn substitute_versionstamp(
 	versionstamp: Versionstamp,
 ) -> Result<(), String> {
 	const VERSIONSTAMP_MARKER: u8 = 0x33;
-	const VERSIONSTAMP_SIZE: usize = 12;
+	const VERSIONSTAMP_SIZE: usize = 10;
 
 	if packed_data.len() < 4 {
 		return Err("Packed data too short to contain versionstamp offset".to_string());
 	}
 
-	let offset_bytes = packed_data.split_off(packed_data.len() - 4);
+	let data_len = packed_data.len() - 4;
+	let offset_bytes = &packed_data[data_len..];
 	let offset = u32::from_le_bytes([
 		offset_bytes[0],
 		offset_bytes[1],
 		offset_bytes[2],
 		offset_bytes[3],
 	]) as usize;
 
-	if offset >= packed_data.len() {
+	if offset >= data_len {
 		return Err(format!(
 			"Invalid versionstamp offset: {} exceeds data length {}",
 			offset,
-			packed_data.len()
+			data_len
 		));
 	}
 
@@ -99,18 +100,19 @@ pub fn substitute_versionstamp(
 
 	let versionstamp_end = versionstamp_start + VERSIONSTAMP_SIZE;
 
-	if versionstamp_end > packed_data.len() {
+	if versionstamp_end > data_len {
 		return Err("Versionstamp extends beyond data bounds".to_string());
 	}
 
 	let existing_bytes = &packed_data[versionstamp_start..versionstamp_end];
 	if existing_bytes[0..10] != [0xff; 10] {
-		// Versionstamp is already complete, nothing to do
+		packed_data.truncate(data_len);
 		return Ok(());
 	}
 
 	let versionstamp_bytes = versionstamp.as_bytes();
-	packed_data[versionstamp_start..versionstamp_end].copy_from_slice(versionstamp_bytes);
+	packed_data[versionstamp_start..versionstamp_end].copy_from_slice(&versionstamp_bytes[..10]);
+	packed_data.truncate(data_len);
 
 	Ok(())
 }
@@ -123,7 +125,8 @@ pub fn substitute_raw_versionstamp(
 		return Err("Packed data too short to contain versionstamp offset".to_string());
 	}
 
-	let offset_bytes = data.split_off(data.len() - 4);
+	let data_len = data.len() - 4;
+	let offset_bytes = &data[data_len..];
 	let offset = u32::from_le_bytes([
 		offset_bytes[0],
 		offset_bytes[1],
@@ -135,15 +138,16 @@ pub fn substitute_raw_versionstamp(
 		.checked_add(versionstamp_len)
 		.ok_or_else(|| "Versionstamp offset overflowed".to_string())?;
 
-	if versionstamp_end > data.len() {
+	if versionstamp_end > data_len {
 		return Err(format!(
 			"Invalid versionstamp offset: {} exceeds data length {}",
 			offset,
-			data.len()
+			data_len
 		));
 	}
 
 	data[offset..versionstamp_end].copy_from_slice(&versionstamp.as_bytes()[..versionstamp_len]);
+	data.truncate(data_len);
 
 	Ok(data)
 }

engine/packages/universaldb/tests/versionstamp.rs

@@ -18,7 +18,7 @@ fn test_generate_versionstamp() {
 
 #[test]
 fn test_substitute_versionstamp_success() {
-	let incomplete = Versionstamp::from([0xff; 12]);
+	let incomplete = Versionstamp::incomplete(100);
 	let tuple = vec![
 		Element::String("mykey".into()),
 		Element::Versionstamp(incomplete),
@@ -99,9 +99,83 @@ fn test_substitute_versionstamp_already_complete() {
 	assert!(result.is_ok());
 }
 
+#[test]
+fn test_substitute_raw_versionstamp_trims_explicit_operand_offset() {
+	let versionstamp = generate_versionstamp(100);
+	let mut param = b"prefix".to_vec();
+	let offset = param.len() as u32;
+	param.extend_from_slice(&[0xff; 10]);
+	param.extend_from_slice(b"suffix");
+	param.extend_from_slice(&offset.to_le_bytes());
+
+	let substituted = substitute_raw_versionstamp(param.clone(), &versionstamp).unwrap();
+
+	assert_eq!(substituted.len(), param.len() - 4);
+	assert_eq!(&substituted[..offset as usize], b"prefix");
+	assert_eq!(
+		&substituted[offset as usize..offset as usize + 10],
+		&versionstamp.as_bytes()[..10]
+	);
+	assert_eq!(&substituted[offset as usize + 10..], b"suffix");
+}
+
+#[test]
+fn test_substitute_raw_versionstamp_matches_fdb_metadata_value_operand() {
+	let versionstamp = generate_versionstamp(100);
+	let mut param = vec![0; 14];
+	param[10..].copy_from_slice(&0u32.to_le_bytes());
+
+	let substituted = substitute_raw_versionstamp(param, &versionstamp).unwrap();
+
+	assert_eq!(substituted, versionstamp.as_bytes()[..10]);
+}
+
+#[test]
+fn test_substitute_raw_versionstamp_preserves_depot_suffix_bytes() {
+	let versionstamp = generate_versionstamp(100);
+	let mut param = vec![0xff; 10];
+	param.extend_from_slice(&[0; 6]);
+	param.extend_from_slice(&0u32.to_le_bytes());
+
+	let substituted = substitute_raw_versionstamp(param, &versionstamp).unwrap();
+
+	assert_eq!(substituted.len(), 16);
+	assert_eq!(&substituted[..10], &versionstamp.as_bytes()[..10]);
+	assert_eq!(&substituted[10..], &[0; 6]);
+}
+
+#[test]
+fn test_substitute_versionstamp_matches_official_tuple_operand_layout() {
+	let tuple = (
+		"prefix",
+		Versionstamp::incomplete(12345),
+		"suffix",
+	);
+	let mut ours = pack_with_versionstamp(&tuple);
+	let official = ours.clone();
+	let versionstamp = generate_versionstamp(54321);
+
+	substitute_versionstamp(&mut ours, versionstamp.clone()).unwrap();
+
+	let offset_start = official.len() - 4;
+	let offset = u32::from_le_bytes(
+		official[offset_start..]
+			.try_into()
+			.expect("official tuple offset should be four bytes"),
+	) as usize;
+	let mut expected = official[..offset_start].to_vec();
+	expected[offset..offset + 10].copy_from_slice(&versionstamp.as_bytes()[..10]);
+
+	assert_eq!(ours, expected);
+
+	let unpacked: (String, Versionstamp, String) = unpack(&ours).unwrap();
+	assert!(unpacked.1.is_complete());
+	assert_eq!(unpacked.1.user_version(), 12345);
+}
+
 #[test]
 fn test_pack_and_substitute_versionstamp() {
-	let incomplete = Versionstamp::from([0xff; 12]);
+	let incomplete = Versionstamp::incomplete(100);
 	let tuple = vec![
 		Element::String("mykey".into()),
 		Element::Versionstamp(incomplete),
@@ -155,8 +229,6 @@ fn test_versionstamp_incomplete_preserves_user_version() {
 		"User version bytes should match"
 	);
 
-	// Test with substitute_versionstamp - the user version from the incomplete versionstamp
-	// should be ignored and replaced with the one from the generated versionstamp
 	let tuple = vec![
 		Element::String("test".into()),
 		Element::Versionstamp(incomplete),
@@ -172,11 +244,10 @@ fn test_versionstamp_incomplete_preserves_user_version() {
 	match &unpacked[1] {
 		Element::Versionstamp(v) => {
 			assert!(v.is_complete());
-			// The user version should be from the generated versionstamp, not the original incomplete one
 			assert_eq!(
 				v.user_version(),
-				new_user_version,
-				"Substituted versionstamp should have the new user version"
+				user_version,
+				"Substituted versionstamp should preserve the tuple user version"
 			);
 		}
 		_ => panic!("Expected versionstamp"),