From a0290f50429a5fa0f977926cb970068ed8a47422 Mon Sep 17 00:00:00 2001 From: yukang Date: Mon, 22 Jan 2024 19:17:02 +0800 Subject: [PATCH] don't acount for cell dep for MAX_ANCESTORS_COUNT --- rpc/src/tests/module/pool.rs | 3 +- test/src/main.rs | 1 + test/src/specs/tx_pool/limit.rs | 83 +++++++++++++++++++++++++++++-- tx-pool/src/component/entry.rs | 3 ++ tx-pool/src/component/links.rs | 20 +++++++- tx-pool/src/component/pool_map.rs | 24 +++++++-- 6 files changed, 124 insertions(+), 10 deletions(-) diff --git a/rpc/src/tests/module/pool.rs b/rpc/src/tests/module/pool.rs index 5d9ba19834..f934b06e53 100644 --- a/rpc/src/tests/module/pool.rs +++ b/rpc/src/tests/module/pool.rs @@ -184,7 +184,8 @@ fn test_send_transaction_exceeded_maximum_ancestors_count() { }); } - // the default value of pool config `max_ancestors_count` is 125, only 125 txs will be added to committed list of the block template + // the default value of pool config `max_ancestors_count` is 125, + // only 125 txs will be added to committed list of the block template suite.wait_block_template_array_ge("transactions", 1); let response = suite.rpc(&RpcTestRequest { diff --git a/test/src/main.rs b/test/src/main.rs index f8f6b16610..4a2a097104 100644 --- a/test/src/main.rs +++ b/test/src/main.rs @@ -488,6 +488,7 @@ fn all_specs() -> Vec> { Box::new(CompactBlockRelayLessThenSharedBestKnown), Box::new(InvalidLocatorSize), Box::new(SizeLimit), + Box::new(TxPoolLimitAncestorCount), 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 ad84e36124..6840558288 100644 --- a/test/src/specs/tx_pool/limit.rs +++ b/test/src/specs/tx_pool/limit.rs @@ -1,9 +1,16 @@ -use crate::{Node, Spec}; - +use crate::{ + util::{cell::gen_spendable, transaction::always_success_transaction}, + Node, Spec, +}; use ckb_logger::info; -use ckb_types::core::FeeRate; +use ckb_types::{ + core::{cell::CellMetaBuilder, DepType, FeeRate}, + packed::CellDepBuilder, +}; use std::{thread::sleep, time::Duration}; +use ckb_types::{packed::OutPoint, prelude::*}; + pub struct SizeLimit; const MAX_MEM_SIZE_FOR_SIZE_LIMIT: usize = 2000; @@ -59,3 +66,73 @@ impl Spec for SizeLimit { config.tx_pool.min_fee_rate = FeeRate::zero(); } } + +pub struct TxPoolLimitAncestorCount; +impl Spec for TxPoolLimitAncestorCount { + fn run(&self, nodes: &mut Vec) { + let node0 = &nodes[0]; + + let initial_inputs = gen_spendable(node0, 130); + let input_a = &initial_inputs[0]; + + // Commit transaction root + let tx_a = { + let tx_a = always_success_transaction(node0, input_a); + node0.submit_transaction(&tx_a); + tx_a + }; + + let cell_dep = CellDepBuilder::default() + .dep_type(DepType::Code.into()) + .out_point(OutPoint::new(tx_a.hash(), 0)) + .build(); + + // Create 125 transactions cell dep on tx_a + for i in 1..=125 { + let cur = always_success_transaction(node0, initial_inputs.get(i).unwrap()); + let cur = cur.as_advanced_builder().cell_dep(cell_dep.clone()).build(); + let _ = node0.rpc_client().send_transaction(cur.data().into()); + } + + // Create a new transaction consume the cell dep, it will be succeed in submit + let input = CellMetaBuilder::from_cell_output(tx_a.output(0).unwrap(), Default::default()) + .out_point(OutPoint::new(tx_a.hash(), 0)) + .build(); + let last = always_success_transaction(node0, &input); + + let res = node0 + .rpc_client() + .send_transaction_result(last.data().into()); + assert!(res.is_ok()); + + // create a transaction chain + let input_c = &initial_inputs[129]; + // Commit transaction root + let tx_c = { + let tx_c = always_success_transaction(node0, input_c); + node0.submit_transaction(&tx_c); + tx_c + }; + + let mut prev = tx_c.clone(); + // Create transaction chain + for i in 0..125 { + let input = + CellMetaBuilder::from_cell_output(prev.output(0).unwrap(), Default::default()) + .out_point(OutPoint::new(prev.hash(), 0)) + .build(); + let cur = always_success_transaction(node0, &input); + let res = node0 + .rpc_client() + .send_transaction_result(cur.data().into()); + prev = cur.clone(); + if i >= 124 { + assert!(res.is_err()); + let msg = res.err().unwrap().to_string(); + assert!(msg.contains("PoolRejectedTransactionByMaxAncestorsCountLimit")); + } else { + assert!(res.is_ok()); + } + } + } +} diff --git a/tx-pool/src/component/entry.rs b/tx-pool/src/component/entry.rs index f45d4feace..07076c6c11 100644 --- a/tx-pool/src/component/entry.rs +++ b/tx-pool/src/component/entry.rs @@ -39,6 +39,8 @@ pub struct TxEntry { pub descendants_cycles: Cycle, /// descendants txs count pub descendants_count: usize, + /// dicrect ancestors txs count + pub direct_ancestors_count: usize, /// The unix timestamp when entering the Txpool, unit: Millisecond pub timestamp: u64, } @@ -71,6 +73,7 @@ impl TxEntry { descendants_cycles: cycles, descendants_count: 1, ancestors_count: 1, + direct_ancestors_count: 1, } } diff --git a/tx-pool/src/component/links.rs b/tx-pool/src/component/links.rs index 5fc681715b..106f9a216d 100644 --- a/tx-pool/src/component/links.rs +++ b/tx-pool/src/component/links.rs @@ -5,12 +5,14 @@ use std::collections::{HashMap, HashSet}; pub struct TxLinks { pub parents: HashSet, pub children: HashSet, + pub direct_parents: HashSet, } -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Eq, PartialEq)] pub enum Relation { Parents, Children, + DirectParents, } impl TxLinks { @@ -18,6 +20,7 @@ impl TxLinks { match relation { Relation::Parents => &self.parents, Relation::Children => &self.children, + Relation::DirectParents => &self.direct_parents, } } } @@ -68,6 +71,11 @@ impl TxLinksMap { stage.remove(&id); relation_ids.insert(id); } + // for direct parents, we don't store children in links map + // so filter those not in links map now, they maybe removed from tx-pool now + if relation == Relation::DirectParents { + relation_ids.retain(|id| self.inner.contains_key(id)); + } relation_ids } @@ -131,6 +139,16 @@ impl TxLinksMap { .map(|links| links.parents.insert(parent)) } + pub fn add_direct_parent( + &mut self, + short_id: &ProposalShortId, + parent: ProposalShortId, + ) -> Option { + self.inner + .get_mut(short_id) + .map(|links| links.direct_parents.insert(parent)) + } + pub fn clear(&mut self) { self.inner.clear(); } diff --git a/tx-pool/src/component/pool_map.rs b/tx-pool/src/component/pool_map.rs index a37d66243b..36adb4f7d7 100644 --- a/tx-pool/src/component/pool_map.rs +++ b/tx-pool/src/component/pool_map.rs @@ -429,6 +429,7 @@ impl PoolMap { let tx_short_id: ProposalShortId = entry.proposal_short_id(); let outputs = entry.transaction().output_pts(); let mut children = HashSet::new(); + let mut direct_children = HashSet::new(); // collect children for o in outputs { @@ -436,7 +437,8 @@ impl PoolMap { children.extend(ids); } if let Some(id) = self.edges.get_input_ref(&o).cloned() { - children.insert(id); + children.insert(id.clone()); + direct_children.insert(id); } } // update children @@ -444,6 +446,9 @@ impl PoolMap { for child in &children { self.links.add_parent(child, tx_short_id.clone()); } + for child in &direct_children { + self.links.add_direct_parent(child, tx_short_id.clone()); + } if let Some(links) = self.links.inner.get_mut(&tx_short_id) { links.children.extend(children); } @@ -458,8 +463,10 @@ impl PoolMap { let mut parents: HashSet = HashSet::with_capacity( entry.transaction().inputs().len() + entry.transaction().cell_deps().len(), ); - let short_id = entry.proposal_short_id(); + let mut direct_parents: HashSet = + HashSet::with_capacity(entry.transaction().inputs().len()); + let short_id = entry.proposal_short_id(); for input in entry.transaction().inputs() { let input_pt = input.previous_output(); if let Some(deps) = self.edges.deps.get(&input_pt) { @@ -469,7 +476,8 @@ impl PoolMap { let parent_hash = &input_pt.tx_hash(); let id = ProposalShortId::from_tx_hash(parent_hash); if self.links.inner.contains_key(&id) { - parents.insert(id); + parents.insert(id.clone()); + direct_parents.insert(id); } } for cell_dep in entry.transaction().cell_deps() { @@ -484,13 +492,18 @@ impl PoolMap { .links .calc_relation_ids(parents.clone(), Relation::Parents); + let direct_ancestors = self + .links + .calc_relation_ids(direct_parents.clone(), Relation::DirectParents); + // update parents references for ancestor_id in &ancestors { let ancestor = self.get_by_id_checked(ancestor_id); + // cell deps don't accord into ancestors entry.add_ancestor_weight(&ancestor.inner); } - if entry.ancestors_count > self.max_ancestors_count { - debug!("debug: exceeded maximum ancestors count"); + entry.direct_ancestors_count = direct_ancestors.len() + 1; + if entry.direct_ancestors_count > self.max_ancestors_count { return Err(Reject::ExceededMaximumAncestorsCount); } @@ -500,6 +513,7 @@ impl PoolMap { let links = TxLinks { parents, + direct_parents, children: Default::default(), }; self.links.inner.insert(short_id, links);