build input vs coinbase_input

switch commit hash key encodes lock_height
cleanup output by commit index (currently broken...)

chain/src/chain.rs

@@ -336,6 +336,14 @@ impl Chain {
 	/// way that's consistent with the current chain state and more
 	/// specifically the current winning fork.
 	pub fn get_unspent(&self, output_ref: &Commitment) -> Result<Output, Error> {
+		// TODO - get output_pos from index
+		// TODO - lookup output in output_pmmr
+		// TODO - do something with it...
+
+		// TODO - what do we return here if we have no outputs in the index???
+
+		panic!("not yet implemented...");
+
 		match self.store.get_output_by_commit(output_ref) {
 			Ok(out) => {
 				let mut sumtrees = self.sumtrees.write().unwrap();

chain/src/pipe.rs

@@ -312,6 +312,11 @@ fn validate_block(
 	}
 
 	for input in &b.inputs {
+		// TODO - get output_pos from index
+		// TODO - lookup switch_commit_hash from output_pmmr
+		// TODO - do something with the switch_commit_hash
+		panic!("not yet implemented... working on it");
+
 		if let Ok(output) = ctx.store.get_output_by_commit(&input.commitment()) {
 			// check the lock_height of the output being spent by this input
 			// is not greater than the current height

chain/src/store.rs

@@ -33,7 +33,7 @@ const HEAD_PREFIX: u8 = 'H' as u8;
 const HEADER_HEAD_PREFIX: u8 = 'I' as u8;
 const SYNC_HEAD_PREFIX: u8 = 's' as u8;
 const HEADER_HEIGHT_PREFIX: u8 = '8' as u8;
-const OUTPUT_COMMIT_PREFIX: u8 = 'o' as u8;
+// const OUTPUT_COMMIT_PREFIX: u8 = 'o' as u8;
 const COMMIT_POS_PREFIX: u8 = 'c' as u8;
 const KERNEL_POS_PREFIX: u8 = 'k' as u8;
 
@@ -115,14 +115,15 @@ impl ChainStore for ChainKVStore {
 				&b.header,
 			)?;
 
-		// saving the full output under its commitment
-		for out in &b.outputs {
-			batch = batch
-				.put_ser(
-					&to_key(OUTPUT_COMMIT_PREFIX, &mut out.commitment().as_ref().to_vec())[..],
-					out,
-				)?;
-		}
+		// // saving the full output under its commitment
+		// for out in &b.outputs {
+		// 	batch = batch
+		// 		.put_ser(
+		// 			&to_key(OUTPUT_COMMIT_PREFIX, &mut out.commitment().as_ref().to_vec())[..],
+		// 			out,
+		// 		)?;
+		// }
+
 		batch.write()
 	}
 
@@ -150,17 +151,17 @@ impl ChainStore for ChainKVStore {
 		self.db.delete(&u64_to_key(HEADER_HEIGHT_PREFIX, height))
 	}
 
-	fn get_output_by_commit(&self, commit: &Commitment) -> Result<Output, Error> {
-		// TODO - how to handle multiple "versions" of an output (multiple forks)
-		// * retrieve list of outputs from the db
-		// * then identify which one is actually valid based on what?
-		// * suspect we need to encode height in the output (based on the block itself?)
-		// * then check the "block at height" for consistency?
-		option_to_not_found(
-			self.db
-				.get_ser(&to_key(OUTPUT_COMMIT_PREFIX, &mut commit.as_ref().to_vec())),
-		)
-	}
+	// fn get_output_by_commit(&self, commit: &Commitment) -> Result<Output, Error> {
+	// 	// TODO - how to handle multiple "versions" of an output (multiple forks)
+	// 	// * retrieve list of outputs from the db
+	// 	// * then identify which one is actually valid based on what?
+	// 	// * suspect we need to encode height in the output (based on the block itself?)
+	// 	// * then check the "block at height" for consistency?
+	// 	option_to_not_found(
+	// 		self.db
+	// 			.get_ser(&to_key(OUTPUT_COMMIT_PREFIX, &mut commit.as_ref().to_vec())),
+	// 	)
+	// }
 
 	fn save_output_pos(&self, commit: &Commitment, pos: u64) -> Result<(), Error> {
 		self.db.put_ser(

chain/src/types.rs

@@ -244,7 +244,7 @@ pub trait ChainStore: Send + Sync {
 	fn is_on_current_chain(&self, header: &BlockHeader) -> Result<(), store::Error>;
 
 	/// Gets an output by its commitment
-	fn get_output_by_commit(&self, commit: &Commitment) -> Result<Output, store::Error>;
+	// fn get_output_by_commit(&self, commit: &Commitment) -> Result<Output, store::Error>;
 
 	/// Saves the position of an output, represented by its commitment, in the
 	/// UTXO MMR. Used as an index for spending and pruning.

chain/tests/test_coinbase_maturity.rs

@@ -92,7 +92,7 @@ fn test_coinbase_maturity() {
 	assert!(
 		block.outputs[0]
 			.features
-			.contains(transaction::COINBASE_OUTPUT,)
+			.contains(transaction::COINBASE_OUTPUT)
 	);
 
 	chain.process_block(block, chain::NONE).unwrap();
@@ -108,7 +108,7 @@ fn test_coinbase_maturity() {
 	// this is not a valid tx as the coinbase output cannot be spent yet
 	let (coinbase_txn, _) = build::transaction(
 		vec![
-			build::input(amount, height, key_id1.clone()),
+			build::coinbase_input(amount, height, key_id1.clone()),
 			build::output(amount - 2, key_id2.clone()),
 			build::with_fee(2),
 		],
@@ -174,7 +174,7 @@ fn test_coinbase_maturity() {
 
 	let (coinbase_txn, _) = build::transaction(
 		vec![
-			build::input(amount, height, key_id1.clone()),
+			build::coinbase_input(amount, height, key_id1.clone()),
 			build::output(amount - 2, key_id2.clone()),
 			build::with_fee(2),
 		],

core/src/core/block.rs

@@ -583,7 +583,7 @@ impl Block {
 
 		let switch_commit_hash = SwitchCommitHash::from_switch_commit(
 			switch_commit,
-			SwitchCommitHashKey::from_features_and_lock_height(COINBASE_OUTPUT, lock_height),
+			SwitchCommitHashKey::from_lock_height(lock_height),
 		);
 
 		trace!(

core/src/core/build.rs

@@ -44,27 +44,40 @@ pub type Append = for<'a> Fn(&'a mut Context, (Transaction, BlindSum)) -> (Trans
 
 /// Adds an input with the provided value and blinding key to the transaction
 /// being built.
-pub fn input(
+fn input_with_lock_height(
 	value: u64,
 	lock_height: u64,
-	key_id: Identifier
+	key_id: Identifier,
 ) -> Box<Append> {
-	debug!(
-		LOGGER,
-		"Building an input: {}, {}, {}",
-		value,
-		lock_height,
-		key_id,
-	);
-
 	Box::new(move |build, (tx, sum)| -> (Transaction, BlindSum) {
 		let commit = build.keychain.commit(value, &key_id).unwrap();
 		let switch_commit = build.keychain.switch_commit(&key_id).unwrap();
-		let input = Input::new(commit, switch_commit, lock_height);
+		let input = Input::new(
+			commit,
+			switch_commit,
+			lock_height,
+		);
 		(tx.with_input(input), sum.sub_key_id(key_id.clone()))
 	})
 }
 
+pub fn input(
+	value: u64,
+	key_id: Identifier,
+) -> Box<Append> {
+	debug!(LOGGER, "Building an input: {}, {}", value, key_id);
+	input_with_lock_height(value, 0, key_id)
+}
+
+pub fn coinbase_input(
+	value: u64,
+	lock_height: u64,
+	key_id: Identifier,
+) -> Box<Append> {
+	debug!(LOGGER, "Building a coinbase input: {}, {}", value, key_id);
+	input_with_lock_height(value, lock_height, key_id)
+}
+
 /// Adds an output with the provided value and key identifier from the
 /// keychain.
 pub fn output(value: u64, key_id: Identifier) -> Box<Append> {

core/src/core/transaction.rs

@@ -394,12 +394,18 @@ impl Transaction {
 	}
 }
 
-/// A transaction input, mostly a reference to an output being spent by the
-/// transaction.
+/// A transaction input.
+///
+/// Primarily a reference to an output being spent by the transaction.
+/// But also information required to verify coinbase maturity through
+/// the lock_height hashed in the switch_commit_hash.
 #[derive(Debug, Clone, Copy)]
 pub struct Input{
 	commit: Commitment,
+	// We need to provide the switch_commit (switch_commit_hash preimage)
+	// to verify coinbase maturity.
 	switch_commit: Commitment,
+	// We need to provide lock_height to verify coinbase maturity.
 	lock_height: u64,
 }
 
@@ -432,10 +438,14 @@ impl Readable for Input {
 /// commitment is a reproduction of the commitment of the output it's spending.
 impl Input {
 	/// Build a new Input from a commit, switch_commit and lock_height
-	pub fn new(commit: Commitment, switch_commit: Commitment, lock_height: u64) -> Input {
+	pub fn new(
+		commit: Commitment,
+		switch_commit: Commitment,
+		lock_height: u64,
+	) -> Input {
 		debug!(
 			LOGGER,
-			"building a new input: {:?}, {:?}, {}",
+			"building a new input: {:?}, {:?}, {:?}",
 			commit,
 			switch_commit,
 			lock_height,
@@ -467,15 +477,15 @@ impl Input {
 	///
 	pub fn verify_lock_height(
 		&self,
-		output: &Output,
+		output_switch_commit_hash: &SwitchCommitHash,
 		height: u64,
 	) -> Result<(), Error> {
 		let switch_commit_hash = SwitchCommitHash::from_switch_commit(
 			self.switch_commit,
-			SwitchCommitHashKey::from_features_and_lock_height(output.features, self.lock_height),
+			SwitchCommitHashKey::from_lock_height(self.lock_height),
 		);
 
-		if switch_commit_hash != output.switch_commit_hash {
+		if switch_commit_hash != *output_switch_commit_hash {
 			return Err(Error::SwitchCommitment);
 		} else {
 			if height <= self.lock_height {
@@ -505,24 +515,17 @@ pub struct SwitchCommitHashKey ([u8; SWITCH_COMMIT_KEY_SIZE]);
 impl SwitchCommitHashKey {
 	/// For a coinbase output use (output_features || lock_height) as the key.
 	/// For regular tx outputs use the zero value as the key.
-	pub fn from_features_and_lock_height(
-		features: OutputFeatures,
+	pub fn from_lock_height(
 		lock_height: u64,
 	) -> SwitchCommitHashKey {
 		let mut bytes = [0; SWITCH_COMMIT_KEY_SIZE];
-		if features.contains(COINBASE_OUTPUT) {
-			bytes[0] = features.bits();
-			// seems wasteful to take up a full 8 bytes (of 20 bytes) to store the lock_height
-			// 4 bytes will give us approx 4,000 years with 1 min blocks (unless my math is way off)
-			BigEndian::write_u32(&mut bytes[1..5], lock_height as u32);
-		}
+		BigEndian::write_u64(&mut bytes[..], lock_height);
 		SwitchCommitHashKey(bytes)
 	}
 
 	/// We use a zero value key for regular transactions.
 	pub fn zero() -> SwitchCommitHashKey {
-		let bytes = [0; SWITCH_COMMIT_KEY_SIZE];
-		SwitchCommitHashKey(bytes)
+		SwitchCommitHashKey::from_lock_height(0)
 	}
 }
 

pool/src/pool.rs

@@ -1228,7 +1228,7 @@ mod tests {
 
 		for input_value in input_values {
 			let key_id = keychain.derive_key_id(input_value as u32).unwrap();
-			tx_elements.push(build::input(input_value, 0, key_id));
+			tx_elements.push(build::input(input_value, key_id));
 		}
 
 		for output_value in output_values {
@@ -1256,7 +1256,7 @@ mod tests {
 
 		for input_value in input_values {
 			let key_id = keychain.derive_key_id(input_value as u32).unwrap();
-			tx_elements.push(build::input(input_value, 0, key_id));
+			tx_elements.push(build::input(input_value, key_id));
 		}
 
 		for output_value in output_values {

wallet/src/restore.rs

@@ -134,15 +134,13 @@ fn find_utxos_with_key(
 
 	for output in block_outputs.outputs {
 		for i in 0..*key_iterations {
+			// TODO - these are very similar, factor out duplicate code
 			let expected_hash = match output.output_type {
 				api::OutputType::Coinbase => {
 					let lock_height = block_outputs.header.height + global::coinbase_maturity();
 					SwitchCommitHash::from_switch_commit(
 						switch_commit_cache[i as usize],
-						SwitchCommitHashKey::from_features_and_lock_height(
-							COINBASE_OUTPUT,
-							lock_height,
-						),
+						SwitchCommitHashKey::from_lock_height(lock_height),
 					)
 				}
 				api::OutputType::Transaction => {

wallet/src/sender.rs

@@ -213,7 +213,11 @@ fn inputs_and_change(
 	// build inputs using the appropriate derived key_ids
 	for coin in coins {
 		let key_id = keychain.derive_key_id(coin.n_child)?;
-		parts.push(build::input(coin.value, coin.lock_height, key_id));
+		if coin.is_coinbase {
+			parts.push(build::coinbase_input(coin.value, coin.lock_height, key_id));
+		} else {
+			parts.push(build::input(coin.value, key_id));
+		}
 	}
 
 	// track the output representing our change