From b74e0fbc1fb2810f970a5e2f1eac51c77635cf76 Mon Sep 17 00:00:00 2001 From: Yeastplume Date: Tue, 21 Aug 2018 14:04:14 +0100 Subject: [PATCH] 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 | 4 +++- api/src/types.rs | 28 +++++++++++++++++++++--- wallet/src/client.rs | 9 +++++--- wallet/src/libwallet/internal/updater.rs | 5 +++-- wallet/src/libwallet/types.rs | 2 +- wallet/tests/common/mod.rs | 3 ++- wallet/tests/common/testclient.rs | 9 +++++--- 7 files changed, 46 insertions(+), 14 deletions(-) diff --git a/api/src/handlers.rs b/api/src/handlers.rs index 2ad468bb0..900b28ebd 100644 --- a/api/src/handlers.rs +++ b/api/src/handlers.rs @@ -69,7 +69,8 @@ fn get_output(chain: &Weak, 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), }) } diff --git a/api/src/types.rs b/api/src/types.rs index fe006e8dd..7fa55d719 100644 --- a/api/src/types.rs +++ b/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, /// Rangeproof hash (as hex string) pub proof_hash: String, - + /// Block height at which the output is found + pub block_height: Option, + /// Merkle Proof pub merkle_proof: Option, } @@ -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::>()? { @@ -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); diff --git a/wallet/src/client.rs b/wallet/src/client.rs index e030c1f6e..bf2fe8a98 100644 --- a/wallet/src/client.rs +++ b/wallet/src/client.rs @@ -121,7 +121,7 @@ impl WalletClient for HTTPWalletClient { fn get_outputs_from_node( &self, wallet_outputs: Vec, - ) -> Result, libwallet::Error> { + ) -> Result, 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 = HashMap::new(); + let mut api_outputs: HashMap = 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) diff --git a/wallet/src/libwallet/internal/updater.rs b/wallet/src/libwallet/internal/updater.rs index c6c2ff744..d82f9eacb 100644 --- a/wallet/src/libwallet/internal/updater.rs +++ b/wallet/src/libwallet/internal/updater.rs @@ -177,7 +177,7 @@ where pub fn apply_api_outputs( wallet: &mut T, wallet_outputs: &HashMap, - api_outputs: &HashMap, + api_outputs: &HashMap, 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(), diff --git a/wallet/src/libwallet/types.rs b/wallet/src/libwallet/types.rs index 2db62d45e..018922d55 100644 --- a/wallet/src/libwallet/types.rs +++ b/wallet/src/libwallet/types.rs @@ -165,7 +165,7 @@ pub trait WalletClient: Sync + Send + Clone { fn get_outputs_from_node( &self, wallet_outputs: Vec, - ) -> Result, Error>; + ) -> Result, Error>; /// Get a list of outputs from the node by traversing the UTXO /// set in PMMR index order. diff --git a/wallet/tests/common/mod.rs b/wallet/tests/common/mod.rs index 2f679b4d1..2b173d165 100644 --- a/wallet/tests/common/mod.rs +++ b/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 diff --git a/wallet/tests/common/testclient.rs b/wallet/tests/common/testclient.rs index c240dad76..013f59670 100644 --- a/wallet/tests/common/testclient.rs +++ b/wallet/tests/common/testclient.rs @@ -379,7 +379,7 @@ impl WalletClient for LocalWalletClient { fn get_outputs_from_node( &self, wallet_outputs: Vec, - ) -> Result, libwallet::Error> { + ) -> Result, libwallet::Error> { let query_params: Vec = 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 = serde_json::from_str(&m.body).unwrap(); - let mut api_outputs: HashMap = HashMap::new(); + let mut api_outputs: HashMap = 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) }