From 083e995492ff67b74a138f2ad564f33e17335217 Mon Sep 17 00:00:00 2001 From: zhangsoledad <787953403@qq.com> Date: Thu, 16 Jul 2020 15:59:04 +0800 Subject: [PATCH 1/3] refactor: rename ContextualTransactionVerifier -> TimeRelativeTransactionVerifier --- tx-pool/src/pool.rs | 4 ++-- tx-pool/src/process.rs | 4 ++-- verification/src/contextual_block_verifier.rs | 6 +++--- verification/src/lib.rs | 2 +- verification/src/transaction_verifier.rs | 6 +++--- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/tx-pool/src/pool.rs b/tx-pool/src/pool.rs index 06362938a4..3cacb542f4 100644 --- a/tx-pool/src/pool.rs +++ b/tx-pool/src/pool.rs @@ -19,7 +19,7 @@ use ckb_types::{ packed::{Byte32, OutPoint, ProposalShortId}, }; use ckb_verification::cache::CacheEntry; -use ckb_verification::{ContextualTransactionVerifier, TransactionVerifier}; +use ckb_verification::{TimeRelativeTransactionVerifier, TransactionVerifier}; use faketime::unix_time_as_millis; use lru_cache::LruCache; use std::collections::HashMap; @@ -341,7 +341,7 @@ impl TxPool { match cache_entry { Some(cache_entry) => { - ContextualTransactionVerifier::new( + TimeRelativeTransactionVerifier::new( &rtx, snapshot, tip_number + 1, diff --git a/tx-pool/src/process.rs b/tx-pool/src/process.rs index 79f3513a85..3b1350e879 100644 --- a/tx-pool/src/process.rs +++ b/tx-pool/src/process.rs @@ -24,7 +24,7 @@ use ckb_types::{ prelude::*, }; use ckb_util::LinkedHashSet; -use ckb_verification::{cache::CacheEntry, ContextualTransactionVerifier, TransactionVerifier}; +use ckb_verification::{cache::CacheEntry, TimeRelativeTransactionVerifier, TransactionVerifier}; use failure::Error as FailureError; use faketime::unix_time_as_millis; use std::collections::HashSet; @@ -593,7 +593,7 @@ fn verify_rtxs( .map(|tx| { let tx_hash = tx.transaction.hash(); if let Some(cache_entry) = txs_verify_cache.get(&tx_hash) { - ContextualTransactionVerifier::new( + TimeRelativeTransactionVerifier::new( &tx, snapshot, tip_number + 1, diff --git a/verification/src/contextual_block_verifier.rs b/verification/src/contextual_block_verifier.rs index 2ff6e94cda..c0ac17c5d8 100644 --- a/verification/src/contextual_block_verifier.rs +++ b/verification/src/contextual_block_verifier.rs @@ -2,8 +2,8 @@ use crate::cache::{CacheEntry, TxVerifyCache}; use crate::error::{BlockTransactionsError, EpochError}; use crate::uncles_verifier::{UncleProvider, UnclesVerifier}; use crate::{ - BlockErrorKind, CellbaseError, CommitError, ContextualTransactionVerifier, TransactionVerifier, - UnknownParentError, + BlockErrorKind, CellbaseError, CommitError, TimeRelativeTransactionVerifier, + TransactionVerifier, UnknownParentError, }; use ckb_async_runtime::Handle; use ckb_chain_spec::consensus::Consensus; @@ -392,7 +392,7 @@ impl<'a, CS: ChainStore<'a>> BlockTxsVerifier<'a, CS> { .map(|(index, tx)| { let tx_hash = tx.transaction.hash(); if let Some(cache_entry) = fetched_cache.get(&tx_hash) { - ContextualTransactionVerifier::new( + TimeRelativeTransactionVerifier::new( &tx, self.context, self.block_number, diff --git a/verification/src/lib.rs b/verification/src/lib.rs index 6e5884341c..0c74981cfd 100644 --- a/verification/src/lib.rs +++ b/verification/src/lib.rs @@ -24,7 +24,7 @@ pub use crate::error::{ pub use crate::genesis_verifier::GenesisVerifier; pub use crate::header_verifier::{HeaderResolver, HeaderVerifier}; pub use crate::transaction_verifier::{ - ContextualTransactionVerifier, ScriptVerifier, Since, SinceMetric, TransactionVerifier, + ScriptVerifier, Since, SinceMetric, TimeRelativeTransactionVerifier, TransactionVerifier, }; pub const ALLOWED_FUTURE_BLOCKTIME: u64 = 15 * 1000; // 15 Second diff --git a/verification/src/transaction_verifier.rs b/verification/src/transaction_verifier.rs index 524395c4ae..0d7062880d 100644 --- a/verification/src/transaction_verifier.rs +++ b/verification/src/transaction_verifier.rs @@ -20,12 +20,12 @@ use lru_cache::LruCache; use std::cell::RefCell; use std::collections::HashSet; -pub struct ContextualTransactionVerifier<'a, M> { +pub struct TimeRelativeTransactionVerifier<'a, M> { pub maturity: MaturityVerifier<'a>, pub since: SinceVerifier<'a, M>, } -impl<'a, M> ContextualTransactionVerifier<'a, M> +impl<'a, M> TimeRelativeTransactionVerifier<'a, M> where M: BlockMedianTimeContext, { @@ -37,7 +37,7 @@ where parent_hash: Byte32, consensus: &'a Consensus, ) -> Self { - ContextualTransactionVerifier { + TimeRelativeTransactionVerifier { maturity: MaturityVerifier::new( &rtx, epoch_number_with_fraction, From 392511c06d674649f27e770a2a73187306ad244d Mon Sep 17 00:00:00 2001 From: zhangsoledad <787953403@qq.com> Date: Fri, 17 Jul 2020 04:30:50 +0800 Subject: [PATCH 2/3] refactor: split TransactionVerifier --- error/src/internal.rs | 6 -- rpc/src/error.rs | 45 +++++++------ test/src/specs/tx_pool/limit.rs | 4 +- tx-pool/src/component/container.rs | 6 +- tx-pool/src/component/pending.rs | 4 +- tx-pool/src/component/proposed.rs | 4 +- tx-pool/src/error.rs | 20 +++++- tx-pool/src/pool.rs | 18 ++--- tx-pool/src/process.rs | 32 +++++++-- tx-pool/src/service.rs | 6 ++ util/snapshot/src/lib.rs | 4 ++ verification/src/lib.rs | 3 +- verification/src/transaction_verifier.rs | 86 +++++++++++++++++++----- 13 files changed, 166 insertions(+), 72 deletions(-) diff --git a/error/src/internal.rs b/error/src/internal.rs index bf05383bab..17c10791ef 100644 --- a/error/src/internal.rs +++ b/error/src/internal.rs @@ -13,12 +13,6 @@ pub enum InternalErrorKind { /// e.g. `Capacity::safe_add` CapacityOverflow, - /// The transaction_pool is already full - TransactionPoolFull, - - /// The transaction already exist in transaction_pool - PoolTransactionDuplicated, - /// Persistent data had corrupted DataCorrupted, diff --git a/rpc/src/error.rs b/rpc/src/error.rs index fc407e2094..55f1a8c8eb 100644 --- a/rpc/src/error.rs +++ b/rpc/src/error.rs @@ -1,5 +1,5 @@ use ckb_error::{Error as CKBError, InternalError, InternalErrorKind}; -use ckb_tx_pool::error::SubmitTxError; +use ckb_tx_pool::error::Reject; use jsonrpc_core::{Error, ErrorCode, Value}; use std::fmt::{Debug, Display}; @@ -33,6 +33,7 @@ pub enum RPCError { PoolRejectedTransactionByMaxAncestorsCountLimit = -1105, PoolIsFull = -1106, PoolRejectedDuplicatedTransaction = -1107, + PoolRejectedMalformedTransaction = -1108, } impl RPCError { @@ -82,21 +83,22 @@ impl RPCError { err.unwrap_cause_or_self(), ), SubmitTransaction => { - let submit_tx_err = match err.downcast_ref::() { + let reject = match err.downcast_ref::() { Some(err) => err, None => return Self::ckb_internal_error(err), }; - let kind = match *submit_tx_err { - SubmitTxError::LowFeeRate(_, _) => { - RPCError::PoolRejectedTransactionByMinFeeRate - } - SubmitTxError::ExceededMaximumAncestorsCount => { + let kind = match *reject { + Reject::LowFeeRate(_, _) => RPCError::PoolRejectedTransactionByMinFeeRate, + Reject::ExceededMaximumAncestorsCount => { RPCError::PoolRejectedTransactionByMaxAncestorsCountLimit } + Reject::Full(_, _) => RPCError::PoolIsFull, + Reject::Duplicated(_) => RPCError::PoolRejectedDuplicatedTransaction, + Reject::Malformed(_) => RPCError::PoolRejectedMalformedTransaction, }; - RPCError::custom_with_error(kind, submit_tx_err) + RPCError::custom_with_error(kind, reject) } Internal => { let internal_err = match err.downcast_ref::() { @@ -106,10 +108,6 @@ impl RPCError { let kind = match internal_err.kind() { InternalErrorKind::CapacityOverflow => RPCError::IntegerOverflow, - InternalErrorKind::TransactionPoolFull => RPCError::PoolIsFull, - InternalErrorKind::PoolTransactionDuplicated => { - RPCError::PoolRejectedDuplicatedTransaction - } InternalErrorKind::DataCorrupted => RPCError::DatabaseIsCorrupt, InternalErrorKind::Database => RPCError::DatabaseError, InternalErrorKind::Config => RPCError::ConfigError, @@ -162,24 +160,33 @@ mod tests { #[test] fn test_submit_tx_error_from_ckb_error() { - let err: CKBError = SubmitTxError::LowFeeRate(100, 50).into(); + let err: CKBError = Reject::LowFeeRate(100, 50).into(); assert_eq!( "PoolRejectedTransactionByMinFeeRate: Transaction fee rate must >= 100 shannons/KB, got: 50", RPCError::from_ckb_error(err).message ); - let err: CKBError = SubmitTxError::ExceededMaximumAncestorsCount.into(); + let err: CKBError = Reject::ExceededMaximumAncestorsCount.into(); assert_eq!( "PoolRejectedTransactionByMaxAncestorsCountLimit: Transaction exceeded maximum ancestors count limit, try send it later", RPCError::from_ckb_error(err).message ); - } - #[test] - fn test_internal_error_from_ckb_error() { - let err: CKBError = InternalErrorKind::TransactionPoolFull.into(); + let err: CKBError = Reject::Full("size".to_owned(), 10).into(); + assert_eq!( + "PoolIsFull: Transaction pool exceeded maximum size limit(10), try send it later", + RPCError::from_ckb_error(err).message + ); + + let err: CKBError = Reject::Duplicated(Byte32::new([0; 32])).into(); + assert_eq!( + "PoolRejectedDuplicatedTransaction: Transaction(Byte32(0x0000000000000000000000000000000000000000000000000000000000000000)) already exist in transaction_pool", + RPCError::from_ckb_error(err).message + ); + + let err: CKBError = Reject::Malformed("cellbase like".to_owned()).into(); assert_eq!( - "PoolIsFull: TransactionPoolFull", + "PoolRejectedMalformedTransaction: Malformed cellbase like transaction", RPCError::from_ckb_error(err).message ); } diff --git a/test/src/specs/tx_pool/limit.rs b/test/src/specs/tx_pool/limit.rs index df2e2f3649..15264cd2c9 100644 --- a/test/src/specs/tx_pool/limit.rs +++ b/test/src/specs/tx_pool/limit.rs @@ -48,7 +48,7 @@ impl Spec for SizeLimit { info!("The next tx reach size limit"); let tx = node.new_transaction(hash); - assert_send_transaction_fail(node, &tx, "TransactionPoolFull"); + assert_send_transaction_fail(node, &tx, "Transaction pool exceeded maximum size limit"); node.assert_tx_pool_serialized_size(max_tx_num * one_tx_size); (0..DEFAULT_TX_PROPOSAL_WINDOW.0).for_each(|_| { @@ -111,7 +111,7 @@ impl Spec for CyclesLimit { info!("The next tx reach cycles limit"); let tx = node.new_transaction(hash); - assert_send_transaction_fail(node, &tx, "TransactionPoolFull"); + assert_send_transaction_fail(node, &tx, "Transaction pool exceeded maximum cycles limit"); node.assert_tx_pool_cycles(max_tx_num * one_tx_cycles); (0..DEFAULT_TX_PROPOSAL_WINDOW.0).for_each(|_| { diff --git a/tx-pool/src/component/container.rs b/tx-pool/src/component/container.rs index d20d2a6369..9d7d41086c 100644 --- a/tx-pool/src/component/container.rs +++ b/tx-pool/src/component/container.rs @@ -1,7 +1,7 @@ //! The primary module containing the implementations of the transaction pool //! and its top-level members. -use crate::{component::entry::TxEntry, error::SubmitTxError}; +use crate::{component::entry::TxEntry, error::Reject}; use ckb_types::{core::Capacity, packed::ProposalShortId}; use std::cmp::Ordering; use std::collections::{BTreeSet, HashMap, HashSet, VecDeque}; @@ -161,7 +161,7 @@ impl SortedTxMap { } } - pub fn add_entry(&mut self, mut entry: TxEntry) -> Result, SubmitTxError> { + pub fn add_entry(&mut self, mut entry: TxEntry) -> Result, Reject> { let short_id = entry.transaction.proposal_short_id(); // find in pool parents @@ -185,7 +185,7 @@ impl SortedTxMap { self.update_ancestors_stat_for_entry(&mut entry, &parents); if entry.ancestors_count > self.max_ancestors_count { - return Err(SubmitTxError::ExceededMaximumAncestorsCount); + return Err(Reject::ExceededMaximumAncestorsCount); } // check duplicate tx diff --git a/tx-pool/src/component/pending.rs b/tx-pool/src/component/pending.rs index 1ec181cea1..8a1f7f2b88 100644 --- a/tx-pool/src/component/pending.rs +++ b/tx-pool/src/component/pending.rs @@ -1,6 +1,6 @@ use crate::component::container::{AncestorsScoreSortKey, SortedTxMap}; use crate::component::entry::TxEntry; -use crate::error::SubmitTxError; +use crate::error::Reject; use ckb_fee_estimator::FeeRate; use ckb_types::{ core::{ @@ -28,7 +28,7 @@ impl PendingQueue { self.inner.size() } - pub(crate) fn add_entry(&mut self, entry: TxEntry) -> Result, SubmitTxError> { + pub(crate) fn add_entry(&mut self, entry: TxEntry) -> Result, Reject> { self.inner.add_entry(entry) } diff --git a/tx-pool/src/component/proposed.rs b/tx-pool/src/component/proposed.rs index 306e55163b..582cdee582 100644 --- a/tx-pool/src/component/proposed.rs +++ b/tx-pool/src/component/proposed.rs @@ -1,6 +1,6 @@ use crate::component::container::SortedTxMap; use crate::component::entry::TxEntry; -use crate::error::SubmitTxError; +use crate::error::Reject; use ckb_types::{ bytes::Bytes, core::{ @@ -196,7 +196,7 @@ impl ProposedPool { removed } - pub(crate) fn add_entry(&mut self, entry: TxEntry) -> Result, SubmitTxError> { + pub(crate) fn add_entry(&mut self, entry: TxEntry) -> Result, Reject> { let inputs = entry.transaction.input_pts_iter(); let outputs = entry.transaction.output_pts(); diff --git a/tx-pool/src/error.rs b/tx-pool/src/error.rs index ea681dd167..182e6f7909 100644 --- a/tx-pool/src/error.rs +++ b/tx-pool/src/error.rs @@ -1,21 +1,35 @@ use ckb_error::{Error, ErrorKind}; +use ckb_types::packed::Byte32; use failure::Fail; use tokio::sync::mpsc::error::TrySendError as TokioTrySendError; #[derive(Debug, PartialEq, Clone, Eq, Fail)] -pub enum SubmitTxError { +pub enum Reject { /// The fee rate of transaction is lower than min fee rate #[fail( display = "Transaction fee rate must >= {} shannons/KB, got: {}", _0, _1 )] LowFeeRate(u64, u64), + #[fail(display = "Transaction exceeded maximum ancestors count limit, try send it later")] ExceededMaximumAncestorsCount, + + #[fail( + display = "Transaction pool exceeded maximum {} limit({}), try send it later", + _0, _1 + )] + Full(String, u64), + + #[fail(display = "Transaction({}) already exist in transaction_pool", _0)] + Duplicated(Byte32), + + #[fail(display = "Malformed {} transaction", _0)] + Malformed(String), } -impl From for Error { - fn from(error: SubmitTxError) -> Self { +impl From for Error { + fn from(error: Reject) -> Self { error.context(ErrorKind::SubmitTransaction).into() } } diff --git a/tx-pool/src/pool.rs b/tx-pool/src/pool.rs index 3cacb542f4..4ae877e868 100644 --- a/tx-pool/src/pool.rs +++ b/tx-pool/src/pool.rs @@ -3,10 +3,10 @@ use super::component::{DefectEntry, TxEntry}; use crate::component::orphan::OrphanPool; use crate::component::pending::PendingQueue; use crate::component::proposed::ProposedPool; -use crate::error::SubmitTxError; +use crate::error::Reject; use ckb_app_config::TxPoolConfig; use ckb_dao::DaoCalculator; -use ckb_error::{Error, ErrorKind, InternalErrorKind}; +use ckb_error::{Error, ErrorKind}; use ckb_logger::{debug, error, trace}; use ckb_snapshot::Snapshot; use ckb_store::ChainStore; @@ -140,7 +140,7 @@ impl TxPool { } // If did have this value present, false is returned. - pub fn add_pending(&mut self, entry: TxEntry) -> Result { + pub fn add_pending(&mut self, entry: TxEntry) -> Result { if self .gap .contains_key(&entry.transaction.proposal_short_id()) @@ -152,12 +152,12 @@ impl TxPool { } // add_gap inserts proposed but still uncommittable transaction. - pub fn add_gap(&mut self, entry: TxEntry) -> Result { + pub fn add_gap(&mut self, entry: TxEntry) -> Result { trace!("add_gap {}", entry.transaction.hash()); self.gap.add_entry(entry).map(|entry| entry.is_none()) } - pub fn add_proposed(&mut self, entry: TxEntry) -> Result { + pub fn add_proposed(&mut self, entry: TxEntry) -> Result { trace!("add_proposed {}", entry.transaction.hash()); self.touch_last_txs_updated_at(); self.proposed.add_entry(entry).map(|entry| entry.is_none()) @@ -549,9 +549,7 @@ impl TxPool { if tx_pool.add_gap(entry)? { Ok(()) } else { - Err(InternalErrorKind::PoolTransactionDuplicated - .reason(tx_hash) - .into()) + Err(Reject::Duplicated(tx_hash).into()) } }, ) @@ -609,9 +607,7 @@ impl TxPool { if tx_pool.add_pending(entry)? { Ok(()) } else { - Err(InternalErrorKind::PoolTransactionDuplicated - .reason(tx_hash) - .into()) + Err(Reject::Duplicated(tx_hash).into()) } }, ) diff --git a/tx-pool/src/process.rs b/tx-pool/src/process.rs index 3b1350e879..b9a26079cf 100644 --- a/tx-pool/src/process.rs +++ b/tx-pool/src/process.rs @@ -1,7 +1,7 @@ use crate::block_assembler::{BlockAssembler, BlockTemplateCacheKey, TemplateCache}; use crate::component::commit_txs_scanner::CommitTxsScanner; use crate::component::entry::TxEntry; -use crate::error::{BlockAssemblerError, SubmitTxError}; +use crate::error::{BlockAssemblerError, Reject}; use crate::pool::TxPool; use crate::service::TxPoolService; use ckb_app_config::BlockAssemblerConfig; @@ -24,7 +24,10 @@ use ckb_types::{ prelude::*, }; use ckb_util::LinkedHashSet; -use ckb_verification::{cache::CacheEntry, TimeRelativeTransactionVerifier, TransactionVerifier}; +use ckb_verification::{ + cache::CacheEntry, ContextualTransactionVerifier, NonContextualTransactionVerifier, + TimeRelativeTransactionVerifier, +}; use failure::Error as FailureError; use faketime::unix_time_as_millis; use std::collections::HashSet; @@ -395,13 +398,13 @@ impl TxPoolService { for ((rtx, cache_entry), (tx_size, fee, status)) in txs.into_iter().zip(status.into_iter()) { if tx_pool.reach_cycles_limit(cache_entry.cycles) { - return Err(InternalErrorKind::TransactionPoolFull.into()); + return Err(Reject::Full("cycles".to_owned(), tx_pool.config.max_cycles).into()); } let min_fee = tx_pool.config.min_fee_rate.fee(tx_size); // reject txs which fee lower than min fee rate if fee < min_fee { - return Err(SubmitTxError::LowFeeRate(min_fee.as_u64(), fee.as_u64()).into()); + return Err(Reject::LowFeeRate(min_fee.as_u64(), fee.as_u64()).into()); } let related_dep_out_points = rtx.related_dep_out_points(); @@ -424,10 +427,25 @@ impl TxPoolService { Ok(()) } + fn non_contextual_verify(&self, txs: &[TransactionView]) -> Result<(), Error> { + for tx in txs { + NonContextualTransactionVerifier::new(tx, &self.consensus).verify()?; + + // cellbase is only valid in a block, not as a loose transaction + if tx.is_cellbase() { + return Err(Reject::Malformed("cellbase like".to_owned()).into()); + } + } + Ok(()) + } + pub(crate) async fn process_txs( &self, txs: Vec, ) -> Result, Error> { + // non contextual verify first + self.non_contextual_verify(&txs)?; + let max_tx_verify_cycles = self.tx_pool_config.max_tx_verify_cycles; let (tip_hash, snapshot, rtxs, status) = self.pre_resolve_txs(&txs).await?; let fetched_cache = self.fetch_txs_verify_cache(txs.iter()).await; @@ -517,7 +535,7 @@ fn check_transaction_hash_collision( for tx in txs { let short_id = tx.proposal_short_id(); if tx_pool.contains_proposal_id(&short_id) { - return Err(InternalErrorKind::PoolTransactionDuplicated.into()); + return Err(Reject::Duplicated(tx.hash()).into()); } } Ok(()) @@ -531,7 +549,7 @@ fn resolve_tx<'a>( ) -> ResolveResult { let tx_size = tx.data().serialized_size_in_block(); if tx_pool.reach_size_limit(tx_size) { - return Err(InternalErrorKind::TransactionPoolFull.into()); + return Err(Reject::Full("size".to_owned(), tx_pool.config.max_mem_size as u64).into()); } let short_id = tx.proposal_short_id(); @@ -604,7 +622,7 @@ fn verify_rtxs( .verify() .map(|_| (tx, *cache_entry)) } else { - TransactionVerifier::new( + ContextualTransactionVerifier::new( &tx, snapshot, tip_number + 1, diff --git a/tx-pool/src/service.rs b/tx-pool/src/service.rs index ad1b52c23c..d00527c4d6 100644 --- a/tx-pool/src/service.rs +++ b/tx-pool/src/service.rs @@ -5,6 +5,7 @@ use crate::pool::{TxPool, TxPoolInfo}; use crate::process::PlugTarget; use ckb_app_config::{BlockAssemblerConfig, TxPoolConfig}; use ckb_async_runtime::{new_runtime, Handle}; +use ckb_chain_spec::consensus::Consensus; use ckb_error::Error; use ckb_jsonrpc_types::BlockTemplate; use ckb_logger::error; @@ -279,12 +280,14 @@ impl TxPoolServiceBuilder { snapshot_mgr: Arc, ) -> TxPoolServiceBuilder { let last_txs_updated_at = Arc::new(AtomicU64::new(0)); + let consensus = snapshot.cloned_consensus(); let tx_pool = TxPool::new(tx_pool_config, snapshot, Arc::clone(&last_txs_updated_at)); let block_assembler = block_assembler_config.map(BlockAssembler::new); TxPoolServiceBuilder { service: Some(TxPoolService::new( tx_pool, + consensus, block_assembler, txs_verify_cache, last_txs_updated_at, @@ -323,6 +326,7 @@ impl TxPoolServiceBuilder { #[derive(Clone)] pub struct TxPoolService { pub(crate) tx_pool: Arc>, + pub(crate) consensus: Arc, pub(crate) tx_pool_config: Arc, pub(crate) block_assembler: Option, pub(crate) txs_verify_cache: Arc>, @@ -333,6 +337,7 @@ pub struct TxPoolService { impl TxPoolService { pub fn new( tx_pool: TxPool, + consensus: Arc, block_assembler: Option, txs_verify_cache: Arc>, last_txs_updated_at: Arc, @@ -341,6 +346,7 @@ impl TxPoolService { let tx_pool_config = Arc::new(tx_pool.config); Self { tx_pool: Arc::new(RwLock::new(tx_pool)), + consensus, tx_pool_config, block_assembler, txs_verify_cache, diff --git a/util/snapshot/src/lib.rs b/util/snapshot/src/lib.rs index 85962f2967..b3e27cdf37 100644 --- a/util/snapshot/src/lib.rs +++ b/util/snapshot/src/lib.rs @@ -110,6 +110,10 @@ impl Snapshot { &self.consensus } + pub fn cloned_consensus(&self) -> Arc { + Arc::clone(&self.consensus) + } + pub fn proposals(&self) -> &ProposalView { &self.proposals } diff --git a/verification/src/lib.rs b/verification/src/lib.rs index 0c74981cfd..6575731acb 100644 --- a/verification/src/lib.rs +++ b/verification/src/lib.rs @@ -24,7 +24,8 @@ pub use crate::error::{ pub use crate::genesis_verifier::GenesisVerifier; pub use crate::header_verifier::{HeaderResolver, HeaderVerifier}; pub use crate::transaction_verifier::{ - ScriptVerifier, Since, SinceMetric, TimeRelativeTransactionVerifier, TransactionVerifier, + ContextualTransactionVerifier, NonContextualTransactionVerifier, ScriptVerifier, Since, + SinceMetric, TimeRelativeTransactionVerifier, TransactionVerifier, }; pub const ALLOWED_FUTURE_BLOCKTIME: u64 = 15 * 1000; // 15 Second diff --git a/verification/src/transaction_verifier.rs b/verification/src/transaction_verifier.rs index 0d7062880d..d2d58682b4 100644 --- a/verification/src/transaction_verifier.rs +++ b/verification/src/transaction_verifier.rs @@ -60,20 +60,44 @@ where } } -pub struct TransactionVerifier<'a, M, CS> { +pub struct NonContextualTransactionVerifier<'a> { pub version: VersionVerifier<'a>, pub size: SizeVerifier<'a>, pub empty: EmptyVerifier<'a>, - pub maturity: MaturityVerifier<'a>, - pub capacity: CapacityVerifier<'a>, pub duplicate_deps: DuplicateDepsVerifier<'a>, pub outputs_data_verifier: OutputsDataVerifier<'a>, - pub script: ScriptVerifier<'a, CS>, +} + +impl<'a> NonContextualTransactionVerifier<'a> { + pub fn new(tx: &'a TransactionView, consensus: &'a Consensus) -> Self { + NonContextualTransactionVerifier { + version: VersionVerifier::new(tx, consensus.tx_version()), + size: SizeVerifier::new(tx, consensus.max_block_bytes()), + empty: EmptyVerifier::new(tx), + duplicate_deps: DuplicateDepsVerifier::new(tx), + outputs_data_verifier: OutputsDataVerifier::new(tx), + } + } + + pub fn verify(&self) -> Result<(), Error> { + self.version.verify()?; + self.size.verify()?; + self.empty.verify()?; + self.duplicate_deps.verify()?; + self.outputs_data_verifier.verify()?; + Ok(()) + } +} + +pub struct ContextualTransactionVerifier<'a, M, CS> { + pub maturity: MaturityVerifier<'a>, pub since: SinceVerifier<'a, M>, + pub capacity: CapacityVerifier<'a>, + pub script: ScriptVerifier<'a, CS>, pub fee_calculator: FeeCalculator<'a, CS>, } -impl<'a, M, CS> TransactionVerifier<'a, M, CS> +impl<'a, M, CS> ContextualTransactionVerifier<'a, M, CS> where M: BlockMedianTimeContext, CS: ChainStore<'a>, @@ -88,17 +112,12 @@ where consensus: &'a Consensus, chain_store: &'a CS, ) -> Self { - TransactionVerifier { - version: VersionVerifier::new(&rtx.transaction, consensus.tx_version()), - size: SizeVerifier::new(&rtx.transaction, consensus.max_block_bytes()), - empty: EmptyVerifier::new(&rtx.transaction), + ContextualTransactionVerifier { maturity: MaturityVerifier::new( &rtx, epoch_number_with_fraction, consensus.cellbase_maturity(), ), - duplicate_deps: DuplicateDepsVerifier::new(&rtx.transaction), - outputs_data_verifier: OutputsDataVerifier::new(&rtx.transaction), script: ScriptVerifier::new(rtx, chain_store), capacity: CapacityVerifier::new(rtx, consensus.dao_type_hash()), since: SinceVerifier::new( @@ -113,13 +132,8 @@ where } pub fn verify(&self, max_cycles: Cycle) -> Result { - self.version.verify()?; - self.size.verify()?; - self.empty.verify()?; self.maturity.verify()?; self.capacity.verify()?; - self.duplicate_deps.verify()?; - self.outputs_data_verifier.verify()?; self.since.verify()?; let cycles = self.script.verify(max_cycles)?; let fee = self.fee_calculator.transaction_fee()?; @@ -127,6 +141,46 @@ where } } +pub struct TransactionVerifier<'a, M, CS> { + pub non_contextual: NonContextualTransactionVerifier<'a>, + pub contextual: ContextualTransactionVerifier<'a, M, CS>, +} + +impl<'a, M, CS> TransactionVerifier<'a, M, CS> +where + M: BlockMedianTimeContext, + CS: ChainStore<'a>, +{ + #[allow(clippy::too_many_arguments)] + pub fn new( + rtx: &'a ResolvedTransaction, + median_time_context: &'a M, + block_number: BlockNumber, + epoch_number_with_fraction: EpochNumberWithFraction, + parent_hash: Byte32, + consensus: &'a Consensus, + chain_store: &'a CS, + ) -> Self { + TransactionVerifier { + non_contextual: NonContextualTransactionVerifier::new(&rtx.transaction, consensus), + contextual: ContextualTransactionVerifier::new( + rtx, + median_time_context, + block_number, + epoch_number_with_fraction, + parent_hash, + consensus, + chain_store, + ), + } + } + + pub fn verify(&self, max_cycles: Cycle) -> Result { + self.non_contextual.verify()?; + self.contextual.verify(max_cycles) + } +} + pub struct FeeCalculator<'a, CS> { transaction: &'a ResolvedTransaction, consensus: &'a Consensus, From 27012052dcf6ea81d5da94469009832db1559216 Mon Sep 17 00:00:00 2001 From: zhangsoledad <787953403@qq.com> Date: Fri, 17 Jul 2020 05:41:09 +0800 Subject: [PATCH 3/3] feat: re-broadcast when duplicated transaction submit --- rpc/src/error.rs | 57 +++++++++++++++++------------ rpc/src/module/pool.rs | 34 +++++++++++------ test/src/specs/tx_pool/collision.rs | 4 +- 3 files changed, 58 insertions(+), 37 deletions(-) diff --git a/rpc/src/error.rs b/rpc/src/error.rs index 55f1a8c8eb..821c19197b 100644 --- a/rpc/src/error.rs +++ b/rpc/src/error.rs @@ -73,6 +73,27 @@ impl RPCError { } } + pub fn from_submit_transaction_reject(reject: &Reject) -> Error { + let code = match reject { + Reject::LowFeeRate(_, _) => RPCError::PoolRejectedTransactionByMinFeeRate, + Reject::ExceededMaximumAncestorsCount => { + RPCError::PoolRejectedTransactionByMaxAncestorsCountLimit + } + Reject::Full(_, _) => RPCError::PoolIsFull, + Reject::Duplicated(_) => RPCError::PoolRejectedDuplicatedTransaction, + Reject::Malformed(_) => RPCError::PoolRejectedMalformedTransaction, + }; + RPCError::custom_with_error(code, reject) + } + + pub fn downcast_submit_transaction_reject(err: &CKBError) -> Option<&Reject> { + use ckb_error::ErrorKind::SubmitTransaction; + match err.kind() { + SubmitTransaction => err.downcast_ref::(), + _ => None, + } + } + pub fn from_ckb_error(err: CKBError) -> Error { use ckb_error::ErrorKind::*; match err.kind() { @@ -82,24 +103,6 @@ impl RPCError { RPCError::TransactionFailedToVerify, err.unwrap_cause_or_self(), ), - SubmitTransaction => { - let reject = match err.downcast_ref::() { - Some(err) => err, - None => return Self::ckb_internal_error(err), - }; - - let kind = match *reject { - Reject::LowFeeRate(_, _) => RPCError::PoolRejectedTransactionByMinFeeRate, - Reject::ExceededMaximumAncestorsCount => { - RPCError::PoolRejectedTransactionByMaxAncestorsCountLimit - } - Reject::Full(_, _) => RPCError::PoolIsFull, - Reject::Duplicated(_) => RPCError::PoolRejectedDuplicatedTransaction, - Reject::Malformed(_) => RPCError::PoolRejectedMalformedTransaction, - }; - - RPCError::custom_with_error(kind, reject) - } Internal => { let internal_err = match err.downcast_ref::() { Some(err) => err, @@ -159,35 +162,41 @@ mod tests { } #[test] - fn test_submit_tx_error_from_ckb_error() { + fn test_submit_transaction_error() { let err: CKBError = Reject::LowFeeRate(100, 50).into(); assert_eq!( "PoolRejectedTransactionByMinFeeRate: Transaction fee rate must >= 100 shannons/KB, got: 50", - RPCError::from_ckb_error(err).message + RPCError::from_submit_transaction_reject(RPCError::downcast_submit_transaction_reject(&err).unwrap()).message ); let err: CKBError = Reject::ExceededMaximumAncestorsCount.into(); assert_eq!( "PoolRejectedTransactionByMaxAncestorsCountLimit: Transaction exceeded maximum ancestors count limit, try send it later", - RPCError::from_ckb_error(err).message + RPCError::from_submit_transaction_reject(RPCError::downcast_submit_transaction_reject(&err).unwrap()).message ); let err: CKBError = Reject::Full("size".to_owned(), 10).into(); assert_eq!( "PoolIsFull: Transaction pool exceeded maximum size limit(10), try send it later", - RPCError::from_ckb_error(err).message + RPCError::from_submit_transaction_reject( + RPCError::downcast_submit_transaction_reject(&err).unwrap() + ) + .message ); let err: CKBError = Reject::Duplicated(Byte32::new([0; 32])).into(); assert_eq!( "PoolRejectedDuplicatedTransaction: Transaction(Byte32(0x0000000000000000000000000000000000000000000000000000000000000000)) already exist in transaction_pool", - RPCError::from_ckb_error(err).message + RPCError::from_submit_transaction_reject(RPCError::downcast_submit_transaction_reject(&err).unwrap()).message ); let err: CKBError = Reject::Malformed("cellbase like".to_owned()).into(); assert_eq!( "PoolRejectedMalformedTransaction: Malformed cellbase like transaction", - RPCError::from_ckb_error(err).message + RPCError::from_submit_transaction_reject( + RPCError::downcast_submit_transaction_reject(&err).unwrap() + ) + .message ); } diff --git a/rpc/src/module/pool.rs b/rpc/src/module/pool.rs index d526733c02..d7b7f132de 100644 --- a/rpc/src/module/pool.rs +++ b/rpc/src/module/pool.rs @@ -7,6 +7,7 @@ use ckb_network::PeerIndex; use ckb_script::IllTransactionChecker; use ckb_shared::shared::Shared; use ckb_sync::SyncShared; +use ckb_tx_pool::error::Reject; use ckb_types::{core, packed, prelude::*, H256}; use ckb_verification::{Since, SinceMetric}; use jsonrpc_core::Result; @@ -100,20 +101,31 @@ impl PoolRpc for PoolRpcImpl { return Err(RPCError::ckb_internal_error(e)); } + let broadcast = |tx_hash: packed::Byte32| { + // workaround: we are using `PeerIndex(usize::max)` to indicate that tx hash source is itself. + let peer_index = PeerIndex::new(usize::max_value()); + self.sync_shared + .state() + .tx_hashes() + .entry(peer_index) + .or_default() + .insert(tx_hash); + }; + let tx_hash = tx.hash(); match submit_txs.unwrap() { Ok(_) => { - // workaround: we are using `PeerIndex(usize::max)` to indicate that tx hash source is itself. - let peer_index = PeerIndex::new(usize::max_value()); - let hash = tx.hash(); - self.sync_shared - .state() - .tx_hashes() - .entry(peer_index) - .or_default() - .insert(hash.clone()); - Ok(hash.unpack()) + broadcast(tx_hash.clone()); + Ok(tx_hash.unpack()) } - Err(e) => Err(RPCError::from_ckb_error(e)), + Err(e) => match RPCError::downcast_submit_transaction_reject(&e) { + Some(reject) => { + if let Reject::Duplicated(_) = reject { + broadcast(tx_hash); + } + Err(RPCError::from_submit_transaction_reject(reject)) + } + None => Err(RPCError::from_ckb_error(e)), + }, } } diff --git a/test/src/specs/tx_pool/collision.rs b/test/src/specs/tx_pool/collision.rs index c87c4264b5..b1e2900e89 100644 --- a/test/src/specs/tx_pool/collision.rs +++ b/test/src/specs/tx_pool/collision.rs @@ -30,7 +30,7 @@ impl Spec for TransactionHashCollisionDifferentWitnessHashes { .err() .unwrap() .to_string() - .contains("PoolTransactionDuplicated")); + .contains("PoolRejectedDuplicatedTransaction")); } } @@ -54,7 +54,7 @@ impl Spec for DuplicatedTransaction { .err() .unwrap() .to_string() - .contains("PoolTransactionDuplicated")); + .contains("PoolRejectedDuplicatedTransaction")); } }