Skip to content

Commit

Permalink
fix: Only check for ill transactions in RPCs, not relays
Browse files Browse the repository at this point in the history
  • Loading branch information
xxuejie committed Jan 19, 2020
1 parent e5c5cd1 commit 87e74d2
Show file tree
Hide file tree
Showing 17 changed files with 32 additions and 48 deletions.
3 changes: 1 addition & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion ckb-bin/src/subcommand/run.rs
Expand Up @@ -62,7 +62,6 @@ pub fn run(args: RunArgs, version: Version) -> Result<(), ExitCode> {
Arc::clone(&sync_shared_state),
args.config.tx_pool.min_fee_rate,
args.config.tx_pool.max_tx_verify_cycles,
args.config.tx_pool.reject_known_bugs,
);
let net_timer = NetTimeProtocol::default();
let alert_signature_config = args.config.alert_signature.unwrap_or_default();
Expand Down Expand Up @@ -130,6 +129,7 @@ pub fn run(args: RunArgs, version: Version) -> Result<(), ExitCode> {
shared.clone(),
sync_shared_state,
args.config.tx_pool.min_fee_rate,
args.config.rpc.reject_ill_transactions,
)
.enable_miner(
shared.clone(),
Expand Down
2 changes: 1 addition & 1 deletion resource/ckb.toml
Expand Up @@ -99,6 +99,7 @@ modules = ["Net", "Pool", "Miner", "Chain", "Stats", "Subscription", "Experiment
# By default RPC only binds to HTTP service, you can bind it to TCP and WebSocket.
# tcp_listen_address = "127.0.0.1:18114"
# ws_listen_address = "127.0.0.1:28114"
reject_ill_transactions = true

[tx_pool]
max_mem_size = 20_000_000 # 20mb
Expand All @@ -107,7 +108,6 @@ max_verify_cache_size = 100_000
max_conflict_cache_size = 1_000
max_committed_txs_hash_cache_size = 100_000
min_fee_rate = 1_000 # shannons/KB
reject_known_bugs = true
max_tx_verify_cycles = 70_000_000
max_ancestors_count = 25

Expand Down
1 change: 1 addition & 0 deletions rpc/Cargo.toml
Expand Up @@ -39,6 +39,7 @@ futures = "0.1"
ckb-error = { path = "../error" }
ckb-reward-calculator = { path = "../util/reward-calculator" }
ckb-tx-pool = { path = "../tx-pool" }
ckb-script = { path = "../script" }

[dev-dependencies]
ckb-chain-spec = { path = "../spec" }
Expand Down
3 changes: 3 additions & 0 deletions rpc/src/config.rs
Expand Up @@ -24,6 +24,9 @@ pub struct Config {
pub max_request_body_size: usize,
pub threads: Option<usize>,
pub modules: Vec<Module>,
// Rejects txs with scripts that might trigger known bugs
#[serde(default)]
pub reject_ill_transactions: bool,
}

impl Config {
Expand Down
10 changes: 10 additions & 0 deletions rpc/src/module/pool.rs
Expand Up @@ -2,6 +2,7 @@ use crate::error::RPCError;
use ckb_jsonrpc_types::{Transaction, TxPoolInfo};
use ckb_logger::error;
use ckb_network::PeerIndex;
use ckb_script::IllTransactionChecker;
use ckb_shared::shared::Shared;
use ckb_sync::SyncSharedState;
use ckb_tx_pool::{error::SubmitTxError, FeeRate};
Expand All @@ -25,18 +26,21 @@ pub(crate) struct PoolRpcImpl {
sync_shared_state: Arc<SyncSharedState>,
shared: Shared,
min_fee_rate: FeeRate,
reject_ill_transactions: bool,
}

impl PoolRpcImpl {
pub fn new(
shared: Shared,
sync_shared_state: Arc<SyncSharedState>,
min_fee_rate: FeeRate,
reject_ill_transactions: bool,
) -> PoolRpcImpl {
PoolRpcImpl {
sync_shared_state,
shared,
min_fee_rate,
reject_ill_transactions,
}
}
}
Expand All @@ -46,6 +50,12 @@ impl PoolRpc for PoolRpcImpl {
let tx: packed::Transaction = tx.into();
let tx: core::TransactionView = tx.into_view();

if self.reject_ill_transactions {
if let Err(e) = IllTransactionChecker::new(&tx).check() {
return Err(RPCError::custom(RPCError::Invalid, format!("{:#}", e)));
}
}

let tx_pool = self.shared.tx_pool_controller();
let submit_txs = tx_pool.submit_txs(vec![tx.clone()]);

Expand Down
9 changes: 8 additions & 1 deletion rpc/src/service_builder.rs
Expand Up @@ -41,10 +41,17 @@ impl<'a> ServiceBuilder<'a> {
shared: Shared,
sync_shared_state: Arc<SyncSharedState>,
min_fee_rate: FeeRate,
reject_ill_transactions: bool,
) -> Self {
if self.config.pool_enable() {
self.io_handler.extend_with(
PoolRpcImpl::new(shared, sync_shared_state, min_fee_rate).to_delegate(),
PoolRpcImpl::new(
shared,
sync_shared_state,
min_fee_rate,
reject_ill_transactions,
)
.to_delegate(),
);
}
self
Expand Down
2 changes: 1 addition & 1 deletion rpc/src/test.rs
Expand Up @@ -257,7 +257,7 @@ fn setup_node(height: u64) -> (Shared, ChainController, RpcServer) {
.to_delegate(),
);
io.extend_with(
PoolRpcImpl::new(shared.clone(), sync_shared_state, FeeRate::zero()).to_delegate(),
PoolRpcImpl::new(shared.clone(), sync_shared_state, FeeRate::zero(), true).to_delegate(),
);
io.extend_with(
NetworkRpcImpl {
Expand Down
1 change: 0 additions & 1 deletion sync/Cargo.toml
Expand Up @@ -25,7 +25,6 @@ sentry = "0.16.0"
futures = "0.1"
ckb-error = {path = "../error"}
ckb-tx-pool = { path = "../tx-pool" }
ckb-script = { path = "../script" }

[dev-dependencies]
ckb-test-chain-utils = { path = "../util/test-chain-utils" }
Expand Down
3 changes: 0 additions & 3 deletions sync/src/relayer/mod.rs
Expand Up @@ -64,7 +64,6 @@ pub struct Relayer {
pub(crate) shared: Arc<SyncSharedState>,
pub(crate) min_fee_rate: FeeRate,
pub(crate) max_tx_verify_cycles: Cycle,
pub(crate) reject_known_bugs: bool,
}

impl Relayer {
Expand All @@ -73,14 +72,12 @@ impl Relayer {
shared: Arc<SyncSharedState>,
min_fee_rate: FeeRate,
max_tx_verify_cycles: Cycle,
reject_known_bugs: bool,
) -> Self {
Relayer {
chain,
shared,
min_fee_rate,
max_tx_verify_cycles,
reject_known_bugs,
}
}

Expand Down
1 change: 0 additions & 1 deletion sync/src/relayer/tests/helper.rs
Expand Up @@ -159,7 +159,6 @@ pub(crate) fn build_chain(tip: BlockNumber) -> (Relayer, OutPoint) {
sync_shared_state,
FeeRate::zero(),
std::u64::MAX,
true,
),
always_success_out_point,
)
Expand Down
14 changes: 0 additions & 14 deletions sync/src/relayer/transactions_process.rs
Expand Up @@ -2,7 +2,6 @@ use crate::relayer::Relayer;
use ckb_error::{Error, ErrorKind, InternalError, InternalErrorKind};
use ckb_logger::debug_target;
use ckb_network::{CKBProtocolContext, PeerIndex};
use ckb_script::IllTransactionChecker;
use ckb_types::{
core::{Cycle, TransactionView},
packed,
Expand Down Expand Up @@ -79,7 +78,6 @@ impl<'a> TransactionsProcess<'a> {

let mut notify_txs = Vec::with_capacity(txs.len());
let max_tx_verify_cycles = self.relayer.max_tx_verify_cycles;
let reject_known_bugs = self.relayer.reject_known_bugs;
let relay_cycles_vec: Vec<_> = txs
.into_iter()
.filter_map(|(tx, relay_cycles)| {
Expand All @@ -94,18 +92,6 @@ impl<'a> TransactionsProcess<'a> {
);
return None;
}
// skip txs with known bugs
if reject_known_bugs {
if let Err(e) = IllTransactionChecker::new(&tx).check() {
debug_target!(
crate::LOG_TARGET_RELAY,
"ignore tx {} with known bug: {:}",
tx.hash(),
e,
);
return None;
}
}
let tx_hash = tx.hash();
let tx_size = tx.data().serialized_size_in_block();
notify_txs.push(tx);
Expand Down
12 changes: 6 additions & 6 deletions test/src/specs/tx_pool/send_defected_binary.rs
Expand Up @@ -16,16 +16,16 @@ use log::info;
pub struct SendDefectedBinary {
privkey: Privkey,
name: &'static str,
reject_known_bugs: bool,
reject_ill_transactions: bool,
}

impl SendDefectedBinary {
pub fn new(name: &'static str, reject_known_bugs: bool) -> Self {
pub fn new(name: &'static str, reject_ill_transactions: bool) -> Self {
let privkey = Generator::random_privkey();
SendDefectedBinary {
name,
privkey,
reject_known_bugs,
reject_ill_transactions,
}
}
}
Expand Down Expand Up @@ -97,7 +97,7 @@ impl Spec for SendDefectedBinary {

let ret = node.rpc_client().send_transaction_result(tx.data().into());

if self.reject_known_bugs {
if self.reject_ill_transactions {
assert!(ret.is_err());
} else {
let tx_hash = ret.expect("transaction should be accepted").pack();
Expand All @@ -121,12 +121,12 @@ impl Spec for SendDefectedBinary {
.expect("Get pubkey failed")
.serialize();
let lock_arg = Bytes::from(&blake2b_256(&pubkey_data)[0..20]);
let reject_known_bugs = self.reject_known_bugs;
let reject_ill_transactions = self.reject_ill_transactions;
Box::new(move |config| {
let block_assembler =
new_block_assembler_config(lock_arg.clone(), ScriptHashType::Type);
config.block_assembler = Some(block_assembler);
config.tx_pool.reject_known_bugs = reject_known_bugs;
config.rpc.reject_ill_transactions = reject_ill_transactions;
})
}
}
1 change: 0 additions & 1 deletion tx-pool/Cargo.toml
Expand Up @@ -29,4 +29,3 @@ crossbeam-channel = "0.3"
ckb-future-executor = { path = "../util/future-executor" }
ckb-stop-handler = { path = "../util/stop-handler" }
ckb-fee-estimator = { path = "../util/fee-estimator" }
ckb-script = { path = "../script" }
4 changes: 0 additions & 4 deletions tx-pool/src/config.rs
Expand Up @@ -27,9 +27,6 @@ pub struct TxPoolConfig {
pub max_committed_txs_hash_cache_size: usize,
// txs with lower fee rate than this will not be relayed or be mined
pub min_fee_rate: FeeRate,
// Rejects txs with scripts that might trigger known bugs
#[serde(default)]
pub reject_known_bugs: bool,
// tx pool rejects txs that cycles greater than max_tx_verify_cycles
pub max_tx_verify_cycles: Cycle,
// max ancestors size limit for a single tx
Expand All @@ -45,7 +42,6 @@ impl Default for TxPoolConfig {
max_conflict_cache_size: 1_000,
max_committed_txs_hash_cache_size: 100_000,
min_fee_rate: DEFAULT_MIN_FEE_RATE,
reject_known_bugs: true,
max_tx_verify_cycles: DEFAULT_MAX_TX_VERIFY_CYCLES,
max_ancestors_count: DEFAULT_MAX_ANCESTORS_COUNT,
}
Expand Down
10 changes: 0 additions & 10 deletions tx-pool/src/process/submit_txs.rs
Expand Up @@ -3,7 +3,6 @@ use crate::error::SubmitTxError;
use crate::pool::TxPool;
use crate::FeeRate;
use ckb_error::{Error, InternalErrorKind};
use ckb_script::IllTransactionChecker;
use ckb_snapshot::Snapshot;
use ckb_types::{
core::{
Expand Down Expand Up @@ -86,7 +85,6 @@ pub struct VerifyTxsProcess {
pub snapshot: Arc<Snapshot>,
pub txs_verify_cache: HashMap<Byte32, CacheEntry>,
pub txs: Option<Vec<ResolvedTransaction>>,
pub reject_known_bugs: bool,
pub max_tx_verify_cycles: Cycle,
}

Expand All @@ -95,14 +93,12 @@ impl VerifyTxsProcess {
snapshot: Arc<Snapshot>,
txs_verify_cache: HashMap<Byte32, CacheEntry>,
txs: Vec<ResolvedTransaction>,
reject_known_bugs: bool,
max_tx_verify_cycles: Cycle,
) -> VerifyTxsProcess {
VerifyTxsProcess {
snapshot,
txs_verify_cache,
txs: Some(txs),
reject_known_bugs,
max_tx_verify_cycles,
}
}
Expand All @@ -119,7 +115,6 @@ impl Future for VerifyTxsProcess {
&self.snapshot,
txs,
&self.txs_verify_cache,
self.reject_known_bugs,
self.max_tx_verify_cycles,
)?))
}
Expand Down Expand Up @@ -328,7 +323,6 @@ fn verify_rtxs(
snapshot: &Snapshot,
txs: Vec<ResolvedTransaction>,
txs_verify_cache: &HashMap<Byte32, CacheEntry>,
reject_known_bugs: bool,
max_tx_verify_cycles: Cycle,
) -> Result<Vec<(ResolvedTransaction, CacheEntry)>, Error> {
let tip_header = snapshot.tip_header();
Expand All @@ -338,10 +332,6 @@ fn verify_rtxs(

txs.into_iter()
.map(|tx| {
if reject_known_bugs {
IllTransactionChecker::new(&tx.transaction).check()?;
}

let tx_hash = tx.transaction.hash();
if let Some(cache_entry) = txs_verify_cache.get(&tx_hash) {
ContextualTransactionVerifier::new(
Expand Down
2 changes: 0 additions & 2 deletions tx-pool/src/service.rs
Expand Up @@ -594,7 +594,6 @@ impl TxPoolService {
let fetched_cache = FetchCache::new(self.txs_verify_cache.clone(), keys);
let txs_verify_cache = self.txs_verify_cache.clone();
let tx_pool = self.tx_pool.clone();
let reject_known_bugs = self.tx_pool_config.reject_known_bugs;
let max_tx_verify_cycles = self.tx_pool_config.max_tx_verify_cycles;

let pre_resolve = PreResolveTxsProcess::new(tx_pool.clone(), txs);
Expand All @@ -606,7 +605,6 @@ impl TxPoolService {
snapshot,
cache.expect("fetched_cache never fail"),
rtxs,
reject_known_bugs,
max_tx_verify_cycles,
)
})
Expand Down

0 comments on commit 87e74d2

Please sign in to comment.