Contracts: emit the lowest slate version that fits the payment proof

A legacy send proof uses no V5 field: V4 carries the sender and receiver
addresses and the signature, and only the timestamp and memo need V5.
Sending those as V5 stopped V4-only wallets from reading the slate.

Add SlateVersion::lowest_for and use it from create_slatepack and from
the contract owner API methods, which returned V4 unconditionally and so
dropped the timestamp and memo that an early proof's promise signature
binds. contract_sign keeps the version the counterparty sent when the
slate does not need more.

api/src/owner_rpc.rs

@@ -2159,7 +2159,9 @@ where
 		args: ContractNewArgsAPI,
 	) -> Result<VersionedSlate, Error> {
 		let slate = Owner::contract_new(self, (&token.keychain_mask).as_ref(), &args)?;
-		let version = SlateVersion::V4;
+		// An early payment proof needs V5: its promise signature binds the timestamp and
+		// memo, which a V4 slate cannot carry.
+		let version = SlateVersion::lowest_for(&slate);
 		VersionedSlate::into_version(slate, version)
 	}
 
@@ -2169,13 +2171,19 @@ where
 		in_slate: VersionedSlate,
 		args: ContractSetupArgsAPI,
 	) -> Result<VersionedSlate, Error> {
+		let in_version = in_slate.version();
 		let slate = Owner::contract_sign(
 			self,
 			(&token.keychain_mask).as_ref(),
 			&Slate::from(in_slate),
 			&args,
 		)?;
-		let version = SlateVersion::V4;
+		// Keep the version the counterparty sent us, since they can clearly read it, but
+		// never below what the slate needs to represent its payment proof.
+		let version = match SlateVersion::lowest_for(&slate) {
+			SlateVersion::V5 => SlateVersion::V5,
+			SlateVersion::V4 => in_version,
+		};
 		VersionedSlate::into_version(slate, version)
 	}
 
@@ -2185,13 +2193,10 @@ where
 		args: ContractRevokeArgsAPI,
 	) -> Result<Option<VersionedSlate>, Error> {
 		let slate_opt = Owner::contract_revoke(self, (&token.keychain_mask).as_ref(), &args)?;
-		let version = SlateVersion::V4;
 		// We return a slate only when we had to perform a self-spend safe cancel
-		if slate_opt.is_some() {
-			return Ok(Some(VersionedSlate::into_version(
-				slate_opt.unwrap(),
-				version,
-			)?));
+		if let Some(slate) = slate_opt {
+			let version = SlateVersion::lowest_for(&slate);
+			return Ok(Some(VersionedSlate::into_version(slate, version)?));
 		}
 		Ok(None)
 	}

libwallet/src/slate_versions/mod.rs

@@ -52,6 +52,19 @@ pub enum SlateVersion {
 	V4,
 }
 
+impl SlateVersion {
+	/// The lowest version that can represent this slate without losing data, so a slate
+	/// is only sent as V5 when it has to be. V4 carries the payment proof's sender and
+	/// receiver addresses and its signature, but neither the timestamp nor the memo, so
+	/// only a proof using one of those needs V5. A legacy send proof sets neither.
+	pub fn lowest_for(slate: &Slate) -> SlateVersion {
+		match &slate.payment_proof {
+			Some(p) if p.memo.is_some() || p.timestamp.timestamp() != 0 => SlateVersion::V5,
+			_ => SlateVersion::V4,
+		}
+	}
+}
+
 #[derive(Debug, Serialize, Deserialize)]
 #[serde(untagged)]
 /// Versions are ordered newest to oldest so serde attempts to
@@ -293,6 +306,21 @@ pub mod tests {
 		let recovered_v4 = packer.get_slate(&packer.create_slatepack(&slate_no_proof)?)?;
 		assert_eq!(recovered_v4.version_info.version, 4);
 
+		// A legacy send proof uses no V5 field: the timestamp is left at the epoch and
+		// there is no memo. It must stay V4 so V4-only wallets can still read the slate,
+		// and the proof has to survive the round trip intact.
+		let mut slate_legacy_proof = populate_test_slate()?;
+		let mut proof = slate_legacy_proof.payment_proof.clone().unwrap();
+		proof.timestamp = DateTime::from_timestamp(0, 0).unwrap();
+		proof.memo = None;
+		slate_legacy_proof.payment_proof = Some(proof.clone());
+		let recovered_legacy = packer.get_slate(&packer.create_slatepack(&slate_legacy_proof)?)?;
+		assert_eq!(recovered_legacy.version_info.version, 4);
+		let recovered_proof = recovered_legacy.payment_proof.unwrap();
+		assert_eq!(recovered_proof.sender_address, proof.sender_address);
+		assert_eq!(recovered_proof.receiver_address, proof.receiver_address);
+		assert_eq!(recovered_proof.promise_signature, proof.promise_signature);
+
 		Ok(())
 	}
 

libwallet/src/slate_versions/v5_bin.rs

@@ -21,7 +21,7 @@ use crate::grin_keychain::BlindingFactor;
 use crate::grin_util::secp::key::PublicKey;
 use crate::grin_util::secp::pedersen::{Commitment, RangeProof};
 use crate::grin_util::secp::Signature;
-use chrono::{DateTime, Utc};
+use chrono::DateTime;
 use ed25519_dalek::Signature as DalekSignature;
 use ed25519_dalek::VerifyingKey as DalekPublicKey;
 use std::convert::{TryFrom, TryInto};
@@ -434,6 +434,7 @@ fn slate_v5_serialize_deserialize() {
 	use crate::grin_util::from_hex;
 	use crate::grin_util::secp::key::PublicKey;
 	use crate::Slate;
+	use chrono::Utc;
 	use grin_core::global::{set_local_chain_type, ChainTypes};
 	use grin_keychain::{ExtKeychain, Keychain, SwitchCommitmentType};
 	set_local_chain_type(ChainTypes::Mainnet);

libwallet/src/slatepack/packer.rs

@@ -95,13 +95,8 @@ impl<'a> Slatepacker<'a> {
 	/// Create slatepack from slate and args
 	pub fn create_slatepack(&self, slate: &Slate) -> Result<Slatepack, Error> {
 		// 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 = if slate.payment_proof.is_some() {
-			SlateVersion::V5
-		} else {
-			SlateVersion::V4
-		};
+		// interoperability with V4-only wallets.
+		let version = SlateVersion::lowest_for(slate);
 		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();