Shorter identifiers and call them key_id (not pubkey) (#175)

* rename pubkey -> key_id, these are key identifiers, not public keys
* reduce identifier seize to 10 bytes, get rid of fingerprints (identifiers are now small enough to use as-is)
* IDENTIFIER_SIZE const
* add FeeDispute error for when sender and recipient disagre on fee calculation (should never happen)
* does not need to be mut
* cleaned up some warnings

chain/src/chain.rs

@@ -70,11 +70,12 @@ impl Chain {
 	/// on the current chain head to make sure it exists and creates one based
 	/// on
 	/// the genesis block if necessary.
-	pub fn init(db_root: String,
+	pub fn init(
-	            adapter: Arc<ChainAdapter>,
+		db_root: String,
-	            gen_block: Option<Block>,
+		adapter: Arc<ChainAdapter>,
-	            pow_verifier: fn(&BlockHeader, u32) -> bool)
+		gen_block: Option<Block>,
-	            -> Result<Chain, Error> {
+		pow_verifier: fn(&BlockHeader, u32) -> bool,
+	) -> Result<Chain, Error> {
 		let chain_store = store::ChainKVStore::new(db_root.clone())?;
 
 		// check if we have a head in store, otherwise the genesis block is it
@@ -152,10 +153,11 @@ impl Chain {
 
 	/// Attempt to add a new header to the header chain. Only necessary during
 	/// sync.
-	pub fn process_block_header(&self,
+	pub fn process_block_header(
-	                            bh: &BlockHeader,
+		&self,
-	                            opts: Options)
+		bh: &BlockHeader,
-	                            -> Result<Option<Tip>, Error> {
+		opts: Options,
+	) -> Result<Option<Tip>, Error> {
 
 		let head = self.store.get_header_head().map_err(&Error::StoreErr)?;
 		let ctx = self.ctx_from_head(head, opts);
@@ -213,9 +215,9 @@ impl Chain {
 		let sumtrees = self.sumtrees.read().unwrap();
 		let is_unspent = sumtrees.is_unspent(output_ref)?;
 		if is_unspent {
-			self.store
+			self.store.get_output_by_commit(output_ref).map_err(
-				.get_output_by_commit(output_ref)
+				&Error::StoreErr,
-				.map_err(&Error::StoreErr)
+			)
 		} else {
 			Err(Error::OutputNotFound)
 		}
@@ -226,7 +228,7 @@ impl Chain {
 	pub fn set_sumtree_roots(&self, b: &mut Block) -> Result<(), Error> {
 		let mut sumtrees = self.sumtrees.write().unwrap();
 
-		let roots = sumtree::extending(&mut sumtrees, |mut extension| {
+		let roots = sumtree::extending(&mut sumtrees, |extension| {
 			// apply the block on the sumtrees and check the resulting root
 			extension.apply_block(b)?;
 			extension.force_rollback();
@@ -266,15 +268,16 @@ impl Chain {
 
 	/// Gets the block header at the provided height
 	pub fn get_header_by_height(&self, height: u64) -> Result<BlockHeader, Error> {
-		self.store
+		self.store.get_header_by_height(height).map_err(
-			.get_header_by_height(height)
+			&Error::StoreErr,
-			.map_err(&Error::StoreErr)
+		)
 	}
 
 	/// Gets the block header by the provided output commitment
-	pub fn get_block_header_by_output_commit(&self,
+	pub fn get_block_header_by_output_commit(
-	                                         commit: &Commitment)
+		&self,
-	                                         -> Result<BlockHeader, Error> {
+		commit: &Commitment,
+	) -> Result<BlockHeader, Error> {
 		self.store
 			.get_block_header_by_output_commit(commit)
 			.map_err(&Error::StoreErr)

chain/tests/mine_simple_chain.rs

@@ -22,7 +22,6 @@ extern crate grin_pow as pow;
 
 use std::fs;
 use std::sync::Arc;
-use rand::os::OsRng;
 
 use chain::Chain;
 use chain::types::*;
@@ -59,7 +58,6 @@ fn setup(dir_name: &str) -> Chain {
 
 #[test]
 fn mine_empty_chain() {
-	let mut rng = OsRng::new().unwrap();
 	let chain = setup(".grin");
 
 	let keychain = Keychain::from_random_seed().unwrap();
@@ -79,7 +77,7 @@ fn mine_empty_chain() {
 	);
 	for n in 1..4 {
 		let prev = chain.head_header().unwrap();
-		let pk = keychain.derive_pubkey(n as u32).unwrap();
+		let pk = keychain.derive_key_id(n as u32).unwrap();
 		let mut b = core::core::Block::new(&prev, vec![], &keychain, &pk).unwrap();
 		b.header.timestamp = prev.timestamp + time::Duration::seconds(60);
 
@@ -206,7 +204,6 @@ fn longer_fork() {
 	// add blocks to both chains, 20 on the main one, only the first 5
 	// for the forked chain
 	let mut prev = chain.head_header().unwrap();
-	let forking_header: BlockHeader;
 	for n in 0..10 {
 		let b = prepare_block(&prev, &chain, n + 2);
 		let bh = b.header.clone();
@@ -233,7 +230,6 @@ fn longer_fork() {
 		let bh_fork = b_fork.header.clone();
 
 		let b = b_fork.clone();
-		let bh = b.header.clone();
 		chain.process_block(b, chain::SKIP_POW).unwrap();
 
 		chain_fork.process_block(b_fork, chain::SKIP_POW).unwrap();
@@ -249,9 +245,9 @@ fn prepare_block(prev: &BlockHeader, chain: &Chain, diff: u64) -> Block {
 
 fn prepare_block_nosum(prev: &BlockHeader, diff: u64) -> Block {
 	let keychain = Keychain::from_random_seed().unwrap();
-	let pubkey = keychain.derive_pubkey(1).unwrap();
+	let key_id = keychain.derive_key_id(1).unwrap();
 
-	let mut b = core::core::Block::new(prev, vec![], &keychain, &pubkey).unwrap();
+	let mut b = core::core::Block::new(prev, vec![], &keychain, &key_id).unwrap();
 	b.header.timestamp = prev.timestamp + time::Duration::seconds(60);
 	b.header.total_difficulty = Difficulty::from_num(diff);
 	b

chain/tests/store_indices.rs

@@ -35,11 +35,11 @@ fn test_various_store_indices() {
 	clean_output_dir(".grin");
 
 	let keychain = Keychain::from_random_seed().unwrap();
-	let pubkey = keychain.derive_pubkey(1).unwrap();
+	let key_id = keychain.derive_key_id(1).unwrap();
 
 	let chain_store = &chain::store::ChainKVStore::new(".grin".to_string()).unwrap() as &ChainStore;
 
-	let block = Block::new(&BlockHeader::default(), vec![], &keychain, &pubkey).unwrap();
+	let block = Block::new(&BlockHeader::default(), vec![], &keychain, &key_id).unwrap();
 	let commit = block.outputs[0].commitment();
 	let block_hash = block.hash();
 

chain/tests/test_coinbase_maturity.rs

@@ -71,12 +71,12 @@ fn test_coinbase_maturity() {
 	let prev = chain.head_header().unwrap();
 
 	let keychain = Keychain::from_random_seed().unwrap();
-	let pk1 = keychain.derive_pubkey(1).unwrap();
+	let key_id1 = keychain.derive_key_id(1).unwrap();
-	let pk2 = keychain.derive_pubkey(2).unwrap();
+	let key_id2 = keychain.derive_key_id(2).unwrap();
-	let pk3 = keychain.derive_pubkey(3).unwrap();
+	let key_id3 = keychain.derive_key_id(3).unwrap();
-	let pk4 = keychain.derive_pubkey(4).unwrap();
+	let key_id4 = keychain.derive_key_id(4).unwrap();
 
-	let mut block = core::core::Block::new(&prev, vec![], &keychain, &pk1).unwrap();
+	let mut block = core::core::Block::new(&prev, vec![], &keychain, &key_id1).unwrap();
 	block.header.timestamp = prev.timestamp + time::Duration::seconds(60);
 
 	let difficulty = consensus::next_difficulty(chain.difficulty_iter()).unwrap();
@@ -102,14 +102,14 @@ fn test_coinbase_maturity() {
 	let amount = consensus::REWARD;
 	let (coinbase_txn, _) = build::transaction(
 		vec![
-			build::input(amount, pk1.clone()),
+			build::input(amount, key_id1.clone()),
-			build::output(amount - 2, pk2),
+			build::output(amount - 2, key_id2),
 			build::with_fee(2),
 		],
 		&keychain,
 	).unwrap();
 
-	let mut block = core::core::Block::new(&prev, vec![&coinbase_txn], &keychain, &pk3).unwrap();
+	let mut block = core::core::Block::new(&prev, vec![&coinbase_txn], &keychain, &key_id3).unwrap();
 	block.header.timestamp = prev.timestamp + time::Duration::seconds(60);
 
 	let difficulty = consensus::next_difficulty(chain.difficulty_iter()).unwrap();
@@ -135,7 +135,7 @@ fn test_coinbase_maturity() {
 		let prev = chain.head_header().unwrap();
 
 		let keychain = Keychain::from_random_seed().unwrap();
-		let pk = keychain.derive_pubkey(1).unwrap();
+		let pk = keychain.derive_key_id(1).unwrap();
 
 		let mut block = core::core::Block::new(&prev, vec![], &keychain, &pk).unwrap();
 		block.header.timestamp = prev.timestamp + time::Duration::seconds(60);
@@ -156,7 +156,7 @@ fn test_coinbase_maturity() {
 
 	let prev = chain.head_header().unwrap();
 
-	let mut block = core::core::Block::new(&prev, vec![&coinbase_txn], &keychain, &pk4).unwrap();
+	let mut block = core::core::Block::new(&prev, vec![&coinbase_txn], &keychain, &key_id4).unwrap();
 
 	block.header.timestamp = prev.timestamp + time::Duration::seconds(60);
 

core/src/core/block.rs

@@ -263,10 +263,10 @@ impl Block {
 		prev: &BlockHeader,
 		txs: Vec<&Transaction>,
 		keychain: &keychain::Keychain,
-		pubkey: &keychain::Identifier,
+		key_id: &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 (reward_out, reward_proof) = Block::reward_output(keychain, key_id, fees)?;
 		let block = Block::with_reward(prev, txs, reward_out, reward_proof)?;
 		Ok(block)
 	}
@@ -501,15 +501,15 @@ impl Block {
 	/// Builds the blinded output and related signature proof for the block reward.
 	pub fn reward_output(
 		keychain: &keychain::Keychain,
-		pubkey: &keychain::Identifier,
+		key_id: &keychain::Identifier,
 		fees: u64,
 	) -> Result<(Output, TxKernel), keychain::Error> {
 		let secp = keychain.secp();
 
-		let commit = keychain.commit(reward(fees), pubkey)?;
+		let commit = keychain.commit(reward(fees), key_id)?;
-		// let switch_commit = keychain.switch_commit(pubkey)?;
+		// let switch_commit = keychain.switch_commit(key_id)?;
 		let msg = secp::pedersen::ProofMessage::empty();
-		let rproof = keychain.range_proof(reward(fees), pubkey, commit, msg)?;
+		let rproof = keychain.range_proof(reward(fees), key_id, commit, msg)?;
 
 		let output = Output {
 			features: COINBASE_OUTPUT,
@@ -522,7 +522,7 @@ impl Block {
 		let excess = secp.commit_sum(vec![out_commit], vec![over_commit])?;
 
 		let msg = secp::Message::from_slice(&[0; secp::constants::MESSAGE_SIZE])?;
-		let sig = keychain.sign(&msg, &pubkey)?;
+		let sig = keychain.sign(&msg, &key_id)?;
 
 		let proof = TxKernel {
 			features: COINBASE_KERNEL,
@@ -550,14 +550,14 @@ mod test {
 	// utility to create a block without worrying about the key or previous
 	// header
 	fn new_block(txs: Vec<&Transaction>, keychain: &Keychain) -> Block {
-		let pubkey = keychain.derive_pubkey(1).unwrap();
+		let key_id = keychain.derive_key_id(1).unwrap();
-		Block::new(&BlockHeader::default(), txs, keychain, &pubkey).unwrap()
+		Block::new(&BlockHeader::default(), txs, keychain, &key_id).unwrap()
 	}
 
 	// utility producing a transaction that spends an output with the provided
 	// value and blinding key
-	fn txspend1i1o(v: u64, keychain: &Keychain, pk1: Identifier, pk2: Identifier) -> Transaction {
+	fn txspend1i1o(v: u64, keychain: &Keychain, key_id1: Identifier, key_id2: Identifier) -> Transaction {
-		build::transaction(vec![input(v, pk1), output(3, pk2), with_fee(2)], &keychain)
+		build::transaction(vec![input(v, key_id1), output(3, key_id2), with_fee(2)], &keychain)
 			.map(|(tx, _)| tx)
 			.unwrap()
 	}
@@ -569,7 +569,7 @@ mod test {
 
 		let mut pks = vec![];
 		for n in 0..(max_out + 1) {
-			pks.push(keychain.derive_pubkey(n as u32).unwrap());
+			pks.push(keychain.derive_key_id(n as u32).unwrap());
 		}
 
 		let mut parts = vec![];
@@ -592,19 +592,19 @@ mod test {
 	// builds a block with a tx spending another and check if merging occurred
 	fn compactable_block() {
 		let keychain = Keychain::from_random_seed().unwrap();
-		let pk1 = keychain.derive_pubkey(1).unwrap();
+		let key_id1 = keychain.derive_key_id(1).unwrap();
-		let pk2 = keychain.derive_pubkey(2).unwrap();
+		let key_id2 = keychain.derive_key_id(2).unwrap();
-		let pk3 = keychain.derive_pubkey(3).unwrap();
+		let key_id3 = keychain.derive_key_id(3).unwrap();
 
 		let mut btx1 = tx2i1o();
 		let (mut btx2, _) = build::transaction(
-			vec![input(7, pk1), output(5, pk2.clone()), with_fee(2)],
+			vec![input(7, key_id1), output(5, key_id2.clone()), with_fee(2)],
 			&keychain,
 		).unwrap();
 
-		// spending tx2 - reuse pk2
+		// spending tx2 - reuse key_id2
 
-		let mut btx3 = txspend1i1o(5, &keychain, pk2.clone(), pk3);
+		let mut btx3 = txspend1i1o(5, &keychain, key_id2.clone(), key_id3);
 		let b = new_block(vec![&mut btx1, &mut btx2, &mut btx3], &keychain);
 
 		// block should have been automatically compacted (including reward
@@ -619,19 +619,19 @@ mod test {
 	// occurs
 	fn mergeable_blocks() {
 		let keychain = Keychain::from_random_seed().unwrap();
-		let pk1 = keychain.derive_pubkey(1).unwrap();
+		let key_id1 = keychain.derive_key_id(1).unwrap();
-		let pk2 = keychain.derive_pubkey(2).unwrap();
+		let key_id2 = keychain.derive_key_id(2).unwrap();
-		let pk3 = keychain.derive_pubkey(3).unwrap();
+		let key_id3 = keychain.derive_key_id(3).unwrap();
 
 		let mut btx1 = tx2i1o();
 
 		let (mut btx2, _) = build::transaction(
-			vec![input(7, pk1), output(5, pk2.clone()), with_fee(2)],
+			vec![input(7, key_id1), output(5, key_id2.clone()), with_fee(2)],
 			&keychain,
 		).unwrap();
 
-		// spending tx2 - reuse pk2
+		// spending tx2 - reuse key_id2
-		let mut btx3 = txspend1i1o(5, &keychain, pk2.clone(), pk3);
+		let mut btx3 = txspend1i1o(5, &keychain, key_id2.clone(), key_id3);
 
 		let b1 = new_block(vec![&mut btx1, &mut btx2], &keychain);
 		b1.validate(&keychain.secp()).unwrap();

core/src/core/build.rs

@@ -43,22 +43,22 @@ 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(value: u64, pubkey: Identifier) -> Box<Append> {
+pub fn input(value: u64, key_id: Identifier) -> Box<Append> {
 	Box::new(move |build, (tx, sum)| -> (Transaction, BlindSum) {
-		let commit = build.keychain.commit(value, &pubkey).unwrap();
+		let commit = build.keychain.commit(value, &key_id).unwrap();
-		(tx.with_input(Input(commit)), sum.sub_pubkey(pubkey.clone()))
+		(tx.with_input(Input(commit)), sum.sub_key_id(key_id.clone()))
 	})
 }
 
 /// Adds an output with the provided value and key identifier from the
 /// keychain.
-pub fn output(value: u64, pubkey: Identifier) -> Box<Append> {
+pub fn output(value: u64, key_id: Identifier) -> Box<Append> {
 	Box::new(move |build, (tx, sum)| -> (Transaction, BlindSum) {
-		let commit = build.keychain.commit(value, &pubkey).unwrap();
+		let commit = build.keychain.commit(value, &key_id).unwrap();
 		let msg = secp::pedersen::ProofMessage::empty();
 		let rproof = build
 			.keychain
-			.range_proof(value, &pubkey, commit, msg)
+			.range_proof(value, &key_id, commit, msg)
 			.unwrap();
 
 		(
@@ -67,7 +67,7 @@ pub fn output(value: u64, pubkey: Identifier) -> Box<Append> {
 				commit: commit,
 				proof: rproof,
 			}),
-			sum.add_pubkey(pubkey.clone()),
+			sum.add_key_id(key_id.clone()),
 		)
 	})
 }
@@ -136,12 +136,12 @@ mod test {
 	#[test]
 	fn blind_simple_tx() {
 		let keychain = Keychain::from_random_seed().unwrap();
-		let pk1 = keychain.derive_pubkey(1).unwrap();
+		let key_id1 = keychain.derive_key_id(1).unwrap();
-		let pk2 = keychain.derive_pubkey(2).unwrap();
+		let key_id2 = keychain.derive_key_id(2).unwrap();
-		let pk3 = keychain.derive_pubkey(3).unwrap();
+		let key_id3 = keychain.derive_key_id(3).unwrap();
 
 		let (tx, _) = transaction(
-			vec![input(10, pk1), input(11, pk2), output(20, pk3), with_fee(1)],
+			vec![input(10, key_id1), input(11, key_id2), output(20, key_id3), with_fee(1)],
 			&keychain,
 		).unwrap();
 
@@ -151,10 +151,10 @@ mod test {
 	#[test]
 	fn blind_simpler_tx() {
 		let keychain = Keychain::from_random_seed().unwrap();
-		let pk1 = keychain.derive_pubkey(1).unwrap();
+		let key_id1 = keychain.derive_key_id(1).unwrap();
-		let pk2 = keychain.derive_pubkey(2).unwrap();
+		let key_id2 = keychain.derive_key_id(2).unwrap();
 
-		let (tx, _) = transaction(vec![input(6, pk1), output(2, pk2), with_fee(4)], &keychain)
+		let (tx, _) = transaction(vec![input(6, key_id1), output(2, key_id2), with_fee(4)], &keychain)
 			.unwrap();
 
 		tx.verify_sig(&keychain.secp()).unwrap();

core/src/core/mod.rs

@@ -196,11 +196,11 @@ mod test {
 	#[should_panic(expected = "InvalidSecretKey")]
 	fn test_zero_commit_fails() {
 		let keychain = Keychain::from_random_seed().unwrap();
-		let pk1 = keychain.derive_pubkey(1).unwrap();
+		let key_id1 = keychain.derive_key_id(1).unwrap();
 
 		// blinding should fail as signing with a zero r*G shouldn't work
 		build::transaction(
-			vec![input(10, pk1.clone()), output(9, pk1.clone()), with_fee(1)],
+			vec![input(10, key_id1.clone()), output(9, key_id1.clone()), with_fee(1)],
 			&keychain,
 		).unwrap();
 	}
@@ -246,15 +246,15 @@ mod test {
 	#[test]
 	fn hash_output() {
 		let keychain = Keychain::from_random_seed().unwrap();
-		let pk1 = keychain.derive_pubkey(1).unwrap();
+		let key_id1 = keychain.derive_key_id(1).unwrap();
-		let pk2 = keychain.derive_pubkey(2).unwrap();
+		let key_id2 = keychain.derive_key_id(2).unwrap();
-		let pk3 = keychain.derive_pubkey(3).unwrap();
+		let key_id3 = keychain.derive_key_id(3).unwrap();
 
 		let (tx, _) = build::transaction(
 			vec![
-				input(75, pk1),
+				input(75, key_id1),
-				output(42, pk2),
+				output(42, key_id2),
-				output(32, pk3),
+				output(32, key_id3),
 				with_fee(1),
 			],
 			&keychain,
@@ -294,10 +294,10 @@ mod test {
 	#[test]
 	fn tx_build_exchange() {
 		let keychain = Keychain::from_random_seed().unwrap();
-		let pk1 = keychain.derive_pubkey(1).unwrap();
+		let key_id1 = keychain.derive_key_id(1).unwrap();
-		let pk2 = keychain.derive_pubkey(2).unwrap();
+		let key_id2 = keychain.derive_key_id(2).unwrap();
-		let pk3 = keychain.derive_pubkey(3).unwrap();
+		let key_id3 = keychain.derive_key_id(3).unwrap();
-		let pk4 = keychain.derive_pubkey(4).unwrap();
+		let key_id4 = keychain.derive_key_id(4).unwrap();
 
 		let tx_alice: Transaction;
 		let blind_sum: BlindingFactor;
@@ -305,12 +305,12 @@ mod test {
 		{
 			// Alice gets 2 of her pre-existing outputs to send 5 coins to Bob, they
 			// become inputs in the new transaction
-			let (in1, in2) = (input(4, pk1), input(3, pk2));
+			let (in1, in2) = (input(4, key_id1), input(3, key_id2));
 
 			// 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();
+				build::transaction(vec![in1, in2, output(1, key_id3), with_fee(2)], &keychain).unwrap();
 			tx_alice = tx;
 			blind_sum = sum;
 		}
@@ -319,7 +319,7 @@ mod test {
 		// 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)],
+			vec![initial_tx(tx_alice), with_excess(blind_sum), output(4, key_id4)],
 			&keychain,
 		).unwrap();
 
@@ -329,28 +329,28 @@ mod test {
 	#[test]
 	fn reward_empty_block() {
 		let keychain = keychain::Keychain::from_random_seed().unwrap();
-		let pubkey = keychain.derive_pubkey(1).unwrap();
+		let key_id = keychain.derive_key_id(1).unwrap();
 
-		let b = Block::new(&BlockHeader::default(), vec![], &keychain, &pubkey).unwrap();
+		let b = Block::new(&BlockHeader::default(), vec![], &keychain, &key_id).unwrap();
 		b.compact().validate(&keychain.secp()).unwrap();
 	}
 
 	#[test]
 	fn reward_with_tx_block() {
 		let keychain = keychain::Keychain::from_random_seed().unwrap();
-		let pubkey = keychain.derive_pubkey(1).unwrap();
+		let key_id = keychain.derive_key_id(1).unwrap();
 
 		let mut tx1 = tx2i1o();
 		tx1.verify_sig(keychain.secp()).unwrap();
 
-		let b = Block::new(&BlockHeader::default(), vec![&mut tx1], &keychain, &pubkey).unwrap();
+		let b = Block::new(&BlockHeader::default(), vec![&mut tx1], &keychain, &key_id).unwrap();
 		b.compact().validate(keychain.secp()).unwrap();
 	}
 
 	#[test]
 	fn simple_block() {
 		let keychain = keychain::Keychain::from_random_seed().unwrap();
-		let pubkey = keychain.derive_pubkey(1).unwrap();
+		let key_id = keychain.derive_key_id(1).unwrap();
 
 		let mut tx1 = tx2i1o();
 		let mut tx2 = tx1i1o();
@@ -359,7 +359,7 @@ mod test {
 			&BlockHeader::default(),
 			vec![&mut tx1, &mut tx2],
 			&keychain,
-			&pubkey,
+			&key_id,
 		).unwrap();
 		b.validate(keychain.secp()).unwrap();
 	}
@@ -368,14 +368,14 @@ mod test {
 	fn test_block_with_timelocked_tx() {
 		let keychain = keychain::Keychain::from_random_seed().unwrap();
 
-		let pk1 = keychain.derive_pubkey(1).unwrap();
+		let key_id1 = keychain.derive_key_id(1).unwrap();
-		let pk2 = keychain.derive_pubkey(2).unwrap();
+		let key_id2 = keychain.derive_key_id(2).unwrap();
-		let pk3 = keychain.derive_pubkey(3).unwrap();
+		let key_id3 = keychain.derive_key_id(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)],
+			vec![input(5, key_id1.clone()), output(3, key_id2.clone()), with_fee(2), with_lock_height(1)],
 			&keychain,
 		).map(|(tx, _)| tx).unwrap();
 
@@ -383,13 +383,13 @@ mod test {
 			&BlockHeader::default(),
 			vec![&tx1],
 			&keychain,
-			&pk3.clone(),
+			&key_id3.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)],
+			vec![input(5, key_id1.clone()), output(3, key_id2.clone()), with_fee(2), with_lock_height(2)],
 			&keychain,
 		).map(|(tx, _)| tx).unwrap();
 
@@ -397,7 +397,7 @@ mod test {
 			&BlockHeader::default(),
 			vec![&tx1],
 			&keychain,
-			&pk3.clone(),
+			&key_id3.clone(),
 		).unwrap();
 		match b.validate(keychain.secp()) {
 			Err(KernelLockHeight{ lock_height: height}) => {
@@ -424,12 +424,12 @@ mod test {
 	// utility producing a transaction with 2 inputs and a single outputs
 	pub fn tx2i1o() -> Transaction {
 		let keychain = keychain::Keychain::from_random_seed().unwrap();
-		let pk1 = keychain.derive_pubkey(1).unwrap();
+		let key_id1 = keychain.derive_key_id(1).unwrap();
-		let pk2 = keychain.derive_pubkey(2).unwrap();
+		let key_id2 = keychain.derive_key_id(2).unwrap();
-		let pk3 = keychain.derive_pubkey(3).unwrap();
+		let key_id3 = keychain.derive_key_id(3).unwrap();
 
 		build::transaction(
-			vec![input(10, pk1), input(11, pk2), output(19, pk3), with_fee(2)],
+			vec![input(10, key_id1), input(11, key_id2), output(19, key_id3), with_fee(2)],
 			&keychain,
 		).map(|(tx, _)| tx)
 			.unwrap()
@@ -438,10 +438,10 @@ mod test {
 	// utility producing a transaction with a single input and output
 	pub fn tx1i1o() -> Transaction {
 		let keychain = keychain::Keychain::from_random_seed().unwrap();
-		let pk1 = keychain.derive_pubkey(1).unwrap();
+		let key_id1 = keychain.derive_key_id(1).unwrap();
-		let pk2 = keychain.derive_pubkey(2).unwrap();
+		let key_id2 = keychain.derive_key_id(2).unwrap();
 
-		build::transaction(vec![input(5, pk1), output(3, pk2), with_fee(2)], &keychain)
+		build::transaction(vec![input(5, key_id1), output(3, key_id2), with_fee(2)], &keychain)
 			.map(|(tx, _)| tx)
 			.unwrap()
 	}

core/src/core/transaction.rs

@@ -282,8 +282,8 @@ impl Transaction {
 		// pretend the sum is a public key (which it is, being of the form r.G) and
 		// verify the transaction sig with it
 		//
-		// we originally converted the commitment to a pubkey here (commitment to zero)
+		// we originally converted the commitment to a key_id here (commitment to zero)
-		// and then passed the pubkey to secp.verify()
+		// and then passed the key_id to secp.verify()
 		// the secp api no longer allows us to do this so we have wrapped the complexity
 		// of generating a public key from a commitment behind verify_from_commit
 		secp.verify_from_commit(&msg, &sig, &rsum)?;
@@ -425,8 +425,8 @@ impl Output {
 
 	/// Given the original blinding factor we can recover the
 	/// value from the range proof and the commitment
-	pub fn recover_value(&self, keychain: &Keychain, pubkey: &Identifier) -> Option<u64> {
+	pub fn recover_value(&self, keychain: &Keychain, key_id: &Identifier) -> Option<u64> {
-		match keychain.rewind_range_proof(pubkey, self.commit, self.proof) {
+		match keychain.rewind_range_proof(key_id, self.commit, self.proof) {
 			Ok(proof_info) => {
 				if proof_info.success {
 					Some(proof_info.value)
@@ -507,8 +507,8 @@ mod test {
 	#[test]
 	fn test_kernel_ser_deser() {
 		let keychain = Keychain::from_random_seed().unwrap();
-		let pubkey = keychain.derive_pubkey(1).unwrap();
+		let key_id = keychain.derive_key_id(1).unwrap();
-		let commit = keychain.commit(5, &pubkey).unwrap();
+		let commit = keychain.commit(5, &key_id).unwrap();
 
 		// just some bytes for testing ser/deser
 		let sig = vec![1, 0, 0, 0, 0, 0, 0, 1];
@@ -552,10 +552,10 @@ mod test {
 	#[test]
 	fn test_output_ser_deser() {
 		let keychain = Keychain::from_random_seed().unwrap();
-		let pubkey = keychain.derive_pubkey(1).unwrap();
+		let key_id = keychain.derive_key_id(1).unwrap();
-		let commit = keychain.commit(5, &pubkey).unwrap();
+		let commit = keychain.commit(5, &key_id).unwrap();
 		let msg = secp::pedersen::ProofMessage::empty();
-		let proof = keychain.range_proof(5, &pubkey, commit, msg).unwrap();
+		let proof = keychain.range_proof(5, &key_id, commit, msg).unwrap();
 
 		let out = Output {
 			features: DEFAULT_OUTPUT,
@@ -575,11 +575,11 @@ mod test {
 	#[test]
 	fn test_output_value_recovery() {
 		let keychain = Keychain::from_random_seed().unwrap();
-		let pubkey = keychain.derive_pubkey(1).unwrap();
+		let key_id = keychain.derive_key_id(1).unwrap();
 
-		let commit = keychain.commit(1003, &pubkey).unwrap();
+		let commit = keychain.commit(1003, &key_id).unwrap();
 		let msg = secp::pedersen::ProofMessage::empty();
-		let proof = keychain.range_proof(1003, &pubkey, commit, msg).unwrap();
+		let proof = keychain.range_proof(1003, &key_id, commit, msg).unwrap();
 
 		let output = Output {
 			features: DEFAULT_OUTPUT,
@@ -588,12 +588,12 @@ mod test {
 		};
 
 		// check we can successfully recover the value with the original blinding factor
-		let recovered_value = output.recover_value(&keychain, &pubkey).unwrap();
+		let recovered_value = output.recover_value(&keychain, &key_id).unwrap();
 		assert_eq!(recovered_value, 1003);
 
 		// check we cannot recover the value without the original blinding factor
-		let pubkey2 = keychain.derive_pubkey(2).unwrap();
+		let key_id2 = keychain.derive_key_id(2).unwrap();
-		let not_recoverable = output.recover_value(&keychain, &pubkey2);
+		let not_recoverable = output.recover_value(&keychain, &key_id2);
 		match not_recoverable {
 			Some(_) => panic!("expected value to be None here"),
 			None => {}

core/src/ser.rs

@@ -22,11 +22,10 @@
 use std::{error, fmt, cmp};
 use std::io::{self, Write, Read};
 use byteorder::{ByteOrder, ReadBytesExt, BigEndian};
-use keychain::Identifier;
+use keychain::{Identifier, IDENTIFIER_SIZE};
 use secp::pedersen::Commitment;
 use secp::pedersen::RangeProof;
-use secp::constants::PEDERSEN_COMMITMENT_SIZE;
+use secp::constants::{MAX_PROOF_SIZE, PEDERSEN_COMMITMENT_SIZE};
-use secp::constants::MAX_PROOF_SIZE;
 
 /// Possible errors deriving from serializing or deserializing.
 #[derive(Debug)]
@@ -294,7 +293,7 @@ impl Writeable for Identifier {
 
 impl Readable for Identifier {
 	fn read(reader: &mut Reader) -> Result<Identifier, Error> {
-		let bytes = reader.read_fixed_bytes(20)?;
+		let bytes = reader.read_fixed_bytes(IDENTIFIER_SIZE)?;
 		Ok(Identifier::from_bytes(&bytes))
 	}
 }
@@ -527,6 +526,6 @@ impl AsFixedBytes for ::secp::pedersen::Commitment {
 }
 impl AsFixedBytes for ::keychain::Identifier {
 	fn len(&self) -> usize {
-		return 20;
+		return IDENTIFIER_SIZE;
 	}
 }

grin/src/miner.rs

@@ -82,8 +82,10 @@ impl Default for HeaderPartWriter {
 
 impl HeaderPartWriter {
 	pub fn parts_as_hex_strings(&self) -> (String, String) {
-		(String::from(format!("{:02x}", self.pre_nonce.iter().format(""))),
+		(
-		 String::from(format!("{:02x}", self.post_nonce.iter().format(""))))
+			String::from(format!("{:02x}", self.pre_nonce.iter().format(""))),
+			String::from(format!("{:02x}", self.post_nonce.iter().format(""))),
+		)
 	}
 }
 
@@ -128,10 +130,11 @@ pub struct Miner {
 impl Miner {
 	/// Creates a new Miner. Needs references to the chain state and its
 	/// storage.
-	pub fn new(config: MinerConfig,
+	pub fn new(
-	           chain_ref: Arc<chain::Chain>,
+		config: MinerConfig,
-	           tx_pool: Arc<RwLock<pool::TransactionPool<PoolToChainAdapter>>>)
+		chain_ref: Arc<chain::Chain>,
-	           -> Miner {
+		tx_pool: Arc<RwLock<pool::TransactionPool<PoolToChainAdapter>>>,
+	) -> Miner {
 		Miner {
 			config: config,
 			chain: chain_ref,
@@ -147,15 +150,16 @@ impl Miner {
 	}
 
 	/// Inner part of the mining loop for cuckoo-miner async mode
-	pub fn inner_loop_async(&self,
+	pub fn inner_loop_async(
-	                        plugin_miner: &mut PluginMiner,
+		&self,
-	                        difficulty: Difficulty,
+		plugin_miner: &mut PluginMiner,
-	                        b: &mut Block,
+		difficulty: Difficulty,
-	                        cuckoo_size: u32,
+		b: &mut Block,
-	                        head: &BlockHeader,
+		cuckoo_size: u32,
-	                        latest_hash: &Hash,
+		head: &BlockHeader,
-	                        attempt_time_per_block: u32)
+		latest_hash: &Hash,
-	                        -> Option<Proof> {
+		attempt_time_per_block: u32,
+	) -> Option<Proof> {
 
 		debug!(
 			LOGGER,
@@ -250,14 +254,15 @@ impl Miner {
 	}
 
 	/// The inner part of mining loop for cuckoo miner sync mode
-	pub fn inner_loop_sync_plugin(&self,
+	pub fn inner_loop_sync_plugin(
-	                              plugin_miner: &mut PluginMiner,
+		&self,
-	                              b: &mut Block,
+		plugin_miner: &mut PluginMiner,
-	                              cuckoo_size: u32,
+		b: &mut Block,
-	                              head: &BlockHeader,
+		cuckoo_size: u32,
-	                              attempt_time_per_block: u32,
+		head: &BlockHeader,
-	                              latest_hash: &mut Hash)
+		attempt_time_per_block: u32,
-	                              -> Option<Proof> {
+		latest_hash: &mut Hash,
+	) -> Option<Proof> {
 		// look for a pow for at most attempt_time_per_block sec on the same block (to
 		// give a chance to new
 		// transactions) and as long as the head hasn't changed
@@ -322,7 +327,8 @@ impl Miner {
 
 			// Artificial slow down
 			if self.config.slow_down_in_millis != None &&
-			   self.config.slow_down_in_millis.unwrap() > 0 {
+				self.config.slow_down_in_millis.unwrap() > 0
+			{
 				thread::sleep(std::time::Duration::from_millis(
 					self.config.slow_down_in_millis.unwrap(),
 				));
@@ -342,14 +348,15 @@ impl Miner {
 	}
 
 	/// The inner part of mining loop for the internal miner
-	pub fn inner_loop_sync_internal<T: MiningWorker>(&self,
+	pub fn inner_loop_sync_internal<T: MiningWorker>(
-	                                                 miner: &mut T,
+		&self,
-	                                                 b: &mut Block,
+		miner: &mut T,
-	                                                 cuckoo_size: u32,
+		b: &mut Block,
-	                                                 head: &BlockHeader,
+		cuckoo_size: u32,
-	                                                 attempt_time_per_block: u32,
+		head: &BlockHeader,
-	                                                 latest_hash: &mut Hash)
+		attempt_time_per_block: u32,
-	                                                 -> Option<Proof> {
+		latest_hash: &mut Hash,
+	) -> Option<Proof> {
 		// look for a pow for at most 2 sec on the same block (to give a chance to new
 		// transactions) and as long as the head hasn't changed
 		let deadline = time::get_time().sec + attempt_time_per_block as i64;
@@ -392,7 +399,8 @@ impl Miner {
 
 			// Artificial slow down
 			if self.config.slow_down_in_millis != None &&
-			   self.config.slow_down_in_millis.unwrap() > 0 {
+				self.config.slow_down_in_millis.unwrap() > 0
+			{
 				thread::sleep(std::time::Duration::from_millis(
 					self.config.slow_down_in_millis.unwrap(),
 				));
@@ -422,16 +430,24 @@ impl Miner {
 		let mut plugin_miner = None;
 		let mut miner = None;
 		if miner_config.use_cuckoo_miner {
-			plugin_miner = Some(PluginMiner::new(consensus::EASINESS, cuckoo_size, proof_size));
+			plugin_miner = Some(PluginMiner::new(
+				consensus::EASINESS,
+				cuckoo_size,
+				proof_size,
+			));
 			plugin_miner.as_mut().unwrap().init(miner_config.clone());
 		} else {
-			miner = Some(cuckoo::Miner::new(consensus::EASINESS, cuckoo_size, proof_size));
+			miner = Some(cuckoo::Miner::new(
+				consensus::EASINESS,
+				cuckoo_size,
+				proof_size,
+			));
 		}
 
 		// to prevent the wallet from generating a new HD key derivation for each
 		// iteration, we keep the returned derivation to provide it back when
 		// nothing has changed
-		let mut pubkey = None;
+		let mut key_id = None;
 
 		loop {
 			debug!(LOGGER, "in miner loop...");
@@ -439,7 +455,7 @@ impl Miner {
 			// get the latest chain state and build a block on top of it
 			let head = self.chain.head_header().unwrap();
 			let mut latest_hash = self.chain.head().unwrap().last_block_h;
-			let (mut b, block_fees) = self.build_block(&head, pubkey);
+			let (mut b, block_fees) = self.build_block(&head, key_id);
 
 			let mut sol = None;
 			let mut use_async = false;
@@ -470,7 +486,7 @@ impl Miner {
 					);
 				}
 			}
-			if let Some(mut m) = miner.as_mut() {
+			if let Some(m) = miner.as_mut() {
 				sol = self.inner_loop_sync_internal(
 					m,
 					&mut b,
@@ -504,25 +520,26 @@ impl Miner {
 						e
 					);
 				}
-				debug!(LOGGER, "resetting pubkey in miner to None");
+				debug!(LOGGER, "resetting key_id in miner to None");
-				pubkey = None;
+				key_id = None;
 			} else {
 				debug!(
 					LOGGER,
 					"setting pubkey in miner to pubkey from block_fees - {:?}",
 					block_fees
 				);
-				pubkey = block_fees.pubkey();
+				key_id = block_fees.key_id();
 			}
 		}
 	}
 
 	/// Builds a new block with the chain head as previous and eligible
 	/// transactions from the pool.
-	fn build_block(&self,
+	fn build_block(
-	               head: &core::BlockHeader,
+		&self,
-	               pubkey: Option<Identifier>)
+		head: &core::BlockHeader,
-	               -> (core::Block, BlockFees) {
+		key_id: Option<Identifier>,
+	) -> (core::Block, BlockFees) {
 		// prepare the block header timestamp
 		let mut now_sec = time::get_time().sec;
 		let head_sec = head.timestamp.to_timespec().sec;
@@ -535,16 +552,19 @@ impl Miner {
 		let difficulty = consensus::next_difficulty(diff_iter).unwrap();
 
 		// extract current transaction from the pool
-		let txs_box = self.tx_pool
+		let txs_box = self.tx_pool.read().unwrap().prepare_mineable_transactions(
-			.read()
+			MAX_TX,
-			.unwrap()
+		);
-			.prepare_mineable_transactions(MAX_TX);
 		let txs: Vec<&Transaction> = txs_box.iter().map(|tx| tx.as_ref()).collect();
 
 		// build the coinbase and the block itself
 		let fees = txs.iter().map(|tx| tx.fee).sum();
 		let height = head.height + 1;
-		let block_fees = BlockFees { fees, pubkey, height };
+		let block_fees = BlockFees {
+			fees,
+			key_id,
+			height,
+		};
 
 		let (output, kernel, block_fees) = self.get_coinbase(block_fees);
 		let mut b = core::Block::with_reward(head, txs, output, kernel).unwrap();
@@ -565,21 +585,18 @@ impl Miner {
 		b.header.nonce = rng.gen();
 		b.header.difficulty = difficulty;
 		b.header.timestamp = time::at_utc(time::Timespec::new(now_sec, 0));
-		self.chain
+		self.chain.set_sumtree_roots(&mut b).expect(
-			.set_sumtree_roots(&mut b)
+			"Error setting sum tree roots",
-			.expect("Error setting sum tree roots");
+		);
 		(b, block_fees)
 	}
 
 	fn get_coinbase(&self, block_fees: BlockFees) -> (core::Output, core::TxKernel, BlockFees) {
 		if self.config.burn_reward {
 			let keychain = Keychain::from_random_seed().unwrap();
-			let pubkey = keychain.derive_pubkey(1).unwrap();
+			let key_id = keychain.derive_key_id(1).unwrap();
-			let (out, kern) = core::Block::reward_output(
+			let (out, kern) = core::Block::reward_output(&keychain, &key_id, block_fees.fees)
-				&keychain,
+				.unwrap();
-				&pubkey,
-				block_fees.fees,
-			).unwrap();
 			(out, kern, block_fees)
 		} else {
 			let url = format!(
@@ -596,12 +613,12 @@ impl Miner {
 			);
 			let out_bin = util::from_hex(res.output).unwrap();
 			let kern_bin = util::from_hex(res.kernel).unwrap();
-			let pubkey_bin = util::from_hex(res.pubkey).unwrap();
+			let key_id_bin = util::from_hex(res.key_id).unwrap();
 			let output = ser::deserialize(&mut &out_bin[..]).unwrap();
 			let kernel = ser::deserialize(&mut &kern_bin[..]).unwrap();
-			let pubkey = ser::deserialize(&mut &pubkey_bin[..]).unwrap();
+			let key_id = ser::deserialize(&mut &key_id_bin[..]).unwrap();
 			let block_fees = BlockFees {
-				pubkey: Some(pubkey),
+				key_id: Some(key_id),
 				..block_fees
 			};
 

keychain/src/blind.rs

@@ -40,8 +40,8 @@ impl BlindingFactor {
 /// Accumulator to compute the sum of blinding factors. Keeps track of each
 /// factor as well as the "sign" with which they should be combined.
 pub struct BlindSum {
-	pub positive_pubkeys: Vec<Identifier>,
+	pub positive_key_ids: Vec<Identifier>,
-	pub negative_pubkeys: Vec<Identifier>,
+	pub negative_key_ids: Vec<Identifier>,
 	pub positive_blinding_factors: Vec<BlindingFactor>,
 	pub negative_blinding_factors: Vec<BlindingFactor>,
 }
@@ -50,20 +50,20 @@ impl BlindSum {
 	/// Creates a new blinding factor sum.
 	pub fn new() -> BlindSum {
 		BlindSum {
-			positive_pubkeys: vec![],
+			positive_key_ids: vec![],
-			negative_pubkeys: vec![],
+			negative_key_ids: vec![],
 			positive_blinding_factors: vec![],
 			negative_blinding_factors: vec![],
 		}
 	}
 
-	pub fn add_pubkey(mut self, pubkey: Identifier) -> BlindSum {
+	pub fn add_key_id(mut self, key_id: Identifier) -> BlindSum {
-		self.positive_pubkeys.push(pubkey);
+		self.positive_key_ids.push(key_id);
 		self
 	}
 
-	pub fn sub_pubkey(mut self, pubkey: Identifier) -> BlindSum {
+	pub fn sub_key_id(mut self, key_id: Identifier) -> BlindSum {
-		self.negative_pubkeys.push(pubkey);
+		self.negative_key_ids.push(key_id);
 		self
 	}
 

keychain/src/extkey.rs

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::{error, fmt};
+use std::{error, fmt, num};
 use std::cmp::min;
 
 use serde::{de, ser};
@@ -24,14 +24,18 @@ use secp::Secp256k1;
 use secp::key::{PublicKey, SecretKey};
 use util;
 
+// Size of an identifier in bytes
+pub const IDENTIFIER_SIZE: usize = 10;
+
 /// An ExtKey error
-#[derive(Copy, PartialEq, Eq, Clone, Debug)]
+#[derive(PartialEq, Eq, Clone, Debug)]
 pub enum Error {
 	/// The size of the seed is invalid
 	InvalidSeedSize,
 	InvalidSliceSize,
 	InvalidExtendedKey,
 	Secp(secp::Error),
+	ParseIntError(num::ParseIntError),
 }
 
 impl From<secp::Error> for Error {
@@ -40,6 +44,12 @@ impl From<secp::Error> for Error {
 	}
 }
 
+impl From<num::ParseIntError> for Error {
+	fn from(e: num::ParseIntError) -> Error {
+		Error::ParseIntError(e)
+	}
+}
+
 // Passthrough Debug to Display, since errors should be user-visible
 impl fmt::Display for Error {
 	fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
@@ -59,35 +69,13 @@ impl error::Error for Error {
 			Error::InvalidSliceSize => "keychain: serialized extended key must be of size 73",
 			Error::InvalidExtendedKey => "keychain: the given serialized extended key is invalid",
 			Error::Secp(_) => "keychain: secp error",
+			Error::ParseIntError(_) => "keychain: error parsing int",
 		}
 	}
 }
 
-#[derive(Serialize, Deserialize, Clone, PartialEq, Eq, Debug, Hash)]
-pub struct Fingerprint(String);
-
-impl Fingerprint {
-	fn zero() -> Fingerprint {
-		Identifier::from_bytes(&[0; 4]).fingerprint()
-	}
-
-	fn from_bytes(bytes: &[u8]) -> Fingerprint {
-		let mut fingerprint = [0; 4];
-		for i in 0..min(4, bytes.len()) {
-			fingerprint[i] = bytes[i];
-		}
-		Fingerprint(util::to_hex(fingerprint.to_vec()))
-	}
-}
-
-impl fmt::Display for Fingerprint {
-	fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
-		f.write_str(&self.0)
-	}
-}
-
 #[derive(Clone, PartialEq, Eq, Hash)]
-pub struct Identifier([u8; 20]);
+pub struct Identifier([u8; IDENTIFIER_SIZE]);
 
 impl ser::Serialize for Identifier {
 	fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
@@ -127,33 +115,32 @@ impl<'de> de::Visitor<'de> for IdentifierVisitor {
 }
 
 impl Identifier {
+	pub fn zero() -> Identifier {
+		Identifier::from_bytes(&[0; IDENTIFIER_SIZE])
+	}
+
 	pub fn from_bytes(bytes: &[u8]) -> Identifier {
-		let mut identifier = [0; 20];
+		let mut identifier = [0; IDENTIFIER_SIZE];
-		for i in 0..min(20, bytes.len()) {
+		for i in 0..min(IDENTIFIER_SIZE, bytes.len()) {
 			identifier[i] = bytes[i];
 		}
 		Identifier(identifier)
 	}
 
-	pub fn from_pubkey(secp: &Secp256k1, pubkey: &PublicKey) -> Identifier {
+	pub fn from_key_id(secp: &Secp256k1, pubkey: &PublicKey) -> Identifier {
 		let bytes = pubkey.serialize_vec(secp, true);
-		let identifier = blake2b(20, &[], &bytes[..]);
+		let identifier = blake2b(IDENTIFIER_SIZE, &[], &bytes[..]);
 		Identifier::from_bytes(&identifier.as_bytes())
 	}
 
 	fn from_hex(hex: &str) -> Result<Identifier, Error> {
-		// TODO - error handling, don't unwrap here
+		let bytes = util::from_hex(hex.to_string())?;
-		let bytes = util::from_hex(hex.to_string()).unwrap();
 		Ok(Identifier::from_bytes(&bytes))
 	}
 
 	pub fn to_hex(&self) -> String {
 		util::to_hex(self.0.to_vec())
 	}
-
-	pub fn fingerprint(&self) -> Fingerprint {
-		Fingerprint::from_bytes(&self.0)
-	}
 }
 
 impl AsRef<[u8]> for Identifier {
@@ -165,13 +152,17 @@ impl AsRef<[u8]> for Identifier {
 impl ::std::fmt::Debug for Identifier {
 	fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
 		try!(write!(f, "{}(", stringify!(Identifier)));
-		for i in self.0.iter().cloned() {
+		try!(write!(f, "{}", self.to_hex()));
-			try!(write!(f, "{:02x}", i));
-		}
 		write!(f, ")")
 	}
 }
 
+impl fmt::Display for Identifier {
+	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+		write!(f, "{}", self.to_hex())
+	}
+}
+
 /// An ExtendedKey is a secret key which can be used to derive new
 /// secret keys to blind the commitment of a transaction output.
 /// To be usable, a secret key should have an amount assigned to it,
@@ -183,8 +174,8 @@ pub struct ExtendedKey {
 	pub depth: u8,
 	/// Child number of the key
 	pub n_child: u32,
-	/// Parent key's fingerprint
+	/// Root key identifier
-	pub fingerprint: Fingerprint,
+	pub root_key_id: Identifier,
 	/// Code of the derivation chain
 	pub chaincode: [u8; 32],
 	/// Actual private key
@@ -195,25 +186,25 @@ impl ExtendedKey {
 	/// Creates a new extended key from a serialized one
 	pub fn from_slice(secp: &Secp256k1, slice: &[u8]) -> Result<ExtendedKey, Error> {
 		// TODO change when ser. ext. size is fixed
-		if slice.len() != 73 {
+		if slice.len() != 79 {
 			return Err(Error::InvalidSliceSize);
 		}
 		let depth: u8 = slice[0];
-		let fingerprint = Fingerprint::from_bytes(&slice[1..5]);
+		let root_key_id = Identifier::from_bytes(&slice[1..11]);
-		let n_child = BigEndian::read_u32(&slice[5..9]);
+		let n_child = BigEndian::read_u32(&slice[11..15]);
 		let mut chaincode: [u8; 32] = [0; 32];
-		(&mut chaincode).copy_from_slice(&slice[9..41]);
+		(&mut chaincode).copy_from_slice(&slice[15..47]);
-		let secret_key = match SecretKey::from_slice(secp, &slice[41..73]) {
+		let key = match SecretKey::from_slice(secp, &slice[47..79]) {
 			Ok(key) => key,
 			Err(_) => return Err(Error::InvalidExtendedKey),
 		};
 
 		Ok(ExtendedKey {
-			depth: depth,
+			depth,
-			fingerprint: fingerprint,
+			root_key_id,
-			n_child: n_child,
+			n_child,
-			chaincode: chaincode,
+			chaincode,
-			key: secret_key,
+			key,
 		})
 	}
 
@@ -234,24 +225,23 @@ impl ExtendedKey {
 
 		let mut ext_key = ExtendedKey {
 			depth: 0,
-			fingerprint: Fingerprint::zero(),
+			root_key_id: Identifier::zero(),
 			n_child: 0,
 			chaincode: chaincode,
 			key: secret_key,
 		};
 
-		let identifier = ext_key.identifier(secp)?;
+		ext_key.root_key_id = ext_key.identifier(secp)?;
-		ext_key.fingerprint = identifier.fingerprint();
 
 		Ok(ext_key)
 	}
 
 	/// Return the identifier of the key
-	/// which is the blake2b hash (20 byte digest) of the PublicKey
+	/// which is the blake2b (10 byte) digest of the PublicKey
 	// corresponding to the underlying SecretKey
 	pub fn identifier(&self, secp: &Secp256k1) -> Result<Identifier, Error> {
-		let pubkey = PublicKey::from_secret_key(secp, &self.key)?;
+		let key_id = PublicKey::from_secret_key(secp, &self.key)?;
-		Ok(Identifier::from_pubkey(secp, &pubkey))
+		Ok(Identifier::from_key_id(secp, &key_id))
 	}
 
 	/// Derive an extended key from an extended key
@@ -273,11 +263,9 @@ impl ExtendedKey {
 		let mut chain_code: [u8; 32] = [0; 32];
 		(&mut chain_code).clone_from_slice(&derived.as_bytes()[32..]);
 
-		let identifier = self.identifier(&secp)?;
-
 		Ok(ExtendedKey {
 			depth: self.depth + 1,
-			fingerprint: identifier.fingerprint(),
+			root_key_id: self.identifier(&secp)?,
 			n_child: n,
 			chaincode: chain_code,
 			key: secret_key,
@@ -291,7 +279,7 @@ mod test {
 
 	use secp::Secp256k1;
 	use secp::key::SecretKey;
-	use super::{ExtendedKey, Fingerprint, Identifier};
+	use super::{ExtendedKey, Identifier};
 	use util;
 
 	fn from_hex(hex_str: &str) -> Vec<u8> {
@@ -312,10 +300,7 @@ mod test {
 
 		let json = serde_json::to_string(&has_an_identifier).unwrap();
 
-		assert_eq!(
+		assert_eq!(json, "{\"identifier\":\"942b6c0bd43bdcb24f3e\"}");
-			json,
-			"{\"identifier\":\"942b6c0bd43bdcb24f3edfe7fadbc77054ecc4f2\"}"
-		);
 
 		let deserialized: HasAnIdentifier = serde_json::from_str(&json).unwrap();
 		assert_eq!(deserialized, has_an_identifier);
@@ -334,8 +319,7 @@ mod test {
 		let chaincode = from_hex(
 			"e7298e68452b0c6d54837670896e1aee76b118075150d90d4ee416ece106ae72",
 		);
-		let identifier = from_hex("d291fc2dca90fc8b005a01638d616fda770ec552");
+		let identifier = from_hex("83e59c48297b78b34b73");
-		let fingerprint = from_hex("d291fc2d");
 		let depth = 0;
 		let n_child = 0;
 		assert_eq!(extk.key, secret_key);
@@ -344,12 +328,8 @@ mod test {
 			Identifier::from_bytes(identifier.as_slice())
 		);
 		assert_eq!(
-			extk.fingerprint,
+			extk.root_key_id,
-			Fingerprint::from_bytes(fingerprint.as_slice())
+			Identifier::from_bytes(identifier.as_slice())
-		);
-		assert_eq!(
-			extk.identifier(&s).unwrap().fingerprint(),
-			Fingerprint::from_bytes(fingerprint.as_slice())
 		);
 		assert_eq!(extk.chaincode, chaincode.as_slice());
 		assert_eq!(extk.depth, depth);
@@ -370,9 +350,8 @@ mod test {
 		let chaincode = from_hex(
 			"243cb881e1549e714db31d23af45540b13ad07941f64a786bbf3313b4de1df52",
 		);
-		let fingerprint = from_hex("d291fc2d");
+		let root_key_id = from_hex("83e59c48297b78b34b73");
-		let identifier = from_hex("027a8e290736af382fc943bdabb774bc2d14fd95");
+		let identifier = from_hex("0185adb4d8b730099c93");
-		let identifier_fingerprint = from_hex("027a8e29");
 		let depth = 1;
 		let n_child = 0;
 		assert_eq!(derived.key, secret_key);
@@ -381,12 +360,8 @@ mod test {
 			Identifier::from_bytes(identifier.as_slice())
 		);
 		assert_eq!(
-			derived.fingerprint,
+			derived.root_key_id,
-			Fingerprint::from_bytes(fingerprint.as_slice())
+			Identifier::from_bytes(root_key_id.as_slice())
-		);
-		assert_eq!(
-			derived.identifier(&s).unwrap().fingerprint(),
-			Fingerprint::from_bytes(identifier_fingerprint.as_slice())
 		);
 		assert_eq!(derived.chaincode, chaincode.as_slice());
 		assert_eq!(derived.depth, depth);

keychain/src/keychain.rs

@@ -19,9 +19,8 @@ use secp::{Message, Secp256k1, Signature};
 use secp::key::SecretKey;
 use secp::pedersen::{Commitment, ProofMessage, ProofInfo, RangeProof};
 use blake2;
-
 use blind::{BlindingFactor, BlindSum};
-use extkey::{self, Fingerprint, Identifier};
+use extkey::{self, Identifier};
 
 
 #[derive(PartialEq, Eq, Clone, Debug)]
@@ -55,8 +54,8 @@ pub struct Keychain {
 }
 
 impl Keychain {
-	pub fn fingerprint(&self) -> Fingerprint {
+	pub fn root_key_id(&self) -> Identifier {
-		self.extkey.fingerprint.clone()
+		self.extkey.root_key_id.clone()
 	}
 
 	pub fn from_seed(seed: &[u8]) -> Result<Keychain, Error> {
@@ -77,52 +76,52 @@ impl Keychain {
 		Keychain::from_seed(seed.as_bytes())
 	}
 
-	pub fn derive_pubkey(&self, derivation: u32) -> Result<Identifier, Error> {
+	pub fn derive_key_id(&self, derivation: u32) -> Result<Identifier, Error> {
 		let extkey = self.extkey.derive(&self.secp, derivation)?;
-		let pubkey = extkey.identifier(&self.secp)?;
+		let key_id = extkey.identifier(&self.secp)?;
-		Ok(pubkey)
+		Ok(key_id)
 	}
 
 	// TODO - this is a work in progress
 	// TODO - smarter lookups - can we cache key_id/fingerprint -> derivation
 	// number somehow?
-	fn derived_key(&self, pubkey: &Identifier) -> Result<SecretKey, Error> {
+	fn derived_key(&self, key_id: &Identifier) -> Result<SecretKey, Error> {
 		if self.enable_burn_key {
 			// for tests and burn only, associate the zero fingerprint to a known
 			// dummy private key
-			if pubkey.fingerprint().to_string() == "00000000" {
+			if *key_id == Identifier::zero() {
 				return Ok(SecretKey::from_slice(&self.secp, &[1; 32])?);
 			}
 		}
 		for i in 1..10000 {
 			let extkey = self.extkey.derive(&self.secp, i)?;
-			if extkey.identifier(&self.secp)? == *pubkey {
+			if extkey.identifier(&self.secp)? == *key_id {
 				return Ok(extkey.key);
 			}
 		}
-		Err(Error::KeyDerivation(format!("cannot find extkey for {}", pubkey.fingerprint())))
+		Err(Error::KeyDerivation(format!("cannot find extkey for {:?}", key_id)))
 	}
 
 	// TODO - clean this and derived_key up, rename them?
-	// TODO - maybe wallet deals exclusively with pubkeys and not derivations - this leaks?
+	// TODO - maybe wallet deals exclusively with key_ids and not derivations - this leaks?
-	pub fn derivation_from_pubkey(&self, pubkey: &Identifier) -> Result<u32, Error> {
+	pub fn derivation_from_key_id(&self, key_id: &Identifier) -> Result<u32, Error> {
 		for i in 1..10000 {
 			let extkey = self.extkey.derive(&self.secp, i)?;
-			if extkey.identifier(&self.secp)? == *pubkey {
+			if extkey.identifier(&self.secp)? == *key_id {
 				return Ok(extkey.n_child);
 			}
 		}
-		Err(Error::KeyDerivation(format!("cannot find extkey for {}", pubkey.fingerprint())))
+		Err(Error::KeyDerivation(format!("cannot find extkey for {:?}", key_id)))
 	}
 
-	pub fn commit(&self, amount: u64, pubkey: &Identifier) -> Result<Commitment, Error> {
+	pub fn commit(&self, amount: u64, key_id: &Identifier) -> Result<Commitment, Error> {
-		let skey = self.derived_key(pubkey)?;
+		let skey = self.derived_key(key_id)?;
 		let commit = self.secp.commit(amount, skey)?;
 		Ok(commit)
 	}
 
-	pub fn switch_commit(&self, pubkey: &Identifier) -> Result<Commitment, Error> {
+	pub fn switch_commit(&self, key_id: &Identifier) -> Result<Commitment, Error> {
-		let skey = self.derived_key(pubkey)?;
+		let skey = self.derived_key(key_id)?;
 		let commit = self.secp.switch_commit(skey)?;
 		Ok(commit)
 	}
@@ -130,34 +129,34 @@ impl Keychain {
 	pub fn range_proof(
 		&self,
 		amount: u64,
-		pubkey: &Identifier,
+		key_id: &Identifier,
 		commit: Commitment,
 		msg: ProofMessage,
 	) -> Result<RangeProof, Error> {
-		let skey = self.derived_key(pubkey)?;
+		let skey = self.derived_key(key_id)?;
 		let range_proof = self.secp.range_proof(0, amount, skey, commit, msg);
 		Ok(range_proof)
 	}
 
 	pub fn rewind_range_proof(
 		&self,
-		pubkey: &Identifier,
+		key_id: &Identifier,
 		commit: Commitment,
 		proof: RangeProof,
 	) -> Result<ProofInfo, Error> {
-		let nonce = self.derived_key(pubkey)?;
+		let nonce = self.derived_key(key_id)?;
 		Ok(self.secp.rewind_range_proof(commit, proof, nonce))
 	}
 
 	pub fn blind_sum(&self, blind_sum: &BlindSum) -> Result<BlindingFactor, Error> {
 		let mut pos_keys: Vec<SecretKey> = blind_sum
-			.positive_pubkeys
+			.positive_key_ids
 			.iter()
 			.filter_map(|k| self.derived_key(&k).ok())
 			.collect();
 
 		let mut neg_keys: Vec<SecretKey> = blind_sum
-			.negative_pubkeys
+			.negative_key_ids
 			.iter()
 			.filter_map(|k| self.derived_key(&k).ok())
 			.collect();
@@ -178,8 +177,8 @@ impl Keychain {
 		Ok(BlindingFactor::new(blinding))
 	}
 
-	pub fn sign(&self, msg: &Message, pubkey: &Identifier) -> Result<Signature, Error> {
+	pub fn sign(&self, msg: &Message, key_id: &Identifier) -> Result<Signature, Error> {
-		let skey = self.derived_key(pubkey)?;
+		let skey = self.derived_key(key_id)?;
 		let sig = self.secp.sign(msg, &skey)?;
 		Ok(sig)
 	}
@@ -209,56 +208,57 @@ mod test {
 		let secp = secp::Secp256k1::with_caps(secp::ContextFlag::Commit);
 		let keychain = Keychain::from_random_seed().unwrap();
 
-		// use the keychain to derive a "pubkey" based on the underlying seed
+		// use the keychain to derive a "key_id" based on the underlying seed
-		let pubkey = keychain.derive_pubkey(1).unwrap();
+		let key_id = keychain.derive_key_id(1).unwrap();
 
 		let msg_bytes = [0; 32];
 		let msg = secp::Message::from_slice(&msg_bytes[..]).unwrap();
 
 		// now create a zero commitment using the key on the keychain associated with
-		// the pubkey
+		// the key_id
-		let commit = keychain.commit(0, &pubkey).unwrap();
+		let commit = keychain.commit(0, &key_id).unwrap();
 
 		// now check we can use our key to verify a signature from this zero commitment
-		let sig = keychain.sign(&msg, &pubkey).unwrap();
+		let sig = keychain.sign(&msg, &key_id).unwrap();
 		secp.verify_from_commit(&msg, &sig, &commit).unwrap();
 	}
 
 	#[test]
 	fn test_rewind_range_proof() {
 		let keychain = Keychain::from_random_seed().unwrap();
-		let pubkey = keychain.derive_pubkey(1).unwrap();
+		let key_id = keychain.derive_key_id(1).unwrap();
-		let commit = keychain.commit(5, &pubkey).unwrap();
+		let commit = keychain.commit(5, &key_id).unwrap();
 		let msg = ProofMessage::empty();
 
-		let proof = keychain.range_proof(5, &pubkey, commit, msg).unwrap();
+		let proof = keychain.range_proof(5, &key_id, commit, msg).unwrap();
-		let proof_info = keychain.rewind_range_proof(&pubkey, commit, proof).unwrap();
+		let proof_info = keychain.rewind_range_proof(&key_id, commit, proof).unwrap();
 
 		assert_eq!(proof_info.success, true);
 		assert_eq!(proof_info.value, 5);
 
-		// now check the recovered message is "empty" (but not truncated) i.e. all zeroes
+		// now check the recovered message is "empty" (but not truncated) i.e. all
+		// zeroes
 		assert_eq!(
 			proof_info.message,
 			secp::pedersen::ProofMessage::from_bytes(&[0; secp::constants::PROOF_MSG_SIZE])
 		);
 
-		let pubkey2 = keychain.derive_pubkey(2).unwrap();
+		let key_id2 = keychain.derive_key_id(2).unwrap();
 
 		// cannot rewind with a different nonce
-		let proof_info = keychain.rewind_range_proof(&pubkey2, commit, proof).unwrap();
+		let proof_info = keychain.rewind_range_proof(&key_id2, commit, proof).unwrap();
 		assert_eq!(proof_info.success, false);
 		assert_eq!(proof_info.value, 0);
 
 		// cannot rewind with a commitment to the same value using a different key
-		let commit2 = keychain.commit(5, &pubkey2).unwrap();
+		let commit2 = keychain.commit(5, &key_id2).unwrap();
-		let proof_info = keychain.rewind_range_proof(&pubkey, commit2, proof).unwrap();
+		let proof_info = keychain.rewind_range_proof(&key_id, commit2, proof).unwrap();
 		assert_eq!(proof_info.success, false);
 		assert_eq!(proof_info.value, 0);
 
 		// cannot rewind with a commitment to a different value
-		let commit3 = keychain.commit(4, &pubkey).unwrap();
+		let commit3 = keychain.commit(4, &key_id).unwrap();
-		let proof_info = keychain.rewind_range_proof(&pubkey, commit3, proof).unwrap();
+		let proof_info = keychain.rewind_range_proof(&key_id, commit3, proof).unwrap();
 		assert_eq!(proof_info.success, false);
 		assert_eq!(proof_info.value, 0);
 	}

keychain/src/lib.rs

@@ -28,6 +28,6 @@ mod blind;
 mod extkey;
 
 pub use blind::{BlindSum, BlindingFactor};
-pub use extkey::{Identifier, Fingerprint, ExtendedKey};
+pub use extkey::{Identifier, ExtendedKey, IDENTIFIER_SIZE};
 pub mod keychain;
 pub use keychain::{Error, Keychain};

pool/src/graph.rs

@@ -254,21 +254,21 @@ mod tests {
 	#[test]
 	fn test_add_entry() {
 		let keychain = Keychain::from_random_seed().unwrap();
-		let pk1 = keychain.derive_pubkey(1).unwrap();
+		let key_id1 = keychain.derive_key_id(1).unwrap();
-		let pk2 = keychain.derive_pubkey(2).unwrap();
+		let key_id2 = keychain.derive_key_id(2).unwrap();
-		let pk3 = keychain.derive_pubkey(3).unwrap();
+		let key_id3 = keychain.derive_key_id(3).unwrap();
 
-		let output_commit = keychain.commit(70, &pk1).unwrap();
+		let output_commit = keychain.commit(70, &key_id1).unwrap();
 		let inputs = vec![
-			core::transaction::Input(keychain.commit(50, &pk2).unwrap()),
+			core::transaction::Input(keychain.commit(50, &key_id2).unwrap()),
-			core::transaction::Input(keychain.commit(25, &pk3).unwrap()),
+			core::transaction::Input(keychain.commit(25, &key_id3).unwrap()),
 		];
 		let msg = secp::pedersen::ProofMessage::empty();
 		let outputs = vec![
 			core::transaction::Output {
 				features: core::transaction::DEFAULT_OUTPUT,
 				commit: output_commit,
-				proof: keychain.range_proof(100, &pk1, output_commit, msg).unwrap(),
+				proof: keychain.range_proof(100, &key_id1, output_commit, msg).unwrap(),
 			},
 		];
 		let test_transaction = core::transaction::Transaction::new(inputs, outputs, 5, 0);

pool/src/pool.rs

@@ -968,13 +968,13 @@ mod tests {
 		let block_transactions = vec![&block_tx_1, &block_tx_2, &block_tx_3, &block_tx_4];
 
 		let keychain = Keychain::from_random_seed().unwrap();
-		let pubkey = keychain.derive_pubkey(1).unwrap();
+		let key_id = keychain.derive_key_id(1).unwrap();
 
 		let block = block::Block::new(
 			&block::BlockHeader::default(),
 			block_transactions,
 			&keychain,
-			&pubkey,
+			&key_id,
 		).unwrap();
 
 		chain_ref.apply_block(&block);
@@ -1077,8 +1077,8 @@ mod tests {
 			let tx_refs = block_txs.iter().collect();
 
 			let keychain = Keychain::from_random_seed().unwrap();
-			let pubkey = keychain.derive_pubkey(1).unwrap();
+			let key_id = keychain.derive_key_id(1).unwrap();
-			block = block::Block::new(&block::BlockHeader::default(), tx_refs, &keychain, &pubkey)
+			block = block::Block::new(&block::BlockHeader::default(), tx_refs, &keychain, &key_id)
 				.unwrap();
 		}
 
@@ -1130,13 +1130,13 @@ mod tests {
 		let mut tx_elements = Vec::new();
 
 		for input_value in input_values {
-			let pubkey = keychain.derive_pubkey(input_value as u32).unwrap();
+			let key_id = keychain.derive_key_id(input_value as u32).unwrap();
-			tx_elements.push(build::input(input_value, pubkey));
+			tx_elements.push(build::input(input_value, key_id));
 		}
 
 		for output_value in output_values {
-			let pubkey = keychain.derive_pubkey(output_value as u32).unwrap();
+			let key_id = keychain.derive_key_id(output_value as u32).unwrap();
-			tx_elements.push(build::output(output_value, pubkey));
+			tx_elements.push(build::output(output_value, key_id));
 		}
 		tx_elements.push(build::with_fee(fees as u64));
 
@@ -1158,13 +1158,13 @@ mod tests {
 		let mut tx_elements = Vec::new();
 
 		for input_value in input_values {
-			let pubkey = keychain.derive_pubkey(input_value as u32).unwrap();
+			let key_id = keychain.derive_key_id(input_value as u32).unwrap();
-			tx_elements.push(build::input(input_value, pubkey));
+			tx_elements.push(build::input(input_value, key_id));
 		}
 
 		for output_value in output_values {
-			let pubkey = keychain.derive_pubkey(output_value as u32).unwrap();
+			let key_id = keychain.derive_key_id(output_value as u32).unwrap();
-			tx_elements.push(build::output(output_value, pubkey));
+			tx_elements.push(build::output(output_value, key_id));
 		}
 		tx_elements.push(build::with_fee(fees as u64));
 
@@ -1176,10 +1176,10 @@ mod tests {
 	/// Deterministically generate an output defined by our test scheme
 	fn test_output(value: u64) -> transaction::Output {
 		let keychain = keychain_for_tests();
-		let pubkey = keychain.derive_pubkey(value as u32).unwrap();
+		let key_id = keychain.derive_key_id(value as u32).unwrap();
-		let commit = keychain.commit(value, &pubkey).unwrap();
+		let commit = keychain.commit(value, &key_id).unwrap();
 		let msg = secp::pedersen::ProofMessage::empty();
-		let proof = keychain.range_proof(value, &pubkey, commit, msg).unwrap();
+		let proof = keychain.range_proof(value, &key_id, commit, msg).unwrap();
 
 		transaction::Output {
 			features: transaction::DEFAULT_OUTPUT,
@@ -1191,10 +1191,10 @@ mod tests {
 	/// Deterministically generate a coinbase output defined by our test scheme
 	fn test_coinbase_output(value: u64) -> transaction::Output {
 		let keychain = keychain_for_tests();
-		let pubkey = keychain.derive_pubkey(value as u32).unwrap();
+		let key_id = keychain.derive_key_id(value as u32).unwrap();
-		let commit = keychain.commit(value, &pubkey).unwrap();
+		let commit = keychain.commit(value, &key_id).unwrap();
 		let msg = secp::pedersen::ProofMessage::empty();
-		let proof = keychain.range_proof(value, &pubkey, commit, msg).unwrap();
+		let proof = keychain.range_proof(value, &key_id, commit, msg).unwrap();
 
 		transaction::Output {
 			features: transaction::COINBASE_OUTPUT,

src/bin/grin.rs

@@ -42,7 +42,7 @@ use config::GlobalConfig;
 use wallet::WalletConfig;
 use core::global;
 use keychain::Keychain;
-use util::{LOGGER,init_logger};
+use util::{LOGGER, init_logger};
 
 fn start_from_config_file(mut global_config: GlobalConfig) {
 	info!(
@@ -85,7 +85,7 @@ fn main() {
 	});
 
 	if global_config.using_config_file {
-		//initialise the logger
+		// initialise the logger
 		init_logger(global_config.members.as_mut().unwrap().logging.clone());
 		info!(
 			LOGGER,

wallet/src/checker.rs

@@ -44,10 +44,10 @@ pub fn refresh_outputs(config: &WalletConfig, keychain: &Keychain) -> Result<(),
 
 	WalletData::with_wallet(&config.data_file_dir, |wallet_data| {
 		// check each output that's not spent
-		for mut out in wallet_data
+		for mut out in wallet_data.outputs.values_mut().filter(|out| {
-		        .outputs
+			out.status != OutputStatus::Spent
-		        .values_mut()
+		})
-		        .filter(|out| out.status != OutputStatus::Spent) {
+		{
 			// TODO check the pool for unconfirmed
 			match get_output_from_node(config, keychain, out.value, out.n_child) {
 				Ok(api_out) => refresh_output(&mut out, api_out, &tip),
@@ -66,7 +66,8 @@ pub fn get_tip_from_node(config: &WalletConfig) -> Result<api::Tip, Error> {
 	api::client::get::<api::Tip>(url.as_str()).map_err(|e| Error::Node(e))
 }
 
-// queries a reachable node for a given output, checking whether it's been confirmed
+// queries a reachable node for a given output, checking whether it's been
+// confirmed
 fn get_output_from_node(
 	config: &WalletConfig,
 	keychain: &Keychain,
@@ -74,8 +75,8 @@ fn get_output_from_node(
 	derivation: u32,
 ) -> Result<Option<api::Output>, Error> {
 	// do we want to store these commitments in wallet.dat?
-	let pubkey = keychain.derive_pubkey(derivation)?;
+	let key_id = keychain.derive_key_id(derivation)?;
-	let commit = keychain.commit(amount, &pubkey)?;
+	let commit = keychain.commit(amount, &key_id)?;
 
 	let url = format!(
 		"{}/v1/chain/utxo/{}",

wallet/src/info.rs

@@ -17,26 +17,26 @@ use keychain::Keychain;
 use types::{WalletConfig, WalletData};
 
 pub fn show_info(config: &WalletConfig, keychain: &Keychain) {
-	let fingerprint = keychain.clone().fingerprint();
+	let root_key_id = keychain.root_key_id();
 	let _ = checker::refresh_outputs(&config, &keychain);
 
 	// operate within a lock on wallet data
 	let _ = WalletData::with_wallet(&config.data_file_dir, |wallet_data| {
 
 		println!("Outputs - ");
-		println!("identifier, height, lock_height, status, value");
+		println!("key_id, height, lock_height, status, value");
 		println!("----------------------------------");
 
 		let mut outputs = wallet_data
 			.outputs
 			.values()
-			.filter(|out| out.fingerprint == fingerprint)
+			.filter(|out| out.root_key_id == root_key_id)
 			.collect::<Vec<_>>();
 		outputs.sort_by_key(|out| out.n_child);
 		for out in outputs {
 			println!(
-				"{}..., {}, {}, {:?}, {}",
+				"{}, {}, {}, {:?}, {}",
-				out.identifier.fingerprint(),
+				out.key_id,
 				out.height,
 				out.lock_height,
 				out.status,

wallet/src/receiver.rs

@@ -111,24 +111,21 @@ impl ApiEndpoint for WalletReceiver {
 							&self.keychain,
 							&cb_fees,
 						).map_err(|e| {
-								api::Error::Internal(format!("Error building coinbase: {:?}", e))
+							api::Error::Internal(format!("Error building coinbase: {:?}", e))
-							})?;
+						})?;
-						let out_bin = ser::ser_vec(&out)
+						let out_bin = ser::ser_vec(&out).map_err(|e| {
-							.map_err(|e| {
+							api::Error::Internal(format!("Error serializing output: {:?}", e))
-								api::Error::Internal(format!("Error serializing output: {:?}", e))
+						})?;
-							})?;
+						let kern_bin = ser::ser_vec(&kern).map_err(|e| {
-						let kern_bin = ser::ser_vec(&kern)
+							api::Error::Internal(format!("Error serializing kernel: {:?}", e))
-							.map_err(|e| {
+						})?;
-								api::Error::Internal(format!("Error serializing kernel: {:?}", e))
+						let key_id_bin = match block_fees.key_id {
-							})?;
+							Some(key_id) => {
-						let pubkey_bin = match block_fees.pubkey {
+								ser::ser_vec(&key_id).map_err(|e| {
-							Some(pubkey) => {
+									api::Error::Internal(
-								ser::ser_vec(&pubkey)
+										format!("Error serializing kernel: {:?}", e),
-									.map_err(|e| {
+									)
-										api::Error::Internal(
+								})?
-											format!("Error serializing kernel: {:?}", e),
-										)
-									})?
 							}
 							None => vec![],
 						};
@@ -136,7 +133,7 @@ impl ApiEndpoint for WalletReceiver {
 						Ok(CbData {
 							output: util::to_hex(out_bin),
 							kernel: util::to_hex(kern_bin),
-							pubkey: util::to_hex(pubkey_bin),
+							key_id: util::to_hex(key_id_bin),
 						})
 					}
 					_ => Err(api::Error::Argument(format!("Incorrect request data: {}", op))),
@@ -149,7 +146,7 @@ impl ApiEndpoint for WalletReceiver {
 							LOGGER,
 							"Operation {} with transaction {}",
 							op,
-							&partial_tx_str
+							&partial_tx_str,
 						);
 						receive_json_tx(&self.config, &self.keychain, &partial_tx_str)
 							.map_err(|e| {
@@ -163,7 +160,7 @@ impl ApiEndpoint for WalletReceiver {
 						Ok(CbData {
 							output: String::from(""),
 							kernel: String::from(""),
-							pubkey: String::from(""),
+							key_id: String::from(""),
 						})
 					}
 					_ => Err(api::Error::Argument(format!("Incorrect request data: {}", op))),
@@ -179,27 +176,27 @@ fn receive_coinbase(config: &WalletConfig,
                     keychain: &Keychain,
                     block_fees: &BlockFees)
                     -> Result<(Output, TxKernel, BlockFees), Error> {
-	let fingerprint = keychain.fingerprint();
+	let root_key_id = keychain.root_key_id();
 
 	// operate within a lock on wallet data
 	WalletData::with_wallet(&config.data_file_dir, |wallet_data| {
-		let pubkey = block_fees.pubkey();
+		let key_id = block_fees.key_id();
-		let (pubkey, derivation) = match pubkey {
+		let (key_id, derivation) = match key_id {
-			Some(pubkey) => {
+			Some(key_id) => {
-				let derivation = keychain.derivation_from_pubkey(&pubkey)?;
+				let derivation = keychain.derivation_from_key_id(&key_id)?;
-				(pubkey.clone(), derivation)
+				(key_id.clone(), derivation)
-			}
+			},
 			None => {
-				let derivation = wallet_data.next_child(fingerprint.clone());
+				let derivation = wallet_data.next_child(root_key_id.clone());
-				let pubkey = keychain.derive_pubkey(derivation)?;
+				let key_id = keychain.derive_key_id(derivation)?;
-				(pubkey, derivation)
+				(key_id, derivation)
 			}
 		};
 
 		// track the new output and return the stuff needed for reward
 		wallet_data.add_output(OutputData {
-			fingerprint: fingerprint.clone(),
+			root_key_id: root_key_id.clone(),
-			identifier: pubkey.clone(),
+			key_id: key_id.clone(),
 			n_child: derivation,
 			value: reward(block_fees.fees),
 			status: OutputStatus::Unconfirmed,
@@ -209,22 +206,22 @@ fn receive_coinbase(config: &WalletConfig,
 
 		debug!(
 			LOGGER,
-			"Received coinbase and built candidate output - {}, {}, {}",
+			"Received coinbase and built candidate output - {:?}, {:?}, {}",
-			fingerprint.clone(),
+			root_key_id.clone(),
-			pubkey.fingerprint(),
+			key_id.clone(),
-			derivation
+			derivation,
 		);
 
 		debug!(LOGGER, "block_fees - {:?}", block_fees);
 
 		let mut block_fees = block_fees.clone();
-		block_fees.pubkey = Some(pubkey.clone());
+		block_fees.key_id = Some(key_id.clone());
 
 		debug!(LOGGER, "block_fees updated - {:?}", block_fees);
 
 		let (out, kern) = Block::reward_output(
 			&keychain,
-			&pubkey,
+			&key_id,
 			block_fees.fees,
 		)?;
 		Ok((out, kern, block_fees))
@@ -239,29 +236,32 @@ fn receive_transaction(
 	blinding: BlindingFactor,
 	partial: Transaction,
 ) -> Result<Transaction, Error> {
-	let fingerprint = keychain.clone().fingerprint();
+	let root_key_id = keychain.root_key_id();
 
 	// operate within a lock on wallet data
 	WalletData::with_wallet(&config.data_file_dir, |wallet_data| {
-		let derivation = wallet_data.next_child(fingerprint.clone());
+		let derivation = wallet_data.next_child(root_key_id.clone());
-		let pubkey = keychain.derive_pubkey(derivation)?;
+		let key_id = keychain.derive_key_id(derivation)?;
 
-		// from pool.rs
+		// double check the fee amount included in the partial tx
-		// (-1 * num_inputs) + (4 * num_outputs) + 1
+		// we don't necessarily want to just trust the sender
-		// then multiply by accept_fee_base==10
+		// we could just overwrite the fee here (but we won't) due to the ecdsa sig
-		// so 80 is basically the minimum fee for a basic transaction
+		let fee = tx_fee(partial.inputs.len(), partial.outputs.len() + 1, None);
-		// so lets use 100 for now (revisit this)
+		if fee != partial.fee {
+			return Err(Error::FeeDispute {
+				sender_fee: partial.fee,
+				recipient_fee: fee,
+			});
+		}
 
-		let fee_amount = tx_fee(partial.inputs.len(), partial.outputs.len() + 1, None);
+		let out_amount = amount - fee;
-		let out_amount = amount - fee_amount;
 
-		let (tx_final, _) = build::transaction(
+		let (tx_final, _) = build::transaction(vec![
-			vec![build::initial_tx(partial),
+			build::initial_tx(partial),
-			     build::with_excess(blinding),
+			build::with_excess(blinding),
-			     build::output(out_amount, pubkey.clone()),
+			build::output(out_amount, key_id.clone()),
-			     build::with_fee(fee_amount)],
+			// build::with_fee(fee_amount),
-			keychain,
+		], keychain)?;
-		)?;
 
 		// make sure the resulting transaction is valid (could have been lied to on
 		// excess)
@@ -269,8 +269,8 @@ fn receive_transaction(
 
 		// track the new output and return the finalized transaction to broadcast
 		wallet_data.add_output(OutputData {
-			fingerprint: fingerprint.clone(),
+			root_key_id: root_key_id.clone(),
-			identifier: pubkey.clone(),
+			key_id: key_id.clone(),
 			n_child: derivation,
 			value: out_amount,
 			status: OutputStatus::Unconfirmed,
@@ -279,11 +279,12 @@ fn receive_transaction(
 		});
 		debug!(
 			LOGGER,
-			"Received txn and built output  - {}, {}, {}",
+			"Received txn and built output - {:?}, {:?}, {}",
-			fingerprint.clone(),
+			root_key_id.clone(),
-			pubkey.fingerprint(),
+			key_id.clone(),
-			derivation
+			derivation,
 		);
+
 		Ok(tx_final)
 	})?
 }

wallet/src/sender.rs

@@ -16,7 +16,7 @@ use api;
 use checker;
 use core::core::{Transaction, build};
 use core::ser;
-use keychain::{BlindingFactor, Keychain, Fingerprint, Identifier};
+use keychain::{BlindingFactor, Keychain, Identifier, IDENTIFIER_SIZE};
 use receiver::TxWrapper;
 use types::*;
 use util::LOGGER;
@@ -66,49 +66,49 @@ fn build_send_tx(
 	amount: u64,
 	lock_height: u64,
 ) -> Result<(Transaction, BlindingFactor), Error> {
-	let fingerprint = keychain.clone().fingerprint();
+	let key_id = keychain.clone().root_key_id();
 
 	// operate within a lock on wallet data
 	WalletData::with_wallet(&config.data_file_dir, |wallet_data| {
 
 		// select some suitable outputs to spend from our local wallet
-		let (coins, change) = wallet_data.select(fingerprint.clone(), amount);
+		let (coins, change) = wallet_data.select(key_id.clone(), amount);
 		if change < 0 {
 			return Err(Error::NotEnoughFunds((-change) as u64));
 		}
 
 		// build transaction skeleton with inputs and change
-		let mut parts = inputs_and_change(&coins, keychain, fingerprint, wallet_data, amount)?;
+		let mut parts = inputs_and_change(&coins, keychain, key_id, wallet_data, amount)?;
 
 		// 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).
+		// lock_height on the transaction being sent (based on current chain height via
+		// api).
 		parts.push(build::with_lock_height(lock_height));
 
-		let result = build::transaction(parts, &keychain)?;
+		let (tx, blind) = build::transaction(parts, &keychain)?;
-		Ok(result)
+
+		Ok((tx, blind))
 	})?
 }
 
-pub fn issue_burn_tx(
+pub fn issue_burn_tx(config: &WalletConfig, keychain: &Keychain, amount: u64) -> Result<(), Error> {
-	config: &WalletConfig,
-	keychain: &Keychain,
-	amount: u64,
-) -> Result<(), Error> {
-
 	let _ = checker::refresh_outputs(config, keychain);
-	let fingerprint = keychain.clone().fingerprint();
+	let key_id = keychain.clone().root_key_id();
 
 	// operate within a lock on wallet data
 	WalletData::with_wallet(&config.data_file_dir, |mut wallet_data| {
 
 		// select all suitable outputs by passing largest amount
-		let (coins, _) = wallet_data.select(fingerprint.clone(), u64::max_value());
+		let (coins, _) = wallet_data.select(key_id.clone(), u64::max_value());
 
 		// build transaction skeleton with inputs and change
-		let mut parts = inputs_and_change(&coins, keychain, fingerprint, &mut wallet_data, amount)?;
+		let mut parts = inputs_and_change(&coins, keychain, key_id, &mut wallet_data, amount)?;
 
 		// add burn output and fees
-		parts.push(build::output(amount, Identifier::from_bytes(&[0; 20])));
+		parts.push(build::output(
+			amount,
+			Identifier::from_bytes(&[0; IDENTIFIER_SIZE]),
+		));
 
 		// finalize the burn transaction and send
 		let (tx_burn, _) = build::transaction(parts, &keychain)?;
@@ -122,36 +122,51 @@ pub fn issue_burn_tx(
 	})?
 }
 
-fn inputs_and_change(coins: &Vec<OutputData>, keychain: &Keychain, fingerprint: Fingerprint, wallet_data: &mut WalletData, amount: u64) -> Result<Vec<Box<build::Append>>, Error> {
+fn inputs_and_change(
+	coins: &Vec<OutputData>,
+	keychain: &Keychain,
+	root_key_id: Identifier,
+	wallet_data: &mut WalletData,
+	amount: u64,
+) -> Result<Vec<Box<build::Append>>, Error> {
 
 	let mut parts = vec![];
 
-	// calculate the total in inputs, fees and how much is left
+	// calculate the total across all inputs, and how much is left
 	let total: u64 = coins.iter().map(|c| c.value).sum();
-	let fee = tx_fee(coins.len(), 2, None);
+	let shortage = (total as i64) - (amount as i64);
-	let shortage = (total as i64) - (amount as i64) - (fee as i64);
 	if shortage < 0 {
 		return Err(Error::NotEnoughFunds((-shortage) as u64));
 	}
-	parts.push(build::with_fee(fee));
-	let change = total - amount - fee;
 
-	// build inputs using the appropriate derived pubkeys
+	// sender is responsible for setting the fee on the partial tx
+	// recipient should double check the fee calculation and not blindly trust the
+	// sender
+	let fee = tx_fee(coins.len(), 2, None);
+	parts.push(build::with_fee(fee));
+
+	// if we are spending 10,000 coins to send 1,000 then our change will be 9,000
+	// the fee will come out of the amount itself
+	// if the fee is 80 then the recipient will only receive 920
+	// but our change will still be 9,000
+	let change = total - amount;
+
+	// build inputs using the appropriate derived key_ids
 	for coin in coins {
-		let pubkey = keychain.derive_pubkey(coin.n_child)?;
+		let key_id = keychain.derive_key_id(coin.n_child)?;
-		parts.push(build::input(coin.value, pubkey));
+		parts.push(build::input(coin.value, key_id));
 	}
 
 	// derive an additional pubkey for change and build the change output
-	let change_derivation = wallet_data.next_child(fingerprint.clone());
+	let change_derivation = wallet_data.next_child(root_key_id.clone());
-	let change_key = keychain.derive_pubkey(change_derivation)?;
+	let change_key = keychain.derive_key_id(change_derivation)?;
 	parts.push(build::output(change, change_key.clone()));
 
 	// we got that far, time to start tracking the new output
 	// and lock the outputs used
 	wallet_data.add_output(OutputData {
-		fingerprint: fingerprint.clone(),
+		root_key_id: root_key_id.clone(),
-		identifier: change_key.clone(),
+		key_id: change_key.clone(),
 		n_child: change_derivation,
 		value: change as u64,
 		status: OutputStatus::Unconfirmed,
@@ -177,12 +192,11 @@ mod test {
 	// based on the public key and amount begin spent
 	fn output_commitment_equals_input_commitment_on_spend() {
 		let keychain = Keychain::from_random_seed().unwrap();
-		let pk1 = keychain.derive_pubkey(1).unwrap();
+		let key_id1 = keychain.derive_key_id(1).unwrap();
 
-		let (tx, _) = transaction(vec![output(105, pk1.clone())], &keychain).unwrap();
+		let (tx1, _) = transaction(vec![output(105, key_id1.clone())], &keychain).unwrap();
+		let (tx2, _) = transaction(vec![input(105, key_id1.clone())], &keychain).unwrap();
 
-		let (tx2, _) = transaction(vec![input(105, pk1.clone())], &keychain).unwrap();
+		assert_eq!(tx1.outputs[0].commitment(), tx2.inputs[0].commitment());
-
-		assert_eq!(tx.outputs[0].commitment(), tx2.inputs[0].commitment());
 	}
 }

wallet/src/types.rs

@@ -38,7 +38,7 @@ const DEFAULT_BASE_FEE: u64 = 10;
 /// Transaction fee calculation
 pub fn tx_fee(input_len: usize, output_len: usize, base_fee: Option<u64>) -> u64 {
 	let use_base_fee = match base_fee {
-		Some(bf)  => bf,
+		Some(bf) => bf,
 		None => DEFAULT_BASE_FEE,
 	};
 	let mut tx_weight = -1 * (input_len as i32) + 4 * (output_len as i32) + 1;
@@ -53,6 +53,7 @@ pub fn tx_fee(input_len: usize, output_len: usize, base_fee: Option<u64>) -> u64
 #[derive(Debug)]
 pub enum Error {
 	NotEnoughFunds(u64),
+	FeeDispute{sender_fee: u64, recipient_fee: u64},
 	Keychain(keychain::Error),
 	Transaction(transaction::Error),
 	Secp(secp::Error),
@@ -153,9 +154,10 @@ impl fmt::Display for OutputStatus {
 /// root private key is known.
 #[derive(Serialize, Deserialize, Debug, Clone)]
 pub struct OutputData {
-	/// Private key fingerprint (in case the wallet tracks multiple)
+	/// Root key_id that the key for this output is derived from
-	pub fingerprint: keychain::Fingerprint,
+	pub root_key_id: keychain::Identifier,
-	pub identifier: keychain::Identifier,
+	/// Derived key for this output
+	pub key_id: keychain::Identifier,
 	/// How many derivations down from the root key
 	pub n_child: u32,
 	/// Value of the output, necessary to rebuild the commitment
@@ -292,13 +294,13 @@ impl WalletData {
 	/// TODO - we should check for overwriting here - only really valid for
 	/// unconfirmed coinbase
 	pub fn add_output(&mut self, out: OutputData) {
-		self.outputs.insert(out.identifier.to_hex(), out.clone());
+		self.outputs.insert(out.key_id.to_hex(), out.clone());
 	}
 
 	/// Lock an output data.
 	/// TODO - we should track identifier on these outputs (not just n_child)
 	pub fn lock_output(&mut self, out: &OutputData) {
-		if let Some(out_to_lock) = self.outputs.get_mut(&out.identifier.to_hex()) {
+		if let Some(out_to_lock) = self.outputs.get_mut(&out.key_id.to_hex()) {
 			if out_to_lock.value == out.value {
 				out_to_lock.lock()
 			}
@@ -307,17 +309,14 @@ impl WalletData {
 
 	/// Select a subset of unspent outputs to spend in a transaction
 	/// transferring the provided amount.
-	pub fn select(&self,
+	pub fn select(&self, root_key_id: keychain::Identifier, amount: u64) -> (Vec<OutputData>, i64) {
-	              fingerprint: keychain::Fingerprint,
-	              amount: u64)
-	              -> (Vec<OutputData>, i64) {
 		let mut to_spend = vec![];
 		let mut input_total = 0;
 
 		// TODO very naive impl for now - definitely better coin selection
 		// algos available
 		for out in self.outputs.values() {
-			if out.status == OutputStatus::Unspent && out.fingerprint == fingerprint {
+			if out.status == OutputStatus::Unspent && out.root_key_id == root_key_id {
 				to_spend.push(out.clone());
 				input_total += out.value;
 				if input_total >= amount {
@@ -330,10 +329,10 @@ impl WalletData {
 	}
 
 	/// Next child index when we want to create a new output.
-	pub fn next_child(&self, fingerprint: keychain::Fingerprint) -> u32 {
+	pub fn next_child(&self, root_key_id: keychain::Identifier) -> u32 {
 		let mut max_n = 0;
 		for out in self.outputs.values() {
-			if max_n < out.n_child && out.fingerprint == fingerprint {
+			if max_n < out.n_child && out.root_key_id == root_key_id {
 				max_n = out.n_child;
 			}
 		}
@@ -399,12 +398,12 @@ pub enum WalletReceiveRequest {
 pub struct BlockFees {
 	pub fees: u64,
 	pub height: u64,
-	pub pubkey: Option<keychain::Identifier>,
+	pub key_id: Option<keychain::Identifier>,
 }
 
 impl BlockFees {
-	pub fn pubkey(&self) -> Option<keychain::Identifier> {
+	pub fn key_id(&self) -> Option<keychain::Identifier> {
-		self.pubkey.clone()
+		self.key_id.clone()
 	}
 }
 
@@ -413,5 +412,5 @@ impl BlockFees {
 pub struct CbData {
 	pub output: String,
 	pub kernel: String,
-	pub pubkey: String,
+	pub key_id: String,
 }