diff --git a/api/src/owner_rpc.rs b/api/src/owner_rpc.rs index 6318071..288d38e 100644 --- a/api/src/owner_rpc.rs +++ b/api/src/owner_rpc.rs @@ -2159,7 +2159,9 @@ where args: ContractNewArgsAPI, ) -> Result { 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 { + 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, 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) } diff --git a/libwallet/src/slate_versions/mod.rs b/libwallet/src/slate_versions/mod.rs index 1b94c45..41797ab 100644 --- a/libwallet/src/slate_versions/mod.rs +++ b/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(()) } diff --git a/libwallet/src/slate_versions/v5_bin.rs b/libwallet/src/slate_versions/v5_bin.rs index 7af92f7..c695006 100644 --- a/libwallet/src/slate_versions/v5_bin.rs +++ b/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); diff --git a/libwallet/src/slatepack/packer.rs b/libwallet/src/slatepack/packer.rs index d4324ad..27dda49 100644 --- a/libwallet/src/slatepack/packer.rs +++ b/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 { // 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();