refactor: improve CLI arguments

crates/client/l1-messages/Cargo.toml

@@ -40,7 +40,7 @@ sc-transaction-pool-api = { workspace = true, default-features = true }
 
 # Other third party dependencies
 ethers = { workspace = true }
-serde = { workspace = true }
-serde_json = { workspace = true }
+serde = { workspace = true, default-features = true }
+serde_json = { workspace = true, default-features = true }
 thiserror = { workspace = true }
 

crates/client/l1-messages/src/config.rs

@@ -2,10 +2,21 @@ use std::fs::File;
 use std::path::PathBuf;
 
 use ethers::types::Address;
+use rustc_hex::FromHexError;
 use serde::{Deserialize, Deserializer};
 use url::Url;
 
-use crate::error::L1MessagesConfigError;
+#[derive(thiserror::Error, Debug)]
+pub enum L1MessagesWorkerConfigError {
+    #[error("File with L1 Messages Worker config not found: {0}")]
+    FileNotFound(#[from] std::io::Error),
+    #[error("Failed to deserialize L1 Messages Worker Config from config file: {0}")]
+    InvalidFile(#[from] serde_json::Error),
+    #[error("Invalid Ethereum Provided Url: {0}")]
+    InvalidProviderUrl(#[from] url::ParseError),
+    #[error("Invalid L1 Contract Address: {0}")]
+    InvalidContractAddress(#[from] FromHexError),
+}
 
 #[derive(Clone, PartialEq, Deserialize, Debug)]
 #[serde(rename_all = "camelCase")]
@@ -29,13 +40,13 @@ impl L1MessagesWorkerConfig {
         Self { http_provider, contract_address }
     }
 
-    pub fn new_from_file(path: &PathBuf) -> Result<Self, L1MessagesConfigError> {
+    pub fn new_from_file(path: &PathBuf) -> Result<Self, L1MessagesWorkerConfigError> {
         let file = File::open(path)?;
         let config = serde_json::from_reader(file)?;
         Ok(config)
     }
 
-    pub fn new_from_params(provider_url: &str, contract_address: &str) -> Result<Self, L1MessagesConfigError> {
+    pub fn new_from_params(provider_url: &str, contract_address: &str) -> Result<Self, L1MessagesWorkerConfigError> {
         let http_provider = Url::parse(provider_url)?;
         let contract_address = contract_address.parse()?;
         Ok(Self { http_provider, contract_address })

crates/client/l1-messages/src/contract.rs

@@ -2,15 +2,26 @@ use ethers::contract::abigen;
 use mp_felt::{Felt252Wrapper, Felt252WrapperError};
 use mp_transactions::HandleL1MessageTransaction;
 
-use crate::error::L1EventToTransactionError;
-
 abigen!(
     L1Contract,
     r"[
 	event LogMessageToL2(address indexed fromAddress, uint256 indexed toAddress, uint256 indexed selector, uint256[] payload, uint256 nonce, uint256 fee)
 ]"
 );
 
+#[derive(thiserror::Error, Debug, PartialEq)]
+#[allow(clippy::enum_variant_names)]
+pub enum L1EventToTransactionError {
+    #[error("Failed to convert Calldata param from L1 Event: `{0}`")]
+    InvalidCalldata(Felt252WrapperError),
+    #[error("Failed to convert Contract Address from L1 Event: `{0}`")]
+    InvalidContractAddress(Felt252WrapperError),
+    #[error("Failed to convert Entrypoint Selector from L1 Event: `{0}`")]
+    InvalidEntryPointSelector(Felt252WrapperError),
+    #[error("Failed to convert Nonce param from L1 Event: `{0}`")]
+    InvalidNonce(Felt252WrapperError),
+}
+
 impl TryFrom<LogMessageToL2Filter> for HandleL1MessageTransaction {
     type Error = L1EventToTransactionError;
 

crates/client/l1-messages/src/error.rs

@@ -1,46 +1,15 @@
 use mc_db::DbError;
-use mp_felt::Felt252WrapperError;
-use rustc_hex::FromHexError;
 use sp_api::ApiError;
-use sp_runtime::DispatchError;
 use url::ParseError;
 
-#[derive(thiserror::Error, Debug, PartialEq)]
-pub enum L1EventToTransactionError {
-    #[error("Failed to convert Calldata param from L1 Event: `{0}`")]
-    InvalidCalldata(Felt252WrapperError),
-    #[error("Failed to convert Contract Address from L1 Event: `{0}`")]
-    InvalidContractAddress(Felt252WrapperError),
-    #[error("Failed to convert Entrypoint Selector from L1 Event: `{0}`")]
-    InvalidEntryPointSelector(Felt252WrapperError),
-    #[error("Failed to convert Nonce param from L1 Event: `{0}`")]
-    InvalidNonce(Felt252WrapperError),
-}
-
-#[derive(thiserror::Error, Debug)]
-pub enum L1MessagesConfigError {
-    #[error("File with L1 Messages Worker config not found: {0}")]
-    FileNotFound(#[from] std::io::Error),
-    #[error("Failed to deserialize L1 Messages Worker Config from config file: {0}")]
-    InvalidFile(#[from] serde_json::Error),
-    #[error("Invalid Ethereum Provided Url: {0}")]
-    InvalidProviderUrl(#[from] url::ParseError),
-    #[error("Invalid L1 Contract Address: {0}")]
-    InvalidContractAddress(#[from] FromHexError),
-    #[error("Missing Ethereum Provided Url")]
-    MissingProviderUrl,
-    #[error("Missing L1 Contract Address")]
-    MissingContractAddress,
-}
+use crate::contract::L1EventToTransactionError;
 
 #[derive(thiserror::Error, Debug)]
 pub enum L1MessagesWorkerError {
     #[error("Failed to initialize L1 Messages Worker based on provided Config: `{0}`")]
     ConfigError(#[from] ParseError),
     #[error("Failed to convert transaction via Runtime API: `{0}`")]
     ConvertTransactionRuntimeApiError(ApiError),
-    #[error("Failed to Dispatch Runtime API")]
-    RuntimeApiDispatchError(DispatchError),
     #[error("Madara Messaging DB Error: `{0}`")]
     DatabaseError(#[from] DbError),
     #[error("Message from L1 has been already processed, nonce: `{0}`")]

crates/client/l1-messages/src/worker.rs

@@ -61,7 +61,7 @@ pub async fn run_worker<C, P, B>(
         );
 
         match process_l1_message(event, &client, &pool, &backend, &meta.block_number.as_u64()).await {
-            Ok(tx_hash) => {
+            Ok(Some(tx_hash)) => {
                 log::info!(
                     "⟠ L1 Message from block: {:?}, transaction_hash: {:?}, log_index: {:?} submitted, transaction \
                      hash on L2: {:?}",
@@ -71,6 +71,7 @@ pub async fn run_worker<C, P, B>(
                     tx_hash
                 );
             }
+            Ok(None) => {}
             Err(e) => {
                 log::error!(
                     "⟠ Unexpected error while processing L1 Message from block: {:?}, transaction_hash: {:?}, \
@@ -91,7 +92,7 @@ async fn process_l1_message<C, P, B>(
     pool: &Arc<P>,
     backend: &Arc<mc_db::Backend<B>>,
     l1_block_number: &u64,
-) -> Result<P::Hash, L1MessagesWorkerError>
+) -> Result<Option<P::Hash>, L1MessagesWorkerError>
 where
     B: BlockT,
     C: ProvideRuntimeApi<B> + HeaderBackend<B>,
@@ -113,25 +114,18 @@ where
         Ok(true) => Ok(()),
         Ok(false) => {
             log::debug!("⟠ Event already processed: {:?}", transaction);
-            Err(L1MessagesWorkerError::L1MessageAlreadyProcessed(transaction.nonce))
+            return Ok(None);
         }
         Err(e) => {
-            log::error!("⟠ Unexpected runtime api error: {:?}", e);
+            log::error!("⟠ Unexpected Runtime Api error: {:?}", e);
             Err(L1MessagesWorkerError::RuntimeApiError(e))
         }
     }?;
 
-    let extrinsic = client
-        .runtime_api()
-        .convert_l1_transaction(best_block_hash, transaction, fee)
-        .map_err(|e| {
-            log::error!("⟠ Failed to convert transaction via runtime api: {:?}", e);
-            L1MessagesWorkerError::ConvertTransactionRuntimeApiError(e)
-        })?
-        .map_err(|e| {
-            log::error!("⟠ Failed to convert transaction via runtime api: {:?}", e);
-            L1MessagesWorkerError::RuntimeApiDispatchError(e)
-        })?;
+    let extrinsic = client.runtime_api().convert_l1_transaction(best_block_hash, transaction, fee).map_err(|e| {
+        log::error!("⟠ Failed to convert L1 Transaction via Runtime Api: {:?}", e);
+        L1MessagesWorkerError::ConvertTransactionRuntimeApiError(e)
+    })?;
 
     let tx_hash = pool.submit_one(best_block_hash, TX_SOURCE, extrinsic).await.map_err(|e| {
         log::error!("⟠ Failed to submit transaction with L1 Message: {:?}", e);
@@ -143,5 +137,5 @@ where
         L1MessagesWorkerError::DatabaseError(e)
     })?;
 
-    Ok(tx_hash)
+    Ok(Some(tx_hash))
 }

crates/client/rpc/src/lib.rs

@@ -1130,18 +1130,12 @@ where
     C: ProvideRuntimeApi<B>,
     C::Api: StarknetRuntimeApi<B> + ConvertTransactionRuntimeApi<B>,
 {
-    let result = client.runtime_api().convert_transaction(best_block_hash, transaction).map_err(|e| {
+    let extrinsic = client.runtime_api().convert_transaction(best_block_hash, transaction).map_err(|e| {
         error!("Failed to convert transaction: {:?}", e);
         StarknetRpcApiError::InternalServerError
     })?;
 
-    match result {
-        Ok(extrinsic) => Ok(extrinsic),
-        Err(dispatch_error) => {
-            error!("Failed to convert transaction: {:?}", dispatch_error);
-            Err(StarknetRpcApiError::InternalServerError)
-        }
-    }
+    Ok(extrinsic)
 }
 
 fn convert_error<C, B, T>(

crates/node/src/commands/run.rs

@@ -2,15 +2,45 @@ use std::path::PathBuf;
 
 use madara_runtime::SealingMode;
 use mc_data_availability::DaLayer;
-use mc_l1_messages::config::L1MessagesWorkerConfig;
-use mc_l1_messages::error::L1MessagesConfigError;
+use mc_l1_messages::config::{L1MessagesWorkerConfig, L1MessagesWorkerConfigError};
 use sc_cli::{Result, RpcMethods, RunCmd, SubstrateCli};
 use sc_service::BasePath;
 use serde::{Deserialize, Serialize};
 
 use crate::cli::Cli;
 use crate::service;
 
+#[derive(Debug, Clone, clap::Args)]
+pub struct L1MessagesParams {
+    /// Ethereum Provider (Node) Url
+    #[clap(
+        long,
+        value_hint=clap::ValueHint::Url,
+    )]
+    pub provider_url: String,
+
+    /// L1 Contract Address
+    #[clap(
+        long,
+        value_hint=clap::ValueHint::Other,
+    )]
+    pub l1_contract_address: String,
+}
+
+#[derive(Debug, Clone, clap::Args)]
+pub struct L1Messages {
+    /// Path to configuration file for Ethereum Core Contract Events Listener
+    #[clap(
+        long,
+        conflicts_with_all=["provider_url", "l1_contract_address"],
+        value_hint=clap::ValueHint::FilePath,
+    )]
+    pub l1_messages_config: Option<PathBuf>,
+
+    #[clap(flatten)]
+    pub config_params: Option<L1MessagesParams>,
+}
+
 /// Available Sealing methods.
 #[derive(Debug, Copy, Clone, clap::ValueEnum, Default, Serialize, Deserialize)]
 pub enum Sealing {
@@ -55,33 +85,9 @@ pub struct ExtendedRunCmd {
     #[clap(long)]
     pub cache: bool,
 
-    /// Path to configuration file for Ethereum Core Contract Events Listener
-    #[clap(
-        long,
-        conflicts_with="provider_url",
-        conflicts_with="l1_contract_address",
-        value_hint=clap::ValueHint::FilePath,
-        require_equals=true,
-    )]
-    pub l1_messages_worker_config_file: Option<PathBuf>,
-
-    /// Ethereum Provider (Node) Url
-    #[clap(
-        long,
-        conflicts_with="l1_messages_worker_config_file",
-        value_hint=clap::ValueHint::Url,
-        require_equals=true,
-    )]
-    pub provider_url: Option<String>,
-
-    /// L1 Contract Address
-    #[clap(
-        long,
-        conflicts_with="l1_messages_worker_config_file",
-        value_hint=clap::ValueHint::Other,
-        require_equals=true,
-    )]
-    pub l1_contract_address: Option<String>,
+    /// Configuration for L1 Messages (Syncing) Worker
+    #[clap(flatten)]
+    pub l1_messages_worker: L1Messages,
 }
 
 impl ExtendedRunCmd {
@@ -117,8 +123,8 @@ pub fn run_node(mut cli: Cli) -> Result<()> {
         }
     };
 
-    let l1_messages_worker_config =
-        extract_l1_messages_worker_config(&cli.run).map_err(|e| sc_cli::Error::Input(e.to_string()))?;
+    let l1_messages_worker_config = extract_l1_messages_worker_config(&cli.run.l1_messages_worker)
+        .map_err(|e| sc_cli::Error::Input(e.to_string()))?;
 
     runner.run_node_until_exit(|config| async move {
         let sealing = cli.run.sealing.map(Into::into).unwrap_or_default();
@@ -128,22 +134,16 @@ pub fn run_node(mut cli: Cli) -> Result<()> {
 }
 
 fn extract_l1_messages_worker_config(
-    run_cmd: &ExtendedRunCmd,
-) -> std::result::Result<Option<L1MessagesWorkerConfig>, L1MessagesConfigError> {
-    if let Some(ref config_path) = run_cmd.l1_messages_worker_config_file {
+    run_cmd: &L1Messages,
+) -> std::result::Result<Option<L1MessagesWorkerConfig>, L1MessagesWorkerConfigError> {
+    if let Some(ref config_path) = run_cmd.l1_messages_config {
         let config = L1MessagesWorkerConfig::new_from_file(config_path)?;
         Ok(Some(config))
-    } else if let Some(ref provider_url) = run_cmd.provider_url {
-        if let Some(ref l1_contract_address) = run_cmd.l1_contract_address {
-            let config = L1MessagesWorkerConfig::new_from_params(provider_url, l1_contract_address)?;
-            Ok(Some(config))
-        } else {
-            Err(L1MessagesConfigError::MissingContractAddress)
-        }
-    } else if let Some(ref _l1_contract_address) = run_cmd.l1_contract_address {
-        Err(L1MessagesConfigError::MissingProviderUrl)
+    } else if let Some(ref config_params) = run_cmd.config_params {
+        let config =
+            L1MessagesWorkerConfig::new_from_params(&config_params.provider_url, &config_params.l1_contract_address)?;
+        Ok(Some(config))
     } else {
-        log::warn!("Madara initialized w/o L1 Messages Worker");
         Ok(None)
     }
 }

crates/pallets/starknet/src/runtime_api.rs

@@ -81,10 +81,10 @@ sp_api::decl_runtime_apis! {
 
     pub trait ConvertTransactionRuntimeApi {
         /// Converts the transaction to an UncheckedExtrinsic for submission to the pool.
-        fn convert_transaction(transaction: UserTransaction) -> Result<<Block as BlockT>::Extrinsic, DispatchError>;
+        fn convert_transaction(transaction: UserTransaction) -> <Block as BlockT>::Extrinsic;
 
         /// Converts the L1 Message transaction to an UncheckedExtrinsic for submission to the pool.
-        fn convert_l1_transaction(transaction: HandleL1MessageTransaction, fee: Fee) -> Result<<Block as BlockT>::Extrinsic, DispatchError>;
+        fn convert_l1_transaction(transaction: HandleL1MessageTransaction, fee: Fee) -> <Block as BlockT>::Extrinsic;
 
         /// Converts the DispatchError to an understandable error for the client
         fn convert_error(error: DispatchError) -> StarknetTransactionExecutionError;

crates/runtime/Cargo.toml

@@ -16,7 +16,6 @@ repository = "https://github.com/keep-starknet-strange/madara"
 targets = ["x86_64-unknown-linux-gnu"]
 
 [dependencies]
-log = { workspace = true }
 parity-scale-codec = { workspace = true, features = [] }
 scale-info = { workspace = true, features = [] }
 serde = { workspace = true }

crates/runtime/src/lib.rs

@@ -410,7 +410,7 @@ impl_runtime_apis! {
     }
 
     impl pallet_starknet::runtime_api::ConvertTransactionRuntimeApi<Block> for Runtime {
-        fn convert_transaction(transaction: UserTransaction) -> Result<UncheckedExtrinsic, DispatchError> {
+        fn convert_transaction(transaction: UserTransaction) -> UncheckedExtrinsic {
             let call = match transaction {
                 UserTransaction::Declare(tx, contract_class) => {
                     pallet_starknet::Call::declare { transaction: tx, contract_class  }
@@ -423,13 +423,13 @@ impl_runtime_apis! {
                 }
             };
 
-            Ok(UncheckedExtrinsic::new_unsigned(call.into()))
+            UncheckedExtrinsic::new_unsigned(call.into())
         }
 
-        fn convert_l1_transaction(transaction: HandleL1MessageTransaction, fee: Fee) -> Result<UncheckedExtrinsic, DispatchError> {
+        fn convert_l1_transaction(transaction: HandleL1MessageTransaction, fee: Fee) -> UncheckedExtrinsic {
             let call =  pallet_starknet::Call::<Runtime>::consume_l1_message { transaction, paid_fee_on_l1: fee };
 
-            Ok(UncheckedExtrinsic::new_unsigned(call.into()))
+            UncheckedExtrinsic::new_unsigned(call.into())
         }
 
         fn convert_error(error: DispatchError) -> StarknetTransactionExecutionError {