Various improvements (#48)

* Introduce RpcParams into run cmd

* No need to clone params in Run

* Use proper subcoin is_major_syncing signal

* Clean up TODOs

* Rename to subcoin_params.rs

* Add book.toml

* Docs

Author: liuchengxu

crates/sc-consensus-nakamoto/src/block_executor.rs

@@ -503,12 +503,14 @@ where
     }
 }
 
+/// Executor for benchmarking the runtime execution with different backends.
 pub struct BenchmarkRuntimeBlockExecutor<Block: BlockT> {
     disk_runtime_block_executor: Box<dyn BlockExecutor<Block>>,
     in_memory_runtime_block_executor: Box<dyn BlockExecutor<Block>>,
 }
 
 impl<Block: BlockT> BenchmarkRuntimeBlockExecutor<Block> {
+    /// Constructs a new instance of [`BenchmarkRuntimeBlockExecutor`].
     pub fn new(
         disk_runtime_block_executor: Box<dyn BlockExecutor<Block>>,
         in_memory_runtime_block_executor: Box<dyn BlockExecutor<Block>>,
@@ -581,6 +583,7 @@ impl<Block: BlockT> BlockExecutor<Block> for BenchmarkRuntimeBlockExecutor<Block
     }
 }
 
+/// This is responsible for benchmarking all kinds of block executors.
 pub struct BenchmarkAllExecutor<
     Block,
     DiskRuntime,
@@ -598,6 +601,7 @@ pub struct BenchmarkAllExecutor<
 impl<Block, DiskRuntime, InMemoryRuntime, DiskOffRuntime, InMemoryOffRuntime>
     BenchmarkAllExecutor<Block, DiskRuntime, InMemoryRuntime, DiskOffRuntime, InMemoryOffRuntime>
 {
+    /// Constructs a new instance of [`BenchmarkAllExecutor`].
     pub fn new(
         disk_runtime_block_executor: DiskRuntime,
         in_memory_runtime_block_executor: InMemoryRuntime,

crates/sc-consensus-nakamoto/src/lib.rs

@@ -19,6 +19,7 @@ pub use import_queue::{
 };
 pub use verification::{BlockVerification, BlockVerifier, HeaderVerifier};
 
+/// Consensus error type.
 #[derive(Debug, thiserror::Error)]
 pub enum Error {
     #[error("Header uses the wrong engine {0:?}")]

crates/subcoin-informant/src/lib.rs

@@ -84,8 +84,7 @@ where
             }
         });
 
-    // TODO: proper status
-    let is_major_syncing = Arc::new(true.into());
+    let is_major_syncing = network.is_major_syncing();
 
     futures::select! {
         () = display_notifications.fuse() => (),

crates/subcoin-network/src/lib.rs

@@ -199,6 +199,7 @@ impl Clone for Bandwidth {
     }
 }
 
+/// Represents the result of sending a transaction to the network.
 #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub enum SendTransactionResult {
@@ -270,6 +271,7 @@ impl NetworkHandle {
         receiver.await.unwrap_or_default()
     }
 
+    /// Fetches the transaction for given txid.
     pub async fn get_transaction(&self, txid: Txid) -> Option<Transaction> {
         let (sender, receiver) = oneshot::channel();
 
@@ -284,6 +286,7 @@ impl NetworkHandle {
         receiver.await.ok().flatten()
     }
 
+    /// Sends an transaction to the network.
     pub async fn send_transaction(&self, transaction: Transaction) -> SendTransactionResult {
         let (sender, receiver) = oneshot::channel();
 
@@ -308,6 +311,7 @@ impl NetworkHandle {
             .unwrap_or(SendTransactionResult::Failure("Internal error".to_string()))
     }
 
+    /// Starts the block sync in chain sync component.
     pub fn start_block_sync(&self) -> bool {
         self.worker_msg_sender
             .unbounded_send(NetworkWorkerMessage::StartBlockSync)

crates/subcoin-node/src/cli.rs

@@ -1,4 +1,5 @@
-pub mod params;
+pub mod rpc_params;
+pub mod subcoin_params;
 
 use crate::commands::blockchain::{Blockchain, BlockchainCmd};
 use crate::commands::import_blocks::{ImportBlocks, ImportBlocksCmd};
@@ -86,11 +87,11 @@ pub fn run() -> sc_cli::Result<()> {
 
     match command {
         Command::Run(run) => {
-            let run_cmd = RunCmd::new(&run);
+            let run_cmd = RunCmd::new(*run);
             let runner = SubstrateCli.create_runner(&run_cmd)?;
             runner.run_node_until_exit(|config| async move {
                 run_cmd
-                    .start(config, *run, no_hardware_benchmarks, storage_monitor)
+                    .start(config, no_hardware_benchmarks, storage_monitor)
                     .await
             })
         }

crates/subcoin-node/src/cli/rpc_params.rs

@@ -0,0 +1,180 @@
+use clap::Parser;
+use sc_cli::{
+    Cors, RpcMethods, RPC_DEFAULT_MAX_CONNECTIONS, RPC_DEFAULT_MAX_REQUEST_SIZE_MB,
+    RPC_DEFAULT_MAX_RESPONSE_SIZE_MB, RPC_DEFAULT_MAX_SUBS_PER_CONN,
+    RPC_DEFAULT_MESSAGE_CAPACITY_PER_CONN,
+};
+use sc_service::config::IpNetwork;
+use std::net::{IpAddr, Ipv4Addr, SocketAddr};
+use std::num::NonZeroU32;
+
+/// RPC parameters extracted from the upstream RunCmd.
+#[derive(Debug, Clone, Parser)]
+pub struct RpcParams {
+    /// Listen to all RPC interfaces (default: local).
+    ///
+    /// Not all RPC methods are safe to be exposed publicly.
+    ///
+    /// Use an RPC proxy server to filter out dangerous methods. More details:
+    /// <https://docs.substrate.io/build/remote-procedure-calls/#public-rpc-interfaces>.
+    ///
+    /// Use `--unsafe-rpc-external` to suppress the warning if you understand the risks.
+    #[arg(long)]
+    pub rpc_external: bool,
+
+    /// Listen to all RPC interfaces.
+    ///
+    /// Same as `--rpc-external`.
+    #[arg(long)]
+    pub unsafe_rpc_external: bool,
+
+    /// RPC methods to expose.
+    #[arg(
+		long,
+		value_name = "METHOD SET",
+		value_enum,
+		ignore_case = true,
+		default_value_t = RpcMethods::Auto,
+		verbatim_doc_comment
+	)]
+    pub rpc_methods: RpcMethods,
+
+    /// RPC rate limiting (calls/minute) for each connection.
+    ///
+    /// This is disabled by default.
+    ///
+    /// For example `--rpc-rate-limit 10` will maximum allow
+    /// 10 calls per minute per connection.
+    #[arg(long)]
+    pub rpc_rate_limit: Option<NonZeroU32>,
+
+    /// Disable RPC rate limiting for certain ip addresses.
+    ///
+    /// Each IP address must be in CIDR notation such as `1.2.3.4/24`.
+    #[arg(long, num_args = 1..)]
+    pub rpc_rate_limit_whitelisted_ips: Vec<IpNetwork>,
+
+    /// Trust proxy headers for disable rate limiting.
+    ///
+    /// By default the rpc server will not trust headers such `X-Real-IP`, `X-Forwarded-For` and
+    /// `Forwarded` and this option will make the rpc server to trust these headers.
+    ///
+    /// For instance this may be secure if the rpc server is behind a reverse proxy and that the
+    /// proxy always sets these headers.
+    #[arg(long)]
+    pub rpc_rate_limit_trust_proxy_headers: bool,
+
+    /// Set the maximum RPC request payload size for both HTTP and WS in megabytes.
+    #[arg(long, default_value_t = RPC_DEFAULT_MAX_REQUEST_SIZE_MB)]
+    pub rpc_max_request_size: u32,
+
+    /// Set the maximum RPC response payload size for both HTTP and WS in megabytes.
+    #[arg(long, default_value_t = RPC_DEFAULT_MAX_RESPONSE_SIZE_MB)]
+    pub rpc_max_response_size: u32,
+
+    /// Set the maximum concurrent subscriptions per connection.
+    #[arg(long, default_value_t = RPC_DEFAULT_MAX_SUBS_PER_CONN)]
+    pub rpc_max_subscriptions_per_connection: u32,
+
+    /// Specify JSON-RPC server TCP port.
+    #[arg(long, value_name = "PORT")]
+    pub rpc_port: Option<u16>,
+
+    /// Maximum number of RPC server connections.
+    #[arg(long, value_name = "COUNT", default_value_t = RPC_DEFAULT_MAX_CONNECTIONS)]
+    pub rpc_max_connections: u32,
+
+    /// The number of messages the RPC server is allowed to keep in memory.
+    ///
+    /// If the buffer becomes full then the server will not process
+    /// new messages until the connected client start reading the
+    /// underlying messages.
+    ///
+    /// This applies per connection which includes both
+    /// JSON-RPC methods calls and subscriptions.
+    #[arg(long, default_value_t = RPC_DEFAULT_MESSAGE_CAPACITY_PER_CONN)]
+    pub rpc_message_buffer_capacity_per_connection: u32,
+
+    /// Disable RPC batch requests
+    #[arg(long, alias = "rpc_no_batch_requests", conflicts_with_all = &["rpc_max_batch_request_len"])]
+    pub rpc_disable_batch_requests: bool,
+
+    /// Limit the max length per RPC batch request
+    #[arg(long, conflicts_with_all = &["rpc_disable_batch_requests"], value_name = "LEN")]
+    pub rpc_max_batch_request_len: Option<u32>,
+
+    /// Specify browser *origins* allowed to access the HTTP & WS RPC servers.
+    ///
+    /// A comma-separated list of origins (protocol://domain or special `null`
+    /// value). Value of `all` will disable origin validation. Default is to
+    /// allow localhost and <https://polkadot.js.org> origins. When running in
+    /// `--dev` mode the default is to allow all origins.
+    #[arg(long, value_name = "ORIGINS")]
+    pub rpc_cors: Option<Cors>,
+}
+
+impl RpcParams {
+    pub(crate) fn rpc_cors(&self, is_dev: bool) -> sc_cli::Result<Option<Vec<String>>> {
+        Ok(self
+            .rpc_cors
+            .clone()
+            .unwrap_or_else(|| {
+                if is_dev {
+                    tracing::warn!("Running in --dev mode, RPC CORS has been disabled.");
+                    Cors::All
+                } else {
+                    Cors::List(vec![
+                        "http://localhost:*".into(),
+                        "http://127.0.0.1:*".into(),
+                        "https://localhost:*".into(),
+                        "https://127.0.0.1:*".into(),
+                        "https://polkadot.js.org".into(),
+                    ])
+                }
+            })
+            .into())
+    }
+
+    pub(crate) fn rpc_addr(&self, default_listen_port: u16) -> sc_cli::Result<Option<SocketAddr>> {
+        let interface = rpc_interface(
+            self.rpc_external,
+            self.unsafe_rpc_external,
+            self.rpc_methods,
+            false, // TODO: miner
+        )?;
+
+        Ok(Some(SocketAddr::new(
+            interface,
+            self.rpc_port.unwrap_or(default_listen_port),
+        )))
+    }
+}
+
+fn rpc_interface(
+    is_external: bool,
+    is_unsafe_external: bool,
+    rpc_methods: RpcMethods,
+    is_validator: bool,
+) -> sc_cli::Result<IpAddr> {
+    if is_external && is_validator && rpc_methods != RpcMethods::Unsafe {
+        return Err(sc_cli::Error::Input(
+            "--rpc-external option shouldn't be used if the node is running as \
+			 a validator. Use `--unsafe-rpc-external` or `--rpc-methods=unsafe` if you understand \
+			 the risks. See the options description for more information."
+                .to_owned(),
+        ));
+    }
+
+    if is_external || is_unsafe_external {
+        if rpc_methods == RpcMethods::Unsafe {
+            tracing::warn!(
+                "It isn't safe to expose RPC publicly without a proxy server that filters \
+				 available set of RPC methods."
+            );
+        }
+
+        Ok(Ipv4Addr::UNSPECIFIED.into())
+    } else {
+        Ok(Ipv4Addr::LOCALHOST.into())
+    }
+}

crates/subcoin-node/src/cli/params.rs …s/subcoin-node/src/cli/subcoin_params.rscrates/subcoin-node/src/cli/params.rs renamed to crates/subcoin-node/src/cli/subcoin_params.rs crates/subcoin-node/src/cli/params.rs renamed to crates/subcoin-node/src/cli/subcoin_params.rs

@@ -1,4 +1,4 @@
-use crate::cli::params::CommonParams;
+use crate::cli::subcoin_params::CommonParams;
 use crate::utils::Yield;
 use sc_cli::{ImportParams, NodeKeyParams, SharedParams};
 use sc_client_api::{HeaderBackend, StorageProvider};

crates/subcoin-node/src/commands/blockchain.rs

@@ -1,4 +1,4 @@
-use crate::cli::params::CommonParams;
+use crate::cli::subcoin_params::CommonParams;
 use crate::utils::Yield;
 use bitcoin_explorer::BitcoinDB;
 use futures::FutureExt;