diff --git a/core/src/core/block.rs b/core/src/core/block.rs index d325e7e7..096709df 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -438,7 +438,7 @@ impl Block { /// Hydrate a block from a compact block. /// Note: caller must validate the block themselves, we do not validate it /// here. - pub fn hydrate_from(cb: CompactBlock, txs: Vec) -> Block { + pub fn hydrate_from(cb: CompactBlock, txs: Vec) -> Result { trace!( LOGGER, "block: hydrate_from: {}, {} txs", @@ -555,7 +555,7 @@ impl Block { secp.commit_sum(excesses, vec![])? }; - Ok(Block { + Block { header: BlockHeader { height: prev.height + 1, timestamp: Utc::now(), @@ -566,7 +566,7 @@ impl Block { ..Default::default() }, body: agg_tx.into(), - }.cut_through()) + }.cut_through() } /// Get inputs @@ -619,46 +619,19 @@ impl Block { /// is a transaction spending a previous coinbase /// we do not want to cut-through (all coinbase must be preserved) /// - pub fn cut_through(self) -> Block { - let in_set = self - .body - .inputs - .iter() - .map(|inp| inp.commitment()) - .collect::>(); + pub fn cut_through(self) -> Result { + let mut inputs = self.inputs().clone(); + let mut outputs = self.outputs().clone(); + transaction::cut_through(&mut inputs, &mut outputs)?; - let out_set = self - .body - .outputs - .iter() - .filter(|out| !out.features.contains(OutputFeatures::COINBASE_OUTPUT)) - .map(|out| out.commitment()) - .collect::>(); - - let to_cut_through = in_set.intersection(&out_set).collect::>(); - - let new_inputs = self - .body - .inputs - .into_iter() - .filter(|inp| !to_cut_through.contains(&inp.commitment())) - .collect::>(); - - let new_outputs = self - .body - .outputs - .into_iter() - .filter(|out| !to_cut_through.contains(&out.commitment())) - .collect::>(); - - Block { + Ok(Block { header: BlockHeader { pow: self.header.pow, total_difficulty: self.header.total_difficulty, ..self.header }, - body: TransactionBody::new(new_inputs, new_outputs, self.body.kernels), - } + body: TransactionBody::new(inputs, outputs, self.body.kernels), + }) } /// Validates all the elements in a block that can be checked without diff --git a/core/src/core/committed.rs b/core/src/core/committed.rs index 22e2c3b8..3982c433 100644 --- a/core/src/core/committed.rs +++ b/core/src/core/committed.rs @@ -30,6 +30,7 @@ pub enum Error { Secp(secp::Error), /// Kernel sums do not equal output sums. KernelSumMismatch, + /// Committed overage (fee or reward) is invalid InvalidValue, } diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 29f5aa58..28f4bb39 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -689,6 +689,33 @@ impl Transaction { } } +/// Matches any output with a potential spending input, eliminating them +/// from the Vec. Provides a simple way to cut-through a block or aggregated +/// transaction. The elimination is stable with respect to the order of inputs +/// and outputs. +pub fn cut_through(inputs: &mut Vec, outputs: &mut Vec) -> Result<(), Error> { + // assemble output commitments set, checking they're all unique + let mut out_set = HashSet::new(); + let all_uniq = { outputs.iter().all(|o| out_set.insert(o.commitment())) }; + if !all_uniq { + return Err(Error::AggregationError); + } + + let in_set = inputs + .iter() + .map(|inp| inp.commitment()) + .collect::>(); + + let to_cut_through = in_set.intersection(&out_set).collect::>(); + + // filter and sort + inputs.retain(|inp| !to_cut_through.contains(&inp.commitment())); + outputs.retain(|out| !to_cut_through.contains(&out.commitment())); + inputs.sort(); + outputs.sort(); + Ok(()) +} + /// Aggregate a vec of transactions into a multi-kernel transaction with /// cut_through. Optionally allows passing a reward output and kernel for /// block building. @@ -717,36 +744,10 @@ pub fn aggregate( outputs.push(out); kernels.push(kernel); } - - // assemble output commitments set, checking they're all unique - let mut out_set = HashSet::new(); - let all_uniq = { outputs.iter().all(|o| out_set.insert(o.commitment())) }; - if !all_uniq { - return Err(Error::AggregationError); - } - - let in_set = inputs - .iter() - .map(|inp| inp.commitment()) - .collect::>(); - - let to_cut_through = in_set.intersection(&out_set).collect::>(); - - let mut new_inputs = inputs - .into_iter() - .filter(|inp| !to_cut_through.contains(&inp.commitment())) - .collect::>(); - - let mut new_outputs = outputs - .into_iter() - .filter(|out| !to_cut_through.contains(&out.commitment())) - .collect::>(); - - // sort them lexicographically - new_inputs.sort(); - new_outputs.sort(); kernels.sort(); + cut_through(&mut inputs, &mut outputs)?; + // now sum the kernel_offsets up to give us an aggregate offset for the // transaction let total_kernel_offset = committed::sum_kernel_offsets(kernel_offsets, vec![])?; @@ -756,7 +757,7 @@ pub fn aggregate( // * cut-through outputs // * full set of tx kernels // * sum of all kernel offsets - let tx = Transaction::new(new_inputs, new_outputs, kernels).with_offset(total_kernel_offset); + let tx = Transaction::new(inputs, outputs, kernels).with_offset(total_kernel_offset); // Now validate the aggregate tx to ensure we have not built something invalid. // The resulting tx could be invalid for a variety of reasons - @@ -1154,9 +1155,9 @@ mod test { commit: commit, }; - let block_hash = - Hash::from_hex("3a42e66e46dd7633b57d1f921780a1ac715e6b93c19ee52ab714178eb3a9f673") - .unwrap(); + let block_hash = Hash::from_hex( + "3a42e66e46dd7633b57d1f921780a1ac715e6b93c19ee52ab714178eb3a9f673", + ).unwrap(); let nonce = 0; diff --git a/core/tests/block.rs b/core/tests/block.rs index b6bee2b9..c7b4378d 100644 --- a/core/tests/block.rs +++ b/core/tests/block.rs @@ -377,7 +377,7 @@ fn hydrate_empty_compact_block() { let key_id = keychain.derive_key_id(1).unwrap(); let b = new_block(vec![], &keychain, &prev, &key_id); let cb = b.as_compact_block(); - let hb = Block::hydrate_from(cb, vec![]); + let hb = Block::hydrate_from(cb, vec![]).unwrap(); assert_eq!(hb.header, b.header); assert_eq!(hb.outputs(), b.outputs()); assert_eq!(hb.kernels(), b.kernels()); diff --git a/core/tests/common/mod.rs b/core/tests/common/mod.rs index c6f32839..495a0425 100644 --- a/core/tests/common/mod.rs +++ b/core/tests/common/mod.rs @@ -19,9 +19,9 @@ extern crate grin_keychain as keychain; extern crate grin_util as util; extern crate grin_wallet as wallet; -use grin_core::core::Transaction; use grin_core::core::block::{Block, BlockHeader}; use grin_core::core::target::Difficulty; +use grin_core::core::Transaction; use keychain::{Identifier, Keychain}; use wallet::libtx::build::{self, input, output, with_fee}; use wallet::libtx::reward; diff --git a/core/tests/core.rs b/core/tests/core.rs index fed368b2..ca9b5eeb 100644 --- a/core/tests/core.rs +++ b/core/tests/core.rs @@ -406,6 +406,7 @@ fn reward_empty_block() { let b = new_block(vec![], &keychain, &previous_header, &key_id); b.cut_through() + .unwrap() .validate(&BlindingFactor::zero(), &zero_commit) .unwrap(); } @@ -425,6 +426,7 @@ fn reward_with_tx_block() { let block = new_block(vec![&mut tx1], &keychain, &previous_header, &key_id); block .cut_through() + .unwrap() .validate(&BlindingFactor::zero(), &zero_commit) .unwrap(); } diff --git a/pool/tests/common/mod.rs b/pool/tests/common/mod.rs index e766412d..159228dc 100644 --- a/pool/tests/common/mod.rs +++ b/pool/tests/common/mod.rs @@ -23,8 +23,8 @@ extern crate grin_store as store; extern crate grin_util as util; extern crate grin_wallet as wallet; -extern crate rand; extern crate chrono; +extern crate rand; use std::fs; use std::sync::{Arc, RwLock}; @@ -41,8 +41,8 @@ use pool::*; use keychain::Keychain; use wallet::libtx; -use pool::TransactionPool; use pool::types::*; +use pool::TransactionPool; #[derive(Clone)] pub struct ChainAdapter { @@ -124,10 +124,7 @@ where // single input spending a single coinbase (deterministic key_id aka height) { let key_id = keychain.derive_key_id(header.height as u32).unwrap(); - tx_elements.push(libtx::build::coinbase_input( - coinbase_reward, - key_id, - )); + tx_elements.push(libtx::build::coinbase_input(coinbase_reward, key_id)); } for output_value in output_values { diff --git a/servers/src/common/adapters.rs b/servers/src/common/adapters.rs index bf50c416..291ccf42 100644 --- a/servers/src/common/adapters.rs +++ b/servers/src/common/adapters.rs @@ -25,11 +25,11 @@ use std::time::Instant; use chain::{self, ChainAdapter, Options, Tip}; use common::types::{self, ChainValidationMode, ServerConfig, SyncState, SyncStatus}; -use core::{core, global}; use core::core::block::BlockHeader; use core::core::hash::{Hash, Hashed}; use core::core::target::Difficulty; use core::core::transaction::Transaction; +use core::{core, global}; use p2p; use pool; use store; @@ -119,12 +119,16 @@ impl p2p::ChainAdapter for NetToChainAdapter { ); if cb.kern_ids.is_empty() { - let block = core::Block::hydrate_from(cb, vec![]); - + let cbh = cb.hash(); // push the freshly hydrated block through the chain pipeline - self.process_block(block, addr) + match core::Block::hydrate_from(cb, vec![]) { + Ok(block) => self.process_block(block, addr), + Err(e) => { + debug!(LOGGER, "Invalid hydrated block {}: {}", cbh, e); + return false; + } + } } else { - // check at least the header is valid before hydrating if let Err(e) = w(&self.chain).process_block_header(&cb.header, self.chain_opts()) { debug!(LOGGER, "Invalid compact block header {}: {}", cb.hash(), e); @@ -143,9 +147,16 @@ impl p2p::ChainAdapter for NetToChainAdapter { // 2) we hydrate an invalid block (txs legit missing from our pool) // 3) we hydrate an invalid block (peer sent us a "bad" compact block) - [TBD] - let block = core::Block::hydrate_from(cb.clone(), txs); + let block = match core::Block::hydrate_from(cb.clone(), txs) { + Ok(block) => block, + Err(e) => { + debug!(LOGGER, "Invalid hydrated block {}: {}", cb.hash(), e); + return false; + } + }; - let chain = self.chain + let chain = self + .chain .upgrade() .expect("failed to upgrade weak ref to chain"); @@ -346,19 +357,13 @@ impl p2p::ChainAdapter for NetToChainAdapter { /// If we're willing to accept that new state, the data stream will be /// read as a zip file, unzipped and the resulting state files should be /// rewound to the provided indexes. - fn txhashset_write( - &self, - h: Hash, - txhashset_data: File, - _peer_addr: SocketAddr, - ) -> bool { + fn txhashset_write(&self, h: Hash, txhashset_data: File, _peer_addr: SocketAddr) -> bool { // check status again after download, in case 2 txhashsets made it somehow if self.sync_state.status() != SyncStatus::TxHashsetDownload { return true; } - - if let Err(e) = - w(&self.chain).txhashset_write(h, txhashset_data, self.sync_state.as_ref()) + + if let Err(e) = w(&self.chain).txhashset_write(h, txhashset_data, self.sync_state.as_ref()) { error!(LOGGER, "Failed to save txhashset archive: {}", e); let is_good_data = !e.is_bad_data(); @@ -441,7 +446,11 @@ impl NetToChainAdapter { let head = chain.head().unwrap(); // we have a fast sync'd node and are sent a block older than our horizon, // only sync can do something with that - if b.header.height < head.height.saturating_sub(global::cut_through_horizon() as u64) { + if b.header.height + < head + .height + .saturating_sub(global::cut_through_horizon() as u64) + { return true; } }