Refactor flag name + don't change import_own_tx behaviour

fix arg name

Note: force commit to try and get gitlab tests working again 😠

ethcore/src/miner/miner.rs

@@ -125,8 +125,8 @@ pub struct MinerOptions {
 	pub tx_queue_strategy: PrioritizationStrategy,
 	/// Simple senders penalization.
 	pub tx_queue_penalization: Penalization,
-	/// Do we want to mark transactions recieved as local if we don't have the sending account?
-	pub tx_queue_allow_unknown_local: bool,
+	/// Do we want to mark transactions recieved locally (e.g. RPC) as local if we don't have the sending account?
+	pub tx_queue_no_unfamiliar_locals: bool,
 	/// Do we refuse to accept service transactions even if sender is certified.
 	pub refuse_service_transactions: bool,
 	/// Transaction pool limits.
@@ -150,7 +150,7 @@ impl Default for MinerOptions {
 			infinite_pending_block: false,
 			tx_queue_strategy: PrioritizationStrategy::GasPriceOnly,
 			tx_queue_penalization: Penalization::Disabled,
-			tx_queue_allow_unknown_local: true,
+			tx_queue_no_unfamiliar_locals: false,
 			refuse_service_transactions: false,
 			pool_limits: pool::Options {
 				max_count: 8_192,
@@ -799,36 +799,44 @@ impl miner::MinerService for Miner {
 		chain: &C,
 		pending: PendingTransaction,
 	) -> Result<(), transaction::Error> {
+		// note: you may want to use `import_claimed_local_transaction` instead of this one.
 
 		trace!(target: "own_tx", "Importing transaction: {:?}", pending);
 
+		let client = self.pool_client(chain);
+		let imported = self.transaction_queue.import(
+			client,
+			vec![pool::verifier::Transaction::Local(pending)]
+		).pop().expect("one result returned per added transaction; one added => one result; qed");
+
+		// --------------------------------------------------------------------------
+		// | NOTE Code below requires sealing locks.                                |
+		// | Make sure to release the locks before calling that method.             |
+		// --------------------------------------------------------------------------
+		if imported.is_ok() && self.options.reseal_on_own_tx && self.sealing.lock().reseal_allowed() {
+			self.prepare_and_update_sealing(chain);
+		}
+
+		imported
+	}
+
+	fn import_claimed_local_transaction<C: miner::BlockChainClient>(
+		&self,
+		chain: &C,
+		pending: PendingTransaction,
+	) -> Result<(), transaction::Error> {
 		// treat the tx as local if the option is enabled, or if we have the account
 		let sender = pending.sender();
-		let treat_as_local = self.options.tx_queue_allow_unknown_local
+		let treat_as_local = !self.options.tx_queue_no_unfamiliar_locals
 			|| self.accounts.as_ref().map(|accts| accts.has_account(sender)).unwrap_or(false);
 
 		if treat_as_local {
-			let client = self.pool_client(chain);
-			let imported = self.transaction_queue.import(
-				client,
-				vec![pool::verifier::Transaction::Local(pending)]
-			).pop().expect("one result returned per added transaction; one added => one result; qed");
-
-            // --------------------------------------------------------------------------
-            // | NOTE Code below requires sealing locks.                                |
-            // | Make sure to release the locks before calling that method.             |
-            // --------------------------------------------------------------------------
-            if imported.is_ok() && self.options.reseal_on_own_tx && self.sealing.lock().reseal_allowed() {
-                self.prepare_and_update_sealing(chain);
-            }
-
-
-			imported
+			self.import_own_transaction(chain, pending)
 		} else {
 			// We want to replicate behaviour for external transactions if we're not going to treat
 			// this as local. This is important with regards to sealing blocks
-			self.import_external_transactions(chain,	vec![pending.transaction.into()])
-				.pop().expect("one result per tx, as below")
+			self.import_external_transactions(chain, vec![pending.transaction.into()])
+				.pop().expect("one result per tx, as in `import_own_transaction`")
 		}
 	}
 
@@ -1162,34 +1170,30 @@ mod tests {
 		assert!(miner.submit_seal(hash, vec![]).is_ok());
 	}
 
-	fn  default_miner_opts() -> MinerOptions {
-		MinerOptions {
-			force_sealing: false,
-			reseal_on_external_tx: false,
-			reseal_on_own_tx: true,
-			reseal_on_uncle: false,
-			reseal_min_period: Duration::from_secs(5),
-			reseal_max_period: Duration::from_secs(120),
-			pending_set: PendingSet::AlwaysSealing,
-			work_queue_size: 5,
-			enable_resubmission: true,
-			infinite_pending_block: false,
-			tx_queue_penalization: Penalization::Disabled,
-			tx_queue_strategy: PrioritizationStrategy::GasPriceOnly,
-			tx_queue_allow_unknown_local: true,
-			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(),
-			},
-		}
-	}
-
 	fn miner() -> Miner {
 		Miner::new(
-			default_miner_opts(),
+			MinerOptions {
+				force_sealing: false,
+				reseal_on_external_tx: false,
+				reseal_on_own_tx: true,
+				reseal_on_uncle: false,
+				reseal_min_period: Duration::from_secs(5),
+				reseal_max_period: Duration::from_secs(120),
+				pending_set: PendingSet::AlwaysSealing,
+				work_queue_size: 5,
+				enable_resubmission: true,
+				infinite_pending_block: false,
+				tx_queue_penalization: Penalization::Disabled,
+				tx_queue_strategy: PrioritizationStrategy::GasPriceOnly,
+				tx_queue_no_unfamiliar_locals: 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(),
+				},
+			},
 			GasPricer::new_fixed(0u64.into()),
 			&Spec::new_test(),
 			None, // accounts provider
@@ -1274,7 +1278,7 @@ mod tests {
 	}
 
 	#[test]
-	fn should_import_own_transaction_selectively() {
+	fn should_treat_unfamiliar_locals_selectively() {
 		// given
 		let keypair = Random.generate().unwrap();
 		let client = TestBlockChainClient::default();
@@ -1283,8 +1287,8 @@ mod tests {
 
 		let miner = Miner::new(
 			MinerOptions {
-				tx_queue_allow_unknown_local: false,
-				..default_miner_opts()
+				tx_queue_no_unfamiliar_locals: true,
+				..miner().options
 			},
 			GasPricer::new_fixed(0u64.into()),
 			&Spec::new_test(),
@@ -1294,7 +1298,7 @@ mod tests {
 		let best_block = 0;
 		// when
 		// This transaction should not be marked as local because our account_provider doesn't have the sender
-		let res = miner.import_own_transaction(&client, PendingTransaction::new(transaction.clone(), None));
+		let res = miner.import_claimed_local_transaction(&client, PendingTransaction::new(transaction.clone(), None));
 
 		// then
 		// Check the same conditions as `should_import_external_transaction` first. Behaviour should be identical.
@@ -1309,7 +1313,7 @@ mod tests {
 		// when - 2nd part: create a local transaction from account_provider.
 		// Borrow the transaction used before & sign with our generated keypair.
 		let local_transaction = transaction.deconstruct().0.as_unsigned().clone().sign(keypair.secret(), Some(TEST_CHAIN_ID));
-		let res2 = miner.import_own_transaction(&client, PendingTransaction::new(local_transaction, None));
+		let res2 = miner.import_claimed_local_transaction(&client, PendingTransaction::new(local_transaction, None));
 
 		// then - 2nd part: we add on the results from the last pending block.
 		// This is borrowed from `should_make_pending_block_when_importing_own_transaction` and slightly modified.

ethcore/src/miner/mod.rs

@@ -138,6 +138,12 @@ pub trait MinerService : Send + Sync {
 		-> Result<(), transaction::Error>
 		where C: BlockChainClient;
 
+	/// Imports transactions from potentially external sources, with behaviour determined
+	/// by the config flag `tx_queue_allow_unfamiliar_locals`
+	fn import_claimed_local_transaction<C>(&self, chain: &C, transaction: PendingTransaction)
+		-> Result<(), transaction::Error>
+		where C: BlockChainClient;
+
 	/// Removes transaction from the pool.
 	///
 	/// Attempts to "cancel" a transaction. If it was not propagated yet (or not accepted by other peers)

parity/cli/mod.rs

@@ -636,6 +636,10 @@ usage! {
 			"--remove-solved",
 			"Move solved blocks from the work package queue instead of cloning them. This gives a slightly faster import speed, but means that extra solutions submitted for the same work package will go unused.",
 
+			FLAG flag_tx_queue_no_unfamiliar_locals: (bool) = false, or |c: &Config| c.mining.as_ref()?.tx_queue_no_unfamiliar_locals.clone(),
+			"--tx-queue-no-unfamiliar-locals",
+			"Transactions recieved via local means (RPC, WS, etc) will be treated as external if the sending account is unknown.",
+
 			FLAG flag_refuse_service_transactions: (bool) = false, or |c: &Config| c.mining.as_ref()?.refuse_service_transactions.clone(),
 			"--refuse-service-transactions",
 			"Always refuse service transactions.",
@@ -712,10 +716,6 @@ usage! {
 			"--tx-queue-strategy=[S]",
 			"Prioritization strategy used to order transactions in the queue. S may be: gas_price - Prioritize txs with high gas price",
 
-			ARG arg_tx_queue_allow_unknown_local: (bool) = true, or |c: &Config| c.mining.as_ref()?.tx_queue_allow_unknown_local.clone(),
-			"--tx-queue-allow-unknown-local=[ALLOW]",
-			"Transactions recieved from outside the network will be treated as local even if the sending account is not local.",
-
 			ARG arg_stratum_interface: (String) = "local", or |c: &Config| c.stratum.as_ref()?.interface.clone(),
 			"--stratum-interface=[IP]",
 			"Interface address for Stratum server.",
@@ -1253,7 +1253,7 @@ struct Mining {
 	tx_queue_strategy: Option<String>,
 	tx_queue_ban_count: Option<u16>,
 	tx_queue_ban_time: Option<u16>,
-	tx_queue_allow_unknown_local: Option<bool>,
+	tx_queue_no_unfamiliar_locals: Option<bool>,
 	remove_solved: Option<bool>,
 	notify_work: Option<Vec<String>>,
 	refuse_service_transactions: Option<bool>,
@@ -1668,14 +1668,14 @@ mod tests {
 			arg_gas_floor_target: "4700000".into(),
 			arg_gas_cap: "6283184".into(),
 			arg_extra_data: Some("Parity".into()),
+			flag_tx_queue_no_unfamiliar_locals: false,
 			arg_tx_queue_size: 8192usize,
 			arg_tx_queue_per_sender: None,
 			arg_tx_queue_mem_limit: 4u32,
 			arg_tx_queue_gas: "off".into(),
 			arg_tx_queue_strategy: "gas_factor".into(),
 			arg_tx_queue_ban_count: 1u16,
 			arg_tx_queue_ban_time: 180u16,
-			arg_tx_queue_allow_unknown_local: true,
 			flag_remove_solved: false,
 			arg_notify_work: Some("http://localhost:3001".into()),
 			flag_refuse_service_transactions: false,
@@ -1935,7 +1935,7 @@ mod tests {
 				tx_queue_strategy: None,
 				tx_queue_ban_count: None,
 				tx_queue_ban_time: None,
-				tx_queue_allow_unknown_local: None,
+				tx_queue_no_unfamiliar_locals: None,
 				tx_gas_limit: None,
 				tx_time_limit: None,
 				extra_data: None,

parity/cli/tests/config.full.toml

@@ -133,7 +133,7 @@ tx_queue_ban_count = 1
 tx_queue_ban_time = 180 #s
 tx_gas_limit = "6283184"
 tx_time_limit = 100 #ms
-tx_queue_allow_unknown_local = true
+tx_queue_no_unfamiliar_locals = false
 extra_data = "Parity"
 remove_solved = false
 notify_work = ["http://localhost:3001"]

parity/configuration.rs

@@ -549,7 +549,7 @@ impl Configuration {
 
 			tx_queue_penalization: to_queue_penalization(self.args.arg_tx_time_limit)?,
 			tx_queue_strategy: to_queue_strategy(&self.args.arg_tx_queue_strategy)?,
-			tx_queue_allow_unknown_local: self.args.arg_tx_queue_allow_unknown_local,
+			tx_queue_no_unfamiliar_locals: self.args.flag_tx_queue_no_unfamiliar_locals,
 			refuse_service_transactions: self.args.flag_refuse_service_transactions,
 
 			pool_limits: self.pool_limits()?,

rpc/src/v1/helpers/dispatch.rs

@@ -126,7 +126,10 @@ impl<C: miner::BlockChainClient, M: MinerService> FullDispatcher<C, M> {
 	pub fn dispatch_transaction(client: &C, miner: &M, signed_transaction: PendingTransaction) -> Result<H256> {
 		let hash = signed_transaction.transaction.hash();
 
-		miner.import_own_transaction(client, signed_transaction)
+		// use `import_claimed_local_transaction` so we can decide (based on config flags) if we want to treat
+		// it as local or not. Nodes with public RPC interfaces will want these transactions to be treated like
+		// external transactions.
+		miner.import_claimed_local_transaction(client, signed_transaction)
 			.map_err(errors::transaction)
 			.map(|_| hash)
 	}