Ensure wallet output heights are always correct (#1395)

* ensure block heights are always returned from API and updated within wallet

* fix api test

api/src/handlers.rs

@@ -69,7 +69,8 @@ fn get_output(chain: &Weak<chain::Chain>, id: &str) -> Result<(Output, OutputIde
 
 	for x in outputs.iter() {
 		if let Ok(_) = w(chain).is_unspent(&x) {
-			return Ok((Output::new(&commit), x.clone()));
+			let block_height = w(chain).get_header_for_output(&x).unwrap().height;
+			return Ok((Output::new(&commit, block_height), x.clone()));
 		}
 	}
 	Err(ErrorKind::NotFound)?
@@ -320,6 +321,7 @@ impl TxHashSetHandler {
 			spent: false,
 			proof: None,
 			proof_hash: "".to_string(),
+			block_height: None,
 			merkle_proof: Some(merkle_proof),
 		})
 	}

api/src/types.rs

@@ -157,14 +157,17 @@ pub enum OutputType {
 pub struct Output {
 	/// The output commitment representing the amount
 	pub commit: PrintableCommitment,
+	/// Height of the block which contains the output
+	pub height: u64,
 }
 
 impl Output {
-	pub fn new(commit: &pedersen::Commitment) -> Output {
+	pub fn new(commit: &pedersen::Commitment, height: u64) -> Output {
 		Output {
 			commit: PrintableCommitment {
 				commit: commit.clone(),
 			},
+			height: height,
 		}
 	}
 }
@@ -235,7 +238,9 @@ pub struct OutputPrintable {
 	pub proof: Option<String>,
 	/// Rangeproof hash (as hex string)
 	pub proof_hash: String,
-
+	/// Block height at which the output is found
+	pub block_height: Option<u64>,
+	/// Merkle Proof
 	pub merkle_proof: Option<MerkleProof>,
 }
 
@@ -257,6 +262,10 @@ impl OutputPrintable {
 
 		let out_id = core::OutputIdentifier::from_output(&output);
 		let spent = chain.is_unspent(&out_id).is_err();
+		let block_height = match spent {
+			true => None,
+			false => Some(chain.get_header_for_output(&out_id).unwrap().height),
+		};
 
 		let proof = if include_proof {
 			Some(util::to_hex(output.proof.proof.to_vec()))
@@ -283,6 +292,7 @@ impl OutputPrintable {
 			spent,
 			proof,
 			proof_hash: util::to_hex(output.proof.hash().to_vec()),
+			block_height,
 			merkle_proof,
 		}
 	}
@@ -320,6 +330,7 @@ impl serde::ser::Serialize for OutputPrintable {
 		state.serialize_field("spent", &self.spent)?;
 		state.serialize_field("proof", &self.proof)?;
 		state.serialize_field("proof_hash", &self.proof_hash)?;
+		state.serialize_field("block_height", &self.block_height)?;
 
 		let hex_merkle_proof = &self.merkle_proof.clone().map(|x| x.to_hex());
 		state.serialize_field("merkle_proof", &hex_merkle_proof)?;
@@ -341,6 +352,7 @@ impl<'de> serde::de::Deserialize<'de> for OutputPrintable {
 			Spent,
 			Proof,
 			ProofHash,
+			BlockHeight,
 			MerkleProof,
 		}
 
@@ -362,6 +374,7 @@ impl<'de> serde::de::Deserialize<'de> for OutputPrintable {
 				let mut spent = None;
 				let mut proof = None;
 				let mut proof_hash = None;
+				let mut block_height = None;
 				let mut merkle_proof = None;
 
 				while let Some(key) = map.next_key()? {
@@ -390,6 +403,10 @@ impl<'de> serde::de::Deserialize<'de> for OutputPrintable {
 							no_dup!(proof_hash);
 							proof_hash = Some(map.next_value()?)
 						}
+						Field::BlockHeight => {
+							no_dup!(block_height);
+							block_height = Some(map.next_value()?)
+						}
 						Field::MerkleProof => {
 							no_dup!(merkle_proof);
 							if let Some(hex) = map.next_value::<Option<String>>()? {
@@ -409,6 +426,7 @@ impl<'de> serde::de::Deserialize<'de> for OutputPrintable {
 					spent: spent.unwrap(),
 					proof: proof,
 					proof_hash: proof_hash.unwrap(),
+					block_height: block_height,
 					merkle_proof: merkle_proof,
 				})
 			}
@@ -643,6 +661,7 @@ mod test {
 			 \"spent\":false,\
 			 \"proof\":null,\
 			 \"proof_hash\":\"ed6ba96009b86173bade6a9227ed60422916593fa32dd6d78b25b7a4eeef4946\",\
+			 \"block_height\":0,\
 			 \"merkle_proof\":null\
 			 }";
 		let deserialized: OutputPrintable = serde_json::from_str(&hex_output).unwrap();
@@ -653,7 +672,10 @@ mod test {
 	#[test]
 	fn serialize_output() {
 		let hex_commit =
-			"{\"commit\":\"083eafae5d61a85ab07b12e1a51b3918d8e6de11fc6cde641d54af53608aa77b9f\"}";
+			"{\
+			 \"commit\":\"083eafae5d61a85ab07b12e1a51b3918d8e6de11fc6cde641d54af53608aa77b9f\",\
+			 \"height\":0\
+			 }";
 		let deserialized: Output = serde_json::from_str(&hex_commit).unwrap();
 		let serialized = serde_json::to_string(&deserialized).unwrap();
 		assert_eq!(serialized, hex_commit);

wallet/src/client.rs

@@ -121,7 +121,7 @@ impl WalletClient for HTTPWalletClient {
 	fn get_outputs_from_node(
 		&self,
 		wallet_outputs: Vec<pedersen::Commitment>,
-	) -> Result<HashMap<pedersen::Commitment, String>, libwallet::Error> {
+	) -> Result<HashMap<pedersen::Commitment, (String, u64)>, libwallet::Error> {
 		let addr = self.node_url();
 		// build the necessary query params -
 		// ?id=xxx&id=yyy&id=zzz
@@ -131,7 +131,7 @@ impl WalletClient for HTTPWalletClient {
 			.collect();
 
 		// build a map of api outputs by commit so we can look them up efficiently
-		let mut api_outputs: HashMap<pedersen::Commitment, String> = HashMap::new();
+		let mut api_outputs: HashMap<pedersen::Commitment, (String, u64)> = HashMap::new();
 		let mut tasks = Vec::new();
 
 		for query_chunk in query_params.chunks(500) {
@@ -152,7 +152,10 @@ impl WalletClient for HTTPWalletClient {
 
 		for res in results {
 			for out in res {
-				api_outputs.insert(out.commit.commit(), util::to_hex(out.commit.to_vec()));
+				api_outputs.insert(
+					out.commit.commit(),
+					(util::to_hex(out.commit.to_vec()), out.height),
+				);
 			}
 		}
 		Ok(api_outputs)

wallet/src/libwallet/internal/updater.rs

@@ -177,7 +177,7 @@ where
 pub fn apply_api_outputs<T: ?Sized, C, K>(
 	wallet: &mut T,
 	wallet_outputs: &HashMap<pedersen::Commitment, Identifier>,
-	api_outputs: &HashMap<pedersen::Commitment, String>,
+	api_outputs: &HashMap<pedersen::Commitment, (String, u64)>,
 	height: u64,
 ) -> Result<(), libwallet::Error>
 where
@@ -209,7 +209,7 @@ where
 		for (commit, id) in wallet_outputs.iter() {
 			if let Ok(mut output) = batch.get(id) {
 				match api_outputs.get(&commit) {
-					Some(_) => {
+					Some(o) => {
 						// if this is a coinbase tx being confirmed, it's recordable in tx log
 						if output.is_coinbase && output.status == OutputStatus::Unconfirmed {
 							let log_id = batch.next_tx_log_id(root_key_id.clone())?;
@@ -235,6 +235,7 @@ where
 								batch.save_tx_log_entry(t)?;
 							}
 						}
+						output.height = o.1;
 						output.mark_unspent();
 					}
 					None => output.mark_spent(),

wallet/src/libwallet/types.rs

@@ -165,7 +165,7 @@ pub trait WalletClient: Sync + Send + Clone {
 	fn get_outputs_from_node(
 		&self,
 		wallet_outputs: Vec<pedersen::Commitment>,
-	) -> Result<HashMap<pedersen::Commitment, String>, Error>;
+	) -> Result<HashMap<pedersen::Commitment, (String, u64)>, Error>;
 
 	/// Get a list of outputs from the node by traversing the UTXO
 	/// set in PMMR index order.

wallet/tests/common/mod.rs

@@ -56,7 +56,8 @@ fn get_output_local(chain: &chain::Chain, commit: &pedersen::Commitment) -> Opti
 
 	for x in outputs.iter() {
 		if let Ok(_) = chain.is_unspent(&x) {
-			return Some(api::Output::new(&commit));
+			let block_height = chain.get_header_for_output(&x).unwrap().height;
+			return Some(api::Output::new(&commit, block_height));
 		}
 	}
 	None

wallet/tests/common/testclient.rs

@@ -379,7 +379,7 @@ impl WalletClient for LocalWalletClient {
 	fn get_outputs_from_node(
 		&self,
 		wallet_outputs: Vec<pedersen::Commitment>,
-	) -> Result<HashMap<pedersen::Commitment, String>, libwallet::Error> {
+	) -> Result<HashMap<pedersen::Commitment, (String, u64)>, libwallet::Error> {
 		let query_params: Vec<String> = wallet_outputs
 			.iter()
 			.map(|commit| format!("{}", util::to_hex(commit.as_ref().to_vec())))
@@ -400,9 +400,12 @@ impl WalletClient for LocalWalletClient {
 		let r = self.rx.lock().unwrap();
 		let m = r.recv().unwrap();
 		let outputs: Vec<api::Output> = serde_json::from_str(&m.body).unwrap();
-		let mut api_outputs: HashMap<pedersen::Commitment, String> = HashMap::new();
+		let mut api_outputs: HashMap<pedersen::Commitment, (String, u64)> = HashMap::new();
 		for out in outputs {
-			api_outputs.insert(out.commit.commit(), util::to_hex(out.commit.to_vec()));
+			api_outputs.insert(
+				out.commit.commit(),
+				(util::to_hex(out.commit.to_vec()), out.height),
+			);
 		}
 		Ok(api_outputs)
 	}