From 50f5ccc4f2800b11a2038b0ba6730e354f652ae3 Mon Sep 17 00:00:00 2001 From: Andrew Plaza Date: Mon, 28 Jan 2019 05:26:11 -0500 Subject: [PATCH] Allow specifying local accounts via CLI (#9960) * Allow specifying local accounts via CLI * Add `tx-queue-locals` CLI option * ethcore: modify miner to check options vec before importing transaction * modify tests (ethcore/parity) Resolves #9634 * fix formatting * fixes: Make prefer HashSet over Vec<>, add test, comment formatting * Update ethcore/src/miner/miner.rs Co-Authored-By: insipx * Fix comments and add helper for set->vec conversion * remove blank line from use statement * fix helper test * formatting * fix test to pass on nightly * revert test fix for nightly --- ethcore/src/miner/miner.rs | 44 +++++++++++++++++++++++++++++-- parity/cli/mod.rs | 10 +++++++ parity/cli/tests/config.full.toml | 1 + parity/configuration.rs | 4 ++- parity/helpers.rs | 20 +++++++++++++- 5 files changed, 75 insertions(+), 4 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 998ce6280..36239a4d6 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -140,6 +140,8 @@ pub struct MinerOptions { /// will be invalid if mined. pub infinite_pending_block: bool, + /// Prioritized Local Addresses + pub tx_queue_locals: HashSet
, /// Strategy to use for prioritizing transactions in the queue. pub tx_queue_strategy: PrioritizationStrategy, /// Simple senders penalization. @@ -167,6 +169,7 @@ impl Default for MinerOptions { work_queue_size: 20, enable_resubmission: true, infinite_pending_block: false, + tx_queue_locals: HashSet::new(), tx_queue_strategy: PrioritizationStrategy::GasPriceOnly, tx_queue_penalization: Penalization::Disabled, tx_queue_no_unfamiliar_locals: false, @@ -917,11 +920,13 @@ impl miner::MinerService for Miner { pending: PendingTransaction, trusted: bool ) -> Result<(), transaction::Error> { - // treat the tx as local if the option is enabled, or if we have the account + // treat the tx as local if the option is enabled, if we have the account, or if + // the account is specified as a Prioritized Local Addresses let sender = pending.sender(); let treat_as_local = trusted || !self.options.tx_queue_no_unfamiliar_locals - || self.accounts.as_ref().map(|accts| accts.has_account(sender)).unwrap_or(false); + || self.accounts.as_ref().map(|accts| accts.has_account(sender)).unwrap_or(false) + || self.options.tx_queue_locals.contains(&sender); if treat_as_local { self.import_own_transaction(chain, pending) @@ -1284,6 +1289,8 @@ impl miner::MinerService for Miner { #[cfg(test)] mod tests { + use std::iter::FromIterator; + use super::*; use ethkey::{Generator, Random}; use hash::keccak; @@ -1342,6 +1349,7 @@ mod tests { enable_resubmission: true, infinite_pending_block: false, tx_queue_penalization: Penalization::Disabled, + tx_queue_locals: HashSet::new(), tx_queue_strategy: PrioritizationStrategy::GasPriceOnly, tx_queue_no_unfamiliar_locals: false, refuse_service_transactions: false, @@ -1510,6 +1518,38 @@ mod tests { assert_eq!(miner.prepare_pending_block(&client), BlockPreparationStatus::NotPrepared); } + #[test] + fn should_prioritize_locals() { + let keypair = Random.generate().unwrap(); + let client = TestBlockChainClient::default(); + let account_provider = AccountProvider::transient_provider(); + account_provider.insert_account(keypair.secret().clone(), &"".into()) + .expect("can add accounts to the provider we just created"); + + let transaction = transaction(); + let miner = Miner::new( + MinerOptions { + tx_queue_no_unfamiliar_locals: true, // should work even with this enabled + tx_queue_locals: HashSet::from_iter(vec![transaction.sender()].into_iter()), + ..miner().options + }, + GasPricer::new_fixed(0u64.into()), + &Spec::new_test(), + Some(Arc::new(account_provider)), + ); + let best_block = 0; + + // Miner with sender as a known local address should prioritize transactions from that address + let res2 = miner.import_claimed_local_transaction(&client, PendingTransaction::new(transaction, None), false); + + // check to make sure the prioritized transaction is pending + assert_eq!(res2.unwrap(), ()); + assert_eq!(miner.pending_transactions(best_block).unwrap().len(), 1); + assert_eq!(miner.pending_receipts(best_block).unwrap().len(), 1); + assert_eq!(miner.ready_transactions(&client, 10, PendingOrdering::Priority).len(), 1); + assert_eq!(miner.prepare_pending_block(&client), BlockPreparationStatus::NotPrepared); + } + #[test] fn should_not_seal_unless_enabled() { let miner = miner(); diff --git a/parity/cli/mod.rs b/parity/cli/mod.rs index 9ed6ed957..873b456ad 100644 --- a/parity/cli/mod.rs +++ b/parity/cli/mod.rs @@ -18,6 +18,9 @@ mod usage; mod presets; +use std::collections::HashSet; +use super::helpers; + usage! { { // CLI subcommands @@ -769,6 +772,10 @@ usage! { "--tx-queue-per-sender=[LIMIT]", "Maximum number of transactions per sender in the queue. By default it's 1% of the entire queue, but not less than 16.", + ARG arg_tx_queue_locals: (Option) = None, or |c: &Config| helpers::join_set(c.mining.as_ref()?.tx_queue_locals.as_ref()), + "--tx-queue-locals=[ACCOUNTS]", + "Specify local accounts for which transactions are prioritized in the queue. ACCOUNTS is a comma-delimited list of addresses.", + ARG arg_tx_queue_strategy: (String) = "gas_price", or |c: &Config| c.mining.as_ref()?.tx_queue_strategy.clone(), "--tx-queue-strategy=[S]", "Prioritization strategy used to order transactions in the queue. S may be: gas_price - Prioritize txs with high gas price", @@ -1356,6 +1363,7 @@ struct Mining { tx_queue_size: Option, tx_queue_per_sender: Option, tx_queue_mem_limit: Option, + tx_queue_locals: Option>, tx_queue_strategy: Option, tx_queue_ban_count: Option, tx_queue_ban_time: Option, @@ -1799,6 +1807,7 @@ mod tests { arg_tx_queue_size: 8192usize, arg_tx_queue_per_sender: None, arg_tx_queue_mem_limit: 4u32, + arg_tx_queue_locals: Some("0xdeadbeefcafe0000000000000000000000000000".into()), arg_tx_queue_strategy: "gas_factor".into(), arg_tx_queue_ban_count: Some(1u16), arg_tx_queue_ban_time: Some(180u16), @@ -2072,6 +2081,7 @@ mod tests { tx_queue_size: Some(8192), tx_queue_per_sender: None, tx_queue_mem_limit: None, + tx_queue_locals: None, tx_queue_strategy: None, tx_queue_ban_count: None, tx_queue_ban_time: None, diff --git a/parity/cli/tests/config.full.toml b/parity/cli/tests/config.full.toml index 99603954c..34dd39058 100644 --- a/parity/cli/tests/config.full.toml +++ b/parity/cli/tests/config.full.toml @@ -129,6 +129,7 @@ price_update_period = "hourly" gas_floor_target = "8000000" gas_cap = "10000000" tx_queue_size = 8192 +tx_queue_locals = ["0xdeadbeefcafe0000000000000000000000000000"] tx_queue_strategy = "gas_factor" tx_queue_ban_count = 1 tx_queue_ban_time = 180 #s diff --git a/parity/configuration.rs b/parity/configuration.rs index 48207683e..aaa554d6a 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -18,7 +18,8 @@ use std::time::Duration; use std::io::Read; use std::net::SocketAddr; use std::path::PathBuf; -use std::collections::BTreeMap; +use std::collections::{HashSet, BTreeMap}; +use std::iter::FromIterator; use std::cmp; use cli::{Args, ArgsError}; use hash::keccak; @@ -572,6 +573,7 @@ impl Configuration { infinite_pending_block: self.args.flag_infinite_pending_block, tx_queue_penalization: to_queue_penalization(self.args.arg_tx_time_limit)?, + tx_queue_locals: HashSet::from_iter(to_addresses(&self.args.arg_tx_queue_locals)?.into_iter()), tx_queue_strategy: to_queue_strategy(&self.args.arg_tx_queue_strategy)?, tx_queue_no_unfamiliar_locals: self.args.flag_tx_queue_no_unfamiliar_locals, refuse_service_transactions: self.args.flag_refuse_service_transactions, diff --git a/parity/helpers.rs b/parity/helpers.rs index 1d018f423..bcdace48c 100644 --- a/parity/helpers.rs +++ b/parity/helpers.rs @@ -18,6 +18,7 @@ use std::io; use std::io::{Write, BufReader, BufRead}; use std::time::Duration; use std::fs::File; +use std::collections::HashSet; use ethereum_types::{U256, clean_0x, Address}; use journaldb::Algorithm; use ethcore::client::{Mode, BlockId, VMType, DatabaseCompactionProfile, ClientConfig, VerifierType}; @@ -134,6 +135,13 @@ pub fn to_price(s: &str) -> Result { s.parse::().map_err(|_| format!("Invalid transaciton price 's' given. Must be a decimal number.")) } +pub fn join_set(set: Option<&HashSet>) -> Option { + match set { + Some(s) => Some(s.iter().map(|s| s.as_str()).collect::>().join(",")), + None => None + } +} + /// Flush output buffer. pub fn flush_stdout() { io::stdout().flush().expect("stdout is flushable; qed"); @@ -327,12 +335,13 @@ mod tests { use std::time::Duration; use std::fs::File; use std::io::Write; + use std::collections::HashSet; use tempdir::TempDir; use ethereum_types::U256; use ethcore::client::{Mode, BlockId}; use ethcore::miner::PendingSet; use ethkey::Password; - use super::{to_duration, to_mode, to_block_id, to_u256, to_pending_set, to_address, to_addresses, to_price, geth_ipc_path, to_bootnodes, password_from_file}; + use super::{to_duration, to_mode, to_block_id, to_u256, to_pending_set, to_address, to_addresses, to_price, geth_ipc_path, to_bootnodes, join_set, password_from_file}; #[test] fn test_to_duration() { @@ -472,4 +481,13 @@ but the first password is trimmed assert_eq!(to_bootnodes(&Some(one_bootnode.into())), Ok(vec![one_bootnode.into()])); assert_eq!(to_bootnodes(&Some(two_bootnodes.into())), Ok(vec![one_bootnode.into(), one_bootnode.into()])); } + + #[test] + fn test_join_set() { + let mut test_set = HashSet::new(); + test_set.insert("0x1111111111111111111111111111111111111111".to_string()); + test_set.insert("0x0000000000000000000000000000000000000000".to_string()); + assert_eq!("0x1111111111111111111111111111111111111111,0x0000000000000000000000000000000000000000".to_string(), + join_set(Some(&test_set)).unwrap()); + } }