Cleanup transaction with offset (#1514)

* cleanup build::transaction_with_offset

* rustfmt

chain/tests/test_txhashset.rs

@@ -16,12 +16,12 @@ extern crate grin_chain as chain;
 extern crate grin_core as core;
 extern crate grin_keychain as keychain;
 extern crate grin_store as store;
-extern crate grin_wallet as wallet;
 extern crate grin_util as util;
+extern crate grin_wallet as wallet;
 
 use std::collections::HashSet;
-use std::iter::FromIterator;
 use std::fs::{self, File, OpenOptions};
+use std::iter::FromIterator;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
@@ -81,12 +81,9 @@ fn test_some_raw_txs() {
 	let coinbase_reward = 60_000_000_000;
 
 	// tx1 spends the original coinbase output from the block
-	let tx1 = build::transaction_with_offset(
+	let tx1 = build::transaction(
 		vec![
-			build::coinbase_input(
+			build::coinbase_input(coinbase_reward, key_id1.clone()),
-				coinbase_reward,
-				key_id1.clone(),
-			),
 			build::output(100, key_id2.clone()),
 			build::output(150, key_id3.clone()),
 		],
@@ -95,19 +92,16 @@ fn test_some_raw_txs() {
 
 	// tx2 attempts to "double spend" the coinbase output from the block (conflicts
 	// with tx1)
-	let tx2 = build::transaction_with_offset(
+	let tx2 = build::transaction(
 		vec![
-			build::coinbase_input(
+			build::coinbase_input(coinbase_reward, key_id1.clone()),
-				coinbase_reward,
-				key_id1.clone(),
-			),
 			build::output(100, key_id4.clone()),
 		],
 		&keychain,
 	).unwrap();
 
 	// tx3 spends one output from tx1
-	let tx3 = build::transaction_with_offset(
+	let tx3 = build::transaction(
 		vec![
 			build::input(100, key_id2.clone()),
 			build::output(90, key_id5.clone()),
@@ -116,7 +110,7 @@ fn test_some_raw_txs() {
 	).unwrap();
 
 	// tx4 spends the other output from tx1 and the output from tx3
-	let tx4 = build::transaction_with_offset(
+	let tx4 = build::transaction(
 		vec![
 			build::input(150, key_id3.clone()),
 			build::input(90, key_id5.clone()),
@@ -160,47 +154,87 @@ fn test_unexpected_zip() {
 	assert!(txhashset::zip_read(db_root.clone(), &BlockHeader::default()).is_ok());
 	// Check that the temp dir dos not contains the strange files
 	let txhashset_zip_path = Path::new(&db_root).join("txhashset_zip");
-	assert!(txhashset_contains_expected_files("txhashset_zip".to_string(), txhashset_zip_path.clone()));
+	assert!(txhashset_contains_expected_files(
+		"txhashset_zip".to_string(),
+		txhashset_zip_path.clone()
+	));
 	fs::remove_dir_all(Path::new(&db_root).join("txhashset_zip")).unwrap();
 
 	let zip_file = File::open(zip_path).unwrap();
 	assert!(txhashset::zip_write(db_root.clone(), zip_file, &BlockHeader::default()).is_ok());
 	// Check that the txhashset dir dos not contains the strange files
 	let txhashset_path = Path::new(&db_root).join("txhashset");
-	assert!(txhashset_contains_expected_files("txhashset".to_string(), txhashset_path.clone()));
+	assert!(txhashset_contains_expected_files(
+		"txhashset".to_string(),
+		txhashset_path.clone()
+	));
 	fs::remove_dir_all(Path::new(&db_root).join("txhashset")).unwrap();
 }
 
-fn write_file (db_root: String) {
+fn write_file(db_root: String) {
 	OpenOptions::new()
 		.create(true)
 		.write(true)
-		.open(Path::new(&db_root).join("txhashset").join("kernel").join("strange0")).unwrap();
+		.open(
+			Path::new(&db_root)
+				.join("txhashset")
+				.join("kernel")
+				.join("strange0"),
+		)
+		.unwrap();
 	OpenOptions::new()
 		.create(true)
 		.write(true)
-		.open(Path::new(&db_root).join("txhashset").join("strange1")).unwrap();
+		.open(Path::new(&db_root).join("txhashset").join("strange1"))
+		.unwrap();
 	fs::create_dir(Path::new(&db_root).join("txhashset").join("strange_dir")).unwrap();
 	OpenOptions::new()
 		.create(true)
 		.write(true)
-		.open(Path::new(&db_root).join("txhashset").join("strange_dir").join("strange2")).unwrap();
+		.open(
-	fs::create_dir(Path::new(&db_root).join("txhashset").join("strange_dir").join("strange_subdir")).unwrap();
+			Path::new(&db_root)
+				.join("txhashset")
+				.join("strange_dir")
+				.join("strange2"),
+		)
+		.unwrap();
+	fs::create_dir(
+		Path::new(&db_root)
+			.join("txhashset")
+			.join("strange_dir")
+			.join("strange_subdir"),
+	).unwrap();
 	OpenOptions::new()
 		.create(true)
 		.write(true)
-		.open(Path::new(&db_root).join("txhashset").join("strange_dir").join("strange_subdir").join("strange3")).unwrap();
+		.open(
+			Path::new(&db_root)
+				.join("txhashset")
+				.join("strange_dir")
+				.join("strange_subdir")
+				.join("strange3"),
+		)
+		.unwrap();
 }
 
 fn txhashset_contains_expected_files(dirname: String, path_buf: PathBuf) -> bool {
 	let list_zip_files = file::list_files(path_buf.into_os_string().into_string().unwrap());
 	let zip_files_hashset: HashSet<_> = HashSet::from_iter(list_zip_files.iter().cloned());
-	let expected_files = vec![dirname, "output".to_string(), "rangeproof".to_string(), "kernel".to_string(), "pmmr_hash.bin".to_string(), "pmmr_data.bin".to_string()];
+	let expected_files = vec![
+		dirname,
+		"output".to_string(),
+		"rangeproof".to_string(),
+		"kernel".to_string(),
+		"pmmr_hash.bin".to_string(),
+		"pmmr_data.bin".to_string(),
+	];
 	let expected_files_hashset = HashSet::from_iter(expected_files.iter().cloned());
-	let intersection: HashSet<_>  = zip_files_hashset.difference(&expected_files_hashset).collect();
+	let intersection: HashSet<_> = zip_files_hashset
+		.difference(&expected_files_hashset)
+		.collect();
 	if intersection.is_empty() {
-	 true
+		true
- 	}	else {
+	} else {
-	 false
+		false
-	 }
+	}
 }

core/fuzz/src/main.rs

@@ -7,7 +7,7 @@ use grin_core::core::{Block, BlockHeader, CompactBlock, Transaction};
 use grin_core::ser;
 use grin_keychain::keychain::ExtKeychain;
 use grin_keychain::Keychain;
-use grin_wallet::libtx::build::{input, output, transaction_with_offset, with_fee};
+use grin_wallet::libtx::build::{input, output, transaction, with_fee};
 use grin_wallet::libtx::reward;
 use std::fs::{self, File};
 use std::path::Path;
@@ -69,7 +69,7 @@ fn tx() -> Transaction {
 	let key_id2 = keychain.derive_key_id(2).unwrap();
 	let key_id3 = keychain.derive_key_id(3).unwrap();
 
-	transaction_with_offset(
+	transaction(
 		vec![
 			input(10, key_id1),
 			input(11, key_id2),

core/tests/common/mod.rs

@@ -33,7 +33,7 @@ pub fn tx2i1o() -> Transaction {
 	let key_id2 = keychain.derive_key_id(2).unwrap();
 	let key_id3 = keychain.derive_key_id(3).unwrap();
 
-	build::transaction_with_offset(
+	build::transaction(
 		vec![
 			input(10, key_id1),
 			input(11, key_id2),
@@ -50,7 +50,7 @@ pub fn tx1i1o() -> Transaction {
 	let key_id1 = keychain.derive_key_id(1).unwrap();
 	let key_id2 = keychain.derive_key_id(2).unwrap();
 
-	build::transaction_with_offset(
+	build::transaction(
 		vec![input(5, key_id1), output(3, key_id2), with_fee(2)],
 		&keychain,
 	).unwrap()
@@ -65,7 +65,7 @@ pub fn tx1i2o() -> Transaction {
 	let key_id2 = keychain.derive_key_id(2).unwrap();
 	let key_id3 = keychain.derive_key_id(3).unwrap();
 
-	build::transaction_with_offset(
+	build::transaction(
 		vec![
 			input(6, key_id1),
 			output(3, key_id2),

core/tests/transaction.rs

@@ -20,7 +20,7 @@ extern crate grin_wallet as wallet;
 
 pub mod common;
 
-use grin_core::core::{Output, OutputFeatures, Transaction};
+use grin_core::core::{Output, OutputFeatures};
 use grin_core::ser;
 use keychain::{ExtKeychain, Keychain};
 use wallet::libtx::proof;

wallet/src/libtx/build.rs

@@ -215,35 +215,6 @@ pub fn transaction<K>(
 	elems: Vec<Box<Append<K>>>,
 	keychain: &K,
 ) -> Result<Transaction, keychain::Error>
-where
-	K: Keychain,
-{
-	let (mut tx, blind_sum) = partial_transaction(elems, keychain)?;
-	assert_eq!(tx.kernels().len(), 1);
-
-	let kern = {
-		let mut kern = tx.kernels_mut().remove(0);
-		let msg = secp::Message::from_slice(&kernel_sig_msg(kern.fee, kern.lock_height))?;
-
-		let skey = blind_sum.secret_key(&keychain.secp())?;
-		kern.excess = keychain.secp().commit(0, skey)?;
-		kern.excess_sig = aggsig::sign_with_blinding(&keychain.secp(), &msg, &blind_sum).unwrap();
-		kern
-	};
-
-	// Now build a new tx with this single kernel.
-	let tx = tx.with_kernel(kern);
-	assert_eq!(tx.kernels().len(), 1);
-
-	Ok(tx)
-}
-
-/// Builds a complete transaction, splitting the key and
-/// setting the excess, excess_sig and tx offset as necessary.
-pub fn transaction_with_offset<K>(
-	elems: Vec<Box<Append<K>>>,
-	keychain: &K,
-) -> Result<Transaction, keychain::Error>
 where
 	K: Keychain,
 {
@@ -254,21 +225,24 @@ where
 	);
 	let blind_sum = ctx.keychain.blind_sum(&sum)?;
 
+	// Split the key so we can generate an offset for the tx.
 	let split = blind_sum.split(&keychain.secp())?;
 	let k1 = split.blind_1;
 	let k2 = split.blind_2;
 
+	// Construct the message to be signed.
 	let msg = secp::Message::from_slice(&kernel_sig_msg(kern.fee, kern.lock_height))?;
 
-	// generate kernel excess and excess_sig using the split key k1
+	// Generate kernel excess and excess_sig using the split key k1.
 	let skey = k1.secret_key(&keychain.secp())?;
 	kern.excess = ctx.keychain.secp().commit(0, skey)?;
 	kern.excess_sig = aggsig::sign_with_blinding(&keychain.secp(), &msg, &k1).unwrap();
 
-	// store the kernel offset (k2) on the tx itself
+	// Store the kernel offset (k2) on the tx.
-	// commitments will sum correctly when including the offset
+	// Commitments will sum correctly when accounting for the offset.
 	tx.offset = k2.clone();
 
+	// Set the kernel on the tx (assert this is now a single-kernel tx).
 	assert!(tx.kernels().is_empty());
 	let tx = tx.with_kernel(kern);
 	assert_eq!(tx.kernels().len(), 1);
@@ -320,7 +294,7 @@ mod test {
 
 		let vc = verifier_cache();
 
-		let tx = transaction_with_offset(
+		let tx = transaction(
 			vec![
 				input(10, key_id1),
 				input(12, key_id2),