From bbdd4a91ce27d6dd5d1450abf4251c1404aaef5b Mon Sep 17 00:00:00 2001 From: AntiochP <30642645+antiochp@users.noreply.github.com> Date: Wed, 17 Jan 2018 11:25:34 -0500 Subject: [PATCH] rework WalletData and OutputData serialization so we serialize block hashes cleanly in wallet.dat (#625) --- keychain/src/extkey.rs | 2 +- wallet/src/checker.rs | 4 +- wallet/src/receiver.rs | 7 ++-- wallet/src/restore.rs | 5 +-- wallet/src/sender.rs | 7 ++-- wallet/src/types.rs | 87 ++++++++++++++++++++++++++++++++++++------ 6 files changed, 86 insertions(+), 26 deletions(-) diff --git a/keychain/src/extkey.rs b/keychain/src/extkey.rs index ebb7a0bf2..3bc769b52 100644 --- a/keychain/src/extkey.rs +++ b/keychain/src/extkey.rs @@ -74,7 +74,7 @@ impl error::Error for Error { } } -#[derive(Clone, PartialEq, Eq, Hash)] +#[derive(Clone, PartialEq, Eq, Ord, Hash, PartialOrd)] pub struct Identifier([u8; IDENTIFIER_SIZE]); impl ser::Serialize for Identifier { diff --git a/wallet/src/checker.rs b/wallet/src/checker.rs index ba55a4837..7a7833557 100644 --- a/wallet/src/checker.rs +++ b/wallet/src/checker.rs @@ -64,7 +64,7 @@ fn refresh_missing_block_hashes(config: &WalletConfig, keychain: &Keychain) -> R .values() .filter(|x| { x.root_key_id == keychain.root_key_id() && - x.block_hash == Hash::zero() && + x.block.hash() == Hash::zero() && x.status == OutputStatus::Unspent }) { @@ -139,7 +139,7 @@ fn refresh_missing_block_hashes(config: &WalletConfig, keychain: &Keychain) -> R if let Entry::Occupied(mut output) = wallet_data.outputs.entry(id.to_hex()) { if let Some(b) = api_blocks.get(&commit) { let output = output.get_mut(); - output.block_hash = Hash::from_hex(&b.hash).unwrap(); + output.block = BlockIdentifier::from_str(&b.hash).unwrap(); output.height = b.height; } } diff --git a/wallet/src/receiver.rs b/wallet/src/receiver.rs index 575a64653..51ca27d72 100644 --- a/wallet/src/receiver.rs +++ b/wallet/src/receiver.rs @@ -25,7 +25,6 @@ use serde_json; use api; use core::consensus::reward; use core::core::{build, Block, Output, Transaction, TxKernel, amount_to_hr_string}; -use core::core::hash::Hash; use core::{global, ser}; use keychain::{Identifier, Keychain}; use types::*; @@ -97,7 +96,7 @@ fn handle_sender_initiation( height: 0, lock_height: 0, is_coinbase: false, - block_hash: Hash::zero(), + block: BlockIdentifier::zero(), }); key_id @@ -282,7 +281,7 @@ pub fn receive_coinbase( height: height, lock_height: lock_height, is_coinbase: true, - block_hash: Hash::zero(), + block: BlockIdentifier::zero(), }); (key_id, derivation) @@ -363,7 +362,7 @@ fn build_final_transaction( height: 0, lock_height: 0, is_coinbase: false, - block_hash: Hash::zero(), + block: BlockIdentifier::zero(), }); (key_id, derivation) diff --git a/wallet/src/restore.rs b/wallet/src/restore.rs index caa85dfa7..31fa5fa7f 100644 --- a/wallet/src/restore.rs +++ b/wallet/src/restore.rs @@ -18,9 +18,8 @@ use util::secp::pedersen; use api; use core::global; use core::core::{Output, SwitchCommitHash}; -use core::core::hash::Hash; use core::core::transaction::{COINBASE_OUTPUT, DEFAULT_OUTPUT}; -use types::{WalletConfig, WalletData, OutputData, OutputStatus, Error}; +use types::{BlockIdentifier, WalletConfig, WalletData, OutputData, OutputStatus, Error}; use byteorder::{BigEndian, ByteOrder}; @@ -311,7 +310,7 @@ pub fn restore( height: output.4, lock_height: output.5, is_coinbase: output.6, - block_hash: Hash::zero(), + block: BlockIdentifier::zero(), }); }; } diff --git a/wallet/src/sender.rs b/wallet/src/sender.rs index 9b2e9795a..43c3d0b4a 100644 --- a/wallet/src/sender.rs +++ b/wallet/src/sender.rs @@ -16,7 +16,6 @@ use api; use client; use checker; use core::core::{build, Transaction, amount_to_hr_string}; -use core::core::hash::Hash; use core::ser; use keychain::{BlindingFactor, Identifier, Keychain}; use receiver::TxWrapper; @@ -267,9 +266,9 @@ fn inputs_and_change( for coin in coins { let key_id = keychain.derive_key_id(coin.n_child)?; if coin.is_coinbase { - parts.push(build::coinbase_input(coin.value, coin.block_hash, key_id)); + parts.push(build::coinbase_input(coin.value, coin.block.hash(), key_id)); } else { - parts.push(build::input(coin.value, coin.block_hash, key_id)); + parts.push(build::input(coin.value, coin.block.hash(), key_id)); } } @@ -288,7 +287,7 @@ fn inputs_and_change( height: 0, lock_height: 0, is_coinbase: false, - block_hash: Hash::zero(), + block: BlockIdentifier::zero(), }); change_key diff --git a/wallet/src/types.rs b/wallet/src/types.rs index cfd0351cf..92ea629ea 100644 --- a/wallet/src/types.rs +++ b/wallet/src/types.rs @@ -24,6 +24,7 @@ use std::collections::HashMap; use std::cmp::min; use hyper; +use serde; use serde_json; use tokio_core::reactor; use tokio_retry::Retry; @@ -205,7 +206,7 @@ impl WalletConfig { /// unconfirmed, spent, unspent, or locked (when it's been used to generate /// a transaction but we don't have confirmation that the transaction was /// broadcasted or mined). -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Ord, PartialOrd)] pub enum OutputStatus { Unconfirmed, Unspent, @@ -224,10 +225,64 @@ impl fmt::Display for OutputStatus { } } +#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord)] +pub struct BlockIdentifier(Hash); + +impl BlockIdentifier { + pub fn hash(&self) -> Hash { + self.0 + } + + pub fn from_str(hex: &str) -> Result { + let hash = Hash::from_hex(hex)?; + Ok(BlockIdentifier(hash)) + } + + pub fn zero() -> BlockIdentifier { + BlockIdentifier(Hash::zero()) + } +} + +impl serde::ser::Serialize for BlockIdentifier { + fn serialize(&self, serializer: S) -> Result + where + S: serde::ser::Serializer, + { + serializer.serialize_str(&self.0.to_hex()) + } +} + +impl<'de> serde::de::Deserialize<'de> for BlockIdentifier { + fn deserialize(deserializer: D) -> Result + where + D: serde::de::Deserializer<'de>, + { + deserializer.deserialize_str(BlockIdentifierVisitor) + } +} + +struct BlockIdentifierVisitor; + +impl<'de> serde::de::Visitor<'de> for BlockIdentifierVisitor { + type Value = BlockIdentifier; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("a block hash") + } + + fn visit_str(self, s: &str) -> Result + where + E: serde::de::Error, + { + let block_hash = Hash::from_hex(s).unwrap(); + Ok(BlockIdentifier(block_hash)) + } +} + /// Information about an output that's being tracked by the wallet. Must be /// enough to reconstruct the commitment associated with the ouput when the /// root private key is known. -#[derive(Serialize, Deserialize, Debug, Clone)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, PartialOrd, Eq, Ord)] pub struct OutputData { /// Root key_id that the key for this output is derived from pub root_key_id: keychain::Identifier, @@ -246,7 +301,7 @@ pub struct OutputData { /// Is this a coinbase output? Is it subject to coinbase locktime? pub is_coinbase: bool, /// Hash of the block this output originated from. - pub block_hash: Hash, + pub block: BlockIdentifier, } impl OutputData { @@ -381,12 +436,6 @@ impl WalletSeed { /// Wallet information tracking all our outputs. Based on HD derivation and /// avoids storing any key data, only storing output amounts and child index. -/// This data structure is directly based on the JSON representation stored -/// on disk, so selection algorithms are fairly primitive and non optimized. -/// -/// TODO optimization so everything isn't O(n) or even O(n^2) -/// TODO account for fees -/// TODO write locks so files don't get overwritten #[derive(Serialize, Deserialize, Debug, Clone)] pub struct WalletData { pub outputs: HashMap, @@ -480,8 +529,8 @@ impl WalletData { } } - /// Read the wallet data from disk. - fn read(data_file_path: &str) -> Result { + /// Read output_data vec from disk. + fn read_outputs(data_file_path: &str) -> Result, Error> { let data_file = File::open(data_file_path).map_err(|e| { Error::WalletData(format!("Could not open {}: {}", data_file_path, e)) })?; @@ -490,12 +539,26 @@ impl WalletData { }) } + /// Populate wallet_data with output_data from disk. + fn read(data_file_path: &str) -> Result { + let outputs = WalletData::read_outputs(data_file_path)?; + let mut wallet_data = WalletData { + outputs: HashMap::new(), + }; + for out in outputs { + wallet_data.add_output(out); + } + Ok(wallet_data) + } + /// Write the wallet data to disk. fn write(&self, data_file_path: &str) -> Result<(), Error> { let mut data_file = File::create(data_file_path).map_err(|e| { Error::WalletData(format!("Could not create {}: {}", data_file_path, e)) })?; - let res_json = serde_json::to_vec_pretty(self).map_err(|e| { + let mut outputs = self.outputs.values().collect::>(); + outputs.sort(); + let res_json = serde_json::to_vec_pretty(&outputs).map_err(|e| { Error::WalletData(format!("Error serializing wallet data: {}", e)) })?; data_file.write_all(res_json.as_slice()).map_err(|e| {