Only require author id on CLI when node is authoring (paritytech#188)

Co-authored-by: 4meta5 <[email protected]>

node/parachain/src/cli.rs

@@ -98,7 +98,7 @@ pub struct RunCmd {
 
 	/// Public identity for participating in staking and receiving rewards
 	#[structopt(long, parse(try_from_str = parse_h160))]
-	pub account_id: Option<H160>,
+	pub author_id: Option<H160>,
 }
 
 fn parse_h160(input: &str) -> Result<H160, String> {

node/parachain/src/command.rs

@@ -33,6 +33,7 @@ use sc_service::{
 	PartialComponents,
 };
 use sp_core::hexdisplay::HexDisplay;
+use sp_core::H160;
 use sp_runtime::traits::Block as _;
 use std::{io::Write, net::SocketAddr};
 
@@ -266,9 +267,13 @@ pub fn run() -> Result<()> {
 		}
 		None => {
 			let runner = cli.create_runner(&*cli.run)?;
-			let account = cli.run.account_id.ok_or(sc_cli::Error::Input(
-				"Account ID required but not set".to_string(),
-			))?;
+			let collator = cli.run.base.validator || cli.collator;
+			let author_id: Option<H160> = cli.run.author_id;
+			if collator {
+				if author_id.is_none() {
+					return Err("Collator nodes must specify an author account id".into());
+				}
+			}
 			runner.run_node_until_exit(|config| async move {
 				let key = sp_core::Pair::generate().0;
 
@@ -297,14 +302,13 @@ pub fn run() -> Result<()> {
 				let polkadot_config =
 					SubstrateCli::create_configuration(&polkadot_cli, &polkadot_cli, task_executor)
 						.map_err(|err| format!("Relay chain argument error: {}", err))?;
-				let collator = cli.run.base.validator || cli.collator;
 
 				info!("Parachain id: {:?}", id);
 				info!("Parachain Account: {}", parachain_account);
 				info!("Parachain genesis state: {}", genesis_state);
 				info!("Is collating: {}", if collator { "yes" } else { "no" });
 
-				crate::service::start_node(config, key, account, polkadot_config, id, collator)
+				crate::service::start_node(config, key, author_id, polkadot_config, id, collator)
 					.await
 					.map(|r| r.0)
 			})

node/parachain/src/service.rs

@@ -135,7 +135,7 @@ pub fn new_partial(
 async fn start_node_impl<RB>(
 	parachain_config: Configuration,
 	collator_key: CollatorPair,
-	account_id: H160,
+	author_id: Option<H160>,
 	polkadot_config: Configuration,
 	id: polkadot_primitives::v0::Id,
 	validator: bool,
@@ -162,7 +162,7 @@ where
 			},
 		)?;
 
-	let params = new_partial(&parachain_config, Some(account_id))?;
+	let params = new_partial(&parachain_config, author_id)?;
 
 	let client = params.client.clone();
 	let backend = params.backend.clone();
@@ -331,15 +331,15 @@ where
 pub async fn start_node(
 	parachain_config: Configuration,
 	collator_key: CollatorPair,
-	account_id: H160,
+	author_id: Option<H160>,
 	polkadot_config: Configuration,
 	id: polkadot_primitives::v0::Id,
 	validator: bool,
 ) -> sc_service::error::Result<(TaskManager, Arc<FullClient>)> {
 	start_node_impl(
 		parachain_config,
 		collator_key,
-		account_id,
+		author_id,
 		polkadot_config,
 		id,
 		validator,

node/standalone/src/cli.rs

@@ -30,7 +30,7 @@ pub struct RunCmd {
 
 	/// Public identity for participating in staking and receiving rewards
 	#[structopt(long, parse(try_from_str = parse_h160))]
-	pub account_id: Option<H160>,
+	pub author_id: Option<H160>,
 }
 
 fn parse_h160(input: &str) -> Result<H160, String> {

node/standalone/src/command.rs

@@ -133,13 +133,10 @@ pub fn run() -> sc_cli::Result<()> {
 		}
 		None => {
 			let runner = cli.create_runner(&cli.run.base)?;
-			let account = cli.run.account_id.ok_or(sc_cli::Error::Input(
-				"Account ID required but not set".to_string(),
-			))?;
 			runner.run_node_until_exit(|config| async move {
 				match config.role {
 					Role::Light => service::new_light(config),
-					_ => service::new_full(config, cli.run.manual_seal, account),
+					_ => service::new_full(config, cli.run.manual_seal, cli.run.author_id),
 				}
 			})
 		}

node/standalone/src/service.rs

@@ -16,13 +16,12 @@
 
 //! Service and ServiceFactory implementation. Specialized wrapper over substrate service.
 
-use std::{sync::{Arc, Mutex}, time::Duration, collections::HashMap};
-use fc_rpc_core::types::PendingTransactions;
 use crate::mock_timestamp::MockTimestampInherentDataProvider;
 use fc_consensus::FrontierBlockImport;
+use fc_rpc_core::types::PendingTransactions;
 use moonbeam_runtime::{self, opaque::Block, RuntimeApi};
-use sc_client_api::{ExecutorProvider, RemoteBackend, BlockchainEvents};
 use parity_scale_codec::Encode;
+use sc_client_api::{BlockchainEvents, ExecutorProvider, RemoteBackend};
 use sc_consensus_manual_seal::{self as manual_seal};
 use sc_executor::native_executor_instance;
 pub use sc_executor::NativeExecutor;
@@ -31,6 +30,11 @@ use sc_service::{error::Error as ServiceError, Configuration, TaskManager};
 use sp_consensus_aura::sr25519::AuthorityPair as AuraPair;
 use sp_core::H160;
 use sp_inherents::InherentDataProviders;
+use std::{
+	collections::HashMap,
+	sync::{Arc, Mutex},
+	time::Duration,
+};
 
 // Our native executor instance.
 native_executor_instance!(
@@ -117,8 +121,7 @@ pub fn new_partial(
 		client.clone(),
 	);
 
-	let pending_transactions: PendingTransactions
-		= Some(Arc::new(Mutex::new(HashMap::new())));
+	let pending_transactions: PendingTransactions = Some(Arc::new(Mutex::new(HashMap::new())));
 
 	if manual_seal {
 		let frontier_block_import = FrontierBlockImport::new(client.clone(), client.clone(), true);
@@ -138,7 +141,10 @@ pub fn new_partial(
 			select_chain,
 			transaction_pool,
 			inherent_data_providers,
-			other: (ConsensusResult::ManualSeal(frontier_block_import), pending_transactions),
+			other: (
+				ConsensusResult::ManualSeal(frontier_block_import),
+				pending_transactions,
+			),
 		});
 	}
 
@@ -176,15 +182,18 @@ pub fn new_partial(
 		select_chain,
 		transaction_pool,
 		inherent_data_providers,
-		other: (ConsensusResult::Aura(aura_block_import, grandpa_link), pending_transactions),
+		other: (
+			ConsensusResult::Aura(aura_block_import, grandpa_link),
+			pending_transactions,
+		),
 	})
 }
 
 /// Builds a new service for a full client.
 pub fn new_full(
 	config: Configuration,
 	manual_seal: bool,
-	author: H160,
+	author_id: Option<H160>,
 ) -> Result<TaskManager, ServiceError> {
 	let sc_service::PartialComponents {
 		client,
@@ -196,7 +205,7 @@ pub fn new_full(
 		transaction_pool,
 		inherent_data_providers,
 		other: (consensus_result, pending_transactions),
-	} = new_partial(&config, manual_seal, Some(author))?;
+	} = new_partial(&config, manual_seal, author_id)?;
 
 	let (network, network_status_sinks, system_rpc_tx, network_starter) = match consensus_result {
 		ConsensusResult::ManualSeal(_) => {
@@ -284,43 +293,48 @@ pub fn new_full(
 
 	// Spawn Frontier pending transactions maintenance task (as essential, otherwise we leak).
 	if pending_transactions.is_some() {
+		use fp_consensus::{ConsensusLog, FRONTIER_ENGINE_ID};
 		use futures::StreamExt;
-		use fp_consensus::{FRONTIER_ENGINE_ID, ConsensusLog};
 		use sp_runtime::generic::OpaqueDigestItemId;
 
 		const TRANSACTION_RETAIN_THRESHOLD: u64 = 5;
 		task_manager.spawn_essential_handle().spawn(
 			"frontier-pending-transactions",
-			client.import_notification_stream().for_each(move |notification| {
-
-				if let Ok(locked) = &mut pending_transactions.clone().unwrap().lock() {
-					// As pending transactions have a finite lifespan anyway
-					// we can ignore MultiplePostRuntimeLogs error checks.
-					let mut frontier_log: Option<_> = None;
-					for log in notification.header.digest.logs {
-						let log = log.try_to::<ConsensusLog>(OpaqueDigestItemId::Consensus(&FRONTIER_ENGINE_ID));
-						if let Some(log) = log {
-							frontier_log = Some(log);
+			client
+				.import_notification_stream()
+				.for_each(move |notification| {
+					if let Ok(locked) = &mut pending_transactions.clone().unwrap().lock() {
+						// As pending transactions have a finite lifespan anyway
+						// we can ignore MultiplePostRuntimeLogs error checks.
+						let mut frontier_log: Option<_> = None;
+						for log in notification.header.digest.logs {
+							let log = log.try_to::<ConsensusLog>(OpaqueDigestItemId::Consensus(
+								&FRONTIER_ENGINE_ID,
+							));
+							if let Some(log) = log {
+								frontier_log = Some(log);
+							}
 						}
-					}
 
-					let imported_number: u64 = notification.header.number as u64;
+						let imported_number: u64 = notification.header.number as u64;
 
-					if let Some(ConsensusLog::EndBlock {
-						block_hash: _, transaction_hashes,
-					}) = frontier_log {
-						// Retain all pending transactions that were not
-						// processed in the current block.
-						locked.retain(|&k, _| !transaction_hashes.contains(&k));
+						if let Some(ConsensusLog::EndBlock {
+							block_hash: _,
+							transaction_hashes,
+						}) = frontier_log
+						{
+							// Retain all pending transactions that were not
+							// processed in the current block.
+							locked.retain(|&k, _| !transaction_hashes.contains(&k));
+						}
+						locked.retain(|_, v| {
+							// Drop all the transactions that exceeded the given lifespan.
+							let lifespan_limit = v.at_block + TRANSACTION_RETAIN_THRESHOLD;
+							lifespan_limit > imported_number
+						});
 					}
-					locked.retain(|_, v| {
-						// Drop all the transactions that exceeded the given lifespan.
-						let lifespan_limit = v.at_block + TRANSACTION_RETAIN_THRESHOLD;
-						lifespan_limit > imported_number
-					});
-				}
-				futures::future::ready(())
-			})
+					futures::future::ready(())
+				}),
 		);
 	}
 

scripts/run-alphanet-parachain.sh

@@ -84,7 +84,7 @@ $PARACHAIN_BINARY \
   --name parachain_$PARACHAIN_INDEX \
   $PARACHAIN_BASE_PATH \
   '-linfo,evm=debug,ethereum=trace,rpc=trace,cumulus_collator=debug,txpool=debug' \
-  --account-id 6Be02d1d3665660d22FF9624b7BE0551ee1Ac91b \
+  --author-id 6Be02d1d3665660d22FF9624b7BE0551ee1Ac91b \
   $PARACHAIN_BOOTNODES_ARGS \
   -- \
     --node-key ${PARACHAIN_KEYS[$PARACHAIN_INDEX]} \

scripts/run-moonbase-standalone.sh

@@ -58,7 +58,7 @@ $EXECUTABLE \
   --rpc-port $((STANDALONE_PORT + 1)) \
   --ws-port $((STANDALONE_PORT + 2)) \
   --validator \
-  --account-id 6Be02d1d3665660d22FF9624b7BE0551ee1Ac91b \
+  --author-id 6Be02d1d3665660d22FF9624b7BE0551ee1Ac91b \
   --rpc-cors all \
   --rpc-methods=unsafe \
   --execution wasm \

tests/tests/util/testWithMoonbeam.ts

@@ -49,7 +49,7 @@ export async function startMoonbeamNode(
     `--no-telemetry`,
     `--no-prometheus`,
     `--manual-seal`,
-    `--account-id=${GENESIS_ACCOUNT.substring(2)}`, // Required by author inherent
+    `--author-id=${GENESIS_ACCOUNT.substring(2)}`, // Required by author inherent
     `--no-grandpa`,
     `--force-authoring`,
     `-l${MOONBEAM_LOG}`,