tx inputs underwent refactoring (#502)

Cargo.lock

@@ -1166,8 +1166,8 @@ checksum = "9b919933a397b79c37e33b77bb2aa3dc8eb6e165ad809e58ff75bc7db2e34574"
 
 [[package]]
 name = "grin_api"
-version = "4.0.1-alpha.1"
+version = "4.1.0-alpha.1"
-source = "git+https://github.com/mimblewimble/grin#30db9c410ee5551ed65ac74a0e4e45a281fac6ef"
+source = "git+https://github.com/mimblewimble/grin#4732a0b62bfaed8066fecf8adc6641ce1e0792f7"
 dependencies = [
  "bytes",
  "easy-jsonrpc-mw",
@@ -1199,8 +1199,8 @@ dependencies = [
 
 [[package]]
 name = "grin_chain"
-version = "4.0.1-alpha.1"
+version = "4.1.0-alpha.1"
-source = "git+https://github.com/mimblewimble/grin#30db9c410ee5551ed65ac74a0e4e45a281fac6ef"
+source = "git+https://github.com/mimblewimble/grin#4732a0b62bfaed8066fecf8adc6641ce1e0792f7"
 dependencies = [
  "bit-vec",
  "bitflags 1.2.1",
@@ -1223,8 +1223,8 @@ dependencies = [
 
 [[package]]
 name = "grin_core"
-version = "4.0.1-alpha.1"
+version = "4.1.0-alpha.1"
-source = "git+https://github.com/mimblewimble/grin#30db9c410ee5551ed65ac74a0e4e45a281fac6ef"
+source = "git+https://github.com/mimblewimble/grin#4732a0b62bfaed8066fecf8adc6641ce1e0792f7"
 dependencies = [
  "blake2-rfc",
  "byteorder",
@@ -1249,8 +1249,8 @@ dependencies = [
 
 [[package]]
 name = "grin_keychain"
-version = "4.0.1-alpha.1"
+version = "4.1.0-alpha.1"
-source = "git+https://github.com/mimblewimble/grin#30db9c410ee5551ed65ac74a0e4e45a281fac6ef"
+source = "git+https://github.com/mimblewimble/grin#4732a0b62bfaed8066fecf8adc6641ce1e0792f7"
 dependencies = [
  "blake2-rfc",
  "byteorder",
@@ -1271,8 +1271,8 @@ dependencies = [
 
 [[package]]
 name = "grin_p2p"
-version = "4.0.1-alpha.1"
+version = "4.1.0-alpha.1"
-source = "git+https://github.com/mimblewimble/grin#30db9c410ee5551ed65ac74a0e4e45a281fac6ef"
+source = "git+https://github.com/mimblewimble/grin#4732a0b62bfaed8066fecf8adc6641ce1e0792f7"
 dependencies = [
  "bitflags 1.2.1",
  "chrono",
@@ -1292,8 +1292,8 @@ dependencies = [
 
 [[package]]
 name = "grin_pool"
-version = "4.0.1-alpha.1"
+version = "4.1.0-alpha.1"
-source = "git+https://github.com/mimblewimble/grin#30db9c410ee5551ed65ac74a0e4e45a281fac6ef"
+source = "git+https://github.com/mimblewimble/grin#4732a0b62bfaed8066fecf8adc6641ce1e0792f7"
 dependencies = [
  "blake2-rfc",
  "chrono",
@@ -1326,8 +1326,8 @@ dependencies = [
 
 [[package]]
 name = "grin_store"
-version = "4.0.1-alpha.1"
+version = "4.1.0-alpha.1"
-source = "git+https://github.com/mimblewimble/grin#30db9c410ee5551ed65ac74a0e4e45a281fac6ef"
+source = "git+https://github.com/mimblewimble/grin#4732a0b62bfaed8066fecf8adc6641ce1e0792f7"
 dependencies = [
  "byteorder",
  "croaring-mw",
@@ -1346,8 +1346,8 @@ dependencies = [
 
 [[package]]
 name = "grin_util"
-version = "4.0.1-alpha.1"
+version = "4.1.0-alpha.1"
-source = "git+https://github.com/mimblewimble/grin#30db9c410ee5551ed65ac74a0e4e45a281fac6ef"
+source = "git+https://github.com/mimblewimble/grin#4732a0b62bfaed8066fecf8adc6641ce1e0792f7"
 dependencies = [
  "backtrace",
  "base64 0.12.1",

controller/tests/revert.rs

@@ -173,11 +173,11 @@ fn revert(
 		api.tx_lock_outputs(m, &slate)?;
 		let slate = client1.send_tx_slate_direct("wallet2", &slate)?;
 		let slate = api.finalize_tx(m, &slate)?;
-		tx = Some(slate.tx);
+		tx = slate.tx;
 
 		Ok(())
 	})?;
-	let tx = tx.unwrap();
+	let tx = tx.expect("tx from slate");
 
 	// Check funds have been received
 	owner(Some(wallet2.clone()), mask2, None, |api, m| {
@@ -207,14 +207,9 @@ fn revert(
 
 	// Build 2 blocks at same height: 1 with the tx, 1 without
 	let head = chain.head_header().unwrap();
-	let block_with = create_block_for_wallet(
+	let block_with =
-		&chain,
+		create_block_for_wallet(&chain, head.clone(), &[tx.clone()], wallet1.clone(), mask1)?;
-		head.clone(),
+	let block_without = create_block_for_wallet(&chain, head, &[], wallet1.clone(), mask1)?;
-		vec![&tx.as_ref().unwrap()],
-		wallet1.clone(),
-		mask1,
-	)?;
-	let block_without = create_block_for_wallet(&chain, head, vec![], wallet1.clone(), mask1)?;
 
 	// Add block with tx to the chain
 	process_block(&chain, block_with.clone());
@@ -246,7 +241,7 @@ fn revert(
 	})?;
 
 	// Attach more blocks to the parallel chain, making it the longest one
-	award_block_to_wallet(&chain2, vec![], wallet1.clone(), mask1)?;
+	award_block_to_wallet(&chain2, &[], wallet1.clone(), mask1)?;
 	assert_eq!(chain2.head_header().unwrap().height, bh + 1);
 	let new_head = chain2
 		.get_block(&chain2.head_header().unwrap().hash())
@@ -282,15 +277,7 @@ fn revert(
 
 	stopper2.store(false, Ordering::Relaxed);
 	Ok((
-		chain,
+		chain, stopper, sent, bh, tx, wallet1, mask1_i, wallet2, mask2_i,
-		stopper,
-		sent,
-		bh,
-		tx.unwrap(),
-		wallet1,
-		mask1_i,
-		wallet2,
-		mask2_i,
 	))
 }
 
@@ -300,7 +287,7 @@ fn revert_reconfirm_impl(test_dir: &'static str) -> Result<(), libwallet::Error>
 	let mask2 = mask2_i.as_ref();
 
 	// Include the tx into the chain again, the tx should no longer be reverted
-	award_block_to_wallet(&chain, vec![&tx], wallet1.clone(), mask1)?;
+	award_block_to_wallet(&chain, &[tx], wallet1.clone(), mask1)?;
 
 	let bh = bh + 1;
 

impls/src/test_framework/mod.rs

@@ -85,9 +85,7 @@ fn get_outputs_by_pmmr_index_local(
 		outputs: outputs
 			.2
 			.iter()
-			.map(|x| {
+			.map(|x| api::OutputPrintable::from_output(x, &chain, None, true, false).unwrap())
-				api::OutputPrintable::from_output(x, chain.clone(), None, true, false).unwrap()
-			})
 			.collect(),
 	}
 }
@@ -111,14 +109,14 @@ fn height_range_to_pmmr_indices_local(
 fn create_block_with_reward(
 	chain: &Chain,
 	prev: core::core::BlockHeader,
-	txs: Vec<&Transaction>,
+	txs: &[Transaction],
 	reward_output: Output,
 	reward_kernel: TxKernel,
 ) -> core::core::Block {
 	let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter().unwrap());
 	let mut b = core::core::Block::new(
 		&prev,
-		txs.into_iter().cloned().collect(),
+		txs,
 		next_header_info.clone().difficulty,
 		(reward_output, reward_kernel),
 	)
@@ -139,7 +137,7 @@ fn create_block_with_reward(
 /// Adds a block with a given reward to the chain and mines it
 pub fn add_block_with_reward(
 	chain: &Chain,
-	txs: Vec<&Transaction>,
+	txs: &[Transaction],
 	reward_output: Output,
 	reward_kernel: TxKernel,
 ) {
@@ -153,7 +151,7 @@ pub fn add_block_with_reward(
 pub fn create_block_for_wallet<'a, L, C, K>(
 	chain: &Chain,
 	prev: core::core::BlockHeader,
-	txs: Vec<&Transaction>,
+	txs: &[Transaction],
 	wallet: Arc<Mutex<Box<dyn WalletInst<'a, L, C, K> + 'a>>>,
 	keychain_mask: Option<&SecretKey>,
 ) -> Result<core::core::Block, libwallet::Error>
@@ -184,7 +182,7 @@ where
 /// Helpful for building up precise wallet balances for testing.
 pub fn award_block_to_wallet<'a, L, C, K>(
 	chain: &Chain,
-	txs: Vec<&Transaction>,
+	txs: &[Transaction],
 	wallet: Arc<Mutex<Box<dyn WalletInst<'a, L, C, K> + 'a>>>,
 	keychain_mask: Option<&SecretKey>,
 ) -> Result<(), libwallet::Error>
@@ -218,7 +216,7 @@ where
 	K: keychain::Keychain + 'a,
 {
 	for _ in 0..number {
-		award_block_to_wallet(chain, vec![], wallet.clone(), keychain_mask)?;
+		award_block_to_wallet(chain, &[], wallet.clone(), keychain_mask)?;
 		if pause_between {
 			thread::sleep(std::time::Duration::from_millis(100));
 		}

impls/src/test_framework/testclient.rs

@@ -190,7 +190,7 @@ where
 
 		super::award_block_to_wallet(
 			&self.chain,
-			vec![&tx],
+			&[tx],
 			dest_wallet,
 			(&dest_wallet_mask).as_ref(),
 		)?;

libwallet/src/internal/tx.rs

@@ -584,7 +584,7 @@ mod test {
 	use super::*;
 	use rand::rngs::mock::StepRng;
 
-	use crate::grin_core::core::KernelFeatures;
+	use crate::grin_core::core::{Input, KernelFeatures};
 	use crate::grin_core::libtx::{build, ProofBuilder};
 	use crate::grin_keychain::{
 		BlindSum, BlindingFactor, ExtKeychain, ExtKeychainPath, Keychain, SwitchCommitmentType,
@@ -601,21 +601,22 @@ mod test {
 
 		let tx1 = build::transaction(
 			KernelFeatures::Plain { fee: 0 },
-			vec![build::output(105, key_id1.clone())],
+			&[build::output(105, key_id1.clone())],
 			&keychain,
 			&builder,
 		)
 		.unwrap();
 		let tx2 = build::transaction(
 			KernelFeatures::Plain { fee: 0 },
-			vec![build::input(105, key_id1.clone())],
+			&[build::input(105, key_id1.clone())],
 			&keychain,
 			&builder,
 		)
 		.unwrap();
 
-		assert_eq!(tx1.outputs()[0].features, tx2.inputs()[0].features);
+		let inputs: Vec<Input> = tx2.inputs().into();
-		assert_eq!(tx1.outputs()[0].commitment(), tx2.inputs()[0].commitment());
+		assert_eq!(tx1.outputs()[0].features, inputs[0].features);
+		assert_eq!(tx1.outputs()[0].commitment(), inputs[0].commitment());
 	}
 
 	#[test]

libwallet/src/slate.rs

@@ -312,7 +312,7 @@ impl Slate {
 			return Ok(BlindingFactor::zero());
 		}
 		let (tx, blind) =
-			build::partial_transaction(self.tx_or_err()?.clone(), elems, keychain, builder)?;
+			build::partial_transaction(self.tx_or_err()?.clone(), &elems, keychain, builder)?;
 		self.tx = Some(tx);
 		Ok(blind)
 	}
@@ -673,18 +673,22 @@ impl Slate {
 
 		debug!("Final Tx excess: {:?}", final_excess);
 
-		let final_tx = self.tx_or_err_mut()?;
+		let final_tx = self.tx_or_err()?;
 
 		// update the tx kernel to reflect the offset excess and sig
 		assert_eq!(final_tx.kernels().len(), 1);
-		final_tx.kernels_mut()[0].excess = final_excess.clone();
+
-		final_tx.kernels_mut()[0].excess_sig = final_sig.clone();
+		let mut kernel = final_tx.kernels()[0];
+		kernel.excess = final_excess;
+		kernel.excess_sig = final_sig.clone();
+
+		let final_tx = final_tx.clone().replace_kernel(kernel);
 
 		// confirm the kernel verifies successfully before proceeding
 		debug!("Validating final transaction");
 		trace!(
 			"Final tx: {}",
-			serde_json::to_string_pretty(final_tx).unwrap()
+			serde_json::to_string_pretty(&final_tx).unwrap()
 		);
 		final_tx.kernels()[0].verify()?;
 
@@ -695,6 +699,9 @@ impl Slate {
 			error!("Error with final tx validation: {}", e);
 			Err(e.into())
 		} else {
+			// replace our slate tx with the new one with updated kernel
+			self.tx = Some(final_tx);
+
 			Ok(())
 		}
 	}
@@ -827,8 +834,14 @@ impl From<&Slate> for SlateV4 {
 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.as_ref() {
+		let (ins, outs) = match &slate.tx {
-			Some(t) => (t.body.inputs.clone(), t.body.outputs.clone()),
+			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())
+			}
 			None => return None,
 		};
 		for i in ins.iter() {
@@ -951,11 +964,11 @@ impl From<&Transaction> for TransactionV4 {
 
 impl From<&TransactionBody> for TransactionBodyV4 {
 	fn from(body: &TransactionBody) -> TransactionBodyV4 {
-		let TransactionBody {
+		// TODO - input features will soon be deprecated.
-			inputs,
+		// We should treat inputs here as vec of commitments.
-			outputs,
+		let inputs: Vec<Input> = body.inputs().into();
-			kernels,
+		let outputs = body.outputs();
-		} = body;
+		let kernels = body.kernels();
 
 		let inputs = map_vec!(inputs, |inp| InputV4::from(inp));
 		let outputs = map_vec!(outputs, |out| OutputV4::from(out));
@@ -1106,19 +1119,23 @@ pub fn tx_from_slate_v4(slate: &SlateV4) -> Option<Transaction> {
 		excess,
 		excess_sig,
 	};
-	let mut tx = Transaction::empty();
+	let mut tx = Transaction::empty().with_kernel(kernel);
-	tx.body.kernels.push(kernel);
+
 	for c in coms.iter() {
 		match &c.p {
-			Some(p) => tx.body.outputs.push(Output {
+			Some(p) => {
-				features: c.f.into(),
+				tx = tx.with_output(Output {
-				commit: c.c,
+					features: c.f.into(),
-				proof: p.clone(),
+					commit: c.c,
-			}),
+					proof: p.clone(),
-			None => tx.body.inputs.push(Input {
+				})
-				features: c.f.into(),
+			}
-				commit: c.c,
+			None => {
-			}),
+				tx = tx.with_input(Input {
+					features: c.f.into(),
+					commit: c.c,
+				})
+			}
 		}
 	}
 	tx.offset = slate.off.clone();
@@ -1230,7 +1247,7 @@ impl From<&TransactionBodyV4> for TransactionBody {
 		let outputs = map_vec!(outs, |out| Output::from(out));
 		let kernels = map_vec!(kers, |kern| TxKernel::from(kern));
 		TransactionBody {
-			inputs,
+			inputs: inputs.into(),
 			outputs,
 			kernels,
 		}