From f7d8e475ac8103fe92ed3783f3be9891590062a9 Mon Sep 17 00:00:00 2001 From: yukang Date: Tue, 31 Oct 2023 18:40:27 +0800 Subject: [PATCH 1/3] add RBFReject for invalid cell from conflicted txs --- test/src/main.rs | 1 + test/src/specs/tx_pool/replace.rs | 72 ++++++++++++++++++++++++++++++- tx-pool/src/pool.rs | 12 ++++++ 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/test/src/main.rs b/test/src/main.rs index 030617b27c..a51b8a1a1b 100644 --- a/test/src/main.rs +++ b/test/src/main.rs @@ -471,6 +471,7 @@ fn all_specs() -> Vec> { Box::new(RbfTooManyDescendants), Box::new(RbfContainNewTx), Box::new(RbfContainInvalidInput), + Box::new(RbfContainInvalidCells), Box::new(RbfRejectReplaceProposed), Box::new(CompactBlockEmpty), Box::new(CompactBlockEmptyParentUnknown), diff --git a/test/src/specs/tx_pool/replace.rs b/test/src/specs/tx_pool/replace.rs index 0e37877801..3c0a4e1144 100644 --- a/test/src/specs/tx_pool/replace.rs +++ b/test/src/specs/tx_pool/replace.rs @@ -3,7 +3,7 @@ use ckb_jsonrpc_types::Status; use ckb_logger::info; use ckb_types::{ core::{capacity_bytes, Capacity, TransactionView}, - packed::{Byte32, CellInput, CellOutputBuilder, OutPoint}, + packed::{Byte32, CellDep, CellInput, CellOutputBuilder, OutPoint}, prelude::*, }; @@ -432,6 +432,76 @@ impl Spec for RbfContainInvalidInput { } } +pub struct RbfContainInvalidCells; + +// RBF Rule, contains cell from conflicts txs +impl Spec for RbfContainInvalidCells { + fn run(&self, nodes: &mut Vec) { + let node0 = &nodes[0]; + + node0.mine_until_out_bootstrap_period(); + + // build txs chain + let tx0 = node0.new_transaction_spend_tip_cellbase(); + let mut txs = vec![tx0]; + let max_count = 5; + while txs.len() <= max_count { + let parent = txs.last().unwrap(); + let child = parent + .as_advanced_builder() + .set_inputs(vec![{ + CellInput::new_builder() + .previous_output(OutPoint::new(parent.hash(), 0)) + .build() + }]) + .set_outputs(vec![parent.output(0).unwrap()]) + .build(); + txs.push(child); + } + assert_eq!(txs.len(), max_count + 1); + // send Tx chain + for tx in txs[..=max_count - 1].iter() { + let ret = node0.rpc_client().send_transaction_result(tx.data().into()); + assert!(ret.is_ok()); + } + + let clone_tx = txs[2].clone(); + // Set tx2 fee to a higher value + let output2 = CellOutputBuilder::default() + .capacity(capacity_bytes!(70).pack()) + .build(); + + // build a cell from conflicts txs's output + let cell = CellDep::new_builder() + .out_point(OutPoint::new(txs[2].hash(), 0)) + .build(); + let tx2 = clone_tx + .as_advanced_builder() + .set_inputs(vec![{ + CellInput::new_builder() + .previous_output(OutPoint::new(txs[1].hash(), 0)) + .build() + }]) + .set_cell_deps(vec![cell]) + .set_outputs(vec![output2]) + .build(); + + let res = node0 + .rpc_client() + .send_transaction_result(tx2.data().into()); + assert!(res.is_err(), "tx2 should be rejected"); + assert!(res + .err() + .unwrap() + .to_string() + .contains("new Tx contains cell deps from conflicts")); + } + + fn modify_app_config(&self, config: &mut ckb_app_config::CKBAppConfig) { + config.tx_pool.min_rbf_rate = ckb_types::core::FeeRate(1500); + } +} + pub struct RbfRejectReplaceProposed; // RBF Rule #6 diff --git a/tx-pool/src/pool.rs b/tx-pool/src/pool.rs index c28cbba83c..d64eb51c20 100644 --- a/tx-pool/src/pool.rs +++ b/tx-pool/src/pool.rs @@ -552,8 +552,10 @@ impl TxPool { // Rule #2, new tx don't contain any new unconfirmed inputs let mut inputs = HashSet::new(); + let mut outputs = HashSet::new(); for c in conflicts.iter() { inputs.extend(c.inner.transaction().input_pts_iter()); + outputs.extend(c.inner.transaction().output_pts_iter()); } if rtx @@ -566,6 +568,16 @@ impl TxPool { )); } + if rtx + .transaction + .cell_deps_iter() + .any(|dep| outputs.contains(&dep.out_point())) + { + return Err(Reject::RBFRejected( + "new Tx contains cell deps from conflicts".to_string(), + )); + } + // Rule #5, the replaced tx's descendants can not more than 100 // and the ancestor of the new tx don't have common set with the replaced tx's descendants let mut replace_count: usize = 0; From 3bfcde00051021a84c6bca50834f57682ae12ede Mon Sep 17 00:00:00 2001 From: yukang Date: Tue, 31 Oct 2023 18:56:09 +0800 Subject: [PATCH 2/3] add debug log for rbf --- tx-pool/src/process.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tx-pool/src/process.rs b/tx-pool/src/process.rs index bbf2ba1777..f57cd6a6f7 100644 --- a/tx-pool/src/process.rs +++ b/tx-pool/src/process.rs @@ -129,6 +129,11 @@ impl TxPoolService { for id in conflicts.iter() { let removed = tx_pool.pool_map.remove_entry_and_descendants(id); for old in removed { + debug!( + "remove conflict tx {} for RBF by new tx {}", + old.transaction().hash(), + entry.transaction().hash() + ); let reject = Reject::RBFRejected(format!( "replaced by {}", entry.proposal_short_id() From a9f17712a6d42806150e7447bd06f1f2cb116aa7 Mon Sep 17 00:00:00 2001 From: yukang Date: Tue, 31 Oct 2023 19:17:47 +0800 Subject: [PATCH 3/3] fix log for RBF --- tx-pool/src/process.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tx-pool/src/process.rs b/tx-pool/src/process.rs index f57cd6a6f7..b717e1b90b 100644 --- a/tx-pool/src/process.rs +++ b/tx-pool/src/process.rs @@ -135,8 +135,8 @@ impl TxPoolService { entry.transaction().hash() ); let reject = Reject::RBFRejected(format!( - "replaced by {}", - entry.proposal_short_id() + "replaced by tx {}", + entry.transaction().hash() )); // remove old tx from tx_pool, not happened in service so we didn't call reject callbacks // here we call them manually