From f28a58cae05123ea3b9abc9fe454b32d1576b11c Mon Sep 17 00:00:00 2001 From: zhangsoledad <787953403@qq.com> Date: Mon, 20 Mar 2023 11:21:11 +0800 Subject: [PATCH 1/8] feat: limit the size of the tx-pool to accept transactions --- rpc/src/error.rs | 5 +++++ rpc/src/tests/error.rs | 6 ++++++ tx-pool/src/util.rs | 15 ++++++++++++++- util/jsonrpc-types/src/pool.rs | 6 ++++++ util/types/src/core/tx_pool.rs | 10 ++++++++++ 5 files changed, 41 insertions(+), 1 deletion(-) diff --git a/rpc/src/error.rs b/rpc/src/error.rs index 869e19333f..8805a7dfb8 100644 --- a/rpc/src/error.rs +++ b/rpc/src/error.rs @@ -110,6 +110,8 @@ pub enum RPCError { PoolRejectedMalformedTransaction = -1108, /// (-1109): The transaction is expired from tx-pool after `expiry_hours`. TransactionExpired = -1109, + /// (-1110): The transaction exceeded maximum size limit. + PoolRejectedTransactionBySizeLimit = -1110, /// (-1200): The indexer error. Indexer = -1200, } @@ -171,6 +173,9 @@ impl RPCError { Reject::DeclaredWrongCycles(..) => RPCError::PoolRejectedMalformedTransaction, Reject::Resolve(_) => RPCError::TransactionFailedToResolve, Reject::Verification(_) => RPCError::TransactionFailedToVerify, + Reject::ExceededTransactionSizeLimit(_, _) => { + RPCError::PoolRejectedTransactionBySizeLimit + } Reject::Expiry(_) => RPCError::TransactionExpired, }; RPCError::custom_with_error(code, reject) diff --git a/rpc/src/tests/error.rs b/rpc/src/tests/error.rs index 0d81c6a798..5efc58d3c7 100644 --- a/rpc/src/tests/error.rs +++ b/rpc/src/tests/error.rs @@ -49,6 +49,12 @@ fn test_submit_transaction_error() { "PoolRejectedMalformedTransaction: Malformed cellbase like transaction", RPCError::from_submit_transaction_reject(&reject).message ); + + let reject = Reject::ExceededTransactionSizeLimit(10, 9); + assert_eq!( + "PoolRejectedTransactionBySizeLimit: Transaction size 10 exceeded maximum limit 9", + RPCError::from_submit_transaction_reject(&reject).message + ); } #[test] diff --git a/tx-pool/src/util.rs b/tx-pool/src/util.rs index ab6e71cfbe..1b3413e971 100644 --- a/tx-pool/src/util.rs +++ b/tx-pool/src/util.rs @@ -5,7 +5,9 @@ use ckb_dao::DaoCalculator; use ckb_snapshot::Snapshot; use ckb_store::data_loader_wrapper::AsDataLoader; use ckb_store::ChainStore; -use ckb_types::core::{cell::ResolvedTransaction, Capacity, Cycle, TransactionView}; +use ckb_types::core::{ + cell::ResolvedTransaction, tx_pool::TRANSACTION_SIZE_LIMIT, Capacity, Cycle, TransactionView, +}; use ckb_verification::{ cache::{CacheEntry, Completed}, ContextualTransactionVerifier, NonContextualTransactionVerifier, @@ -69,6 +71,17 @@ pub(crate) fn non_contextual_verify( NonContextualTransactionVerifier::new(tx, consensus) .verify() .map_err(Reject::Verification)?; + + // The ckb consensus does not limit the size of a single transaction, + // but if the size of the transaction is close to the limit of the block, + // it may cause the transaction to fail to be packed + let tx_size = tx.data().serialized_size_in_block() as u64; + if tx_size > TRANSACTION_SIZE_LIMIT { + return Err(Reject::ExceededTransactionSizeLimit( + tx_size, + TRANSACTION_SIZE_LIMIT, + )); + } // cellbase is only valid in a block, not as a loose transaction if tx.is_cellbase() { return Err(Reject::Malformed("cellbase like".to_owned())); diff --git a/util/jsonrpc-types/src/pool.rs b/util/jsonrpc-types/src/pool.rs index 70910ee973..b478cb5d5d 100644 --- a/util/jsonrpc-types/src/pool.rs +++ b/util/jsonrpc-types/src/pool.rs @@ -190,6 +190,9 @@ pub enum PoolTransactionReject { /// Transaction exceeded maximum ancestors count limit ExceededMaximumAncestorsCount(String), + /// Transaction exceeded maximum size limit + ExceededTransactionSizeLimit(String), + /// Transaction pool exceeded maximum size or cycles limit, Full(String), @@ -219,6 +222,9 @@ impl From for PoolTransactionReject { Reject::ExceededMaximumAncestorsCount => { Self::ExceededMaximumAncestorsCount(format!("{reject}")) } + Reject::ExceededTransactionSizeLimit(..) => { + Self::ExceededTransactionSizeLimit(format!("{reject}")) + } Reject::Full(..) => Self::Full(format!("{reject}")), Reject::Duplicated(_) => Self::Duplicated(format!("{reject}")), Reject::Malformed(_) => Self::Malformed(format!("{reject}")), diff --git a/util/types/src/core/tx_pool.rs b/util/types/src/core/tx_pool.rs index 81d53b07c6..7949873def 100644 --- a/util/types/src/core/tx_pool.rs +++ b/util/types/src/core/tx_pool.rs @@ -21,6 +21,10 @@ pub enum Reject { #[error("Transaction exceeded maximum ancestors count limit, try send it later")] ExceededMaximumAncestorsCount, + /// Transaction exceeded maximum size limit + #[error("Transaction size {0} exceeded maximum limit {1}")] + ExceededTransactionSizeLimit(u64, u64), + /// Transaction pool exceeded maximum size or cycles limit, #[error("Transaction pool exceeded maximum {0} limit({1}), try send it later")] Full(String, u64), @@ -252,3 +256,9 @@ pub fn get_transaction_weight(tx_size: usize, cycles: u64) -> u64 { (cycles as f64 * DEFAULT_BYTES_PER_CYCLES) as u64, ) } + +/// The maximum size of the tx-pool to accept transactions +/// The ckb consensus does not limit the size of a single transaction, +/// but if the size of the transaction is close to the limit of the block, +/// it may cause the transaction to fail to be packed +pub const TRANSACTION_SIZE_LIMIT: u64 = 512 * 1_000; From 12a43be7db7bdfed2ad731517684847b26b127b7 Mon Sep 17 00:00:00 2001 From: zhangsoledad <787953403@qq.com> Date: Tue, 21 Mar 2023 17:16:40 +0800 Subject: [PATCH 2/8] feat: limit tx-pool size through evict transactions --- rpc/src/error.rs | 2 +- rpc/src/tests/error.rs | 4 +- test/src/main.rs | 1 - test/src/specs/tx_pool/limit.rs | 76 +++------------------------- tx-pool/src/chunk_process.rs | 11 ++-- tx-pool/src/component/entry.rs | 56 +++++++++++++++++++- tx-pool/src/component/pending.rs | 4 ++ tx-pool/src/pool.rs | 71 +++++++++++++++++++++++++- tx-pool/src/process.rs | 31 ++++++------ tx-pool/src/util.rs | 30 +++++------ util/jsonrpc-types/src/pool.rs | 2 +- util/types/src/core/tests/tx_pool.rs | 5 +- util/types/src/core/tx_pool.rs | 6 +-- 13 files changed, 179 insertions(+), 120 deletions(-) diff --git a/rpc/src/error.rs b/rpc/src/error.rs index 8805a7dfb8..1f86db9b72 100644 --- a/rpc/src/error.rs +++ b/rpc/src/error.rs @@ -167,7 +167,7 @@ impl RPCError { Reject::ExceededMaximumAncestorsCount => { RPCError::PoolRejectedTransactionByMaxAncestorsCountLimit } - Reject::Full(_, _) => RPCError::PoolIsFull, + Reject::Full(_) => RPCError::PoolIsFull, Reject::Duplicated(_) => RPCError::PoolRejectedDuplicatedTransaction, Reject::Malformed(_) => RPCError::PoolRejectedMalformedTransaction, Reject::DeclaredWrongCycles(..) => RPCError::PoolRejectedMalformedTransaction, diff --git a/rpc/src/tests/error.rs b/rpc/src/tests/error.rs index 5efc58d3c7..8235ab180d 100644 --- a/rpc/src/tests/error.rs +++ b/rpc/src/tests/error.rs @@ -32,9 +32,9 @@ fn test_submit_transaction_error() { RPCError::from_submit_transaction_reject(&reject).message ); - let reject = Reject::Full("size".to_owned(), 10); + let reject = Reject::Full("size".to_owned()); assert_eq!( - "PoolIsFull: Transaction pool exceeded maximum size limit(10), try send it later", + "PoolIsFull: Transaction are replaced because the pool is full size", RPCError::from_submit_transaction_reject(&reject).message ); diff --git a/test/src/main.rs b/test/src/main.rs index 8157d41669..6b8034fd34 100644 --- a/test/src/main.rs +++ b/test/src/main.rs @@ -442,7 +442,6 @@ fn all_specs() -> Vec> { Box::new(CompactBlockRelayLessThenSharedBestKnown), Box::new(InvalidLocatorSize), Box::new(SizeLimit), - Box::new(CyclesLimit), Box::new(SendDefectedBinary::new( "send_defected_binary_reject_known_bugs", true, diff --git a/test/src/specs/tx_pool/limit.rs b/test/src/specs/tx_pool/limit.rs index 9e8b605c1c..28da63962e 100644 --- a/test/src/specs/tx_pool/limit.rs +++ b/test/src/specs/tx_pool/limit.rs @@ -1,12 +1,11 @@ -use crate::utils::assert_send_transaction_fail; use crate::{Node, Spec}; use ckb_logger::info; use ckb_types::core::FeeRate; +use std::{thread::sleep, time::Duration}; pub struct SizeLimit; -const MAX_CYCLES_FOR_SIZE_LIMIT: u64 = 200_000_000_000; const MAX_MEM_SIZE_FOR_SIZE_LIMIT: usize = 2000; impl Spec for SizeLimit { @@ -35,87 +34,28 @@ impl Spec for SizeLimit { let max_tx_num = (MAX_MEM_SIZE_FOR_SIZE_LIMIT as u64) / one_tx_size; - assert!(one_tx_cycles * max_tx_num < MAX_CYCLES_FOR_SIZE_LIMIT); - info!("Generate as much as possible txs on node"); (0..(max_tx_num - 1)).for_each(|_| { let tx = node.new_transaction(hash.clone()); hash = node.rpc_client().send_transaction(tx.data().into()); txs_hash.push(hash.clone()); + sleep(Duration::from_millis(10)); }); info!("The next tx reach size limit"); let tx = node.new_transaction(hash); - assert_send_transaction_fail(node, &tx, "Transaction pool exceeded maximum size limit"); - + let _hash = node.rpc_client().send_transaction(tx.data().into()); + node.assert_tx_pool_serialized_size((max_tx_num + 1) * one_tx_size); + let last = node + .mine_with_blocking(|template| template.proposals.len() != (max_tx_num + 1) as usize); node.assert_tx_pool_serialized_size(max_tx_num * one_tx_size); - node.mine_until_transactions_confirm(); - node.mine(1); + node.mine_with_blocking(|template| template.number.value() != (last + 1)); + node.mine_with_blocking(|template| template.transactions.len() != max_tx_num as usize); node.assert_tx_pool_serialized_size(0); } fn modify_app_config(&self, config: &mut ckb_app_config::CKBAppConfig) { config.tx_pool.max_mem_size = MAX_MEM_SIZE_FOR_SIZE_LIMIT; - config.tx_pool.max_cycles = MAX_CYCLES_FOR_SIZE_LIMIT; - config.tx_pool.min_fee_rate = FeeRate::zero(); - } -} - -pub struct CyclesLimit; - -const MAX_CYCLES_FOR_CYCLE_LIMIT: u64 = 6000; -const MAX_MEM_SIZE_FOR_CYCLE_LIMIT: usize = 20_000_000; - -impl Spec for CyclesLimit { - fn run(&self, nodes: &mut Vec) { - let node = &nodes[0]; - - info!("Generate DEFAULT_TX_PROPOSAL_WINDOW block on node"); - node.mine_until_out_bootstrap_period(); - - info!("Generate 1 tx on node"); - let mut txs_hash = Vec::new(); - let tx = node.new_transaction_spend_tip_cellbase(); - let mut hash = node.submit_transaction(&tx); - txs_hash.push(hash.clone()); - - let tx_pool_info = node.get_tip_tx_pool_info(); - let one_tx_cycles = tx_pool_info.total_tx_cycles.value(); - let one_tx_size = tx.data().serialized_size_in_block(); - - info!( - "one_tx_cycles: {}, one_tx_size: {}", - one_tx_cycles, one_tx_size - ); - - assert!(MAX_CYCLES_FOR_CYCLE_LIMIT > one_tx_cycles * 2); - - let max_tx_num = MAX_CYCLES_FOR_CYCLE_LIMIT / one_tx_cycles; - - assert!(one_tx_size * (max_tx_num as usize) < MAX_MEM_SIZE_FOR_CYCLE_LIMIT); - - info!("Generate as much as possible txs on node"); - (0..(max_tx_num - 1)).for_each(|_| { - let tx = node.new_transaction(hash.clone()); - hash = node.rpc_client().send_transaction(tx.data().into()); - txs_hash.push(hash.clone()); - }); - - info!("The next tx reach cycles limit"); - let tx = node.new_transaction(hash); - assert_send_transaction_fail(node, &tx, "Transaction pool exceeded maximum cycles limit"); - - node.assert_tx_pool_cycles(max_tx_num * one_tx_cycles); - let proposed = - node.mine_with_blocking(|template| template.proposals.len() != max_tx_num as usize); - node.mine_with_blocking(|template| template.number.value() != (proposed + 1)); - node.mine_with_blocking(|template| template.transactions.len() != max_tx_num as usize); - node.assert_tx_pool_cycles(0); - } - - fn modify_app_config(&self, config: &mut ckb_app_config::CKBAppConfig) { - config.tx_pool.max_mem_size = MAX_MEM_SIZE_FOR_CYCLE_LIMIT; - config.tx_pool.max_cycles = MAX_CYCLES_FOR_CYCLE_LIMIT; config.tx_pool.min_fee_rate = FeeRate::zero(); } } diff --git a/tx-pool/src/chunk_process.rs b/tx-pool/src/chunk_process.rs index a7877d9120..880d74f34d 100644 --- a/tx-pool/src/chunk_process.rs +++ b/tx-pool/src/chunk_process.rs @@ -241,10 +241,8 @@ impl ChunkProcess { let completed = try_or_return_with_snapshot!(ret, snapshot); let entry = TxEntry::new(rtx, completed.cycles, fee, tx_size); - let (ret, submit_snapshot) = self - .service - .submit_entry(completed, tip_hash, entry, status) - .await; + let (ret, submit_snapshot) = + self.service.submit_entry(tip_hash, entry, status).await; try_or_return_with_snapshot!(ret, submit_snapshot); self.service .after_process(tx, remote, &submit_snapshot, &Ok(completed)) @@ -306,10 +304,7 @@ impl ChunkProcess { } let entry = TxEntry::new(rtx, completed.cycles, fee, tx_size); - let (ret, submit_snapshot) = self - .service - .submit_entry(completed, tip_hash, entry, status) - .await; + let (ret, submit_snapshot) = self.service.submit_entry(tip_hash, entry, status).await; try_or_return_with_snapshot!(ret, snapshot); self.service.notify_block_assembler(status).await; diff --git a/tx-pool/src/component/entry.rs b/tx-pool/src/component/entry.rs index 2403a6b6b6..a96b01f54c 100644 --- a/tx-pool/src/component/entry.rs +++ b/tx-pool/src/component/entry.rs @@ -4,7 +4,7 @@ use ckb_types::{ core::{ cell::ResolvedTransaction, tx_pool::{get_transaction_weight, TxEntryInfo}, - Capacity, Cycle, TransactionView, + Capacity, Cycle, FeeRate, TransactionView, }, packed::{OutPoint, ProposalShortId}, }; @@ -38,16 +38,27 @@ pub struct TxEntry { impl TxEntry { /// Create new transaction pool entry pub fn new(rtx: Arc, cycles: Cycle, fee: Capacity, size: usize) -> Self { + Self::new_with_timestamp(rtx, cycles, fee, size, unix_time_as_millis()) + } + + /// Create new transaction pool entry with specified timestamp + pub fn new_with_timestamp( + rtx: Arc, + cycles: Cycle, + fee: Capacity, + size: usize, + timestamp: u64, + ) -> Self { TxEntry { rtx, cycles, size, fee, + timestamp, ancestors_size: size, ancestors_fee: fee, ancestors_cycles: cycles, ancestors_count: 1, - timestamp: unix_time_as_millis(), } } @@ -83,6 +94,11 @@ impl TxEntry { AncestorsScoreSortKey::from(self) } + /// Returns a evict_key + pub fn as_evict_key(&self) -> EvictKey { + EvictKey::from(self) + } + /// Update ancestor state for add an entry pub fn add_entry_weight(&mut self, entry: &TxEntry) { self.ancestors_count = self.ancestors_count.saturating_add(1); @@ -166,3 +182,39 @@ impl Ord for TxEntry { self.as_sorted_key().cmp(&other.as_sorted_key()) } } + +/// Currently we do not have trace descendants, +/// so we take the simplest strategy to evict the transactions by first comparing the fee_rate, +/// and then taking the one with the largest timestamp, +/// which also means that the fewer descendants may exist. +#[derive(Eq, PartialEq, Clone, Debug)] +pub struct EvictKey { + fee_rate: FeeRate, + timestamp: u64, +} + +impl From<&TxEntry> for EvictKey { + fn from(entry: &TxEntry) -> Self { + let weight = get_transaction_weight(entry.size, entry.cycles); + EvictKey { + fee_rate: FeeRate::calculate(entry.fee, weight), + timestamp: entry.timestamp, + } + } +} + +impl PartialOrd for EvictKey { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for EvictKey { + fn cmp(&self, other: &Self) -> Ordering { + if self.fee_rate == other.fee_rate { + self.timestamp.cmp(&other.timestamp).reverse() + } else { + self.fee_rate.cmp(&other.fee_rate) + } + } +} diff --git a/tx-pool/src/component/pending.rs b/tx-pool/src/component/pending.rs index aba79f3bf1..d0973c78dd 100644 --- a/tx-pool/src/component/pending.rs +++ b/tx-pool/src/component/pending.rs @@ -42,6 +42,10 @@ impl PendingQueue { self.inner.len() } + pub(crate) fn is_empty(&self) -> bool { + self.inner.len() == 0 + } + #[cfg(test)] pub(crate) fn outputs_len(&self) -> usize { self.outputs.len() diff --git a/tx-pool/src/pool.rs b/tx-pool/src/pool.rs index 01ee9c33b8..98340b5e0e 100644 --- a/tx-pool/src/pool.rs +++ b/tx-pool/src/pool.rs @@ -313,6 +313,69 @@ impl TxPool { } } + pub(crate) fn limit_size(&mut self, callbacks: &Callbacks) { + while self.total_tx_size > self.config.max_mem_size { + if !self.pending.is_empty() { + if let Some(id) = self + .pending + .iter() + .min_by_key(|(_id, entry)| entry.as_evict_key()) + .map(|(id, _)| id) + .cloned() + { + let removed = self.pending.remove_entry_and_descendants(&id); + for entry in removed { + let tx_hash = entry.transaction().hash(); + debug!( + "removed by size limit from pending {} timestamp({})", + tx_hash, entry.timestamp + ); + let reject = Reject::Full(format!("fee: {}", entry.fee)); + callbacks.call_reject(self, &entry, reject); + } + } + } else if !self.gap.is_empty() { + if let Some(id) = self + .gap + .iter() + .min_by_key(|(_id, entry)| entry.as_evict_key()) + .map(|(id, _)| id) + .cloned() + { + let removed = self.gap.remove_entry_and_descendants(&id); + for entry in removed { + let tx_hash = entry.transaction().hash(); + debug!( + "removed by size limit from gap {} timestamp({})", + tx_hash, entry.timestamp + ); + let reject = Reject::Full(format!("fee: {}", entry.fee)); + callbacks.call_reject(self, &entry, reject); + } + } + } else { + if let Some(id) = self + .proposed + .iter() + .min_by_key(|(_id, entry)| entry.as_evict_key()) + .map(|(id, _)| id) + .cloned() + { + let removed = self.proposed.remove_entry_and_descendants(&id); + for entry in removed { + let tx_hash = entry.transaction().hash(); + debug!( + "removed by size limit from proposed {} timestamp({})", + tx_hash, entry.timestamp + ); + let reject = Reject::Full(format!("fee: {}", entry.fee)); + callbacks.call_reject(self, &entry, reject); + } + } + } + } + } + // remove transaction with detached proposal from gap and proposed // try re-put to pending pub(crate) fn remove_by_detached_proposal<'a>( @@ -422,6 +485,7 @@ impl TxPool { &mut self, cache_entry: CacheEntry, size: usize, + timestamp: u64, rtx: Arc, ) -> Result { let snapshot = self.cloned_snapshot(); @@ -438,7 +502,8 @@ impl TxPool { max_cycles, )?; - let entry = TxEntry::new(rtx, verified.cycles, verified.fee, size); + let entry = + TxEntry::new_with_timestamp(rtx, verified.cycles, verified.fee, size, timestamp); let tx_hash = entry.transaction().hash(); if self.add_gap(entry) { Ok(CacheEntry::Completed(verified)) @@ -451,6 +516,7 @@ impl TxPool { &mut self, cache_entry: CacheEntry, size: usize, + timestamp: u64, rtx: Arc, ) -> Result { let snapshot = self.cloned_snapshot(); @@ -467,7 +533,8 @@ impl TxPool { max_cycles, )?; - let entry = TxEntry::new(rtx, verified.cycles, verified.fee, size); + let entry = + TxEntry::new_with_timestamp(rtx, verified.cycles, verified.fee, size, timestamp); let tx_hash = entry.transaction().hash(); if self.add_proposed(entry)? { Ok(CacheEntry::Completed(verified)) diff --git a/tx-pool/src/process.rs b/tx-pool/src/process.rs index 484cea31c2..f1df3e5fc9 100644 --- a/tx-pool/src/process.rs +++ b/tx-pool/src/process.rs @@ -1,5 +1,4 @@ use crate::callback::Callbacks; -use crate::component::chunk::DEFAULT_MAX_CHUNK_TRANSACTIONS; use crate::component::entry::TxEntry; use crate::component::orphan::Entry as OrphanEntry; use crate::error::Reject; @@ -7,8 +6,8 @@ use crate::pool::TxPool; use crate::service::{BlockAssemblerMessage, TxPoolService, TxVerificationResult}; use crate::try_or_return_with_snapshot; use crate::util::{ - check_tx_cycle_limit, check_tx_fee, check_tx_size_limit, check_txid_collision, - is_missing_input, non_contextual_verify, time_relative_verify, verify_rtx, + check_tx_fee, check_txid_collision, is_missing_input, non_contextual_verify, + time_relative_verify, verify_rtx, }; use ckb_error::{AnyError, InternalErrorKind}; use ckb_jsonrpc_types::BlockTemplate; @@ -94,15 +93,12 @@ impl TxPoolService { pub(crate) async fn submit_entry( &self, - verified: Completed, pre_resolve_tip: Byte32, entry: TxEntry, mut status: TxStatus, ) -> (Result<(), Reject>, Arc) { let (ret, snapshot) = self .with_tx_pool_write_lock(move |tx_pool, snapshot| { - check_tx_cycle_limit(tx_pool, verified.cycles)?; - // if snapshot changed by context switch // we need redo time_relative verify let tip_hash = snapshot.tip_hash(); @@ -201,7 +197,6 @@ impl TxPoolService { let (ret, snapshot) = self .with_tx_pool_read_lock(|tx_pool, snapshot| { let tip_hash = snapshot.tip_hash(); - check_tx_size_limit(tx_pool, tx_size)?; check_txid_collision(tx_pool, tx)?; @@ -578,10 +573,7 @@ impl TxPoolService { } ScriptVerifyResult::Suspended(state) => { if is_chunk_full { - Err(Reject::Full( - "chunk".to_owned(), - DEFAULT_MAX_CHUNK_TRANSACTIONS as u64, - )) + Err(Reject::Full("chunk".to_owned())) } else { let snap = Arc::new(state.try_into().map_err(Reject::Verification)?); Ok(CacheEntry::suspended(snap, fee)) @@ -605,7 +597,7 @@ impl TxPoolService { let entry = TxEntry::new(rtx, completed.cycles, fee, tx_size); - let (ret, submit_snapshot) = self.submit_entry(completed, tip_hash, entry, status).await; + let (ret, submit_snapshot) = self.submit_entry(tip_hash, entry, status).await; try_or_return_with_snapshot!(ret, submit_snapshot); self.notify_block_assembler(status).await; @@ -679,7 +671,7 @@ impl TxPoolService { let entry = TxEntry::new(rtx, verified.cycles, fee, tx_size); - let (ret, submit_snapshot) = self.submit_entry(verified, tip_hash, entry, status).await; + let (ret, submit_snapshot) = self.submit_entry(tip_hash, entry, status).await; try_or_return_with_snapshot!(ret, submit_snapshot); self.notify_block_assembler(status).await; @@ -941,7 +933,9 @@ fn _update_tx_pool_for_reorg( debug!("tx move to proposed {}", entry.transaction().hash()); let cached = CacheEntry::completed(entry.cycles, entry.fee); let tx_hash = entry.transaction().hash(); - if let Err(e) = tx_pool.proposed_rtx(cached, entry.size, Arc::clone(&entry.rtx)) { + if let Err(e) = + tx_pool.proposed_rtx(cached, entry.size, entry.timestamp, Arc::clone(&entry.rtx)) + { debug!("Failed to add proposed tx {}, reason: {}", tx_hash, e); callbacks.call_reject(tx_pool, &entry, e.clone()); } else { @@ -953,15 +947,20 @@ fn _update_tx_pool_for_reorg( debug!("tx move to gap {}", entry.transaction().hash()); let tx_hash = entry.transaction().hash(); let cached = CacheEntry::completed(entry.cycles, entry.fee); - if let Err(e) = tx_pool.gap_rtx(cached, entry.size, Arc::clone(&entry.rtx)) { + if let Err(e) = + tx_pool.gap_rtx(cached, entry.size, entry.timestamp, Arc::clone(&entry.rtx)) + { debug!("Failed to add tx to gap {}, reason: {}", tx_hash, e); callbacks.call_reject(tx_pool, &entry, e.clone()); } } } - // remove expired transaction from pending + // Remove expired transaction from pending tx_pool.remove_expired(callbacks); + + // Remove transactions from the pool until its size < size_limit. + tx_pool.limit_size(callbacks); } pub fn all_inputs_is_unknown(snapshot: &Snapshot, tx: &TransactionView) -> bool { diff --git a/tx-pool/src/util.rs b/tx-pool/src/util.rs index 1b3413e971..8bfff54124 100644 --- a/tx-pool/src/util.rs +++ b/tx-pool/src/util.rs @@ -24,22 +24,22 @@ pub(crate) fn check_txid_collision(tx_pool: &TxPool, tx: &TransactionView) -> Re Ok(()) } -pub(crate) fn check_tx_size_limit(tx_pool: &TxPool, tx_size: usize) -> Result<(), Reject> { - if tx_pool.reach_size_limit(tx_size) { - return Err(Reject::Full( - "size".to_owned(), - tx_pool.config.max_mem_size as u64, - )); - } - Ok(()) -} +// pub(crate) fn check_tx_size_limit(tx_pool: &TxPool, tx_size: usize) -> Result<(), Reject> { +// if tx_pool.reach_size_limit(tx_size) { +// return Err(Reject::Full( +// "size".to_owned(), +// tx_pool.config.max_mem_size as u64, +// )); +// } +// Ok(()) +// } -pub(crate) fn check_tx_cycle_limit(tx_pool: &TxPool, cycles: Cycle) -> Result<(), Reject> { - if tx_pool.reach_cycles_limit(cycles) { - return Err(Reject::Full("cycles".to_owned(), tx_pool.config.max_cycles)); - } - Ok(()) -} +// pub(crate) fn check_tx_cycle_limit(tx_pool: &TxPool, cycles: Cycle) -> Result<(), Reject> { +// if tx_pool.reach_cycles_limit(cycles) { +// return Err(Reject::Full("cycles".to_owned(), tx_pool.config.max_cycles)); +// } +// Ok(()) +// } pub(crate) fn check_tx_fee( tx_pool: &TxPool, diff --git a/util/jsonrpc-types/src/pool.rs b/util/jsonrpc-types/src/pool.rs index b478cb5d5d..ade4be43b9 100644 --- a/util/jsonrpc-types/src/pool.rs +++ b/util/jsonrpc-types/src/pool.rs @@ -193,7 +193,7 @@ pub enum PoolTransactionReject { /// Transaction exceeded maximum size limit ExceededTransactionSizeLimit(String), - /// Transaction pool exceeded maximum size or cycles limit, + /// Transaction are replaced because the pool is full Full(String), /// Transaction already exist in transaction_pool diff --git a/util/types/src/core/tests/tx_pool.rs b/util/types/src/core/tests/tx_pool.rs index 530b4f0c04..4930ffb3d2 100644 --- a/util/types/src/core/tests/tx_pool.rs +++ b/util/types/src/core/tests/tx_pool.rs @@ -13,7 +13,10 @@ fn test_if_is_malformed_tx() { let reject = Reject::ExceededMaximumAncestorsCount; assert!(!reject.is_malformed_tx()); - let reject = Reject::Full(Default::default(), 0); + let reject = Reject::ExceededTransactionSizeLimit(0, 0); + assert!(!reject.is_malformed_tx()); + + let reject = Reject::Full(Default::default()); assert!(!reject.is_malformed_tx()); let reject = Reject::Duplicated(Default::default()); diff --git a/util/types/src/core/tx_pool.rs b/util/types/src/core/tx_pool.rs index 7949873def..b3ff936056 100644 --- a/util/types/src/core/tx_pool.rs +++ b/util/types/src/core/tx_pool.rs @@ -25,9 +25,9 @@ pub enum Reject { #[error("Transaction size {0} exceeded maximum limit {1}")] ExceededTransactionSizeLimit(u64, u64), - /// Transaction pool exceeded maximum size or cycles limit, - #[error("Transaction pool exceeded maximum {0} limit({1}), try send it later")] - Full(String, u64), + /// Transaction are replaced because the pool is full + #[error("Transaction are replaced because the pool is full {0}")] + Full(String), /// Transaction already exist in transaction_pool #[error("Transaction({0}) already exist in transaction_pool")] From 0ac887a3453995b4df145f55d26d392b713b6b81 Mon Sep 17 00:00:00 2001 From: zhangsoledad <787953403@qq.com> Date: Tue, 21 Mar 2023 18:43:42 +0800 Subject: [PATCH 3/8] fix: tx-pool remove_expired --- tx-pool/src/pool.rs | 63 +++++++++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 25 deletions(-) diff --git a/tx-pool/src/pool.rs b/tx-pool/src/pool.rs index 98340b5e0e..007257cc95 100644 --- a/tx-pool/src/pool.rs +++ b/tx-pool/src/pool.rs @@ -296,23 +296,38 @@ impl TxPool { } } + // Expire all transaction (and their dependencies) in the pool. pub(crate) fn remove_expired(&mut self, callbacks: &Callbacks) { let now_ms = ckb_systemtime::unix_time_as_millis(); - let removed = self - .pending - .remove_entries_by_filter(|_id, tx_entry| now_ms > self.expiry + tx_entry.timestamp); + let expired = + |_id: &ProposalShortId, tx_entry: &TxEntry| self.expiry + tx_entry.timestamp < now_ms; + let mut removed = self.pending.remove_entries_by_filter(expired); + removed.extend(self.gap.remove_entries_by_filter(expired)); + let removed_proposed_ids: Vec<_> = self + .proposed + .iter() + .filter_map(|(id, tx_entry)| { + if self.expiry + tx_entry.timestamp < now_ms { + Some(id) + } else { + None + } + }) + .cloned() + .collect(); + for id in removed_proposed_ids { + removed.extend(self.proposed.remove_entry_and_descendants(&id)) + } for entry in removed { let tx_hash = entry.transaction().hash(); - debug!( - "remove_expired from pending {} timestamp({})", - tx_hash, entry.timestamp - ); + debug!("remove_expired {} timestamp({})", tx_hash, entry.timestamp); let reject = Reject::Expiry(entry.timestamp); callbacks.call_reject(self, &entry, reject); } } + // Remove transactions from the pool until total size < size_limit. pub(crate) fn limit_size(&mut self, callbacks: &Callbacks) { while self.total_tx_size > self.config.max_mem_size { if !self.pending.is_empty() { @@ -353,24 +368,22 @@ impl TxPool { callbacks.call_reject(self, &entry, reject); } } - } else { - if let Some(id) = self - .proposed - .iter() - .min_by_key(|(_id, entry)| entry.as_evict_key()) - .map(|(id, _)| id) - .cloned() - { - let removed = self.proposed.remove_entry_and_descendants(&id); - for entry in removed { - let tx_hash = entry.transaction().hash(); - debug!( - "removed by size limit from proposed {} timestamp({})", - tx_hash, entry.timestamp - ); - let reject = Reject::Full(format!("fee: {}", entry.fee)); - callbacks.call_reject(self, &entry, reject); - } + } else if let Some(id) = self + .proposed + .iter() + .min_by_key(|(_id, entry)| entry.as_evict_key()) + .map(|(id, _)| id) + .cloned() + { + let removed = self.proposed.remove_entry_and_descendants(&id); + for entry in removed { + let tx_hash = entry.transaction().hash(); + debug!( + "removed by size limit from proposed {} timestamp({})", + tx_hash, entry.timestamp + ); + let reject = Reject::Full(format!("fee: {}", entry.fee)); + callbacks.call_reject(self, &entry, reject); } } } From e78a6eccea644405aec6c9f85203bcfe8b0b9755 Mon Sep 17 00:00:00 2001 From: zhangsoledad <787953403@qq.com> Date: Tue, 21 Mar 2023 20:53:31 +0800 Subject: [PATCH 4/8] feat: tweak default max_tx_pool_size --- resource/ckb.toml | 3 +-- test/src/specs/tx_pool/limit.rs | 2 +- test/template/ckb.toml | 2 +- tx-pool/src/pool.rs | 9 ++------- tx-pool/src/util.rs | 17 ---------------- util/app-config/src/configs/tx_pool.rs | 6 ++---- util/app-config/src/legacy/tx_pool.rs | 27 ++++++++++++++++++-------- 7 files changed, 26 insertions(+), 40 deletions(-) diff --git a/resource/ckb.toml b/resource/ckb.toml index 77f8082878..2eb5f3336a 100644 --- a/resource/ckb.toml +++ b/resource/ckb.toml @@ -132,8 +132,7 @@ enable_deprecated_rpc = false # {{ # }} [tx_pool] -max_mem_size = 20_000_000 # 20mb -max_cycles = 200_000_000_000 +max_tx_pool_size = 180_000_000 # 180mb min_fee_rate = 1_000 # shannons/KB max_tx_verify_cycles = 70_000_000 max_ancestors_count = 25 diff --git a/test/src/specs/tx_pool/limit.rs b/test/src/specs/tx_pool/limit.rs index 28da63962e..b4defc175b 100644 --- a/test/src/specs/tx_pool/limit.rs +++ b/test/src/specs/tx_pool/limit.rs @@ -55,7 +55,7 @@ impl Spec for SizeLimit { } fn modify_app_config(&self, config: &mut ckb_app_config::CKBAppConfig) { - config.tx_pool.max_mem_size = MAX_MEM_SIZE_FOR_SIZE_LIMIT; + config.tx_pool.max_tx_pool_size = MAX_MEM_SIZE_FOR_SIZE_LIMIT; config.tx_pool.min_fee_rate = FeeRate::zero(); } } diff --git a/test/template/ckb.toml b/test/template/ckb.toml index 44917e52db..b9bce4c01c 100644 --- a/test/template/ckb.toml +++ b/test/template/ckb.toml @@ -77,7 +77,7 @@ reject_ill_transactions = true enable_deprecated_rpc = true [tx_pool] -max_mem_size = 20_000_000 # 20mb +max_tx_pool_size = 180_000_000 # 180mb max_cycles = 200_000_000_000 min_fee_rate = 0 # shannons/KB max_tx_verify_cycles = 70_000_000 diff --git a/tx-pool/src/pool.rs b/tx-pool/src/pool.rs index 007257cc95..7e9a57c0bf 100644 --- a/tx-pool/src/pool.rs +++ b/tx-pool/src/pool.rs @@ -112,12 +112,7 @@ impl TxPool { /// Whether Tx-pool reach size limit pub fn reach_size_limit(&self, tx_size: usize) -> bool { - (self.total_tx_size + tx_size) > self.config.max_mem_size - } - - /// Whether Tx-pool reach cycles limit - pub fn reach_cycles_limit(&self, cycles: Cycle) -> bool { - (self.total_tx_cycles + cycles) > self.config.max_cycles + (self.total_tx_size + tx_size) > self.config.max_tx_pool_size } /// Update size and cycles statics for add tx @@ -329,7 +324,7 @@ impl TxPool { // Remove transactions from the pool until total size < size_limit. pub(crate) fn limit_size(&mut self, callbacks: &Callbacks) { - while self.total_tx_size > self.config.max_mem_size { + while self.total_tx_size > self.config.max_tx_pool_size { if !self.pending.is_empty() { if let Some(id) = self .pending diff --git a/tx-pool/src/util.rs b/tx-pool/src/util.rs index 8bfff54124..040767f1ea 100644 --- a/tx-pool/src/util.rs +++ b/tx-pool/src/util.rs @@ -24,23 +24,6 @@ pub(crate) fn check_txid_collision(tx_pool: &TxPool, tx: &TransactionView) -> Re Ok(()) } -// pub(crate) fn check_tx_size_limit(tx_pool: &TxPool, tx_size: usize) -> Result<(), Reject> { -// if tx_pool.reach_size_limit(tx_size) { -// return Err(Reject::Full( -// "size".to_owned(), -// tx_pool.config.max_mem_size as u64, -// )); -// } -// Ok(()) -// } - -// pub(crate) fn check_tx_cycle_limit(tx_pool: &TxPool, cycles: Cycle) -> Result<(), Reject> { -// if tx_pool.reach_cycles_limit(cycles) { -// return Err(Reject::Full("cycles".to_owned(), tx_pool.config.max_cycles)); -// } -// Ok(()) -// } - pub(crate) fn check_tx_fee( tx_pool: &TxPool, snapshot: &Snapshot, diff --git a/util/app-config/src/configs/tx_pool.rs b/util/app-config/src/configs/tx_pool.rs index 057576ddf3..b71223ef7e 100644 --- a/util/app-config/src/configs/tx_pool.rs +++ b/util/app-config/src/configs/tx_pool.rs @@ -9,10 +9,8 @@ use url::Url; /// Transaction pool configuration #[derive(Clone, Debug, Serialize)] pub struct TxPoolConfig { - /// Keep the transaction pool below mb - pub max_mem_size: usize, - /// Keep the transaction pool below cycles - pub max_cycles: Cycle, + /// Keep the transaction pool below mb + pub max_tx_pool_size: usize, /// txs with lower fee rate than this will not be relayed or be mined #[serde(with = "FeeRateDef")] pub min_fee_rate: FeeRate, diff --git a/util/app-config/src/legacy/tx_pool.rs b/util/app-config/src/legacy/tx_pool.rs index f4ae0e20e4..00c0c6390d 100644 --- a/util/app-config/src/legacy/tx_pool.rs +++ b/util/app-config/src/legacy/tx_pool.rs @@ -13,12 +13,17 @@ const DEFAULT_MAX_TX_VERIFY_CYCLES: Cycle = TWO_IN_TWO_OUT_CYCLES * 20; const DEFAULT_MAX_ANCESTORS_COUNT: usize = 125; // Default expiration time for pool transactions in hours const DEFAULT_EXPIRY_HOURS: u8 = 24; +// Default max_tx_pool_size 180mb +const DEFAULT_MAX_TX_POOL_SIZE: usize = 180_000_000; #[derive(Clone, Debug, Deserialize)] #[serde(deny_unknown_fields)] +#[allow(dead_code)] pub(crate) struct TxPoolConfig { - max_mem_size: usize, - max_cycles: Cycle, + #[serde(default = "default_max_tx_pool_size")] + max_tx_pool_size: usize, + max_mem_size: Option, + max_cycles: Option, pub(crate) max_verify_cache_size: Option, pub(crate) max_conflict_cache_size: Option, pub(crate) max_committed_txs_hash_cache_size: Option, @@ -50,6 +55,10 @@ fn default_expiry_hours() -> u8 { DEFAULT_EXPIRY_HOURS } +fn default_max_tx_pool_size() -> usize { + DEFAULT_MAX_TX_POOL_SIZE +} + impl Default for crate::TxPoolConfig { fn default() -> Self { TxPoolConfig::default().into() @@ -59,8 +68,9 @@ impl Default for crate::TxPoolConfig { impl Default for TxPoolConfig { fn default() -> Self { Self { - max_mem_size: 20_000_000, // 20mb - max_cycles: 200_000_000_000, + max_mem_size: None, + max_tx_pool_size: DEFAULT_MAX_TX_POOL_SIZE, + max_cycles: None, max_verify_cache_size: None, max_conflict_cache_size: None, max_committed_txs_hash_cache_size: None, @@ -79,8 +89,9 @@ impl Default for TxPoolConfig { impl From for crate::TxPoolConfig { fn from(input: TxPoolConfig) -> Self { let TxPoolConfig { - max_mem_size, - max_cycles, + max_mem_size: _, + max_tx_pool_size, + max_cycles: _, max_verify_cache_size: _, max_conflict_cache_size: _, max_committed_txs_hash_cache_size: _, @@ -93,9 +104,9 @@ impl From for crate::TxPoolConfig { recent_reject, expiry_hours, } = input; + Self { - max_mem_size, - max_cycles, + max_tx_pool_size, min_fee_rate, max_tx_verify_cycles, max_ancestors_count: cmp::max(DEFAULT_MAX_ANCESTORS_COUNT, max_ancestors_count), From 3061dcf09bb5aa7a6a5344b4e28c3cc70b9c2c5c Mon Sep 17 00:00:00 2001 From: zhangsoledad <787953403@qq.com> Date: Tue, 21 Mar 2023 21:00:58 +0800 Subject: [PATCH 5/8] feat: tweak default expiration time for tx-pool --- rpc/README.md | 9 +++++++-- util/app-config/src/legacy/tx_pool.rs | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/rpc/README.md b/rpc/README.md index c1cba7f132..f3800eeb65 100644 --- a/rpc/README.md +++ b/rpc/README.md @@ -4986,6 +4986,10 @@ For example, a cellbase transaction is not allowed in `send_transaction` RPC. (-1109): The transaction is expired from tx-pool after `expiry_hours`. +### Error `PoolRejectedTransactionBySizeLimit` + +(-1110): The transaction exceeded maximum size limit. + ### Error `Indexer` (-1200): The indexer error. @@ -6329,14 +6333,15 @@ TX reject message `PoolTransactionReject` is a JSON object with following fields. -* `type`: `"LowFeeRate" | "ExceededMaximumAncestorsCount" | "Full" | "Duplicated" | "Malformed" | "DeclaredWrongCycles" | "Resolve" | "Verification" | "Expiry"` - Reject type. +* `type`: `"LowFeeRate" | "ExceededMaximumAncestorsCount" | "ExceededTransactionSizeLimit" | "Full" | "Duplicated" | "Malformed" | "DeclaredWrongCycles" | "Resolve" | "Verification" | "Expiry"` - Reject type. * `description`: `string` - Detailed description about why the transaction is rejected. Different reject types: * `LowFeeRate`: Transaction fee lower than config * `ExceededMaximumAncestorsCount`: Transaction exceeded maximum ancestors count limit -* `Full`: Transaction pool exceeded maximum size or cycles limit, +* `ExceededTransactionSizeLimit`: Transaction exceeded maximum size limit +* `Full`: Transaction are replaced because the pool is full * `Duplicated`: Transaction already exist in transaction_pool * `Malformed`: Malformed transaction * `DeclaredWrongCycles`: Declared wrong cycles diff --git a/util/app-config/src/legacy/tx_pool.rs b/util/app-config/src/legacy/tx_pool.rs index 00c0c6390d..bf82ecc1af 100644 --- a/util/app-config/src/legacy/tx_pool.rs +++ b/util/app-config/src/legacy/tx_pool.rs @@ -12,7 +12,7 @@ const DEFAULT_MAX_TX_VERIFY_CYCLES: Cycle = TWO_IN_TWO_OUT_CYCLES * 20; // default max ancestors count const DEFAULT_MAX_ANCESTORS_COUNT: usize = 125; // Default expiration time for pool transactions in hours -const DEFAULT_EXPIRY_HOURS: u8 = 24; +const DEFAULT_EXPIRY_HOURS: u8 = 12; // Default max_tx_pool_size 180mb const DEFAULT_MAX_TX_POOL_SIZE: usize = 180_000_000; From 7e46eaca2f55de812a7a96dec0240fc2f379b59f Mon Sep 17 00:00:00 2001 From: zhangsoledad <787953403@qq.com> Date: Wed, 22 Mar 2023 10:58:22 +0800 Subject: [PATCH 6/8] chore: remove duplicate code with macro refactoring --- tx-pool/src/pool.rs | 80 +++++++++++++++------------------------------ 1 file changed, 27 insertions(+), 53 deletions(-) diff --git a/tx-pool/src/pool.rs b/tx-pool/src/pool.rs index 7e9a57c0bf..73f0e02832 100644 --- a/tx-pool/src/pool.rs +++ b/tx-pool/src/pool.rs @@ -26,6 +26,29 @@ use std::sync::Arc; const COMMITTED_HASH_CACHE_SIZE: usize = 100_000; +// limit the size of the pool by sorting out tx based on EvictKey. +macro_rules! evict_for_trim_size { + ($self:ident, $pool:expr, $callbacks:expr) => { + if let Some(id) = $pool + .iter() + .min_by_key(|(_id, entry)| entry.as_evict_key()) + .map(|(id, _)| id) + .cloned() + { + let removed = $pool.remove_entry_and_descendants(&id); + for entry in removed { + let tx_hash = entry.transaction().hash(); + debug!( + "removed by size limit {} timestamp({})", + tx_hash, entry.timestamp + ); + let reject = Reject::Full(format!("fee: {}", entry.fee)); + $callbacks.call_reject($self, &entry, reject); + } + } + }; +} + /// Tx-pool implementation pub struct TxPool { pub(crate) config: TxPoolConfig, @@ -326,60 +349,11 @@ impl TxPool { pub(crate) fn limit_size(&mut self, callbacks: &Callbacks) { while self.total_tx_size > self.config.max_tx_pool_size { if !self.pending.is_empty() { - if let Some(id) = self - .pending - .iter() - .min_by_key(|(_id, entry)| entry.as_evict_key()) - .map(|(id, _)| id) - .cloned() - { - let removed = self.pending.remove_entry_and_descendants(&id); - for entry in removed { - let tx_hash = entry.transaction().hash(); - debug!( - "removed by size limit from pending {} timestamp({})", - tx_hash, entry.timestamp - ); - let reject = Reject::Full(format!("fee: {}", entry.fee)); - callbacks.call_reject(self, &entry, reject); - } - } + evict_for_trim_size!(self, self.pending, callbacks) } else if !self.gap.is_empty() { - if let Some(id) = self - .gap - .iter() - .min_by_key(|(_id, entry)| entry.as_evict_key()) - .map(|(id, _)| id) - .cloned() - { - let removed = self.gap.remove_entry_and_descendants(&id); - for entry in removed { - let tx_hash = entry.transaction().hash(); - debug!( - "removed by size limit from gap {} timestamp({})", - tx_hash, entry.timestamp - ); - let reject = Reject::Full(format!("fee: {}", entry.fee)); - callbacks.call_reject(self, &entry, reject); - } - } - } else if let Some(id) = self - .proposed - .iter() - .min_by_key(|(_id, entry)| entry.as_evict_key()) - .map(|(id, _)| id) - .cloned() - { - let removed = self.proposed.remove_entry_and_descendants(&id); - for entry in removed { - let tx_hash = entry.transaction().hash(); - debug!( - "removed by size limit from proposed {} timestamp({})", - tx_hash, entry.timestamp - ); - let reject = Reject::Full(format!("fee: {}", entry.fee)); - callbacks.call_reject(self, &entry, reject); - } + evict_for_trim_size!(self, self.gap, callbacks) + } else { + evict_for_trim_size!(self, self.proposed, callbacks) } } } From 72918fbb84f38ea88a47ae2a77db257e1b86a13d Mon Sep 17 00:00:00 2001 From: zhangsoledad <787953403@qq.com> Date: Wed, 22 Mar 2023 17:26:18 +0800 Subject: [PATCH 7/8] chore: improve error message --- resource/ckb.toml | 2 +- rpc/src/tests/error.rs | 9 ++++++--- .../relay/transaction_relay_low_fee_rate.rs | 2 +- test/template/ckb.toml | 3 +-- tx-pool/src/component/entry.rs | 16 +++++++++++----- tx-pool/src/pool.rs | 5 ++++- tx-pool/src/process.rs | 2 +- util/types/src/core/fee_rate.rs | 2 +- util/types/src/core/tx_pool.rs | 4 ++-- 9 files changed, 28 insertions(+), 17 deletions(-) diff --git a/resource/ckb.toml b/resource/ckb.toml index 2eb5f3336a..7cb1f2321d 100644 --- a/resource/ckb.toml +++ b/resource/ckb.toml @@ -133,7 +133,7 @@ enable_deprecated_rpc = false # {{ [tx_pool] max_tx_pool_size = 180_000_000 # 180mb -min_fee_rate = 1_000 # shannons/KB +min_fee_rate = 1_000 # Here fee_rate are calculated directly using size in units of shannons/KB max_tx_verify_cycles = 70_000_000 max_ancestors_count = 25 diff --git a/rpc/src/tests/error.rs b/rpc/src/tests/error.rs index 8235ab180d..fc32e7f253 100644 --- a/rpc/src/tests/error.rs +++ b/rpc/src/tests/error.rs @@ -22,7 +22,7 @@ fn test_submit_transaction_error() { let min_fee_rate = FeeRate::from_u64(500); let reject = Reject::LowFeeRate(min_fee_rate, 100, 50); assert_eq!( - "PoolRejectedTransactionByMinFeeRate: The min fee rate is 500 shannons/KB, so the transaction fee should be 100 shannons at least, but only got 50", + "PoolRejectedTransactionByMinFeeRate: The min fee rate is 500 shannons/KW, so the transaction fee should be 100 shannons at least, but only got 50", RPCError::from_submit_transaction_reject(&reject).message ); @@ -32,9 +32,12 @@ fn test_submit_transaction_error() { RPCError::from_submit_transaction_reject(&reject).message ); - let reject = Reject::Full("size".to_owned()); + let reject = Reject::Full(format!( + "the fee_rate for this transaction is: {}", + FeeRate::from_u64(500) + )); assert_eq!( - "PoolIsFull: Transaction are replaced because the pool is full size", + "PoolIsFull: Transaction are replaced because the pool is full, the fee_rate for this transaction is: 500 shannons/KW", RPCError::from_submit_transaction_reject(&reject).message ); diff --git a/test/src/specs/relay/transaction_relay_low_fee_rate.rs b/test/src/specs/relay/transaction_relay_low_fee_rate.rs index 51a3c8b9e1..dfe086636d 100644 --- a/test/src/specs/relay/transaction_relay_low_fee_rate.rs +++ b/test/src/specs/relay/transaction_relay_low_fee_rate.rs @@ -36,7 +36,7 @@ impl Spec for TransactionRelayLowFeeRate { node1, 0, 10, - "reject tx The min fee rate is 1000 shannons/KB, so the transaction fee should be 242 shannons at least, but only got 0" + "reject tx The min fee rate is 1000 shannons/KW, so the transaction fee should be 242 shannons at least, but only got 0" ) .is_some()); } diff --git a/test/template/ckb.toml b/test/template/ckb.toml index b9bce4c01c..c722fa1e55 100644 --- a/test/template/ckb.toml +++ b/test/template/ckb.toml @@ -78,8 +78,7 @@ enable_deprecated_rpc = true [tx_pool] max_tx_pool_size = 180_000_000 # 180mb -max_cycles = 200_000_000_000 -min_fee_rate = 0 # shannons/KB +min_fee_rate = 0 # Here fee_rate are calculated directly using size in units of shannons/KB max_tx_verify_cycles = 70_000_000 max_ancestors_count = 25 diff --git a/tx-pool/src/component/entry.rs b/tx-pool/src/component/entry.rs index a96b01f54c..6a1d6028cc 100644 --- a/tx-pool/src/component/entry.rs +++ b/tx-pool/src/component/entry.rs @@ -99,6 +99,12 @@ impl TxEntry { EvictKey::from(self) } + /// Returns fee rate + pub fn fee_rate(&self) -> FeeRate { + let weight = get_transaction_weight(self.size, self.cycles); + FeeRate::calculate(self.fee, weight) + } + /// Update ancestor state for add an entry pub fn add_entry_weight(&mut self, entry: &TxEntry) { self.ancestors_count = self.ancestors_count.saturating_add(1); @@ -184,9 +190,10 @@ impl Ord for TxEntry { } /// Currently we do not have trace descendants, -/// so we take the simplest strategy to evict the transactions by first comparing the fee_rate, -/// and then taking the one with the largest timestamp, -/// which also means that the fewer descendants may exist. +/// so first take the simplest strategy, +/// first compare fee_rate, select the smallest fee_rate, +/// and then select the latest timestamp, for eviction, +/// the latest timestamp which also means that the fewer descendants may exist. #[derive(Eq, PartialEq, Clone, Debug)] pub struct EvictKey { fee_rate: FeeRate, @@ -195,9 +202,8 @@ pub struct EvictKey { impl From<&TxEntry> for EvictKey { fn from(entry: &TxEntry) -> Self { - let weight = get_transaction_weight(entry.size, entry.cycles); EvictKey { - fee_rate: FeeRate::calculate(entry.fee, weight), + fee_rate: entry.fee_rate(), timestamp: entry.timestamp, } } diff --git a/tx-pool/src/pool.rs b/tx-pool/src/pool.rs index 73f0e02832..3443ff0c76 100644 --- a/tx-pool/src/pool.rs +++ b/tx-pool/src/pool.rs @@ -42,7 +42,10 @@ macro_rules! evict_for_trim_size { "removed by size limit {} timestamp({})", tx_hash, entry.timestamp ); - let reject = Reject::Full(format!("fee: {}", entry.fee)); + let reject = Reject::Full(format!( + "the fee_rate for this transaction is: {}", + entry.fee_rate() + )); $callbacks.call_reject($self, &entry, reject); } } diff --git a/tx-pool/src/process.rs b/tx-pool/src/process.rs index f1df3e5fc9..3907c1e11d 100644 --- a/tx-pool/src/process.rs +++ b/tx-pool/src/process.rs @@ -959,7 +959,7 @@ fn _update_tx_pool_for_reorg( // Remove expired transaction from pending tx_pool.remove_expired(callbacks); - // Remove transactions from the pool until its size < size_limit. + // Remove transactions from the pool until its size <= size_limit. tx_pool.limit_size(callbacks); } diff --git a/util/types/src/core/fee_rate.rs b/util/types/src/core/fee_rate.rs index 8bb2caee30..edb4cc8f0c 100644 --- a/util/types/src/core/fee_rate.rs +++ b/util/types/src/core/fee_rate.rs @@ -39,6 +39,6 @@ impl FeeRate { impl ::std::fmt::Display for FeeRate { fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result { - write!(f, "{}", self.0) + write!(f, "{} shannons/KW", self.0) } } diff --git a/util/types/src/core/tx_pool.rs b/util/types/src/core/tx_pool.rs index b3ff936056..b1eac056f1 100644 --- a/util/types/src/core/tx_pool.rs +++ b/util/types/src/core/tx_pool.rs @@ -14,7 +14,7 @@ use std::collections::HashMap; #[derive(Error, Debug, Clone)] pub enum Reject { /// Transaction fee lower than config - #[error("The min fee rate is {0} shannons/KB, so the transaction fee should be {1} shannons at least, but only got {2}")] + #[error("The min fee rate is {0}, so the transaction fee should be {1} shannons at least, but only got {2}")] LowFeeRate(FeeRate, u64, u64), /// Transaction exceeded maximum ancestors count limit @@ -26,7 +26,7 @@ pub enum Reject { ExceededTransactionSizeLimit(u64, u64), /// Transaction are replaced because the pool is full - #[error("Transaction are replaced because the pool is full {0}")] + #[error("Transaction are replaced because the pool is full, {0}")] Full(String), /// Transaction already exist in transaction_pool From 98678a812023cfbbae363912995e4d3c3eb51458 Mon Sep 17 00:00:00 2001 From: zhangsoledad <787953403@qq.com> Date: Thu, 23 Mar 2023 11:35:17 +0800 Subject: [PATCH 8/8] feat: RPC tx_pool_info add tx_size_limit and max_tx_pool_size --- rpc/README.md | 12 +++++++-- rpc/src/module/pool.rs | 21 ++++------------ rpc/src/service_builder.rs | 4 +-- rpc/src/tests/examples.rs | 6 ++--- rpc/src/tests/mod.rs | 6 ++--- tx-pool/src/pool.rs | 33 ------------------------ tx-pool/src/service.rs | 7 ++++-- util/jsonrpc-types/src/pool.rs | 30 +++++++++++++++++++++- util/launcher/src/lib.rs | 1 - util/types/src/core/tx_pool.rs | 46 +++++++++++++++++++++++++++++++++- 10 files changed, 101 insertions(+), 65 deletions(-) diff --git a/rpc/README.md b/rpc/README.md index f3800eeb65..2a99fb7f50 100644 --- a/rpc/README.md +++ b/rpc/README.md @@ -4435,14 +4435,16 @@ Response "jsonrpc": "2.0", "result": { "last_txs_updated_at": "0x0", - "min_fee_rate": "0x0", + "min_fee_rate": "0x3e8", + "max_tx_pool_size": "0xaba9500", "orphan": "0x0", "pending": "0x1", "proposed": "0x0", "tip_hash": "0xa5f5c85987a15de25661e5a214f2c1449cd803f071acc7999820f25246471f40", "tip_number": "0x400", "total_tx_cycles": "0x219", - "total_tx_size": "0x112" + "total_tx_size": "0x112", + "tx_size_limit": "0x7d000" } } ``` @@ -6916,6 +6918,12 @@ Transaction pool information. * `last_txs_updated_at`: [`Timestamp`](#type-timestamp) - Last updated time. This is the Unix timestamp in milliseconds. +* `tx_size_limit`: [`Uint64`](#type-uint64) - Limiting transactions to tx_size_limit + + Transactions with a large size close to the block size limit may not be packaged, because the block header and cellbase are occupied, so the tx-pool is limited to accepting transaction up to tx_size_limit. + +* `max_tx_pool_size`: [`Uint64`](#type-uint64) - Total limit on the size of transactions in the tx-pool + ### Type `TxStatus` diff --git a/rpc/src/module/pool.rs b/rpc/src/module/pool.rs index 7013787e37..d2832c734b 100644 --- a/rpc/src/module/pool.rs +++ b/rpc/src/module/pool.rs @@ -161,14 +161,16 @@ pub trait PoolRpc { /// "jsonrpc": "2.0", /// "result": { /// "last_txs_updated_at": "0x0", - /// "min_fee_rate": "0x0", + /// "min_fee_rate": "0x3e8", + /// "max_tx_pool_size": "0xaba9500", /// "orphan": "0x0", /// "pending": "0x1", /// "proposed": "0x0", /// "tip_hash": "0xa5f5c85987a15de25661e5a214f2c1449cd803f071acc7999820f25246471f40", /// "tip_number": "0x400", /// "total_tx_cycles": "0x219", - /// "total_tx_size": "0x112" + /// "total_tx_size": "0x112", + /// "tx_size_limit": "0x7d000" /// } /// } /// ``` @@ -276,7 +278,6 @@ pub trait PoolRpc { pub(crate) struct PoolRpcImpl { shared: Shared, - min_fee_rate: core::FeeRate, well_known_lock_scripts: Vec, well_known_type_scripts: Vec, } @@ -284,7 +285,6 @@ pub(crate) struct PoolRpcImpl { impl PoolRpcImpl { pub fn new( shared: Shared, - min_fee_rate: core::FeeRate, mut extra_well_known_lock_scripts: Vec, mut extra_well_known_type_scripts: Vec, ) -> PoolRpcImpl { @@ -298,7 +298,6 @@ impl PoolRpcImpl { PoolRpcImpl { shared, - min_fee_rate, well_known_lock_scripts, well_known_type_scripts, } @@ -449,17 +448,7 @@ impl PoolRpc for PoolRpcImpl { let tx_pool_info = get_tx_pool_info.unwrap(); - Ok(TxPoolInfo { - tip_hash: tx_pool_info.tip_hash.unpack(), - tip_number: tx_pool_info.tip_number.into(), - pending: (tx_pool_info.pending_size as u64).into(), - proposed: (tx_pool_info.proposed_size as u64).into(), - orphan: (tx_pool_info.orphan_size as u64).into(), - total_tx_size: (tx_pool_info.total_tx_size as u64).into(), - total_tx_cycles: tx_pool_info.total_tx_cycles.into(), - min_fee_rate: self.min_fee_rate.as_u64().into(), - last_txs_updated_at: tx_pool_info.last_txs_updated_at.into(), - }) + Ok(tx_pool_info.into()) } fn clear_tx_pool(&self) -> Result<()> { diff --git a/rpc/src/service_builder.rs b/rpc/src/service_builder.rs index c72953d0ef..19372ce8a0 100644 --- a/rpc/src/service_builder.rs +++ b/rpc/src/service_builder.rs @@ -15,7 +15,7 @@ use ckb_network_alert::{notifier::Notifier as AlertNotifier, verifier::Verifier use ckb_pow::Pow; use ckb_shared::shared::Shared; use ckb_sync::SyncShared; -use ckb_types::{core::FeeRate, packed::Script}; +use ckb_types::packed::Script; use ckb_util::Mutex; use jsonrpc_core::RemoteProcedure; use std::sync::Arc; @@ -52,13 +52,11 @@ impl<'a> ServiceBuilder<'a> { pub fn enable_pool( mut self, shared: Shared, - min_fee_rate: FeeRate, extra_well_known_lock_scripts: Vec