diff --git a/pool/src/pool.rs b/pool/src/pool.rs index feb6f494f..eb2b5faf0 100644 --- a/pool/src/pool.rs +++ b/pool/src/pool.rs @@ -114,7 +114,6 @@ impl TransactionPool where T: BlockChain { // Making sure the transaction is valid before anything else. let secp = secp::Secp256k1::with_caps(secp::ContextFlag::Commit); tx.validate(&secp).map_err(|_| PoolError::Invalid)?; - // The first check invovles ensuring that an identical transaction is // not already in the pool's transaction set. diff --git a/wallet/src/checker.rs b/wallet/src/checker.rs index 0455e9f5b..77d775e6b 100644 --- a/wallet/src/checker.rs +++ b/wallet/src/checker.rs @@ -16,7 +16,7 @@ //! the wallet storage and update them. use api; -use core::core::{Output, DEFAULT_OUTPUT, COINBASE_OUTPUT}; +use core::core::Output; use secp::{self, pedersen}; use util; @@ -27,32 +27,34 @@ use types::{WalletConfig, OutputStatus, WalletData}; /// with a node whether their status has changed. pub fn refresh_outputs(config: &WalletConfig, ext_key: &ExtendedKey) { let secp = secp::Secp256k1::with_caps(secp::ContextFlag::Commit); - let mut wallet_data = WalletData::read_or_create().expect("Could not open wallet data."); - let mut changed = 0; - for out in &mut wallet_data.outputs { - if out.status != OutputStatus::Spent { - let key = ext_key.derive(&secp, out.n_child).unwrap(); - let commitment = secp.commit(out.value, key.key).unwrap(); + // operate within a lock on wallet data + WalletData::with_wallet(|wallet_data| { - // TODO check the pool for unconfirmed + // check each output that's not spent + for out in &mut wallet_data.outputs { + if out.status != OutputStatus::Spent { - let out_res = get_output_by_commitment(config, commitment); - if out_res.is_ok() { - out.status = OutputStatus::Unspent; - changed += 1; - } else if out.status == OutputStatus::Unspent { - // a UTXO we can't find anymore has been spent - if let Err(api::Error::NotFound) = out_res { - out.status = OutputStatus::Spent; - changed += 1; + // figure out the commitment + let key = ext_key.derive(&secp, out.n_child).unwrap(); + let commitment = secp.commit(out.value, key.key).unwrap(); + + // TODO check the pool for unconfirmed + + let out_res = get_output_by_commitment(config, commitment); + if out_res.is_ok() { + // output is known, it's a new utxo + out.status = OutputStatus::Unspent; + + } else if out.status == OutputStatus::Unspent { + // a UTXO we can't find anymore has been spent + if let Err(api::Error::NotFound) = out_res { + out.status = OutputStatus::Spent; + } } } } - } - if changed > 0 { - wallet_data.write().unwrap(); - } + }); } // queries a reachable node for a given output, checking whether it's been diff --git a/wallet/src/receiver.rs b/wallet/src/receiver.rs index 9f6505808..7a0fafd2f 100644 --- a/wallet/src/receiver.rs +++ b/wallet/src/receiver.rs @@ -143,24 +143,25 @@ impl ApiEndpoint for WalletReceiver { fn receive_coinbase(ext_key: &ExtendedKey, amount: u64) -> Result<(Output, TxKernel), Error> { let secp = secp::Secp256k1::with_caps(secp::ContextFlag::Commit); - // derive a new private for the reward - let mut wallet_data = WalletData::read_or_create()?; - let next_child = wallet_data.next_child(ext_key.fingerprint); - let coinbase_key = ext_key.derive(&secp, next_child).map_err(|e| Error::Key(e))?; + // operate within a lock on wallet data + WalletData::with_wallet(|wallet_data| { - // track the new output and return the stuff needed for reward - wallet_data.append_output(OutputData { - fingerprint: coinbase_key.fingerprint, - n_child: coinbase_key.n_child, - value: amount, - status: OutputStatus::Unconfirmed, - }); - wallet_data.write()?; + // derive a new private for the reward + let next_child = wallet_data.next_child(ext_key.fingerprint); + let coinbase_key = ext_key.derive(&secp, next_child).map_err(|e| Error::Key(e))?; - debug!("Using child {} for a new coinbase output.", - coinbase_key.n_child); + // track the new output and return the stuff needed for reward + wallet_data.append_output(OutputData { + fingerprint: coinbase_key.fingerprint, + n_child: coinbase_key.n_child, + value: amount, + status: OutputStatus::Unconfirmed, + }); + debug!("Using child {} for a new coinbase output.", + coinbase_key.n_child); - Block::reward_output(coinbase_key.key, &secp).map_err(&From::from) + Block::reward_output(coinbase_key.key, &secp).map_err(&From::from) + })? } /// Builds a full transaction from the partial one sent to us for transfer @@ -172,30 +173,31 @@ fn receive_transaction(ext_key: &ExtendedKey, let secp = secp::Secp256k1::with_caps(secp::ContextFlag::Commit); - // derive a new private for the receiving output - let mut wallet_data = WalletData::read_or_create()?; - let next_child = wallet_data.next_child(ext_key.fingerprint); - let out_key = ext_key.derive(&secp, next_child).map_err(|e| Error::Key(e))?; + // operate within a lock on wallet data + WalletData::with_wallet(|wallet_data| { - let (tx_final, _) = build::transaction(vec![build::initial_tx(partial), - build::with_excess(blinding), - build::output(amount, out_key.key)])?; + let next_child = wallet_data.next_child(ext_key.fingerprint); + let out_key = ext_key.derive(&secp, next_child).map_err(|e| Error::Key(e))?; - // make sure the resulting transaction is valid (could have been lied to - // on excess) - tx_final.validate(&secp)?; + let (tx_final, _) = build::transaction(vec![build::initial_tx(partial), + build::with_excess(blinding), + build::output(amount, out_key.key)])?; - // track the new output and return the finalized transaction to broadcast - wallet_data.append_output(OutputData { - fingerprint: out_key.fingerprint, - n_child: out_key.n_child, - value: amount, - status: OutputStatus::Unconfirmed, - }); - wallet_data.write()?; + // make sure the resulting transaction is valid (could have been lied to + // on excess) + tx_final.validate(&secp)?; - debug!("Using child {} for a new transaction output.", - out_key.n_child); + // track the new output and return the finalized transaction to broadcast + wallet_data.append_output(OutputData { + fingerprint: out_key.fingerprint, + n_child: out_key.n_child, + value: amount, + status: OutputStatus::Unconfirmed, + }); - Ok(tx_final) + debug!("Using child {} for a new transaction output.", + out_key.n_child); + + Ok(tx_final) + })? } diff --git a/wallet/src/sender.rs b/wallet/src/sender.rs index 6cd77b628..6898b0f0c 100644 --- a/wallet/src/sender.rs +++ b/wallet/src/sender.rs @@ -46,38 +46,41 @@ fn build_send_tx(ext_key: &ExtendedKey, amount: u64) -> Result<(Transaction, Sec // first, rebuild the private key from the seed let secp = secp::Secp256k1::with_caps(secp::ContextFlag::Commit); - // second, check from our local wallet data for outputs to spend - let mut wallet_data = WalletData::read()?; - let (coins, change) = wallet_data.select(ext_key.fingerprint, amount); - if change < 0 { - return Err(Error::NotEnoughFunds((-change) as u64)); - } + // operate within a lock on wallet data + WalletData::with_wallet(|wallet_data| { - // TODO add fees, which is likely going to make this iterative + // second, check from our local wallet data for outputs to spend + let (coins, change) = wallet_data.select(ext_key.fingerprint, amount); + if change < 0 { + return Err(Error::NotEnoughFunds((-change) as u64)); + } - // third, build inputs using the appropriate key - let mut parts = vec![]; - for coin in &coins { - let in_key = ext_key.derive(&secp, coin.n_child).map_err(|e| Error::Key(e))?; - parts.push(build::input(coin.value, in_key.key)); - } + // TODO add fees, which is likely going to make this iterative - // fourth, derive a new private for change and build the change output - let next_child = wallet_data.next_child(ext_key.fingerprint); - let change_key = ext_key.derive(&secp, next_child).map_err(|e| Error::Key(e))?; - parts.push(build::output(change as u64, change_key.key)); + // third, build inputs using the appropriate key + let mut parts = vec![]; + for coin in &coins { + let in_key = ext_key.derive(&secp, coin.n_child).map_err(|e| Error::Key(e))?; + parts.push(build::input(coin.value, in_key.key)); + } - // we got that far, time to start tracking the new output, finalize tx - // and lock the outputs used - wallet_data.append_output(OutputData { - fingerprint: change_key.fingerprint, - n_child: change_key.n_child, - value: change as u64, - status: OutputStatus::Unconfirmed, - }); - for mut coin in coins { - coin.lock(); - } - wallet_data.write()?; - build::transaction(parts).map_err(&From::from) + // fourth, derive a new private for change and build the change output + let next_child = wallet_data.next_child(ext_key.fingerprint); + let change_key = ext_key.derive(&secp, next_child).map_err(|e| Error::Key(e))?; + parts.push(build::output(change as u64, change_key.key)); + + // we got that far, time to start tracking the new output, finalize tx + // and lock the outputs used + wallet_data.append_output(OutputData { + fingerprint: change_key.fingerprint, + n_child: change_key.n_child, + value: change as u64, + status: OutputStatus::Unconfirmed, + }); + for mut coin in coins { + coin.lock(); + } + + build::transaction(parts).map_err(&From::from) + })? } diff --git a/wallet/src/types.rs b/wallet/src/types.rs index 7e6e5e7b0..17c192648 100644 --- a/wallet/src/types.rs +++ b/wallet/src/types.rs @@ -13,7 +13,7 @@ // limitations under the License. use std::convert::From; -use std::fs::File; +use std::fs::{self, File, OpenOptions}; use std::io::Write; use std::num; use std::path::Path; @@ -30,6 +30,7 @@ use extkey; use util; const DAT_FILE: &'static str = "wallet.dat"; +const LOCK_FILE: &'static str = "wallet.lock"; /// Wallet errors, mostly wrappers around underlying crypto or I/O errors. #[derive(Debug)] @@ -133,8 +134,38 @@ pub struct WalletData { } impl WalletData { + /// Allows the reading and writing of the wallet data within a file lock. + /// Just provide a closure taking a mutable WalletData. The lock should + /// be held for as short a period as possible to avoid contention. + /// Note that due to the impossibility to do an actual file lock easily + /// across operating systems, this just creates a lock file with a "should + /// not exist" option. + pub fn with_wallet(f: F) -> Result + where F: FnOnce(&mut WalletData) -> T + { + // create the lock files, if it already exists, will produce an error + OpenOptions::new().write(true).create_new(true).open(LOCK_FILE).map_err(|e| { + Error::WalletData(format!("Could not create wallet lock file. Either \ + some other process is using the wallet or there's a write access \ + issue.")) + })?; + + // do what needs to be done + let mut wdat = WalletData::read_or_create()?; + let res = f(&mut wdat); + wdat.write()?; + + // delete the lock file + fs::remove_file(LOCK_FILE).map_err(|e| { + Error::WalletData(format!("Could not remove wallet lock file. Maybe insufficient \ + rights?")) + })?; + + Ok(res) + } + /// Read the wallet data or created a brand new one if it doesn't exist yet - pub fn read_or_create() -> Result { + fn read_or_create() -> Result { if Path::new(DAT_FILE).exists() { WalletData::read() } else { @@ -144,7 +175,7 @@ impl WalletData { } /// Read the wallet data from disk. - pub fn read() -> Result { + fn read() -> Result { let data_file = File::open(DAT_FILE) .map_err(|e| Error::WalletData(format!("Could not open {}: {}", DAT_FILE, e)))?; serde_json::from_reader(data_file) @@ -152,7 +183,7 @@ impl WalletData { } /// Write the wallet data to disk. - pub fn write(&self) -> Result<(), Error> { + fn write(&self) -> Result<(), Error> { let mut data_file = File::create(DAT_FILE) .map_err(|e| Error::WalletData(format!("Could not create {}: {}", DAT_FILE, e)))?; let res_json = serde_json::to_vec_pretty(self)