add lock_height to Transaction and TxKernel (#167)

* add lock_height to Transaction and TxKernel, sign msg including both fee and lock_height in kernel
* make the order of the fields in tx and kernel more consistent
* rename to kernel_sig_msg
* add test to cover adding timelocked tx to pool, where tx is invalid based on current height of the blockchain
* add tests for adding timelocked txs to blocks (valid and otherwise)

.hooks/pre-commit

@@ -26,23 +26,23 @@ problem_files=()
 printf "[pre_commit] rustfmt "
 for file in $(git diff --name-only --cached); do
 	if [ ${file: -3} == ".rs" ]; then
-        cargo +nightly fmt -- --skip-children --write-mode=diff $file &>/dev/null
-				if [ $? != 0 ]; then
-					problem_files+=($file)
-					result=1
-				fi
+		cargo +nightly fmt -- --skip-children --write-mode=diff $file &>/dev/null
+		if [ $? != 0 ]; then
+			problem_files+=($file)
+			result=1
+		fi
 	fi
 done
 
 if [ $result != 0 ]; then
 	printf "\033[0;31mfail\033[0m \n"
-  printf "[pre_commit] the following files need formatting: \n"
+	printf "[pre_commit] the following files need formatting: \n"
 
 	for file in $problem_files; do
 		printf "    cargo +nightly fmt -- $file\n"
 	done
 else
-  printf "\033[0;32mok\033[0m \n"
+	printf "\033[0;32mok\033[0m \n"
 fi
 
 exit 0

api/src/endpoints.rs

@@ -76,6 +76,7 @@ impl ApiEndpoint for OutputApi {
 		let commit = Commitment::from_vec(c);
 
 		let out = self.chain.get_unspent(&commit).map_err(|_| Error::NotFound)?;
+
 		let header = self.chain
 			.get_block_header_by_output_commit(&commit)
 			.map_err(|_| Error::NotFound)?;

api/src/types.rs

@@ -50,7 +50,7 @@ pub struct Output {
 	pub proof: pedersen::RangeProof,
 	/// The height of the block creating this output
 	pub height: u64,
-	/// The lock height (spendable after block)
+	/// The lock height (earliest block this output can be spent)
 	pub lock_height: u64,
 }
 
@@ -65,6 +65,7 @@ impl Output {
 			}
 			_ => (OutputType::Transaction, 0),
 		};
+
 		Output {
 			output_type: output_type,
 			commit: output.commit,

chain/src/pipe.rs

@@ -134,8 +134,10 @@ fn validate_header(header: &BlockHeader, ctx: &mut BlockContext) -> Result<(), E
 
 	// check version, enforces scheduled hard fork
 	if !consensus::valid_header_version(header.height, header.version) {
-		error!("Invalid block header version received ({}), maybe update Grin?",
-			header.version);
+		error!(
+			"Invalid block header version received ({}), maybe update Grin?",
+			header.version
+		);
 		return Err(Error::InvalidBlockVersion(header.version));
 	}
 
@@ -269,8 +271,7 @@ fn validate_block(
 		return Err(Error::InvalidRoot);
 	}
 
-	// check that any coinbase outputs are spendable (that they have matured
-	// sufficiently)
+	// check for any outputs with lock_heights greater than current block height
 	for input in &b.inputs {
 		if let Ok(output) = ctx.store.get_output_by_commit(&input.commitment()) {
 			if output.features.contains(transaction::COINBASE_OUTPUT) {

core/Cargo.toml

@@ -8,6 +8,7 @@ workspace = ".."
 bitflags = "~0.7.0"
 blake2-rfc = "~0.2.17"
 byteorder = "^0.5"
+log = "~0.3"
 num-bigint = "^0.1.35"
 rand = "^0.3"
 serde = "~1.0.8"

core/rustfmt.toml

@@ -1,3 +1,4 @@
 hard_tabs = true
 wrap_comments = true
+comment_width = 120 # we have some long urls in comments
 write_mode = "Overwrite"

core/src/core/block.rs

@@ -39,8 +39,12 @@ pub enum Error {
 	OddKernelFee,
 	/// Too many inputs, outputs or kernels in the block
 	WeightExceeded,
-	/// Underlying Secp256k1 error (signature validation or invalid public
-	/// key typically)
+	/// Kernel not valid due to lock_height exceeding block header height
+	KernelLockHeight {
+		/// The lock_height causing this validation error
+		lock_height: u64,
+	},
+	/// Underlying Secp256k1 error (signature validation or invalid public key typically)
 	Secp(secp::Error),
 }
 
@@ -252,13 +256,15 @@ impl Block {
 	/// Builds a new block from the header of the previous block, a vector of
 	/// transactions and the private key that will receive the reward. Checks
 	/// that all transactions are valid and calculates the Merkle tree.
+	///
+	/// Only used in tests (to be confirmed, may be wrong here).
+	///
 	pub fn new(
 		prev: &BlockHeader,
 		txs: Vec<&Transaction>,
 		keychain: &keychain::Keychain,
 		pubkey: &keychain::Identifier,
 	) -> Result<Block, keychain::Error> {
-
 		let fees = txs.iter().map(|tx| tx.fee).sum();
 		let (reward_out, reward_proof) = Block::reward_output(keychain, pubkey, fees)?;
 		let block = Block::with_reward(prev, txs, reward_out, reward_proof)?;
@@ -415,6 +421,9 @@ impl Block {
 	/// Validates all the elements in a block that can be checked without
 	/// additional data. Includes commitment sums and kernels, Merkle
 	/// trees, reward, etc.
+	///
+	/// TODO - performs various verification steps - discuss renaming this to "verify"
+	///
 	pub fn validate(&self, secp: &Secp256k1) -> Result<(), Error> {
 		if exceeds_weight(self.inputs.len(), self.outputs.len(), self.kernels.len()) {
 			return Err(Error::WeightExceeded);
@@ -424,14 +433,20 @@ impl Block {
 		Ok(())
 	}
 
-	/// Validate the sum of input/output commitments match the sum in kernels
+	/// Verifies the sum of input/output commitments match the sum in kernels
 	/// and that all kernel signatures are valid.
+	/// TODO - when would we skip_sig? Is this needed or used anywhere?
 	fn verify_kernels(&self, secp: &Secp256k1, skip_sig: bool) -> Result<(), Error> {
 		for k in &self.kernels {
 			if k.fee & 1 != 0 {
 				return Err(Error::OddKernelFee);
 			}
+
+			if k.lock_height > self.header.height {
+				return Err(Error::KernelLockHeight { lock_height: k.lock_height });
+			}
 		}
+
 		// sum all inputs and outs commitments
 		let io_sum = self.sum_commitments(secp)?;
 
@@ -483,8 +498,7 @@ impl Block {
 		Ok(())
 	}
 
-	/// Builds the blinded output and related signature proof for the block
-	/// reward.
+	/// Builds the blinded output and related signature proof for the block reward.
 	pub fn reward_output(
 		keychain: &keychain::Keychain,
 		pubkey: &keychain::Identifier,
@@ -515,6 +529,7 @@ impl Block {
 			excess: excess,
 			excess_sig: sig.serialize_der(&secp),
 			fee: 0,
+			lock_height: 0,
 		};
 		Ok((output, proof))
 	}
@@ -553,7 +568,7 @@ mod test {
 		let max_out = MAX_BLOCK_WEIGHT / BLOCK_OUTPUT_WEIGHT;
 
 		let mut pks = vec![];
-		for n in 0..(max_out+1) {
+		for n in 0..(max_out + 1) {
 			pks.push(keychain.derive_pubkey(n as u32).unwrap());
 		}
 
@@ -564,7 +579,9 @@ mod test {
 
 		let now = Instant::now();
 		parts.append(&mut vec![input(500000, pks.pop().unwrap()), with_fee(2)]);
-		let mut tx = build::transaction(parts, &keychain).map(|(tx, _)| tx).unwrap();
+		let mut tx = build::transaction(parts, &keychain)
+			.map(|(tx, _)| tx)
+			.unwrap();
 		println!("Build tx: {}", now.elapsed().as_secs());
 
 		let b = new_block(vec![&mut tx], &keychain);

core/src/core/build.rs

@@ -25,10 +25,10 @@
 //! build::transaction(vec![input_rand(75), output_rand(42), output_rand(32),
 //!   with_fee(1)])
 
-use byteorder::{ByteOrder, BigEndian};
 use secp;
 
 use core::{Transaction, Input, Output, DEFAULT_OUTPUT};
+use core::transaction::kernel_sig_msg;
 use keychain;
 use keychain::{Keychain, BlindSum, BlindingFactor, Identifier};
 
@@ -56,13 +56,19 @@ pub fn output(value: u64, pubkey: Identifier) -> Box<Append> {
 	Box::new(move |build, (tx, sum)| -> (Transaction, BlindSum) {
 		let commit = build.keychain.commit(value, &pubkey).unwrap();
 		let msg = secp::pedersen::ProofMessage::empty();
-		let rproof = build.keychain.range_proof(value, &pubkey, commit, msg).unwrap();
+		let rproof = build
+			.keychain
+			.range_proof(value, &pubkey, commit, msg)
+			.unwrap();
 
-		(tx.with_output(Output {
-			features: DEFAULT_OUTPUT,
-			commit: commit,
-			proof: rproof,
-		}), sum.add_pubkey(pubkey.clone()))
+		(
+			tx.with_output(Output {
+				features: DEFAULT_OUTPUT,
+				commit: commit,
+				proof: rproof,
+			}),
+			sum.add_pubkey(pubkey.clone()),
+		)
 	})
 }
 
@@ -73,6 +79,13 @@ pub fn with_fee(fee: u64) -> Box<Append> {
 	})
 }
 
+/// Sets the lock_height on the transaction being built.
+pub fn with_lock_height(lock_height: u64) -> Box<Append> {
+	Box::new(move |_build, (tx, sum)| -> (Transaction, BlindSum) {
+		(tx.with_lock_height(lock_height), sum)
+	})
+}
+
 /// Sets a known excess value on the transaction being built. Usually used in
 /// combination with the initial_tx function when a new transaction is built
 /// by adding to a pre-existing one.
@@ -95,9 +108,9 @@ pub fn initial_tx(tx: Transaction) -> Box<Append> {
 ///
 /// Example:
 /// let (tx1, sum) = build::transaction(vec![input_rand(4), output_rand(1),
-///   with_fee(1)]).unwrap();
+///   with_fee(1)], keychain).unwrap();
 /// let (tx2, _) = build::transaction(vec![initial_tx(tx1), with_excess(sum),
-///   output_rand(2)]).unwrap();
+///   output_rand(2)], keychain).unwrap();
 ///
 pub fn transaction(
 	elems: Vec<Box<Append>>,
@@ -105,21 +118,16 @@ pub fn transaction(
 ) -> Result<(Transaction, BlindingFactor), keychain::Error> {
 	let mut ctx = Context { keychain };
 	let (mut tx, sum) = elems.iter().fold(
-		(Transaction::empty(), BlindSum::new()), |acc, elem| elem(&mut ctx, acc)
+		(Transaction::empty(), BlindSum::new()),
+		|acc, elem| elem(&mut ctx, acc),
 	);
 	let blind_sum = ctx.keychain.blind_sum(&sum)?;
-	let msg = secp::Message::from_slice(&u64_to_32bytes(tx.fee))?;
+	let msg = secp::Message::from_slice(&kernel_sig_msg(tx.fee, tx.lock_height))?;
 	let sig = ctx.keychain.sign_with_blinding(&msg, &blind_sum)?;
 	tx.excess_sig = sig.serialize_der(&ctx.keychain.secp());
 	Ok((tx, blind_sum))
 }
 
-fn u64_to_32bytes(n: u64) -> [u8; 32] {
-	let mut bytes = [0; 32];
-	BigEndian::write_u64(&mut bytes[24..32], n);
-	bytes
-}
-
 // Just a simple test, most exhaustive tests in the core mod.rs.
 #[cfg(test)]
 mod test {
@@ -146,10 +154,8 @@ mod test {
 		let pk1 = keychain.derive_pubkey(1).unwrap();
 		let pk2 = keychain.derive_pubkey(2).unwrap();
 
-		let (tx, _) = transaction(
-			vec![input(6, pk1), output(2, pk2), with_fee(4)],
-			&keychain,
-		).unwrap();
+		let (tx, _) = transaction(vec![input(6, pk1), output(2, pk2), with_fee(4)], &keychain)
+			.unwrap();
 
 		tx.verify_sig(&keychain.secp()).unwrap();
 	}

core/src/core/mod.rs

@@ -186,7 +186,8 @@ impl Writeable for Proof {
 mod test {
 	use super::*;
 	use core::hash::ZERO_HASH;
-	use core::build::{input, output, with_fee, initial_tx, with_excess};
+	use core::build::{input, output, with_fee, initial_tx, with_excess, with_lock_height};
+	use core::block::Error::KernelLockHeight;
 	use ser;
 	use keychain;
 	use keychain::{Keychain, BlindingFactor};
@@ -209,8 +210,8 @@ mod test {
 		let tx = tx2i1o();
 		let mut vec = Vec::new();
 		ser::serialize(&mut vec, &tx).expect("serialized failed");
-		assert!(vec.len() > 5320);
-		assert!(vec.len() < 5340);
+		assert!(vec.len() > 5340);
+		assert!(vec.len() < 5360);
 	}
 
 	#[test]
@@ -249,11 +250,15 @@ mod test {
 		let pk2 = keychain.derive_pubkey(2).unwrap();
 		let pk3 = keychain.derive_pubkey(3).unwrap();
 
-		let (tx, _) =
-			build::transaction(
-				vec![input(75, pk1), output(42, pk2), output(32, pk3), with_fee(1)],
-				&keychain,
-			).unwrap();
+		let (tx, _) = build::transaction(
+			vec![
+				input(75, pk1),
+				output(42, pk2),
+				output(32, pk3),
+				with_fee(1),
+			],
+			&keychain,
+		).unwrap();
 		let h = tx.outputs[0].hash();
 		assert!(h != ZERO_HASH);
 		let h2 = tx.outputs[1].hash();
@@ -304,10 +309,8 @@ mod test {
 
 			// Alice builds her transaction, with change, which also produces the sum
 			// of blinding factors before they're obscured.
-			let (tx, sum) = build::transaction(
-				vec![in1, in2, output(1, pk3), with_fee(2)],
-				&keychain,
-			).unwrap();
+			let (tx, sum) =
+				build::transaction(vec![in1, in2, output(1, pk3), with_fee(2)], &keychain).unwrap();
 			tx_alice = tx;
 			blind_sum = sum;
 		}
@@ -315,31 +318,26 @@ mod test {
 		// From now on, Bob only has the obscured transaction and the sum of
 		// blinding factors. He adds his output, finalizes the transaction so it's
 		// ready for broadcast.
-		let (tx_final, _) =
-			build::transaction(
-				vec![initial_tx(tx_alice), with_excess(blind_sum), output(4, pk4)],
-				&keychain,
-			).unwrap();
+		let (tx_final, _) = build::transaction(
+			vec![initial_tx(tx_alice), with_excess(blind_sum), output(4, pk4)],
+			&keychain,
+		).unwrap();
 
 		tx_final.validate(&keychain.secp()).unwrap();
 	}
 
 	#[test]
 	fn reward_empty_block() {
-		let keychain = new_keychain();
+		let keychain = keychain::Keychain::from_random_seed().unwrap();
 		let pubkey = keychain.derive_pubkey(1).unwrap();
 
 		let b = Block::new(&BlockHeader::default(), vec![], &keychain, &pubkey).unwrap();
 		b.compact().validate(&keychain.secp()).unwrap();
 	}
 
-	fn new_keychain() -> keychain::Keychain {
-		keychain::Keychain::from_random_seed().unwrap()
-	}
-
 	#[test]
 	fn reward_with_tx_block() {
-		let keychain = new_keychain();
+		let keychain = keychain::Keychain::from_random_seed().unwrap();
 		let pubkey = keychain.derive_pubkey(1).unwrap();
 
 		let mut tx1 = tx2i1o();
@@ -351,36 +349,81 @@ mod test {
 
 	#[test]
 	fn simple_block() {
-		let keychain = new_keychain();
+		let keychain = keychain::Keychain::from_random_seed().unwrap();
 		let pubkey = keychain.derive_pubkey(1).unwrap();
 
 		let mut tx1 = tx2i1o();
-		tx1.verify_sig(keychain.secp()).unwrap();
-
 		let mut tx2 = tx1i1o();
-		tx2.verify_sig(keychain.secp()).unwrap();
 
-		let b = Block::new(&BlockHeader::default(), vec![&mut tx1, &mut tx2], &keychain, &pubkey).unwrap();
+		let b = Block::new(
+			&BlockHeader::default(),
+			vec![&mut tx1, &mut tx2],
+			&keychain,
+			&pubkey,
+		).unwrap();
 		b.validate(keychain.secp()).unwrap();
 	}
 
+	#[test]
+	fn test_block_with_timelocked_tx() {
+		let keychain = keychain::Keychain::from_random_seed().unwrap();
+
+		let pk1 = keychain.derive_pubkey(1).unwrap();
+		let pk2 = keychain.derive_pubkey(2).unwrap();
+		let pk3 = keychain.derive_pubkey(3).unwrap();
+
+		// first check we can add a timelocked tx where lock height matches current block height
+		// and that the resulting block is valid
+		let tx1 = build::transaction(
+			vec![input(5, pk1.clone()), output(3, pk2.clone()), with_fee(2), with_lock_height(1)],
+			&keychain,
+		).map(|(tx, _)| tx).unwrap();
+
+		let b = Block::new(
+			&BlockHeader::default(),
+			vec![&tx1],
+			&keychain,
+			&pk3.clone(),
+		).unwrap();
+		b.validate(keychain.secp()).unwrap();
+
+		// now try adding a timelocked tx where lock height is greater than current block height
+		let tx1 = build::transaction(
+			vec![input(5, pk1.clone()), output(3, pk2.clone()), with_fee(2), with_lock_height(2)],
+			&keychain,
+		).map(|(tx, _)| tx).unwrap();
+
+		let b = Block::new(
+			&BlockHeader::default(),
+			vec![&tx1],
+			&keychain,
+			&pk3.clone(),
+		).unwrap();
+		match b.validate(keychain.secp()) {
+			Err(KernelLockHeight{ lock_height: height}) => {
+				assert_eq!(height, 2);
+			},
+			_ => panic!("expecting KernelLockHeight error here"),
+		}
+	}
+
 	#[test]
 	pub fn test_verify_1i1o_sig() {
-		let keychain = new_keychain();
+		let keychain = keychain::Keychain::from_random_seed().unwrap();
 		let tx = tx1i1o();
 		tx.verify_sig(keychain.secp()).unwrap();
 	}
 
 	#[test]
 	pub fn test_verify_2i1o_sig() {
-		let keychain = new_keychain();
+		let keychain = keychain::Keychain::from_random_seed().unwrap();
 		let tx = tx2i1o();
 		tx.verify_sig(keychain.secp()).unwrap();
 	}
 
 	// utility producing a transaction with 2 inputs and a single outputs
 	pub fn tx2i1o() -> Transaction {
-		let keychain = new_keychain();
+		let keychain = keychain::Keychain::from_random_seed().unwrap();
 		let pk1 = keychain.derive_pubkey(1).unwrap();
 		let pk2 = keychain.derive_pubkey(2).unwrap();
 		let pk3 = keychain.derive_pubkey(3).unwrap();
@@ -388,18 +431,18 @@ mod test {
 		build::transaction(
 			vec![input(10, pk1), input(11, pk2), output(19, pk3), with_fee(2)],
 			&keychain,
-		).map(|(tx, _)| tx).unwrap()
+		).map(|(tx, _)| tx)
+			.unwrap()
 	}
 
 	// utility producing a transaction with a single input and output
 	pub fn tx1i1o() -> Transaction {
-		let keychain = new_keychain();
+		let keychain = keychain::Keychain::from_random_seed().unwrap();
 		let pk1 = keychain.derive_pubkey(1).unwrap();
 		let pk2 = keychain.derive_pubkey(2).unwrap();
 
-		build::transaction(
-			vec![input(5, pk1), output(3, pk2), with_fee(2)],
-			&keychain,
-		).map(|(tx, _)| tx).unwrap()
+		build::transaction(vec![input(5, pk1), output(3, pk2), with_fee(2)], &keychain)
+			.map(|(tx, _)| tx)
+			.unwrap()
 	}
 }

core/src/core/pmmr.rs

@@ -1,4 +1,4 @@
-// Copyright 2016 The Grin Developers
+// Copyright 2017 The Grin Developers
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -15,11 +15,7 @@
 //! Persistent and prunable Merkle Mountain Range implementation. For a high
 //! level description of MMRs, see:
 //!
-//! https://github.
-//! com/opentimestamps/opentimestamps-server/blob/master/doc/merkle-mountain-range.
-//!
-//!
-//! md
+//! https://github.com/opentimestamps/opentimestamps-server/blob/master/doc/merkle-mountain-range.md
 //!
 //! This implementation is built in two major parts:
 //!
@@ -648,7 +644,9 @@ fn peaks(num: u64) -> Vec<u64> {
 /// any node, from its postorder traversal position. Which is the order in which
 /// nodes are added in a MMR.
 ///
-/// [1]  https://github.com/opentimestamps/opentimestamps-server/blob/master/doc/merkle-mountain-range.md
+/// [1]  https://github.
+/// com/opentimestamps/opentimestamps-server/blob/master/doc/merkle-mountain-range.
+/// md
 pub fn bintree_postorder_height(num: u64) -> u64 {
 	let mut h = num;
 	while !all_ones(h) {

core/src/core/transaction.rs

@@ -25,13 +25,13 @@ use keychain::{Identifier, Keychain};
 use ser::{self, Reader, Writer, Readable, Writeable};
 
 bitflags! {
-    /// Options for a kernel's structure or use
-    pub flags KernelFeatures: u8 {
-        /// No flags
-        const DEFAULT_KERNEL = 0b00000000,
-        /// Kernel matching a coinbase output
-        const COINBASE_KERNEL = 0b00000001,
-    }
+	/// Options for a kernel's structure or use
+	pub flags KernelFeatures: u8 {
+		/// No flags
+		const DEFAULT_KERNEL = 0b00000000,
+		/// Kernel matching a coinbase output
+		const COINBASE_KERNEL = 0b00000001,
+	}
 }
 
 /// Errors thrown by Block validation
@@ -50,14 +50,28 @@ impl From<secp::Error> for Error {
 	}
 }
 
+/// Construct msg bytes from tx fee and lock_height
+pub fn kernel_sig_msg(fee: u64, lock_height: u64) -> [u8; 32] {
+	let mut bytes = [0; 32];
+	BigEndian::write_u64(&mut bytes[16..24], fee);
+	BigEndian::write_u64(&mut bytes[24..], lock_height);
+	bytes
+}
+
 /// A proof that a transaction sums to zero. Includes both the transaction's
 /// Pedersen commitment and the signature, that guarantees that the commitments
-/// amount to zero. The signature signs the fee, which is retained for
+/// amount to zero.
+/// The signature signs the fee and the lock_height, which are retained for
 /// signature validation.
 #[derive(Debug, Clone, PartialEq)]
 pub struct TxKernel {
 	/// Options for a kernel's structure or use
 	pub features: KernelFeatures,
+	/// Fee originally included in the transaction this proof is for.
+	pub fee: u64,
+	/// This kernel is not valid earlier than lock_height blocks
+	/// The max lock_height of all *inputs* to this transaction
+	pub lock_height: u64,
 	/// Remainder of the sum of all transaction commitments. If the transaction
 	/// is well formed, amounts components should sum to zero and the excess
 	/// is hence a valid public key.
@@ -65,8 +79,6 @@ pub struct TxKernel {
 	/// The signature proving the excess is a valid public key, which signs
 	/// the transaction fee.
 	pub excess_sig: Vec<u8>,
-	/// Fee originally included in the transaction this proof is for.
-	pub fee: u64,
 }
 
 impl Writeable for TxKernel {
@@ -74,9 +86,10 @@ impl Writeable for TxKernel {
 		ser_multiwrite!(
 			writer,
 			[write_u8, self.features.bits()],
+			[write_u64, self.fee],
+			[write_u64, self.lock_height],
 			[write_fixed_bytes, &self.excess],
-			[write_bytes, &self.excess_sig],
-			[write_u64, self.fee]
+			[write_bytes, &self.excess_sig]
 		);
 		Ok(())
 	}
@@ -84,13 +97,16 @@ impl Writeable for TxKernel {
 
 impl Readable for TxKernel {
 	fn read(reader: &mut Reader) -> Result<TxKernel, ser::Error> {
+		let features = KernelFeatures::from_bits(reader.read_u8()?).ok_or(
+			ser::Error::CorruptedData,
+		)?;
+
 		Ok(TxKernel {
-			features: KernelFeatures::from_bits(reader.read_u8()?).ok_or(
-				ser::Error::CorruptedData,
-			)?,
+			features: features,
+			fee: reader.read_u64()?,
+			lock_height: reader.read_u64()?,
 			excess: Commitment::read(reader)?,
 			excess_sig: reader.read_vec()?,
-			fee: reader.read_u64()?,
 		})
 	}
 }
@@ -100,7 +116,9 @@ impl TxKernel {
 	/// as a public key and checking the signature verifies with the fee as
 	/// message.
 	pub fn verify(&self, secp: &Secp256k1) -> Result<(), secp::Error> {
-		let msg = try!(Message::from_slice(&u64_to_32bytes(self.fee)));
+		let msg = try!(Message::from_slice(
+			&kernel_sig_msg(self.fee, self.lock_height),
+		));
 		let sig = try!(Signature::from_der(secp, &self.excess_sig));
 		secp.verify_from_commit(&msg, &sig, &self.excess)
 	}
@@ -115,6 +133,9 @@ pub struct Transaction {
 	pub outputs: Vec<Output>,
 	/// Fee paid by the transaction.
 	pub fee: u64,
+	/// Transaction is not valid before this block height.
+	/// It is invalid for this to be less than the lock_height of any UTXO being spent.
+	pub lock_height: u64,
 	/// The signature proving the excess is a valid public key, which signs
 	/// the transaction fee.
 	pub excess_sig: Vec<u8>,
@@ -127,6 +148,7 @@ impl Writeable for Transaction {
 		ser_multiwrite!(
 			writer,
 			[write_u64, self.fee],
+			[write_u64, self.lock_height],
 			[write_bytes, &self.excess_sig],
 			[write_u64, self.inputs.len() as u64],
 			[write_u64, self.outputs.len() as u64]
@@ -145,14 +167,15 @@ impl Writeable for Transaction {
 /// transaction from a binary stream.
 impl Readable for Transaction {
 	fn read(reader: &mut Reader) -> Result<Transaction, ser::Error> {
-		let (fee, excess_sig, input_len, output_len) =
-			ser_multiread!(reader, read_u64, read_vec, read_u64, read_u64);
+		let (fee, lock_height, excess_sig, input_len, output_len) =
+			ser_multiread!(reader, read_u64, read_u64, read_vec, read_u64, read_u64);
 
 		let inputs = try!((0..input_len).map(|_| Input::read(reader)).collect());
 		let outputs = try!((0..output_len).map(|_| Output::read(reader)).collect());
 
 		Ok(Transaction {
 			fee: fee,
+			lock_height: lock_height,
 			excess_sig: excess_sig,
 			inputs: inputs,
 			outputs: outputs,
@@ -185,17 +208,24 @@ impl Transaction {
 	pub fn empty() -> Transaction {
 		Transaction {
 			fee: 0,
+			lock_height: 0,
 			excess_sig: vec![],
 			inputs: vec![],
 			outputs: vec![],
 		}
 	}
 
-	/// Creates a new transaction initialized with the provided inputs,
-	/// outputs and fee.
-	pub fn new(inputs: Vec<Input>, outputs: Vec<Output>, fee: u64) -> Transaction {
+	/// Creates a new transaction initialized with
+	/// the provided inputs, outputs, fee and lock_height.
+	pub fn new(
+		inputs: Vec<Input>,
+		outputs: Vec<Output>,
+		fee: u64,
+		lock_height: u64,
+	) -> Transaction {
 		Transaction {
 			fee: fee,
+			lock_height: lock_height,
 			excess_sig: vec![],
 			inputs: inputs,
 			outputs: outputs,
@@ -229,6 +259,14 @@ impl Transaction {
 		Transaction { fee: fee, ..self }
 	}
 
+	/// Builds a new transaction with the provided lock_height.
+	pub fn with_lock_height(self, lock_height: u64) -> Transaction {
+		Transaction {
+			lock_height: lock_height,
+			..self
+		}
+	}
+
 	/// The verification for a MimbleWimble transaction involves getting the
 	/// excess of summing all commitments and using it as a public key
 	/// to verify the embedded signature. The rational is that if the values
@@ -238,7 +276,7 @@ impl Transaction {
 	pub fn verify_sig(&self, secp: &Secp256k1) -> Result<TxKernel, secp::Error> {
 		let rsum = self.sum_commitments(secp)?;
 
-		let msg = Message::from_slice(&u64_to_32bytes(self.fee))?;
+		let msg = Message::from_slice(&kernel_sig_msg(self.fee, self.lock_height))?;
 		let sig = Signature::from_der(secp, &self.excess_sig)?;
 
 		// pretend the sum is a public key (which it is, being of the form r.G) and
@@ -250,12 +288,20 @@ impl Transaction {
 		// of generating a public key from a commitment behind verify_from_commit
 		secp.verify_from_commit(&msg, &sig, &rsum)?;
 
-		Ok(TxKernel {
+		let kernel = TxKernel {
 			features: DEFAULT_KERNEL,
 			excess: rsum,
 			excess_sig: self.excess_sig.clone(),
 			fee: self.fee,
-		})
+			lock_height: self.lock_height,
+		};
+		debug!(
+			"tx verify_sig: fee - {}, lock_height - {}",
+			kernel.fee,
+			kernel.lock_height
+		);
+
+		Ok(kernel)
 	}
 
 	/// Validates all relevant parts of a fully built transaction. Checks the
@@ -303,14 +349,14 @@ impl Input {
 }
 
 bitflags! {
-    /// Options for block validation
-    #[derive(Serialize, Deserialize)]
-    pub flags OutputFeatures: u8 {
-        /// No flags
-        const DEFAULT_OUTPUT = 0b00000000,
-        /// Output is a coinbase output, has fixed amount and must not be spent until maturity
-        const COINBASE_OUTPUT = 0b00000001,
-    }
+	/// Options for block validation
+	#[derive(Serialize, Deserialize)]
+	pub flags OutputFeatures: u8 {
+		/// No flags
+		const DEFAULT_OUTPUT = 0b00000000,
+		/// Output is a coinbase output, must not be spent until maturity
+		const COINBASE_OUTPUT = 0b00000001,
+	}
 }
 
 /// Output for a transaction, defining the new ownership of coins that are being
@@ -318,9 +364,8 @@ bitflags! {
 /// range proof guarantees the commitment includes a positive value without
 /// overflow and the ownership of the private key.
 ///
-/// The hash of an output only covers its features and commitment. The range
-/// proof is expected to have its own hash and is stored and committed to
-/// separately.
+/// The hash of an output only covers its features, lock_height and commitment.
+/// The range proof is expected to have its own hash and is stored and committed to separately.
 #[derive(Debug, Copy, Clone, PartialEq, Serialize, Deserialize)]
 pub struct Output {
 	/// Options for an output's structure or use
@@ -335,11 +380,9 @@ pub struct Output {
 /// an Output as binary.
 impl Writeable for Output {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
-		ser_multiwrite!(
-			writer,
-			[write_u8, self.features.bits()],
-			[write_fixed_bytes, &self.commit]
-		);
+		writer.write_u8(self.features.bits())?;
+		writer.write_fixed_bytes(&self.commit)?;
+
 		// The hash of an output doesn't include the range proof
 		if writer.serialization_mode() == ser::SerializationMode::Full {
 			writer.write_bytes(&self.proof)?
@@ -352,10 +395,12 @@ impl Writeable for Output {
 /// an Output from a binary stream.
 impl Readable for Output {
 	fn read(reader: &mut Reader) -> Result<Output, ser::Error> {
+		let features = OutputFeatures::from_bits(reader.read_u8()?).ok_or(
+			ser::Error::CorruptedData,
+		)?;
+
 		Ok(Output {
-			features: OutputFeatures::from_bits(reader.read_u8()?).ok_or(
-				ser::Error::CorruptedData,
-			)?,
+			features: features,
 			commit: Commitment::read(reader)?,
 			proof: RangeProof::read(reader)?,
 		})
@@ -388,8 +433,8 @@ impl Output {
 				} else {
 					None
 				}
-			},
-			Err(_) => None
+			}
+			Err(_) => None,
 		}
 	}
 }
@@ -453,18 +498,80 @@ impl ops::Add for SumCommit {
 	}
 }
 
-fn u64_to_32bytes(n: u64) -> [u8; 32] {
-	let mut bytes = [0; 32];
-	BigEndian::write_u64(&mut bytes[24..32], n);
-	bytes
-}
-
 #[cfg(test)]
 mod test {
 	use super::*;
 	use keychain::Keychain;
 	use secp;
 
+	#[test]
+	fn test_kernel_ser_deser() {
+		let keychain = Keychain::from_random_seed().unwrap();
+		let pubkey = keychain.derive_pubkey(1).unwrap();
+		let commit = keychain.commit(5, &pubkey).unwrap();
+
+		// just some bytes for testing ser/deser
+		let sig = vec![1, 0, 0, 0, 0, 0, 0, 1];
+
+		let kernel = TxKernel {
+			features: DEFAULT_KERNEL,
+			lock_height: 0,
+			excess: commit,
+			excess_sig: sig.clone(),
+			fee: 10,
+		};
+
+		let mut vec = vec![];
+		ser::serialize(&mut vec, &kernel).expect("serialized failed");
+		let kernel2: TxKernel = ser::deserialize(&mut &vec[..]).unwrap();
+		assert_eq!(kernel2.features, DEFAULT_KERNEL);
+		assert_eq!(kernel2.lock_height, 0);
+		assert_eq!(kernel2.excess, commit);
+		assert_eq!(kernel2.excess_sig, sig.clone());
+		assert_eq!(kernel2.fee, 10);
+
+		// now check a kernel with lock_height serializes/deserializes correctly
+		let kernel = TxKernel {
+			features: DEFAULT_KERNEL,
+			lock_height: 100,
+			excess: commit,
+			excess_sig: sig.clone(),
+			fee: 10,
+		};
+
+		let mut vec = vec![];
+		ser::serialize(&mut vec, &kernel).expect("serialized failed");
+		let kernel2: TxKernel = ser::deserialize(&mut &vec[..]).unwrap();
+		assert_eq!(kernel2.features, DEFAULT_KERNEL);
+		assert_eq!(kernel2.lock_height, 100);
+		assert_eq!(kernel2.excess, commit);
+		assert_eq!(kernel2.excess_sig, sig.clone());
+		assert_eq!(kernel2.fee, 10);
+	}
+
+	#[test]
+	fn test_output_ser_deser() {
+		let keychain = Keychain::from_random_seed().unwrap();
+		let pubkey = keychain.derive_pubkey(1).unwrap();
+		let commit = keychain.commit(5, &pubkey).unwrap();
+		let msg = secp::pedersen::ProofMessage::empty();
+		let proof = keychain.range_proof(5, &pubkey, commit, msg).unwrap();
+
+		let out = Output {
+			features: DEFAULT_OUTPUT,
+			commit: commit,
+			proof: proof,
+		};
+
+		let mut vec = vec![];
+		ser::serialize(&mut vec, &out).expect("serialized failed");
+		let dout: Output = ser::deserialize(&mut &vec[..]).unwrap();
+
+		assert_eq!(dout.features, DEFAULT_OUTPUT);
+		assert_eq!(dout.commit, out.commit);
+		assert_eq!(dout.proof, out.proof);
+	}
+
 	#[test]
 	fn test_output_value_recovery() {
 		let keychain = Keychain::from_random_seed().unwrap();

core/src/lib.rs

@@ -25,6 +25,8 @@
 extern crate bitflags;
 extern crate blake2_rfc as blake2;
 extern crate byteorder;
+#[macro_use]
+extern crate log;
 extern crate num_bigint as bigint;
 extern crate rand;
 extern crate secp256k1zkp as secp;

grin/src/miner.rs

@@ -537,10 +537,8 @@ impl Miner {
 
 		// build the coinbase and the block itself
 		let fees = txs.iter().map(|tx| tx.fee).sum();
-		let block_fees = BlockFees {
-			fees: fees,
-			pubkey: pubkey,
-		};
+		let height = head.height + 1;
+		let block_fees = BlockFees { fees, pubkey, height };
 
 		let (output, kernel, block_fees) = self.get_coinbase(block_fees);
 		let mut b = core::Block::with_reward(head, txs, output, kernel).unwrap();
@@ -576,7 +574,7 @@ impl Miner {
 			let (out, kern) = core::Block::reward_output(
 				&keychain,
 				&pubkey,
-				block_fees.fees
+				block_fees.fees,
 			).unwrap();
 			(out, kern, block_fees)
 		} else {

pool/src/graph.rs

@@ -271,7 +271,7 @@ mod tests {
 				proof: keychain.range_proof(100, &pk1, output_commit, msg).unwrap(),
 			},
 		];
-		let test_transaction = core::transaction::Transaction::new(inputs, outputs, 5);
+		let test_transaction = core::transaction::Transaction::new(inputs, outputs, 5, 0);
 
 		let test_pool_entry = PoolEntry::new(&test_transaction);
 

pool/src/pool.rs

@@ -153,6 +153,13 @@ where
 			return Err(PoolError::AlreadyInPool);
 		}
 
+		let head_header = self.blockchain.head_header()?;
+		if head_header.height < tx.lock_height {
+			return Err(PoolError::ImmatureTransaction {
+				lock_height: tx.lock_height,
+			});
+		}
+
 		// The next issue is to identify all unspent outputs that
 		// this transaction will consume and make sure they exist in the set.
 		let mut pool_refs: Vec<graph::Edge> = Vec::new();
@@ -174,15 +181,12 @@ where
 						if let Ok(out_header) = self.blockchain
 							.get_block_header_by_output_commit(&output.commitment())
 						{
-							if let Ok(head_header) = self.blockchain.head_header() {
-								if head_header.height <=
-									out_header.height + global::coinbase_maturity()
-								{
-									return Err(PoolError::ImmatureCoinbase {
-										header: out_header,
-										output: output.commitment(),
-									});
-								};
+							let lock_height = out_header.height + global::coinbase_maturity();
+							if head_header.height < lock_height {
+								return Err(PoolError::ImmatureCoinbase {
+									header: out_header,
+									output: output.commitment(),
+								});
 							};
 						};
 					};
@@ -549,6 +553,10 @@ where
 	/// Fetch mineable transactions.
 	///
 	/// Select a set of mineable transactions for block building.
+	///
+	/// TODO - txs have lock_heights, so possible to have "invalid" (immature)
+	/// txs here?
+	///
 	pub fn prepare_mineable_transactions(
 		&self,
 		num_to_fetch: u32,
@@ -589,7 +597,6 @@ where
 #[cfg(test)]
 mod tests {
 	use super::*;
-	use types::*;
 	use core::core::build;
 	use blockchain::{DummyChain, DummyChainImpl, DummyUtxoSet};
 	use secp;
@@ -617,6 +624,11 @@ mod tests {
 	/// A basic test; add a pair of transactions to the pool.
 	fn test_basic_pool_add() {
 		let mut dummy_chain = DummyChainImpl::new();
+		let head_header = block::BlockHeader {
+			height: 1,
+			..block::BlockHeader::default()
+		};
+		dummy_chain.store_head_header(&head_header);
 
 		let parent_transaction = test_transaction(vec![5, 6, 7], vec![11, 3]);
 		// We want this transaction to be rooted in the blockchain.
@@ -674,6 +686,11 @@ mod tests {
 	/// Testing various expected error conditions
 	pub fn test_pool_add_error() {
 		let mut dummy_chain = DummyChainImpl::new();
+		let head_header = block::BlockHeader {
+			height: 1,
+			..block::BlockHeader::default()
+		};
+		dummy_chain.store_head_header(&head_header);
 
 		let new_utxo = DummyUtxoSet::empty()
 			.with_output(test_output(5))
@@ -757,6 +774,17 @@ mod tests {
 			};
 
 			assert_eq!(write_pool.total_size(), 1);
+
+			// now attempt to add a timelocked tx to the pool
+			// should fail as invalid based on current height
+			let timelocked_tx_1 = timelocked_transaction(vec![9], vec![5], 10);
+			match write_pool.add_to_memory_pool(test_source(), timelocked_tx_1) {
+				Err(PoolError::ImmatureTransaction { lock_height: height }) => {
+					assert_eq!(height, 10);
+				}
+				Err(e) => panic!("expected ImmatureTransaction error here - {:?}", e),
+				Ok(_) => panic!("expected ImmatureTransaction error here"),
+			};
 		}
 	}
 
@@ -801,7 +829,7 @@ mod tests {
 			};
 
 			let head_header = block::BlockHeader {
-				height: 4,
+				height: 3,
 				..block::BlockHeader::default()
 			};
 			chain_ref.store_head_header(&head_header);
@@ -843,6 +871,11 @@ mod tests {
 	/// Testing block reconciliation
 	fn test_block_reconciliation() {
 		let mut dummy_chain = DummyChainImpl::new();
+		let head_header = block::BlockHeader {
+			height: 1,
+			..block::BlockHeader::default()
+		};
+		dummy_chain.store_head_header(&head_header);
 
 		let new_utxo = DummyUtxoSet::empty()
 			.with_output(test_output(10))
@@ -992,6 +1025,11 @@ mod tests {
 	fn test_block_building() {
 		// Add a handful of transactions
 		let mut dummy_chain = DummyChainImpl::new();
+		let head_header = block::BlockHeader {
+			height: 1,
+			..block::BlockHeader::default()
+		};
+		dummy_chain.store_head_header(&head_header);
 
 		let new_utxo = DummyUtxoSet::empty()
 			.with_output(test_output(10))
@@ -1005,7 +1043,7 @@ mod tests {
 
 		let pool = RwLock::new(test_setup(&chain_ref));
 
-		let root_tx_1 = test_transaction(vec![10,20], vec![24]);
+		let root_tx_1 = test_transaction(vec![10, 20], vec![24]);
 		let root_tx_2 = test_transaction(vec![30], vec![28]);
 		let root_tx_3 = test_transaction(vec![40], vec![38]);
 
@@ -1106,6 +1144,35 @@ mod tests {
 		tx
 	}
 
+	fn timelocked_transaction(
+		input_values: Vec<u64>,
+		output_values: Vec<u64>,
+		lock_height: u64,
+	) -> transaction::Transaction {
+		let keychain = keychain_for_tests();
+
+		let fees: i64 = input_values.iter().sum::<u64>() as i64 -
+			output_values.iter().sum::<u64>() as i64;
+		assert!(fees >= 0);
+
+		let mut tx_elements = Vec::new();
+
+		for input_value in input_values {
+			let pubkey = keychain.derive_pubkey(input_value as u32).unwrap();
+			tx_elements.push(build::input(input_value, pubkey));
+		}
+
+		for output_value in output_values {
+			let pubkey = keychain.derive_pubkey(output_value as u32).unwrap();
+			tx_elements.push(build::output(output_value, pubkey));
+		}
+		tx_elements.push(build::with_fee(fees as u64));
+
+		tx_elements.push(build::with_lock_height(lock_height));
+		let (tx, _) = build::transaction(tx_elements, &keychain).unwrap();
+		tx
+	}
+
 	/// Deterministically generate an output defined by our test scheme
 	fn test_output(value: u64) -> transaction::Output {
 		let keychain = keychain_for_tests();

pool/src/types.rs

@@ -115,13 +115,20 @@ pub enum PoolError {
 		/// The spent output
 		spent_output: Commitment,
 	},
-	/// Attempt to spend a coinbase output before it matures (1000 blocks?)
+	/// Attempt to spend an output before it matures
+	/// lock_height must not exceed current block height
 	ImmatureCoinbase {
-		/// The block header of the block containing the coinbase output
+		/// The block header of the block containing the output
 		header: block::BlockHeader,
-		/// The unspent coinbase output
+		/// The unspent output
 		output: Commitment,
 	},
+	/// Attempt to add a transaction to the pool with lock_height
+	/// greater than height of current block
+	ImmatureTransaction {
+		/// The lock height of the invalid transaction
+		lock_height: u64,
+	},
 	/// An orphan successfully added to the orphans set
 	OrphanTransaction,
 	/// TODO - wip, just getting imports working, remove this and use more

wallet/src/checker.rs

@@ -28,7 +28,7 @@ fn refresh_output(out: &mut OutputData, api_out: Option<api::Output>, tip: &api:
 
 		if out.status == OutputStatus::Locked {
 			// leave it Locked locally for now
-		} else if api_out.lock_height >= tip.height {
+		} else if api_out.lock_height > tip.height {
 			out.status = OutputStatus::Immature;
 		} else {
 			out.status = OutputStatus::Unspent;
@@ -67,7 +67,7 @@ pub fn refresh_outputs(
 	})
 }
 
-fn get_tip_from_node(config: &WalletConfig) -> Result<api::Tip, Error> {
+pub fn get_tip_from_node(config: &WalletConfig) -> Result<api::Tip, Error> {
 	let url = format!("{}/v1/chain/1", config.check_node_api_http_addr);
 	api::client::get::<api::Tip>(url.as_str()).map_err(|e| Error::Node(e))
 }
@@ -79,6 +79,7 @@ fn get_output_from_node(
 	amount: u64,
 	derivation: u32,
 ) -> Result<Option<api::Output>, Error> {
+	// do we want to store these commitments in wallet.dat?
 	let pubkey = keychain.derive_pubkey(derivation)?;
 	let commit = keychain.commit(amount, &pubkey)?;
 

wallet/src/receiver.rs

@@ -213,7 +213,11 @@ fn receive_coinbase(
 
 		debug!("block_fees updated - {:?}", block_fees);
 
-		let (out, kern) = Block::reward_output(&keychain, &pubkey, block_fees.fees)?;
+		let (out, kern) = Block::reward_output(
+			&keychain,
+			&pubkey,
+			block_fees.fees,
+		)?;
 		Ok((out, kern, block_fees))
 	})?
 }
@@ -226,7 +230,6 @@ fn receive_transaction(
 	blinding: BlindingFactor,
 	partial: Transaction,
 ) -> Result<Transaction, Error> {
-
 	let fingerprint = keychain.clone().fingerprint();
 
 	// operate within a lock on wallet data
@@ -263,7 +266,7 @@ fn receive_transaction(
 			height: 0,
 			lock_height: 0,
 		});
-		debug!("Received txn and built output  - {}, {}, {}",
+		debug!("Received txn and built output - {}, {}, {}",
 			fingerprint.clone(), pubkey.fingerprint(), derivation);
 
 		Ok(tx_final)

wallet/src/sender.rs

@@ -28,9 +28,12 @@ pub fn issue_send_tx(
 	amount: u64,
 	dest: String,
 ) -> Result<(), Error> {
-	let _ = checker::refresh_outputs(config, keychain);
+	checker::refresh_outputs(config, keychain)?;
 
-	let (tx, blind_sum) = build_send_tx(config, keychain, amount)?;
+	let chain_tip = checker::get_tip_from_node(config)?;
+	let lock_height = chain_tip.height;
+
+	let (tx, blind_sum) = build_send_tx(config, keychain, amount, lock_height)?;
 	let json_tx = partial_tx_to_json(amount, blind_sum, tx);
 
 	if dest == "stdout" {
@@ -54,6 +57,7 @@ fn build_send_tx(
 	config: &WalletConfig,
 	keychain: &Keychain,
 	amount: u64,
+	lock_height: u64,
 ) -> Result<(Transaction, BlindingFactor), Error> {
 	let fingerprint = keychain.clone().fingerprint();
 
@@ -66,10 +70,16 @@ fn build_send_tx(
 			return Err(Error::NotEnoughFunds((-change) as u64));
 		}
 
-		// TODO add fees, which is likely going to make this iterative
-
 		// build inputs using the appropriate derived pubkeys
 		let mut parts = vec![];
+
+		// This is more proof of concept than anything but here we set a
+		// lock_height on the transaction being sent (based on current chain height via api).
+		parts.push(build::with_lock_height(lock_height));
+
+		// TODO add fees, which is likely going to make this iterative
+		// parts.push(build::with_fees(100));
+
 		for coin in &coins {
 			let pubkey = keychain.derive_pubkey(coin.n_child)?;
 			parts.push(build::input(coin.value, pubkey));

wallet/src/types.rs

@@ -385,6 +385,7 @@ pub enum WalletReceiveRequest {
 #[derive(Serialize, Deserialize, Debug, Clone)]
 pub struct BlockFees {
 	pub fees: u64,
+	pub height: u64,
 	pub pubkey: Option<keychain::Identifier>,
 }