Skip to content

Commit

Permalink
don't acount for cell dep for MAX_ANCESTORS_COUNT
Browse files Browse the repository at this point in the history
  • Loading branch information
chenyukang committed Jan 24, 2024
1 parent 194d935 commit a0290f5
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 10 deletions.
3 changes: 2 additions & 1 deletion rpc/src/tests/module/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions test/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ fn all_specs() -> Vec<Box<dyn Spec>> {
Box::new(CompactBlockRelayLessThenSharedBestKnown),
Box::new(InvalidLocatorSize),
Box::new(SizeLimit),
Box::new(TxPoolLimitAncestorCount),
Box::new(SendDefectedBinary::new(
"send_defected_binary_reject_known_bugs",
true,
Expand Down
83 changes: 80 additions & 3 deletions test/src/specs/tx_pool/limit.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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<Node>) {
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());
}
}
}
}
3 changes: 3 additions & 0 deletions tx-pool/src/component/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down Expand Up @@ -71,6 +73,7 @@ impl TxEntry {
descendants_cycles: cycles,
descendants_count: 1,
ancestors_count: 1,
direct_ancestors_count: 1,
}
}

Expand Down
20 changes: 19 additions & 1 deletion tx-pool/src/component/links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,22 @@ use std::collections::{HashMap, HashSet};
pub struct TxLinks {
pub parents: HashSet<ProposalShortId>,
pub children: HashSet<ProposalShortId>,
pub direct_parents: HashSet<ProposalShortId>,
}

#[derive(Clone, Copy)]
#[derive(Clone, Copy, Eq, PartialEq)]
pub enum Relation {
Parents,
Children,
DirectParents,
}

impl TxLinks {
fn get_direct_ids(&self, relation: Relation) -> &HashSet<ProposalShortId> {
match relation {
Relation::Parents => &self.parents,
Relation::Children => &self.children,
Relation::DirectParents => &self.direct_parents,
}
}
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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<bool> {
self.inner
.get_mut(short_id)
.map(|links| links.direct_parents.insert(parent))
}

pub fn clear(&mut self) {
self.inner.clear();
}
Expand Down
24 changes: 19 additions & 5 deletions tx-pool/src/component/pool_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,21 +429,26 @@ 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 {
if let Some(ids) = self.edges.get_deps_ref(&o).cloned() {
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
if !children.is_empty() {
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);
}
Expand All @@ -458,8 +463,10 @@ impl PoolMap {
let mut parents: HashSet<ProposalShortId> = HashSet::with_capacity(
entry.transaction().inputs().len() + entry.transaction().cell_deps().len(),
);
let short_id = entry.proposal_short_id();
let mut direct_parents: HashSet<ProposalShortId> =
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) {
Expand All @@ -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() {
Expand All @@ -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);
}

Expand All @@ -500,6 +513,7 @@ impl PoolMap {

let links = TxLinks {
parents,
direct_parents,
children: Default::default(),
};
self.links.inner.insert(short_id, links);
Expand Down

0 comments on commit a0290f5

Please sign in to comment.