Make Transaction Field in V4 Slate Optional (#356)

* make transaction field in V4 Slate optional

* add mutable/non mutable version of tx getter

* removal of mut references where not needed

* remove more muts

* update from master

* update from master

* test fixes

libwallet/src/api_impl/foreign.rs

@@ -148,7 +148,7 @@ where
 	check_ttl(w, &sl)?;
 	let context = w.get_private_context(keychain_mask, sl.id.as_bytes(), 1)?;
 	tx::complete_tx(&mut *w, keychain_mask, &mut sl, 1, &context)?;
-	tx::update_stored_tx(&mut *w, keychain_mask, &context, &sl, true)?;
+	tx::update_stored_tx(&mut *w, keychain_mask, &context, &mut sl, true)?;
 	tx::update_message(&mut *w, keychain_mask, &sl)?;
 	{
 		let mut batch = w.batch(keychain_mask)?;

libwallet/src/error.rs

@@ -202,6 +202,14 @@ pub enum ErrorKind {
 	#[fail(display = "Unknown Slate Version: {}", _0)]
 	SlateVersion(u16),
 
+	/// Attempt to use slate transaction data that doesn't exists
+	#[fail(display = "Slate transaction required in this context")]
+	SlateTransactionRequired,
+
+	/// Attempt to downgrade slate that can't be downgraded
+	#[fail(display = "Can't downgrade slate: {}", _0)]
+	SlateInvalidDowngrade(String),
+
 	/// Compatibility error between incoming slate versions and what's expected
 	#[fail(display = "Compatibility Error: {}", _0)]
 	Compatibility(String),

libwallet/src/internal/selection.rs

@@ -211,7 +211,10 @@ where
 		batch.commit()?;
 		t
 	};
-	wallet.store_tx(&format!("{}", tx_entry.tx_slate_id.unwrap()), &slate.tx)?;
+	wallet.store_tx(
+		&format!("{}", tx_entry.tx_slate_id.unwrap()),
+		slate.tx_or_err()?,
+	)?;
 	Ok(())
 }
 

libwallet/src/internal/tx.rs

@@ -351,11 +351,11 @@ where
 		Some(t) => t,
 		None => return Err(ErrorKind::TransactionDoesntExist(slate.id.to_string()).into()),
 	};
-	wallet.store_tx(&format!("{}", tx.tx_slate_id.unwrap()), &slate.tx)?;
+	wallet.store_tx(&format!("{}", tx.tx_slate_id.unwrap()), slate.tx_or_err()?)?;
 	let parent_key = tx.parent_key_id.clone();
-	tx.kernel_excess = Some(slate.tx.body.kernels[0].excess);
+	tx.kernel_excess = Some(slate.tx_or_err()?.body.kernels[0].excess);
 
-	if let Some(ref p) = slate.payment_proof {
+	if let Some(ref p) = slate.clone().payment_proof {
 		let derivation_index = match context.payment_proof_derivation_index {
 			Some(i) => i,
 			None => 0,
@@ -493,9 +493,9 @@ where
 		.into());
 	}
 
-	if let Some(ref p) = slate.payment_proof {
+	if let Some(ref p) = slate.clone().payment_proof {
 		let orig_proof_info = match orig_proof_info {
-			Some(p) => p,
+			Some(p) => p.clone(),
 			None => {
 				return Err(ErrorKind::PaymentProof(
 					"Original proof info not stored in tx".to_owned(),

libwallet/src/internal/updater.rs

@@ -77,7 +77,7 @@ where
 		.into_iter()
 		.map(|output| {
 			let commit = match output.commit.clone() {
-				Some(c) => pedersen::Commitment::from_vec(util::from_hex(c).unwrap()),
+				Some(c) => pedersen::Commitment::from_vec(util::from_hex(&c).unwrap()),
 				None => keychain
 					.commit(output.value, &output.key_id, SwitchCommitmentType::Regular)
 					.unwrap(), // TODO: proper support for different switch commitment schemes
@@ -187,7 +187,7 @@ where
 
 	for out in unspents {
 		let commit = match out.commit.clone() {
-			Some(c) => pedersen::Commitment::from_vec(util::from_hex(c).unwrap()),
+			Some(c) => pedersen::Commitment::from_vec(util::from_hex(&c).unwrap()),
 			None => keychain
 				.commit(out.value, &out.key_id, SwitchCommitmentType::Regular)
 				.unwrap(), // TODO: proper support for different switch commitment schemes

libwallet/src/slate.rs

@@ -38,6 +38,7 @@ use rand::rngs::mock::StepRng;
 use rand::thread_rng;
 use serde::ser::{Serialize, Serializer};
 use serde_json;
+use std::convert::TryFrom;
 use std::fmt;
 use std::sync::Arc;
 use uuid::Uuid;
@@ -172,7 +173,9 @@ pub struct Slate {
 	pub id: Uuid,
 	/// The core transaction data:
 	/// inputs, outputs, kernels, kernel offset
-	pub tx: Transaction,
+	/// Optional as of V4 to allow for a compact
+	/// transaction initiation
+	pub tx: Option<Transaction>,
 	/// base amount (excluding fee)
 	#[serde(with = "secp_ser::string_or_u64")]
 	pub amount: u64,
@@ -221,6 +224,23 @@ pub struct ParticipantMessages {
 }
 
 impl Slate {
+	/// Return the transaction, throwing an error if it doesn't exist
+	/// to be used at points in the code where the existence of a transaction
+	/// is assumed
+	pub fn tx_or_err(&self) -> Result<&Transaction, Error> {
+		match &self.tx {
+			Some(t) => Ok(t),
+			None => Err(ErrorKind::SlateTransactionRequired.into()),
+		}
+	}
+
+	/// As above, but return mutable reference
+	pub fn tx_or_err_mut(&mut self) -> Result<&mut Transaction, Error> {
+		match &mut self.tx {
+			Some(t) => Ok(t),
+			None => Err(ErrorKind::SlateTransactionRequired.into()),
+		}
+	}
 	/// Attempt to find slate version
 	pub fn parse_slate_version(slate_json: &str) -> Result<u16, Error> {
 		let probe: SlateVersionProbe =
@@ -248,7 +268,7 @@ impl Slate {
 		Slate {
 			num_participants: num_participants,
 			id: Uuid::new_v4(),
-			tx: Transaction::empty(),
+			tx: Some(Transaction::empty()),
 			amount: 0,
 			fee: 0,
 			height: 0,
@@ -276,20 +296,23 @@ impl Slate {
 		K: Keychain,
 		B: ProofBuild,
 	{
-		self.update_kernel();
-		let (tx, blind) = build::partial_transaction(self.tx.clone(), elems, keychain, builder)?;
-		self.tx = tx;
+		self.update_kernel()?;
+		let (tx, blind) =
+			build::partial_transaction(self.tx_or_err()?.clone(), elems, keychain, builder)?;
+		self.tx = Some(tx);
 		Ok(blind)
 	}
 
 	/// Update the tx kernel based on kernel features derived from the current slate.
 	/// The fee may change as we build a transaction and we need to
 	/// update the tx kernel to reflect this during the tx building process.
-	pub fn update_kernel(&mut self) {
-		self.tx = self
-			.tx
-			.clone()
-			.replace_kernel(TxKernel::with_features(self.kernel_features()));
+	pub fn update_kernel(&mut self) -> Result<(), Error> {
+		self.tx = Some(
+			self.tx_or_err()?
+				.clone()
+				.replace_kernel(TxKernel::with_features(self.kernel_features())),
+		);
+		Ok(())
 	}
 
 	/// Completes callers part of round 1, adding public key info
@@ -307,7 +330,7 @@ impl Slate {
 		K: Keychain,
 	{
 		// Whoever does this first generates the offset
-		if self.tx.offset == BlindingFactor::zero() {
+		if self.tx_or_err()?.offset == BlindingFactor::zero() {
 			self.generate_offset(keychain, sec_key, use_test_rng)?;
 		}
 		self.add_participant_info(
@@ -507,7 +530,7 @@ impl Slate {
 		// Generate a random kernel offset here
 		// and subtract it from the blind_sum so we create
 		// the aggsig context with the "split" key
-		self.tx.offset = match use_test_rng {
+		self.tx_or_err_mut()?.offset = match use_test_rng {
 			false => {
 				BlindingFactor::from_secret_key(SecretKey::new(&keychain.secp(), &mut thread_rng()))
 			}
@@ -521,7 +544,7 @@ impl Slate {
 		let blind_offset = keychain.blind_sum(
 			&BlindSum::new()
 				.add_blinding_factor(BlindingFactor::from_secret_key(sec_key.clone()))
-				.sub_blinding_factor(self.tx.offset.clone()),
+				.sub_blinding_factor(self.tx_or_err()?.offset.clone()),
 		)?;
 		*sec_key = blind_offset.secret_key(&keychain.secp())?;
 		Ok(())
@@ -529,19 +552,20 @@ impl Slate {
 
 	/// Checks the fees in the transaction in the given slate are valid
 	fn check_fees(&self) -> Result<(), Error> {
+		let tx = self.tx_or_err()?;
 		// double check the fee amount included in the partial tx
 		// we don't necessarily want to just trust the sender
 		// we could just overwrite the fee here (but we won't) due to the sig
 		let fee = tx_fee(
-			self.tx.inputs().len(),
-			self.tx.outputs().len(),
-			self.tx.kernels().len(),
+			tx.inputs().len(),
+			tx.outputs().len(),
+			tx.kernels().len(),
 			None,
 		);
 
-		if fee > self.tx.fee() {
+		if fee > tx.fee() {
 			return Err(
-				ErrorKind::Fee(format!("Fee Dispute Error: {}, {}", self.tx.fee(), fee,)).into(),
+				ErrorKind::Fee(format!("Fee Dispute Error: {}, {}", tx.fee(), fee,)).into(),
 			);
 		}
 
@@ -668,8 +692,8 @@ impl Slate {
 	where
 		K: Keychain,
 	{
-		let kernel_offset = &self.tx.offset;
-		let tx = self.tx.clone();
+		let tx = self.tx_or_err()?.clone();
+		let kernel_offset = tx.offset.clone();
 		let overage = tx.fee() as i64;
 		let tx_excess = tx.sum_commitments(overage)?;
 
@@ -697,7 +721,7 @@ impl Slate {
 
 		debug!("Final Tx excess: {:?}", final_excess);
 
-		let mut final_tx = self.tx.clone();
+		let final_tx = self.tx_or_err_mut()?;
 
 		// update the tx kernel to reflect the offset excess and sig
 		assert_eq!(final_tx.kernels().len(), 1);
@@ -713,7 +737,6 @@ impl Slate {
 		let verifier_cache = Arc::new(RwLock::new(LruVerifierCache::new()));
 		final_tx.validate(Weighting::AsTransaction, verifier_cache)?;
 
-		self.tx = final_tx;
 		Ok(())
 	}
 }
@@ -730,7 +753,10 @@ impl Serialize for Slate {
 			4 => v4.serialize(serializer),
 			// left as a reminder
 			3 => {
-				let v3 = SlateV3::from(&v4);
+				let v3 = match SlateV3::try_from(&v4) {
+					Ok(s) => s,
+					Err(e) => return Err(S::Error::custom(format!("{}", e))),
+				};
 				v3.serialize(serializer)
 			}
 			v => Err(S::Error::custom(format!("Unknown slate version {}", v))),
@@ -793,11 +819,14 @@ impl From<Slate> for SlateV4 {
 			Some(p) => Some(PaymentInfoV4::from(&p)),
 			None => None,
 		};
-		let tx = TransactionV4::from(tx);
+		let tx = match tx {
+			Some(t) => Some(TransactionV4::from(t)),
+			None => None,
+		};
 		SlateV4 {
 			num_participants,
 			id,
-			tx,
+			tx: tx,
 			amount,
 			fee,
 			height,
@@ -827,7 +856,6 @@ impl From<&Slate> for SlateV4 {
 		} = slate;
 		let num_participants = *num_participants;
 		let id = *id;
-		let tx = TransactionV4::from(tx);
 		let amount = *amount;
 		let fee = *fee;
 		let height = *height;
@@ -839,6 +867,10 @@ impl From<&Slate> for SlateV4 {
 			Some(p) => Some(PaymentInfoV4::from(p)),
 			None => None,
 		};
+		let tx = match tx {
+			Some(t) => Some(TransactionV4::from(t)),
+			None => None,
+		};
 		SlateV4 {
 			num_participants,
 			id,
@@ -1017,7 +1049,10 @@ impl From<SlateV4> for Slate {
 			Some(p) => Some(PaymentInfo::from(&p)),
 			None => None,
 		};
-		let tx = Transaction::from(tx);
+		let tx = match tx {
+			Some(t) => Some(Transaction::from(t)),
+			None => None,
+		};
 		Slate {
 			num_participants,
 			id,

libwallet/src/slate_versions/mod.rs

@@ -21,6 +21,8 @@ use crate::slate::Slate;
 use crate::slate_versions::v3::{CoinbaseV3, SlateV3};
 use crate::slate_versions::v4::{CoinbaseV4, SlateV4};
 use crate::types::CbData;
+use crate::Error;
+use std::convert::TryFrom;
 
 pub mod ser;
 
@@ -65,13 +67,13 @@ impl VersionedSlate {
 	}
 
 	/// convert this slate type to a specified older version
-	pub fn into_version(slate: Slate, version: SlateVersion) -> VersionedSlate {
+	pub fn into_version(slate: Slate, version: SlateVersion) -> Result<VersionedSlate, Error> {
 		match version {
-			SlateVersion::V4 => VersionedSlate::V4(slate.into()),
+			SlateVersion::V4 => Ok(VersionedSlate::V4(slate.into())),
 			SlateVersion::V3 => {
 				let s = SlateV4::from(slate);
-				let s = SlateV3::from(&s);
-				VersionedSlate::V3(s)
+				let s = SlateV3::try_from(&s)?;
+				Ok(VersionedSlate::V3(s))
 			}
 		}
 	}

libwallet/src/slate_versions/ser.rs

@@ -96,7 +96,7 @@ pub mod dalek_pubkey_serde {
 	{
 		use serde::de::Error;
 		String::deserialize(deserializer)
-			.and_then(|string| from_hex(string).map_err(|err| Error::custom(err.to_string())))
+			.and_then(|string| from_hex(&string).map_err(|err| Error::custom(err.to_string())))
 			.and_then(|bytes: Vec<u8>| {
 				DalekPublicKey::from_bytes(&bytes).map_err(|err| Error::custom(err.to_string()))
 			})
@@ -128,7 +128,7 @@ pub mod option_dalek_pubkey_serde {
 		D: Deserializer<'de>,
 	{
 		Option::<String>::deserialize(deserializer).and_then(|res| match res {
-			Some(string) => from_hex(string)
+			Some(string) => from_hex(&string)
 				.map_err(|err| Error::custom(err.to_string()))
 				.and_then(|bytes: Vec<u8>| {
 					let mut b = [0u8; 32];
@@ -164,7 +164,7 @@ pub mod dalek_sig_serde {
 		D: Deserializer<'de>,
 	{
 		String::deserialize(deserializer)
-			.and_then(|string| from_hex(string).map_err(|err| Error::custom(err.to_string())))
+			.and_then(|string| from_hex(&string).map_err(|err| Error::custom(err.to_string())))
 			.and_then(|bytes: Vec<u8>| {
 				let mut b = [0u8; 64];
 				b.copy_from_slice(&bytes[0..64]);
@@ -198,7 +198,7 @@ pub mod option_dalek_sig_serde {
 		D: Deserializer<'de>,
 	{
 		Option::<String>::deserialize(deserializer).and_then(|res| match res {
-			Some(string) => from_hex(string)
+			Some(string) => from_hex(&string)
 				.map_err(|err| Error::custom(err.to_string()))
 				.and_then(|bytes: Vec<u8>| {
 					let mut b = [0u8; 64];

libwallet/src/slate_versions/v4.rs

@@ -26,8 +26,10 @@ use crate::grin_util::secp::pedersen::{Commitment, RangeProof};
 use crate::grin_util::secp::Signature;
 use crate::slate::CompatKernelFeatures;
 use crate::slate_versions::ser as dalek_ser;
+use crate::{Error, ErrorKind};
 use ed25519_dalek::PublicKey as DalekPublicKey;
 use ed25519_dalek::Signature as DalekSignature;
+use std::convert::TryFrom;
 use uuid::Uuid;
 
 use crate::slate_versions::v3::{
@@ -45,7 +47,9 @@ pub struct SlateV4 {
 	pub id: Uuid,
 	/// The core transaction data:
 	/// inputs, outputs, kernels, kernel offset
-	pub tx: TransactionV4,
+	/// Optional as of V4 to allow for a compact
+	/// transaction initiation
+	pub tx: Option<TransactionV4>,
 	/// base amount (excluding fee)
 	#[serde(with = "secp_ser::string_or_u64")]
 	pub amount: u64,
@@ -236,7 +240,7 @@ impl From<SlateV3> for SlateV4 {
 			version_info,
 			num_participants,
 			id,
-			tx,
+			tx: Some(tx),
 			amount,
 			fee,
 			height,
@@ -372,8 +376,9 @@ impl From<&PaymentInfoV3> for PaymentInfoV4 {
 
 // V4 to V3
 #[allow(unused_variables)]
-impl From<&SlateV4> for SlateV3 {
-	fn from(slate: &SlateV4) -> SlateV3 {
+impl TryFrom<&SlateV4> for SlateV3 {
+	type Error = Error;
+	fn try_from(slate: &SlateV4) -> Result<SlateV3, Error> {
 		let SlateV4 {
 			num_participants,
 			id,
@@ -389,7 +394,6 @@ impl From<&SlateV4> for SlateV3 {
 		} = slate;
 		let num_participants = *num_participants;
 		let id = *id;
-		let tx = TransactionV3::from(tx);
 		let amount = *amount;
 		let fee = *fee;
 		let height = *height;
@@ -400,8 +404,18 @@ impl From<&SlateV4> for SlateV3 {
 			Some(p) => Some(PaymentInfoV3::from(p)),
 			None => None,
 		};
+		let tx = match tx {
+			Some(t) => TransactionV3::from(t),
+			None => {
+				return Err(ErrorKind::SlateInvalidDowngrade(
+					"Full transaction info required".to_owned(),
+				)
+				.into())
+			}
+		};
+
 		let ttl_cutoff_height = *ttl_cutoff_height;
-		SlateV3 {
+		Ok(SlateV3 {
 			num_participants,
 			id,
 			tx,
@@ -413,7 +427,7 @@ impl From<&SlateV4> for SlateV3 {
 			participant_data,
 			version_info,
 			payment_proof,
-		}
+		})
 	}
 }