From 62dd210749b5e9b389bae1e3e0cf17878d43a8f6 Mon Sep 17 00:00:00 2001 From: Peter Pratscher Date: Tue, 17 Jul 2018 11:07:40 +0200 Subject: [PATCH 1/3] Added --tx-queue-no-early-reject flag to disable early tx queue rejects because of low gas price --- ethcore/private-tx/src/private_transactions.rs | 1 + ethcore/src/miner/miner.rs | 4 ++++ miner/src/pool/queue.rs | 12 ++++++++---- miner/src/pool/verifier.rs | 5 ++++- parity/cli/mod.rs | 6 ++++++ parity/configuration.rs | 1 + 6 files changed, 24 insertions(+), 5 deletions(-) diff --git a/ethcore/private-tx/src/private_transactions.rs b/ethcore/private-tx/src/private_transactions.rs index fcc6da514ed..e28579c1d8e 100644 --- a/ethcore/private-tx/src/private_transactions.rs +++ b/ethcore/private-tx/src/private_transactions.rs @@ -67,6 +67,7 @@ impl Default for VerificationStore { minimal_gas_price: 0.into(), block_gas_limit: 8_000_000.into(), tx_gas_limit: U256::max_value(), + tx_queue_no_early_reject: false }, pool::PrioritizationStrategy::GasPriceOnly, ) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 16a20f8a007..92be8eb819c 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -165,6 +165,7 @@ impl Default for MinerOptions { minimal_gas_price: DEFAULT_MINIMAL_GAS_PRICE.into(), block_gas_limit: U256::max_value(), tx_gas_limit: U256::max_value(), + tx_queue_no_early_reject: false, }, } } @@ -272,6 +273,7 @@ impl Miner { minimal_gas_price, block_gas_limit: U256::max_value(), tx_gas_limit: U256::max_value(), + tx_queue_no_early_reject: false, }, reseal_min_period: Duration::from_secs(0), ..Default::default() @@ -1297,12 +1299,14 @@ mod tests { tx_queue_penalization: Penalization::Disabled, tx_queue_strategy: PrioritizationStrategy::GasPriceOnly, tx_queue_no_unfamiliar_locals: false, + tx_queue_no_early_reject: false, refuse_service_transactions: false, pool_limits: Default::default(), pool_verification_options: pool::verifier::Options { minimal_gas_price: 0.into(), block_gas_limit: U256::max_value(), tx_gas_limit: U256::max_value(), + tx_queue_no_early_reject: false, }, }, GasPricer::new_fixed(0u64.into()), diff --git a/miner/src/pool/queue.rs b/miner/src/pool/queue.rs index 24a56c226a1..d2a3c0d9186 100644 --- a/miner/src/pool/queue.rs +++ b/miner/src/pool/queue.rs @@ -243,11 +243,15 @@ impl TransactionQueue { let options = self.options.read().clone(); let transaction_to_replace = { - let pool = self.pool.read(); - if pool.is_full() { - pool.worst_transaction().map(|worst| (pool.scoring().clone(), worst)) - } else { + if options.tx_queue_no_early_reject { None + } else { + let pool = self.pool.read(); + if pool.is_full() { + pool.worst_transaction().map(|worst| (pool.scoring().clone(), worst)) + } else { + None + } } }; diff --git a/miner/src/pool/verifier.rs b/miner/src/pool/verifier.rs index 4703088ffb6..f390f4dd646 100644 --- a/miner/src/pool/verifier.rs +++ b/miner/src/pool/verifier.rs @@ -43,6 +43,8 @@ pub struct Options { pub block_gas_limit: U256, /// Maximal gas limit for a single transaction. pub tx_gas_limit: U256, + /// Skip check checks + pub tx_queue_no_early_reject: bool, } #[cfg(test)] @@ -52,6 +54,7 @@ impl Default for Options { minimal_gas_price: 0.into(), block_gas_limit: U256::max_value(), tx_gas_limit: U256::max_value(), + tx_queue_no_early_reject: false, } } } @@ -204,7 +207,7 @@ impl txpool::Verifier for Verifier, tx_queue_ban_time: Option, tx_queue_no_unfamiliar_locals: Option, + tx_queue_no_early_reject: Option, remove_solved: Option, notify_work: Option>, refuse_service_transactions: Option, @@ -1996,6 +2001,7 @@ mod tests { tx_queue_ban_count: None, tx_queue_ban_time: None, tx_queue_no_unfamiliar_locals: None, + tx_queue_no_early_reject: None, tx_gas_limit: None, tx_time_limit: None, extra_data: None, diff --git a/parity/configuration.rs b/parity/configuration.rs index f904c169c0f..8322d3c66dc 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -571,6 +571,7 @@ impl Configuration { Some(ref d) => to_u256(d)?, None => U256::max_value(), }, + tx_queue_no_early_reject: self.args.flag_tx_queue_no_early_reject, }) } From 9f877cef79efaacbaf1402cfb6570d18cb9c8eb1 Mon Sep 17 00:00:00 2001 From: Peter Pratscher Date: Tue, 17 Jul 2018 13:05:42 +0200 Subject: [PATCH 2/3] Fixed failing tests, clarified comments and simplified no_early_reject field name. --- ethcore/private-tx/src/private_transactions.rs | 2 +- ethcore/src/miner/miner.rs | 7 +++---- miner/src/pool/queue.rs | 2 +- miner/src/pool/tests/mod.rs | 8 ++++++++ miner/src/pool/verifier.rs | 6 +++--- parity/cli/mod.rs | 1 + parity/cli/tests/config.full.toml | 1 + parity/configuration.rs | 2 +- rpc/src/v1/tests/helpers/miner_service.rs | 1 + 9 files changed, 20 insertions(+), 10 deletions(-) diff --git a/ethcore/private-tx/src/private_transactions.rs b/ethcore/private-tx/src/private_transactions.rs index e28579c1d8e..e16d6ab911b 100644 --- a/ethcore/private-tx/src/private_transactions.rs +++ b/ethcore/private-tx/src/private_transactions.rs @@ -67,7 +67,7 @@ impl Default for VerificationStore { minimal_gas_price: 0.into(), block_gas_limit: 8_000_000.into(), tx_gas_limit: U256::max_value(), - tx_queue_no_early_reject: false + no_early_reject: false }, pool::PrioritizationStrategy::GasPriceOnly, ) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 92be8eb819c..6ef6876c043 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -165,7 +165,7 @@ impl Default for MinerOptions { minimal_gas_price: DEFAULT_MINIMAL_GAS_PRICE.into(), block_gas_limit: U256::max_value(), tx_gas_limit: U256::max_value(), - tx_queue_no_early_reject: false, + no_early_reject: false, }, } } @@ -273,7 +273,7 @@ impl Miner { minimal_gas_price, block_gas_limit: U256::max_value(), tx_gas_limit: U256::max_value(), - tx_queue_no_early_reject: false, + no_early_reject: false, }, reseal_min_period: Duration::from_secs(0), ..Default::default() @@ -1299,14 +1299,13 @@ mod tests { tx_queue_penalization: Penalization::Disabled, tx_queue_strategy: PrioritizationStrategy::GasPriceOnly, tx_queue_no_unfamiliar_locals: false, - tx_queue_no_early_reject: false, refuse_service_transactions: false, pool_limits: Default::default(), pool_verification_options: pool::verifier::Options { minimal_gas_price: 0.into(), block_gas_limit: U256::max_value(), tx_gas_limit: U256::max_value(), - tx_queue_no_early_reject: false, + no_early_reject: false, }, }, GasPricer::new_fixed(0u64.into()), diff --git a/miner/src/pool/queue.rs b/miner/src/pool/queue.rs index d2a3c0d9186..d1105210844 100644 --- a/miner/src/pool/queue.rs +++ b/miner/src/pool/queue.rs @@ -243,7 +243,7 @@ impl TransactionQueue { let options = self.options.read().clone(); let transaction_to_replace = { - if options.tx_queue_no_early_reject { + if options.no_early_reject { None } else { let pool = self.pool.read(); diff --git a/miner/src/pool/tests/mod.rs b/miner/src/pool/tests/mod.rs index b5cbaab6319..d6e98b48159 100644 --- a/miner/src/pool/tests/mod.rs +++ b/miner/src/pool/tests/mod.rs @@ -37,6 +37,7 @@ fn new_queue() -> TransactionQueue { minimal_gas_price: 1.into(), block_gas_limit: 1_000_000.into(), tx_gas_limit: 1_000_000.into(), + no_early_reject: false, }, PrioritizationStrategy::GasPriceOnly, ) @@ -54,6 +55,7 @@ fn should_return_correct_nonces_when_dropped_because_of_limit() { minimal_gas_price: 1.into(), block_gas_limit: 1_000_000.into(), tx_gas_limit: 1_000_000.into(), + no_early_reject: false, }, PrioritizationStrategy::GasPriceOnly, ); @@ -105,6 +107,7 @@ fn should_never_drop_local_transactions_from_different_senders() { minimal_gas_price: 1.into(), block_gas_limit: 1_000_000.into(), tx_gas_limit: 1_000_000.into(), + no_early_reject: false, }, PrioritizationStrategy::GasPriceOnly, ); @@ -479,6 +482,7 @@ fn should_prefer_current_transactions_when_hitting_the_limit() { minimal_gas_price: 1.into(), block_gas_limit: 1_000_000.into(), tx_gas_limit: 1_000_000.into(), + no_early_reject: false, }, PrioritizationStrategy::GasPriceOnly, ); @@ -891,6 +895,7 @@ fn should_include_local_transaction_to_a_full_pool() { minimal_gas_price: 1.into(), block_gas_limit: 1_000_000.into(), tx_gas_limit: 1_000_000.into(), + no_early_reject: false, }, PrioritizationStrategy::GasPriceOnly, ); @@ -922,6 +927,7 @@ fn should_avoid_verifying_transaction_already_in_pool() { minimal_gas_price: 1.into(), block_gas_limit: 1_000_000.into(), tx_gas_limit: 1_000_000.into(), + no_early_reject: false, }, PrioritizationStrategy::GasPriceOnly, ); @@ -956,6 +962,7 @@ fn should_avoid_reverifying_recently_rejected_transactions() { minimal_gas_price: 1.into(), block_gas_limit: 1_000_000.into(), tx_gas_limit: 1_000_000.into(), + no_early_reject: false, }, PrioritizationStrategy::GasPriceOnly, ); @@ -997,6 +1004,7 @@ fn should_reject_early_in_case_gas_price_is_less_than_min_effective() { minimal_gas_price: 1.into(), block_gas_limit: 1_000_000.into(), tx_gas_limit: 1_000_000.into(), + no_early_reject: false, }, PrioritizationStrategy::GasPriceOnly, ); diff --git a/miner/src/pool/verifier.rs b/miner/src/pool/verifier.rs index f390f4dd646..eaa13b3da1f 100644 --- a/miner/src/pool/verifier.rs +++ b/miner/src/pool/verifier.rs @@ -43,8 +43,8 @@ pub struct Options { pub block_gas_limit: U256, /// Maximal gas limit for a single transaction. pub tx_gas_limit: U256, - /// Skip check checks - pub tx_queue_no_early_reject: bool, + /// Skip checks for early rejection, to make sure that local transactions are always imported. + pub no_early_reject: bool, } #[cfg(test)] @@ -54,7 +54,7 @@ impl Default for Options { minimal_gas_price: 0.into(), block_gas_limit: U256::max_value(), tx_gas_limit: U256::max_value(), - tx_queue_no_early_reject: false, + no_early_reject: false, } } } diff --git a/parity/cli/mod.rs b/parity/cli/mod.rs index 3d56df722cd..a7f74881350 100644 --- a/parity/cli/mod.rs +++ b/parity/cli/mod.rs @@ -1731,6 +1731,7 @@ mod tests { arg_gas_cap: "6283184".into(), arg_extra_data: Some("Parity".into()), flag_tx_queue_no_unfamiliar_locals: false, + flag_tx_queue_no_early_reject: false, arg_tx_queue_size: 8192usize, arg_tx_queue_per_sender: None, arg_tx_queue_mem_limit: 4u32, diff --git a/parity/cli/tests/config.full.toml b/parity/cli/tests/config.full.toml index 0c1efb18404..3bd2eae3a5c 100644 --- a/parity/cli/tests/config.full.toml +++ b/parity/cli/tests/config.full.toml @@ -135,6 +135,7 @@ tx_queue_ban_time = 180 #s tx_gas_limit = "6283184" tx_time_limit = 100 #ms tx_queue_no_unfamiliar_locals = false +tx_queue_no_early_reject = false extra_data = "Parity" remove_solved = false notify_work = ["http://localhost:3001"] diff --git a/parity/configuration.rs b/parity/configuration.rs index 8322d3c66dc..a6217acea5a 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -571,7 +571,7 @@ impl Configuration { Some(ref d) => to_u256(d)?, None => U256::max_value(), }, - tx_queue_no_early_reject: self.args.flag_tx_queue_no_early_reject, + no_early_reject: self.args.flag_tx_queue_no_early_reject, }) } diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index fa9f22b2479..bed4a59d5a2 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -264,6 +264,7 @@ impl MinerService for TestMinerService { minimal_gas_price: 0x1312d00.into(), block_gas_limit: 5_000_000.into(), tx_gas_limit: 5_000_000.into(), + no_early_reject: false, }, status: txpool::LightStatus { mem_usage: 1_000, From 6f8e8ff939a6c1afca3030b12a0a518098273c60 Mon Sep 17 00:00:00 2001 From: Peter Pratscher Date: Wed, 18 Jul 2018 09:26:48 +0200 Subject: [PATCH 3/3] Added test case for the --tx-queue-no-early-reject flag --- miner/src/pool/tests/mod.rs | 39 +++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/miner/src/pool/tests/mod.rs b/miner/src/pool/tests/mod.rs index d6e98b48159..bb029f6147a 100644 --- a/miner/src/pool/tests/mod.rs +++ b/miner/src/pool/tests/mod.rs @@ -1029,3 +1029,42 @@ fn should_reject_early_in_case_gas_price_is_less_than_min_effective() { // then assert_eq!(txq.status().status.transaction_count, 1); } + + +#[test] +fn should_not_reject_early_in_case_gas_price_is_less_than_min_effective() { + // given + let txq = TransactionQueue::new( + txpool::Options { + max_count: 1, + max_per_sender: 2, + max_mem_usage: 50 + }, + verifier::Options { + minimal_gas_price: 1.into(), + block_gas_limit: 1_000_000.into(), + tx_gas_limit: 1_000_000.into(), + no_early_reject: true, + }, + PrioritizationStrategy::GasPriceOnly, + ); + // when + let tx1 = Tx::gas_price(2).signed(); + let client = TestClient::new().with_local(&tx1.sender()); + let res = txq.import(client.clone(), vec![tx1.unverified()]); + + // then + assert_eq!(res, vec![Ok(())]); + assert_eq!(txq.status().status.transaction_count, 1); + assert!(client.was_verification_triggered()); + + // when + let tx1 = Tx::gas_price(1).signed(); + let client = TestClient::new().with_local(&tx1.sender()); + let res = txq.import(client.clone(), vec![tx1.unverified()]); + + // then + assert_eq!(res, vec![Ok(())]); + assert_eq!(txq.status().status.transaction_count, 2); + assert!(client.was_verification_triggered()); +}