From 9a193fd637649a95f165a9bdc95c71ced60fb17c Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Fri, 27 Oct 2017 17:19:54 -0400 Subject: [PATCH 01/36] experiment with lock_heights on outputs --- api/src/types.rs | 24 ++++++++++++------------ chain/src/pipe.rs | 9 ++++++++- chain/tests/test_coinbase_maturity.rs | 2 +- core/src/core/block.rs | 25 ++++++++++++++++++++----- core/src/core/build.rs | 9 ++++++--- core/src/core/transaction.rs | 15 ++++++++++++++- grin/src/miner.rs | 10 ++++++---- pool/src/pool.rs | 4 ++-- wallet/src/receiver.rs | 20 ++++++++++++++++---- wallet/src/sender.rs | 15 +++++++-------- 10 files changed, 92 insertions(+), 41 deletions(-) diff --git a/api/src/types.rs b/api/src/types.rs index 59a7d92ed1..b3b56ba823 100644 --- a/api/src/types.rs +++ b/api/src/types.rs @@ -12,9 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::sync::Arc; -use core::{core, global}; -use core::core::hash::Hashed; +use core::core; use chain; use util::secp::pedersen; use rest::*; @@ -148,14 +146,16 @@ pub struct Output { } impl Output { - pub fn from_output(output: &core::Output, block_header: &core::BlockHeader, - include_proof:bool, include_switch: bool) -> Output { - let (output_type, lock_height) = match output.features { - x if x.contains(core::transaction::COINBASE_OUTPUT) => ( - OutputType::Coinbase, - block_header.height + global::coinbase_maturity(), - ), - _ => (OutputType::Transaction, 0), + pub fn from_output( + output: &core::Output, + block_header: &core::BlockHeader, + include_proof: bool, + include_switch: bool, + ) -> Output { + let output_type = if output.features.contains(core::transaction::COINBASE_OUTPUT) { + OutputType::Coinbase + } else { + OutputType::Transaction }; Output { @@ -170,7 +170,7 @@ impl Output { false => None, }, height: block_header.height, - lock_height: lock_height, + lock_height: output.lock_height, } } } diff --git a/chain/src/pipe.rs b/chain/src/pipe.rs index dc652c6f07..51d14b617f 100644 --- a/chain/src/pipe.rs +++ b/chain/src/pipe.rs @@ -311,9 +311,16 @@ fn validate_block( return Err(Error::InvalidRoot); } - // check for any outputs with lock_heights greater than current block height for input in &b.inputs { if let Ok(output) = ctx.store.get_output_by_commit(&input.commitment()) { + + // check for any outputs with lock_heights greater than current block height + if output.lock_height > b.header.height { + return Err(Error::ImmatureCoinbase); + } + + // and check that any coinbase output also lines up correctly + // with the lock_height based on the original block header if output.features.contains(transaction::COINBASE_OUTPUT) { if let Ok(output_header) = ctx.store .get_block_header_by_output_commit(&input.commitment()) diff --git a/chain/tests/test_coinbase_maturity.rs b/chain/tests/test_coinbase_maturity.rs index 60c1bb0853..764f818c3f 100644 --- a/chain/tests/test_coinbase_maturity.rs +++ b/chain/tests/test_coinbase_maturity.rs @@ -103,7 +103,7 @@ fn test_coinbase_maturity() { let (coinbase_txn, _) = build::transaction( vec![ build::input(amount, key_id1.clone()), - build::output(amount - 2, key_id2), + build::output(amount - 2, block.header.height, key_id2), build::with_fee(2), ], &keychain, diff --git a/core/src/core/block.rs b/core/src/core/block.rs index 66f1dee5c3..c557b2221b 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -26,8 +26,8 @@ use consensus; use consensus::{exceeds_weight, reward, MINIMUM_DIFFICULTY, REWARD, VerifySortOrder}; use core::hash::{Hash, Hashed, ZERO_HASH}; use core::target::Difficulty; -use core::transaction; -use ser::{self, read_and_verify_sorted, Readable, Reader, Writeable, WriteableSorted, Writer}; +use ser::{self, Readable, Reader, Writeable, Writer, WriteableSorted, read_and_verify_sorted}; +use core::transaction::kernel_sig_msg; use util::LOGGER; use global; use keychain; @@ -294,7 +294,12 @@ impl Block { key_id: &keychain::Identifier, ) -> Result { let fees = txs.iter().map(|tx| tx.fee).sum(); - let (reward_out, reward_proof) = Block::reward_output(keychain, key_id, fees)?; + let (reward_out, reward_proof) = Block::reward_output( + keychain, + key_id, + fees, + prev.height + 1, + )?; let block = Block::with_reward(prev, txs, reward_out, reward_proof)?; Ok(block) } @@ -453,6 +458,7 @@ impl Block { /// trees, reward, etc. /// /// TODO - performs various verification steps - discuss renaming this to "verify" + /// as all the steps within are verify steps. /// pub fn validate(&self) -> Result<(), Error> { if exceeds_weight(self.inputs.len(), self.outputs.len(), self.kernels.len()) { @@ -553,7 +559,9 @@ impl Block { keychain: &keychain::Keychain, key_id: &keychain::Identifier, fees: u64, + height: u64, ) -> Result<(Output, TxKernel), keychain::Error> { + let lock_height = height + global::coinbase_maturity(); let commit = keychain.commit(reward(fees), key_id)?; let switch_commit = keychain.switch_commit(key_id)?; let switch_commit_hash = SwitchCommitHash::from_switch_commit(switch_commit); @@ -573,6 +581,7 @@ impl Block { let output = Output { features: COINBASE_OUTPUT, + lock_height: lock_height, commit: commit, switch_commit_hash: switch_commit_hash, proof: rproof, @@ -584,7 +593,12 @@ impl Block { let out_commit = output.commitment(); let excess = secp.commit_sum(vec![out_commit], vec![over_commit])?; - let msg = util::secp::Message::from_slice(&[0; secp::constants::MESSAGE_SIZE])?; + // NOTE: Remember we sign the fee *and* the lock_height. + // For a coinbase output the fee is 0 and the lock_height is + // the lock_height of the coinbase output itself, + // not the lock_height of the tx (there is no tx for a coinbase output). + // This output will not be spendable earlier than lock_height (and we sign this here). + let msg = secp::Message::from_slice(&kernel_sig_msg(0, lock_height))?; let sig = keychain.sign(&msg, &key_id)?; let excess_sig = sig.serialize_der(&secp); @@ -594,7 +608,8 @@ impl Block { excess: excess, excess_sig: excess_sig, fee: 0, - lock_height: 0, + // lock_height here is the height of the block (tx should be valid immediately) + lock_height: height, }; Ok((output, proof)) } diff --git a/core/src/core/build.rs b/core/src/core/build.rs index 90044d79c3..f2fd29a78f 100644 --- a/core/src/core/build.rs +++ b/core/src/core/build.rs @@ -29,9 +29,9 @@ use util::{secp, static_secp_instance}; use core::{Input, Output, SwitchCommitHash, Transaction, DEFAULT_OUTPUT}; use core::transaction::kernel_sig_msg; -use util::LOGGER; use keychain; -use keychain::{BlindSum, BlindingFactor, Identifier, Keychain}; +use keychain::{Keychain, BlindSum, BlindingFactor, Identifier}; +use util::LOGGER; /// Context information available to transaction combinators. pub struct Context<'a> { @@ -53,8 +53,10 @@ pub fn input(value: u64, key_id: Identifier) -> Box { /// Adds an output with the provided value and key identifier from the /// keychain. -pub fn output(value: u64, key_id: Identifier) -> Box { +pub fn output(value: u64, lock_height: u64, key_id: Identifier) -> Box { Box::new(move |build, (tx, sum)| -> (Transaction, BlindSum) { + println!("************* building an output ********** {}, {}", value, lock_height); + let commit = build.keychain.commit(value, &key_id).unwrap(); let switch_commit = build.keychain.switch_commit(&key_id).unwrap(); let switch_commit_hash = SwitchCommitHash::from_switch_commit(switch_commit); @@ -78,6 +80,7 @@ pub fn output(value: u64, key_id: Identifier) -> Box { ( tx.with_output(Output { features: DEFAULT_OUTPUT, + lock_height: lock_height, commit: commit, switch_commit_hash: switch_commit_hash, proof: rproof, diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 6c3777a34e..50c7fa75e6 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -27,6 +27,7 @@ use consensus::VerifySortOrder; use core::Committed; use core::hash::Hashed; use core::pmmr::Summable; +use global; use keychain::{Identifier, Keychain}; use ser::{self, read_and_verify_sorted, Readable, Reader, Writeable, WriteableSorted, Writer}; @@ -160,8 +161,14 @@ impl TxKernel { /// as a public key and checking the signature verifies with the fee as /// message. pub fn verify(&self) -> Result<(), secp::Error> { + let lock_height = if self.features.contains(COINBASE_KERNEL) { + self.lock_height + global::coinbase_maturity() + } else { + self.lock_height + }; + let msg = try!(Message::from_slice( - &kernel_sig_msg(self.fee, self.lock_height), + &kernel_sig_msg(self.fee, lock_height), )); let secp = static_secp_instance(); let secp = secp.lock().unwrap(); @@ -484,6 +491,8 @@ impl SwitchCommitHash { pub struct Output { /// Options for an output's structure or use pub features: OutputFeatures, + /// This output cannot be spent earlier than block at lock_height + pub lock_height: u64, /// The homomorphic commitment representing the output's amount pub commit: Commitment, /// The switch commitment hash, a 160 bit length blake2 hash of blind*J @@ -499,6 +508,7 @@ hashable_ord!(Output); impl Writeable for Output { fn write(&self, writer: &mut W) -> Result<(), ser::Error> { writer.write_u8(self.features.bits())?; + writer.write_u64(self.lock_height)?; writer.write_fixed_bytes(&self.commit)?; writer.write_fixed_bytes(&self.switch_commit_hash)?; @@ -520,6 +530,7 @@ impl Readable for Output { Ok(Output { features: features, + lock_height: reader.read_u64()?, commit: Commitment::read(reader)?, switch_commit_hash: SwitchCommitHash::read(reader)?, proof: RangeProof::read(reader)?, @@ -683,6 +694,7 @@ mod test { let out = Output { features: DEFAULT_OUTPUT, + lock_height: 0, commit: commit, switch_commit_hash: switch_commit_hash, proof: proof, @@ -710,6 +722,7 @@ mod test { let output = Output { features: DEFAULT_OUTPUT, + lock_height: 0, commit: commit, switch_commit_hash: switch_commit_hash, proof: proof, diff --git a/grin/src/miner.rs b/grin/src/miner.rs index 716c01766f..cdca0877b7 100644 --- a/grin/src/miner.rs +++ b/grin/src/miner.rs @@ -563,8 +563,6 @@ impl Miner { height, }; - // TODO - error handling, things can go wrong with get_coinbase (wallet api - // down etc.) let (output, kernel, block_fees) = self.get_coinbase(block_fees).unwrap(); let mut b = core::Block::with_reward(head, txs, output, kernel).unwrap(); @@ -610,8 +608,12 @@ impl Miner { ) -> Result<(core::Output, core::TxKernel, BlockFees), Error> { let keychain = Keychain::from_random_seed().unwrap(); let key_id = keychain.derive_key_id(1).unwrap(); - let (out, kernel) = - core::Block::reward_output(&keychain, &key_id, block_fees.fees).unwrap(); + let (out, kernel) = core::Block::reward_output( + &keychain, + &key_id, + block_fees.fees, + block_fees.height, + ).unwrap(); Ok((out, kernel, block_fees)) } diff --git a/pool/src/pool.rs b/pool/src/pool.rs index 0597241777..64ff01c894 100644 --- a/pool/src/pool.rs +++ b/pool/src/pool.rs @@ -1236,7 +1236,7 @@ mod tests { for output_value in output_values { let key_id = keychain.derive_key_id(output_value as u32).unwrap(); - tx_elements.push(build::output(output_value, key_id)); + tx_elements.push(build::output(output_value, 0, key_id)); } tx_elements.push(build::with_fee(fees as u64)); @@ -1264,7 +1264,7 @@ mod tests { for output_value in output_values { let key_id = keychain.derive_key_id(output_value as u32).unwrap(); - tx_elements.push(build::output(output_value, key_id)); + tx_elements.push(build::output(output_value, 0, key_id)); } tx_elements.push(build::with_fee(fees as u64)); diff --git a/wallet/src/receiver.rs b/wallet/src/receiver.rs index ee9ba5261e..1fa256c413 100644 --- a/wallet/src/receiver.rs +++ b/wallet/src/receiver.rs @@ -160,7 +160,12 @@ pub fn receive_coinbase( debug!(LOGGER, "block_fees updated - {:?}", block_fees); - let (out, kern) = Block::reward_output(&keychain, &key_id, block_fees.fees)?; + let (out, kern) = Block::reward_output( + &keychain, + &key_id, + block_fees.fees, + block_fees.height, + )?; Ok((out, kern, block_fees)) } @@ -174,6 +179,10 @@ fn receive_transaction( ) -> Result { let root_key_id = keychain.root_key_id(); + let (key_id, derivation) = next_available_key(config, keychain)?; + + let lock_height = partial.lock_height; + // double check the fee amount included in the partial tx // we don't necessarily want to just trust the sender // we could just overwrite the fee here (but we won't) due to the ecdsa sig @@ -198,7 +207,7 @@ fn receive_transaction( value: out_amount, status: OutputStatus::Unconfirmed, height: 0, - lock_height: 0, + lock_height: lock_height, is_coinbase: false, }); @@ -209,8 +218,11 @@ fn receive_transaction( vec![ build::initial_tx(partial), build::with_excess(blinding), - build::output(out_amount, key_id.clone()), - // build::with_fee(fee_amount), + build::output( + out_amount, + lock_height, + key_id.clone(), + ), ], keychain, )?; diff --git a/wallet/src/sender.rs b/wallet/src/sender.rs index eef99577c1..2ae21a547e 100644 --- a/wallet/src/sender.rs +++ b/wallet/src/sender.rs @@ -44,7 +44,7 @@ pub fn issue_send_tx( let chain_tip = checker::get_tip_from_node(config)?; let current_height = chain_tip.height; - // proof of concept - set lock_height on the tx + // proof of concept - set lock_height on the tx (only valid in a block later than this) let lock_height = chain_tip.height; let (tx, blind_sum, coins, change_key) = build_send_tx( @@ -126,7 +126,7 @@ fn build_send_tx( })?; // build transaction skeleton with inputs and change - let (mut parts, change_key) = inputs_and_change(&coins, config, keychain, amount)?; + let (mut parts, change_key) = inputs_and_change(&coins, config, keychain, key_id, amount, lock_height)?; // This is more proof of concept than anything but here we set lock_height // on tx being sent (based on current chain height via api). @@ -165,13 +165,11 @@ pub fn issue_burn_tx( ) })?; - debug!(LOGGER, "selected some coins - {}", coins.len()); - - let (mut parts, _) = inputs_and_change(&coins, config, keychain, amount)?; + let (mut parts, _) = inputs_and_change(&coins, config, keychain, key_id, amount, current_height)?; // add burn output and fees let fee = tx_fee(coins.len(), 2, None); - parts.push(build::output(amount - fee, Identifier::zero())); + parts.push(build::output(amount - fee, current_height, Identifier::zero())); // finalize the burn transaction and send let (tx_burn, _) = build::transaction(parts, &keychain)?; @@ -189,6 +187,7 @@ fn inputs_and_change( config: &WalletConfig, keychain: &Keychain, amount: u64, + lock_height: u64, ) -> Result<(Vec>, Identifier), Error> { let mut parts = vec![]; @@ -213,7 +212,7 @@ fn inputs_and_change( // build inputs using the appropriate derived key_ids for coin in coins { let key_id = keychain.derive_key_id(coin.n_child)?; - parts.push(build::input(coin.value, key_id)); + parts.push(build::input(coin.value, coin.lock_height, key_id)); } // track the output representing our change @@ -229,7 +228,7 @@ fn inputs_and_change( value: change as u64, status: OutputStatus::Unconfirmed, height: 0, - lock_height: 0, + lock_height: lock_height, is_coinbase: false, }); From 1e210296f3d431a8a7a65c3d5bba3fe5d872b463 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Mon, 30 Oct 2017 12:09:59 -0400 Subject: [PATCH 02/36] playing around with lock_height as part of the switch commitment hash --- api/src/types.rs | 10 +- chain/src/pipe.rs | 27 +++--- chain/src/types.rs | 11 ++- chain/tests/test_coinbase_maturity.rs | 2 +- core/src/core/block.rs | 64 ++++++++++--- core/src/core/build.rs | 57 ++++++++++-- core/src/core/mod.rs | 36 +++---- core/src/core/transaction.rs | 129 +++++++++++++++++++++----- wallet/src/checker.rs | 2 +- wallet/src/receiver.rs | 6 +- 10 files changed, 256 insertions(+), 88 deletions(-) diff --git a/api/src/types.rs b/api/src/types.rs index b3b56ba823..f028e17e9f 100644 --- a/api/src/types.rs +++ b/api/src/types.rs @@ -12,11 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -use core::core; +use std::sync::Arc; + +use core::{core, global}; +use core::core::hash::Hashed; use chain; -use util::secp::pedersen; use rest::*; use util; +use util::secp::pedersen; /// The state of the current fork tip #[derive(Serialize, Deserialize, Debug, Clone)] @@ -141,8 +144,6 @@ pub struct Output { pub proof: Option, /// The height of the block creating this output pub height: u64, - /// The lock height (earliest block this output can be spent) - pub lock_height: u64, } impl Output { @@ -170,7 +171,6 @@ impl Output { false => None, }, height: block_header.height, - lock_height: output.lock_height, } } } diff --git a/chain/src/pipe.rs b/chain/src/pipe.rs index 51d14b617f..cdc251225c 100644 --- a/chain/src/pipe.rs +++ b/chain/src/pipe.rs @@ -313,22 +313,23 @@ fn validate_block( for input in &b.inputs { if let Ok(output) = ctx.store.get_output_by_commit(&input.commitment()) { - - // check for any outputs with lock_heights greater than current block height - if output.lock_height > b.header.height { - return Err(Error::ImmatureCoinbase); - } - // and check that any coinbase output also lines up correctly // with the lock_height based on the original block header if output.features.contains(transaction::COINBASE_OUTPUT) { - if let Ok(output_header) = ctx.store - .get_block_header_by_output_commit(&input.commitment()) - { - if b.header.height <= output_header.height + global::coinbase_maturity() { - return Err(Error::ImmatureCoinbase); - } - }; + input.verify_lock_height(&output, b.header.height)?; + + // + // falling back to dealing with coinbase explicitly here + // + // if let Ok(output_header) = ctx.store + // .get_block_header_by_output_commit( + // &input.commitment(), + // ) + // { + // if b.header.height <= output_header.height + global::coinbase_maturity() { + // return Err(Error::ImmatureCoinbase); + // } + // }; }; }; } diff --git a/chain/src/types.rs b/chain/src/types.rs index a9cfab2790..b0039e063e 100644 --- a/chain/src/types.rs +++ b/chain/src/types.rs @@ -19,7 +19,7 @@ use std::io; use util::secp::pedersen::Commitment; use grin_store as store; -use core::core::{block, Block, BlockHeader, Output}; +use core::core::{Block, BlockHeader, block, Output, transaction}; use core::core::hash::{Hash, Hashed}; use core::core::target::Difficulty; use core::ser; @@ -58,6 +58,8 @@ pub enum Error { InvalidBlockHeight, /// One of the root hashes in the block is invalid InvalidRoot, + /// Something does not look right with the switch commitment + InvalidSwitchCommit, /// One of the inputs in the block has already been spent AlreadySpent(Commitment), /// An output with that commitment already exists (should be unique) @@ -80,6 +82,7 @@ pub enum Error { SumTreeErr(String), /// No chain exists and genesis block is required GenesisBlockRequired, + Transaction(transaction::Error), /// Anything else Other(String), } @@ -117,6 +120,12 @@ impl Error { } } +impl From for Error { + fn from(e: transaction::Error) -> Error { + Error::Transaction(e) + } +} + /// The tip of a fork. A handle to the fork ancestry from its leaf in the /// blockchain tree. References the max height and the latest and previous /// blocks diff --git a/chain/tests/test_coinbase_maturity.rs b/chain/tests/test_coinbase_maturity.rs index 764f818c3f..26bb667dfc 100644 --- a/chain/tests/test_coinbase_maturity.rs +++ b/chain/tests/test_coinbase_maturity.rs @@ -102,7 +102,7 @@ fn test_coinbase_maturity() { let amount = consensus::REWARD; let (coinbase_txn, _) = build::transaction( vec![ - build::input(amount, key_id1.clone()), + build::input(amount, block.header.height, key_id1.clone()), build::output(amount - 2, block.header.height, key_id2), build::with_fee(2), ], diff --git a/core/src/core/block.rs b/core/src/core/block.rs index c557b2221b..4df2cb21e1 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -19,9 +19,18 @@ use util; use util::{secp, static_secp_instance}; use std::collections::HashSet; -use core::Committed; -use core::{Input, Output, Proof, SwitchCommitHash, Transaction, TxKernel, COINBASE_KERNEL, - COINBASE_OUTPUT}; +use core::{ + Committed, + Input, + Output, + SwitchCommitKey, + SwitchCommitHash, + Proof, + TxKernel, + Transaction, + COINBASE_KERNEL, + COINBASE_OUTPUT +}; use consensus; use consensus::{exceeds_weight, reward, MINIMUM_DIFFICULTY, REWARD, VerifySortOrder}; use core::hash::{Hash, Hashed, ZERO_HASH}; @@ -561,10 +570,41 @@ impl Block { fees: u64, height: u64, ) -> Result<(Output, TxKernel), keychain::Error> { - let lock_height = height + global::coinbase_maturity(); + debug!( + LOGGER, + "Block::reward_output: {}, {}", + fees, + height, + ); let commit = keychain.commit(reward(fees), key_id)?; let switch_commit = keychain.switch_commit(key_id)?; - let switch_commit_hash = SwitchCommitHash::from_switch_commit(switch_commit); + + debug!( + LOGGER, + "Block::reward_output: switch_commit: {:?}", + switch_commit, + ); + + let lock_height = height + global::coinbase_maturity(); + debug!( + LOGGER, + "Block::reward_output: lock_height: {}, {}", + lock_height, + key_id, + ); + + let switch_commit_hash = SwitchCommitHash::from_switch_commit( + switch_commit, + SwitchCommitKey::from_lock_height(lock_height), + ); + + debug!( + LOGGER, + "Block::reward_output: switch_commit_hash: {}, {:?}", + lock_height, + switch_commit_hash, + ); + trace!( LOGGER, "Block reward - Pedersen Commit is: {:?}, Switch Commit is: {:?}", @@ -581,7 +621,6 @@ impl Block { let output = Output { features: COINBASE_OUTPUT, - lock_height: lock_height, commit: commit, switch_commit_hash: switch_commit_hash, proof: rproof, @@ -591,7 +630,7 @@ impl Block { let secp = secp.lock().unwrap(); let over_commit = secp.commit_value(reward(fees))?; let out_commit = output.commitment(); - let excess = secp.commit_sum(vec![out_commit], vec![over_commit])?; + let excess = keychain.secp().commit_sum(vec![out_commit], vec![over_commit])?; // NOTE: Remember we sign the fee *and* the lock_height. // For a coinbase output the fee is 0 and the lock_height is @@ -609,6 +648,7 @@ impl Block { excess_sig: excess_sig, fee: 0, // lock_height here is the height of the block (tx should be valid immediately) + // *not* the lock_height of the coinbase output (only spendable 1,000 blocks later) lock_height: height, }; Ok((output, proof)) @@ -643,7 +683,7 @@ mod test { key_id2: Identifier, ) -> Transaction { build::transaction( - vec![input(v, key_id1), output(3, key_id2), with_fee(2)], + vec![input(v, 0, key_id1), output(3, 0, key_id2), with_fee(2)], &keychain, ).map(|(tx, _)| tx) .unwrap() @@ -663,11 +703,11 @@ mod test { let mut parts = vec![]; for _ in 0..max_out { - parts.push(output(5, pks.pop().unwrap())); + parts.push(output(5, 0, pks.pop().unwrap())); } let now = Instant::now(); - parts.append(&mut vec![input(500000, pks.pop().unwrap()), with_fee(2)]); + parts.append(&mut vec![input(500000, 0, pks.pop().unwrap()), with_fee(2)]); let mut tx = build::transaction(parts, &keychain) .map(|(tx, _)| tx) .unwrap(); @@ -687,7 +727,7 @@ mod test { let mut btx1 = tx2i1o(); let (mut btx2, _) = build::transaction( - vec![input(7, key_id1), output(5, key_id2.clone()), with_fee(2)], + vec![input(7, 0, key_id1), output(5, 0, key_id2.clone()), with_fee(2)], &keychain, ).unwrap(); @@ -715,7 +755,7 @@ mod test { let mut btx1 = tx2i1o(); let (mut btx2, _) = build::transaction( - vec![input(7, key_id1), output(5, key_id2.clone()), with_fee(2)], + vec![input(7, 0, key_id1), output(5, 0, key_id2.clone()), with_fee(2)], &keychain, ).unwrap(); diff --git a/core/src/core/build.rs b/core/src/core/build.rs index f2fd29a78f..615b2af136 100644 --- a/core/src/core/build.rs +++ b/core/src/core/build.rs @@ -27,7 +27,7 @@ use util::{secp, static_secp_instance}; -use core::{Input, Output, SwitchCommitHash, Transaction, DEFAULT_OUTPUT}; +use core::{Transaction, Input, Output, SwitchCommitKey, SwitchCommitHash, DEFAULT_OUTPUT}; use core::transaction::kernel_sig_msg; use keychain; use keychain::{Keychain, BlindSum, BlindingFactor, Identifier}; @@ -44,10 +44,39 @@ 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, key_id: Identifier) -> Box { +pub fn input(value: u64, lock_height: u64, key_id: Identifier) -> Box { + debug!( + LOGGER, + "Building an input: {}, {}, {}", + value, + lock_height, + key_id, + ); + Box::new(move |build, (tx, sum)| -> (Transaction, BlindSum) { let commit = build.keychain.commit(value, &key_id).unwrap(); - (tx.with_input(Input(commit)), sum.sub_key_id(key_id.clone())) + let switch_commit = build.keychain.switch_commit(&key_id).unwrap(); + + debug!( + LOGGER, + "built switch_commit for input: {}, {:?}", + lock_height, + switch_commit, + ); + + let switch_commit_hash = SwitchCommitHash::from_switch_commit( + switch_commit, + SwitchCommitKey::from_lock_height(lock_height), + ); + debug!( + LOGGER, + "built temp switch_commit_hash for input: {}, {:?}", + lock_height, + switch_commit_hash, + ); + + let input = Input::new(commit, switch_commit, lock_height); + (tx.with_input(input), sum.sub_key_id(key_id.clone())) }) } @@ -55,11 +84,20 @@ pub fn input(value: u64, key_id: Identifier) -> Box { /// keychain. pub fn output(value: u64, lock_height: u64, key_id: Identifier) -> Box { Box::new(move |build, (tx, sum)| -> (Transaction, BlindSum) { - println!("************* building an output ********** {}, {}", value, lock_height); + debug!( + LOGGER, + "Building an output: {}, {}, {}", + value, + lock_height, + key_id, + ); let commit = build.keychain.commit(value, &key_id).unwrap(); let switch_commit = build.keychain.switch_commit(&key_id).unwrap(); - let switch_commit_hash = SwitchCommitHash::from_switch_commit(switch_commit); + let switch_commit_hash = SwitchCommitHash::from_switch_commit( + switch_commit, + SwitchCommitKey::from_lock_height(lock_height), + ); trace!( LOGGER, "Builder - Pedersen Commit is: {:?}, Switch Commit is: {:?}", @@ -80,7 +118,6 @@ pub fn output(value: u64, lock_height: u64, key_id: Identifier) -> Box { ( tx.with_output(Output { features: DEFAULT_OUTPUT, - lock_height: lock_height, commit: commit, switch_commit_hash: switch_commit_hash, proof: rproof, @@ -164,9 +201,9 @@ mod test { let (tx, _) = transaction( vec![ - input(10, key_id1), - input(11, key_id2), - output(20, key_id3), + input(10, 0, key_id1), + input(11, 0, key_id2), + output(20, 0, key_id3), with_fee(1), ], &keychain, @@ -182,7 +219,7 @@ mod test { let key_id2 = keychain.derive_key_id(2).unwrap(); let (tx, _) = transaction( - vec![input(6, key_id1), output(2, key_id2), with_fee(4)], + vec![input(6, 0, key_id1), output(2, 0, key_id2), with_fee(4)], &keychain, ).unwrap(); diff --git a/core/src/core/mod.rs b/core/src/core/mod.rs index 798da30a36..685e39a632 100644 --- a/core/src/core/mod.rs +++ b/core/src/core/mod.rs @@ -247,8 +247,8 @@ mod test { // blinding should fail as signing with a zero r*G shouldn't work build::transaction( vec![ - input(10, key_id1.clone()), - output(9, key_id1.clone()), + input(10, 0, key_id1.clone()), + output(9, 0, key_id1.clone()), with_fee(1), ], &keychain, @@ -260,8 +260,8 @@ mod test { let tx = tx2i1o(); let mut vec = Vec::new(); ser::serialize(&mut vec, &tx).expect("serialized failed"); - assert!(vec.len() > 5360); - assert!(vec.len() < 5380); + assert!(vec.len() > 5440); + assert!(vec.len() < 5450); } #[test] @@ -302,9 +302,9 @@ mod test { let (tx, _) = build::transaction( vec![ - input(75, key_id1), - output(42, key_id2), - output(32, key_id3), + input(75, 0, key_id1), + output(42, 0, key_id2), + output(32, 0, key_id3), with_fee(1), ], &keychain, @@ -357,12 +357,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, key_id1), input(3, key_id2)); + let (in1, in2) = (input(4, 0, key_id1), input(3, 0, 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, key_id3), with_fee(2)], &keychain) + build::transaction(vec![in1, in2, output(1, 0, key_id3), with_fee(2)], &keychain) .unwrap(); tx_alice = tx; blind_sum = sum; @@ -375,7 +375,7 @@ mod test { vec![ initial_tx(tx_alice), with_excess(blind_sum), - output(4, key_id4), + output(4, 0, key_id4), ], &keychain, ).unwrap(); @@ -433,8 +433,8 @@ mod test { // and that the resulting block is valid let tx1 = build::transaction( vec![ - input(5, key_id1.clone()), - output(3, key_id2.clone()), + input(5, 0, key_id1.clone()), + output(3, 0, key_id2.clone()), with_fee(2), with_lock_height(1), ], @@ -453,8 +453,8 @@ mod test { // now try adding a timelocked tx where lock height is greater than current block height let tx1 = build::transaction( vec![ - input(5, key_id1.clone()), - output(3, key_id2.clone()), + input(5, 0, key_id1.clone()), + output(3, 0, key_id2.clone()), with_fee(2), with_lock_height(2), ], @@ -497,9 +497,9 @@ mod test { build::transaction( vec![ - input(10, key_id1), - input(11, key_id2), - output(19, key_id3), + input(10, 0, key_id1), + input(11, 0, key_id2), + output(19, 0, key_id3), with_fee(2), ], &keychain, @@ -514,7 +514,7 @@ mod test { let key_id2 = keychain.derive_key_id(2).unwrap(); build::transaction( - vec![input(5, key_id1), output(3, key_id2), with_fee(2)], + vec![input(5, 0, key_id1), output(3, 0, key_id2), with_fee(2)], &keychain, ).map(|(tx, _)| tx) .unwrap() diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 50c7fa75e6..b86bd1d214 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -34,6 +34,8 @@ use ser::{self, read_and_verify_sorted, Readable, Reader, Writeable, WriteableSo /// The size to use for the stored blake2 hash of a switch_commitment pub const SWITCH_COMMIT_HASH_SIZE: usize = 20; +pub const SWITCH_COMMIT_KEY_SIZE: usize = 20; + bitflags! { /// Options for a kernel's structure or use pub flags KernelFeatures: u8 { @@ -72,13 +74,14 @@ macro_rules! hashable_ord { pub enum Error { /// Transaction fee can't be odd, due to half fee burning OddFee, - /// Underlying Secp256k1 error (signature validation or invalid public - /// key typically) + /// Underlying Secp256k1 error (signature validation or invalid public key typically) Secp(secp::Error), /// Restrict number of incoming inputs TooManyInputs, /// Underlying consensus error (currently for sort order) ConsensusError(consensus::Error), + LockHeight(u64), + SwitchCommitment, } impl From for Error { @@ -388,8 +391,12 @@ impl Transaction { /// A transaction input, mostly a reference to an output being spent by the /// transaction. -#[derive(Debug, Copy, Clone)] -pub struct Input(pub Commitment); +#[derive(Debug, Clone, Copy)] +pub struct Input{ + commit: Commitment, + switch_commit: Commitment, + lock_height: u64, +} hashable_ord!(Input); @@ -397,7 +404,10 @@ hashable_ord!(Input); /// an Input as binary. impl Writeable for Input { fn write(&self, writer: &mut W) -> Result<(), ser::Error> { - writer.write_fixed_bytes(&self.0) + writer.write_fixed_bytes(&self.commit)?; + writer.write_fixed_bytes(&self.switch_commit)?; + writer.write_u64(self.lock_height)?; + Ok(()) } } @@ -405,16 +415,63 @@ impl Writeable for Input { /// an Input from a binary stream. impl Readable for Input { fn read(reader: &mut Reader) -> Result { - Ok(Input(Commitment::read(reader)?)) + Ok(Input { + commit: Commitment::read(reader)?, + switch_commit: Commitment::read(reader)?, + lock_height: reader.read_u64()?, + }) } } /// The input for a transaction, which spends a pre-existing output. The input /// commitment is a reproduction of the commitment of the output it's spending. impl Input { + pub fn new(commit: Commitment, switch_commit: Commitment, lock_height: u64) -> Input { + debug!( + LOGGER, + "building a new input: {:?}, {:?}, {}", + commit, + switch_commit, + lock_height, + ); + Input { + commit, + switch_commit, + lock_height, + } + } + /// Extracts the referenced commitment from a transaction output pub fn commitment(&self) -> Commitment { - self.0 + self.commit + } + + /// Given the output being spent by this input along with the current chain height + /// we can verify the output has sufficiently matured and is spendable. + /// We do this by reconstructing the switch_commit_hash from the switch_commit. + /// The lock_height is passed in as the secret key to the hash so we can verify + /// we are not being lied to about the lock_height. + /// + /// Note: we are only enforcing this for coinbase outputs currently. + /// + pub fn verify_lock_height( + &self, + output: &Output, + height: u64, + ) -> Result<(), Error> { + let switch_commit_hash = SwitchCommitHash::from_switch_commit( + self.switch_commit, + SwitchCommitKey::from_lock_height(self.lock_height), + ); + + if switch_commit_hash != output.switch_commit_hash { + return Err(Error::SwitchCommitment); + } else { + if height <= self.lock_height { + return Err(Error::LockHeight(self.lock_height)); + } + } + Ok(()) } } @@ -429,6 +486,17 @@ bitflags! { } } +pub struct SwitchCommitKey ([u8; SWITCH_COMMIT_KEY_SIZE]); + +impl SwitchCommitKey { + // TODO - pass the output features in here also? How to do this cleanly? + pub fn from_lock_height(lock_height: u64) -> SwitchCommitKey { + let mut bytes = [0; SWITCH_COMMIT_KEY_SIZE]; + BigEndian::write_u64(&mut bytes[0..8], lock_height); + SwitchCommitKey(bytes) + } +} + /// Definition of the switch commitment hash #[derive(Debug, Copy, Clone, PartialEq, Serialize, Deserialize)] pub struct SwitchCommitHash { @@ -436,10 +504,14 @@ pub struct SwitchCommitHash { pub hash: [u8; SWITCH_COMMIT_HASH_SIZE], } +/// Definition of the switch commitment hash +#[derive(Debug, Copy, Clone, PartialEq, Serialize, Deserialize)] +pub struct SwitchCommitHash([u8; SWITCH_COMMIT_HASH_SIZE]); + /// Implementation of Writeable for a switch commitment hash impl Writeable for SwitchCommitHash { fn write(&self, writer: &mut W) -> Result<(), ser::Error> { - writer.write_fixed_bytes(&self.hash)?; + writer.write_fixed_bytes(&self.0)?; Ok(()) } } @@ -453,26 +525,32 @@ impl Readable for SwitchCommitHash { for i in 0..SWITCH_COMMIT_HASH_SIZE { c[i] = a[i]; } - Ok(SwitchCommitHash { hash: c }) + Ok(SwitchCommitHash(c)) } } // As Ref for AsFixedBytes impl AsRef<[u8]> for SwitchCommitHash { fn as_ref(&self) -> &[u8] { - &self.hash + &self.0 } } impl SwitchCommitHash { /// Builds a switch commitment hash from a switch commit using blake2 - pub fn from_switch_commit(switch_commit: Commitment) -> SwitchCommitHash { - let switch_commit_hash = blake2b(SWITCH_COMMIT_HASH_SIZE, &[], &switch_commit.0); + pub fn from_switch_commit(switch_commit: Commitment, key: SwitchCommitKey) -> SwitchCommitHash { + debug!( + LOGGER, + "SwitchCommitHash::from_switch_commit: {:?}, {:?}", + switch_commit, + key, + ); + let switch_commit_hash = blake2b(SWITCH_COMMIT_HASH_SIZE, &key.0, &switch_commit.0); let switch_commit_hash = switch_commit_hash.as_bytes(); let mut h = [0; SWITCH_COMMIT_HASH_SIZE]; for i in 0..SWITCH_COMMIT_HASH_SIZE { h[i] = switch_commit_hash[i]; } - SwitchCommitHash { hash: h } + SwitchCommitHash(h) } } @@ -480,20 +558,18 @@ impl SwitchCommitHash { /// transferred. The commitment is a blinded value for the output while the /// range proof guarantees the commitment includes a positive value without /// overflow and the ownership of the private key. The switch commitment hash -/// provides future-proofing against quantum-based attacks, as well as provides +/// provides future-proofing against quantum-based attacks, as well as providing /// wallet implementations with a way to identify their outputs for wallet -/// reconstruction +/// reconstruction. /// -/// The hash of an output only covers its features, lock_height, commitment, +/// The hash of an output only covers its features, commitment, /// and switch commitment. The range proof is expected to have its own hash /// and is stored and committed to separately. #[derive(Debug, Copy, Clone, Serialize, Deserialize)] pub struct Output { /// Options for an output's structure or use pub features: OutputFeatures, - /// This output cannot be spent earlier than block at lock_height - pub lock_height: u64, - /// The homomorphic commitment representing the output's amount + /// The homomorphic commitment representing the output amount pub commit: Commitment, /// The switch commitment hash, a 160 bit length blake2 hash of blind*J pub switch_commit_hash: SwitchCommitHash, @@ -508,7 +584,6 @@ hashable_ord!(Output); impl Writeable for Output { fn write(&self, writer: &mut W) -> Result<(), ser::Error> { writer.write_u8(self.features.bits())?; - writer.write_u64(self.lock_height)?; writer.write_fixed_bytes(&self.commit)?; writer.write_fixed_bytes(&self.switch_commit_hash)?; @@ -530,7 +605,6 @@ impl Readable for Output { Ok(Output { features: features, - lock_height: reader.read_u64()?, commit: Commitment::read(reader)?, switch_commit_hash: SwitchCommitHash::read(reader)?, proof: RangeProof::read(reader)?, @@ -635,6 +709,7 @@ impl ops::Add for SumCommit { mod test { use super::*; use keychain::Keychain; + use util; use util::secp; #[test] @@ -688,13 +763,15 @@ mod test { let key_id = keychain.derive_key_id(1).unwrap(); let commit = keychain.commit(5, &key_id).unwrap(); let switch_commit = keychain.switch_commit(&key_id).unwrap(); - let switch_commit_hash = SwitchCommitHash::from_switch_commit(switch_commit); + let switch_commit_hash = SwitchCommitHash::from_switch_commit( + switch_commit, + SwitchCommitKey::from_lock_height(0), + ); let msg = secp::pedersen::ProofMessage::empty(); let proof = keychain.range_proof(5, &key_id, commit, msg).unwrap(); let out = Output { features: DEFAULT_OUTPUT, - lock_height: 0, commit: commit, switch_commit_hash: switch_commit_hash, proof: proof, @@ -716,13 +793,15 @@ mod test { let commit = keychain.commit(1003, &key_id).unwrap(); let switch_commit = keychain.switch_commit(&key_id).unwrap(); - let switch_commit_hash = SwitchCommitHash::from_switch_commit(switch_commit); + let switch_commit_hash = SwitchCommitHash::from_switch_commit( + switch_commit, + SwitchCommitKey::from_lock_height(0), + ); let msg = secp::pedersen::ProofMessage::empty(); let proof = keychain.range_proof(1003, &key_id, commit, msg).unwrap(); let output = Output { features: DEFAULT_OUTPUT, - lock_height: 0, commit: commit, switch_commit_hash: switch_commit_hash, proof: proof, diff --git a/wallet/src/checker.rs b/wallet/src/checker.rs index 38a5abe1f0..684cae0893 100644 --- a/wallet/src/checker.rs +++ b/wallet/src/checker.rs @@ -29,7 +29,7 @@ use util::LOGGER; // Also updates the height and lock_height based on latest from the api. fn refresh_output(out: &mut OutputData, api_out: &api::Output) { out.height = api_out.height; - out.lock_height = api_out.lock_height; + // out.lock_height = api_out.lock_height; match out.status { OutputStatus::Unconfirmed => { diff --git a/wallet/src/receiver.rs b/wallet/src/receiver.rs index 1fa256c413..611d627256 100644 --- a/wallet/src/receiver.rs +++ b/wallet/src/receiver.rs @@ -25,7 +25,7 @@ use serde_json; use api; use core::consensus::reward; use core::core::{build, Block, Output, Transaction, TxKernel}; -use core::ser; +use core::{global, ser}; use keychain::{BlindingFactor, Identifier, Keychain}; use types::*; use util; @@ -122,6 +122,8 @@ pub fn receive_coinbase( ) -> Result<(Output, TxKernel, BlockFees), Error> { let root_key_id = keychain.root_key_id(); + let output_lock_height = block_fees.height + global::coinbase_maturity(); + // Now acquire the wallet lock and write the new output. let (key_id, derivation) = WalletData::with_wallet(&config.data_file_dir, |wallet_data| { let key_id = block_fees.key_id(); @@ -138,7 +140,7 @@ pub fn receive_coinbase( value: reward(block_fees.fees), status: OutputStatus::Unconfirmed, height: 0, - lock_height: 0, + lock_height: output_lock_height, is_coinbase: true, }); From 1bb9ed7ff6edd641977873018fab4b40640335fb Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Thu, 2 Nov 2017 15:16:06 -0400 Subject: [PATCH 03/36] cleanup --- core/src/core/block.rs | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/core/src/core/block.rs b/core/src/core/block.rs index 4df2cb21e1..ac59a33f80 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -570,41 +570,16 @@ impl Block { fees: u64, height: u64, ) -> Result<(Output, TxKernel), keychain::Error> { - debug!( - LOGGER, - "Block::reward_output: {}, {}", - fees, - height, - ); let commit = keychain.commit(reward(fees), key_id)?; let switch_commit = keychain.switch_commit(key_id)?; - debug!( - LOGGER, - "Block::reward_output: switch_commit: {:?}", - switch_commit, - ); - let lock_height = height + global::coinbase_maturity(); - debug!( - LOGGER, - "Block::reward_output: lock_height: {}, {}", - lock_height, - key_id, - ); let switch_commit_hash = SwitchCommitHash::from_switch_commit( switch_commit, SwitchCommitKey::from_lock_height(lock_height), ); - debug!( - LOGGER, - "Block::reward_output: switch_commit_hash: {}, {:?}", - lock_height, - switch_commit_hash, - ); - trace!( LOGGER, "Block reward - Pedersen Commit is: {:?}, Switch Commit is: {:?}", From d13b39b70faf27f96d897ac2b00c9328007e3e72 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Thu, 2 Nov 2017 15:38:37 -0400 Subject: [PATCH 04/36] include features in the switch commit hash key --- core/src/core/block.rs | 2 +- core/src/core/build.rs | 27 ++++++--------------------- core/src/core/transaction.rs | 12 ++++++------ 3 files changed, 13 insertions(+), 28 deletions(-) diff --git a/core/src/core/block.rs b/core/src/core/block.rs index ac59a33f80..3ce2e3fbee 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -577,7 +577,7 @@ impl Block { let switch_commit_hash = SwitchCommitHash::from_switch_commit( switch_commit, - SwitchCommitKey::from_lock_height(lock_height), + SwitchCommitKey::from_features_and_lock_height(COINBASE_OUTPUT, lock_height), ); trace!( diff --git a/core/src/core/build.rs b/core/src/core/build.rs index 615b2af136..81610808c5 100644 --- a/core/src/core/build.rs +++ b/core/src/core/build.rs @@ -44,7 +44,11 @@ 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, lock_height: u64, key_id: Identifier) -> Box { +pub fn input( + value: u64, + lock_height: u64, + key_id: Identifier +) -> Box { debug!( LOGGER, "Building an input: {}, {}, {}", @@ -56,25 +60,6 @@ pub fn input(value: u64, lock_height: u64, key_id: Identifier) -> Box { Box::new(move |build, (tx, sum)| -> (Transaction, BlindSum) { let commit = build.keychain.commit(value, &key_id).unwrap(); let switch_commit = build.keychain.switch_commit(&key_id).unwrap(); - - debug!( - LOGGER, - "built switch_commit for input: {}, {:?}", - lock_height, - switch_commit, - ); - - let switch_commit_hash = SwitchCommitHash::from_switch_commit( - switch_commit, - SwitchCommitKey::from_lock_height(lock_height), - ); - debug!( - LOGGER, - "built temp switch_commit_hash for input: {}, {:?}", - lock_height, - switch_commit_hash, - ); - let input = Input::new(commit, switch_commit, lock_height); (tx.with_input(input), sum.sub_key_id(key_id.clone())) }) @@ -96,7 +81,7 @@ pub fn output(value: u64, lock_height: u64, key_id: Identifier) -> Box { let switch_commit = build.keychain.switch_commit(&key_id).unwrap(); let switch_commit_hash = SwitchCommitHash::from_switch_commit( switch_commit, - SwitchCommitKey::from_lock_height(lock_height), + SwitchCommitKey::from_features_and_lock_height(DEFAULT_OUTPUT, lock_height), ); trace!( LOGGER, diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index b86bd1d214..59934a9768 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -461,7 +461,7 @@ impl Input { ) -> Result<(), Error> { let switch_commit_hash = SwitchCommitHash::from_switch_commit( self.switch_commit, - SwitchCommitKey::from_lock_height(self.lock_height), + SwitchCommitKey::from_features_and_lock_height(output.features, self.lock_height), ); if switch_commit_hash != output.switch_commit_hash { @@ -489,10 +489,10 @@ bitflags! { pub struct SwitchCommitKey ([u8; SWITCH_COMMIT_KEY_SIZE]); impl SwitchCommitKey { - // TODO - pass the output features in here also? How to do this cleanly? - pub fn from_lock_height(lock_height: u64) -> SwitchCommitKey { + pub fn from_features_and_lock_height(features: OutputFeatures, lock_height: u64) -> SwitchCommitKey { let mut bytes = [0; SWITCH_COMMIT_KEY_SIZE]; - BigEndian::write_u64(&mut bytes[0..8], lock_height); + bytes[0] = features.bits(); + BigEndian::write_u64(&mut bytes[1..9], lock_height); SwitchCommitKey(bytes) } } @@ -765,7 +765,7 @@ mod test { let switch_commit = keychain.switch_commit(&key_id).unwrap(); let switch_commit_hash = SwitchCommitHash::from_switch_commit( switch_commit, - SwitchCommitKey::from_lock_height(0), + SwitchCommitKey::from_features_and_lock_height(DEFAULT_OUTPUT, 0), ); let msg = secp::pedersen::ProofMessage::empty(); let proof = keychain.range_proof(5, &key_id, commit, msg).unwrap(); @@ -795,7 +795,7 @@ mod test { let switch_commit = keychain.switch_commit(&key_id).unwrap(); let switch_commit_hash = SwitchCommitHash::from_switch_commit( switch_commit, - SwitchCommitKey::from_lock_height(0), + SwitchCommitKey::from_features_and_lock_height(DEFAULT_OUTPUT, 0), ); let msg = secp::pedersen::ProofMessage::empty(); let proof = keychain.range_proof(1003, &key_id, commit, msg).unwrap(); From ba22ed5a1a17ea48c8addc5d92b6f2d73366786d Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Thu, 2 Nov 2017 21:20:32 -0400 Subject: [PATCH 05/36] commit --- core/src/core/block.rs | 4 ++-- core/src/core/build.rs | 4 ++-- core/src/core/transaction.rs | 31 ++++++++++++++----------------- wallet/src/receiver.rs | 2 +- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/core/src/core/block.rs b/core/src/core/block.rs index 3ce2e3fbee..9bb19ce0e8 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -23,7 +23,7 @@ use core::{ Committed, Input, Output, - SwitchCommitKey, + SwitchCommitHashKey, SwitchCommitHash, Proof, TxKernel, @@ -577,7 +577,7 @@ impl Block { let switch_commit_hash = SwitchCommitHash::from_switch_commit( switch_commit, - SwitchCommitKey::from_features_and_lock_height(COINBASE_OUTPUT, lock_height), + SwitchCommitHashKey::from_features_and_lock_height(COINBASE_OUTPUT, lock_height), ); trace!( diff --git a/core/src/core/build.rs b/core/src/core/build.rs index 81610808c5..cf65b2a14b 100644 --- a/core/src/core/build.rs +++ b/core/src/core/build.rs @@ -27,7 +27,7 @@ use util::{secp, static_secp_instance}; -use core::{Transaction, Input, Output, SwitchCommitKey, SwitchCommitHash, DEFAULT_OUTPUT}; +use core::{Transaction, Input, Output, SwitchCommitHashKey, SwitchCommitHash, DEFAULT_OUTPUT}; use core::transaction::kernel_sig_msg; use keychain; use keychain::{Keychain, BlindSum, BlindingFactor, Identifier}; @@ -81,7 +81,7 @@ pub fn output(value: u64, lock_height: u64, key_id: Identifier) -> Box { let switch_commit = build.keychain.switch_commit(&key_id).unwrap(); let switch_commit_hash = SwitchCommitHash::from_switch_commit( switch_commit, - SwitchCommitKey::from_features_and_lock_height(DEFAULT_OUTPUT, lock_height), + SwitchCommitHashKey::from_features_and_lock_height(DEFAULT_OUTPUT, lock_height), ); trace!( LOGGER, diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 59934a9768..43f0890847 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -189,8 +189,7 @@ pub struct Transaction { pub outputs: Vec, /// Fee paid by the transaction. pub fee: u64, - /// Transaction is not valid before this block height. - /// It is invalid for this to be less than the lock_height of any UTXO being spent. + /// Transaction is not valid before this chain height. pub lock_height: u64, /// The signature proving the excess is a valid public key, which signs /// the transaction fee. @@ -461,7 +460,7 @@ impl Input { ) -> Result<(), Error> { let switch_commit_hash = SwitchCommitHash::from_switch_commit( self.switch_commit, - SwitchCommitKey::from_features_and_lock_height(output.features, self.lock_height), + SwitchCommitHashKey::from_features_and_lock_height(output.features, self.lock_height), ); if switch_commit_hash != output.switch_commit_hash { @@ -486,14 +485,18 @@ bitflags! { } } -pub struct SwitchCommitKey ([u8; SWITCH_COMMIT_KEY_SIZE]); +/// Definition of the switch commitment hash +#[derive(Debug, Copy, Clone, PartialEq, Serialize, Deserialize)] +pub struct SwitchCommitHashKey ([u8; SWITCH_COMMIT_KEY_SIZE]); -impl SwitchCommitKey { - pub fn from_features_and_lock_height(features: OutputFeatures, lock_height: u64) -> SwitchCommitKey { +impl SwitchCommitHashKey { + pub fn from_features_and_lock_height(features: OutputFeatures, lock_height: u64) -> SwitchCommitHashKey { let mut bytes = [0; SWITCH_COMMIT_KEY_SIZE]; bytes[0] = features.bits(); - BigEndian::write_u64(&mut bytes[1..9], lock_height); - SwitchCommitKey(bytes) + // seems wasteful to take up a full 8 bytes (of 20 bytes) to store the lock_height + // 4 bytes will give us approx 4,000 years with 1 min blocks (unless my math is way off) + BigEndian::write_u32(&mut bytes[1..5], lock_height as u32); + SwitchCommitHashKey(bytes) } } @@ -537,13 +540,7 @@ impl AsRef<[u8]> for SwitchCommitHash { impl SwitchCommitHash { /// Builds a switch commitment hash from a switch commit using blake2 - pub fn from_switch_commit(switch_commit: Commitment, key: SwitchCommitKey) -> SwitchCommitHash { - debug!( - LOGGER, - "SwitchCommitHash::from_switch_commit: {:?}, {:?}", - switch_commit, - key, - ); + pub fn from_switch_commit(switch_commit: Commitment, key: SwitchCommitHashKey) -> SwitchCommitHash { let switch_commit_hash = blake2b(SWITCH_COMMIT_HASH_SIZE, &key.0, &switch_commit.0); let switch_commit_hash = switch_commit_hash.as_bytes(); let mut h = [0; SWITCH_COMMIT_HASH_SIZE]; @@ -765,7 +762,7 @@ mod test { let switch_commit = keychain.switch_commit(&key_id).unwrap(); let switch_commit_hash = SwitchCommitHash::from_switch_commit( switch_commit, - SwitchCommitKey::from_features_and_lock_height(DEFAULT_OUTPUT, 0), + SwitchCommitHashKey::from_features_and_lock_height(DEFAULT_OUTPUT, 0), ); let msg = secp::pedersen::ProofMessage::empty(); let proof = keychain.range_proof(5, &key_id, commit, msg).unwrap(); @@ -795,7 +792,7 @@ mod test { let switch_commit = keychain.switch_commit(&key_id).unwrap(); let switch_commit_hash = SwitchCommitHash::from_switch_commit( switch_commit, - SwitchCommitKey::from_features_and_lock_height(DEFAULT_OUTPUT, 0), + SwitchCommitHashKey::from_features_and_lock_height(DEFAULT_OUTPUT, 0), ); let msg = secp::pedersen::ProofMessage::empty(); let proof = keychain.range_proof(1003, &key_id, commit, msg).unwrap(); diff --git a/wallet/src/receiver.rs b/wallet/src/receiver.rs index 611d627256..eaa243fbf2 100644 --- a/wallet/src/receiver.rs +++ b/wallet/src/receiver.rs @@ -160,7 +160,7 @@ pub fn receive_coinbase( let mut block_fees = block_fees.clone(); block_fees.key_id = Some(key_id.clone()); - debug!(LOGGER, "block_fees updated - {:?}", block_fees); + debug!(LOGGER, "block_fees updated key_id - {:?}", block_fees.key_id); let (out, kern) = Block::reward_output( &keychain, From 1267fd5b136da020547ccef004fb98e9fc3cc158 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Mon, 13 Nov 2017 10:56:00 -0500 Subject: [PATCH 06/36] rebase off master --- core/src/core/block.rs | 2 +- core/src/core/transaction.rs | 16 +++++++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/core/src/core/block.rs b/core/src/core/block.rs index 9bb19ce0e8..b1ef9fb4d4 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -35,8 +35,8 @@ use consensus; use consensus::{exceeds_weight, reward, MINIMUM_DIFFICULTY, REWARD, VerifySortOrder}; use core::hash::{Hash, Hashed, ZERO_HASH}; use core::target::Difficulty; +use core::transaction::{self, kernel_sig_msg}; use ser::{self, Readable, Reader, Writeable, Writer, WriteableSorted, read_and_verify_sorted}; -use core::transaction::kernel_sig_msg; use util::LOGGER; use global; use keychain; diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 43f0890847..80350f9d69 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -30,6 +30,7 @@ use core::pmmr::Summable; use global; use keychain::{Identifier, Keychain}; use ser::{self, read_and_verify_sorted, Readable, Reader, Writeable, WriteableSorted, Writer}; +use util::LOGGER; /// The size to use for the stored blake2 hash of a switch_commitment pub const SWITCH_COMMIT_HASH_SIZE: usize = 20; @@ -490,12 +491,17 @@ bitflags! { pub struct SwitchCommitHashKey ([u8; SWITCH_COMMIT_KEY_SIZE]); impl SwitchCommitHashKey { - pub fn from_features_and_lock_height(features: OutputFeatures, lock_height: u64) -> SwitchCommitHashKey { + pub fn from_features_and_lock_height( + features: OutputFeatures, + lock_height: u64 + ) -> SwitchCommitHashKey { let mut bytes = [0; SWITCH_COMMIT_KEY_SIZE]; - bytes[0] = features.bits(); - // seems wasteful to take up a full 8 bytes (of 20 bytes) to store the lock_height - // 4 bytes will give us approx 4,000 years with 1 min blocks (unless my math is way off) - BigEndian::write_u32(&mut bytes[1..5], lock_height as u32); + if features.contains(COINBASE_OUTPUT) { + bytes[0] = features.bits(); + // seems wasteful to take up a full 8 bytes (of 20 bytes) to store the lock_height + // 4 bytes will give us approx 4,000 years with 1 min blocks (unless my math is way off) + BigEndian::write_u32(&mut bytes[1..5], lock_height as u32); + } SwitchCommitHashKey(bytes) } } From 13acb428f588c7e003a2ca895db19122cf16e9a0 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Mon, 13 Nov 2017 11:04:56 -0500 Subject: [PATCH 07/36] commit --- core/src/core/transaction.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 80350f9d69..0eab72ad54 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -30,6 +30,7 @@ use core::pmmr::Summable; use global; use keychain::{Identifier, Keychain}; use ser::{self, read_and_verify_sorted, Readable, Reader, Writeable, WriteableSorted, Writer}; +use util; use util::LOGGER; /// The size to use for the stored blake2 hash of a switch_commitment @@ -514,7 +515,7 @@ pub struct SwitchCommitHash { } /// Definition of the switch commitment hash -#[derive(Debug, Copy, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Copy, Clone, PartialEq, Serialize, Deserialize)] pub struct SwitchCommitHash([u8; SWITCH_COMMIT_HASH_SIZE]); /// Implementation of Writeable for a switch commitment hash @@ -544,6 +545,14 @@ impl AsRef<[u8]> for SwitchCommitHash { } } +impl ::std::fmt::Debug for SwitchCommitHash { + fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result { + try!(write!(f, "{}(", stringify!(SwitchCommitHash))); + try!(write!(f, "{}", self.to_hex())); + write!(f, ")") + } +} + impl SwitchCommitHash { /// Builds a switch commitment hash from a switch commit using blake2 pub fn from_switch_commit(switch_commit: Commitment, key: SwitchCommitHashKey) -> SwitchCommitHash { @@ -555,6 +564,10 @@ impl SwitchCommitHash { } SwitchCommitHash(h) } + + pub fn to_hex(&self) -> String { + util::to_hex(self.0.to_vec()) + } } /// Output for a transaction, defining the new ownership of coins that are being From 8de1189e5aba1014b309cf90c8e09eaf2f7ccbb2 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Mon, 13 Nov 2017 11:10:14 -0500 Subject: [PATCH 08/36] cleanup --- chain/src/pipe.rs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/chain/src/pipe.rs b/chain/src/pipe.rs index cdc251225c..2ba71da275 100644 --- a/chain/src/pipe.rs +++ b/chain/src/pipe.rs @@ -313,23 +313,8 @@ fn validate_block( for input in &b.inputs { if let Ok(output) = ctx.store.get_output_by_commit(&input.commitment()) { - // and check that any coinbase output also lines up correctly - // with the lock_height based on the original block header if output.features.contains(transaction::COINBASE_OUTPUT) { input.verify_lock_height(&output, b.header.height)?; - - // - // falling back to dealing with coinbase explicitly here - // - // if let Ok(output_header) = ctx.store - // .get_block_header_by_output_commit( - // &input.commitment(), - // ) - // { - // if b.header.height <= output_header.height + global::coinbase_maturity() { - // return Err(Error::ImmatureCoinbase); - // } - // }; }; }; } From 27ca4356834c50e4950679eb8c3f1ad0e4cf4198 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Mon, 13 Nov 2017 11:11:20 -0500 Subject: [PATCH 09/36] missing docs --- chain/src/types.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/chain/src/types.rs b/chain/src/types.rs index b0039e063e..15167bcf21 100644 --- a/chain/src/types.rs +++ b/chain/src/types.rs @@ -82,6 +82,7 @@ pub enum Error { SumTreeErr(String), /// No chain exists and genesis block is required GenesisBlockRequired, + /// Error from underlying tx handling Transaction(transaction::Error), /// Anything else Other(String), From 70a99f4c538dc211222116c5505a40edf54680a2 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Mon, 13 Nov 2017 15:08:03 -0500 Subject: [PATCH 10/36] rework coinbase maturity test to build valid tx --- chain/src/pipe.rs | 2 +- chain/tests/test_coinbase_maturity.rs | 44 ++++++++++++++++++++++----- core/src/core/transaction.rs | 5 ++- 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/chain/src/pipe.rs b/chain/src/pipe.rs index 2ba71da275..7317bb4a5e 100644 --- a/chain/src/pipe.rs +++ b/chain/src/pipe.rs @@ -314,7 +314,7 @@ fn validate_block( for input in &b.inputs { if let Ok(output) = ctx.store.get_output_by_commit(&input.commitment()) { if output.features.contains(transaction::COINBASE_OUTPUT) { - input.verify_lock_height(&output, b.header.height)?; + input.verify_lock_height(&output, b.header.height).map_err(|_| Error::ImmatureCoinbase)?; }; }; } diff --git a/chain/tests/test_coinbase_maturity.rs b/chain/tests/test_coinbase_maturity.rs index 26bb667dfc..434a5f59c3 100644 --- a/chain/tests/test_coinbase_maturity.rs +++ b/chain/tests/test_coinbase_maturity.rs @@ -99,18 +99,28 @@ fn test_coinbase_maturity() { let prev = chain.head_header().unwrap(); + let height = prev.height; + assert_eq!(height, 1); + let amount = consensus::REWARD; + + // here we build a tx that attempts to spend the earlier coinbase output + // this is not a valid tx as the coinbase output cannot be spent yet let (coinbase_txn, _) = build::transaction( vec![ - build::input(amount, block.header.height, key_id1.clone()), - build::output(amount - 2, block.header.height, key_id2), + build::input(amount, height, key_id1.clone()), + build::output(amount - 2, height, key_id2.clone()), build::with_fee(2), ], &keychain, ).unwrap(); - let mut block = - core::core::Block::new(&prev, vec![&coinbase_txn], &keychain, &key_id3).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(); @@ -124,14 +134,16 @@ fn test_coinbase_maturity() { global::sizeshift() as u32, ).unwrap(); - let result = chain.process_block(block, chain::NONE); + // confirm the block fails validation due to the invalid tx attempting to spend + // the immature coinbase + let result = chain.process_block(block, chain::EASY_POW); match result { Err(Error::ImmatureCoinbase) => (), _ => panic!("expected ImmatureCoinbase error here"), }; // mine enough blocks to increase the height sufficiently for - // coinbase to reach maturity and be spendable in the next block + // coinbase to reach maturity and be spendable in the next block for _ in 0..3 { let prev = chain.head_header().unwrap(); @@ -157,8 +169,24 @@ fn test_coinbase_maturity() { let prev = chain.head_header().unwrap(); - let mut block = - core::core::Block::new(&prev, vec![&coinbase_txn], &keychain, &key_id4).unwrap(); + let height = prev.height; + assert_eq!(height, 4); + + let (coinbase_txn, _) = build::transaction( + vec![ + build::input(amount, height, key_id1.clone()), + build::output(amount - 2, height, key_id2.clone()), + build::with_fee(2), + ], + &keychain, + ).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); diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 0eab72ad54..b681fb6da3 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -492,9 +492,11 @@ bitflags! { pub struct SwitchCommitHashKey ([u8; SWITCH_COMMIT_KEY_SIZE]); impl SwitchCommitHashKey { + /// For a coinbase output use (output_features || lock_height) as the key. + /// For regular tx outputs use the zero value as the key. pub fn from_features_and_lock_height( features: OutputFeatures, - lock_height: u64 + lock_height: u64, ) -> SwitchCommitHashKey { let mut bytes = [0; SWITCH_COMMIT_KEY_SIZE]; if features.contains(COINBASE_OUTPUT) { @@ -565,6 +567,7 @@ impl SwitchCommitHash { SwitchCommitHash(h) } + /// Hex string represenation of a switch commitment hash. pub fn to_hex(&self) -> String { util::to_hex(self.0.to_vec()) } From 3bab309c710bc4afd042590da0651bbee90792d4 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Mon, 13 Nov 2017 15:32:26 -0500 Subject: [PATCH 11/36] pool and chain tests passing (inputs have switch commitments) --- core/src/core/build.rs | 2 +- core/src/core/transaction.rs | 14 ++++++++++++-- pool/src/graph.rs | 20 ++++++++++++++++---- pool/src/pool.rs | 17 ++++++++++++----- 4 files changed, 41 insertions(+), 12 deletions(-) diff --git a/core/src/core/build.rs b/core/src/core/build.rs index cf65b2a14b..bac609ba30 100644 --- a/core/src/core/build.rs +++ b/core/src/core/build.rs @@ -81,7 +81,7 @@ pub fn output(value: u64, lock_height: u64, key_id: Identifier) -> Box { let switch_commit = build.keychain.switch_commit(&key_id).unwrap(); let switch_commit_hash = SwitchCommitHash::from_switch_commit( switch_commit, - SwitchCommitHashKey::from_features_and_lock_height(DEFAULT_OUTPUT, lock_height), + SwitchCommitHashKey::zero(), ); trace!( LOGGER, diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index b681fb6da3..e8fd5c7954 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -36,6 +36,7 @@ use util::LOGGER; /// The size to use for the stored blake2 hash of a switch_commitment pub const SWITCH_COMMIT_HASH_SIZE: usize = 20; +/// The size of the secret key used to generate the switch commitment hash (blake2) pub const SWITCH_COMMIT_KEY_SIZE: usize = 20; bitflags! { @@ -82,7 +83,9 @@ pub enum Error { TooManyInputs, /// Underlying consensus error (currently for sort order) ConsensusError(consensus::Error), + /// Error originating from an invalid lock-height LockHeight(u64), + /// Error originating from an invalid switch commitment (coinbase lock_height related) SwitchCommitment, } @@ -427,6 +430,7 @@ impl Readable for Input { /// The input for a transaction, which spends a pre-existing output. The input /// commitment is a reproduction of the commitment of the output it's spending. impl Input { + /// Build a new Input from a commit, switch_commit and lock_height pub fn new(commit: Commitment, switch_commit: Commitment, lock_height: u64) -> Input { debug!( LOGGER, @@ -507,6 +511,12 @@ impl SwitchCommitHashKey { } SwitchCommitHashKey(bytes) } + + /// We use a zero value key for regular transactions. + pub fn zero() -> SwitchCommitHashKey { + let bytes = [0; SWITCH_COMMIT_KEY_SIZE]; + SwitchCommitHashKey(bytes) + } } /// Definition of the switch commitment hash @@ -784,7 +794,7 @@ mod test { let switch_commit = keychain.switch_commit(&key_id).unwrap(); let switch_commit_hash = SwitchCommitHash::from_switch_commit( switch_commit, - SwitchCommitHashKey::from_features_and_lock_height(DEFAULT_OUTPUT, 0), + SwitchCommitHashKey::zero(), ); let msg = secp::pedersen::ProofMessage::empty(); let proof = keychain.range_proof(5, &key_id, commit, msg).unwrap(); @@ -814,7 +824,7 @@ mod test { let switch_commit = keychain.switch_commit(&key_id).unwrap(); let switch_commit_hash = SwitchCommitHash::from_switch_commit( switch_commit, - SwitchCommitHashKey::from_features_and_lock_height(DEFAULT_OUTPUT, 0), + SwitchCommitHashKey::zero(), ); let msg = secp::pedersen::ProofMessage::empty(); let proof = keychain.range_proof(1003, &key_id, commit, msg).unwrap(); diff --git a/pool/src/graph.rs b/pool/src/graph.rs index 913c2964bc..772896e41d 100644 --- a/pool/src/graph.rs +++ b/pool/src/graph.rs @@ -292,7 +292,7 @@ mod tests { use util::secp; use keychain::Keychain; use rand; - use core::core::SwitchCommitHash; + use core::core::{SwitchCommitHash, SwitchCommitHashKey}; #[test] fn test_add_entry() { @@ -303,10 +303,22 @@ mod tests { let output_commit = keychain.commit(70, &key_id1).unwrap(); let switch_commit = keychain.switch_commit(&key_id1).unwrap(); - let switch_commit_hash = SwitchCommitHash::from_switch_commit(switch_commit); + let switch_commit_hash = SwitchCommitHash::from_switch_commit( + switch_commit, + SwitchCommitHashKey::zero(), + ); + let inputs = vec![ - core::transaction::Input(keychain.commit(50, &key_id2).unwrap()), - core::transaction::Input(keychain.commit(25, &key_id3).unwrap()), + core::transaction::Input::new( + keychain.commit(50, &key_id2).unwrap(), + keychain.switch_commit(&key_id2).unwrap(), + 0, + ), + core::transaction::Input::new( + keychain.commit(25, &key_id3).unwrap(), + keychain.switch_commit(&key_id2).unwrap(), + 0, + ), ]; let msg = secp::pedersen::ProofMessage::empty(); let outputs = vec![ diff --git a/pool/src/pool.rs b/pool/src/pool.rs index 64ff01c894..31079bc172 100644 --- a/pool/src/pool.rs +++ b/pool/src/pool.rs @@ -609,7 +609,8 @@ mod tests { use std::sync::{Arc, RwLock}; use blake2; use core::global::ChainTypes; - use core::core::SwitchCommitHash; + use core::core::{SwitchCommitHash, SwitchCommitHashKey}; + macro_rules! expect_output_parent { ($pool:expr, $expected:pat, $( $output:expr ),+ ) => { @@ -1231,7 +1232,7 @@ mod tests { for input_value in input_values { let key_id = keychain.derive_key_id(input_value as u32).unwrap(); - tx_elements.push(build::input(input_value, key_id)); + tx_elements.push(build::input(input_value, 0, key_id)); } for output_value in output_values { @@ -1259,7 +1260,7 @@ mod tests { for input_value in input_values { let key_id = keychain.derive_key_id(input_value as u32).unwrap(); - tx_elements.push(build::input(input_value, key_id)); + tx_elements.push(build::input(input_value, 0, key_id)); } for output_value in output_values { @@ -1279,7 +1280,10 @@ mod tests { let key_id = keychain.derive_key_id(value as u32).unwrap(); let commit = keychain.commit(value, &key_id).unwrap(); let switch_commit = keychain.switch_commit(&key_id).unwrap(); - let switch_commit_hash = SwitchCommitHash::from_switch_commit(switch_commit); + let switch_commit_hash = SwitchCommitHash::from_switch_commit( + switch_commit, + SwitchCommitHashKey::zero(), + ); let msg = secp::pedersen::ProofMessage::empty(); let proof = keychain.range_proof(value, &key_id, commit, msg).unwrap(); @@ -1297,7 +1301,10 @@ mod tests { let key_id = keychain.derive_key_id(value as u32).unwrap(); let commit = keychain.commit(value, &key_id).unwrap(); let switch_commit = keychain.switch_commit(&key_id).unwrap(); - let switch_commit_hash = SwitchCommitHash::from_switch_commit(switch_commit); + let switch_commit_hash = SwitchCommitHash::from_switch_commit( + switch_commit, + SwitchCommitHashKey::zero(), + ); let msg = secp::pedersen::ProofMessage::empty(); let proof = keychain.range_proof(value, &key_id, commit, msg).unwrap(); From 60f8c0b0e4a3528c0fec9f9c2a9ab008a01dabe0 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Mon, 13 Nov 2017 15:38:46 -0500 Subject: [PATCH 12/36] commit --- wallet/src/sender.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wallet/src/sender.rs b/wallet/src/sender.rs index 2ae21a547e..fbf4913e78 100644 --- a/wallet/src/sender.rs +++ b/wallet/src/sender.rs @@ -252,8 +252,8 @@ mod test { let keychain = Keychain::from_random_seed().unwrap(); let key_id1 = keychain.derive_key_id(1).unwrap(); - let (tx1, _) = transaction(vec![output(105, key_id1.clone())], &keychain).unwrap(); - let (tx2, _) = transaction(vec![input(105, key_id1.clone())], &keychain).unwrap(); + let (tx1, _) = transaction(vec![output(105, 0, key_id1.clone())], &keychain).unwrap(); + let (tx2, _) = transaction(vec![input(105, 0, key_id1.clone())], &keychain).unwrap(); assert_eq!(tx1.outputs[0].commitment(), tx2.inputs[0].commitment()); } From f8e5d35340aba6604e4fb0bd835d4446de5f2650 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Mon, 13 Nov 2017 16:39:09 -0500 Subject: [PATCH 13/36] cleanup --- core/src/core/block.rs | 54 +++++++++++++++++++++--------------- core/src/core/mod.rs | 2 +- core/src/core/transaction.rs | 1 - 3 files changed, 32 insertions(+), 25 deletions(-) diff --git a/core/src/core/block.rs b/core/src/core/block.rs index b1ef9fb4d4..4e83f301c6 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -54,10 +54,7 @@ pub enum Error { /// Too many inputs, outputs or kernels in the block WeightExceeded, /// Kernel not valid due to lock_height exceeding block header height - KernelLockHeight { - /// The lock_height causing this validation error - lock_height: u64, - }, + KernelLockHeight(u64), /// Underlying tx related error Transaction(transaction::Error), /// Underlying Secp256k1 error (signature validation or invalid public key typically) @@ -470,15 +467,20 @@ impl Block { /// as all the steps within are verify steps. /// pub fn validate(&self) -> Result<(), Error> { - if exceeds_weight(self.inputs.len(), self.outputs.len(), self.kernels.len()) { - return Err(Error::WeightExceeded); - } + self.verify_weight()?; self.verify_sorted()?; self.verify_coinbase()?; self.verify_kernels(false)?; Ok(()) } + fn verify_weight(&self) -> Result<(), Error> { + if exceeds_weight(self.inputs.len(), self.outputs.len(), self.kernels.len()) { + return Err(Error::WeightExceeded); + } + Ok(()) + } + fn verify_sorted(&self) -> Result<(), Error> { self.inputs.verify_sort_order()?; self.outputs.verify_sort_order()?; @@ -495,8 +497,10 @@ impl Block { return Err(Error::OddKernelFee); } + // check we have no kernels with lock_heights greater than current height + // no tx can be included in a block earlier than its lock_height if k.lock_height > self.header.height { - return Err(Error::KernelLockHeight { lock_height: k.lock_height }); + return Err(Error::KernelLockHeight(k.lock_height)); } } @@ -532,19 +536,17 @@ impl Block { // * That the sum of blinding factors for all coinbase-marked outputs match // the coinbase-marked kernels. fn verify_coinbase(&self) -> Result<(), Error> { - let cb_outs = filter_map_vec!(self.outputs, |out| if out.features.contains( - COINBASE_OUTPUT, - ) - { - Some(out.commitment()) - } else { - None - }); - let cb_kerns = filter_map_vec!(self.kernels, |k| if k.features.contains(COINBASE_KERNEL) { - Some(k.excess) - } else { - None - }); + let cb_outs = self.outputs + .iter() + .filter(|out| out.features.contains(COINBASE_OUTPUT)) + .cloned() + .collect::>(); + + let cb_kerns = self.kernels + .iter() + .filter(|kernel| kernel.features.contains(COINBASE_KERNEL)) + .cloned() + .collect::>(); let over_commit; let out_adjust_sum; @@ -553,8 +555,14 @@ impl Block { let secp = static_secp_instance(); let secp = secp.lock().unwrap(); over_commit = secp.commit_value(reward(self.total_fees()))?; - out_adjust_sum = secp.commit_sum(cb_outs, vec![over_commit])?; - kerns_sum = secp.commit_sum(cb_kerns, vec![])?; + out_adjust_sum = secp.commit_sum( + cb_outs.iter().map(|x| x.commitment()).collect(), + vec![over_commit], + )?; + kerns_sum = secp.commit_sum( + cb_kerns.iter().map(|x| x.excess).collect(), + vec![], + )?; } if kerns_sum != out_adjust_sum { diff --git a/core/src/core/mod.rs b/core/src/core/mod.rs index 685e39a632..aefc89dc3a 100644 --- a/core/src/core/mod.rs +++ b/core/src/core/mod.rs @@ -469,7 +469,7 @@ mod test { &key_id3.clone(), ).unwrap(); match b.validate() { - Err(KernelLockHeight { lock_height: height }) => { + Err(KernelLockHeight(height)) => { assert_eq!(height, 2); } _ => panic!("expecting KernelLockHeight error here"), diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index e8fd5c7954..45306720f1 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -738,7 +738,6 @@ impl ops::Add for SumCommit { mod test { use super::*; use keychain::Keychain; - use util; use util::secp; #[test] From 1c46b862be3898af73ca5273a952475927baaa7e Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Mon, 13 Nov 2017 16:58:47 -0500 Subject: [PATCH 14/36] check inputs spending coinbase outputs have valid lock_heights --- chain/src/pipe.rs | 20 +++++++++++++++++--- core/src/core/transaction.rs | 6 ++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/chain/src/pipe.rs b/chain/src/pipe.rs index 7317bb4a5e..faf835f188 100644 --- a/chain/src/pipe.rs +++ b/chain/src/pipe.rs @@ -314,9 +314,23 @@ fn validate_block( for input in &b.inputs { if let Ok(output) = ctx.store.get_output_by_commit(&input.commitment()) { if output.features.contains(transaction::COINBASE_OUTPUT) { - input.verify_lock_height(&output, b.header.height).map_err(|_| Error::ImmatureCoinbase)?; - }; - }; + // check the lock_height of the output being spent by this input + // is not greater than the current height + input.verify_lock_height(&output, b.header.height) + .map_err(|_| Error::ImmatureCoinbase)?; + + // This input is spending an output that has sufficiently matured. + // For a coinbase output we need to verify it + // was originally created with a valid lock_height + // based on the coinbase maturity consensus rule. + let out_header = ctx.store.get_block_header_by_output_commit(&input.commitment()); + if let Ok(out_header) = out_header { + if input.lock_height() > out_header.height + global::coinbase_maturity() { + return Err(Error::ImmatureCoinbase); + } + } + } + } } Ok(()) diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 45306720f1..57aadc3e78 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -451,6 +451,11 @@ impl Input { self.commit } + /// The lock_height of this input (only relevant for an input spending a coinbase output). + pub fn lock_height(&self) -> u64 { + self.lock_height + } + /// Given the output being spent by this input along with the current chain height /// we can verify the output has sufficiently matured and is spendable. /// We do this by reconstructing the switch_commit_hash from the switch_commit. @@ -476,6 +481,7 @@ impl Input { return Err(Error::LockHeight(self.lock_height)); } } + Ok(()) } } From ab4102982bd9befe844c2dd5eb3b9a0597f4e956 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Wed, 3 Jan 2018 10:33:20 -0500 Subject: [PATCH 15/36] wip - got it building (tests still failing) --- api/src/types.rs | 26 ++++++++++++++++++++++---- chain/src/store.rs | 4 ---- chain/src/types.rs | 3 --- core/src/core/transaction.rs | 21 ++++++++++++++------- wallet/src/receiver.rs | 16 +++++----------- wallet/src/restore.rs | 36 +++++++++++++++++++++++++++--------- wallet/src/sender.rs | 15 +++++++++------ 7 files changed, 77 insertions(+), 44 deletions(-) diff --git a/api/src/types.rs b/api/src/types.rs index f028e17e9f..a102f3d0a7 100644 --- a/api/src/types.rs +++ b/api/src/types.rs @@ -207,7 +207,7 @@ impl OutputPrintable { OutputPrintable { output_type: output_type, commit: util::to_hex(output.commit.0.to_vec()), - switch_commit_hash: util::to_hex(output.switch_commit_hash.hash.to_vec()), + switch_commit_hash: output.switch_commit_hash.to_hex(), height: block_header.height, lock_height: lock_height, spent: true, @@ -222,22 +222,40 @@ impl OutputPrintable { // As above, except just the info needed for wallet reconstruction #[derive(Debug, Serialize, Deserialize, Clone)] pub struct OutputSwitch { + /// The type of output Coinbase|Transaction + pub output_type: OutputType, /// the commit pub commit: String, /// switch commit hash - pub switch_commit_hash: [u8; core::SWITCH_COMMIT_HASH_SIZE], + pub switch_commit_hash: String, /// The height of the block creating this output pub height: u64, + /// The lock height (earliest block this output can be spent) + pub lock_height: u64, } impl OutputSwitch { pub fn from_output(output: &core::Output, block_header: &core::BlockHeader) -> OutputSwitch { + let (output_type, lock_height) = match output.features { + x if x.contains(core::transaction::COINBASE_OUTPUT) => ( + OutputType::Coinbase, + block_header.height + global::coinbase_maturity(), + ), + _ => (OutputType::Transaction, 0), + }; + OutputSwitch { + output_type: output_type, commit: util::to_hex(output.commit.0.to_vec()), - switch_commit_hash: output.switch_commit_hash.hash, + switch_commit_hash: output.switch_commit_hash.to_hex(), height: block_header.height, + lock_height: lock_height, } } + + pub fn switch_commit_hash(&self) -> core::SwitchCommitHash { + core::SwitchCommitHash::from_hex(&self.switch_commit_hash).unwrap() + } } // Printable representation of a block @@ -344,7 +362,7 @@ impl BlockPrintable { pub fn from_block(block: &core::Block) -> BlockPrintable { let inputs = block.inputs .iter() - .map(|input| util::to_hex((input.0).0.to_vec())) + .map(|x| util::to_hex(x.commitment().0.to_vec())) .collect(); let outputs = block.outputs .iter() diff --git a/chain/src/store.rs b/chain/src/store.rs index 093479999a..35a9eddaa0 100644 --- a/chain/src/store.rs +++ b/chain/src/store.rs @@ -106,10 +106,6 @@ impl ChainStore for ChainKVStore { ) } - fn check_block_exists(&self, h: &Hash) -> Result { - self.db.exists(&to_key(BLOCK_PREFIX, &mut h.to_vec())) - } - fn save_block(&self, b: &Block) -> Result<(), Error> { // saving the block and its header let mut batch = self.db diff --git a/chain/src/types.rs b/chain/src/types.rs index 15167bcf21..15fd157624 100644 --- a/chain/src/types.rs +++ b/chain/src/types.rs @@ -212,9 +212,6 @@ pub trait ChainStore: Send + Sync { /// Gets a block header by hash fn get_block_header(&self, h: &Hash) -> Result; - /// Checks whether a block has been been processed and saved - fn check_block_exists(&self, h: &Hash) -> Result; - /// Save the provided block in store fn save_block(&self, b: &Block) -> Result<(), store::Error>; diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 57aadc3e78..3705b34b0f 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -19,6 +19,7 @@ use blake2::blake2b::blake2b; use util::secp::{self, Message, Signature}; use util::static_secp_instance; use util::secp::pedersen::{Commitment, RangeProof}; +use std::cmp::min; use std::cmp::Ordering; use std::ops; @@ -525,13 +526,6 @@ impl SwitchCommitHashKey { } } -/// Definition of the switch commitment hash -#[derive(Debug, Copy, Clone, PartialEq, Serialize, Deserialize)] -pub struct SwitchCommitHash { - /// simple hash - pub hash: [u8; SWITCH_COMMIT_HASH_SIZE], -} - /// Definition of the switch commitment hash #[derive(Copy, Clone, PartialEq, Serialize, Deserialize)] pub struct SwitchCommitHash([u8; SWITCH_COMMIT_HASH_SIZE]); @@ -583,10 +577,23 @@ impl SwitchCommitHash { SwitchCommitHash(h) } + pub fn from_bytes(bytes: &[u8]) -> SwitchCommitHash { + let mut hash = [0; SWITCH_COMMIT_HASH_SIZE]; + for i in 0..min(SWITCH_COMMIT_HASH_SIZE, bytes.len()) { + hash[i] = bytes[i]; + } + SwitchCommitHash(hash) + } + /// Hex string represenation of a switch commitment hash. pub fn to_hex(&self) -> String { util::to_hex(self.0.to_vec()) } + + pub fn from_hex(hex: &str) -> Result { + let bytes = util::from_hex(hex.to_string()).unwrap(); + Ok(SwitchCommitHash::from_bytes(&bytes)) + } } /// Output for a transaction, defining the new ownership of coins that are being diff --git a/wallet/src/receiver.rs b/wallet/src/receiver.rs index eaa243fbf2..678cab5a76 100644 --- a/wallet/src/receiver.rs +++ b/wallet/src/receiver.rs @@ -122,7 +122,7 @@ pub fn receive_coinbase( ) -> Result<(Output, TxKernel, BlockFees), Error> { let root_key_id = keychain.root_key_id(); - let output_lock_height = block_fees.height + global::coinbase_maturity(); + let lock_height = block_fees.height + global::coinbase_maturity(); // Now acquire the wallet lock and write the new output. let (key_id, derivation) = WalletData::with_wallet(&config.data_file_dir, |wallet_data| { @@ -140,7 +140,7 @@ pub fn receive_coinbase( value: reward(block_fees.fees), status: OutputStatus::Unconfirmed, height: 0, - lock_height: output_lock_height, + lock_height: lock_height, is_coinbase: true, }); @@ -160,13 +160,13 @@ pub fn receive_coinbase( let mut block_fees = block_fees.clone(); block_fees.key_id = Some(key_id.clone()); - debug!(LOGGER, "block_fees updated key_id - {:?}", block_fees.key_id); + debug!(LOGGER, "block_fees updated - {:?}", block_fees); let (out, kern) = Block::reward_output( &keychain, &key_id, - block_fees.fees, block_fees.height, + block_fees.fees, )?; Ok((out, kern, block_fees)) } @@ -181,8 +181,6 @@ fn receive_transaction( ) -> Result { let root_key_id = keychain.root_key_id(); - let (key_id, derivation) = next_available_key(config, keychain)?; - let lock_height = partial.lock_height; // double check the fee amount included in the partial tx @@ -220,11 +218,7 @@ fn receive_transaction( vec![ build::initial_tx(partial), build::with_excess(blinding), - build::output( - out_amount, - lock_height, - key_id.clone(), - ), + build::output(out_amount, lock_height, key_id.clone()), ], keychain, )?; diff --git a/wallet/src/restore.rs b/wallet/src/restore.rs index ee4e9d8772..3a99e77f2a 100644 --- a/wallet/src/restore.rs +++ b/wallet/src/restore.rs @@ -16,8 +16,8 @@ use keychain::{Keychain, Identifier}; use util::{LOGGER, from_hex}; use util::secp::pedersen; use api; -use core::core::{Output, SwitchCommitHash}; -use core::core::transaction::{COINBASE_OUTPUT, DEFAULT_OUTPUT, SWITCH_COMMIT_HASH_SIZE}; +use core::core::{Output, SwitchCommitHash, SwitchCommitHashKey}; +use core::core::transaction::{COINBASE_OUTPUT, DEFAULT_OUTPUT}; use types::{WalletConfig, WalletData, OutputData, OutputStatus, Error}; use byteorder::{BigEndian, ByteOrder}; @@ -115,7 +115,7 @@ pub fn utxos_batch_block( fn find_utxos_with_key( config: &WalletConfig, keychain: &Keychain, - switch_commit_cache: &Vec<[u8; SWITCH_COMMIT_HASH_SIZE]>, + switch_commit_cache: &Vec, block_outputs: api::BlockOutputs, key_iterations: &mut usize, padding: &mut usize, @@ -133,11 +133,31 @@ fn find_utxos_with_key( for output in block_outputs.outputs { for i in 0..*key_iterations { - if switch_commit_cache[i as usize] == output.switch_commit_hash { + let expected_hash = match output.output_type { + api::OutputType::Coinbase => { + SwitchCommitHash::from_switch_commit( + switch_commit_cache[i as usize], + SwitchCommitHashKey::from_features_and_lock_height( + COINBASE_OUTPUT, + output.lock_height, + ), + ) + } + api::OutputType::Transaction => { + SwitchCommitHash::from_switch_commit( + switch_commit_cache[i as usize], + SwitchCommitHashKey::from_features_and_lock_height( + DEFAULT_OUTPUT, + output.lock_height, + ), + ) + } + }; + if expected_hash == output.switch_commit_hash() { info!( LOGGER, "Output found: {:?}, key_index: {:?}", - output.switch_commit_hash, + output, i, ); @@ -179,7 +199,6 @@ fn find_utxos_with_key( LOGGER, "Unable to retrieve the amount (needs investigating)", ); - } } } @@ -219,7 +238,7 @@ pub fn restore( chain_height ); - let mut switch_commit_cache: Vec<[u8; SWITCH_COMMIT_HASH_SIZE]> = vec![]; + let mut switch_commit_cache: Vec = vec![]; info!( LOGGER, "Building key derivation cache ({}) ...", @@ -227,8 +246,7 @@ pub fn restore( ); for i in 0..key_derivations { let switch_commit = keychain.switch_commit_from_index(i as u32).unwrap(); - let switch_commit_hash = SwitchCommitHash::from_switch_commit(switch_commit); - switch_commit_cache.push(switch_commit_hash.hash); + switch_commit_cache.push(switch_commit); } debug!(LOGGER, "... done"); diff --git a/wallet/src/sender.rs b/wallet/src/sender.rs index fbf4913e78..d47c20c9fb 100644 --- a/wallet/src/sender.rs +++ b/wallet/src/sender.rs @@ -44,7 +44,7 @@ pub fn issue_send_tx( let chain_tip = checker::get_tip_from_node(config)?; let current_height = chain_tip.height; - // proof of concept - set lock_height on the tx (only valid in a block later than this) + // proof of concept - set lock_height on the tx let lock_height = chain_tip.height; let (tx, blind_sum, coins, change_key) = build_send_tx( @@ -126,7 +126,7 @@ fn build_send_tx( })?; // build transaction skeleton with inputs and change - let (mut parts, change_key) = inputs_and_change(&coins, config, keychain, key_id, amount, lock_height)?; + let (mut parts, change_key) = inputs_and_change(&coins, config, keychain, amount, lock_height)?; // This is more proof of concept than anything but here we set lock_height // on tx being sent (based on current chain height via api). @@ -165,7 +165,10 @@ pub fn issue_burn_tx( ) })?; - let (mut parts, _) = inputs_and_change(&coins, config, keychain, key_id, amount, current_height)?; + debug!(LOGGER, "selected some coins - {}", coins.len()); + + let lock_height = current_height; + let (mut parts, _) = inputs_and_change(&coins, config, keychain, amount, lock_height)?; // add burn output and fees let fee = tx_fee(coins.len(), 2, None); @@ -235,7 +238,7 @@ fn inputs_and_change( change_key })?; - parts.push(build::output(change, change_key.clone())); + parts.push(build::output(change, lock_height, change_key.clone())); Ok((parts, change_key)) } @@ -252,8 +255,8 @@ mod test { let keychain = Keychain::from_random_seed().unwrap(); let key_id1 = keychain.derive_key_id(1).unwrap(); - let (tx1, _) = transaction(vec![output(105, 0, key_id1.clone())], &keychain).unwrap(); - let (tx2, _) = transaction(vec![input(105, 0, key_id1.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(); assert_eq!(tx1.outputs[0].commitment(), tx2.inputs[0].commitment()); } From 6abdc25063d807af21b943f1c2b1edb5acec5e36 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Wed, 3 Jan 2018 10:42:19 -0500 Subject: [PATCH 16/36] use zero key for non coinbase switch commit hash --- wallet/src/restore.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/wallet/src/restore.rs b/wallet/src/restore.rs index 3a99e77f2a..28ac2d12f0 100644 --- a/wallet/src/restore.rs +++ b/wallet/src/restore.rs @@ -146,10 +146,7 @@ fn find_utxos_with_key( api::OutputType::Transaction => { SwitchCommitHash::from_switch_commit( switch_commit_cache[i as usize], - SwitchCommitHashKey::from_features_and_lock_height( - DEFAULT_OUTPUT, - output.lock_height, - ), + SwitchCommitHashKey::zero(), ) } }; From 6861862b22c35bc4b4d14fdbd924cd338fa3d5b5 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Wed, 3 Jan 2018 12:01:55 -0500 Subject: [PATCH 17/36] fees and height wrong order... --- core/src/core/block.rs | 15 ++++++--------- wallet/src/receiver.rs | 2 +- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/core/src/core/block.rs b/core/src/core/block.rs index 4e83f301c6..a75bebed01 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -470,7 +470,7 @@ impl Block { self.verify_weight()?; self.verify_sorted()?; self.verify_coinbase()?; - self.verify_kernels(false)?; + self.verify_kernels()?; Ok(()) } @@ -490,8 +490,7 @@ impl Block { /// Verifies the sum of input/output commitments match the sum in kernels /// and that all kernel signatures are valid. - /// TODO - when would we skip_sig? Is this needed or used anywhere? - fn verify_kernels(&self, skip_sig: bool) -> Result<(), Error> { + fn verify_kernels(&self) -> Result<(), Error> { for k in &self.kernels { if k.fee & 1 != 0 { return Err(Error::OddKernelFee); @@ -522,11 +521,10 @@ impl Block { } // verify all signatures with the commitment as pk - if !skip_sig { - for proof in &self.kernels { - proof.verify()?; - } + for proof in &self.kernels { + proof.verify()?; } + Ok(()) } @@ -800,7 +798,7 @@ mod test { b.verify_coinbase(), Err(Error::CoinbaseSumMismatch) ); - assert_eq!(b.verify_kernels(false), Ok(())); + assert_eq!(b.verify_kernels(), Ok(())); assert_eq!( b.validate(), @@ -822,7 +820,6 @@ mod test { b.verify_coinbase(), Err(Error::Secp(secp::Error::IncorrectCommitSum)) ); - assert_eq!(b.verify_kernels(true), Ok(())); assert_eq!( b.validate(), diff --git a/wallet/src/receiver.rs b/wallet/src/receiver.rs index 678cab5a76..4a6196eb48 100644 --- a/wallet/src/receiver.rs +++ b/wallet/src/receiver.rs @@ -165,8 +165,8 @@ pub fn receive_coinbase( let (out, kern) = Block::reward_output( &keychain, &key_id, - block_fees.height, block_fees.fees, + block_fees.height, )?; Ok((out, kern, block_fees)) } From 2db64ff57fd8a8cc4aeb787fb1e39b27737c675a Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Wed, 3 Jan 2018 12:39:21 -0500 Subject: [PATCH 18/36] send output lock_height over to wallet via api --- api/src/types.rs | 13 +++++++++---- wallet/src/checker.rs | 2 +- wallet/src/restore.rs | 9 +++++---- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/api/src/types.rs b/api/src/types.rs index a102f3d0a7..bd15e820c4 100644 --- a/api/src/types.rs +++ b/api/src/types.rs @@ -144,6 +144,8 @@ pub struct Output { pub proof: Option, /// The height of the block creating this output pub height: u64, + /// The lock height (earliest block this output can be spent) + pub lock_height: u64, } impl Output { @@ -153,10 +155,12 @@ impl Output { include_proof: bool, include_switch: bool, ) -> Output { - let output_type = if output.features.contains(core::transaction::COINBASE_OUTPUT) { - OutputType::Coinbase - } else { - OutputType::Transaction + let (output_type, lock_height) = match output.features { + x if x.contains(core::transaction::COINBASE_OUTPUT) => ( + OutputType::Coinbase, + block_header.height + global::coinbase_maturity(), + ), + _ => (OutputType::Transaction, 0), }; Output { @@ -171,6 +175,7 @@ impl Output { false => None, }, height: block_header.height, + lock_height: lock_height, } } } diff --git a/wallet/src/checker.rs b/wallet/src/checker.rs index 684cae0893..38a5abe1f0 100644 --- a/wallet/src/checker.rs +++ b/wallet/src/checker.rs @@ -29,7 +29,7 @@ use util::LOGGER; // Also updates the height and lock_height based on latest from the api. fn refresh_output(out: &mut OutputData, api_out: &api::Output) { out.height = api_out.height; - // out.lock_height = api_out.lock_height; + out.lock_height = api_out.lock_height; match out.status { OutputStatus::Unconfirmed => { diff --git a/wallet/src/restore.rs b/wallet/src/restore.rs index 28ac2d12f0..3b988e1ba6 100644 --- a/wallet/src/restore.rs +++ b/wallet/src/restore.rs @@ -119,8 +119,8 @@ fn find_utxos_with_key( block_outputs: api::BlockOutputs, key_iterations: &mut usize, padding: &mut usize, -) -> Vec<(pedersen::Commitment, Identifier, u32, u64, u64, bool)> { - let mut wallet_outputs: Vec<(pedersen::Commitment, Identifier, u32, u64, u64, bool)> = +) -> Vec<(pedersen::Commitment, Identifier, u32, u64, u64, u64, bool)> { + let mut wallet_outputs: Vec<(pedersen::Commitment, Identifier, u32, u64, u64, u64, bool)> = Vec::new(); info!( @@ -181,6 +181,7 @@ fn find_utxos_with_key( i as u32, amount, output.height, + output.lock_height, is_coinbase, )); @@ -286,8 +287,8 @@ pub fn restore( value: output.3, status: OutputStatus::Unconfirmed, height: output.4, - lock_height: 0, - is_coinbase: output.5, + lock_height: output.5, + is_coinbase: output.6, }); }; } From 22a04d5a160af0ab5367053decf3bf2869061987 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Wed, 3 Jan 2018 16:57:01 -0500 Subject: [PATCH 19/36] no more header by height index workaround this for wallet refresh and wallet restore --- api/src/handlers.rs | 25 ++----- api/src/types.rs | 112 +++++++------------------------ chain/src/chain.rs | 10 --- chain/src/pipe.rs | 15 +---- chain/src/store.rs | 29 -------- chain/src/types.rs | 6 -- chain/tests/mine_simple_chain.rs | 7 -- chain/tests/store_indices.rs | 5 -- grin/src/adapters.rs | 10 --- pool/src/blockchain.rs | 24 ------- pool/src/pool.rs | 30 ++++----- pool/src/types.rs | 13 ++-- wallet/src/checker.rs | 33 ++++++--- wallet/src/info.rs | 17 +++-- wallet/src/receiver.rs | 7 +- wallet/src/restore.rs | 20 ++++-- wallet/src/sender.rs | 2 +- wallet/src/types.rs | 2 +- 18 files changed, 102 insertions(+), 265 deletions(-) diff --git a/api/src/handlers.rs b/api/src/handlers.rs index d21ff24494..9520e0327e 100644 --- a/api/src/handlers.rs +++ b/api/src/handlers.rs @@ -60,7 +60,7 @@ struct UtxoHandler { } impl UtxoHandler { - fn get_utxo(&self, id: &str, include_rp: bool, include_switch: bool) -> Result { + fn get_utxo(&self, id: &str, include_rp: bool) -> Result { debug!(LOGGER, "getting utxo: {}", id); let c = util::from_hex(String::from(id)) .map_err(|_| Error::Argument(format!("Not a valid commitment: {}", id)))?; @@ -70,22 +70,12 @@ impl UtxoHandler { .get_unspent(&commit) .map_err(|_| Error::NotFound)?; - let header = self.chain - .get_block_header_by_output_commit(&commit) - .map_err(|_| Error::NotFound)?; - - Ok(Output::from_output( - &out, - &header, - include_rp, - include_switch, - )) + Ok(Output::from_output(&out, include_rp)) } fn utxos_by_ids(&self, req: &mut Request) -> Vec { let mut commitments: Vec<&str> = vec![]; - let mut rp = false; - let mut switch = false; + let mut include_rp = false; if let Ok(params) = req.get_ref::() { if let Some(ids) = params.get("id") { for id in ids { @@ -95,15 +85,12 @@ impl UtxoHandler { } } if let Some(_) = params.get("include_rp") { - rp = true; - } - if let Some(_) = params.get("include_switch") { - switch = true; + include_rp = true; } } let mut utxos: Vec = vec![]; for commit in commitments { - if let Ok(out) = self.get_utxo(commit, rp, switch) { + if let Ok(out) = self.get_utxo(commit, include_rp) { utxos.push(out); } } @@ -120,7 +107,7 @@ impl UtxoHandler { .outputs .iter() .filter(|c| self.chain.is_unspent(&c.commit).unwrap()) - .map(|k| OutputSwitch::from_output(k, &header)) + .map(|k| OutputPrintable::from_output(k)) .collect(); BlockOutputs { header: BlockHeaderInfo::from_header(&header), diff --git a/api/src/types.rs b/api/src/types.rs index bd15e820c4..f3f7244e2f 100644 --- a/api/src/types.rs +++ b/api/src/types.rs @@ -14,10 +14,9 @@ use std::sync::Arc; -use core::{core, global}; +use core::core; use core::core::hash::Hashed; use chain; -use rest::*; use util; use util::secp::pedersen; @@ -85,11 +84,8 @@ impl SumTreeNode { let mut return_vec = Vec::new(); let last_n = chain.get_last_n_utxo(distance); for elem_output in last_n { - let header = chain - .get_block_header_by_output_commit(&elem_output.1.commit) - .map_err(|_| Error::NotFound); // Need to call further method to check if output is spent - let mut output = OutputPrintable::from_output(&elem_output.1, &header.unwrap(),true); + let mut output = OutputPrintable::from_output(&elem_output.1); if let Ok(_) = chain.get_unspent(&elem_output.1.commit) { output.spent = false; } @@ -136,46 +132,34 @@ pub enum OutputType { pub struct Output { /// The type of output Coinbase|Transaction pub output_type: OutputType, - /// The homomorphic commitment representing the output's amount + /// The homomorphic commitment representing the output amount pub commit: pedersen::Commitment, /// switch commit hash - pub switch_commit_hash: Option, + pub switch_commit_hash: core::SwitchCommitHash, /// A proof that the commitment is in the right range pub proof: Option, - /// The height of the block creating this output - pub height: u64, - /// The lock height (earliest block this output can be spent) - pub lock_height: u64, } impl Output { pub fn from_output( output: &core::Output, - block_header: &core::BlockHeader, include_proof: bool, - include_switch: bool, ) -> Output { - let (output_type, lock_height) = match output.features { - x if x.contains(core::transaction::COINBASE_OUTPUT) => ( - OutputType::Coinbase, - block_header.height + global::coinbase_maturity(), - ), - _ => (OutputType::Transaction, 0), - }; + let output_type = + if output.features.contains(core::transaction::COINBASE_OUTPUT) { + OutputType::Coinbase + } else { + OutputType::Transaction + }; Output { output_type: output_type, commit: output.commit, - switch_commit_hash: match include_switch { - true => Some(output.switch_commit_hash), - false => None, - }, + switch_commit_hash: output.switch_commit_hash, proof: match include_proof { true => Some(output.proof), false => None, }, - height: block_header.height, - lock_height: lock_height, } } } @@ -185,76 +169,32 @@ impl Output { pub struct OutputPrintable { /// The type of output Coinbase|Transaction pub output_type: OutputType, - /// The homomorphic commitment representing the output's amount (as hex - /// string) + /// The homomorphic commitment representing the output's amount + /// (as hex string) pub commit: String, /// switch commit hash pub switch_commit_hash: String, - /// The height of the block creating this output - pub height: u64, - /// The lock height (earliest block this output can be spent) - pub lock_height: u64, /// Whether the output has been spent pub spent: bool, - /// Rangeproof hash (as hex string) - pub proof_hash: Option, + /// Rangeproof hash (as hex string) + pub proof_hash: String, } impl OutputPrintable { - pub fn from_output(output: &core::Output, block_header: &core::BlockHeader, include_proof_hash:bool) -> OutputPrintable { - let (output_type, lock_height) = match output.features { - x if x.contains(core::transaction::COINBASE_OUTPUT) => ( - OutputType::Coinbase, - block_header.height + global::coinbase_maturity(), - ), - _ => (OutputType::Transaction, 0), - }; + pub fn from_output(output: &core::Output) -> OutputPrintable { + let output_type = + if output.features.contains(core::transaction::COINBASE_OUTPUT) { + OutputType::Coinbase + } else { + OutputType::Transaction + }; + OutputPrintable { output_type: output_type, commit: util::to_hex(output.commit.0.to_vec()), switch_commit_hash: output.switch_commit_hash.to_hex(), - height: block_header.height, - lock_height: lock_height, spent: true, - proof_hash: match include_proof_hash { - true => Some(util::to_hex(output.proof.hash().to_vec())), - false => None, - } - } - } -} - -// As above, except just the info needed for wallet reconstruction -#[derive(Debug, Serialize, Deserialize, Clone)] -pub struct OutputSwitch { - /// The type of output Coinbase|Transaction - pub output_type: OutputType, - /// the commit - pub commit: String, - /// switch commit hash - pub switch_commit_hash: String, - /// The height of the block creating this output - pub height: u64, - /// The lock height (earliest block this output can be spent) - pub lock_height: u64, -} - -impl OutputSwitch { - pub fn from_output(output: &core::Output, block_header: &core::BlockHeader) -> OutputSwitch { - let (output_type, lock_height) = match output.features { - x if x.contains(core::transaction::COINBASE_OUTPUT) => ( - OutputType::Coinbase, - block_header.height + global::coinbase_maturity(), - ), - _ => (OutputType::Transaction, 0), - }; - - OutputSwitch { - output_type: output_type, - commit: util::to_hex(output.commit.0.to_vec()), - switch_commit_hash: output.switch_commit_hash.to_hex(), - height: block_header.height, - lock_height: lock_height, + proof_hash: util::to_hex(output.proof.hash().to_vec()), } } @@ -371,7 +311,7 @@ impl BlockPrintable { .collect(); let outputs = block.outputs .iter() - .map(|output| OutputPrintable::from_output(output, &block.header, true)) + .map(|output| OutputPrintable::from_output(output)) .collect(); let kernels = block.kernels .iter() @@ -393,7 +333,7 @@ pub struct BlockOutputs { /// The block header pub header: BlockHeaderInfo, /// A printable version of the outputs - pub outputs: Vec, + pub outputs: Vec, } #[derive(Serialize, Deserialize)] diff --git a/chain/src/chain.rs b/chain/src/chain.rs index 59e2efcb5e..943d3d1360 100644 --- a/chain/src/chain.rs +++ b/chain/src/chain.rs @@ -465,16 +465,6 @@ impl Chain { }) } - /// Gets the block header by the provided output commitment - pub fn get_block_header_by_output_commit( - &self, - commit: &Commitment, - ) -> Result { - self.store - .get_block_header_by_output_commit(commit) - .map_err(|e| Error::StoreErr(e, "chain get commitment".to_owned())) - } - /// Get the tip of the current "sync" header chain. /// This may be significantly different to current header chain. pub fn get_sync_head(&self) -> Result { diff --git a/chain/src/pipe.rs b/chain/src/pipe.rs index faf835f188..953ef2e646 100644 --- a/chain/src/pipe.rs +++ b/chain/src/pipe.rs @@ -313,22 +313,11 @@ fn validate_block( for input in &b.inputs { if let Ok(output) = ctx.store.get_output_by_commit(&input.commitment()) { + // check the lock_height of the output being spent by this input + // is not greater than the current height if output.features.contains(transaction::COINBASE_OUTPUT) { - // check the lock_height of the output being spent by this input - // is not greater than the current height input.verify_lock_height(&output, b.header.height) .map_err(|_| Error::ImmatureCoinbase)?; - - // This input is spending an output that has sufficiently matured. - // For a coinbase output we need to verify it - // was originally created with a valid lock_height - // based on the coinbase maturity consensus rule. - let out_header = ctx.store.get_block_header_by_output_commit(&input.commitment()); - if let Ok(out_header) = out_header { - if input.lock_height() > out_header.height + global::coinbase_maturity() { - return Err(Error::ImmatureCoinbase); - } - } } } } diff --git a/chain/src/store.rs b/chain/src/store.rs index 35a9eddaa0..7499907142 100644 --- a/chain/src/store.rs +++ b/chain/src/store.rs @@ -137,35 +137,6 @@ impl ChainStore for ChainKVStore { batch.write() } - // lookup the block header hash by output commitment - // lookup the block header based on this hash - // to check the chain is correct compare this block header to - // the block header currently indexed at the relevant block height (tbd if - // actually necessary) - // - // NOTE: This index is not exhaustive. - // This node may not have seen this full block, so may not have populated the - // index. - // Block headers older than some threshold (2 months?) will not necessarily be - // included - // in this index. - // - fn get_block_header_by_output_commit(&self, commit: &Commitment) -> Result { - let block_hash = self.db.get_ser(&to_key( - HEADER_BY_OUTPUT_PREFIX, - &mut commit.as_ref().to_vec(), - ))?; - - match block_hash { - Some(hash) => { - let block_header = self.get_block_header(&hash)?; - self.is_on_current_chain(&block_header)?; - Ok(block_header) - } - None => Err(Error::NotFoundErr), - } - } - fn is_on_current_chain(&self, header: &BlockHeader) -> Result<(), Error> { let header_at_height = self.get_header_by_height(header.height)?; if header.hash() == header_at_height.hash() { diff --git a/chain/src/types.rs b/chain/src/types.rs index 15fd157624..369a269920 100644 --- a/chain/src/types.rs +++ b/chain/src/types.rs @@ -246,12 +246,6 @@ pub trait ChainStore: Send + Sync { /// Gets an output by its commitment fn get_output_by_commit(&self, commit: &Commitment) -> Result; - /// Gets a block_header for the given input commit - fn get_block_header_by_output_commit( - &self, - commit: &Commitment, - ) -> Result; - /// Saves the position of an output, represented by its commitment, in the /// UTXO MMR. Used as an index for spending and pruning. fn save_output_pos(&self, commit: &Commitment, pos: u64) -> Result<(), store::Error>; diff --git a/chain/tests/mine_simple_chain.rs b/chain/tests/mine_simple_chain.rs index 499aa407a7..a804824eff 100644 --- a/chain/tests/mine_simple_chain.rs +++ b/chain/tests/mine_simple_chain.rs @@ -110,13 +110,6 @@ fn mine_empty_chain() { // now check the block height index let header_by_height = chain.get_header_by_height(n).unwrap(); assert_eq!(header_by_height.hash(), bhash); - - // now check the header output index - let output = block.outputs[0]; - let header_by_output_commit = chain - .get_block_header_by_output_commit(&output.commitment()) - .unwrap(); - assert_eq!(header_by_output_commit.hash(), bhash); } } diff --git a/chain/tests/store_indices.rs b/chain/tests/store_indices.rs index 2d35867eec..7134f6de40 100644 --- a/chain/tests/store_indices.rs +++ b/chain/tests/store_indices.rs @@ -59,9 +59,4 @@ fn test_various_store_indices() { let block_header = chain_store.get_header_by_height(1).unwrap(); assert_eq!(block_header.hash(), block_hash); - - let block_header = chain_store - .get_block_header_by_output_commit(&commit) - .unwrap(); - assert_eq!(block_header.hash(), block_hash); } diff --git a/grin/src/adapters.rs b/grin/src/adapters.rs index 4cf763333b..a6ab14654b 100644 --- a/grin/src/adapters.rs +++ b/grin/src/adapters.rs @@ -343,16 +343,6 @@ impl pool::BlockChain for PoolToChainAdapter { }) } - fn get_block_header_by_output_commit( - &self, - commit: &Commitment, - ) -> Result { - self.chain - .borrow() - .get_block_header_by_output_commit(commit) - .map_err(|_| pool::PoolError::GenericPoolError) - } - fn head_header(&self) -> Result { self.chain .borrow() diff --git a/pool/src/blockchain.rs b/pool/src/blockchain.rs index b56a8a0dbe..0bea9f64ed 100644 --- a/pool/src/blockchain.rs +++ b/pool/src/blockchain.rs @@ -29,16 +29,6 @@ impl DummyBlockHeaderIndex { pub fn insert(&mut self, commit: Commitment, block_header: block::BlockHeader) { self.block_headers.insert(commit, block_header); } - - pub fn get_block_header_by_output_commit( - &self, - commit: Commitment, - ) -> Result<&block::BlockHeader, PoolError> { - match self.block_headers.get(&commit) { - Some(h) => Ok(h), - None => Err(PoolError::GenericPoolError), - } - } } /// A DummyUtxoSet for mocking up the chain @@ -136,20 +126,6 @@ impl BlockChain for DummyChainImpl { } } - fn get_block_header_by_output_commit( - &self, - commit: &Commitment, - ) -> Result { - match self.block_headers - .read() - .unwrap() - .get_block_header_by_output_commit(*commit) - { - Ok(h) => Ok(h.clone()), - Err(e) => Err(e), - } - } - fn head_header(&self) -> Result { let headers = self.head_header.read().unwrap(); if headers.len() > 0 { diff --git a/pool/src/pool.rs b/pool/src/pool.rs index 31079bc172..944f475bd9 100644 --- a/pool/src/pool.rs +++ b/pool/src/pool.rs @@ -20,7 +20,6 @@ pub use graph; use core::core::transaction; use core::core::block; use core::core::hash; -use core::global; use util::secp::pedersen::Commitment; @@ -168,7 +167,7 @@ where } // The next issue is to identify all unspent outputs that - // this transaction will consume and make sure they exist in the set. + // this transaction will consume and make sure they exist in the set. let mut pool_refs: Vec = Vec::new(); let mut orphan_refs: Vec = Vec::new(); let mut blockchain_refs: Vec = Vec::new(); @@ -177,25 +176,22 @@ where let base = graph::Edge::new(None, Some(tx_hash), input.commitment()); // Note that search_for_best_output does not examine orphans, by - // design. If an incoming transaction consumes pool outputs already - // spent by the orphans set, this does not preclude its inclusion - // into the pool. + // design. If an incoming transaction consumes pool outputs already + // spent by the orphans set, this does not preclude its inclusion + // into the pool. match self.search_for_best_output(&input.commitment()) { Parent::PoolTransaction { tx_ref: x } => pool_refs.push(base.with_source(Some(x))), Parent::BlockTransaction { output } => { - // TODO - pull this out into a separate function? + // TODO - pull this out into a separate function + // TODO - or wrap it up in search_for_best_output? if output.features.contains(transaction::COINBASE_OUTPUT) { - if let Ok(out_header) = self.blockchain - .get_block_header_by_output_commit(&output.commitment()) - { - let lock_height = out_header.height + global::coinbase_maturity(); - if head_header.height < lock_height { - return Err(PoolError::ImmatureCoinbase { - header: out_header, - output: output.commitment(), - }); - }; - }; + let height = head_header.height + 1; + input.verify_lock_height(&output, height) + .map_err(|_| PoolError::ImmatureCoinbase { + height: height, + lock_height: input.lock_height(), + output: output.commitment(), + })?; }; blockchain_refs.push(base); } diff --git a/pool/src/types.rs b/pool/src/types.rs index 02c182ba66..403757ca37 100644 --- a/pool/src/types.rs +++ b/pool/src/types.rs @@ -123,8 +123,10 @@ pub enum PoolError { /// Attempt to spend an output before it matures /// lock_height must not exceed current block height ImmatureCoinbase { - /// The block header of the block containing the output - header: block::BlockHeader, + /// The height of the block we are attempting to spend the coinbase output + height: u64, + /// The lock_height of the coinbase output + lock_height: u64, /// The unspent output output: Commitment, }, @@ -157,13 +159,6 @@ pub trait BlockChain { /// orphans, etc. fn get_unspent(&self, output_ref: &Commitment) -> Result; - /// Get the block header by output commitment (needed for spending coinbase - /// after n blocks) - fn get_block_header_by_output_commit( - &self, - commit: &Commitment, - ) -> Result; - /// Get the block header at the head fn head_header(&self) -> Result; } diff --git a/wallet/src/checker.rs b/wallet/src/checker.rs index 38a5abe1f0..f62811b5f6 100644 --- a/wallet/src/checker.rs +++ b/wallet/src/checker.rs @@ -26,11 +26,7 @@ use util; use util::LOGGER; // Transitions a local wallet output from Unconfirmed -> Unspent. -// Also updates the height and lock_height based on latest from the api. -fn refresh_output(out: &mut OutputData, api_out: &api::Output) { - out.height = api_out.height; - out.lock_height = api_out.lock_height; - +fn refresh_output(out: &mut OutputData, _api_out: &api::Output) { match out.status { OutputStatus::Unconfirmed => { out.status = OutputStatus::Unspent; @@ -50,9 +46,26 @@ fn mark_spent_output(out: &mut OutputData) { } } +pub fn refresh_outputs(config: &WalletConfig, keychain: &Keychain) -> Result<(), Error> { + refresh_output_state(config, keychain)?; + refresh_missing_heights(config, keychain)?; + Ok(()) +} + +fn refresh_missing_heights(config: &WalletConfig, keychain: &Keychain) -> Result<(), Error> { + debug!(LOGGER, "Refreshing missing heights (to be implemented)"); + + // TODO - implement this - + // * compile list of Unspent outputs with height 0 + // * hit the utxos by height api (all of the block chain?) + // * fill heights in based on block headers and their contained outputs + + Ok(()) +} + /// Builds a single api query to retrieve the latest output data from the node. /// So we can refresh the local wallet outputs. -pub fn refresh_outputs(config: &WalletConfig, keychain: &Keychain) -> Result<(), Error> { +fn refresh_output_state(config: &WalletConfig, keychain: &Keychain) -> Result<(), Error> { debug!(LOGGER, "Refreshing wallet outputs"); let mut wallet_outputs: HashMap = HashMap::new(); let mut commits: Vec = vec![]; @@ -73,7 +86,7 @@ pub fn refresh_outputs(config: &WalletConfig, keychain: &Keychain) -> Result<(), }); // build the necessary query params - - // ?id=xxx&id=yyy&id=zzz + // ?id=xxx&id=yyy&id=zzz let query_params: Vec = commits .iter() .map(|commit| { @@ -103,9 +116,9 @@ pub fn refresh_outputs(config: &WalletConfig, keychain: &Keychain) -> Result<(), }; // now for each commit, find the output in the wallet and - // the corresponding api output (if it exists) - // and refresh it in-place in the wallet. - // Note: minimizing the time we spend holding the wallet lock. + // the corresponding api output (if it exists) + // and refresh it in-place in the wallet. + // Note: minimizing the time we spend holding the wallet lock. WalletData::with_wallet(&config.data_file_dir, |wallet_data| for commit in commits { let id = wallet_outputs.get(&commit).unwrap(); if let Entry::Occupied(mut output) = wallet_data.outputs.entry(id.to_hex()) { diff --git a/wallet/src/info.rs b/wallet/src/info.rs index f9a36c80a5..d242650997 100644 --- a/wallet/src/info.rs +++ b/wallet/src/info.rs @@ -23,7 +23,6 @@ use std::io::prelude::*; pub fn show_info(config: &WalletConfig, keychain: &Keychain) { let result = checker::refresh_outputs(&config, &keychain); - let _ = WalletData::read_wallet(&config.data_file_dir, |wallet_data| { let current_height = match checker::get_tip_from_node(config) { Ok(tip) => tip.height, @@ -32,26 +31,26 @@ pub fn show_info(config: &WalletConfig, keychain: &Keychain) { None => 0, }, }; - let mut unspent_total=0; - let mut unspent_but_locked_total=0; - let mut unconfirmed_total=0; - let mut locked_total=0; + let mut unspent_total = 0; + let mut unspent_but_locked_total = 0; + let mut unconfirmed_total = 0; + let mut locked_total = 0; for out in wallet_data .outputs .values() .filter(|out| out.root_key_id == keychain.root_key_id()) { if out.status == OutputStatus::Unspent { - unspent_total+=out.value; + unspent_total += out.value; if out.lock_height > current_height { - unspent_but_locked_total+=out.value; + unspent_but_locked_total += out.value; } } if out.status == OutputStatus::Unconfirmed && !out.is_coinbase { - unconfirmed_total+=out.value; + unconfirmed_total += out.value; } if out.status == OutputStatus::Locked { - locked_total+=out.value; + locked_total += out.value; } }; diff --git a/wallet/src/receiver.rs b/wallet/src/receiver.rs index 4a6196eb48..c7a205965d 100644 --- a/wallet/src/receiver.rs +++ b/wallet/src/receiver.rs @@ -122,7 +122,8 @@ pub fn receive_coinbase( ) -> Result<(Output, TxKernel, BlockFees), Error> { let root_key_id = keychain.root_key_id(); - let lock_height = block_fees.height + global::coinbase_maturity(); + let height = block_fees.height; + let lock_height = height + global::coinbase_maturity(); // Now acquire the wallet lock and write the new output. let (key_id, derivation) = WalletData::with_wallet(&config.data_file_dir, |wallet_data| { @@ -139,7 +140,7 @@ pub fn receive_coinbase( n_child: derivation, value: reward(block_fees.fees), status: OutputStatus::Unconfirmed, - height: 0, + height: height, lock_height: lock_height, is_coinbase: true, }); @@ -207,7 +208,7 @@ fn receive_transaction( value: out_amount, status: OutputStatus::Unconfirmed, height: 0, - lock_height: lock_height, + lock_height: 0, is_coinbase: false, }); diff --git a/wallet/src/restore.rs b/wallet/src/restore.rs index 3b988e1ba6..680bdbecd3 100644 --- a/wallet/src/restore.rs +++ b/wallet/src/restore.rs @@ -16,6 +16,7 @@ use keychain::{Keychain, Identifier}; use util::{LOGGER, from_hex}; use util::secp::pedersen; use api; +use core::global; use core::core::{Output, SwitchCommitHash, SwitchCommitHashKey}; use core::core::transaction::{COINBASE_OUTPUT, DEFAULT_OUTPUT}; use types::{WalletConfig, WalletData, OutputData, OutputStatus, Error}; @@ -37,7 +38,7 @@ pub fn get_chain_height(config: &WalletConfig) -> Result { fn output_with_range_proof(config: &WalletConfig, commit_id: &str) -> Result { let url = format!( - "{}/v1/chain/utxos/byids?id={}&include_rp&include_switch", + "{}/v1/chain/utxos/byids?id={}&include_rp", config.check_node_api_http_addr, commit_id, ); @@ -72,7 +73,7 @@ fn retrieve_amount_and_coinbase_status( api::OutputType::Transaction => DEFAULT_OUTPUT, }, proof: output.proof.expect("output with proof"), - switch_commit_hash: output.switch_commit_hash.expect("output with switch_commit_hash"), + switch_commit_hash: output.switch_commit_hash, commit: output.commit, }; if let Some(amount) = core_output.recover_value(keychain, &key_id) { @@ -92,7 +93,6 @@ pub fn utxos_batch_block( end_height: u64, ) -> Result, Error> { // build the necessary query param - - // ?height=x let query_param = format!("start_height={}&end_height={}", start_height, end_height); let url = @@ -135,11 +135,12 @@ fn find_utxos_with_key( for i in 0..*key_iterations { let expected_hash = match output.output_type { api::OutputType::Coinbase => { + let lock_height = block_outputs.header.height + global::coinbase_maturity(); SwitchCommitHash::from_switch_commit( switch_commit_cache[i as usize], SwitchCommitHashKey::from_features_and_lock_height( COINBASE_OUTPUT, - output.lock_height, + lock_height, ), ) } @@ -175,13 +176,20 @@ fn find_utxos_with_key( .commit_with_key_index(BigEndian::read_u64(&commit_id), i as u32) .expect("commit with key index"); + let height = block_outputs.header.height; + let lock_height = if is_coinbase { + height + global::coinbase_maturity() + } else { + 0 + }; + wallet_outputs.push(( commit, key_id.clone(), i as u32, amount, - output.height, - output.lock_height, + height, + lock_height, is_coinbase, )); diff --git a/wallet/src/sender.rs b/wallet/src/sender.rs index d47c20c9fb..15c8ac4717 100644 --- a/wallet/src/sender.rs +++ b/wallet/src/sender.rs @@ -231,7 +231,7 @@ fn inputs_and_change( value: change as u64, status: OutputStatus::Unconfirmed, height: 0, - lock_height: lock_height, + lock_height: 0, is_coinbase: false, }); diff --git a/wallet/src/types.rs b/wallet/src/types.rs index 67cabef8ed..0e9faa0863 100644 --- a/wallet/src/types.rs +++ b/wallet/src/types.rs @@ -247,7 +247,7 @@ impl OutputData { pub fn num_confirmations(&self, current_height: u64) -> u64 { if self.status == OutputStatus::Unconfirmed { 0 - } else if self.status == OutputStatus::Spent && self.height == 0 { + } else if self.height == 0 { 0 } else { // if an output has height n and we are at block n From 47e47193071069ebc7422c0cea2fd28a2bc62bfc Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Wed, 3 Jan 2018 21:37:24 -0500 Subject: [PATCH 20/36] refresh heights for unspent wallet outputs where missing --- api/src/handlers.rs | 33 +++++++++--- api/src/types.rs | 19 +++---- wallet/src/checker.rs | 116 +++++++++++++++++++++++++++++++++++------- wallet/src/restore.rs | 1 + 4 files changed, 134 insertions(+), 35 deletions(-) diff --git a/api/src/handlers.rs b/api/src/handlers.rs index 9520e0327e..793042d975 100644 --- a/api/src/handlers.rs +++ b/api/src/handlers.rs @@ -54,7 +54,7 @@ impl Handler for IndexHandler { // Supports retrieval of multiple outputs in a single request - // GET /v1/chain/utxos/byids?id=xxx,yyy,zzz // GET /v1/chain/utxos/byids?id=xxx&id=yyy&id=zzz -// GET /v1/chain/utxos/byheight?height=n +// GET /v1/chain/utxos/atheight?start_height=101&end_height=200 struct UtxoHandler { chain: Arc, } @@ -107,7 +107,9 @@ impl UtxoHandler { .outputs .iter() .filter(|c| self.chain.is_unspent(&c.commit).unwrap()) - .map(|k| OutputPrintable::from_output(k)) + .map(|k| { + OutputPrintable::from_output(k, self.chain.clone()) + }) .collect(); BlockOutputs { header: BlockHeaderInfo::from_header(&header), @@ -117,9 +119,17 @@ impl UtxoHandler { // returns utxos for a specified range of blocks fn utxo_block_batch(&self, req: &mut Request) -> Vec { + let mut commitments: Vec<&str> = vec![]; let mut start_height = 1; let mut end_height = 1; if let Ok(params) = req.get_ref::() { + if let Some(ids) = params.get("id") { + for id in ids { + for id in id.split(",") { + commitments.push(id.clone()); + } + } + } if let Some(heights) = params.get("start_height") { for height in heights { start_height = height.parse().unwrap(); @@ -133,7 +143,17 @@ impl UtxoHandler { } let mut return_vec = vec![]; for i in start_height..end_height + 1 { - return_vec.push(self.utxos_at_height(i)); + let res = self.utxos_at_height(i); + if commitments.is_empty() { + return_vec.push(res); + } else { + for x in &res.outputs { + if commitments.contains(&x.commit.as_str()) { + return_vec.push(res.clone()); + break; + } + } + } } return_vec } @@ -331,11 +351,8 @@ pub struct BlockHandler { impl BlockHandler { fn get_block(&self, h: &Hash) -> Result { - let block = self.chain - .clone() - .get_block(h) - .map_err(|_| Error::NotFound)?; - Ok(BlockPrintable::from_block(&block)) + let block = self.chain.clone().get_block(h).map_err(|_| Error::NotFound)?; + Ok(BlockPrintable::from_block(&block, self.chain.clone())) } // Try to decode the string as a height or a hash. diff --git a/api/src/types.rs b/api/src/types.rs index f3f7244e2f..0c5032cfc9 100644 --- a/api/src/types.rs +++ b/api/src/types.rs @@ -84,11 +84,7 @@ impl SumTreeNode { let mut return_vec = Vec::new(); let last_n = chain.get_last_n_utxo(distance); for elem_output in last_n { - // Need to call further method to check if output is spent - let mut output = OutputPrintable::from_output(&elem_output.1); - if let Ok(_) = chain.get_unspent(&elem_output.1.commit) { - output.spent = false; - } + let output = OutputPrintable::from_output(&elem_output.1, chain.clone()); return_vec.push(SumTreeNode { hash: util::to_hex(elem_output.0.to_vec()), output: Some(output), @@ -181,7 +177,7 @@ pub struct OutputPrintable { } impl OutputPrintable { - pub fn from_output(output: &core::Output) -> OutputPrintable { + pub fn from_output(output: &core::Output, chain: Arc) -> OutputPrintable { let output_type = if output.features.contains(core::transaction::COINBASE_OUTPUT) { OutputType::Coinbase @@ -189,11 +185,16 @@ impl OutputPrintable { OutputType::Transaction }; + let spent = match chain.get_unspent(&output.commit) { + Ok(_) => false, + Err(_) => true, + }; + OutputPrintable { output_type: output_type, commit: util::to_hex(output.commit.0.to_vec()), switch_commit_hash: output.switch_commit_hash.to_hex(), - spent: true, + spent: spent, proof_hash: util::to_hex(output.proof.hash().to_vec()), } } @@ -304,14 +305,14 @@ pub struct BlockPrintable { } impl BlockPrintable { - pub fn from_block(block: &core::Block) -> BlockPrintable { + pub fn from_block(block: &core::Block, chain: Arc) -> BlockPrintable { let inputs = block.inputs .iter() .map(|x| util::to_hex(x.commitment().0.to_vec())) .collect(); let outputs = block.outputs .iter() - .map(|output| OutputPrintable::from_output(output)) + .map(|output| OutputPrintable::from_output(output, chain.clone())) .collect(); let kernels = block.kernels .iter() diff --git a/wallet/src/checker.rs b/wallet/src/checker.rs index f62811b5f6..468ade5dc3 100644 --- a/wallet/src/checker.rs +++ b/wallet/src/checker.rs @@ -1,4 +1,4 @@ -// Copyright 2016 The Grin Developers +// Copyright 2017 The Grin Developers // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -25,6 +25,7 @@ use util::secp::pedersen; use util; use util::LOGGER; + // Transitions a local wallet output from Unconfirmed -> Unspent. fn refresh_output(out: &mut OutputData, _api_out: &api::Output) { match out.status { @@ -53,42 +54,119 @@ pub fn refresh_outputs(config: &WalletConfig, keychain: &Keychain) -> Result<(), } fn refresh_missing_heights(config: &WalletConfig, keychain: &Keychain) -> Result<(), Error> { - debug!(LOGGER, "Refreshing missing heights (to be implemented)"); + // build a local map of wallet outputs keyed by commit + // and a list of outputs we want to query the node for + let mut wallet_outputs: HashMap = HashMap::new(); + let _ = WalletData::read_wallet(&config.data_file_dir, |wallet_data| { + for out in wallet_data + .outputs + .values() + .filter(|x| { + x.root_key_id == keychain.root_key_id() && + x.height == 0 && + x.status == OutputStatus::Unspent - // TODO - implement this - - // * compile list of Unspent outputs with height 0 - // * hit the utxos by height api (all of the block chain?) - // * fill heights in based on block headers and their contained outputs + }) + { + let commit = keychain.commit_with_key_index(out.value, out.n_child).unwrap(); + wallet_outputs.insert(commit, out.key_id.clone()); + } + }); - Ok(()) + // nothing to do so return (otherwise we hit the api with a monster query...) + if wallet_outputs.is_empty() { + return Ok(()); + } + + debug!(LOGGER, "Refreshing missing heights for {} outputs", wallet_outputs.len()); + + let mut id_params: Vec = wallet_outputs + .keys() + .map(|commit| { + let id = util::to_hex(commit.as_ref().to_vec()); + format!("id={}", id) + }) + .collect(); + + let tip = get_tip_from_node(config)?; + + let height_params = format!( + "start_height={}&end_height={}", + 0, + tip.height, + ); + let mut query_params = vec![height_params]; + query_params.append(&mut id_params); + + let url = + format!( + "{}/v1/chain/utxos/atheight?{}", + config.check_node_api_http_addr, + query_params.join("&"), + ); + debug!(LOGGER, "{:?}", url); + + let mut api_heights: HashMap = HashMap::new(); + match api::client::get::>(url.as_str()) { + Ok(blocks) => { + for block in blocks { + for out in block.outputs { + if let Ok(c) = util::from_hex(String::from(out.commit)) { + let commit = pedersen::Commitment::from_vec(c); + api_heights.insert(commit, block.header.height); + } + } + } + } + Err(e) => { + // if we got anything other than 200 back from server, bye + error!(LOGGER, "Refresh failed... unable to contact node: {}", e); + return Err(Error::Node(e)); + } + } + + // now for each commit, find the output in the wallet and + // the corresponding api output (if it exists) + // and refresh it in-place in the wallet. + // Note: minimizing the time we spend holding the wallet lock. + WalletData::with_wallet(&config.data_file_dir, |wallet_data| { + for commit in wallet_outputs.keys() { + let id = wallet_outputs.get(&commit).unwrap(); + if let Entry::Occupied(mut output) = wallet_data.outputs.entry(id.to_hex()) { + if let Some(height) = api_heights.get(&commit) { + output.get_mut().height = *height; + } + } + } + }) } /// Builds a single api query to retrieve the latest output data from the node. /// So we can refresh the local wallet outputs. fn refresh_output_state(config: &WalletConfig, keychain: &Keychain) -> Result<(), Error> { debug!(LOGGER, "Refreshing wallet outputs"); - let mut wallet_outputs: HashMap = HashMap::new(); - let mut commits: Vec = vec![]; - // build a local map of wallet outputs by commits + // build a local map of wallet outputs keyed by commit // and a list of outputs we want to query the node for + let mut wallet_outputs: HashMap = HashMap::new(); let _ = WalletData::read_wallet(&config.data_file_dir, |wallet_data| { for out in wallet_data .outputs .values() - .filter(|out| out.root_key_id == keychain.root_key_id()) - .filter(|out| out.status != OutputStatus::Spent) + .filter(|x| { + x.root_key_id == keychain.root_key_id() && + x.status != OutputStatus::Spent + }) { let commit = keychain.commit_with_key_index(out.value, out.n_child).unwrap(); - commits.push(commit); wallet_outputs.insert(commit, out.key_id.clone()); } }); // build the necessary query params - // ?id=xxx&id=yyy&id=zzz - let query_params: Vec = commits - .iter() + let query_params: Vec = wallet_outputs + .keys() .map(|commit| { let id = util::to_hex(commit.as_ref().to_vec()); format!("id={}", id) @@ -105,8 +183,10 @@ fn refresh_output_state(config: &WalletConfig, keychain: &Keychain) -> Result<() // build a map of api outputs by commit so we can look them up efficiently let mut api_outputs: HashMap = HashMap::new(); match api::client::get::>(url.as_str()) { - Ok(outputs) => for out in outputs { - api_outputs.insert(out.commit, out); + Ok(outputs) => { + for out in outputs { + api_outputs.insert(out.commit, out); + } }, Err(e) => { // if we got anything other than 200 back from server, don't attempt to refresh the wallet @@ -119,7 +199,7 @@ fn refresh_output_state(config: &WalletConfig, keychain: &Keychain) -> Result<() // the corresponding api output (if it exists) // and refresh it in-place in the wallet. // Note: minimizing the time we spend holding the wallet lock. - WalletData::with_wallet(&config.data_file_dir, |wallet_data| for commit in commits { + WalletData::with_wallet(&config.data_file_dir, |wallet_data| for commit in wallet_outputs.keys() { let id = wallet_outputs.get(&commit).unwrap(); if let Entry::Occupied(mut output) = wallet_data.outputs.entry(id.to_hex()) { match api_outputs.get(&commit) { diff --git a/wallet/src/restore.rs b/wallet/src/restore.rs index 680bdbecd3..b8dc97ee45 100644 --- a/wallet/src/restore.rs +++ b/wallet/src/restore.rs @@ -112,6 +112,7 @@ pub fn utxos_batch_block( } } +// TODO - wrap the many return values in a struct fn find_utxos_with_key( config: &WalletConfig, keychain: &Keychain, From 4d3b48caa4a1edc285f3d63ebb880a4011b67095 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Wed, 3 Jan 2018 21:40:18 -0500 Subject: [PATCH 21/36] TODO - might be slow? --- wallet/src/checker.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/wallet/src/checker.rs b/wallet/src/checker.rs index 468ade5dc3..9c2b3cbc72 100644 --- a/wallet/src/checker.rs +++ b/wallet/src/checker.rs @@ -53,6 +53,7 @@ pub fn refresh_outputs(config: &WalletConfig, keychain: &Keychain) -> Result<(), Ok(()) } +// TODO - this might be slow if we have really old outputs that have never been refreshed fn refresh_missing_heights(config: &WalletConfig, keychain: &Keychain) -> Result<(), Error> { // build a local map of wallet outputs keyed by commit // and a list of outputs we want to query the node for From ced54eced2a486d7f3759c53058c82726758833b Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Wed, 3 Jan 2018 23:32:33 -0500 Subject: [PATCH 22/36] simplify - do not pass around lock_height for non coinbase outputs --- chain/src/store.rs | 5 +++++ core/src/core/build.rs | 7 +++---- wallet/src/receiver.rs | 8 +++----- wallet/src/sender.rs | 20 +++++++++----------- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/chain/src/store.rs b/chain/src/store.rs index 7499907142..2391462692 100644 --- a/chain/src/store.rs +++ b/chain/src/store.rs @@ -162,6 +162,11 @@ impl ChainStore for ChainKVStore { } fn get_output_by_commit(&self, commit: &Commitment) -> Result { + // TODO - how to handle multiple "versions" of an output (multiple forks) + // * retrieve list of outputs from the db + // * then identify which one is actually valid based on what? + // * suspect we need to encode height in the output (based on the block itself?) + // * then check the "block at height" for consistency? option_to_not_found( self.db .get_ser(&to_key(OUTPUT_COMMIT_PREFIX, &mut commit.as_ref().to_vec())), diff --git a/core/src/core/build.rs b/core/src/core/build.rs index bac609ba30..c4478e5893 100644 --- a/core/src/core/build.rs +++ b/core/src/core/build.rs @@ -67,13 +67,12 @@ pub fn input( /// Adds an output with the provided value and key identifier from the /// keychain. -pub fn output(value: u64, lock_height: u64, key_id: Identifier) -> Box { +pub fn output(value: u64, key_id: Identifier) -> Box { Box::new(move |build, (tx, sum)| -> (Transaction, BlindSum) { debug!( LOGGER, - "Building an output: {}, {}, {}", + "Building an output: {}, {}", value, - lock_height, key_id, ); @@ -87,7 +86,7 @@ pub fn output(value: u64, lock_height: u64, key_id: Identifier) -> Box { LOGGER, "Builder - Pedersen Commit is: {:?}, Switch Commit is: {:?}", commit, - switch_commit + switch_commit, ); trace!( LOGGER, diff --git a/wallet/src/receiver.rs b/wallet/src/receiver.rs index c7a205965d..54ad213662 100644 --- a/wallet/src/receiver.rs +++ b/wallet/src/receiver.rs @@ -182,11 +182,9 @@ fn receive_transaction( ) -> Result { let root_key_id = keychain.root_key_id(); - let lock_height = partial.lock_height; - // double check the fee amount included in the partial tx - // we don't necessarily want to just trust the sender - // we could just overwrite the fee here (but we won't) due to the ecdsa sig + // we don't necessarily want to just trust the sender + // we could just overwrite the fee here (but we won't) due to the ecdsa sig let fee = tx_fee(partial.inputs.len(), partial.outputs.len() + 1, None); if fee != partial.fee { return Err(Error::FeeDispute { @@ -219,7 +217,7 @@ fn receive_transaction( vec![ build::initial_tx(partial), build::with_excess(blinding), - build::output(out_amount, lock_height, key_id.clone()), + build::output(out_amount, key_id.clone()), ], keychain, )?; diff --git a/wallet/src/sender.rs b/wallet/src/sender.rs index 15c8ac4717..b316c32aa4 100644 --- a/wallet/src/sender.rs +++ b/wallet/src/sender.rs @@ -126,7 +126,7 @@ fn build_send_tx( })?; // build transaction skeleton with inputs and change - let (mut parts, change_key) = inputs_and_change(&coins, config, keychain, amount, lock_height)?; + let (mut parts, change_key) = inputs_and_change(&coins, config, keychain, amount)?; // This is more proof of concept than anything but here we set lock_height // on tx being sent (based on current chain height via api). @@ -167,12 +167,11 @@ pub fn issue_burn_tx( debug!(LOGGER, "selected some coins - {}", coins.len()); - let lock_height = current_height; - let (mut parts, _) = inputs_and_change(&coins, config, keychain, amount, lock_height)?; + let (mut parts, _) = inputs_and_change(&coins, config, keychain, amount)?; // add burn output and fees let fee = tx_fee(coins.len(), 2, None); - parts.push(build::output(amount - fee, current_height, Identifier::zero())); + parts.push(build::output(amount - fee, Identifier::zero())); // finalize the burn transaction and send let (tx_burn, _) = build::transaction(parts, &keychain)?; @@ -190,7 +189,6 @@ fn inputs_and_change( config: &WalletConfig, keychain: &Keychain, amount: u64, - lock_height: u64, ) -> Result<(Vec>, Identifier), Error> { let mut parts = vec![]; @@ -201,15 +199,15 @@ fn inputs_and_change( } // sender is responsible for setting the fee on the partial tx - // recipient should double check the fee calculation and not blindly trust the - // sender + // 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 + // 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 @@ -238,7 +236,7 @@ fn inputs_and_change( change_key })?; - parts.push(build::output(change, lock_height, change_key.clone())); + parts.push(build::output(change, change_key.clone())); Ok((parts, change_key)) } From 1d2cb4eab1a9c1285a5f78080d4b5f610e145d75 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Thu, 4 Jan 2018 16:10:11 -0500 Subject: [PATCH 23/36] commit --- chain/src/store.rs | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/chain/src/store.rs b/chain/src/store.rs index 2391462692..93c79a0868 100644 --- a/chain/src/store.rs +++ b/chain/src/store.rs @@ -34,7 +34,6 @@ const HEADER_HEAD_PREFIX: u8 = 'I' as u8; const SYNC_HEAD_PREFIX: u8 = 's' as u8; const HEADER_HEIGHT_PREFIX: u8 = '8' as u8; const OUTPUT_COMMIT_PREFIX: u8 = 'o' as u8; -const HEADER_BY_OUTPUT_PREFIX: u8 = 'p' as u8; const COMMIT_POS_PREFIX: u8 = 'c' as u8; const KERNEL_POS_PREFIX: u8 = 'k' as u8; @@ -116,22 +115,12 @@ impl ChainStore for ChainKVStore { &b.header, )?; - // saving the full output under its hash, as well as a commitment to hash index + // saving the full output under its commitment for out in &b.outputs { batch = batch .put_ser( - &to_key( - OUTPUT_COMMIT_PREFIX, - &mut out.commitment().as_ref().to_vec(), - )[..], + &to_key(OUTPUT_COMMIT_PREFIX, &mut out.commitment().as_ref().to_vec())[..], out, - )? - .put_ser( - &to_key( - HEADER_BY_OUTPUT_PREFIX, - &mut out.commitment().as_ref().to_vec(), - )[..], - &b.hash(), )?; } batch.write() From 94f70f519cbe82977aa6ed7b5e8cf9032a7552b6 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Sun, 7 Jan 2018 11:01:32 -0500 Subject: [PATCH 24/36] fix tests after merge --- chain/tests/store_indices.rs | 1 - chain/tests/test_coinbase_maturity.rs | 6 +++--- core/src/core/block.rs | 8 ++++---- core/src/core/build.rs | 4 ++-- core/src/core/mod.rs | 18 +++++++++--------- 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/chain/tests/store_indices.rs b/chain/tests/store_indices.rs index 7134f6de40..d813e882d4 100644 --- a/chain/tests/store_indices.rs +++ b/chain/tests/store_indices.rs @@ -48,7 +48,6 @@ fn test_various_store_indices() { chain_store.setup_height(&genesis.header, &Tip::new(genesis.hash())).unwrap(); let block = Block::new(&genesis.header, vec![], &keychain, &key_id).unwrap(); - let commit = block.outputs[0].commitment(); let block_hash = block.hash(); chain_store.save_block(&block).unwrap(); diff --git a/chain/tests/test_coinbase_maturity.rs b/chain/tests/test_coinbase_maturity.rs index 434a5f59c3..ffb99efcea 100644 --- a/chain/tests/test_coinbase_maturity.rs +++ b/chain/tests/test_coinbase_maturity.rs @@ -109,7 +109,7 @@ fn test_coinbase_maturity() { let (coinbase_txn, _) = build::transaction( vec![ build::input(amount, height, key_id1.clone()), - build::output(amount - 2, height, key_id2.clone()), + build::output(amount - 2, key_id2.clone()), build::with_fee(2), ], &keychain, @@ -136,7 +136,7 @@ fn test_coinbase_maturity() { // confirm the block fails validation due to the invalid tx attempting to spend // the immature coinbase - let result = chain.process_block(block, chain::EASY_POW); + let result = chain.process_block(block, chain::SKIP_POW); match result { Err(Error::ImmatureCoinbase) => (), _ => panic!("expected ImmatureCoinbase error here"), @@ -175,7 +175,7 @@ fn test_coinbase_maturity() { let (coinbase_txn, _) = build::transaction( vec![ build::input(amount, height, key_id1.clone()), - build::output(amount - 2, height, key_id2.clone()), + build::output(amount - 2, key_id2.clone()), build::with_fee(2), ], &keychain, diff --git a/core/src/core/block.rs b/core/src/core/block.rs index a75bebed01..6087456694 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -664,7 +664,7 @@ mod test { key_id2: Identifier, ) -> Transaction { build::transaction( - vec![input(v, 0, key_id1), output(3, 0, key_id2), with_fee(2)], + vec![input(v, 0, key_id1), output(3, key_id2), with_fee(2)], &keychain, ).map(|(tx, _)| tx) .unwrap() @@ -684,7 +684,7 @@ mod test { let mut parts = vec![]; for _ in 0..max_out { - parts.push(output(5, 0, pks.pop().unwrap())); + parts.push(output(5, pks.pop().unwrap())); } let now = Instant::now(); @@ -708,7 +708,7 @@ mod test { let mut btx1 = tx2i1o(); let (mut btx2, _) = build::transaction( - vec![input(7, 0, key_id1), output(5, 0, key_id2.clone()), with_fee(2)], + vec![input(7, 0, key_id1), output(5, key_id2.clone()), with_fee(2)], &keychain, ).unwrap(); @@ -736,7 +736,7 @@ mod test { let mut btx1 = tx2i1o(); let (mut btx2, _) = build::transaction( - vec![input(7, 0, key_id1), output(5, 0, key_id2.clone()), with_fee(2)], + vec![input(7, 0, key_id1), output(5, key_id2.clone()), with_fee(2)], &keychain, ).unwrap(); diff --git a/core/src/core/build.rs b/core/src/core/build.rs index c4478e5893..d74a93039e 100644 --- a/core/src/core/build.rs +++ b/core/src/core/build.rs @@ -187,7 +187,7 @@ mod test { vec![ input(10, 0, key_id1), input(11, 0, key_id2), - output(20, 0, key_id3), + output(20, key_id3), with_fee(1), ], &keychain, @@ -203,7 +203,7 @@ mod test { let key_id2 = keychain.derive_key_id(2).unwrap(); let (tx, _) = transaction( - vec![input(6, 0, key_id1), output(2, 0, key_id2), with_fee(4)], + vec![input(6, 0, key_id1), output(2, key_id2), with_fee(4)], &keychain, ).unwrap(); diff --git a/core/src/core/mod.rs b/core/src/core/mod.rs index aefc89dc3a..a60e13d26f 100644 --- a/core/src/core/mod.rs +++ b/core/src/core/mod.rs @@ -248,7 +248,7 @@ mod test { build::transaction( vec![ input(10, 0, key_id1.clone()), - output(9, 0, key_id1.clone()), + output(9, key_id1.clone()), with_fee(1), ], &keychain, @@ -303,8 +303,8 @@ mod test { let (tx, _) = build::transaction( vec![ input(75, 0, key_id1), - output(42, 0, key_id2), - output(32, 0, key_id3), + output(42, key_id2), + output(32, key_id3), with_fee(1), ], &keychain, @@ -362,7 +362,7 @@ mod test { // 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, 0, key_id3), with_fee(2)], &keychain) + build::transaction(vec![in1, in2, output(1, key_id3), with_fee(2)], &keychain) .unwrap(); tx_alice = tx; blind_sum = sum; @@ -375,7 +375,7 @@ mod test { vec![ initial_tx(tx_alice), with_excess(blind_sum), - output(4, 0, key_id4), + output(4, key_id4), ], &keychain, ).unwrap(); @@ -434,7 +434,7 @@ mod test { let tx1 = build::transaction( vec![ input(5, 0, key_id1.clone()), - output(3, 0, key_id2.clone()), + output(3, key_id2.clone()), with_fee(2), with_lock_height(1), ], @@ -454,7 +454,7 @@ mod test { let tx1 = build::transaction( vec![ input(5, 0, key_id1.clone()), - output(3, 0, key_id2.clone()), + output(3, key_id2.clone()), with_fee(2), with_lock_height(2), ], @@ -499,7 +499,7 @@ mod test { vec![ input(10, 0, key_id1), input(11, 0, key_id2), - output(19, 0, key_id3), + output(19, key_id3), with_fee(2), ], &keychain, @@ -514,7 +514,7 @@ mod test { let key_id2 = keychain.derive_key_id(2).unwrap(); build::transaction( - vec![input(5, 0, key_id1), output(3, 0, key_id2), with_fee(2)], + vec![input(5, 0, key_id1), output(3, key_id2), with_fee(2)], &keychain, ).map(|(tx, _)| tx) .unwrap() From 057d58188e4bedac43cb3ff08ea446f35eb8574d Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Sun, 7 Jan 2018 12:57:09 -0500 Subject: [PATCH 25/36] build input vs coinbase_input switch commit hash key encodes lock_height cleanup output by commit index (currently broken...) --- chain/src/chain.rs | 8 ++++++ chain/src/pipe.rs | 5 ++++ chain/src/store.rs | 41 ++++++++++++++------------- chain/src/types.rs | 2 +- chain/tests/test_coinbase_maturity.rs | 6 ++-- core/src/core/block.rs | 2 +- core/src/core/build.rs | 35 ++++++++++++++++------- core/src/core/transaction.rs | 37 +++++++++++++----------- pool/src/pool.rs | 4 +-- wallet/src/restore.rs | 6 ++-- wallet/src/sender.rs | 6 +++- 11 files changed, 92 insertions(+), 60 deletions(-) diff --git a/chain/src/chain.rs b/chain/src/chain.rs index 943d3d1360..6cbfd9f313 100644 --- a/chain/src/chain.rs +++ b/chain/src/chain.rs @@ -336,6 +336,14 @@ impl Chain { /// way that's consistent with the current chain state and more /// specifically the current winning fork. pub fn get_unspent(&self, output_ref: &Commitment) -> Result { + // TODO - get output_pos from index + // TODO - lookup output in output_pmmr + // TODO - do something with it... + + // TODO - what do we return here if we have no outputs in the index??? + + panic!("not yet implemented..."); + match self.store.get_output_by_commit(output_ref) { Ok(out) => { let mut sumtrees = self.sumtrees.write().unwrap(); diff --git a/chain/src/pipe.rs b/chain/src/pipe.rs index 953ef2e646..cee671eae4 100644 --- a/chain/src/pipe.rs +++ b/chain/src/pipe.rs @@ -312,6 +312,11 @@ fn validate_block( } for input in &b.inputs { + // TODO - get output_pos from index + // TODO - lookup switch_commit_hash from output_pmmr + // TODO - do something with the switch_commit_hash + panic!("not yet implemented... working on it"); + if let Ok(output) = ctx.store.get_output_by_commit(&input.commitment()) { // check the lock_height of the output being spent by this input // is not greater than the current height diff --git a/chain/src/store.rs b/chain/src/store.rs index 93c79a0868..30f434c98c 100644 --- a/chain/src/store.rs +++ b/chain/src/store.rs @@ -33,7 +33,7 @@ const HEAD_PREFIX: u8 = 'H' as u8; const HEADER_HEAD_PREFIX: u8 = 'I' as u8; const SYNC_HEAD_PREFIX: u8 = 's' as u8; const HEADER_HEIGHT_PREFIX: u8 = '8' as u8; -const OUTPUT_COMMIT_PREFIX: u8 = 'o' as u8; +// const OUTPUT_COMMIT_PREFIX: u8 = 'o' as u8; const COMMIT_POS_PREFIX: u8 = 'c' as u8; const KERNEL_POS_PREFIX: u8 = 'k' as u8; @@ -115,14 +115,15 @@ impl ChainStore for ChainKVStore { &b.header, )?; - // saving the full output under its commitment - for out in &b.outputs { - batch = batch - .put_ser( - &to_key(OUTPUT_COMMIT_PREFIX, &mut out.commitment().as_ref().to_vec())[..], - out, - )?; - } + // // saving the full output under its commitment + // for out in &b.outputs { + // batch = batch + // .put_ser( + // &to_key(OUTPUT_COMMIT_PREFIX, &mut out.commitment().as_ref().to_vec())[..], + // out, + // )?; + // } + batch.write() } @@ -150,17 +151,17 @@ impl ChainStore for ChainKVStore { self.db.delete(&u64_to_key(HEADER_HEIGHT_PREFIX, height)) } - fn get_output_by_commit(&self, commit: &Commitment) -> Result { - // TODO - how to handle multiple "versions" of an output (multiple forks) - // * retrieve list of outputs from the db - // * then identify which one is actually valid based on what? - // * suspect we need to encode height in the output (based on the block itself?) - // * then check the "block at height" for consistency? - option_to_not_found( - self.db - .get_ser(&to_key(OUTPUT_COMMIT_PREFIX, &mut commit.as_ref().to_vec())), - ) - } + // fn get_output_by_commit(&self, commit: &Commitment) -> Result { + // // TODO - how to handle multiple "versions" of an output (multiple forks) + // // * retrieve list of outputs from the db + // // * then identify which one is actually valid based on what? + // // * suspect we need to encode height in the output (based on the block itself?) + // // * then check the "block at height" for consistency? + // option_to_not_found( + // self.db + // .get_ser(&to_key(OUTPUT_COMMIT_PREFIX, &mut commit.as_ref().to_vec())), + // ) + // } fn save_output_pos(&self, commit: &Commitment, pos: u64) -> Result<(), Error> { self.db.put_ser( diff --git a/chain/src/types.rs b/chain/src/types.rs index 369a269920..2b5cdd3243 100644 --- a/chain/src/types.rs +++ b/chain/src/types.rs @@ -244,7 +244,7 @@ pub trait ChainStore: Send + Sync { fn is_on_current_chain(&self, header: &BlockHeader) -> Result<(), store::Error>; /// Gets an output by its commitment - fn get_output_by_commit(&self, commit: &Commitment) -> Result; + // fn get_output_by_commit(&self, commit: &Commitment) -> Result; /// Saves the position of an output, represented by its commitment, in the /// UTXO MMR. Used as an index for spending and pruning. diff --git a/chain/tests/test_coinbase_maturity.rs b/chain/tests/test_coinbase_maturity.rs index ffb99efcea..a8e25645d4 100644 --- a/chain/tests/test_coinbase_maturity.rs +++ b/chain/tests/test_coinbase_maturity.rs @@ -92,7 +92,7 @@ fn test_coinbase_maturity() { assert!( block.outputs[0] .features - .contains(transaction::COINBASE_OUTPUT,) + .contains(transaction::COINBASE_OUTPUT) ); chain.process_block(block, chain::NONE).unwrap(); @@ -108,7 +108,7 @@ fn test_coinbase_maturity() { // this is not a valid tx as the coinbase output cannot be spent yet let (coinbase_txn, _) = build::transaction( vec![ - build::input(amount, height, key_id1.clone()), + build::coinbase_input(amount, height, key_id1.clone()), build::output(amount - 2, key_id2.clone()), build::with_fee(2), ], @@ -174,7 +174,7 @@ fn test_coinbase_maturity() { let (coinbase_txn, _) = build::transaction( vec![ - build::input(amount, height, key_id1.clone()), + build::coinbase_input(amount, height, key_id1.clone()), build::output(amount - 2, key_id2.clone()), build::with_fee(2), ], diff --git a/core/src/core/block.rs b/core/src/core/block.rs index 6087456694..ce11014132 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -583,7 +583,7 @@ impl Block { let switch_commit_hash = SwitchCommitHash::from_switch_commit( switch_commit, - SwitchCommitHashKey::from_features_and_lock_height(COINBASE_OUTPUT, lock_height), + SwitchCommitHashKey::from_lock_height(lock_height), ); trace!( diff --git a/core/src/core/build.rs b/core/src/core/build.rs index d74a93039e..dd14b5b1e0 100644 --- a/core/src/core/build.rs +++ b/core/src/core/build.rs @@ -44,27 +44,40 @@ 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( +fn input_with_lock_height( value: u64, lock_height: u64, - key_id: Identifier + key_id: Identifier, ) -> Box { - debug!( - LOGGER, - "Building an input: {}, {}, {}", - value, - lock_height, - key_id, - ); - Box::new(move |build, (tx, sum)| -> (Transaction, BlindSum) { let commit = build.keychain.commit(value, &key_id).unwrap(); let switch_commit = build.keychain.switch_commit(&key_id).unwrap(); - let input = Input::new(commit, switch_commit, lock_height); + let input = Input::new( + commit, + switch_commit, + lock_height, + ); (tx.with_input(input), sum.sub_key_id(key_id.clone())) }) } +pub fn input( + value: u64, + key_id: Identifier, +) -> Box { + debug!(LOGGER, "Building an input: {}, {}", value, key_id); + input_with_lock_height(value, 0, key_id) +} + +pub fn coinbase_input( + value: u64, + lock_height: u64, + key_id: Identifier, +) -> Box { + debug!(LOGGER, "Building a coinbase input: {}, {}", value, key_id); + input_with_lock_height(value, lock_height, key_id) +} + /// Adds an output with the provided value and key identifier from the /// keychain. pub fn output(value: u64, key_id: Identifier) -> Box { diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 617d860413..75a0a33a63 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -394,12 +394,18 @@ impl Transaction { } } -/// A transaction input, mostly a reference to an output being spent by the -/// transaction. +/// A transaction input. +/// +/// Primarily a reference to an output being spent by the transaction. +/// But also information required to verify coinbase maturity through +/// the lock_height hashed in the switch_commit_hash. #[derive(Debug, Clone, Copy)] pub struct Input{ commit: Commitment, + // We need to provide the switch_commit (switch_commit_hash preimage) + // to verify coinbase maturity. switch_commit: Commitment, + // We need to provide lock_height to verify coinbase maturity. lock_height: u64, } @@ -432,10 +438,14 @@ impl Readable for Input { /// commitment is a reproduction of the commitment of the output it's spending. impl Input { /// Build a new Input from a commit, switch_commit and lock_height - pub fn new(commit: Commitment, switch_commit: Commitment, lock_height: u64) -> Input { + pub fn new( + commit: Commitment, + switch_commit: Commitment, + lock_height: u64, + ) -> Input { debug!( LOGGER, - "building a new input: {:?}, {:?}, {}", + "building a new input: {:?}, {:?}, {:?}", commit, switch_commit, lock_height, @@ -467,15 +477,15 @@ impl Input { /// pub fn verify_lock_height( &self, - output: &Output, + output_switch_commit_hash: &SwitchCommitHash, height: u64, ) -> Result<(), Error> { let switch_commit_hash = SwitchCommitHash::from_switch_commit( self.switch_commit, - SwitchCommitHashKey::from_features_and_lock_height(output.features, self.lock_height), + SwitchCommitHashKey::from_lock_height(self.lock_height), ); - if switch_commit_hash != output.switch_commit_hash { + if switch_commit_hash != *output_switch_commit_hash { return Err(Error::SwitchCommitment); } else { if height <= self.lock_height { @@ -505,24 +515,17 @@ pub struct SwitchCommitHashKey ([u8; SWITCH_COMMIT_KEY_SIZE]); impl SwitchCommitHashKey { /// For a coinbase output use (output_features || lock_height) as the key. /// For regular tx outputs use the zero value as the key. - pub fn from_features_and_lock_height( - features: OutputFeatures, + pub fn from_lock_height( lock_height: u64, ) -> SwitchCommitHashKey { let mut bytes = [0; SWITCH_COMMIT_KEY_SIZE]; - if features.contains(COINBASE_OUTPUT) { - bytes[0] = features.bits(); - // seems wasteful to take up a full 8 bytes (of 20 bytes) to store the lock_height - // 4 bytes will give us approx 4,000 years with 1 min blocks (unless my math is way off) - BigEndian::write_u32(&mut bytes[1..5], lock_height as u32); - } + BigEndian::write_u64(&mut bytes[..], lock_height); SwitchCommitHashKey(bytes) } /// We use a zero value key for regular transactions. pub fn zero() -> SwitchCommitHashKey { - let bytes = [0; SWITCH_COMMIT_KEY_SIZE]; - SwitchCommitHashKey(bytes) + SwitchCommitHashKey::from_lock_height(0) } } diff --git a/pool/src/pool.rs b/pool/src/pool.rs index 944f475bd9..9815cf03ed 100644 --- a/pool/src/pool.rs +++ b/pool/src/pool.rs @@ -1228,7 +1228,7 @@ mod tests { for input_value in input_values { let key_id = keychain.derive_key_id(input_value as u32).unwrap(); - tx_elements.push(build::input(input_value, 0, key_id)); + tx_elements.push(build::input(input_value, key_id)); } for output_value in output_values { @@ -1256,7 +1256,7 @@ mod tests { for input_value in input_values { let key_id = keychain.derive_key_id(input_value as u32).unwrap(); - tx_elements.push(build::input(input_value, 0, key_id)); + tx_elements.push(build::input(input_value, key_id)); } for output_value in output_values { diff --git a/wallet/src/restore.rs b/wallet/src/restore.rs index b8dc97ee45..3c63820db2 100644 --- a/wallet/src/restore.rs +++ b/wallet/src/restore.rs @@ -134,15 +134,13 @@ fn find_utxos_with_key( for output in block_outputs.outputs { for i in 0..*key_iterations { + // TODO - these are very similar, factor out duplicate code let expected_hash = match output.output_type { api::OutputType::Coinbase => { let lock_height = block_outputs.header.height + global::coinbase_maturity(); SwitchCommitHash::from_switch_commit( switch_commit_cache[i as usize], - SwitchCommitHashKey::from_features_and_lock_height( - COINBASE_OUTPUT, - lock_height, - ), + SwitchCommitHashKey::from_lock_height(lock_height), ) } api::OutputType::Transaction => { diff --git a/wallet/src/sender.rs b/wallet/src/sender.rs index b316c32aa4..06018b4dbb 100644 --- a/wallet/src/sender.rs +++ b/wallet/src/sender.rs @@ -213,7 +213,11 @@ fn inputs_and_change( // build inputs using the appropriate derived key_ids for coin in coins { let key_id = keychain.derive_key_id(coin.n_child)?; - parts.push(build::input(coin.value, coin.lock_height, key_id)); + if coin.is_coinbase { + parts.push(build::coinbase_input(coin.value, coin.lock_height, key_id)); + } else { + parts.push(build::input(coin.value, key_id)); + } } // track the output representing our change From 5c5fe3912d22b388a272d1b2fbac42b6a90794d7 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Sun, 7 Jan 2018 15:08:17 -0500 Subject: [PATCH 26/36] is_unspent and get_unspent cleanup - we have no outputs, only switch_commit_hashes --- api/src/handlers.rs | 4 +-- chain/src/chain.rs | 57 +++++++++++++----------------------------- chain/src/pipe.rs | 16 ------------ chain/src/store.rs | 2 +- chain/src/sumtree.rs | 54 ++++++++++++++++++++++++--------------- chain/src/types.rs | 2 +- grin/src/adapters.rs | 5 +++- pool/src/blockchain.rs | 21 +++++++++++----- pool/src/pool.rs | 34 ++++++++++++------------- pool/src/types.rs | 13 +++++++--- 10 files changed, 102 insertions(+), 106 deletions(-) diff --git a/api/src/handlers.rs b/api/src/handlers.rs index e7552eb3de..70cff72cac 100644 --- a/api/src/handlers.rs +++ b/api/src/handlers.rs @@ -106,7 +106,7 @@ impl UtxoHandler { let outputs = block .outputs .iter() - .filter(|c| self.chain.is_unspent(&c.commit).unwrap()) + .filter(|c| self.chain.get_unspent(&c.commit).is_ok()) .map(|k| { OutputPrintable::from_output(k, self.chain.clone()) }) @@ -444,7 +444,7 @@ where tx.inputs.len(), tx.outputs.len() ); - + let res = self.tx_pool .write() .unwrap() diff --git a/chain/src/chain.rs b/chain/src/chain.rs index 6cbfd9f313..85c71f4d08 100644 --- a/chain/src/chain.rs +++ b/chain/src/chain.rs @@ -21,7 +21,7 @@ use std::time::{Duration, Instant}; use util::secp::pedersen::{Commitment, RangeProof}; -use core::core::SumCommit; +use core::core::{SumCommit, SwitchCommitHash}; use core::core::pmmr::{HashSum, NoSum}; use core::core::{Block, BlockHeader, Output, TxKernel}; @@ -331,37 +331,14 @@ impl Chain { } } - /// Gets an unspent output from its commitment. With return None if the - /// output doesn't exist or has been spent. This querying is done in a - /// way that's consistent with the current chain state and more - /// specifically the current winning fork. - pub fn get_unspent(&self, output_ref: &Commitment) -> Result { - // TODO - get output_pos from index - // TODO - lookup output in output_pmmr - // TODO - do something with it... - - // TODO - what do we return here if we have no outputs in the index??? - - panic!("not yet implemented..."); - - match self.store.get_output_by_commit(output_ref) { - Ok(out) => { - let mut sumtrees = self.sumtrees.write().unwrap(); - if sumtrees.is_unspent(output_ref)? { - Ok(out) - } else { - Err(Error::OutputNotFound) - } - } - Err(NotFoundErr) => Err(Error::OutputNotFound), - Err(e) => Err(Error::StoreErr(e, "chain get unspent".to_owned())), - } - } - - /// Checks whether an output is unspent - pub fn is_unspent(&self, output_ref: &Commitment) -> Result { + /// For the given commitment find the unspent output and return the associated + /// switch_commit_hash. + /// Return an error if the output does not exist or has been spent. + /// This querying is done in a way that is consistent with the current chain state, + /// specifically the current winning (valid, most work) fork. + pub fn get_unspent(&self, output_ref: &Commitment) -> Result { let mut sumtrees = self.sumtrees.write().unwrap(); - sumtrees.is_unspent(output_ref) + sumtrees.get_unspent(output_ref) } /// Sets the sumtree roots on a brand new block by applying the block on the @@ -398,14 +375,16 @@ impl Chain { /// returns sum tree hash plus output itself (as the sum is contained /// in the output anyhow) pub fn get_last_n_utxo(&self, distance: u64) -> Vec<(Hash, Output)> { - let mut sumtrees = self.sumtrees.write().unwrap(); - let mut return_vec = Vec::new(); - let sum_nodes = sumtrees.last_n_utxo(distance); - for sum_commit in sum_nodes { - let output = self.store.get_output_by_commit(&sum_commit.sum.commit); - return_vec.push((sum_commit.hash, output.unwrap())); - } - return_vec + panic!("we do not store outputs in the index any more... how to implement this???"); + + // let mut sumtrees = self.sumtrees.write().unwrap(); + // let mut return_vec = Vec::new(); + // let sum_nodes = sumtrees.last_n_utxo(distance); + // for sum_commit in sum_nodes { + // let output = self.store.get_output_by_commit(&sum_commit.sum.commit); + // return_vec.push((sum_commit.hash, output.unwrap())); + // } + // return_vec } /// as above, for rangeproofs diff --git a/chain/src/pipe.rs b/chain/src/pipe.rs index cee671eae4..1a0f979ec2 100644 --- a/chain/src/pipe.rs +++ b/chain/src/pipe.rs @@ -311,22 +311,6 @@ fn validate_block( return Err(Error::InvalidRoot); } - for input in &b.inputs { - // TODO - get output_pos from index - // TODO - lookup switch_commit_hash from output_pmmr - // TODO - do something with the switch_commit_hash - panic!("not yet implemented... working on it"); - - if let Ok(output) = ctx.store.get_output_by_commit(&input.commitment()) { - // check the lock_height of the output being spent by this input - // is not greater than the current height - if output.features.contains(transaction::COINBASE_OUTPUT) { - input.verify_lock_height(&output, b.header.height) - .map_err(|_| Error::ImmatureCoinbase)?; - } - } - } - Ok(()) } diff --git a/chain/src/store.rs b/chain/src/store.rs index 30f434c98c..7c5061b17a 100644 --- a/chain/src/store.rs +++ b/chain/src/store.rs @@ -107,7 +107,7 @@ impl ChainStore for ChainKVStore { fn save_block(&self, b: &Block) -> Result<(), Error> { // saving the block and its header - let mut batch = self.db + let batch = self.db .batch() .put_ser(&to_key(BLOCK_PREFIX, &mut b.hash().to_vec())[..], b)? .put_ser( diff --git a/chain/src/sumtree.rs b/chain/src/sumtree.rs index fa16fb5c41..7f599f630d 100644 --- a/chain/src/sumtree.rs +++ b/chain/src/sumtree.rs @@ -22,7 +22,7 @@ use std::sync::Arc; use util::secp::pedersen::{RangeProof, Commitment}; -use core::core::{Block, SumCommit, TxKernel}; +use core::core::{Block, SumCommit, SwitchCommitHash, TxKernel}; use core::core::pmmr::{HashSum, NoSum, Summable, PMMR}; use core::core::hash::Hashed; use grin_store; @@ -89,10 +89,11 @@ impl SumTrees { }) } - /// Whether a given commitment exists in the Output MMR and it's unspent - pub fn is_unspent(&mut self, commit: &Commitment) -> Result { - let rpos = self.commit_index.get_output_pos(commit); - match rpos { + /// Check if the given commitment exists in the output_pmmr. + /// If it does and everything looks good then return the + /// switch_commit_hash for the unspent output, otherwise return an error. + pub fn get_unspent(&mut self, commit: &Commitment) -> Result { + match self.commit_index.get_output_pos(commit) { Ok(pos) => { let output_pmmr = PMMR::at( &mut self.output_pmmr_h.backend, @@ -103,13 +104,17 @@ impl SumTrees { pos, &SumCommit::from_commit(&commit), ); - Ok(hs.hash == hashsum.hash) + if hs.hash == hashsum.hash { + Ok(hs.sum.switch_commit_hash) + } else { + Err(Error::SumTreeErr(format!("sumtree hash mismatch"))) + } } else { - Ok(false) + Err(Error::OutputNotFound) } } - Err(grin_store::Error::NotFoundErr) => Ok(false), - Err(e) => Err(Error::StoreErr(e, "sumtree unspent check".to_owned())), + Err(grin_store::Error::NotFoundErr) => Err(Error::OutputNotFound), + Err(e) => Err(Error::StoreErr(e, format!("sumtree unspent check"))), } } @@ -248,19 +253,28 @@ impl<'a> Extension<'a> { // same block, enforcing block cut-through for input in &b.inputs { let commit = input.commitment(); - let pos_res = self.get_output_pos(&commit); - if let Ok(pos) = pos_res { - match self.output_pmmr.prune(pos, b.header.height as u32) { - Ok(true) => { - self.rproof_pmmr - .prune(pos, b.header.height as u32) - .map_err(|s| Error::SumTreeErr(s))?; + match self.get_output_pos(&commit) { + Ok(pos) => { + // First verify the lock_height (coinbase maturity) + if let Some(HashSum{hash: _, sum: sum}) = self.output_pmmr.get(pos) { + input.verify_lock_height(&sum.switch_commit_hash, b.header.height) + .map_err(|_| Error::ImmatureCoinbase)?; + } + + // Now attempt to prune the entry from the output_pmmr + match self.output_pmmr.prune(pos, b.header.height as u32) { + Ok(true) => { + self.rproof_pmmr + .prune(pos, b.header.height as u32) + .map_err(|s| Error::SumTreeErr(s))?; + } + Ok(false) => return Err(Error::AlreadySpent(commit)), + Err(s) => return Err(Error::SumTreeErr(s)), } - Ok(false) => return Err(Error::AlreadySpent(commit)), - Err(s) => return Err(Error::SumTreeErr(s)), } - } else { - return Err(Error::AlreadySpent(commit)); + Err(_) => { + return Err(Error::AlreadySpent(commit)); + } } } diff --git a/chain/src/types.rs b/chain/src/types.rs index 2b5cdd3243..bb8aca004b 100644 --- a/chain/src/types.rs +++ b/chain/src/types.rs @@ -78,7 +78,7 @@ pub enum Error { StoreErr(grin_store::Error, String), /// Error serializing or deserializing a type SerErr(ser::Error), - /// Error while updating the sum trees + /// Error with the sumtrees SumTreeErr(String), /// No chain exists and genesis block is required GenesisBlockRequired, diff --git a/grin/src/adapters.rs b/grin/src/adapters.rs index a6ab14654b..e2a3ae042f 100644 --- a/grin/src/adapters.rs +++ b/grin/src/adapters.rs @@ -332,7 +332,10 @@ impl PoolToChainAdapter { } impl pool::BlockChain for PoolToChainAdapter { - fn get_unspent(&self, output_ref: &Commitment) -> Result { + fn get_unspent( + &self, + output_ref: &Commitment, + ) -> Result { self.chain .borrow() .get_unspent(output_ref) diff --git a/pool/src/blockchain.rs b/pool/src/blockchain.rs index 0bea9f64ed..65bb3ece6d 100644 --- a/pool/src/blockchain.rs +++ b/pool/src/blockchain.rs @@ -71,8 +71,15 @@ impl DummyUtxoSet { outputs: HashMap::new(), } } - pub fn get_output(&self, output_ref: &Commitment) -> Option<&transaction::Output> { - self.outputs.get(output_ref) + pub fn get_unspent( + &self, + output_ref: &Commitment, + ) -> Option { + if let Some(x) = self.outputs.get(output_ref) { + Some(x.switch_commit_hash()) + } else { + None + } } fn clone(&self) -> DummyUtxoSet { @@ -118,10 +125,12 @@ impl DummyChainImpl { } impl BlockChain for DummyChainImpl { - fn get_unspent(&self, commitment: &Commitment) -> Result { - let output = self.utxo.read().unwrap().get_output(commitment).cloned(); - match output { - Some(o) => Ok(o), + fn get_unspent( + &self, + commitment: &Commitment, + ) -> Result { + match self.utxo.read().unwrap().get_unspent(commitment) { + Some(x) => Ok(x), None => Err(PoolError::GenericPoolError), } } diff --git a/pool/src/pool.rs b/pool/src/pool.rs index 9815cf03ed..919e65c9c8 100644 --- a/pool/src/pool.rs +++ b/pool/src/pool.rs @@ -90,12 +90,14 @@ where self.blockchain .get_unspent(output_commitment) .ok() - .map(|output| { + .map(|switch_commit_hash| { match self.pool.get_blockchain_spent(output_commitment) { Some(x) => Parent::AlreadySpent { other_tx: x.destination_hash().unwrap(), }, - None => Parent::BlockTransaction { output }, + None => Parent::BlockTransaction { + switch_commit_hash, + }, } }) } @@ -181,18 +183,16 @@ where // into the pool. match self.search_for_best_output(&input.commitment()) { Parent::PoolTransaction { tx_ref: x } => pool_refs.push(base.with_source(Some(x))), - Parent::BlockTransaction { output } => { - // TODO - pull this out into a separate function - // TODO - or wrap it up in search_for_best_output? - if output.features.contains(transaction::COINBASE_OUTPUT) { - let height = head_header.height + 1; - input.verify_lock_height(&output, height) - .map_err(|_| PoolError::ImmatureCoinbase { - height: height, - lock_height: input.lock_height(), - output: output.commitment(), - })?; - }; + Parent::BlockTransaction { switch_commit_hash } => { + // TODO - wrap lock_height verification up in search_for_best_output? + let height = head_header.height + 1; + input.verify_lock_height(&switch_commit_hash, height) + .map_err(|_| PoolError::ImmatureCoinbase { + height: height, + lock_height: input.lock_height(), + output: input.commitment(), + })?; + blockchain_refs.push(base); } Parent::Unknown => orphan_refs.push(base), @@ -677,9 +677,9 @@ mod tests { { let read_pool = pool.read().unwrap(); assert_eq!(read_pool.total_size(), 2); - expect_output_parent!(read_pool, Parent::PoolTransaction{tx_ref: _}, 12); - expect_output_parent!(read_pool, Parent::AlreadySpent{other_tx: _}, 11, 5); - expect_output_parent!(read_pool, Parent::BlockTransaction{output: _}, 8); + expect_output_parent!(read_pool, Parent::PoolTransaction{_}, 12); + expect_output_parent!(read_pool, Parent::AlreadySpent{_}, 11, 5); + expect_output_parent!(read_pool, Parent::BlockTransaction{_}, 8); expect_output_parent!(read_pool, Parent::Unknown, 20); } } diff --git a/pool/src/types.rs b/pool/src/types.rs index 403757ca37..5484315ffd 100644 --- a/pool/src/types.rs +++ b/pool/src/types.rs @@ -78,7 +78,9 @@ pub struct TxSource { #[derive(Clone)] pub enum Parent { Unknown, - BlockTransaction { output: transaction::Output }, + /// We no longer maintain outputs in the index, we only have the + /// switch_commit_hash from the output_pmmr sumtree. + BlockTransaction { switch_commit_hash: transaction::SwitchCommitHash }, PoolTransaction { tx_ref: hash::Hash }, AlreadySpent { other_tx: hash::Hash }, } @@ -87,7 +89,7 @@ impl fmt::Debug for Parent { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { &Parent::Unknown => write!(f, "Parent: Unknown"), - &Parent::BlockTransaction { output: _ } => write!(f, "Parent: Block Transaction"), + &Parent::BlockTransaction { switch_commit_hash: _ } => write!(f, "Parent: Block Transaction"), &Parent::PoolTransaction { tx_ref: x } => { write!(f, "Parent: Pool Transaction ({:?})", x) } @@ -157,7 +159,12 @@ pub trait BlockChain { /// is spent or if it doesn't exist. The blockchain is expected to produce /// a result with its current view of the most worked chain, ignoring /// orphans, etc. - fn get_unspent(&self, output_ref: &Commitment) -> Result; + /// We do not maintain outputs themselves. The only information we have is the + /// switch_commit_hash from the output_pmmr sumtree. + fn get_unspent( + &self, + output_ref: &Commitment, + ) -> Result; /// Get the block header at the head fn head_header(&self) -> Result; From 6598d4542ac942b0dcefd59460c2f5114898314a Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Sun, 7 Jan 2018 15:31:53 -0500 Subject: [PATCH 27/36] separate concept of utxo vs output in the api utxos come from the sumtrees (and only the sumtrees, limited info) outputs come from blocks (and we need to look them up via block height) --- api/src/handlers.rs | 30 +++++++++++++----------------- api/src/types.rs | 34 +++++++++------------------------- chain/src/sumtree.rs | 2 +- 3 files changed, 23 insertions(+), 43 deletions(-) diff --git a/api/src/handlers.rs b/api/src/handlers.rs index 70cff72cac..ae6a2c02a6 100644 --- a/api/src/handlers.rs +++ b/api/src/handlers.rs @@ -60,22 +60,21 @@ struct UtxoHandler { } impl UtxoHandler { - fn get_utxo(&self, id: &str, include_rp: bool) -> Result { + fn get_utxo(&self, id: &str) -> Result { debug!(LOGGER, "getting utxo: {}", id); let c = util::from_hex(String::from(id)) .map_err(|_| Error::Argument(format!("Not a valid commitment: {}", id)))?; let commit = Commitment::from_vec(c); - let out = self.chain + let switch_commit_hash = self.chain .get_unspent(&commit) .map_err(|_| Error::NotFound)?; - Ok(Output::from_output(&out, include_rp)) + Ok(Utxo::new(&commit, &switch_commit_hash)) } - fn utxos_by_ids(&self, req: &mut Request) -> Vec { + fn utxos_by_ids(&self, req: &mut Request) -> Vec { let mut commitments: Vec<&str> = vec![]; - let mut include_rp = false; if let Ok(params) = req.get_ref::() { if let Some(ids) = params.get("id") { for id in ids { @@ -84,20 +83,17 @@ impl UtxoHandler { } } } - if let Some(_) = params.get("include_rp") { - include_rp = true; - } } - let mut utxos: Vec = vec![]; - for commit in commitments { - if let Ok(out) = self.get_utxo(commit, include_rp) { - utxos.push(out); + let mut utxos: Vec = vec![]; + for x in commitments { + if let Ok(utxo) = self.get_utxo(x) { + utxos.push(utxo); } } utxos } - fn utxos_at_height(&self, block_height: u64) -> BlockOutputs { + fn outputs_at_height(&self, block_height: u64) -> BlockOutputs { let header = self.chain .clone() .get_header_by_height(block_height) @@ -117,8 +113,8 @@ impl UtxoHandler { } } - // returns utxos for a specified range of blocks - fn utxo_block_batch(&self, req: &mut Request) -> Vec { + // returns outputs for a specified range of blocks + fn outputs_block_batch(&self, req: &mut Request) -> Vec { let mut commitments: Vec<&str> = vec![]; let mut start_height = 1; let mut end_height = 1; @@ -143,7 +139,7 @@ impl UtxoHandler { } let mut return_vec = vec![]; for i in start_height..end_height + 1 { - let res = self.utxos_at_height(i); + let res = self.outputs_at_height(i); if commitments.is_empty() { return_vec.push(res); } else { @@ -168,7 +164,7 @@ impl Handler for UtxoHandler { } match *path_elems.last().unwrap() { "byids" => json_response(&self.utxos_by_ids(req)), - "byheight" => json_response(&self.utxo_block_batch(req)), + "byheight" => json_response(&self.outputs_block_batch(req)), _ => Ok(Response::with((status::BadRequest, ""))), } } diff --git a/api/src/types.rs b/api/src/types.rs index 0c5032cfc9..380f76f97d 100644 --- a/api/src/types.rs +++ b/api/src/types.rs @@ -125,37 +125,21 @@ pub enum OutputType { } #[derive(Debug, Serialize, Deserialize, Clone)] -pub struct Output { - /// The type of output Coinbase|Transaction - pub output_type: OutputType, +pub struct Utxo { /// The homomorphic commitment representing the output amount pub commit: pedersen::Commitment, /// switch commit hash pub switch_commit_hash: core::SwitchCommitHash, - /// A proof that the commitment is in the right range - pub proof: Option, } -impl Output { - pub fn from_output( - output: &core::Output, - include_proof: bool, - ) -> Output { - let output_type = - if output.features.contains(core::transaction::COINBASE_OUTPUT) { - OutputType::Coinbase - } else { - OutputType::Transaction - }; - - Output { - output_type: output_type, - commit: output.commit, - switch_commit_hash: output.switch_commit_hash, - proof: match include_proof { - true => Some(output.proof), - false => None, - }, +impl Utxo { + pub fn new ( + commit: &pedersen::Commitment, + switch_commit_hash: &core::SwitchCommitHash, + ) -> Utxo { + Utxo { + commit: commit.clone(), + switch_commit_hash: switch_commit_hash.clone(), } } } diff --git a/chain/src/sumtree.rs b/chain/src/sumtree.rs index 7f599f630d..f8e6ffa287 100644 --- a/chain/src/sumtree.rs +++ b/chain/src/sumtree.rs @@ -97,7 +97,7 @@ impl SumTrees { Ok(pos) => { let output_pmmr = PMMR::at( &mut self.output_pmmr_h.backend, - self.output_pmmr_h.last_pos + self.output_pmmr_h.last_pos, ); if let Some(hs) = output_pmmr.get(pos) { let hashsum = HashSum::from_summable( From 7b3b5d6062fc0fda1936059b7bf06138faab13e3 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Sun, 7 Jan 2018 15:46:30 -0500 Subject: [PATCH 28/36] cleanup --- chain/src/store.rs | 30 +++++------------------------- 1 file changed, 5 insertions(+), 25 deletions(-) diff --git a/chain/src/store.rs b/chain/src/store.rs index 7c5061b17a..0da05bd79a 100644 --- a/chain/src/store.rs +++ b/chain/src/store.rs @@ -33,7 +33,6 @@ const HEAD_PREFIX: u8 = 'H' as u8; const HEADER_HEAD_PREFIX: u8 = 'I' as u8; const SYNC_HEAD_PREFIX: u8 = 's' as u8; const HEADER_HEIGHT_PREFIX: u8 = '8' as u8; -// const OUTPUT_COMMIT_PREFIX: u8 = 'o' as u8; const COMMIT_POS_PREFIX: u8 = 'c' as u8; const KERNEL_POS_PREFIX: u8 = 'k' as u8; @@ -105,25 +104,18 @@ impl ChainStore for ChainKVStore { ) } + /// Save the block and its header fn save_block(&self, b: &Block) -> Result<(), Error> { - // saving the block and its header let batch = self.db .batch() - .put_ser(&to_key(BLOCK_PREFIX, &mut b.hash().to_vec())[..], b)? + .put_ser( + &to_key(BLOCK_PREFIX, &mut b.hash().to_vec())[..], + b, + )? .put_ser( &to_key(BLOCK_HEADER_PREFIX, &mut b.hash().to_vec())[..], &b.header, )?; - - // // saving the full output under its commitment - // for out in &b.outputs { - // batch = batch - // .put_ser( - // &to_key(OUTPUT_COMMIT_PREFIX, &mut out.commitment().as_ref().to_vec())[..], - // out, - // )?; - // } - batch.write() } @@ -151,18 +143,6 @@ impl ChainStore for ChainKVStore { self.db.delete(&u64_to_key(HEADER_HEIGHT_PREFIX, height)) } - // fn get_output_by_commit(&self, commit: &Commitment) -> Result { - // // TODO - how to handle multiple "versions" of an output (multiple forks) - // // * retrieve list of outputs from the db - // // * then identify which one is actually valid based on what? - // // * suspect we need to encode height in the output (based on the block itself?) - // // * then check the "block at height" for consistency? - // option_to_not_found( - // self.db - // .get_ser(&to_key(OUTPUT_COMMIT_PREFIX, &mut commit.as_ref().to_vec())), - // ) - // } - fn save_output_pos(&self, commit: &Commitment, pos: u64) -> Result<(), Error> { self.db.put_ser( &to_key(COMMIT_POS_PREFIX, &mut commit.as_ref().to_vec())[..], From 90ad055009e9608e1780ae95e52767b63c90ba16 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Sun, 7 Jan 2018 20:35:06 -0500 Subject: [PATCH 29/36] better api support for block outputs with range proofs --- api/src/handlers.rs | 40 ++++++++++++++++++++++++---------------- api/src/types.rs | 29 +++++++++++++++++++++++------ wallet/src/checker.rs | 19 +++++++++---------- wallet/src/restore.rs | 34 ++++++++++++++++++++++++++-------- 4 files changed, 82 insertions(+), 40 deletions(-) diff --git a/api/src/handlers.rs b/api/src/handlers.rs index ae6a2c02a6..3a855c6e39 100644 --- a/api/src/handlers.rs +++ b/api/src/handlers.rs @@ -54,7 +54,7 @@ impl Handler for IndexHandler { // Supports retrieval of multiple outputs in a single request - // GET /v1/chain/utxos/byids?id=xxx,yyy,zzz // GET /v1/chain/utxos/byids?id=xxx&id=yyy&id=zzz -// GET /v1/chain/utxos/atheight?start_height=101&end_height=200 +// GET /v1/chain/utxos/byheight?start_height=101&end_height=200 struct UtxoHandler { chain: Arc, } @@ -93,7 +93,12 @@ impl UtxoHandler { utxos } - fn outputs_at_height(&self, block_height: u64) -> BlockOutputs { + fn outputs_at_height( + &self, + block_height: u64, + commitments: Vec, + include_proof: bool, + ) -> BlockOutputs { let header = self.chain .clone() .get_header_by_height(block_height) @@ -102,9 +107,12 @@ impl UtxoHandler { let outputs = block .outputs .iter() - .filter(|c| self.chain.get_unspent(&c.commit).is_ok()) + .filter(|c| { + (commitments.is_empty() || commitments.contains(&c.commit)) && + self.chain.get_unspent(&c.commit).is_ok() + }) .map(|k| { - OutputPrintable::from_output(k, self.chain.clone()) + OutputPrintable::from_output(k, self.chain.clone(), include_proof) }) .collect(); BlockOutputs { @@ -115,14 +123,18 @@ impl UtxoHandler { // returns outputs for a specified range of blocks fn outputs_block_batch(&self, req: &mut Request) -> Vec { - let mut commitments: Vec<&str> = vec![]; + let mut commitments: Vec = vec![]; let mut start_height = 1; let mut end_height = 1; + let mut include_rp = false; + if let Ok(params) = req.get_ref::() { if let Some(ids) = params.get("id") { for id in ids { for id in id.split(",") { - commitments.push(id.clone()); + if let Ok(x) = util::from_hex(String::from(id)) { + commitments.push(Commitment::from_vec(x)); + } } } } @@ -136,19 +148,15 @@ impl UtxoHandler { end_height = height.parse().unwrap(); } } + if let Some(_) = params.get("include_rp") { + include_rp = true; + } } let mut return_vec = vec![]; for i in start_height..end_height + 1 { - let res = self.outputs_at_height(i); - if commitments.is_empty() { + let res = self.outputs_at_height(i, commitments.clone(), include_rp); + if res.outputs.len() > 0 { return_vec.push(res); - } else { - for x in &res.outputs { - if commitments.contains(&x.commit.as_str()) { - return_vec.push(res.clone()); - break; - } - } } } return_vec @@ -348,7 +356,7 @@ pub struct BlockHandler { impl BlockHandler { fn get_block(&self, h: &Hash) -> Result { let block = self.chain.clone().get_block(h).map_err(|_| Error::NotFound)?; - Ok(BlockPrintable::from_block(&block, self.chain.clone())) + Ok(BlockPrintable::from_block(&block, self.chain.clone(), false)) } // Try to decode the string as a height or a hash. diff --git a/api/src/types.rs b/api/src/types.rs index 380f76f97d..cbf5f74aef 100644 --- a/api/src/types.rs +++ b/api/src/types.rs @@ -84,7 +84,7 @@ impl SumTreeNode { let mut return_vec = Vec::new(); let last_n = chain.get_last_n_utxo(distance); for elem_output in last_n { - let output = OutputPrintable::from_output(&elem_output.1, chain.clone()); + let output = OutputPrintable::from_output(&elem_output.1, chain.clone(), false); return_vec.push(SumTreeNode { hash: util::to_hex(elem_output.0.to_vec()), output: Some(output), @@ -126,9 +126,9 @@ pub enum OutputType { #[derive(Debug, Serialize, Deserialize, Clone)] pub struct Utxo { - /// The homomorphic commitment representing the output amount + /// The output commitment representing the amount pub commit: pedersen::Commitment, - /// switch commit hash + /// The corresponding switch commit hash (can be used to verify lock height) pub switch_commit_hash: core::SwitchCommitHash, } @@ -156,12 +156,18 @@ pub struct OutputPrintable { pub switch_commit_hash: String, /// Whether the output has been spent pub spent: bool, + /// Rangeproof (as hex string) + pub proof: Option, /// Rangeproof hash (as hex string) pub proof_hash: String, } impl OutputPrintable { - pub fn from_output(output: &core::Output, chain: Arc) -> OutputPrintable { + pub fn from_output( + output: &core::Output, + chain: Arc, + include_proof: bool, + ) -> OutputPrintable { let output_type = if output.features.contains(core::transaction::COINBASE_OUTPUT) { OutputType::Coinbase @@ -174,11 +180,18 @@ impl OutputPrintable { Err(_) => true, }; + let proof = if include_proof { + Some(util::to_hex(output.proof.proof.to_vec())) + } else { + None + }; + OutputPrintable { output_type: output_type, commit: util::to_hex(output.commit.0.to_vec()), switch_commit_hash: output.switch_commit_hash.to_hex(), spent: spent, + proof: proof, proof_hash: util::to_hex(output.proof.hash().to_vec()), } } @@ -289,14 +302,18 @@ pub struct BlockPrintable { } impl BlockPrintable { - pub fn from_block(block: &core::Block, chain: Arc) -> BlockPrintable { + pub fn from_block( + block: &core::Block, + chain: Arc, + include_proof: bool, + ) -> BlockPrintable { let inputs = block.inputs .iter() .map(|x| util::to_hex(x.commitment().0.to_vec())) .collect(); let outputs = block.outputs .iter() - .map(|output| OutputPrintable::from_output(output, chain.clone())) + .map(|output| OutputPrintable::from_output(output, chain.clone(), include_proof)) .collect(); let kernels = block.kernels .iter() diff --git a/wallet/src/checker.rs b/wallet/src/checker.rs index 5be5adacd6..099a98b042 100644 --- a/wallet/src/checker.rs +++ b/wallet/src/checker.rs @@ -27,11 +27,9 @@ use util::LOGGER; // Transitions a local wallet output from Unconfirmed -> Unspent. -fn refresh_output(out: &mut OutputData, _api_out: &api::Output) { +fn mark_unspent_output(out: &mut OutputData) { match out.status { - OutputStatus::Unconfirmed => { - out.status = OutputStatus::Unspent; - } + OutputStatus::Unconfirmed => out.status = OutputStatus::Unspent, _ => (), } } @@ -42,7 +40,8 @@ fn refresh_output(out: &mut OutputData, _api_out: &api::Output) { // Locked -> Spent fn mark_spent_output(out: &mut OutputData) { match out.status { - OutputStatus::Unspent | OutputStatus::Locked => out.status = OutputStatus::Spent, + OutputStatus::Unspent => out.status = OutputStatus::Spent, + OutputStatus::Locked => out.status = OutputStatus::Spent, _ => (), } } @@ -175,7 +174,7 @@ fn refresh_output_state(config: &WalletConfig, keychain: &Keychain) -> Result<() .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_utxos: HashMap = HashMap::new(); let query_string = query_params.join("&"); @@ -184,9 +183,9 @@ fn refresh_output_state(config: &WalletConfig, keychain: &Keychain) -> Result<() config.check_node_api_http_addr, query_string, ); - match api::client::get::>(url.as_str()) { + match api::client::get::>(url.as_str()) { Ok(outputs) => for out in outputs { - api_outputs.insert(out.commit, out); + api_utxos.insert(out.commit, out); }, Err(e) => { // if we got anything other than 200 back from server, don't attempt to refresh @@ -202,8 +201,8 @@ fn refresh_output_state(config: &WalletConfig, keychain: &Keychain) -> Result<() WalletData::with_wallet(&config.data_file_dir, |wallet_data| for commit in wallet_outputs.keys() { let id = wallet_outputs.get(&commit).unwrap(); if let Entry::Occupied(mut output) = wallet_data.outputs.entry(id.to_hex()) { - match api_outputs.get(&commit) { - Some(api_output) => refresh_output(&mut output.get_mut(), api_output), + match api_utxos.get(&commit) { + Some(api_output) => mark_unspent_output(&mut output.get_mut()), None => mark_spent_output(&mut output.get_mut()), }; } diff --git a/wallet/src/restore.rs b/wallet/src/restore.rs index 3c63820db2..a3fc7e42d0 100644 --- a/wallet/src/restore.rs +++ b/wallet/src/restore.rs @@ -35,18 +35,28 @@ pub fn get_chain_height(config: &WalletConfig) -> Result { } } -fn output_with_range_proof(config: &WalletConfig, commit_id: &str) -> Result { +fn output_with_range_proof( + config: &WalletConfig, + commit_id: &str, + height: u64, +) -> Result { let url = format!( - "{}/v1/chain/utxos/byids?id={}&include_rp", + "{}/v1/chain/utxos/byheight?start_height={}&end_height={}&id={}&include_rp", config.check_node_api_http_addr, + height, + height, commit_id, ); - match api::client::get::>(url.as_str()) { - Ok(outputs) => { - if let Some(output) = outputs.first() { - Ok(output.clone()) + match api::client::get::>(url.as_str()) { + Ok(block_outputs) => { + if let Some(block_output) = block_outputs.first() { + if let Some(output) = block_output.outputs.first() { + Ok(output.clone()) + } else { + Err(Error::Node(api::Error::NotFound)) + } } else { Err(Error::Node(api::Error::NotFound)) } @@ -65,14 +75,21 @@ fn retrieve_amount_and_coinbase_status( keychain: &Keychain, key_id: Identifier, commit_id: &str, + height: u64, ) -> Result<(u64, bool), Error> { - let output = output_with_range_proof(config, commit_id)?; + let output = output_with_range_proof(config, commit_id, height)?; + + let proof = { + let vec = util::from_hex(input)?; + RangeProof { proof: vec, len: vec.len() } + }; + let core_output = Output { features: match output.output_type { api::OutputType::Coinbase => COINBASE_OUTPUT, api::OutputType::Transaction => DEFAULT_OUTPUT, }, - proof: output.proof.expect("output with proof"), + proof: proof, switch_commit_hash: output.switch_commit_hash, commit: output.commit, }; @@ -166,6 +183,7 @@ fn find_utxos_with_key( keychain, key_id.clone(), &output.commit, + block_outputs.header.height, ); if let Ok((amount, is_coinbase)) = res { From 1a8dfde670c6787b00b5e8411fee609cae91c74f Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Mon, 8 Jan 2018 13:33:00 -0500 Subject: [PATCH 30/36] basic wallet operations appear to work restore is not working fully refresh refreshes heights correctly (at least appears to) --- api/src/handlers.rs | 14 ++++- api/src/types.rs | 28 ++++++++- core/src/core/transaction.rs | 5 +- core/src/ser.rs | 4 ++ grin/src/adapters.rs | 3 +- wallet/src/checker.rs | 2 +- wallet/src/restore.rs | 116 ++++++++++++++++++----------------- wallet/src/types.rs | 8 +++ 8 files changed, 116 insertions(+), 64 deletions(-) diff --git a/api/src/handlers.rs b/api/src/handlers.rs index 3a855c6e39..5181f6a8be 100644 --- a/api/src/handlers.rs +++ b/api/src/handlers.rs @@ -61,7 +61,6 @@ struct UtxoHandler { impl UtxoHandler { fn get_utxo(&self, id: &str) -> Result { - debug!(LOGGER, "getting utxo: {}", id); let c = util::from_hex(String::from(id)) .map_err(|_| Error::Argument(format!("Not a valid commitment: {}", id)))?; let commit = Commitment::from_vec(c); @@ -84,6 +83,9 @@ impl UtxoHandler { } } } + + debug!(LOGGER, "utxos_by_ids: {:?}", commitments); + let mut utxos: Vec = vec![]; for x in commitments { if let Ok(utxo) = self.get_utxo(x) { @@ -152,6 +154,16 @@ impl UtxoHandler { include_rp = true; } } + + debug!( + LOGGER, + "outputs_block_batch: {:?}, {:?}, {:?}, {:?}", + start_height, + end_height, + commitments, + include_rp, + ); + let mut return_vec = vec![]; for i in start_height..end_height + 1 { let res = self.outputs_at_height(i, commitments.clone(), include_rp); diff --git a/api/src/types.rs b/api/src/types.rs index cbf5f74aef..914637f086 100644 --- a/api/src/types.rs +++ b/api/src/types.rs @@ -14,11 +14,12 @@ use std::sync::Arc; -use core::core; +use core::{core, ser}; use core::core::hash::Hashed; use chain; use util; use util::secp::pedersen; +use util::secp::constants::MAX_PROOF_SIZE; /// The state of the current fork tip #[derive(Serialize, Deserialize, Debug, Clone)] @@ -196,8 +197,29 @@ impl OutputPrintable { } } - pub fn switch_commit_hash(&self) -> core::SwitchCommitHash { - core::SwitchCommitHash::from_hex(&self.switch_commit_hash).unwrap() + // Convert the hex string back into a switch_commit_hash instance + pub fn switch_commit_hash(&self) -> Result { + core::SwitchCommitHash::from_hex(&self.switch_commit_hash) + } + + pub fn commit(&self) -> Result { + let vec = util::from_hex(self.commit.clone()) + .map_err(|_| ser::Error::HexError(format!("output commit hex_error")))?; + Ok(pedersen::Commitment::from_vec(vec)) + } + + pub fn range_proof(&self) -> Result { + if let Some(ref proof) = self.proof { + let vec = util::from_hex(proof.clone()) + .map_err(|_| ser::Error::HexError(format!("output range_proof hex_error")))?; + let mut bytes = [0; MAX_PROOF_SIZE]; + for i in 0..vec.len() { + bytes[i] = vec[i]; + } + Ok(pedersen::RangeProof { proof: bytes, plen: vec.len() }) + } else { + Err(ser::Error::HexError(format!("output range_proof missing"))) + } } } diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 75a0a33a63..9481a08a2e 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -593,8 +593,9 @@ impl SwitchCommitHash { util::to_hex(self.0.to_vec()) } - pub fn from_hex(hex: &str) -> Result { - let bytes = util::from_hex(hex.to_string()).unwrap(); + pub fn from_hex(hex: &str) -> Result { + let bytes = util::from_hex(hex.to_string()) + .map_err(|_| ser::Error::HexError(format!("switch_commit_hash from_hex error")))?; Ok(SwitchCommitHash::from_bytes(&bytes)) } diff --git a/core/src/ser.rs b/core/src/ser.rs index d3b014c6a1..b96ff31dad 100644 --- a/core/src/ser.rs +++ b/core/src/ser.rs @@ -49,6 +49,8 @@ pub enum Error { TooLargeReadErr, /// Consensus rule failure (currently sort order) ConsensusError(consensus::Error), + /// Error from from_hex deserialization + HexError(String), } impl From for Error { @@ -74,6 +76,7 @@ impl fmt::Display for Error { Error::CorruptedData => f.write_str("corrupted data"), Error::TooLargeReadErr => f.write_str("too large read"), Error::ConsensusError(ref e) => write!(f, "consensus error {:?}", e), + Error::HexError(ref e) => write!(f, "hex error {:?}", e), } } } @@ -96,6 +99,7 @@ impl error::Error for Error { Error::CorruptedData => "corrupted data", Error::TooLargeReadErr => "too large read", Error::ConsensusError(_) => "consensus error (sort order)", + Error::HexError(_) => "hex error", } } } diff --git a/grin/src/adapters.rs b/grin/src/adapters.rs index e2a3ae042f..5073d9b5c9 100644 --- a/grin/src/adapters.rs +++ b/grin/src/adapters.rs @@ -21,6 +21,7 @@ use core::core::{self, Output}; use core::core::block::BlockHeader; use core::core::hash::{Hash, Hashed}; use core::core::target::Difficulty; +use core::core::transaction::SwitchCommitHash; use p2p; use pool; use util::secp::pedersen::Commitment; @@ -335,7 +336,7 @@ impl pool::BlockChain for PoolToChainAdapter { fn get_unspent( &self, output_ref: &Commitment, - ) -> Result { + ) -> Result { self.chain .borrow() .get_unspent(output_ref) diff --git a/wallet/src/checker.rs b/wallet/src/checker.rs index 099a98b042..92bd89f108 100644 --- a/wallet/src/checker.rs +++ b/wallet/src/checker.rs @@ -100,7 +100,7 @@ fn refresh_missing_heights(config: &WalletConfig, keychain: &Keychain) -> Result let url = format!( - "{}/v1/chain/utxos/atheight?{}", + "{}/v1/chain/utxos/byheight?{}", config.check_node_api_http_addr, query_params.join("&"), ); diff --git a/wallet/src/restore.rs b/wallet/src/restore.rs index a3fc7e42d0..555859fcda 100644 --- a/wallet/src/restore.rs +++ b/wallet/src/restore.rs @@ -21,6 +21,10 @@ use core::core::{Output, SwitchCommitHash, SwitchCommitHashKey}; use core::core::transaction::{COINBASE_OUTPUT, DEFAULT_OUTPUT}; use types::{WalletConfig, WalletData, OutputData, OutputStatus, Error}; use byteorder::{BigEndian, ByteOrder}; +use util; +use util::secp::constants::MAX_PROOF_SIZE; +use util::secp::pedersen::{Commitment, RangeProof}; + pub fn get_chain_height(config: &WalletConfig) -> Result { let url = format!("{}/v1/chain", config.check_node_api_http_addr); @@ -79,19 +83,14 @@ fn retrieve_amount_and_coinbase_status( ) -> Result<(u64, bool), Error> { let output = output_with_range_proof(config, commit_id, height)?; - let proof = { - let vec = util::from_hex(input)?; - RangeProof { proof: vec, len: vec.len() } - }; - let core_output = Output { features: match output.output_type { api::OutputType::Coinbase => COINBASE_OUTPUT, api::OutputType::Transaction => DEFAULT_OUTPUT, }, - proof: proof, - switch_commit_hash: output.switch_commit_hash, - commit: output.commit, + proof: output.range_proof()?, + switch_commit_hash: output.switch_commit_hash()?, + commit: output.commit()?, }; if let Some(amount) = core_output.recover_value(keychain, &key_id) { let is_coinbase = match output.output_type { @@ -167,61 +166,66 @@ fn find_utxos_with_key( ) } }; - if expected_hash == output.switch_commit_hash() { - info!( - LOGGER, - "Output found: {:?}, key_index: {:?}", - output, - i, - ); - // add it to result set here - let commit_id = from_hex(output.commit.clone()).unwrap(); - let key_id = keychain.derive_key_id(i as u32).unwrap(); - let res = retrieve_amount_and_coinbase_status( - config, - keychain, - key_id.clone(), - &output.commit, - block_outputs.header.height, - ); + // TODO - can we reduce the nesting here of the let/if/if ? + let switch_commit_hash = output.switch_commit_hash(); + if let Ok(x) = output.switch_commit_hash() { + if x == expected_hash { + info!( + LOGGER, + "Output found: {:?}, key_index: {:?}", + output, + i, + ); - if let Ok((amount, is_coinbase)) = res { - info!(LOGGER, "Amount: {}", amount); + // add it to result set here + let commit_id = from_hex(output.commit.clone()).unwrap(); + let key_id = keychain.derive_key_id(i as u32).unwrap(); + let res = retrieve_amount_and_coinbase_status( + config, + keychain, + key_id.clone(), + &output.commit, + block_outputs.header.height, + ); - let commit = keychain - .commit_with_key_index(BigEndian::read_u64(&commit_id), i as u32) - .expect("commit with key index"); + if let Ok((amount, is_coinbase)) = res { + info!(LOGGER, "Amount: {}", amount); - let height = block_outputs.header.height; - let lock_height = if is_coinbase { - height + global::coinbase_maturity() - } else { - 0 - }; + let commit = keychain + .commit_with_key_index(BigEndian::read_u64(&commit_id), i as u32) + .expect("commit with key index"); - wallet_outputs.push(( - commit, - key_id.clone(), - i as u32, - amount, - height, - lock_height, - is_coinbase, - )); + let height = block_outputs.header.height; + let lock_height = if is_coinbase { + height + global::coinbase_maturity() + } else { + 0 + }; + + wallet_outputs.push(( + commit, + key_id.clone(), + i as u32, + amount, + height, + lock_height, + is_coinbase, + )); - // probably don't have to look for indexes greater than this now - *key_iterations = i + *padding; - if *key_iterations > switch_commit_cache.len() { - *key_iterations = switch_commit_cache.len(); + // probably don't have to look for indexes greater than this now + *key_iterations = i + *padding; + if *key_iterations > switch_commit_cache.len() { + *key_iterations = switch_commit_cache.len(); + } + info!(LOGGER, "Setting max key index to: {}", *key_iterations); + break; + } else { + info!( + LOGGER, + "Unable to retrieve the amount (needs investigating)", + ); } - info!(LOGGER, "Setting max key index to: {}", *key_iterations); - break; - } else { - info!( - LOGGER, - "Unable to retrieve the amount (needs investigating)", - ); } } } diff --git a/wallet/src/types.rs b/wallet/src/types.rs index 0e9faa0863..43f186c055 100644 --- a/wallet/src/types.rs +++ b/wallet/src/types.rs @@ -121,12 +121,20 @@ impl From for Error { } } +// TODO - rethink this, would be nice not to have to worry about +// low level hex conversion errors like this impl From for Error { fn from(_: num::ParseIntError) -> Error { Error::Format("Invalid hex".to_string()) } } +impl From for Error { + fn from(e: ser::Error) -> Error { + Error::Format(e.to_string()) + } +} + impl From for Error { fn from(e: api::Error) -> Error { Error::Node(e) From eb62f2fae01752c91b619e42dbefa694e04db9b7 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Mon, 8 Jan 2018 16:40:14 -0500 Subject: [PATCH 31/36] wallet refresh and wallet restore appear to be working now --- api/src/handlers.rs | 3 ++- api/src/types.rs | 2 +- grin/src/adapters.rs | 2 +- wallet/src/checker.rs | 2 +- wallet/src/restore.rs | 15 ++++++--------- 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/api/src/handlers.rs b/api/src/handlers.rs index 5181f6a8be..b1f593a8a8 100644 --- a/api/src/handlers.rs +++ b/api/src/handlers.rs @@ -157,7 +157,7 @@ impl UtxoHandler { debug!( LOGGER, - "outputs_block_batch: {:?}, {:?}, {:?}, {:?}", + "outputs_block_batch: {}-{}, {:?}, {:?}", start_height, end_height, commitments, @@ -171,6 +171,7 @@ impl UtxoHandler { return_vec.push(res); } } + return_vec } } diff --git a/api/src/types.rs b/api/src/types.rs index 914637f086..aed037556c 100644 --- a/api/src/types.rs +++ b/api/src/types.rs @@ -182,7 +182,7 @@ impl OutputPrintable { }; let proof = if include_proof { - Some(util::to_hex(output.proof.proof.to_vec())) + Some(util::to_hex(output.proof.bytes().to_vec())) } else { None }; diff --git a/grin/src/adapters.rs b/grin/src/adapters.rs index 5073d9b5c9..9c17647dfd 100644 --- a/grin/src/adapters.rs +++ b/grin/src/adapters.rs @@ -17,7 +17,7 @@ use std::sync::{Arc, RwLock}; use std::sync::atomic::{AtomicBool, Ordering}; use chain::{self, ChainAdapter}; -use core::core::{self, Output}; +use core::core; use core::core::block::BlockHeader; use core::core::hash::{Hash, Hashed}; use core::core::target::Difficulty; diff --git a/wallet/src/checker.rs b/wallet/src/checker.rs index 92bd89f108..5dcea09021 100644 --- a/wallet/src/checker.rs +++ b/wallet/src/checker.rs @@ -202,7 +202,7 @@ fn refresh_output_state(config: &WalletConfig, keychain: &Keychain) -> Result<() let id = wallet_outputs.get(&commit).unwrap(); if let Entry::Occupied(mut output) = wallet_data.outputs.entry(id.to_hex()) { match api_utxos.get(&commit) { - Some(api_output) => mark_unspent_output(&mut output.get_mut()), + Some(_) => mark_unspent_output(&mut output.get_mut()), None => mark_spent_output(&mut output.get_mut()), }; } diff --git a/wallet/src/restore.rs b/wallet/src/restore.rs index 555859fcda..012625f28d 100644 --- a/wallet/src/restore.rs +++ b/wallet/src/restore.rs @@ -21,9 +21,6 @@ use core::core::{Output, SwitchCommitHash, SwitchCommitHashKey}; use core::core::transaction::{COINBASE_OUTPUT, DEFAULT_OUTPUT}; use types::{WalletConfig, WalletData, OutputData, OutputStatus, Error}; use byteorder::{BigEndian, ByteOrder}; -use util; -use util::secp::constants::MAX_PROOF_SIZE; -use util::secp::pedersen::{Commitment, RangeProof}; pub fn get_chain_height(config: &WalletConfig) -> Result { @@ -92,6 +89,7 @@ fn retrieve_amount_and_coinbase_status( switch_commit_hash: output.switch_commit_hash()?, commit: output.commit()?, }; + if let Some(amount) = core_output.recover_value(keychain, &key_id) { let is_coinbase = match output.output_type { api::OutputType::Coinbase => true, @@ -108,7 +106,6 @@ pub fn utxos_batch_block( start_height: u64, end_height: u64, ) -> Result, Error> { - // build the necessary query param - let query_param = format!("start_height={}&end_height={}", start_height, end_height); let url = @@ -167,10 +164,8 @@ fn find_utxos_with_key( } }; - // TODO - can we reduce the nesting here of the let/if/if ? - let switch_commit_hash = output.switch_commit_hash(); - if let Ok(x) = output.switch_commit_hash() { - if x == expected_hash { + if let Ok(x) = output.switch_commit_hash() { + if x == expected_hash { info!( LOGGER, "Output found: {:?}, key_index: {:?}", @@ -181,6 +176,7 @@ fn find_utxos_with_key( // add it to result set here let commit_id = from_hex(output.commit.clone()).unwrap(); let key_id = keychain.derive_key_id(i as u32).unwrap(); + let res = retrieve_amount_and_coinbase_status( config, keychain, @@ -223,7 +219,8 @@ fn find_utxos_with_key( } else { info!( LOGGER, - "Unable to retrieve the amount (needs investigating)", + "Unable to retrieve the amount (needs investigating) {:?}", + res, ); } } From 8c531b23a9a8070d1facb62d97d486585524b565 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Mon, 8 Jan 2018 17:27:36 -0500 Subject: [PATCH 32/36] fix core tests --- core/src/core/block.rs | 8 ++++---- core/src/core/build.rs | 10 +++++++--- core/src/core/mod.rs | 16 ++++++++-------- core/src/core/transaction.rs | 4 +++- 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/core/src/core/block.rs b/core/src/core/block.rs index ce11014132..ba576a7ed6 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -664,7 +664,7 @@ mod test { key_id2: Identifier, ) -> Transaction { build::transaction( - vec![input(v, 0, key_id1), output(3, key_id2), with_fee(2)], + vec![input(v, key_id1), output(3, key_id2), with_fee(2)], &keychain, ).map(|(tx, _)| tx) .unwrap() @@ -688,7 +688,7 @@ mod test { } let now = Instant::now(); - parts.append(&mut vec![input(500000, 0, pks.pop().unwrap()), with_fee(2)]); + parts.append(&mut vec![input(500000, pks.pop().unwrap()), with_fee(2)]); let mut tx = build::transaction(parts, &keychain) .map(|(tx, _)| tx) .unwrap(); @@ -708,7 +708,7 @@ mod test { let mut btx1 = tx2i1o(); let (mut btx2, _) = build::transaction( - vec![input(7, 0, key_id1), output(5, key_id2.clone()), with_fee(2)], + vec![input(7, key_id1), output(5, key_id2.clone()), with_fee(2)], &keychain, ).unwrap(); @@ -736,7 +736,7 @@ mod test { let mut btx1 = tx2i1o(); let (mut btx2, _) = build::transaction( - vec![input(7, 0, key_id1), output(5, key_id2.clone()), with_fee(2)], + vec![input(7, key_id1), output(5, key_id2.clone()), with_fee(2)], &keychain, ).unwrap(); diff --git a/core/src/core/build.rs b/core/src/core/build.rs index dd14b5b1e0..3f6b5c3884 100644 --- a/core/src/core/build.rs +++ b/core/src/core/build.rs @@ -61,6 +61,8 @@ fn input_with_lock_height( }) } +/// Adds an input with the provided value and blinding key to the transaction +/// being built. pub fn input( value: u64, key_id: Identifier, @@ -69,6 +71,8 @@ pub fn input( input_with_lock_height(value, 0, key_id) } +/// Adds a coinbase input with the provided value and blinding key to the transaction +/// being built, with an additional lock_height specified. pub fn coinbase_input( value: u64, lock_height: u64, @@ -198,8 +202,8 @@ mod test { let (tx, _) = transaction( vec![ - input(10, 0, key_id1), - input(11, 0, key_id2), + input(10, key_id1), + input(11, key_id2), output(20, key_id3), with_fee(1), ], @@ -216,7 +220,7 @@ mod test { let key_id2 = keychain.derive_key_id(2).unwrap(); let (tx, _) = transaction( - vec![input(6, 0, key_id1), output(2, key_id2), with_fee(4)], + vec![input(6, key_id1), output(2, key_id2), with_fee(4)], &keychain, ).unwrap(); diff --git a/core/src/core/mod.rs b/core/src/core/mod.rs index a60e13d26f..7aa9bdd237 100644 --- a/core/src/core/mod.rs +++ b/core/src/core/mod.rs @@ -247,7 +247,7 @@ mod test { // blinding should fail as signing with a zero r*G shouldn't work build::transaction( vec![ - input(10, 0, key_id1.clone()), + input(10, key_id1.clone()), output(9, key_id1.clone()), with_fee(1), ], @@ -302,7 +302,7 @@ mod test { let (tx, _) = build::transaction( vec![ - input(75, 0, key_id1), + input(75, key_id1), output(42, key_id2), output(32, key_id3), with_fee(1), @@ -357,7 +357,7 @@ 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, 0, key_id1), input(3, 0, key_id2)); + 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. @@ -433,7 +433,7 @@ mod test { // and that the resulting block is valid let tx1 = build::transaction( vec![ - input(5, 0, key_id1.clone()), + input(5, key_id1.clone()), output(3, key_id2.clone()), with_fee(2), with_lock_height(1), @@ -453,7 +453,7 @@ mod test { // now try adding a timelocked tx where lock height is greater than current block height let tx1 = build::transaction( vec![ - input(5, 0, key_id1.clone()), + input(5, key_id1.clone()), output(3, key_id2.clone()), with_fee(2), with_lock_height(2), @@ -497,8 +497,8 @@ mod test { build::transaction( vec![ - input(10, 0, key_id1), - input(11, 0, key_id2), + input(10, key_id1), + input(11, key_id2), output(19, key_id3), with_fee(2), ], @@ -514,7 +514,7 @@ mod test { let key_id2 = keychain.derive_key_id(2).unwrap(); build::transaction( - vec![input(5, 0, key_id1), output(3, key_id2), with_fee(2)], + vec![input(5, key_id1), output(3, key_id2), with_fee(2)], &keychain, ).map(|(tx, _)| tx) .unwrap() diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index f7f16a79d0..79d5c4b0cd 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -569,7 +569,7 @@ impl ::std::fmt::Debug for SwitchCommitHash { } impl SwitchCommitHash { - /// Builds a switch commitment hash from a switch commit using blake2 + /// Builds a switch commit hash from a switch commit using blake2 pub fn from_switch_commit(switch_commit: Commitment, key: SwitchCommitHashKey) -> SwitchCommitHash { let switch_commit_hash = blake2b(SWITCH_COMMIT_HASH_SIZE, &key.0, &switch_commit.0); let switch_commit_hash = switch_commit_hash.as_bytes(); @@ -580,6 +580,7 @@ impl SwitchCommitHash { SwitchCommitHash(h) } + /// Reconstructs a switch commit hash from an array of bytes. pub fn from_bytes(bytes: &[u8]) -> SwitchCommitHash { let mut hash = [0; SWITCH_COMMIT_HASH_SIZE]; for i in 0..min(SWITCH_COMMIT_HASH_SIZE, bytes.len()) { @@ -593,6 +594,7 @@ impl SwitchCommitHash { util::to_hex(self.0.to_vec()) } + /// Reconstrcuts a switch commit hash from a hex string. pub fn from_hex(hex: &str) -> Result { let bytes = util::from_hex(hex.to_string()) .map_err(|_| ser::Error::HexError(format!("switch_commit_hash from_hex error")))?; From fea627d4df21a985544c86ffa2e0a4921865495e Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Mon, 8 Jan 2018 21:34:36 -0500 Subject: [PATCH 33/36] fix some mine_simple_chain tests --- chain/tests/mine_simple_chain.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/chain/tests/mine_simple_chain.rs b/chain/tests/mine_simple_chain.rs index 34db7e598b..2a6e739add 100644 --- a/chain/tests/mine_simple_chain.rs +++ b/chain/tests/mine_simple_chain.rs @@ -284,9 +284,8 @@ fn spend_in_fork() { let head = chain.head_header().unwrap(); assert_eq!(head.height, 6); assert_eq!(head.hash(), prev_main.hash()); - assert!(chain.is_unspent(&tx2.outputs[0].commitment()).unwrap()); - let res = chain.is_unspent(&tx1.outputs[0].commitment()); - assert!(!res.unwrap()); + assert!(chain.get_unspent(&tx2.outputs[0].commitment()).is_ok()); + assert!(chain.get_unspent(&tx1.outputs[0].commitment()).is_err()); // make the fork win let fork_next = prepare_fork_block(&kc, &prev_fork, &chain, 10); @@ -297,8 +296,8 @@ fn spend_in_fork() { let head = chain.head_header().unwrap(); assert_eq!(head.height, 7); assert_eq!(head.hash(), prev_fork.hash()); - assert!(chain.is_unspent(&tx2.outputs[0].commitment()).unwrap()); - assert!(!chain.is_unspent(&tx1.outputs[0].commitment()).unwrap()); + assert!(chain.get_unspent(&tx2.outputs[0].commitment()).is_ok()); + assert!(chain.get_unspent(&tx1.outputs[0].commitment()).is_err()); } fn prepare_block(kc: &Keychain, prev: &BlockHeader, chain: &Chain, diff: u64) -> Block { From 3b5387e0c7e297ff9ce649af8a4c00a845efb7e1 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Tue, 9 Jan 2018 07:25:26 -0500 Subject: [PATCH 34/36] fixup chain tests --- chain/tests/mine_simple_chain.rs | 5 ++++- chain/tests/test_coinbase_maturity.rs | 27 ++++++++++----------------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/chain/tests/mine_simple_chain.rs b/chain/tests/mine_simple_chain.rs index 2a6e739add..626ebf68d2 100644 --- a/chain/tests/mine_simple_chain.rs +++ b/chain/tests/mine_simple_chain.rs @@ -245,9 +245,12 @@ fn spend_in_fork() { chain.process_block(b, chain::SKIP_POW).unwrap(); } + let lock_height = 1 + global::coinbase_maturity(); + assert_eq!(lock_height, 4); + let (tx1, _) = build::transaction( vec![ - build::input(consensus::REWARD, kc.derive_key_id(2).unwrap()), + build::coinbase_input(consensus::REWARD, lock_height, kc.derive_key_id(2).unwrap()), build::output(consensus::REWARD - 20000, kc.derive_key_id(30).unwrap()), build::with_fee(20000), ], diff --git a/chain/tests/test_coinbase_maturity.rs b/chain/tests/test_coinbase_maturity.rs index b6bbc83cf8..b3fd20b077 100644 --- a/chain/tests/test_coinbase_maturity.rs +++ b/chain/tests/test_coinbase_maturity.rs @@ -99,16 +99,16 @@ fn test_coinbase_maturity() { let prev = chain.head_header().unwrap(); - let height = prev.height; - assert_eq!(height, 1); - let amount = consensus::REWARD; + let lock_height = 1 + global::coinbase_maturity(); + assert_eq!(lock_height, 4); + // here we build a tx that attempts to spend the earlier coinbase output // this is not a valid tx as the coinbase output cannot be spent yet let (coinbase_txn, _) = build::transaction( vec![ - build::coinbase_input(amount, height, key_id1.clone()), + build::coinbase_input(amount, lock_height, key_id1.clone()), build::output(amount - 2, key_id2.clone()), build::with_fee(2), ], @@ -125,7 +125,11 @@ fn test_coinbase_maturity() { let difficulty = consensus::next_difficulty(chain.difficulty_iter()).unwrap(); block.header.difficulty = difficulty.clone(); - chain.set_sumtree_roots(&mut block, false).unwrap(); + + match chain.set_sumtree_roots(&mut block, false) { + Err(Error::ImmatureCoinbase) => (), + _ => panic!("expected ImmatureCoinbase error here"), + } pow::pow_size( &mut cuckoo_miner, @@ -134,14 +138,6 @@ fn test_coinbase_maturity() { global::sizeshift() as u32, ).unwrap(); - // confirm the block fails validation due to the invalid tx attempting to spend - // the immature coinbase - let result = chain.process_block(block, chain::SKIP_POW); - match result { - Err(Error::ImmatureCoinbase) => (), - _ => panic!("expected ImmatureCoinbase error here"), - }; - // mine enough blocks to increase the height sufficiently for // coinbase to reach maturity and be spendable in the next block for _ in 0..3 { @@ -169,12 +165,9 @@ fn test_coinbase_maturity() { let prev = chain.head_header().unwrap(); - let height = prev.height; - assert_eq!(height, 4); - let (coinbase_txn, _) = build::transaction( vec![ - build::coinbase_input(amount, height, key_id1.clone()), + build::coinbase_input(amount, lock_height, key_id1.clone()), build::output(amount - 2, key_id2.clone()), build::with_fee(2), ], From 289ed5469d755a8c8fd0fde7e05bf4358b4d1286 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Tue, 9 Jan 2018 11:39:13 -0500 Subject: [PATCH 35/36] rework so pool tests pass --- core/src/core/transaction.rs | 2 + pool/Cargo.toml | 1 + pool/src/blockchain.rs | 72 +++++++---------------- pool/src/lib.rs | 2 + pool/src/pool.rs | 108 ++++++++++++++++++++++------------- 5 files changed, 93 insertions(+), 92 deletions(-) diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 79d5c4b0cd..e6a4f3782f 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -480,6 +480,8 @@ impl Input { output_switch_commit_hash: &SwitchCommitHash, height: u64, ) -> Result<(), Error> { + debug!(LOGGER, "verify_lock_height: {}, {}", self.lock_height, height); + let switch_commit_hash = SwitchCommitHash::from_switch_commit( self.switch_commit, SwitchCommitHashKey::from_lock_height(self.lock_height), diff --git a/pool/Cargo.toml b/pool/Cargo.toml index 1b0d23470d..7a66c5ae66 100644 --- a/pool/Cargo.toml +++ b/pool/Cargo.toml @@ -12,5 +12,6 @@ grin_p2p = { path = "../p2p" } grin_util = { path = "../util" } serde = "~1.0.8" serde_derive = "~1.0.8" +slog = { version = "^2.0.12", features = ["max_level_trace", "release_max_level_trace"] } time = "^0.1" rand = "0.3" diff --git a/pool/src/blockchain.rs b/pool/src/blockchain.rs index 65bb3ece6d..4460858533 100644 --- a/pool/src/blockchain.rs +++ b/pool/src/blockchain.rs @@ -20,22 +20,12 @@ use std::sync::RwLock; use types::{BlockChain, PoolError}; -#[derive(Debug)] -pub struct DummyBlockHeaderIndex { - block_headers: HashMap, -} - -impl DummyBlockHeaderIndex { - pub fn insert(&mut self, commit: Commitment, block_header: block::BlockHeader) { - self.block_headers.insert(commit, block_header); - } -} - /// A DummyUtxoSet for mocking up the chain pub struct DummyUtxoSet { outputs: HashMap, } + #[allow(dead_code)] impl DummyUtxoSet { pub fn empty() -> DummyUtxoSet { @@ -43,21 +33,25 @@ impl DummyUtxoSet { outputs: HashMap::new(), } } + pub fn root(&self) -> hash::Hash { hash::ZERO_HASH } + pub fn apply(&self, b: &block::Block) -> DummyUtxoSet { - let mut new_hashmap = self.outputs.clone(); + let mut new_outputs = self.outputs.clone(); + for input in &b.inputs { - new_hashmap.remove(&input.commitment()); + new_outputs.remove(&input.commitment()); } for output in &b.outputs { - new_hashmap.insert(output.commitment(), output.clone()); + new_outputs.insert(output.commitment(), output.clone()); } DummyUtxoSet { - outputs: new_hashmap, + outputs: new_outputs, } } + pub fn with_block(&mut self, b: &block::Block) { for input in &b.inputs { self.outputs.remove(&input.commitment()); @@ -66,20 +60,15 @@ impl DummyUtxoSet { self.outputs.insert(output.commitment(), output.clone()); } } + pub fn rewind(&self, _: &block::Block) -> DummyUtxoSet { DummyUtxoSet { outputs: HashMap::new(), } } - pub fn get_unspent( - &self, - output_ref: &Commitment, - ) -> Option { - if let Some(x) = self.outputs.get(output_ref) { - Some(x.switch_commit_hash()) - } else { - None - } + + pub fn get_output(&self, output_ref: &Commitment) -> Option<&transaction::Output> { + self.outputs.get(output_ref) } fn clone(&self) -> DummyUtxoSet { @@ -89,14 +78,12 @@ impl DummyUtxoSet { } // only for testing: add an output to the map - pub fn add_output(&mut self, output: transaction::Output) { - self.outputs.insert(output.commitment(), output); - } - // like above, but doesn't modify in-place so no mut ref needed pub fn with_output(&self, output: transaction::Output) -> DummyUtxoSet { - let mut new_map = self.outputs.clone(); - new_map.insert(output.commitment(), output); - DummyUtxoSet { outputs: new_map } + let mut new_outputs = self.outputs.clone(); + new_outputs.insert(output.commitment(), output); + DummyUtxoSet { + outputs: new_outputs, + } } } @@ -105,7 +92,6 @@ impl DummyUtxoSet { #[allow(dead_code)] pub struct DummyChainImpl { utxo: RwLock, - block_headers: RwLock, head_header: RwLock>, } @@ -116,9 +102,6 @@ impl DummyChainImpl { utxo: RwLock::new(DummyUtxoSet { outputs: HashMap::new(), }), - block_headers: RwLock::new(DummyBlockHeaderIndex { - block_headers: HashMap::new(), - }), head_header: RwLock::new(vec![]), } } @@ -129,8 +112,8 @@ impl BlockChain for DummyChainImpl { &self, commitment: &Commitment, ) -> Result { - match self.utxo.read().unwrap().get_unspent(commitment) { - Some(x) => Ok(x), + match self.utxo.read().unwrap().get_output(commitment) { + Some(x) => Ok(x.switch_commit_hash), None => Err(PoolError::GenericPoolError), } } @@ -152,16 +135,6 @@ impl DummyChain for DummyChainImpl { fn apply_block(&self, b: &block::Block) { self.utxo.write().unwrap().with_block(b); } - fn store_header_by_output_commitment( - &self, - commitment: Commitment, - block_header: &block::BlockHeader, - ) { - self.block_headers - .write() - .unwrap() - .insert(commitment, block_header.clone()); - } fn store_head_header(&self, block_header: &block::BlockHeader) { let mut h = self.head_header.write().unwrap(); h.clear(); @@ -172,10 +145,5 @@ impl DummyChain for DummyChainImpl { pub trait DummyChain: BlockChain { fn update_utxo_set(&mut self, new_utxo: DummyUtxoSet); fn apply_block(&self, b: &block::Block); - fn store_header_by_output_commitment( - &self, - commitment: Commitment, - block_header: &block::BlockHeader, - ); fn store_head_header(&self, block_header: &block::BlockHeader); } diff --git a/pool/src/lib.rs b/pool/src/lib.rs index 2b2c51d1ca..56e03da098 100644 --- a/pool/src/lib.rs +++ b/pool/src/lib.rs @@ -34,6 +34,8 @@ extern crate rand; extern crate serde; #[macro_use] extern crate serde_derive; +#[macro_use] +extern crate slog; extern crate time; pub use pool::TransactionPool; diff --git a/pool/src/pool.rs b/pool/src/pool.rs index 919e65c9c8..e67f83dd1d 100644 --- a/pool/src/pool.rs +++ b/pool/src/pool.rs @@ -22,6 +22,7 @@ use core::core::block; use core::core::hash; use util::secp::pedersen::Commitment; +use util::LOGGER; use std::sync::Arc; use std::collections::{HashMap, HashSet}; @@ -139,6 +140,8 @@ where _: TxSource, tx: transaction::Transaction, ) -> Result<(), PoolError> { + debug!(LOGGER, "add_to_memory_pool"); + // Do we have the capacity to accept this transaction? if let Err(e) = self.is_acceptable(&tx) { return Err(e); @@ -599,6 +602,7 @@ where mod tests { use super::*; use core::core::build; + use core::global; use blockchain::{DummyChain, DummyChainImpl, DummyUtxoSet}; use util::secp; use keychain::Keychain; @@ -607,7 +611,6 @@ mod tests { use core::global::ChainTypes; use core::core::{SwitchCommitHash, SwitchCommitHashKey}; - macro_rules! expect_output_parent { ($pool:expr, $expected:pat, $( $output:expr ),+ ) => { $( @@ -672,14 +675,13 @@ mod tests { } } - // Now take the read lock and use a few exposed methods to check - // consistency + // Now take the read lock and use a few exposed methods to check consistency { let read_pool = pool.read().unwrap(); assert_eq!(read_pool.total_size(), 2); - expect_output_parent!(read_pool, Parent::PoolTransaction{_}, 12); - expect_output_parent!(read_pool, Parent::AlreadySpent{_}, 11, 5); - expect_output_parent!(read_pool, Parent::BlockTransaction{_}, 8); + expect_output_parent!(read_pool, Parent::PoolTransaction{tx_ref: _}, 12); + expect_output_parent!(read_pool, Parent::AlreadySpent{other_tx: _}, 11, 5); + expect_output_parent!(read_pool, Parent::BlockTransaction{switch_commit_hash: _}, 8); expect_output_parent!(read_pool, Parent::Unknown, 20); } } @@ -794,7 +796,11 @@ mod tests { fn test_immature_coinbase() { global::set_mining_mode(ChainTypes::AutomatedTesting); let mut dummy_chain = DummyChainImpl::new(); - let coinbase_output = test_coinbase_output(15); + + let lock_height = 1 + global::coinbase_maturity(); + assert_eq!(lock_height, 4); + + let coinbase_output = test_coinbase_output(15, lock_height); dummy_chain.update_utxo_set(DummyUtxoSet::empty().with_output(coinbase_output)); let chain_ref = Arc::new(dummy_chain); @@ -807,8 +813,7 @@ mod tests { height: 1, ..block::BlockHeader::default() }; - chain_ref - .store_header_by_output_commitment(coinbase_output.commitment(), &coinbase_header); + chain_ref.store_head_header(&coinbase_header); let head_header = block::BlockHeader { height: 2, @@ -816,11 +821,12 @@ mod tests { }; chain_ref.store_head_header(&head_header); - let txn = test_transaction(vec![15], vec![10, 3]); + let txn = test_transaction_with_coinbase_input(15, 4, vec![10, 3]); let result = write_pool.add_to_memory_pool(test_source(), txn); match result { Err(PoolError::ImmatureCoinbase { - header: _, + height: _, + lock_height: _, output: out, }) => { assert_eq!(out, coinbase_output.commitment()); @@ -829,30 +835,12 @@ mod tests { }; let head_header = block::BlockHeader { - height: 3, + height: 4, ..block::BlockHeader::default() }; chain_ref.store_head_header(&head_header); - let txn = test_transaction(vec![15], vec![10, 3]); - let result = write_pool.add_to_memory_pool(test_source(), txn); - match result { - Err(PoolError::ImmatureCoinbase { - header: _, - output: out, - }) => { - assert_eq!(out, coinbase_output.commitment()); - } - _ => panic!("expected ImmatureCoinbase error here"), - }; - - let head_header = block::BlockHeader { - height: 5, - ..block::BlockHeader::default() - }; - chain_ref.store_head_header(&head_header); - - let txn = test_transaction(vec![15], vec![10, 3]); + let txn = test_transaction_with_coinbase_input(15, 4, vec![10, 3]); let result = write_pool.add_to_memory_pool(test_source(), txn); match result { Ok(_) => {} @@ -1077,7 +1065,7 @@ mod tests { assert_eq!(read_pool.total_size(), 4); // We should have available blockchain outputs - expect_output_parent!(read_pool, Parent::BlockTransaction{output: _}, 9, 1); + expect_output_parent!(read_pool, Parent::BlockTransaction{switch_commit_hash: _}, 9, 1); // We should have spent blockchain outputs expect_output_parent!(read_pool, Parent::AlreadySpent{other_tx: _}, 5, 6); @@ -1212,16 +1200,22 @@ mod tests { /// Every output is given a blinding key equal to its value, so that the /// entire commitment can be derived deterministically from just the value. /// - /// Fees are the remainder between input and output values, so the numbers - /// should make sense. + /// Fees are the remainder between input and output values, + /// so the numbers should make sense. fn test_transaction( input_values: Vec, output_values: Vec, ) -> transaction::Transaction { let keychain = keychain_for_tests(); - let fees: i64 = - input_values.iter().sum::() as i64 - output_values.iter().sum::() as i64; + let input_sum = input_values + .iter() + .sum::() as i64; + let output_sum = output_values + .iter() + .sum::() as i64; + + let fees: i64 = input_sum - output_sum; assert!(fees >= 0); let mut tx_elements = Vec::new(); @@ -1233,7 +1227,39 @@ mod tests { for output_value in output_values { let key_id = keychain.derive_key_id(output_value as u32).unwrap(); - tx_elements.push(build::output(output_value, 0, key_id)); + tx_elements.push(build::output(output_value, key_id)); + } + tx_elements.push(build::with_fee(fees as u64)); + + let (tx, _) = build::transaction(tx_elements, &keychain).unwrap(); + tx + } + + /// Very un-dry way of building a tx with a coinbase input (with lock_height). + /// TODO - rethink this. + fn test_transaction_with_coinbase_input( + input_value: u64, + input_lock_height: u64, + output_values: Vec, + ) -> transaction::Transaction { + let keychain = keychain_for_tests(); + + let output_sum = output_values + .iter() + .sum::() as i64; + + let fees: i64 = input_value as i64 - output_sum; + assert!(fees >= 0); + + let mut tx_elements = Vec::new(); + + // for input_value in input_values { + let key_id = keychain.derive_key_id(input_value as u32).unwrap(); + tx_elements.push(build::coinbase_input(input_value, input_lock_height, key_id)); + + for output_value in output_values { + let key_id = keychain.derive_key_id(output_value as u32).unwrap(); + tx_elements.push(build::output(output_value, key_id)); } tx_elements.push(build::with_fee(fees as u64)); @@ -1241,6 +1267,8 @@ mod tests { tx } + /// Very un-dry way of building a vanilla tx and adding a lock_height to it. + /// TODO - rethink this. fn timelocked_transaction( input_values: Vec, output_values: Vec, @@ -1261,7 +1289,7 @@ mod tests { for output_value in output_values { let key_id = keychain.derive_key_id(output_value as u32).unwrap(); - tx_elements.push(build::output(output_value, 0, key_id)); + tx_elements.push(build::output(output_value, key_id)); } tx_elements.push(build::with_fee(fees as u64)); @@ -1292,14 +1320,14 @@ mod tests { } /// Deterministically generate a coinbase output defined by our test scheme - fn test_coinbase_output(value: u64) -> transaction::Output { + fn test_coinbase_output(value: u64, lock_height: u64) -> transaction::Output { let keychain = keychain_for_tests(); let key_id = keychain.derive_key_id(value as u32).unwrap(); let commit = keychain.commit(value, &key_id).unwrap(); let switch_commit = keychain.switch_commit(&key_id).unwrap(); let switch_commit_hash = SwitchCommitHash::from_switch_commit( switch_commit, - SwitchCommitHashKey::zero(), + SwitchCommitHashKey::from_lock_height(lock_height), ); let msg = secp::pedersen::ProofMessage::empty(); let proof = keychain.range_proof(value, &key_id, commit, msg).unwrap(); From a988f95b1b69f3678767a9186c0f253a3d76bec0 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Tue, 9 Jan 2018 15:09:02 -0500 Subject: [PATCH 36/36] wallet restore now safely habndles duplicate commitments (reused wallet keys) for coinbase outputs where lock_height is _very_ important --- api/src/handlers.rs | 12 ++++++++++-- api/src/types.rs | 7 ++++++- core/src/core/transaction.rs | 2 -- pool/src/pool.rs | 3 --- wallet/src/receiver.rs | 2 +- 5 files changed, 17 insertions(+), 9 deletions(-) diff --git a/api/src/handlers.rs b/api/src/handlers.rs index b1f593a8a8..95053deae0 100644 --- a/api/src/handlers.rs +++ b/api/src/handlers.rs @@ -111,7 +111,12 @@ impl UtxoHandler { .iter() .filter(|c| { (commitments.is_empty() || commitments.contains(&c.commit)) && - self.chain.get_unspent(&c.commit).is_ok() + match self.chain.get_unspent(&c.commit) { + Ok(switch_commit_hash) => { + switch_commit_hash == c.switch_commit_hash + } + Err(_) => false, + } }) .map(|k| { OutputPrintable::from_output(k, self.chain.clone(), include_proof) @@ -469,7 +474,10 @@ where match res { Ok(()) => Ok(Response::with(status::Ok)), - Err(e) => Err(IronError::from(Error::Argument(format!("{:?}", e)))) + Err(e) => { + debug!(LOGGER, "error - {:?}", e); + Err(IronError::from(Error::Argument(format!("{:?}", e)))) + } } } } diff --git a/api/src/types.rs b/api/src/types.rs index 5259d597af..e4788806ab 100644 --- a/api/src/types.rs +++ b/api/src/types.rs @@ -176,8 +176,13 @@ impl OutputPrintable { OutputType::Transaction }; + // output for commitment is spent if - + // * not found in the pmmr (via the chain) + // * or commitment is a duplicate (found in pmmr but switch commit hash differs) let spent = match chain.get_unspent(&output.commit) { - Ok(_) => false, + Ok(switch_commit_hash) => { + switch_commit_hash != output.switch_commit_hash + }, Err(_) => true, }; diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index e6a4f3782f..79d5c4b0cd 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -480,8 +480,6 @@ impl Input { output_switch_commit_hash: &SwitchCommitHash, height: u64, ) -> Result<(), Error> { - debug!(LOGGER, "verify_lock_height: {}, {}", self.lock_height, height); - let switch_commit_hash = SwitchCommitHash::from_switch_commit( self.switch_commit, SwitchCommitHashKey::from_lock_height(self.lock_height), diff --git a/pool/src/pool.rs b/pool/src/pool.rs index e67f83dd1d..92a32fff9f 100644 --- a/pool/src/pool.rs +++ b/pool/src/pool.rs @@ -22,7 +22,6 @@ use core::core::block; use core::core::hash; use util::secp::pedersen::Commitment; -use util::LOGGER; use std::sync::Arc; use std::collections::{HashMap, HashSet}; @@ -140,8 +139,6 @@ where _: TxSource, tx: transaction::Transaction, ) -> Result<(), PoolError> { - debug!(LOGGER, "add_to_memory_pool"); - // Do we have the capacity to accept this transaction? if let Err(e) = self.is_acceptable(&tx) { return Err(e); diff --git a/wallet/src/receiver.rs b/wallet/src/receiver.rs index eeed364d8b..ecafd53efd 100644 --- a/wallet/src/receiver.rs +++ b/wallet/src/receiver.rs @@ -89,7 +89,7 @@ impl Handler for WalletReceiver { if let Ok(Some(partial_tx)) = struct_body { receive_json_tx(&self.config, &self.keychain, &partial_tx) .map_err(|e| { - error!(LOGGER, "Problematic partial tx, looks like this: {:?}", partial_tx); + error!(LOGGER, "Problematic partial tx, looks like this: {:?}, {:?}", partial_tx, e); api::Error::Internal( format!("Error processing partial transaction: {:?}", e), )})