handle transaction inputs correctly (#512)

* commit

* bump grin crates to latest version on master

impls/src/node_clients/http.rs

@@ -374,12 +374,14 @@ impl NodeClient for HTTPNodeClient {
 #[cfg(test)]
 mod tests {
 	use super::*;
-	use crate::core::core::KernelFeatures;
+	use crate::core::core::{KernelFeatures, OutputFeatures, OutputIdentifier};
 	use crate::core::libtx::build;
 	use crate::core::libtx::ProofBuilder;
 	use crate::keychain::{ExtKeychain, Keychain};
 
-	fn tx1i1o() -> Transaction {
+	// JSON api for "push_transaction" between wallet->node currently only supports "feature and commit" inputs.
+	// We will need to revisit this if we decide to support "commit only" inputs (no features) at wallet level.
+	fn tx1i1o_v2_compatible() -> Transaction {
 		let keychain = ExtKeychain::from_random_seed(false).unwrap();
 		let builder = ProofBuilder::new(&keychain);
 		let key_id1 = ExtKeychain::derive_key_id(1, 1, 0, 0, 0);
@@ -391,14 +393,26 @@ mod tests {
 			&builder,
 		)
 		.unwrap();
-		tx
+
+		let inputs: Vec<_> = tx.inputs().into();
+		let inputs: Vec<_> = inputs
+			.iter()
+			.map(|input| OutputIdentifier {
+				features: OutputFeatures::Plain,
+				commit: input.commitment(),
+			})
+			.collect();
+		Transaction {
+			body: tx.body.replace_inputs(inputs.as_slice().into()),
+			..tx
+		}
 	}
 
 	// Wallet will "push" a transaction to node, serializing the transaction as json.
 	// We are testing the json structure is what we expect here.
 	#[test]
 	fn test_transaction_json_ser_deser() {
-		let tx1 = tx1i1o();
+		let tx1 = tx1i1o_v2_compatible();
 		let value = serde_json::to_value(&tx1).unwrap();
 
 		assert!(value["offset"].is_string());

libwallet/src/api_impl/foreign.rs

@@ -17,7 +17,6 @@ use strum::IntoEnumIterator;
 
 use crate::api_impl::owner::finalize_tx as owner_finalize;
 use crate::api_impl::owner::{check_ttl, post_tx};
-use crate::grin_core::core::transaction::Transaction;
 use crate::grin_keychain::Keychain;
 use crate::grin_util::secp::key::SecretKey;
 use crate::internal::{selection, tx, updater};
@@ -91,7 +90,7 @@ where
 		}
 	}
 
-	ret_slate.tx = Some(Transaction::empty());
+	ret_slate.tx = Some(Slate::empty_transaction());
 
 	let height = w.last_confirmed_height()?;
 	let keychain = w.keychain(keychain_mask)?;

libwallet/src/api_impl/owner.rs

@@ -647,7 +647,7 @@ where
 	}
 
 	// if this is compact mode, we need to create the transaction now
-	ret_slate.tx = Some(Transaction::empty());
+	ret_slate.tx = Some(Slate::empty_transaction());
 
 	// if self sending, make sure to store 'initiator' keys
 	let context_res = w.get_private_context(keychain_mask, slate.id.as_bytes());
@@ -721,7 +721,7 @@ where
 	let mut sl = slate.clone();
 
 	if sl.tx == None {
-		sl.tx = Some(Transaction::empty());
+		sl.tx = Some(Slate::empty_transaction());
 		selection::repopulate_tx(&mut *w, keychain_mask, &mut sl, &context, true)?;
 	}
 

libwallet/src/internal/tx.rs

@@ -585,7 +585,7 @@ mod test {
 	use super::*;
 	use rand::rngs::mock::StepRng;
 
-	use crate::grin_core::core::{Input, KernelFeatures};
+	use crate::grin_core::core::KernelFeatures;
 	use crate::grin_core::libtx::{build, ProofBuilder};
 	use crate::grin_keychain::{
 		BlindSum, BlindingFactor, ExtKeychain, ExtKeychainPath, Keychain, SwitchCommitmentType,
@@ -615,8 +615,7 @@ mod test {
 		)
 		.unwrap();
 
-		let inputs: Vec<Input> = tx2.inputs().into();
-		assert_eq!(tx1.outputs()[0].features(), inputs[0].features);
+		let inputs: Vec<_> = tx2.inputs().into();
 		assert_eq!(tx1.outputs()[0].commitment(), inputs[0].commitment());
 	}
 

libwallet/src/slate.rs

@@ -18,8 +18,8 @@
 use crate::error::{Error, ErrorKind};
 use crate::grin_core::core::amount_to_hr_string;
 use crate::grin_core::core::transaction::{
-	Input, KernelFeatures, NRDRelativeHeight, Output, OutputFeatures, Transaction, TxKernel,
-	Weighting,
+	Input, Inputs, KernelFeatures, NRDRelativeHeight, Output, OutputFeatures, Transaction,
+	TxKernel, Weighting,
 };
 use crate::grin_core::core::verifier_cache::LruVerifierCache;
 use crate::grin_core::libtx::{aggsig, build, proof::ProofBuild, tx_fee};
@@ -242,6 +242,14 @@ impl Slate {
 		Ok(())
 	}
 
+	/// Build a new empty transaction.
+	/// Wallet currently only supports tx with "features and commit" inputs.
+	pub fn empty_transaction() -> Transaction {
+		let mut tx = Transaction::empty();
+		tx.body = tx.body.replace_inputs(Inputs::FeaturesAndCommit(vec![]));
+		tx
+	}
+
 	/// Create a new slate
 	pub fn blank(num_participants: u8, is_invoice: bool) -> Slate {
 		let np = match num_participants {
@@ -256,7 +264,7 @@ impl Slate {
 			num_participants: np, // assume 2 if not present
 			id: Uuid::new_v4(),
 			state,
-			tx: Some(Transaction::empty()),
+			tx: Some(Slate::empty_transaction()),
 			amount: 0,
 			fee: 0,
 			ttl_cutoff_height: 0,
@@ -816,35 +824,26 @@ impl From<&Slate> for SlateV4 {
 	}
 }
 
-// Node's Transaction object and lock height to SlateV4 `coms`
 impl From<&Slate> for Option<Vec<CommitsV4>> {
-	fn from(slate: &Slate) -> Option<Vec<CommitsV4>> {
-		let mut ret_vec = vec![];
-		let (ins, outs) = match &slate.tx {
-			Some(t) => {
-				// TODO - input features are to be deprecated
-				// inputs here should be treated as a vec of commitments
-				// CommitsV4 should probably handle optional features.
-				let ins: Vec<Input> = t.inputs().into();
-				(ins, t.outputs().to_vec())
+	fn from(slate: &Slate) -> Self {
+		match slate.tx {
+			None => None,
+			Some(ref tx) => {
+				let mut ret_vec = vec![];
+				match tx.inputs() {
+					Inputs::CommitOnly(_) => panic!("commit only inputs unsupported"),
+					Inputs::FeaturesAndCommit(ref inputs) => {
+						for input in inputs {
+							ret_vec.push(input.into());
+						}
+					}
+				}
+				for output in tx.outputs() {
+					ret_vec.push(output.into());
+				}
+				Some(ret_vec)
 			}
-			None => return None,
-		};
-		for i in ins.iter() {
-			ret_vec.push(CommitsV4 {
-				f: i.features.into(),
-				c: i.commit,
-				p: None,
-			});
-		}
-		for o in outs.iter() {
-			ret_vec.push(CommitsV4 {
-				f: o.features().into(),
-				c: o.commitment(),
-				p: Some(o.proof),
-			});
 		}
-		Some(ret_vec)
 	}
 }
 
@@ -1017,20 +1016,30 @@ pub fn tx_from_slate_v4(slate: &SlateV4) -> Option<Transaction> {
 		excess,
 		excess_sig,
 	};
-	let mut tx = Transaction::empty().with_kernel(kernel);
+	let mut tx = Slate::empty_transaction().with_kernel(kernel);
+
+	let mut outputs = vec![];
+	let mut inputs = vec![];
 
 	for c in coms.iter() {
-		match c.p {
-			Some(p) => tx = tx.with_output(Output::new(c.f.into(), c.c, p)),
+		match &c.p {
+			Some(p) => {
+				outputs.push(Output::new(c.f.into(), c.c, p.clone()));
+			}
 			None => {
-				tx = tx.with_input(Input {
+				inputs.push(Input {
 					features: c.f.into(),
 					commit: c.c,
-				})
+				});
 			}
 		}
 	}
-	tx = tx.with_offset(slate.off.clone());
+
+	tx.body = tx
+		.body
+		.replace_inputs(inputs.as_slice().into())
+		.replace_outputs(outputs.as_slice());
+	tx.offset = slate.off.clone();
 	Some(tx)
 }
 

libwallet/src/slate_versions/v4.rs

@@ -53,7 +53,7 @@
 //! *  The `receiver_signature` field is renamed to `rsig`
 //! * `rsig` may be omitted if it has not yet been filled out
 
-use crate::grin_core::core::{Output, TxKernel};
+use crate::grin_core::core::{Input, Output, TxKernel};
 use crate::grin_core::libtx::secp_ser;
 use crate::grin_keychain::{BlindingFactor, Identifier};
 use crate::grin_util::secp;
@@ -252,6 +252,27 @@ pub struct CommitsV4 {
 	pub p: Option<RangeProof>,
 }
 
+impl From<&Output> for CommitsV4 {
+	fn from(out: &Output) -> CommitsV4 {
+		CommitsV4 {
+			f: out.features().into(),
+			c: out.commitment(),
+			p: Some(out.proof()),
+		}
+	}
+}
+
+// This will need to be reworked once we no longer support input features with "commit only" inputs.
+impl From<&Input> for CommitsV4 {
+	fn from(input: &Input) -> CommitsV4 {
+		CommitsV4 {
+			f: input.features.into(),
+			c: input.commitment(),
+			p: None,
+		}
+	}
+}
+
 fn default_output_feature() -> OutputFeaturesV4 {
 	OutputFeaturesV4(0)
 }