Introduce cut_through method to avoid code duplication (#1355)

* Introduce cut_through method to avoid code duplication
* Improve error handling, fix sorting.
This commit is contained in:
Ignotus Peverell
2018-08-15 16:20:33 -07:00
committed by GitHub
parent 99a66c1960
commit 6775112f61
8 changed files with 77 additions and 94 deletions
+10 -37
View File
@@ -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<Transaction>) -> Block {
pub fn hydrate_from(cb: CompactBlock, txs: Vec<Transaction>) -> Result<Block, Error> {
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::<HashSet<_>>();
pub fn cut_through(self) -> Result<Block, Error> {
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::<HashSet<_>>();
let to_cut_through = in_set.intersection(&out_set).collect::<HashSet<_>>();
let new_inputs = self
.body
.inputs
.into_iter()
.filter(|inp| !to_cut_through.contains(&inp.commitment()))
.collect::<Vec<_>>();
let new_outputs = self
.body
.outputs
.into_iter()
.filter(|out| !to_cut_through.contains(&out.commitment()))
.collect::<Vec<_>>();
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
+1
View File
@@ -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,
}
+33 -32
View File
@@ -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<Input>, outputs: &mut Vec<Output>) -> 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::<HashSet<_>>();
let to_cut_through = in_set.intersection(&out_set).collect::<HashSet<_>>();
// 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::<HashSet<_>>();
let to_cut_through = in_set.intersection(&out_set).collect::<HashSet<_>>();
let mut new_inputs = inputs
.into_iter()
.filter(|inp| !to_cut_through.contains(&inp.commitment()))
.collect::<Vec<_>>();
let mut new_outputs = outputs
.into_iter()
.filter(|out| !to_cut_through.contains(&out.commitment()))
.collect::<Vec<_>>();
// 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;
+1 -1
View File
@@ -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());
+1 -1
View File
@@ -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;
+2
View File
@@ -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();
}
+3 -6
View File
@@ -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 {
+26 -17
View File
@@ -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;
}
}