align proof memo encoding

doc/contracts.md

@@ -83,9 +83,8 @@ account selected with `--account`.
   format. The full proposal is described in
   [Early Payment Proofs](https://github.com/mimblewimble/grin-rfcs/pull/70)
 * There are no contract-specific history, lookup or transport commands. Payment proof
-  memos cannot be set through the contract API or CLI. The current proof stores a memo
-  type and 32 bytes instead of the text and hash described by the early payment proofs
-  proposal
+  memos cannot be set through the contract API or CLI. Early proofs support UTF-8 memos
+  up to 1024 bytes and bind their Blake2b hash
 * If writing the signed transaction file fails, the wallet state has already been saved.
   Cancelling the transaction releases the locked inputs
 

libwallet/src/contract/actions/README.md

@@ -39,11 +39,10 @@ Separating side effects until the 'save_step' part would make these functions mu
 
  - Add RBF support to the Grin node before raising the replacement fee when `revoke`
    races a transaction already in the mempool.
- - Merge invoice-proof retrieval with the existing payment-proof API.
  - Add a Grin node API for kernel lookup by MMR index, then use the witness index instead
    of searching by commitment.
- - Align the proof memo encoding with the early payment proofs proposal.
- - Implement sender-nonce proofs for the RSR flow.
+ - Implement sender-nonce proofs for the RSR flow and expose both early-proof types
+   through one API.
  - Move payment-proof creation and verification onto `Slate` so it can also be used
    outside contracts and is versioned with the slate format.
  - Keep V5 when decoding Slatepacks through Owner RPC.

libwallet/src/contract/proofs.rs

@@ -17,11 +17,12 @@
 //! by legacy transactions
 
 use crate::backend::WalletBackend;
+use crate::blake2::blake2b::blake2b;
 use crate::contract::types::{ProofArgs, ProofType};
 use crate::grin_core::libtx::aggsig;
 use crate::grin_core::libtx::secp_ser;
 use crate::grin_core::ser as grin_ser;
-use crate::grin_core::ser::{Readable, Reader, Writeable, Writer};
+use crate::grin_core::ser::{Writeable, Writer};
 use crate::grin_keychain::Keychain;
 use crate::grin_util::secp::key::{PublicKey, SecretKey};
 use crate::grin_util::secp::pedersen::Commitment;
@@ -95,7 +96,7 @@ pub struct ProofWitness {
 }
 
 /// Payment proof, to be extracted from slates for
-/// signing (when wrapped as PaymentProofBin) or json export from stored tx data
+/// signing (when wrapped as InvoiceProofBin) or json export from stored tx data
 #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
 pub struct InvoiceProof {
 	/// Proof type, 0x00 legacy (though this will use StoredProofInfo above, 1 invoice, 2 Sender nonce)
@@ -127,9 +128,9 @@ pub struct InvoiceProof {
 	pub witness_data: Option<ProofWitness>,
 }
 
-struct InvoiceProofBin(InvoiceProof);
+struct InvoiceProofBin<'a>(&'a InvoiceProof);
 
-impl Writeable for InvoiceProofBin {
+impl Writeable for InvoiceProofBin<'_> {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), grin_ser::Error> {
 		writer.write_u8(self.0.proof_type.as_u8())?;
 
@@ -160,80 +161,12 @@ impl Writeable for InvoiceProofBin {
 		}
 		writer.write_fixed_bytes(self.0.sender_address.as_bytes())?;
 		writer.write_i64(self.0.timestamp)?;
-		match &self.0.memo {
-			Some(s) => {
-				writer.write_u8(s.memo_type)?;
-				writer.write_fixed_bytes(&s.memo.to_vec())?;
-			}
-			None => {
-				writer.write_u8(0)?;
-				writer.write_fixed_bytes([0u8; 32].to_vec())?;
-			}
-		}
+		let memo = self.0.memo.as_ref().map(PaymentMemo::as_str).unwrap_or("");
+		writer.write_fixed_bytes(blake2b(32, &[], memo.as_bytes()).as_bytes())?;
 		Ok(())
 	}
 }
 
-/// Currently only exercised by the tests, but this is a normal Readable, so every
-/// externally shaped value it reads is mapped to a serialization error rather than
-/// unwrapped.
-impl Readable for InvoiceProofBin {
-	fn read<R: Reader>(reader: &mut R) -> Result<InvoiceProofBin, grin_ser::Error> {
-		// first 8 bytes are proof type + 7 bytes worth of amount
-		let mut amount = reader.read_u64()?;
-		// The shift leaves a single byte, so this conversion cannot lose data
-		let proof_type = PaymentProofType::try_from(((amount & 0xFF00_0000_0000_0000) >> 56) as u8)
-			.map_err(|_| grin_ser::Error::CorruptedData)?;
-		amount &= 0x00FF_FFFF_FFFF_FFFF;
-
-		let receiver_public_nonce;
-		let receiver_public_excess;
-		{
-			let static_secp = static_secp_instance();
-			let static_secp = static_secp.lock();
-			receiver_public_nonce =
-				PublicKey::from_slice(&static_secp, &reader.read_fixed_bytes(33)?)
-					.map_err(|_| grin_ser::Error::CorruptedData)?;
-			receiver_public_excess =
-				PublicKey::from_slice(&static_secp, &reader.read_fixed_bytes(33)?)
-					.map_err(|_| grin_ser::Error::CorruptedData)?;
-		}
-
-		let sender_address_vec = reader.read_fixed_bytes(32)?;
-		let sender_address_bytes = <&[u8; 32]>::try_from(sender_address_vec.as_slice())
-			.map_err(|_| grin_ser::Error::CorruptedData)?;
-		let sender_address = DalekPublicKey::from_bytes(sender_address_bytes)
-			.map_err(|_| grin_ser::Error::CorruptedData)?;
-
-		let timestamp = reader.read_i64()?;
-
-		let memo_type = reader.read_u8()?;
-		let memo = reader.read_fixed_bytes(32)?;
-		let memo_bytes =
-			<[u8; 32]>::try_from(memo.as_slice()).map_err(|_| grin_ser::Error::CorruptedData)?;
-
-		let res = InvoiceProof {
-			proof_type,
-			amount,
-			receiver_public_nonce,
-			receiver_public_excess,
-			sender_address,
-			timestamp,
-			memo: match memo_type {
-				0 => None,
-				_ => Some(PaymentMemo {
-					memo_type,
-					memo: memo_bytes,
-				}),
-			},
-			promise_signature: None,
-			witness_data: None,
-		};
-
-		Ok(InvoiceProofBin(res))
-	}
-}
-
 impl InvoiceProof {
 	/// Extracts as much data as possible from the slate to create an invoice proof
 	pub fn from_slate(
@@ -304,9 +237,9 @@ impl InvoiceProof {
 		let d_skey = DalekSecretKey::from_bytes(&sec_key.0);
 		let pub_key = d_skey.verifying_key();
 		let mut sig_data_bin = Vec::new();
-		grin_ser::serialize_default(&mut sig_data_bin, &InvoiceProofBin(self.clone())).map_err(
-			|e| Error::GenericError(format!("InvoiceProof serialization failed: {}", e)),
-		)?;
+		grin_ser::serialize_default(&mut sig_data_bin, &InvoiceProofBin(self)).map_err(|e| {
+			Error::GenericError(format!("InvoiceProof serialization failed: {}", e))
+		})?;
 
 		Ok((d_skey.sign(&sig_data_bin), pub_key))
 	}
@@ -325,9 +258,9 @@ impl InvoiceProof {
 
 		// Rebuild message
 		let mut sig_data_bin = Vec::new();
-		grin_ser::serialize_default(&mut sig_data_bin, &InvoiceProofBin(self.clone())).map_err(
-			|e| Error::GenericError(format!("InvoiceProof serialization failed: {}", e)),
-		)?;
+		grin_ser::serialize_default(&mut sig_data_bin, &InvoiceProofBin(self)).map_err(|e| {
+			Error::GenericError(format!("InvoiceProof serialization failed: {}", e))
+		})?;
 
 		if recipient_address
 			.verify(&sig_data_bin, self.promise_signature.as_ref().unwrap())
@@ -402,18 +335,6 @@ impl InvoiceProof {
 	}
 }
 
-impl serde::Serialize for InvoiceProofBin {
-	fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-	where
-		S: serde::Serializer,
-	{
-		let mut vec = vec![];
-		grin_ser::serialize(&mut vec, grin_ser::ProtocolVersion(4), self)
-			.map_err(|err| serde::ser::Error::custom(err.to_string()))?;
-		serializer.serialize_bytes(&vec)
-	}
-}
-
 /// Adds all info needed for a payment proof to a slate, complete with signed recipient data
 pub(super) fn add_payment_proof<C, K>(
 	wallet: &mut WalletBackend<C, K>,
@@ -550,17 +471,25 @@ mod tests {
 		// Should fail, amount too big
 		let invoice_proof = InvoiceProof::from_slate(&slate, 1, None)?;
 		let mut vec = Vec::new();
-		assert!(grin_ser::serialize_default(&mut vec, &InvoiceProofBin(invoice_proof)).is_err());
+		assert!(grin_ser::serialize_default(&mut vec, &InvoiceProofBin(&invoice_proof)).is_err());
 
 		// Should be okay now
 		slate.amount = 1234;
-		let invoice_proof = InvoiceProof::from_slate(&slate, 1, None)?;
+		let mut invoice_proof = InvoiceProof::from_slate(&slate, 1, None)?;
 		let mut vec = Vec::new();
-		grin_ser::serialize_default(&mut vec, &InvoiceProofBin(invoice_proof.clone()))
+		grin_ser::serialize_default(&mut vec, &InvoiceProofBin(&invoice_proof))
 			.expect("Serialization Failed");
-
-		let proof_deser: InvoiceProofBin = grin_ser::deserialize_default(&mut &vec[..]).unwrap();
-		assert_eq!(invoice_proof, proof_deser.0);
+		let memo = invoice_proof.memo.as_ref().unwrap().as_str();
+		assert_eq!(
+			&vec[vec.len() - 32..],
+			blake2b(32, &[], memo.as_bytes()).as_bytes()
+		);
+		let proof_key = SecretKey::from_slice(&Secp256k1::new(), &[7; 32])?;
+		let (signature, recipient) = invoice_proof.sign(&proof_key)?;
+		invoice_proof.promise_signature = Some(signature);
+		invoice_proof.verify_promise_signature(&recipient)?;
+		invoice_proof.memo = Some(PaymentMemo::new("changed details".to_string())?);
+		assert!(invoice_proof.verify_promise_signature(&recipient).is_err());
 
 		let mut wrong_type = invoice_proof;
 		wrong_type.proof_type = PaymentProofType::Legacy;
@@ -570,34 +499,11 @@ mod tests {
 		Ok(())
 	}
 
-	// Every key the reader takes from the encoded form is externally shaped, so a
-	// corrupted one has to come back as a serialization error rather than a panic.
 	#[test]
-	fn deser_invoice_proof_bin_rejects_bad_keys() -> Result<(), Error> {
-		let mut slate = populate_test_slate()?;
-		slate.amount = 1234;
-		slate.payment_proof.as_mut().unwrap().promise_signature = None;
-		let invoice_proof = InvoiceProof::from_slate(&slate, 1, None)?;
-
-		let mut vec = Vec::new();
-		grin_ser::serialize_default(&mut vec, &InvoiceProofBin(invoice_proof))
-			.expect("Serialization Failed");
-
-		// The two secp keys follow the 8 amount bytes, then the ed25519 sender address
-		for offset in [8, 41, 74] {
-			let mut corrupted = vec.clone();
-			// An all-ones prefix is not a valid point in either encoding
-			for b in corrupted[offset..offset + 8].iter_mut() {
-				*b = 0xFF;
-			}
-			let res: Result<InvoiceProofBin, _> =
-				grin_ser::deserialize_default(&mut &corrupted[..]);
-			assert!(
-				res.is_err(),
-				"corrupting offset {} should not parse",
-				offset
-			);
-		}
-		Ok(())
+	fn memo_limit() {
+		assert!(PaymentMemo::new("a".repeat(PaymentMemo::MAX_LEN)).is_ok());
+		assert!(PaymentMemo::new("a".repeat(PaymentMemo::MAX_LEN + 1)).is_err());
+		assert!(PaymentMemo::new("ä".repeat(PaymentMemo::MAX_LEN / 2)).is_ok());
+		assert!(PaymentMemo::new("ä".repeat(PaymentMemo::MAX_LEN / 2 + 1)).is_err());
 	}
 }

libwallet/src/slate.rs

@@ -42,7 +42,7 @@ use crate::slate_versions::v4::{
 };
 use crate::slate_versions::v5::{
 	CommitsV5, KernelFeaturesArgsV5, OutputFeaturesV5, ParticipantDataV5, PaymentInfoV5,
-	PaymentMemoV5, SlateStateV5, SlateV5, VersionCompatInfoV5,
+	SlateStateV5, SlateV5, VersionCompatInfoV5,
 };
 use crate::slate_versions::VersionedSlate;
 use crate::slate_versions::{CURRENT_SLATE_VERSION, GRIN_BLOCK_HEADER_VERSION};
@@ -114,15 +114,38 @@ pub(crate) mod payment_proof_type_serde {
 	}
 }
 
+/// Payment details bound to an early payment proof
 #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
-pub struct PaymentMemo {
-	// The type of memo
-	// 0x00 is the absence of any specific memo data
-	// 0x01 is directly embedded additional payment details
-	// 0x02 represents the blake2b hash of an arbitrary invoice document
-	pub memo_type: u8,
-	// memo data itself
-	pub memo: [u8; 32],
+#[serde(try_from = "String")]
+pub struct PaymentMemo(String);
+
+impl PaymentMemo {
+	/// Maximum memo size from the early payment proofs proposal
+	pub const MAX_LEN: usize = 1024;
+
+	/// Create a payment memo
+	pub fn new(memo: String) -> Result<Self, Error> {
+		if memo.len() > Self::MAX_LEN {
+			return Err(Error::PaymentProof(format!(
+				"Payment memo exceeds {} bytes",
+				Self::MAX_LEN
+			)));
+		}
+		Ok(Self(memo))
+	}
+
+	/// Memo text
+	pub fn as_str(&self) -> &str {
+		&self.0
+	}
+}
+
+impl TryFrom<String> for PaymentMemo {
+	type Error = Error;
+
+	fn try_from(memo: String) -> Result<Self, Self::Error> {
+		Self::new(memo)
+	}
 }
 
 #[derive(Debug, Clone, Eq, PartialEq)]
@@ -1020,13 +1043,7 @@ impl From<&PaymentInfo> for PaymentInfoV5 {
 		let receiver_address = *receiver_address;
 		let promise_signature = *promise_signature;
 		let timestamp = *timestamp;
-		let memo = match memo {
-			Some(m) => Some(PaymentMemoV5 {
-				memo_type: m.memo_type,
-				memo: m.memo,
-			}),
-			None => None,
-		};
+		let memo = memo.clone();
 		PaymentInfoV5 {
 			ptype: proof_type,
 			saddr: sender_address,
@@ -1238,16 +1255,7 @@ impl From<&PaymentInfoV5> for PaymentInfo {
 		let receiver_address = *receiver_address;
 		let promise_signature = *promise_signature;
 		let timestamp = *timestamp;
-		let memo: Option<PaymentMemo> = match memo {
-			Some(m) => {
-				//memo_ret.copy_from_slice(&grin_util::from_hex(m.memo).unwrap_or_default()[0..32]);
-				Some(PaymentMemo {
-					memo_type: m.memo_type,
-					memo: m.memo,
-				})
-			}
-			None => None,
-		};
+		let memo = memo.clone();
 		PaymentInfo {
 			proof_type,
 			sender_address,

libwallet/src/slate_versions/mod.rs

@@ -247,10 +247,7 @@ pub mod tests {
 		let d_pkey = DalekPublicKey::from_bytes(b).unwrap();
 		// Need to remove milliseconds component for comparison. Won't be serialized
 		let ts = DateTime::from_timestamp(Utc::now().timestamp(), 0).unwrap();
-		let pm = PaymentMemo {
-			memo_type: 1,
-			memo: [9; 32],
-		};
+		let pm = PaymentMemo::new("payment details".to_string()).unwrap();
 
 		let psig = DalekSignature::from_bytes(&[0u8; 64]);
 		slate_internal.payment_proof = Some(PaymentInfo {

libwallet/src/slate_versions/v5.rs

@@ -26,9 +26,7 @@
 //!
 //! * `ptype` identifies the payment proof type
 //! * `ts` adds an optional timestamp, serialized as seconds since the epoch
-//! * `memo` added as optional MemoV5 Struct, which contains:
-//!   * `memo_type`: u8
-//!   * `memo`: [u8;32] the memo data itself
+//! * `memo` adds optional payment details
 
 use crate::grin_core::core::FeeFields;
 use crate::grin_core::core::{Input, Output, TxKernel};
@@ -38,7 +36,7 @@ use crate::grin_util::secp;
 use crate::grin_util::secp::key::PublicKey;
 use crate::grin_util::secp::pedersen::{Commitment, RangeProof};
 use crate::grin_util::secp::Signature;
-use crate::slate::PaymentProofType;
+use crate::slate::{PaymentMemo, PaymentProofType};
 use crate::{slate_versions::ser, CbData};
 use chrono::prelude::{DateTime, Utc};
 use ed25519_dalek::Signature as DalekSignature;
@@ -196,14 +194,6 @@ fn default_part_sig_none() -> Option<Signature> {
 	None
 }
 
-#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
-pub struct PaymentMemoV5 {
-	// Uses the values defined by PaymentMemo
-	pub memo_type: u8,
-	// memo data itself
-	pub memo: [u8; 32],
-}
-
 #[serde_as]
 #[derive(Serialize, Deserialize, Debug, Clone, Eq, PartialEq)]
 pub struct PaymentInfoV5 {
@@ -223,7 +213,7 @@ pub struct PaymentInfoV5 {
 	#[serde(skip_serializing_if = "Option::is_none")]
 	pub psig: Option<DalekSignature>,
 	#[serde(skip_serializing_if = "Option::is_none")]
-	pub memo: Option<PaymentMemoV5>,
+	pub memo: Option<PaymentMemo>,
 }
 
 fn default_promise_signature_none() -> Option<DalekSignature> {

libwallet/src/slate_versions/v5_bin.rs

@@ -21,15 +21,15 @@ 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 crate::slate::PaymentProofType;
+use crate::slate::{PaymentMemo, PaymentProofType};
 use chrono::DateTime;
 use ed25519_dalek::Signature as DalekSignature;
 use ed25519_dalek::VerifyingKey as DalekPublicKey;
-use std::convert::{TryFrom, TryInto};
+use std::convert::TryFrom;
 
 use crate::slate_versions::v5::{
-	CommitsV5, KernelFeaturesArgsV5, ParticipantDataV5, PaymentInfoV5, PaymentMemoV5, SlateStateV5,
-	SlateV5, VersionCompatInfoV5,
+	CommitsV5, KernelFeaturesArgsV5, ParticipantDataV5, PaymentInfoV5, SlateStateV5, SlateV5,
+	VersionCompatInfoV5,
 };
 
 use crate::slate_versions::v4_bin::{SlateOptFields, UuidWrap};
@@ -247,8 +247,7 @@ impl<'a> Writeable for ProofWrapRef<'a> {
 		match &self.0.memo {
 			Some(s) => {
 				writer.write_u8(1)?;
-				writer.write_u8(s.memo_type)?;
-				writer.write_fixed_bytes(&s.memo)?;
+				writer.write_bytes(s.as_str().as_bytes())?;
 			}
 			None => writer.write_u8(0)?,
 		}
@@ -297,13 +296,11 @@ impl Readable for ProofWrap {
 		};
 		let memo = match reader.read_u8()? {
 			0 => None,
-			1 => Some(PaymentMemoV5 {
-				memo_type: reader.read_u8()?,
-				memo: reader
-					.read_fixed_bytes(32)?
-					.try_into()
-					.map_err(|_| grin_ser::Error::CorruptedData)?,
-			}),
+			1 => {
+				let memo = String::from_utf8(reader.read_bytes_len_prefix()?)
+					.map_err(|_| grin_ser::Error::CorruptedData)?;
+				Some(PaymentMemo::new(memo).map_err(|_| grin_ser::Error::CorruptedData)?)
+			}
 			_ => return Err(grin_ser::Error::CorruptedData),
 		};
 		Ok(ProofWrap(PaymentInfoV5 {
@@ -573,10 +570,7 @@ fn slate_v5_serialize_deserialize() {
 	let d_pkey = DalekPublicKey::from_bytes(b).unwrap();
 	// Need to remove milliseconds component for comparison. Won't be serialized
 	let ts = DateTime::from_timestamp(Utc::now().timestamp(), 0).unwrap();
-	let pm = PaymentMemoV5 {
-		memo_type: 0,
-		memo: [9; 32],
-	};
+	let pm = PaymentMemo::new("payment details".to_string()).unwrap();
 	v5.proof = Some(PaymentInfoV5 {
 		ptype: PaymentProofType::Invoice,
 		raddr: d_pkey.clone(),