From e88567c75aed615a2ea3a9032fe7c71fa9d96d04 Mon Sep 17 00:00:00 2001 From: quake wang Date: Tue, 19 Mar 2019 17:31:23 +0900 Subject: [PATCH 01/13] refactor: move some code to CellSetDiff --- chain/src/chain.rs | 52 ++++++------------------------- shared/src/cell_set.rs | 71 ++++++++++++++++++++++++++---------------- shared/src/shared.rs | 3 +- 3 files changed, 55 insertions(+), 71 deletions(-) diff --git a/chain/src/chain.rs b/chain/src/chain.rs index 1d9d00eb7e..28fb79cbcc 100644 --- a/chain/src/chain.rs +++ b/chain/src/chain.rs @@ -426,23 +426,7 @@ impl ChainService { chain_state: &mut ChainState, ) -> Result { let skip_verify = !self.verification; - - let mut old_inputs = FnvHashSet::default(); - let mut old_outputs = FnvHashSet::default(); - let mut new_inputs = FnvHashSet::default(); - let mut new_outputs = FnvHashMap::default(); - - let push_new = |b: &Block, - new_inputs: &mut FnvHashSet, - new_outputs: &mut FnvHashMap| { - for tx in b.commit_transactions() { - let input_pts = tx.input_pts(); - let tx_hash = tx.hash(); - let output_len = tx.outputs().len(); - new_inputs.extend(input_pts); - new_outputs.insert(tx_hash, output_len); - } - }; + let mut cell_set_diff = CellSetDiff::default(); let attached_blocks_iter = fork.attached_blocks().iter().rev(); let detached_blocks_iter = fork.detached_blocks().iter().rev(); @@ -451,17 +435,11 @@ impl ChainService { let verified_len = attached_blocks_len - fork.dirty_exts.len(); for b in detached_blocks_iter { - for tx in b.commit_transactions() { - let input_pts = tx.input_pts(); - let tx_hash = tx.hash(); - - old_inputs.extend(input_pts); - old_outputs.insert(tx_hash); - } + cell_set_diff.push_old(b); } - for b in attached_blocks_iter.clone().take(verified_len) { - push_new(b, &mut new_inputs, &mut new_outputs); + for b in attached_blocks_iter.take(verified_len) { + cell_set_diff.push_new(b); } // The verify function @@ -477,20 +455,20 @@ impl ChainService { { let cell_resolver = |op: &OutPoint| { self.shared.cell_at(op, |op| { - if new_inputs.contains(op) { + if cell_set_diff.new_inputs.contains(op) { Some(true) - } else if let Some(x) = new_outputs.get(&op.hash) { + } else if let Some(x) = cell_set_diff.new_outputs.get(&op.hash) { if op.index < (*x as u32) { Some(false) } else { Some(true) } - } else if old_outputs.contains(&op.hash) { + } else if cell_set_diff.old_outputs.contains(&op.hash) { None } else { chain_state .is_dead(op) - .map(|x| x && !old_inputs.contains(op)) + .map(|x| x && !cell_set_diff.old_inputs.contains(op)) } }) }; @@ -527,7 +505,7 @@ impl ChainService { .verify(chain_state.mut_txs_verify_cache(), &resolved) .is_ok() { - push_new(b, &mut new_inputs, &mut new_outputs); + cell_set_diff.push_new(b); ext.txs_verified = Some(true); } else { found_error = true; @@ -551,17 +529,7 @@ impl ChainService { Err(SharedError::InvalidTransaction)?; } - let old_inputs: Vec = old_inputs.into_iter().collect(); - let old_outputs: Vec = old_outputs.into_iter().collect(); - let new_inputs: Vec = new_inputs.into_iter().collect(); - let new_outputs: Vec<(H256, usize)> = new_outputs.into_iter().collect(); - - Ok(CellSetDiff { - old_inputs, - old_outputs, - new_inputs, - new_outputs, - }) + Ok(cell_set_diff) } // TODO: beatify diff --git a/shared/src/cell_set.rs b/shared/src/cell_set.rs index 3b1eab26a3..4c4cfe9f3a 100644 --- a/shared/src/cell_set.rs +++ b/shared/src/cell_set.rs @@ -1,15 +1,38 @@ +use ckb_core::block::Block; use ckb_core::transaction::OutPoint; use ckb_core::transaction_meta::TransactionMeta; -use fnv::FnvHashMap; +use fnv::{FnvHashMap, FnvHashSet}; use numext_fixed_hash::H256; use serde_derive::{Deserialize, Serialize}; #[derive(Default, Debug, Clone)] pub struct CellSetDiff { - pub old_inputs: Vec, - pub old_outputs: Vec, - pub new_inputs: Vec, - pub new_outputs: Vec<(H256, usize)>, + pub old_inputs: FnvHashSet, + pub old_outputs: FnvHashSet, + pub new_inputs: FnvHashSet, + pub new_outputs: FnvHashMap, +} + +impl CellSetDiff { + pub fn push_new(&mut self, block: &Block) { + for tx in block.commit_transactions() { + let input_pts = tx.input_pts(); + let tx_hash = tx.hash(); + let output_len = tx.outputs().len(); + self.new_inputs.extend(input_pts); + self.new_outputs.insert(tx_hash, output_len); + } + } + + pub fn push_old(&mut self, block: &Block) { + for tx in block.commit_transactions() { + let input_pts = tx.input_pts(); + let tx_hash = tx.hash(); + + self.old_inputs.extend(input_pts); + self.old_outputs.insert(tx_hash); + } + } } #[derive(Default, Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] @@ -32,8 +55,9 @@ impl CellSet { self.inner.get(h) } - pub fn insert(&mut self, hash: H256, outputs_len: usize) { - self.inner.insert(hash, TransactionMeta::new(outputs_len)); + pub fn insert(&mut self, hash: &H256, outputs_len: usize) { + self.inner + .insert(hash.clone(), TransactionMeta::new(outputs_len)); } pub fn remove(&mut self, hash: &H256) -> Option { @@ -52,28 +76,21 @@ impl CellSet { } } - fn rollback(&mut self, inputs: Vec, outputs: Vec) { - for h in outputs { - self.remove(&h); - } - - for o in inputs { - self.mark_live(&o); - } - } + pub fn update(&mut self, diff: CellSetDiff) { + diff.old_outputs.iter().for_each(|h| { + self.remove(h); + }); - fn forward(&mut self, inputs: Vec, outputs: Vec<(H256, usize)>) { - for (hash, len) in outputs { - self.insert(hash, len); - } + diff.old_inputs.iter().for_each(|o| { + self.mark_live(o); + }); - for o in inputs { - self.mark_dead(&o); - } - } + diff.new_outputs.iter().for_each(|(hash, len)| { + self.insert(hash, *len); + }); - pub fn update(&mut self, diff: CellSetDiff) { - self.rollback(diff.old_inputs, diff.old_outputs); - self.forward(diff.new_inputs, diff.new_outputs); + diff.new_inputs.iter().for_each(|o| { + self.mark_dead(o); + }); } } diff --git a/shared/src/shared.rs b/shared/src/shared.rs index 263b85c729..8633eb78f9 100644 --- a/shared/src/shared.rs +++ b/shared/src/shared.rs @@ -135,14 +135,13 @@ impl Shared { let hash = store.get_block_hash(n).unwrap(); for tx in store.get_block_body(&hash).unwrap() { let inputs = tx.input_pts(); - let tx_hash = tx.hash(); let output_len = tx.outputs().len(); for o in inputs { cell_set.mark_dead(&o); } - cell_set.insert(tx_hash, output_len); + cell_set.insert(&tx.hash(), output_len); } } From 87b56a77dc5b461f377f4a4aa77d4210a32e016b Mon Sep 17 00:00:00 2001 From: quake wang Date: Tue, 19 Mar 2019 17:49:43 +0900 Subject: [PATCH 02/13] chore: remove unused resolve_transaction in CellProvider trait --- core/src/cell.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/core/src/cell.rs b/core/src/cell.rs index ba18000dad..5d942f2fe7 100644 --- a/core/src/cell.rs +++ b/core/src/cell.rs @@ -62,11 +62,6 @@ pub trait CellProvider { ) -> CellStatus { unreachable!() } - - fn resolve_transaction(&self, transaction: &Transaction) -> ResolvedTransaction { - let mut seen_inputs = FnvHashSet::default(); - resolve_transaction(transaction, &mut seen_inputs, |x| self.cell(x)) - } } pub fn resolve_transaction CellStatus>( From 08395262865e6ea42e1b5e0b3f4b55c46e3caa6c Mon Sep 17 00:00:00 2001 From: quake wang Date: Wed, 20 Mar 2019 09:08:43 +0900 Subject: [PATCH 03/13] refactor: add OverlayCellProvider and refactor code --- chain/src/chain.rs | 43 ++++----------- chain/src/tests/basic.rs | 2 +- core/src/cell.rs | 86 +++++++++++++++++++++++------- rpc/src/module/chain.rs | 7 ++- shared/src/cell_set.rs | 26 +++++++++ shared/src/chain_state.rs | 76 +++++++++----------------- shared/src/shared.rs | 30 +---------- verification/src/block_verifier.rs | 6 +-- verification/src/tests/dummy.rs | 4 -- 9 files changed, 138 insertions(+), 142 deletions(-) diff --git a/chain/src/chain.rs b/chain/src/chain.rs index 28fb79cbcc..fba62693ac 100644 --- a/chain/src/chain.rs +++ b/chain/src/chain.rs @@ -1,8 +1,10 @@ use ckb_core::block::Block; -use ckb_core::cell::{resolve_transaction, CellProvider, CellStatus, ResolvedTransaction}; +use ckb_core::cell::{ + resolve_transaction, BlockCellProvider, OverlayCellProvider, ResolvedTransaction, +}; use ckb_core::extras::BlockExt; use ckb_core::service::{Request, DEFAULT_CHANNEL_SIZE, SIGNAL_CHANNEL_SIZE}; -use ckb_core::transaction::{OutPoint, ProposalShortId}; +use ckb_core::transaction::ProposalShortId; use ckb_core::BlockNumber; use ckb_db::batch::Batch; use ckb_notify::NotifyController; @@ -453,26 +455,6 @@ impl ChainService { .zip(fork.attached_blocks.iter()) .rev() { - let cell_resolver = |op: &OutPoint| { - self.shared.cell_at(op, |op| { - if cell_set_diff.new_inputs.contains(op) { - Some(true) - } else if let Some(x) = cell_set_diff.new_outputs.get(&op.hash) { - if op.index < (*x as u32) { - Some(false) - } else { - Some(true) - } - } else if cell_set_diff.old_outputs.contains(&op.hash) { - None - } else { - chain_state - .is_dead(op) - .map(|x| x && !cell_set_diff.old_inputs.contains(op)) - } - }) - }; - let mut output_indexs = FnvHashMap::default(); let mut seen_inputs = FnvHashSet::default(); @@ -480,23 +462,16 @@ impl ChainService { output_indexs.insert(tx.hash(), i); } + let cell_set_diff_cp = OverlayCellProvider::new(&cell_set_diff, chain_state.cell_set()); + let block_cp = BlockCellProvider::new(b); + let cell_provider = OverlayCellProvider::new(&block_cp, &cell_set_diff_cp); + // cellbase verified let resolved: Vec = b .commit_transactions() .iter() .skip(1) - .map(|x| { - resolve_transaction(x, &mut seen_inputs, |o| { - if let Some(i) = output_indexs.get(&o.hash) { - match b.commit_transactions()[*i].outputs().get(o.index as usize) { - Some(x) => CellStatus::Live(x.clone()), - None => CellStatus::Unknown, - } - } else { - cell_resolver(o) - } - }) - }) + .map(|x| resolve_transaction(x, &mut seen_inputs, &cell_provider)) .collect(); if !found_error diff --git a/chain/src/tests/basic.rs b/chain/src/tests/basic.rs index 4f59999211..af2867e66f 100644 --- a/chain/src/tests/basic.rs +++ b/chain/src/tests/basic.rs @@ -79,7 +79,7 @@ fn test_genesis_transaction_fetch() { let (_chain_controller, shared) = start_chain(Some(consensus)); let out_point = OutPoint::new(root_hash, 0); - let state = shared.cell(&out_point); + let state = shared.chain_state().lock().cell(&out_point); assert!(state.is_live()); } diff --git a/core/src/cell.rs b/core/src/cell.rs index 5d942f2fe7..06f567c111 100644 --- a/core/src/cell.rs +++ b/core/src/cell.rs @@ -1,5 +1,7 @@ +use crate::block::Block; use crate::transaction::{CellOutput, OutPoint, Transaction}; -use fnv::FnvHashSet; +use fnv::{FnvHashMap, FnvHashSet}; +use numext_fixed_hash::H256; use std::iter::Chain; use std::slice; @@ -54,27 +56,81 @@ pub struct ResolvedTransaction { pub trait CellProvider { fn cell(&self, out_point: &OutPoint) -> CellStatus; +} + +pub struct OverlayCellProvider<'a, O, CP> { + overlay: &'a O, + cell_provider: &'a CP, +} + +impl<'a, O, CP> OverlayCellProvider<'a, O, CP> { + pub fn new(overlay: &'a O, cell_provider: &'a CP) -> Self { + OverlayCellProvider { + overlay, + cell_provider, + } + } +} + +impl<'a, O, CP> CellProvider for OverlayCellProvider<'a, O, CP> +where + O: CellProvider, + CP: CellProvider, +{ + fn cell(&self, out_point: &OutPoint) -> CellStatus { + match self.overlay.cell(out_point) { + CellStatus::Live(co) => CellStatus::Live(co), + CellStatus::Dead => CellStatus::Dead, + CellStatus::Unknown => self.cell_provider.cell(out_point), + } + } +} + +pub struct BlockCellProvider<'a> { + output_indexs: FnvHashMap, + block: &'a Block, +} + +impl<'a> BlockCellProvider<'a> { + pub fn new(block: &'a Block) -> Self { + let mut output_indexs = FnvHashMap::default(); + for (i, tx) in block.commit_transactions().iter().enumerate() { + output_indexs.insert(tx.hash(), i); + } + Self { + output_indexs, + block, + } + } +} - fn cell_at Option>( - &self, - _out_point: &OutPoint, - _is_dead: F, - ) -> CellStatus { - unreachable!() +impl<'a> CellProvider for BlockCellProvider<'a> { + fn cell(&self, out_point: &OutPoint) -> CellStatus { + if let Some(i) = self.output_indexs.get(&out_point.hash) { + match self.block.commit_transactions()[*i] + .outputs() + .get(out_point.index as usize) + { + Some(x) => CellStatus::Live(x.clone()), + None => CellStatus::Unknown, + } + } else { + CellStatus::Unknown + } } } -pub fn resolve_transaction CellStatus>( +pub fn resolve_transaction( transaction: &Transaction, seen_inputs: &mut FnvHashSet, - cell: F, + cell_provider: &CP, ) -> ResolvedTransaction { let input_cells = transaction .input_pts() .iter() .map(|input| { if seen_inputs.insert(input.clone()) { - cell(input) + cell_provider.cell(input) } else { CellStatus::Dead } @@ -86,7 +142,7 @@ pub fn resolve_transaction CellStatus>( .iter() .map(|dep| { if seen_inputs.insert(dep.clone()) { - cell(dep) + cell_provider.cell(dep) } else { CellStatus::Dead } @@ -141,14 +197,6 @@ mod tests { None => CellStatus::Unknown, } } - - fn cell_at Option>(&self, o: &OutPoint, _: F) -> CellStatus { - match self.cells.get(o) { - Some(&Some(ref cell_output)) => CellStatus::Live(cell_output.clone()), - Some(&None) => CellStatus::Dead, - None => CellStatus::Unknown, - } - } } #[test] diff --git a/rpc/src/module/chain.rs b/rpc/src/module/chain.rs index fe18d3663e..b64c87f046 100644 --- a/rpc/src/module/chain.rs +++ b/rpc/src/module/chain.rs @@ -96,7 +96,12 @@ impl ChainRpc for ChainRpcImpl { } fn get_live_cell(&self, out_point: OutPoint) -> Result { - Ok(self.shared.cell(&(out_point.into())).into()) + Ok(self + .shared + .chain_state() + .lock() + .cell(&(out_point.into())) + .into()) } fn get_tip_block_number(&self) -> Result { diff --git a/shared/src/cell_set.rs b/shared/src/cell_set.rs index 4c4cfe9f3a..f280fe329a 100644 --- a/shared/src/cell_set.rs +++ b/shared/src/cell_set.rs @@ -1,4 +1,5 @@ use ckb_core::block::Block; +use ckb_core::cell::{CellProvider, CellStatus}; use ckb_core::transaction::OutPoint; use ckb_core::transaction_meta::TransactionMeta; use fnv::{FnvHashMap, FnvHashSet}; @@ -35,6 +36,16 @@ impl CellSetDiff { } } +impl CellProvider for CellSetDiff { + fn cell(&self, out_point: &OutPoint) -> CellStatus { + if self.new_inputs.contains(out_point) { + CellStatus::Dead + } else { + CellStatus::Unknown + } + } +} + #[derive(Default, Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] pub struct CellSet { pub inner: FnvHashMap, @@ -94,3 +105,18 @@ impl CellSet { }); } } + +impl CellProvider for CellSet { + fn cell(&self, out_point: &OutPoint) -> CellStatus { + match self.get(&out_point.hash) { + Some(meta) => { + if meta.is_dead(out_point.index as usize) { + CellStatus::Dead + } else { + CellStatus::Unknown + } + } + None => CellStatus::Unknown, + } + } +} diff --git a/shared/src/chain_state.rs b/shared/src/chain_state.rs index b351ea94bd..366cde5b07 100644 --- a/shared/src/chain_state.rs +++ b/shared/src/chain_state.rs @@ -4,7 +4,9 @@ use crate::index::ChainIndex; use crate::tx_pool::{PoolEntry, PoolError, StagingTxResult, TxPool}; use crate::tx_proposal_table::TxProposalTable; use ckb_core::block::Block; -use ckb_core::cell::{CellProvider, CellStatus, ResolvedTransaction}; +use ckb_core::cell::{ + resolve_transaction, CellProvider, CellStatus, OverlayCellProvider, ResolvedTransaction, +}; use ckb_core::header::{BlockNumber, Header}; use ckb_core::transaction::{OutPoint, ProposalShortId, Transaction}; use ckb_core::Cycle; @@ -124,58 +126,10 @@ impl ChainState { } } - fn get_cell_status_from_store(&self, out_point: &OutPoint) -> CellStatus { - let index = out_point.index as usize; - if let Some(f) = self.is_dead(out_point) { - if f { - CellStatus::Dead - } else { - let transaction = self - .store - .get_transaction(&out_point.hash) - .expect("transaction must exist"); - CellStatus::Live(transaction.outputs()[index].clone()) - } - } else { - CellStatus::Unknown - } - } - pub fn resolve_tx_from_pool(&self, tx: &Transaction, tx_pool: &TxPool) -> ResolvedTransaction { - let fetch_cell = |op| match tx_pool.staging.cell(op) { - CellStatus::Unknown => self.get_cell_status_from_store(op), - cs => cs, - }; + let cell_provider = OverlayCellProvider::new(&tx_pool.staging, self); let mut seen_inputs = FnvHashSet::default(); - let inputs = tx.input_pts(); - let input_cells = inputs - .iter() - .map(|input| { - if seen_inputs.insert(input.clone()) { - fetch_cell(input) - } else { - CellStatus::Dead - } - }) - .collect(); - - let dep_cells = tx - .dep_pts() - .iter() - .map(|dep| { - if seen_inputs.insert(dep.clone()) { - fetch_cell(dep) - } else { - CellStatus::Dead - } - }) - .collect(); - - ResolvedTransaction { - transaction: tx.clone(), - input_cells, - dep_cells, - } + resolve_transaction(tx, &mut seen_inputs, &cell_provider) } pub fn verify_rtx( @@ -370,3 +324,23 @@ impl ChainState { self.tx_pool.get_mut() } } + +impl CellProvider for ChainState { + fn cell(&self, out_point: &OutPoint) -> CellStatus { + if let Some(f) = self.is_dead(out_point) { + if f { + CellStatus::Dead + } else { + match self.store.get_transaction(&out_point.hash) { + Some(tx) => match tx.outputs().get(out_point.index as usize) { + Some(output) => CellStatus::Live(output.clone()), + None => CellStatus::Unknown, + }, + None => CellStatus::Unknown, + } + } + } else { + CellStatus::Unknown + } + } +} diff --git a/shared/src/shared.rs b/shared/src/shared.rs index 8633eb78f9..e530c8f888 100644 --- a/shared/src/shared.rs +++ b/shared/src/shared.rs @@ -9,10 +9,9 @@ use crate::tx_proposal_table::TxProposalTable; use crate::{COLUMNS, COLUMN_BLOCK_HEADER}; use ckb_chain_spec::consensus::{Consensus, ProposalWindow}; use ckb_core::block::Block; -use ckb_core::cell::{CellProvider, CellStatus}; use ckb_core::extras::BlockExt; use ckb_core::header::{BlockNumber, Header}; -use ckb_core::transaction::{Capacity, OutPoint, ProposalShortId, Transaction}; +use ckb_core::transaction::{Capacity, ProposalShortId, Transaction}; use ckb_core::uncle::UncleBlock; use ckb_db::{DBConfig, KeyValueDB, MemoryKeyValueDB, RocksDB}; use ckb_traits::{BlockMedianTimeContext, ChainProvider}; @@ -149,33 +148,6 @@ impl Shared { } } -impl CellProvider for Shared { - fn cell(&self, out_point: &OutPoint) -> CellStatus { - self.cell_at(out_point, |op| self.chain_state.lock().is_dead(op)) - } - - fn cell_at Option>( - &self, - out_point: &OutPoint, - is_dead: F, - ) -> CellStatus { - let index = out_point.index as usize; - if let Some(f) = is_dead(out_point) { - if f { - CellStatus::Dead - } else { - let transaction = self - .store - .get_transaction(&out_point.hash) - .expect("transaction must exist"); - CellStatus::Live(transaction.outputs()[index].clone()) - } - } else { - CellStatus::Unknown - } - } -} - impl ChainProvider for Shared { fn block(&self, hash: &H256) -> Option { self.store.get_block(hash) diff --git a/verification/src/block_verifier.rs b/verification/src/block_verifier.rs index 3f1fd78020..54754add0b 100644 --- a/verification/src/block_verifier.rs +++ b/verification/src/block_verifier.rs @@ -2,7 +2,7 @@ use crate::error::{CellbaseError, CommitError, Error, UnclesError}; use crate::header_verifier::HeaderResolver; use crate::{InputVerifier, TransactionVerifier, Verifier}; use ckb_core::block::Block; -use ckb_core::cell::{CellProvider, ResolvedTransaction}; +use ckb_core::cell::ResolvedTransaction; use ckb_core::header::Header; use ckb_core::transaction::{Capacity, CellInput}; use ckb_core::Cycle; @@ -32,7 +32,7 @@ pub struct BlockVerifier

{ impl

BlockVerifier

where - P: ChainProvider + CellProvider + Clone + 'static, + P: ChainProvider + Clone + 'static, { pub fn new(provider: P) -> Self { BlockVerifier { @@ -46,7 +46,7 @@ where } } -impl Verifier for BlockVerifier

{ +impl Verifier for BlockVerifier

{ type Target = Block; fn verify(&self, target: &Block) -> Result<(), Error> { diff --git a/verification/src/tests/dummy.rs b/verification/src/tests/dummy.rs index 07178b50d9..a2aae0d810 100644 --- a/verification/src/tests/dummy.rs +++ b/verification/src/tests/dummy.rs @@ -92,8 +92,4 @@ impl CellProvider for DummyChainProvider { fn cell(&self, _o: &OutPoint) -> CellStatus { panic!("Not implemented!"); } - - fn cell_at Option>(&self, _o: &OutPoint, _: F) -> CellStatus { - panic!("Not implemented!"); - } } From e3f531a5e0d5108045de433afa39e4a834e25bc2 Mon Sep 17 00:00:00 2001 From: quake wang Date: Thu, 21 Mar 2019 10:22:50 +0900 Subject: [PATCH 04/13] chore: tweak test code --- chain/src/tests/basic.rs | 52 ++++++++++++++++++------------------ chain/src/tests/find_fork.rs | 40 +++++++++++++-------------- chain/src/tests/util.rs | 19 ++++++------- 3 files changed, 56 insertions(+), 55 deletions(-) diff --git a/chain/src/tests/basic.rs b/chain/src/tests/basic.rs index af2867e66f..4e9f9b762c 100644 --- a/chain/src/tests/basic.rs +++ b/chain/src/tests/basic.rs @@ -32,17 +32,17 @@ fn test_genesis_transaction_spend() { .with_header_builder(HeaderBuilder::default().difficulty(U256::from(1000u64))); let consensus = Consensus::default().set_genesis_block(genesis_block); - let (chain_controller, shared) = start_chain(Some(consensus)); + let (chain_controller, shared) = start_chain(Some(consensus), false); let end = 21; let mut blocks1: Vec = vec![]; let mut parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); - for i in 1..end { + for _ in 1..end { let difficulty = parent.difficulty().clone(); let tx = create_transaction(root_hash); root_hash = tx.hash().clone(); - let new_block = gen_block(&parent, i, difficulty + U256::from(1u64), vec![tx], vec![]); + let new_block = gen_block(&parent, difficulty + U256::from(1u64), vec![tx], vec![], vec![]); blocks1.push(new_block.clone()); parent = new_block.header().clone(); } @@ -76,7 +76,7 @@ fn test_genesis_transaction_fetch() { .with_header_builder(HeaderBuilder::default().difficulty(U256::from(1000u64))); let consensus = Consensus::default().set_genesis_block(genesis_block); - let (_chain_controller, shared) = start_chain(Some(consensus)); + let (_chain_controller, shared) = start_chain(Some(consensus), false); let out_point = OutPoint::new(root_hash, 0); let state = shared.chain_state().lock().cell(&out_point); @@ -85,16 +85,16 @@ fn test_genesis_transaction_fetch() { #[test] fn test_chain_fork_by_total_difficulty() { - let (chain_controller, shared) = start_chain(None); + let (chain_controller, shared) = start_chain(None, false); let final_number = 20; let mut chain1: Vec = Vec::new(); let mut chain2: Vec = Vec::new(); let mut parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); - for i in 1..final_number { + for _ in 1..final_number { let difficulty = parent.difficulty().clone(); - let new_block = gen_block(&parent, i, difficulty + U256::from(100u64), vec![], vec![]); + let new_block = gen_block(&parent, difficulty + U256::from(100u64), vec![], vec![], vec![]); chain1.push(new_block.clone()); parent = new_block.header().clone(); } @@ -105,10 +105,10 @@ fn test_chain_fork_by_total_difficulty() { let j = if i > 10 { 110 } else { 99 }; let new_block = gen_block( &parent, - i + 1000, difficulty + U256::from(j as u32), vec![], vec![], + vec![], ); chain2.push(new_block.clone()); parent = new_block.header().clone(); @@ -133,29 +133,29 @@ fn test_chain_fork_by_total_difficulty() { #[test] fn test_chain_fork_by_hash() { - let (chain_controller, shared) = start_chain(None); + let (chain_controller, shared) = start_chain(None, false); let final_number = 20; let mut chain1: Vec = Vec::new(); let mut chain2: Vec = Vec::new(); let mut parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); - for i in 1..final_number { + for _ in 1..final_number { let difficulty = parent.difficulty().clone(); - let new_block = gen_block(&parent, i, difficulty + U256::from(100u64), vec![], vec![]); + let new_block = gen_block(&parent, difficulty + U256::from(100u64), vec![], vec![], vec![]); chain1.push(new_block.clone()); parent = new_block.header().clone(); } parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); - for i in 1..final_number { + for _ in 1..final_number { let difficulty = parent.difficulty().clone(); let new_block = gen_block( &parent, - i + 1000, difficulty + U256::from(100u64), vec![], vec![], + vec![], ); chain2.push(new_block.clone()); parent = new_block.header().clone(); @@ -195,29 +195,29 @@ fn test_chain_fork_by_hash() { #[test] fn test_chain_get_ancestor() { - let (chain_controller, shared) = start_chain(None); + let (chain_controller, shared) = start_chain(None, false); let final_number = 20; let mut chain1: Vec = Vec::new(); let mut chain2: Vec = Vec::new(); let mut parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); - for i in 1..final_number { + for _ in 1..final_number { let difficulty = parent.difficulty().clone(); - let new_block = gen_block(&parent, i, difficulty + U256::from(100u64), vec![], vec![]); + let new_block = gen_block(&parent, difficulty + U256::from(100u64), vec![], vec![], vec![]); chain1.push(new_block.clone()); parent = new_block.header().clone(); } parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); - for i in 1..final_number { + for _ in 1..final_number { let difficulty = parent.difficulty().clone(); let new_block = gen_block( &parent, - i + 1000, difficulty + U256::from(100u64), vec![], vec![], + vec![], ); chain2.push(new_block.clone()); parent = new_block.header().clone(); @@ -258,16 +258,16 @@ fn test_calculate_difficulty() { consensus.pow_time_span = 200; consensus.pow_spacing = 1; - let (chain_controller, shared) = start_chain(Some(consensus.clone())); + let (chain_controller, shared) = start_chain(Some(consensus.clone()), false); let final_number = shared.consensus().difficulty_adjustment_interval(); let mut chain1: Vec = Vec::new(); let mut chain2: Vec = Vec::new(); let mut parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); - for i in 1..final_number - 1 { + for _ in 1..final_number - 1 { let difficulty = shared.calculate_difficulty(&parent).unwrap(); - let new_block = gen_block(&parent, i, difficulty, vec![], vec![]); + let new_block = gen_block(&parent, difficulty, vec![], vec![], vec![]); chain_controller .process_block(Arc::new(new_block.clone())) .expect("process block ok"); @@ -282,7 +282,7 @@ fn test_calculate_difficulty() { if i < 26 { uncles.push(chain1[i as usize].clone().into()); } - let new_block = gen_block(&parent, i + 100, difficulty, vec![], uncles); + let new_block = gen_block(&parent, difficulty, vec![], vec![], uncles); chain_controller .process_block(Arc::new(new_block.clone())) .expect("process block ok"); @@ -297,7 +297,7 @@ fn test_calculate_difficulty() { // 25 * 10 * 1000 / 200 assert_eq!(difficulty, U256::from(1250u64)); - let (chain_controller, shared) = start_chain(Some(consensus.clone())); + let (chain_controller, shared) = start_chain(Some(consensus.clone()), false); let mut chain2: Vec = Vec::new(); for i in 1..final_number - 1 { chain_controller @@ -312,7 +312,7 @@ fn test_calculate_difficulty() { if i < 11 { uncles.push(chain1[i as usize].clone().into()); } - let new_block = gen_block(&parent, i + 100, difficulty, vec![], uncles); + let new_block = gen_block(&parent, difficulty, vec![], vec![], uncles); chain_controller .process_block(Arc::new(new_block.clone())) .expect("process block ok"); @@ -327,7 +327,7 @@ fn test_calculate_difficulty() { // min[10 * 10 * 1000 / 200, 1000] assert_eq!(difficulty, U256::from(1000u64)); - let (chain_controller, shared) = start_chain(Some(consensus.clone())); + let (chain_controller, shared) = start_chain(Some(consensus.clone()), false); let mut chain2: Vec = Vec::new(); for i in 1..final_number - 1 { chain_controller @@ -342,7 +342,7 @@ fn test_calculate_difficulty() { if i < 151 { uncles.push(chain1[i as usize].clone().into()); } - let new_block = gen_block(&parent, i + 100, difficulty, vec![], uncles); + let new_block = gen_block(&parent, difficulty, vec![], vec![], uncles); chain_controller .process_block(Arc::new(new_block.clone())) .expect("process block ok"); diff --git a/chain/src/tests/find_fork.rs b/chain/src/tests/find_fork.rs index 105c9defe6..4c06cfd62c 100644 --- a/chain/src/tests/find_fork.rs +++ b/chain/src/tests/find_fork.rs @@ -32,15 +32,15 @@ fn test_find_fork_case1() { let mut fork2: Vec = Vec::new(); let mut parent = genesis.clone(); - for i in 0..4 { - let new_block = gen_block(&parent, i, U256::from(100u64), vec![], vec![]); + for _ in 0..4 { + let new_block = gen_block(&parent, U256::from(100u64), vec![], vec![], vec![]); fork1.push(new_block.clone()); parent = new_block.header().clone(); } let mut parent = genesis.clone(); - for i in 0..3 { - let new_block = gen_block(&parent, i, U256::from(90u64), vec![], vec![]); + for _ in 0..3 { + let new_block = gen_block(&parent, U256::from(90u64), vec![], vec![], vec![]); fork2.push(new_block.clone()); parent = new_block.header().clone(); } @@ -58,7 +58,7 @@ fn test_find_fork_case1() { let tip_number = { shared.chain_state().lock().tip_number() }; // fork2 total_difficulty 470 - let new_block = gen_block(&parent, 100, U256::from(200u64), vec![], vec![]); + let new_block = gen_block(&parent, U256::from(200u64), vec![], vec![], vec![]); fork2.push(new_block.clone()); let ext = BlockExt { @@ -104,15 +104,15 @@ fn test_find_fork_case2() { let mut fork2: Vec = Vec::new(); let mut parent = genesis.clone(); - for i in 0..4 { - let new_block = gen_block(&parent, i, U256::from(100u64), vec![], vec![]); + for _ in 0..4 { + let new_block = gen_block(&parent, U256::from(100u64), vec![], vec![], vec![]); fork1.push(new_block.clone()); parent = new_block.header().clone(); } let mut parent = fork1[0].header().clone(); - for i in 0..2 { - let new_block = gen_block(&parent, i, U256::from(90u64), vec![], vec![]); + for _ in 0..2 { + let new_block = gen_block(&parent, U256::from(90u64), vec![], vec![], vec![]); fork2.push(new_block.clone()); parent = new_block.header().clone(); } @@ -132,10 +132,10 @@ fn test_find_fork_case2() { let difficulty = parent.difficulty().clone(); let new_block = gen_block( &parent, - 100, difficulty + U256::from(200u64), vec![], vec![], + vec![], ); fork2.push(new_block.clone()); @@ -182,15 +182,15 @@ fn test_find_fork_case3() { let mut fork2: Vec = Vec::new(); let mut parent = genesis.clone(); - for i in 0..3 { - let new_block = gen_block(&parent, i, U256::from(80u64), vec![], vec![]); + for _ in 0..3 { + let new_block = gen_block(&parent, U256::from(80u64), vec![], vec![], vec![]); fork1.push(new_block.clone()); parent = new_block.header().clone(); } let mut parent = genesis.clone(); - for i in 0..5 { - let new_block = gen_block(&parent, i, U256::from(40u64), vec![], vec![]); + for _ in 0..5 { + let new_block = gen_block(&parent, U256::from(40u64), vec![], vec![], vec![]); fork2.push(new_block.clone()); parent = new_block.header().clone(); } @@ -209,7 +209,7 @@ fn test_find_fork_case3() { println!("case3 tip{}", tip_number); - let new_block = gen_block(&parent, 100, U256::from(100u64), vec![], vec![]); + let new_block = gen_block(&parent, U256::from(100u64), vec![], vec![], vec![]); fork2.push(new_block.clone()); let ext = BlockExt { @@ -254,15 +254,15 @@ fn test_find_fork_case4() { let mut fork2: Vec = Vec::new(); let mut parent = genesis.clone(); - for i in 0..5 { - let new_block = gen_block(&parent, i, U256::from(40u64), vec![], vec![]); + for _ in 0..5 { + let new_block = gen_block(&parent, U256::from(40u64), vec![], vec![], vec![]); fork1.push(new_block.clone()); parent = new_block.header().clone(); } let mut parent = genesis.clone(); - for i in 0..2 { - let new_block = gen_block(&parent, i, U256::from(80u64), vec![], vec![]); + for _ in 0..2 { + let new_block = gen_block(&parent, U256::from(80u64), vec![], vec![], vec![]); fork2.push(new_block.clone()); parent = new_block.header().clone(); } @@ -281,7 +281,7 @@ fn test_find_fork_case4() { println!("case3 tip{}", tip_number); - let new_block = gen_block(&parent, 100, U256::from(100u64), vec![], vec![]); + let new_block = gen_block(&parent, U256::from(100u64), vec![], vec![], vec![]); fork2.push(new_block.clone()); let ext = BlockExt { diff --git a/chain/src/tests/util.rs b/chain/src/tests/util.rs index c14c93d8d8..3a81ab7310 100644 --- a/chain/src/tests/util.rs +++ b/chain/src/tests/util.rs @@ -4,7 +4,7 @@ use ckb_core::block::Block; use ckb_core::block::BlockBuilder; use ckb_core::header::{Header, HeaderBuilder}; use ckb_core::transaction::{ - CellInput, CellOutput, OutPoint, ProposalShortId, Transaction, TransactionBuilder, + CellInput, CellOutput, OutPoint, Transaction, TransactionBuilder, }; use ckb_core::uncle::UncleBlock; use ckb_core::BlockNumber; @@ -16,9 +16,11 @@ use ckb_shared::store::ChainKVStore; use faketime::unix_time_as_millis; use numext_fixed_hash::H256; use numext_fixed_uint::U256; +use rand; pub(crate) fn start_chain( consensus: Option, + verification: bool, ) -> (ChainController, Shared>) { let builder = SharedBuilder::::new(); let shared = builder @@ -27,7 +29,7 @@ pub(crate) fn start_chain( let notify = NotifyService::default().start::<&str>(None); let chain_service = ChainBuilder::new(shared.clone(), notify) - .verification(false) + .verification(verification) .build(); let chain_controller = chain_service.start::<&str>(None); (chain_controller, shared) @@ -42,28 +44,27 @@ fn create_cellbase(number: BlockNumber) -> Transaction { pub(crate) fn gen_block( parent_header: &Header, - nonce: u64, difficulty: U256, commit_transactions: Vec, + proposal_transactions: Vec, uncles: Vec, ) -> Block { let number = parent_header.number() + 1; let cellbase = create_cellbase(number); - let header = HeaderBuilder::default() + let header_builder = HeaderBuilder::default() .parent_hash(parent_header.hash().clone()) .timestamp(unix_time_as_millis()) .number(number) .difficulty(difficulty) - .nonce(nonce) - .build(); + .cellbase_id(cellbase.hash()) + .nonce(rand::random()); BlockBuilder::default() - .header(header) .commit_transaction(cellbase) .commit_transactions(commit_transactions) .uncles(uncles) - .proposal_transaction(ProposalShortId::from_slice(&[1; 10]).unwrap()) - .build() + .proposal_transactions(proposal_transactions.iter().map(|tx| tx.proposal_short_id()).collect()) + .with_header_builder(header_builder) } pub(crate) fn create_transaction(parent: H256) -> Transaction { From 2913cca3275aa752256e2eededa7a0450b7cddcb Mon Sep 17 00:00:00 2001 From: quake wang Date: Fri, 22 Mar 2019 11:05:25 +0900 Subject: [PATCH 05/13] fix: resolve transactions verifier bug and add some test --- chain/src/chain.rs | 75 ++++--- chain/src/tests/basic.rs | 265 ++++++++++++++++++++++- chain/src/tests/util.rs | 48 +++- core/src/cell.rs | 19 ++ core/src/transaction.rs | 4 + shared/src/chain_state.rs | 20 +- shared/src/error.rs | 4 +- verification/src/block_verifier.rs | 10 + verification/src/transaction_verifier.rs | 3 +- 9 files changed, 379 insertions(+), 69 deletions(-) diff --git a/chain/src/chain.rs b/chain/src/chain.rs index fba62693ac..2fb65c351a 100644 --- a/chain/src/chain.rs +++ b/chain/src/chain.rs @@ -18,7 +18,7 @@ use ckb_verification::{BlockVerifier, TransactionsVerifier, Verifier}; use crossbeam_channel::{self, select, Receiver, Sender}; use failure::Error as FailureError; use faketime::unix_time_as_millis; -use fnv::{FnvHashMap, FnvHashSet}; +use fnv::FnvHashSet; use log::{self, debug, error, log_enabled}; use numext_fixed_hash::H256; use numext_fixed_uint::U256; @@ -198,7 +198,7 @@ impl ChainService { ); if parent_ext.txs_verified == Some(false) { - Err(SharedError::InvalidTransaction)?; + Err(SharedError::InvalidParentBlock)?; } let ext = BlockExt { @@ -427,7 +427,6 @@ impl ChainService { fork: &mut ForkChanges, chain_state: &mut ChainState, ) -> Result { - let skip_verify = !self.verification; let mut cell_set_diff = CellSetDiff::default(); let attached_blocks_iter = fork.attached_blocks().iter().rev(); @@ -447,7 +446,7 @@ impl ChainService { // The verify function let txs_verifier = TransactionsVerifier::new(self.shared.consensus().max_block_cycles()); - let mut found_error = false; + let mut found_error = None; // verify transaction for (ext, b) in fork .dirty_exts @@ -455,36 +454,42 @@ impl ChainService { .zip(fork.attached_blocks.iter()) .rev() { - let mut output_indexs = FnvHashMap::default(); - let mut seen_inputs = FnvHashSet::default(); - - for (i, tx) in b.commit_transactions().iter().enumerate() { - output_indexs.insert(tx.hash(), i); - } - - let cell_set_diff_cp = OverlayCellProvider::new(&cell_set_diff, chain_state.cell_set()); - let block_cp = BlockCellProvider::new(b); - let cell_provider = OverlayCellProvider::new(&block_cp, &cell_set_diff_cp); - - // cellbase verified - let resolved: Vec = b - .commit_transactions() - .iter() - .skip(1) - .map(|x| resolve_transaction(x, &mut seen_inputs, &cell_provider)) - .collect(); - - if !found_error - || skip_verify - || txs_verifier - .verify(chain_state.mut_txs_verify_cache(), &resolved) - .is_ok() - { + if self.verification { + if found_error.is_none() { + let mut seen_inputs = FnvHashSet::default(); + + // TODO Q impl CellProvider for store, use chain_state as CellProvider here directly. + let cell_set_cp = OverlayCellProvider::new(chain_state.cell_set(), chain_state); + let cell_set_diff_cp = OverlayCellProvider::new(&cell_set_diff, &cell_set_cp); + let block_cp = BlockCellProvider::new(b); + let cell_provider = OverlayCellProvider::new(&block_cp, &cell_set_diff_cp); + + let resolved: Vec = b + .commit_transactions() + .iter() + .map(|x| resolve_transaction(x, &mut seen_inputs, &cell_provider)) + .collect(); + + match txs_verifier.verify( + chain_state.mut_txs_verify_cache(), + &resolved, + self.shared.block_reward(b.header().number()), + ) { + Ok(_) => { + cell_set_diff.push_new(b); + ext.txs_verified = Some(true); + } + Err(err) => { + found_error = Some(err); + ext.txs_verified = Some(false); + } + } + } else { + ext.txs_verified = Some(false); + } + } else { cell_set_diff.push_new(b); ext.txs_verified = Some(true); - } else { - found_error = true; - ext.txs_verified = Some(false); } } @@ -500,8 +505,10 @@ impl ChainService { .insert_block_ext(batch, &b.header().hash(), ext); } - if found_error { - Err(SharedError::InvalidTransaction)?; + if found_error.is_some() { + Err(SharedError::InvalidTransaction( + found_error.unwrap().to_string(), + ))?; } Ok(cell_set_diff) diff --git a/chain/src/tests/basic.rs b/chain/src/tests/basic.rs index 4e9f9b762c..94ae6ec3ee 100644 --- a/chain/src/tests/basic.rs +++ b/chain/src/tests/basic.rs @@ -38,11 +38,17 @@ fn test_genesis_transaction_spend() { let mut blocks1: Vec = vec![]; let mut parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); - for _ in 1..end { + for i in 1..end { let difficulty = parent.difficulty().clone(); - let tx = create_transaction(root_hash); + let tx = create_transaction(root_hash, i as u8); root_hash = tx.hash().clone(); - let new_block = gen_block(&parent, difficulty + U256::from(1u64), vec![tx], vec![], vec![]); + let new_block = gen_block( + &parent, + difficulty + U256::from(1u64), + vec![tx], + vec![], + vec![], + ); blocks1.push(new_block.clone()); parent = new_block.header().clone(); } @@ -54,6 +60,235 @@ fn test_genesis_transaction_spend() { } } +#[test] +fn test_transaction_spend_in_same_block() { + let (chain_controller, shared) = start_chain(None, true); + let mut chain: Vec = Vec::new(); + let mut parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); + { + let difficulty = parent.difficulty().clone(); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + vec![], + vec![], + vec![], + ); + parent = new_block.header().clone(); + chain.push(new_block); + } + + let last_cell_base = &chain.last().unwrap().commit_transactions()[0]; + let tx1 = create_transaction(last_cell_base.hash(), 1); + let tx2 = create_transaction(tx1.hash(), 2); + let txs = vec![tx1, tx2]; + // proposal txs + { + let difficulty = parent.difficulty().clone(); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + vec![], + txs.clone(), + vec![], + ); + parent = new_block.header().clone(); + chain.push(new_block); + } + // empty N+1 block + { + let difficulty = parent.difficulty().clone(); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + vec![], + vec![], + vec![], + ); + parent = new_block.header().clone(); + chain.push(new_block); + } + // commit txs in N+2 block + { + let difficulty = parent.difficulty().clone(); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + txs.clone(), + vec![], + vec![], + ); + chain.push(new_block); + } + for block in &chain { + chain_controller + .process_block(Arc::new(block.clone())) + .expect("process block ok"); + } +} + +#[test] +fn test_transaction_conflict_in_same_block() { + let (chain_controller, shared) = start_chain(None, true); + let mut chain: Vec = Vec::new(); + let mut parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); + { + let difficulty = parent.difficulty().clone(); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + vec![], + vec![], + vec![], + ); + parent = new_block.header().clone(); + chain.push(new_block); + } + + let last_cell_base = &chain.last().unwrap().commit_transactions()[0]; + let tx1 = create_transaction(last_cell_base.hash(), 1); + let tx2 = create_transaction(tx1.hash(), 2); + let tx3 = create_transaction(tx1.hash(), 3); + let txs = vec![tx1, tx2, tx3]; + // proposal txs + { + let difficulty = parent.difficulty().clone(); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + vec![], + txs.clone(), + vec![], + ); + parent = new_block.header().clone(); + chain.push(new_block); + } + // empty N+1 block + { + let difficulty = parent.difficulty().clone(); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + vec![], + vec![], + vec![], + ); + parent = new_block.header().clone(); + chain.push(new_block); + } + // commit txs in N+2 block + { + let difficulty = parent.difficulty().clone(); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + txs.clone(), + vec![], + vec![], + ); + chain.push(new_block); + } + for i in 0..3 { + chain_controller + .process_block(Arc::new(chain[i].clone())) + .expect("process block ok"); + } + assert_eq!( + "Err(InvalidTransaction(\"Transactions((2, Conflict))\"))", + format!( + "{:?}", + chain_controller.process_block(Arc::new(chain[3].clone())) + ) + ); +} + +#[test] +fn test_transaction_conflict_in_different_blocks() { + let (chain_controller, shared) = start_chain(None, true); + let mut chain: Vec = Vec::new(); + let mut parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); + { + let difficulty = parent.difficulty().clone(); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + vec![], + vec![], + vec![], + ); + parent = new_block.header().clone(); + chain.push(new_block); + } + + let last_cell_base = &chain.last().unwrap().commit_transactions()[0]; + let tx1 = create_transaction(last_cell_base.hash(), 1); + let tx2 = create_transaction(tx1.hash(), 2); + let tx3 = create_transaction(tx1.hash(), 3); + // proposal txs + { + let difficulty = parent.difficulty().clone(); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + vec![], + vec![tx1.clone(), tx2.clone(), tx3.clone()], + vec![], + ); + parent = new_block.header().clone(); + chain.push(new_block); + } + // empty N+1 block + { + let difficulty = parent.difficulty().clone(); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + vec![], + vec![], + vec![], + ); + parent = new_block.header().clone(); + chain.push(new_block); + } + // commit tx1 and tx2 in N+2 block + { + let difficulty = parent.difficulty().clone(); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + vec![tx1.clone(), tx2.clone()], + vec![], + vec![], + ); + parent = new_block.header().clone(); + chain.push(new_block); + } + // commit tx3 in N+3 block + { + let difficulty = parent.difficulty().clone(); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + vec![tx3.clone()], + vec![], + vec![], + ); + chain.push(new_block); + } + for i in 0..4 { + chain_controller + .process_block(Arc::new(chain[i].clone())) + .expect("process block ok"); + } + assert_eq!( + "Err(InvalidTransaction(\"Transactions((0, Conflict))\"))", + format!( + "{:?}", + chain_controller.process_block(Arc::new(chain[4].clone())) + ) + ); +} + #[test] fn test_genesis_transaction_fetch() { let tx = TransactionBuilder::default() @@ -94,7 +329,13 @@ fn test_chain_fork_by_total_difficulty() { let mut parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); for _ in 1..final_number { let difficulty = parent.difficulty().clone(); - let new_block = gen_block(&parent, difficulty + U256::from(100u64), vec![], vec![], vec![]); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + vec![], + vec![], + vec![], + ); chain1.push(new_block.clone()); parent = new_block.header().clone(); } @@ -142,7 +383,13 @@ fn test_chain_fork_by_hash() { let mut parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); for _ in 1..final_number { let difficulty = parent.difficulty().clone(); - let new_block = gen_block(&parent, difficulty + U256::from(100u64), vec![], vec![], vec![]); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + vec![], + vec![], + vec![], + ); chain1.push(new_block.clone()); parent = new_block.header().clone(); } @@ -204,7 +451,13 @@ fn test_chain_get_ancestor() { let mut parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); for _ in 1..final_number { let difficulty = parent.difficulty().clone(); - let new_block = gen_block(&parent, difficulty + U256::from(100u64), vec![], vec![], vec![]); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + vec![], + vec![], + vec![], + ); chain1.push(new_block.clone()); parent = new_block.header().clone(); } diff --git a/chain/src/tests/util.rs b/chain/src/tests/util.rs index 3a81ab7310..8fc7b4ac5b 100644 --- a/chain/src/tests/util.rs +++ b/chain/src/tests/util.rs @@ -3,9 +3,8 @@ use ckb_chain_spec::consensus::Consensus; use ckb_core::block::Block; use ckb_core::block::BlockBuilder; use ckb_core::header::{Header, HeaderBuilder}; -use ckb_core::transaction::{ - CellInput, CellOutput, OutPoint, Transaction, TransactionBuilder, -}; +use ckb_core::script::Script; +use ckb_core::transaction::{CellInput, CellOutput, OutPoint, Transaction, TransactionBuilder}; use ckb_core::uncle::UncleBlock; use ckb_core::BlockNumber; use ckb_db::memorydb::MemoryKeyValueDB; @@ -17,6 +16,9 @@ use faketime::unix_time_as_millis; use numext_fixed_hash::H256; use numext_fixed_uint::U256; use rand; +use std::fs::File; +use std::io::Read; +use std::path::Path; pub(crate) fn start_chain( consensus: Option, @@ -38,7 +40,12 @@ pub(crate) fn start_chain( fn create_cellbase(number: BlockNumber) -> Transaction { TransactionBuilder::default() .input(CellInput::new_cellbase_input(number)) - .output(CellOutput::new(0, vec![], H256::zero(), None)) + .output(CellOutput::new( + 5000, + vec![], + create_script().type_hash(), + None, + )) .build() } @@ -63,17 +70,34 @@ pub(crate) fn gen_block( .commit_transaction(cellbase) .commit_transactions(commit_transactions) .uncles(uncles) - .proposal_transactions(proposal_transactions.iter().map(|tx| tx.proposal_short_id()).collect()) + .proposal_transactions( + proposal_transactions + .iter() + .map(|tx| tx.proposal_short_id()) + .collect(), + ) .with_header_builder(header_builder) } -pub(crate) fn create_transaction(parent: H256) -> Transaction { - let mut output = CellOutput::default(); - output.capacity = 100_000_000 / 100 as u64; - let outputs: Vec = vec![output.clone(); 100]; - +pub(crate) fn create_transaction(parent: H256, unique_data: u8) -> Transaction { + let script = create_script(); TransactionBuilder::default() - .input(CellInput::new(OutPoint::new(parent, 0), Default::default())) - .outputs(outputs) + .output(CellOutput::new( + 5000, + vec![unique_data], + script.type_hash(), + None, + )) + .input(CellInput::new(OutPoint::new(parent, 0), script)) .build() } + +fn create_script() -> Script { + let mut file = File::open( + Path::new(env!("CARGO_MANIFEST_DIR")).join("../nodes_template/spec/cells/always_success"), + ) + .unwrap(); + let mut buffer = Vec::new(); + file.read_to_end(&mut buffer).unwrap(); + Script::new(0, Vec::new(), None, Some(buffer), Vec::new()) +} diff --git a/core/src/cell.rs b/core/src/cell.rs index 06f567c111..40b11ce4b0 100644 --- a/core/src/cell.rs +++ b/core/src/cell.rs @@ -1,5 +1,6 @@ use crate::block::Block; use crate::transaction::{CellOutput, OutPoint, Transaction}; +use crate::Capacity; use fnv::{FnvHashMap, FnvHashSet}; use numext_fixed_hash::H256; use std::iter::Chain; @@ -178,6 +179,24 @@ impl ResolvedTransaction { pub fn is_fully_resolved(&self) -> bool { self.cells_iter().all(|state| state.is_live()) } + + pub fn fee(&self) -> Capacity { + self.inputs_capacity() + .saturating_sub(self.transaction.outputs_capacity()) + } + + pub fn inputs_capacity(&self) -> Capacity { + self.input_cells + .iter() + .filter_map(|cell_status| { + if let CellStatus::Live(cell_output) = cell_status { + Some(cell_output.capacity) + } else { + None + } + }) + .sum() + } } #[cfg(test)] diff --git a/core/src/transaction.rs b/core/src/transaction.rs index d35d1d861c..c0b7b89d05 100644 --- a/core/src/transaction.rs +++ b/core/src/transaction.rs @@ -297,6 +297,10 @@ impl Transaction { pub fn get_output(&self, i: usize) -> Option { self.outputs.get(i).cloned() } + + pub fn outputs_capacity(&self) -> Capacity { + self.outputs.iter().map(|output| output.capacity).sum() + } } #[derive(Default)] diff --git a/shared/src/chain_state.rs b/shared/src/chain_state.rs index 366cde5b07..153bf17ed5 100644 --- a/shared/src/chain_state.rs +++ b/shared/src/chain_state.rs @@ -327,20 +327,12 @@ impl ChainState { impl CellProvider for ChainState { fn cell(&self, out_point: &OutPoint) -> CellStatus { - if let Some(f) = self.is_dead(out_point) { - if f { - CellStatus::Dead - } else { - match self.store.get_transaction(&out_point.hash) { - Some(tx) => match tx.outputs().get(out_point.index as usize) { - Some(output) => CellStatus::Live(output.clone()), - None => CellStatus::Unknown, - }, - None => CellStatus::Unknown, - } - } - } else { - CellStatus::Unknown + match self.store.get_transaction(&out_point.hash) { + Some(tx) => match tx.outputs().get(out_point.index as usize) { + Some(output) => CellStatus::Live(output.clone()), + None => CellStatus::Unknown, + }, + None => CellStatus::Unknown, } } } diff --git a/shared/src/error.rs b/shared/src/error.rs index 9d8c32a0f0..1f73d9002c 100644 --- a/shared/src/error.rs +++ b/shared/src/error.rs @@ -8,7 +8,9 @@ pub enum SharedError { #[fail(display = "InvalidOutput")] InvalidOutput, #[fail(display = "InvalidTransaction")] - InvalidTransaction, + InvalidTransaction(String), + #[fail(display = "InvalidParentBlock")] + InvalidParentBlock, #[fail(display = "DB error: {}", _0)] DB(DBError), } diff --git a/verification/src/block_verifier.rs b/verification/src/block_verifier.rs index 54754add0b..d1f40bba98 100644 --- a/verification/src/block_verifier.rs +++ b/verification/src/block_verifier.rs @@ -389,10 +389,20 @@ impl TransactionsVerifier { &self, txs_verify_cache: &mut LruCache, resolved: &[ResolvedTransaction], + block_reward: Capacity, ) -> Result<(), Error> { + // verify cellbase reward + let cellbase = &resolved[0]; + let fee: Capacity = resolved.iter().skip(1).map(|rt| rt.fee()).sum(); + if cellbase.transaction.outputs_capacity() > block_reward + fee { + return Err(Error::Cellbase(CellbaseError::InvalidReward)); + } + // TODO use TransactionScriptsVerifier to verify cellbase script + // make verifiers orthogonal let cycles_set = resolved .par_iter() + .skip(1) .enumerate() .map(|(index, tx)| { if let Some(cycles) = txs_verify_cache.get(&tx.transaction.hash()) { diff --git a/verification/src/transaction_verifier.rs b/verification/src/transaction_verifier.rs index 845b66b7b0..4205874a5f 100644 --- a/verification/src/transaction_verifier.rs +++ b/verification/src/transaction_verifier.rs @@ -32,10 +32,9 @@ impl<'a> TransactionVerifier<'a> { self.version.verify()?; self.empty.verify()?; self.null.verify()?; + self.inputs.verify()?; self.capacity.verify()?; self.duplicate_inputs.verify()?; - // InputVerifier should be executed before ScriptVerifier - self.inputs.verify()?; let cycles = self.script.verify(max_cycles)?; Ok(cycles) } From cd853373dac86395c3310f8c92e5230b4d89f877 Mon Sep 17 00:00:00 2001 From: quake wang Date: Fri, 22 Mar 2019 11:52:03 +0900 Subject: [PATCH 06/13] chore: remove some unused test case and fix clippy --- chain/src/tests/basic.rs | 8 +-- verification/src/tests/block_verifier.rs | 81 ------------------------ 2 files changed, 4 insertions(+), 85 deletions(-) diff --git a/chain/src/tests/basic.rs b/chain/src/tests/basic.rs index 94ae6ec3ee..64151db291 100644 --- a/chain/src/tests/basic.rs +++ b/chain/src/tests/basic.rs @@ -188,9 +188,9 @@ fn test_transaction_conflict_in_same_block() { ); chain.push(new_block); } - for i in 0..3 { + for block in chain.iter().take(3) { chain_controller - .process_block(Arc::new(chain[i].clone())) + .process_block(Arc::new(block.clone())) .expect("process block ok"); } assert_eq!( @@ -275,9 +275,9 @@ fn test_transaction_conflict_in_different_blocks() { ); chain.push(new_block); } - for i in 0..4 { + for block in chain.iter().take(4) { chain_controller - .process_block(Arc::new(chain[i].clone())) + .process_block(Arc::new(block.clone())) .expect("process block ok"); } assert_eq!( diff --git a/verification/src/tests/block_verifier.rs b/verification/src/tests/block_verifier.rs index 5d7cd96100..46b7e258f8 100644 --- a/verification/src/tests/block_verifier.rs +++ b/verification/src/tests/block_verifier.rs @@ -153,87 +153,6 @@ pub fn test_cellbase_with_more_reward_than_available() { ); } -#[test] -pub fn test_cellbase_with_invalid_transaction() { - let mut transaction_fees = HashMap::>::new(); - let transaction = create_normal_transaction(); - transaction_fees.insert(transaction.hash().clone(), Err(SharedError::InvalidOutput)); - - let block = BlockBuilder::default() - .commit_transaction(create_cellbase_transaction_with_capacity(100)) - .commit_transaction(transaction) - .build(); - - let provider = DummyChainProvider { - block_reward: 100, - transaction_fees, - }; - - let verifier = CellbaseVerifier::new(provider); - assert_eq!( - verifier.verify(&block), - Err(VerifyError::Chain(format!( - "{}", - SharedError::InvalidOutput - ))) - ); -} - -#[test] -pub fn test_cellbase_with_two_outputs() { - let mut transaction_fees = HashMap::>::new(); - let transaction = create_normal_transaction(); - transaction_fees.insert(transaction.hash().clone(), Ok(0)); - - let cellbase_transaction = TransactionBuilder::default() - .input(CellInput::new_cellbase_input(0)) - .output(CellOutput::new(100, Vec::new(), H256::default(), None)) - .output(CellOutput::new(50, Vec::new(), H256::default(), None)) - .build(); - - let block = BlockBuilder::default() - .commit_transaction(cellbase_transaction) - .commit_transaction(transaction) - .build(); - - let provider = DummyChainProvider { - block_reward: 150, - transaction_fees, - }; - - let verifier = CellbaseVerifier::new(provider); - assert!(verifier.verify(&block).is_ok()); -} - -#[test] -pub fn test_cellbase_with_two_outputs_and_more_rewards_than_maximum() { - let mut transaction_fees = HashMap::>::new(); - let transaction = create_normal_transaction(); - transaction_fees.insert(transaction.hash().clone(), Ok(0)); - - let cellbase_transaction = TransactionBuilder::default() - .input(CellInput::new_cellbase_input(0)) - .output(CellOutput::new(100, Vec::new(), H256::default(), None)) - .output(CellOutput::new(50, Vec::new(), H256::default(), None)) - .build(); - - let block = BlockBuilder::default() - .commit_transaction(cellbase_transaction) - .commit_transaction(transaction) - .build(); - - let provider = DummyChainProvider { - block_reward: 100, - transaction_fees, - }; - - let verifier = CellbaseVerifier::new(provider); - assert_eq!( - verifier.verify(&block), - Err(VerifyError::Cellbase(CellbaseError::InvalidReward)) - ); -} - #[test] pub fn test_empty_transactions() { let block = BlockBuilder::default().build(); From 948bb60ad6d0c79b80cc128fa2cba5e7ea95f93a Mon Sep 17 00:00:00 2001 From: quake wang Date: Fri, 22 Mar 2019 12:06:34 +0900 Subject: [PATCH 07/13] test: add delay verify test --- chain/src/tests/delay_verify.rs | 200 ++++++++++++++++++++++++++++++++ chain/src/tests/mod.rs | 1 + 2 files changed, 201 insertions(+) create mode 100644 chain/src/tests/delay_verify.rs diff --git a/chain/src/tests/delay_verify.rs b/chain/src/tests/delay_verify.rs new file mode 100644 index 0000000000..d6963c04d1 --- /dev/null +++ b/chain/src/tests/delay_verify.rs @@ -0,0 +1,200 @@ +use crate::tests::util::{create_transaction, gen_block, start_chain}; +use ckb_core::block::Block; +use ckb_traits::ChainProvider; +use numext_fixed_uint::U256; +use std::sync::Arc; + +#[test] +fn test_dead_cell_in_same_block() { + let (chain_controller, shared) = start_chain(None, true); + let final_number = 20; + let switch_fork_number = 10; + + let mut chain1: Vec = Vec::new(); + let mut chain2: Vec = Vec::new(); + + let mut parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); + for _ in 1..final_number { + let difficulty = parent.difficulty().clone(); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + vec![], + vec![], + vec![], + ); + chain1.push(new_block.clone()); + parent = new_block.header().clone(); + } + + parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); + for _ in 1..switch_fork_number { + let difficulty = parent.difficulty().clone(); + let new_block = gen_block( + &parent, + difficulty + U256::from(99u64), + vec![], + vec![], + vec![], + ); + chain2.push(new_block.clone()); + parent = new_block.header().clone(); + } + + let last_cell_base = &chain2.last().unwrap().commit_transactions()[0]; + let tx1 = create_transaction(last_cell_base.hash(), 1); + let tx2 = create_transaction(tx1.hash(), 2); + let tx3 = create_transaction(tx1.hash(), 3); + let txs = vec![tx1, tx2, tx3]; + for i in switch_fork_number..final_number { + let difficulty = parent.difficulty().clone(); + let new_block = if i == switch_fork_number { + gen_block( + &parent, + difficulty + U256::from(20000u64), + vec![], + txs.clone(), + vec![], + ) + } else if i == switch_fork_number + 2 { + gen_block( + &parent, + difficulty + U256::from(20000u64), + txs.clone(), + vec![], + vec![], + ) + } else { + gen_block( + &parent, + difficulty + U256::from(20000u64), + vec![], + vec![], + vec![], + ) + }; + chain2.push(new_block.clone()); + parent = new_block.header().clone(); + } + + for block in &chain1 { + chain_controller + .process_block(Arc::new(block.clone())) + .expect("process block ok"); + } + + for block in chain2.iter().take(switch_fork_number + 1) { + chain_controller + .process_block(Arc::new(block.clone())) + .expect("process block ok"); + } + + assert_eq!( + "Err(InvalidTransaction(\"Transactions((2, Conflict))\"))", + format!( + "{:?}", + chain_controller.process_block(Arc::new(chain2[switch_fork_number + 1].clone())) + ) + ); +} + +#[test] +fn test_dead_cell_in_different_block() { + let (chain_controller, shared) = start_chain(None, true); + let final_number = 20; + let switch_fork_number = 10; + + let mut chain1: Vec = Vec::new(); + let mut chain2: Vec = Vec::new(); + + let mut parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); + for _ in 1..final_number { + let difficulty = parent.difficulty().clone(); + let new_block = gen_block( + &parent, + difficulty + U256::from(100u64), + vec![], + vec![], + vec![], + ); + chain1.push(new_block.clone()); + parent = new_block.header().clone(); + } + + parent = shared.block_header(&shared.block_hash(0).unwrap()).unwrap(); + for _ in 1..switch_fork_number { + let difficulty = parent.difficulty().clone(); + let new_block = gen_block( + &parent, + difficulty + U256::from(99u64), + vec![], + vec![], + vec![], + ); + chain2.push(new_block.clone()); + parent = new_block.header().clone(); + } + + let last_cell_base = &chain2.last().unwrap().commit_transactions()[0]; + let tx1 = create_transaction(last_cell_base.hash(), 1); + let tx2 = create_transaction(tx1.hash(), 2); + let tx3 = create_transaction(tx1.hash(), 3); + for i in switch_fork_number..final_number { + let difficulty = parent.difficulty().clone(); + let new_block = if i == switch_fork_number { + gen_block( + &parent, + difficulty + U256::from(20000u64), + vec![], + vec![tx1.clone(), tx2.clone(), tx3.clone()], + vec![], + ) + } else if i == switch_fork_number + 2 { + gen_block( + &parent, + difficulty + U256::from(20000u64), + vec![tx1.clone(), tx2.clone()], + vec![], + vec![], + ) + } else if i == switch_fork_number + 3 { + gen_block( + &parent, + difficulty + U256::from(20000u64), + vec![tx3.clone()], + vec![], + vec![], + ) + } else { + gen_block( + &parent, + difficulty + U256::from(20000u64), + vec![], + vec![], + vec![], + ) + }; + chain2.push(new_block.clone()); + parent = new_block.header().clone(); + } + + for block in &chain1 { + chain_controller + .process_block(Arc::new(block.clone())) + .expect("process block ok"); + } + + for block in chain2.iter().take(switch_fork_number + 2) { + chain_controller + .process_block(Arc::new(block.clone())) + .expect("process block ok"); + } + + assert_eq!( + "Err(InvalidTransaction(\"Transactions((0, Conflict))\"))", + format!( + "{:?}", + chain_controller.process_block(Arc::new(chain2[switch_fork_number + 2].clone())) + ) + ); +} diff --git a/chain/src/tests/mod.rs b/chain/src/tests/mod.rs index 757d06d3ca..969c0a2f57 100644 --- a/chain/src/tests/mod.rs +++ b/chain/src/tests/mod.rs @@ -1,3 +1,4 @@ mod basic; +mod delay_verify; mod find_fork; mod util; From c3735207f25caa4e33a8115b9677c370dd7ecb67 Mon Sep 17 00:00:00 2001 From: quake wang Date: Fri, 22 Mar 2019 12:24:05 +0900 Subject: [PATCH 08/13] chore: remove unused test case --- verification/src/tests/block_verifier.rs | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/verification/src/tests/block_verifier.rs b/verification/src/tests/block_verifier.rs index 46b7e258f8..7020930f4c 100644 --- a/verification/src/tests/block_verifier.rs +++ b/verification/src/tests/block_verifier.rs @@ -130,29 +130,6 @@ pub fn test_cellbase_with_fee() { assert!(verifier.verify(&block).is_ok()); } -#[test] -pub fn test_cellbase_with_more_reward_than_available() { - let mut transaction_fees = HashMap::>::new(); - let transaction = create_normal_transaction(); - transaction_fees.insert(transaction.hash().clone(), Ok(10)); - - let block = BlockBuilder::default() - .commit_transaction(create_cellbase_transaction_with_capacity(130)) - .commit_transaction(transaction) - .build(); - - let provider = DummyChainProvider { - block_reward: 100, - transaction_fees, - }; - - let verifier = CellbaseVerifier::new(provider); - assert_eq!( - verifier.verify(&block), - Err(VerifyError::Cellbase(CellbaseError::InvalidReward)) - ); -} - #[test] pub fn test_empty_transactions() { let block = BlockBuilder::default().build(); From 6ee407f1e80ed9211be4d9e7b8da075394cfbd6b Mon Sep 17 00:00:00 2001 From: quake wang Date: Fri, 22 Mar 2019 12:24:34 +0900 Subject: [PATCH 09/13] chore: resolve review issue --- core/src/cell.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/core/src/cell.rs b/core/src/cell.rs index 40b11ce4b0..4640f406e6 100644 --- a/core/src/cell.rs +++ b/core/src/cell.rs @@ -88,18 +88,20 @@ where } pub struct BlockCellProvider<'a> { - output_indexs: FnvHashMap, + output_indices: FnvHashMap, block: &'a Block, } impl<'a> BlockCellProvider<'a> { pub fn new(block: &'a Block) -> Self { - let mut output_indexs = FnvHashMap::default(); - for (i, tx) in block.commit_transactions().iter().enumerate() { - output_indexs.insert(tx.hash(), i); - } + let output_indices = block + .commit_transactions() + .iter() + .enumerate() + .map(|(idx, tx)| (tx.hash(), idx)) + .collect(); Self { - output_indexs, + output_indices, block, } } @@ -107,7 +109,7 @@ impl<'a> BlockCellProvider<'a> { impl<'a> CellProvider for BlockCellProvider<'a> { fn cell(&self, out_point: &OutPoint) -> CellStatus { - if let Some(i) = self.output_indexs.get(&out_point.hash) { + if let Some(i) = self.output_indices.get(&out_point.hash) { match self.block.commit_transactions()[*i] .outputs() .get(out_point.index as usize) From 581c0ecdedcea029d6de21f99fb502d34a6fa27f Mon Sep 17 00:00:00 2001 From: quake wang Date: Fri, 22 Mar 2019 12:40:54 +0900 Subject: [PATCH 10/13] rebase --- verification/src/block_verifier.rs | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/verification/src/block_verifier.rs b/verification/src/block_verifier.rs index d1f40bba98..287254767f 100644 --- a/verification/src/block_verifier.rs +++ b/verification/src/block_verifier.rs @@ -100,25 +100,7 @@ impl CellbaseVerifier { return Err(Error::Cellbase(CellbaseError::InvalidOutput)); } - let block_reward = self.provider.block_reward(block.header().number()); - let mut fee = 0; - for transaction in block.commit_transactions().iter().skip(1) { - fee += self - .provider - .calculate_transaction_fee(transaction) - .map_err(|e| Error::Chain(format!("{}", e)))?; - } - let total_reward = block_reward + fee; - let output_capacity: Capacity = cellbase_transaction - .outputs() - .iter() - .map(|output| output.capacity) - .sum(); - if output_capacity > total_reward { - Err(Error::Cellbase(CellbaseError::InvalidReward)) - } else { - Ok(()) - } + Ok(()) } } From 4a8533377c85a1594460def46a822bfd12707e3c Mon Sep 17 00:00:00 2001 From: quake wang Date: Fri, 22 Mar 2019 14:14:32 +0900 Subject: [PATCH 11/13] test: resolve backtrace assert issue --- chain/src/tests/basic.rs | 23 +++++++++++++---------- chain/src/tests/delay_verify.rs | 23 +++++++++++++---------- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/chain/src/tests/basic.rs b/chain/src/tests/basic.rs index 64151db291..e8c3ab3ab6 100644 --- a/chain/src/tests/basic.rs +++ b/chain/src/tests/basic.rs @@ -5,6 +5,7 @@ use ckb_core::block::BlockBuilder; use ckb_core::cell::CellProvider; use ckb_core::header::HeaderBuilder; use ckb_core::transaction::{CellInput, CellOutput, OutPoint, TransactionBuilder}; +use ckb_shared::error::SharedError; use ckb_traits::ChainProvider; use numext_fixed_hash::H256; use numext_fixed_uint::U256; @@ -194,11 +195,12 @@ fn test_transaction_conflict_in_same_block() { .expect("process block ok"); } assert_eq!( - "Err(InvalidTransaction(\"Transactions((2, Conflict))\"))", - format!( - "{:?}", - chain_controller.process_block(Arc::new(chain[3].clone())) - ) + SharedError::InvalidTransaction("Transactions((2, Conflict))".to_string()), + chain_controller + .process_block(Arc::new(chain[3].clone())) + .unwrap_err() + .downcast() + .unwrap() ); } @@ -281,11 +283,12 @@ fn test_transaction_conflict_in_different_blocks() { .expect("process block ok"); } assert_eq!( - "Err(InvalidTransaction(\"Transactions((0, Conflict))\"))", - format!( - "{:?}", - chain_controller.process_block(Arc::new(chain[4].clone())) - ) + SharedError::InvalidTransaction("Transactions((0, Conflict))".to_string()), + chain_controller + .process_block(Arc::new(chain[4].clone())) + .unwrap_err() + .downcast() + .unwrap() ); } diff --git a/chain/src/tests/delay_verify.rs b/chain/src/tests/delay_verify.rs index d6963c04d1..4395ae9fa9 100644 --- a/chain/src/tests/delay_verify.rs +++ b/chain/src/tests/delay_verify.rs @@ -1,5 +1,6 @@ use crate::tests::util::{create_transaction, gen_block, start_chain}; use ckb_core::block::Block; +use ckb_shared::error::SharedError; use ckb_traits::ChainProvider; use numext_fixed_uint::U256; use std::sync::Arc; @@ -90,11 +91,12 @@ fn test_dead_cell_in_same_block() { } assert_eq!( - "Err(InvalidTransaction(\"Transactions((2, Conflict))\"))", - format!( - "{:?}", - chain_controller.process_block(Arc::new(chain2[switch_fork_number + 1].clone())) - ) + SharedError::InvalidTransaction("Transactions((2, Conflict))".to_string()), + chain_controller + .process_block(Arc::new(chain2[switch_fork_number + 1].clone())) + .unwrap_err() + .downcast() + .unwrap() ); } @@ -191,10 +193,11 @@ fn test_dead_cell_in_different_block() { } assert_eq!( - "Err(InvalidTransaction(\"Transactions((0, Conflict))\"))", - format!( - "{:?}", - chain_controller.process_block(Arc::new(chain2[switch_fork_number + 2].clone())) - ) + SharedError::InvalidTransaction("Transactions((0, Conflict))".to_string()), + chain_controller + .process_block(Arc::new(chain2[switch_fork_number + 2].clone())) + .unwrap_err() + .downcast() + .unwrap() ); } From 3b958767926a1152cecd618c743aa7a643f128fa Mon Sep 17 00:00:00 2001 From: quake wang Date: Fri, 22 Mar 2019 22:23:47 +0900 Subject: [PATCH 12/13] chore: resolve review comment --- shared/src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/src/error.rs b/shared/src/error.rs index 1f73d9002c..e374b686c7 100644 --- a/shared/src/error.rs +++ b/shared/src/error.rs @@ -7,7 +7,7 @@ pub enum SharedError { InvalidInput, #[fail(display = "InvalidOutput")] InvalidOutput, - #[fail(display = "InvalidTransaction")] + #[fail(display = "InvalidTransaction: {}", _0)] InvalidTransaction(String), #[fail(display = "InvalidParentBlock")] InvalidParentBlock, From 7018ec2b62871123ad01ecf6ab24650623e2883a Mon Sep 17 00:00:00 2001 From: quake wang Date: Mon, 25 Mar 2019 13:59:43 +0900 Subject: [PATCH 13/13] refactor: resolve review comment issue --- Cargo.lock | 1 - chain/Cargo.toml | 1 - chain/src/chain.rs | 10 ++++------ chain/src/tests/util.rs | 4 +--- shared/src/cell_set.rs | 22 ++++++++++++++-------- shared/src/shared.rs | 2 +- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d6878be97a..1c21024d4c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -351,7 +351,6 @@ dependencies = [ "lru-cache 0.1.0 (git+https://github.com/nervosnetwork/lru-cache)", "numext-fixed-hash 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "numext-fixed-uint 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", - "rand 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.89 (registry+https://github.com/rust-lang/crates.io-index)", "stop-handler 0.7.0-pre", "tempfile 3.0.7 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/chain/Cargo.toml b/chain/Cargo.toml index f6ab75bd31..8126ab43f4 100644 --- a/chain/Cargo.toml +++ b/chain/Cargo.toml @@ -27,4 +27,3 @@ failure = "0.1.5" [dev-dependencies] env_logger = "0.6" tempfile = "3.0" -rand = "0.6" diff --git a/chain/src/chain.rs b/chain/src/chain.rs index 2fb65c351a..d91be481dc 100644 --- a/chain/src/chain.rs +++ b/chain/src/chain.rs @@ -505,13 +505,11 @@ impl ChainService { .insert_block_ext(batch, &b.header().hash(), ext); } - if found_error.is_some() { - Err(SharedError::InvalidTransaction( - found_error.unwrap().to_string(), - ))?; + if let Some(err) = found_error { + Err(SharedError::InvalidTransaction(err.to_string()))? + } else { + Ok(cell_set_diff) } - - Ok(cell_set_diff) } // TODO: beatify diff --git a/chain/src/tests/util.rs b/chain/src/tests/util.rs index 8fc7b4ac5b..14517b5134 100644 --- a/chain/src/tests/util.rs +++ b/chain/src/tests/util.rs @@ -15,7 +15,6 @@ use ckb_shared::store::ChainKVStore; use faketime::unix_time_as_millis; use numext_fixed_hash::H256; use numext_fixed_uint::U256; -use rand; use std::fs::File; use std::io::Read; use std::path::Path; @@ -63,8 +62,7 @@ pub(crate) fn gen_block( .timestamp(unix_time_as_millis()) .number(number) .difficulty(difficulty) - .cellbase_id(cellbase.hash()) - .nonce(rand::random()); + .cellbase_id(cellbase.hash()); BlockBuilder::default() .commit_transaction(cellbase) diff --git a/shared/src/cell_set.rs b/shared/src/cell_set.rs index f280fe329a..d42d1cf972 100644 --- a/shared/src/cell_set.rs +++ b/shared/src/cell_set.rs @@ -66,9 +66,8 @@ impl CellSet { self.inner.get(h) } - pub fn insert(&mut self, hash: &H256, outputs_len: usize) { - self.inner - .insert(hash.clone(), TransactionMeta::new(outputs_len)); + pub fn insert(&mut self, hash: H256, outputs_len: usize) { + self.inner.insert(hash, TransactionMeta::new(outputs_len)); } pub fn remove(&mut self, hash: &H256) -> Option { @@ -88,19 +87,26 @@ impl CellSet { } pub fn update(&mut self, diff: CellSetDiff) { - diff.old_outputs.iter().for_each(|h| { + let CellSetDiff { + old_inputs, + old_outputs, + new_inputs, + new_outputs, + } = diff; + + old_outputs.iter().for_each(|h| { self.remove(h); }); - diff.old_inputs.iter().for_each(|o| { + old_inputs.iter().for_each(|o| { self.mark_live(o); }); - diff.new_outputs.iter().for_each(|(hash, len)| { - self.insert(hash, *len); + new_outputs.into_iter().for_each(|(hash, len)| { + self.insert(hash, len); }); - diff.new_inputs.iter().for_each(|o| { + new_inputs.iter().for_each(|o| { self.mark_dead(o); }); } diff --git a/shared/src/shared.rs b/shared/src/shared.rs index e530c8f888..7942986dec 100644 --- a/shared/src/shared.rs +++ b/shared/src/shared.rs @@ -140,7 +140,7 @@ impl Shared { cell_set.mark_dead(&o); } - cell_set.insert(&tx.hash(), output_len); + cell_set.insert(tx.hash(), output_len); } }