Start mining when mining_author is set. (#1706)

* Start mining when `mining_author` is set.

And use another configuration `stop_mining` to allow seting
mining_author without mining blocks for testing purposes.

* Allow "0x" prefix in configurations that take hex strings.

* Fix unit test.

* Use disable_stratum and start_mining to config minings.

* Allow 0x prefix in node id.

* Fix unit test.

* Update CHANGELOG.

* Fix tests.

* Remove false check.

* Fix.

* Use `mining_type`.

* Fix comments.

Author: peilun-conflux

blockgen/src/lib.rs

@@ -174,7 +174,7 @@ impl BlockGenerator {
 
     /// Send new PoW problem to workers
     pub fn send_problem(bg: Arc<BlockGenerator>, problem: ProofOfWorkProblem) {
-        if bg.pow_config.use_stratum {
+        if bg.pow_config.use_stratum() {
             let stratum = bg.stratum.read();
             stratum.as_ref().unwrap().notify(problem);
         } else {
@@ -697,7 +697,7 @@ impl BlockGenerator {
             time::Duration::from_millis(BLOCKGEN_LOOP_SLEEP_IN_MILISECS);
 
         let receiver: mpsc::Receiver<ProofOfWorkSolution> =
-            if bg.pow_config.use_stratum {
+            if bg.pow_config.use_stratum() {
                 BlockGenerator::start_new_stratum_worker(bg.clone())
             } else {
                 BlockGenerator::start_new_worker(1, bg.clone())
@@ -793,7 +793,7 @@ impl BlockGenerator {
                     // disconnected people may lose the previous message
                     if let Some(problem) = current_problem {
                         if let Ok(elapsed) = last_notify.elapsed() {
-                            if bg.pow_config.use_stratum
+                            if bg.pow_config.use_stratum()
                                 && elapsed > Duration::from_secs(60)
                             {
                                 BlockGenerator::send_problem(

changelogs/CHANGELOG-0.6.x.md

@@ -1,3 +1,9 @@
+# 0.6.2
+
+## Improvement
+- Start stratum services automatically if `mining_author` is set. 
+Use `mining_type` to allow start CPU mining or disable mining manually.
+
 # 0.6.0
 
 ## Bug Fixes

changelogs/CHANGELOG-1.0.x.md

@@ -1,5 +1,9 @@
 # 1.0.0
 
+## Improvement
+- Start stratum services automatically if `mining_author` is set. 
+Use `mining_type` to allow start CPU mining or disable mining manually.
+
 ## Bug Fixes
 
 - Fix bug where users need to restart node before they can use a newly created account to send transactions.

client/src/common/mod.rs

@@ -414,10 +414,9 @@ pub fn initialize_not_light_node_modules(
     );
 
     let maybe_author: Option<Address> =
-        conf.raw_conf.mining_author.clone().map(|hex_str| {
-            Address::from_str(hex_str.as_str()).expect(
-                "mining-author should be 40-digit hex string without 0x prefix",
-            )
+        conf.raw_conf.mining_author.as_ref().map(|hex_str| {
+            parse_hex_string(hex_str)
+                .expect("mining-author should be 40-digit hex string")
         });
     let blockgen = Arc::new(BlockGenerator::new(
         sync_graph,
@@ -438,21 +437,19 @@ pub fn initialize_not_light_node_modules(
                 bg.auto_block_generation(interval_ms);
             })
             .expect("Mining thread spawn error");
-    } else if conf.raw_conf.start_mining {
-        if let Some(author) = maybe_author {
-            if !author.is_valid_address() || author.is_builtin_address() {
-                panic!("mining-author must starts with 0x1 (user address) or 0x8 (contract address), otherwise you will not get mining rewards!!!");
-            }
-        } else {
-            panic!("mining-author is not set correctly, so you'll not get mining rewards!!!");
+    } else if let Some(author) = maybe_author {
+        if !author.is_valid_address() || author.is_builtin_address() {
+            panic!("mining-author must start with 0x1 (user address) or 0x8 (contract address), otherwise you will not get mining rewards!!!");
+        }
+        if blockgen.pow_config.enable_mining() {
+            let bg = blockgen.clone();
+            thread::Builder::new()
+                .name("mining".into())
+                .spawn(move || {
+                    BlockGenerator::start_mining(bg, 0);
+                })
+                .expect("Mining thread spawn error");
         }
-        let bg = blockgen.clone();
-        thread::Builder::new()
-            .name("mining".into())
-            .spawn(move || {
-                BlockGenerator::start_mining(bg, 0);
-            })
-            .expect("Mining thread spawn error");
     }
 
     let rpc_impl = Arc::new(RpcImpl::new(
@@ -706,6 +703,7 @@ pub mod delegate_convert {
 pub use crate::configuration::Configuration;
 use crate::{
     accounts::{account_provider, keys_path},
+    configuration::parse_hex_string,
     rpc::{
         extractor::RpcExtractor,
         impls::{

client/src/configuration.rs

@@ -76,7 +76,7 @@ build_config! {
         //     * Open port 12535 for ws rpc if `jsonrpc_ws_port` is not provided.
         //     * Open port 12536 for tcp rpc if `jsonrpc_tcp_port` is not provided.
         //     * Open port 12537 for http rpc if `jsonrpc_http_port` is not provided.
-        //     * generate blocks automatically without PoW if `start_mining` is false
+        //     * generate blocks automatically without PoW.
         //     * Skip catch-up mode even there is no peer
         //
         (mode, (Option<String>), None)
@@ -85,7 +85,7 @@ build_config! {
         (debug_invalid_state_root_epoch, (Option<String>), None)
         (debug_dump_dir_invalid_state_root, (String), "./storage_db/debug_dump_invalid_state_root/".to_string())
         // Controls block generation speed.
-        // Only effective in `dev` mode and `start_mining` is false
+        // Only effective in `dev` mode
         (dev_block_interval_ms, (u64), 250)
         (enable_state_expose, (bool), false)
         (generate_tx, (bool), false)
@@ -124,11 +124,10 @@ build_config! {
 
         // Mining section.
         (mining_author, (Option<String>), None)
-        (start_mining, (bool), false)
+        (mining_type, (Option<String>), None)
         (stratum_listen_address, (String), "127.0.0.1".into())
         (stratum_port, (u16), 32525)
         (stratum_secret, (Option<String>), None)
-        (use_stratum, (bool), false)
         (use_octopus_in_test_mode, (bool), false)
 
         // Network section.
@@ -313,9 +312,8 @@ impl Configuration {
             network_config.config_path = self.raw_conf.netconf_dir.clone();
         }
         network_config.use_secret =
-            self.raw_conf.net_key.clone().map(|sec_str| {
-                sec_str
-                    .parse()
+            self.raw_conf.net_key.as_ref().map(|sec_str| {
+                parse_hex_string(sec_str)
                     .expect("net_key is not a valid secret string")
             });
         if let Some(addr) = self.raw_conf.public_address.clone() {
@@ -450,16 +448,25 @@ impl Configuration {
 
     pub fn pow_config(&self) -> ProofOfWorkConfig {
         let stratum_secret =
-            self.raw_conf
-                .stratum_secret
-                .clone()
-                .map(|hex_str| H256::from_str(hex_str.as_str())
-                    .expect("Stratum secret should be 64-digit hex string without 0x prefix"));
+            self.raw_conf.stratum_secret.as_ref().map(|hex_str| {
+                parse_hex_string(hex_str)
+                    .expect("Stratum secret should be 64-digit hex string")
+            });
 
         ProofOfWorkConfig::new(
             self.is_test_or_dev_mode(),
             self.raw_conf.use_octopus_in_test_mode,
-            self.raw_conf.use_stratum,
+            self.raw_conf.mining_type.as_ref().map_or_else(
+                || {
+                    // Enable stratum implicitly if `mining_author` is set.
+                    if self.raw_conf.mining_author.is_some() {
+                        "stratum"
+                    } else {
+                        "disable"
+                    }
+                },
+                |s| s.as_str(),
+            ),
             self.raw_conf.initial_difficulty,
             self.raw_conf.stratum_listen_address.clone(),
             self.raw_conf.stratum_port,
@@ -752,6 +759,8 @@ pub fn to_bootnodes(bootnodes: &Option<String>) -> Result<Vec<String>, String> {
     match *bootnodes {
         Some(ref x) if !x.is_empty() => x
             .split(',')
+            // ignore empty strings
+            .filter(|s| !s.is_empty())
             .map(|s| match validate_node_url(s).map(Into::into) {
                 None => Ok(s.to_owned()),
                 Some(ErrorKind::AddressResolve(_)) => Err(format!(
@@ -768,3 +777,7 @@ pub fn to_bootnodes(bootnodes: &Option<String>) -> Result<Vec<String>, String> {
         None => Ok(vec![]),
     }
 }
+
+pub fn parse_hex_string<F: FromStr>(hex_str: &str) -> Result<F, F::Err> {
+    hex_str.strip_prefix("0x").unwrap_or(hex_str).parse()
+}

client/src/tests/blockgen_tests.rs

@@ -84,7 +84,8 @@ fn test_mining_10_epochs() {
     conf.raw_conf.tcp_port = 13001;
     conf.raw_conf.jsonrpc_http_port = Some(18001);
     conf.raw_conf.mining_author =
-        Some("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".into());
+        Some("1aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".into());
+    conf.raw_conf.mining_type = Some("cpu".into());
 
     let exit = Arc::new((Mutex::new(false), Condvar::new()));
     let handle = ArchiveClient::start(conf, exit).unwrap();

core/src/pow/mod.rs

@@ -16,6 +16,7 @@ use cfx_types::{BigEndianHash, H256, U256, U512};
 use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
 use malloc_size_of_derive::MallocSizeOf as DeriveMallocSizeOf;
 use parking_lot::RwLock;
+use static_assertions::_core::str::FromStr;
 use std::{
     collections::{HashMap, VecDeque},
     convert::TryFrom,
@@ -60,11 +61,32 @@ pub struct ProofOfWorkSolution {
     pub nonce: U256,
 }
 
+#[derive(Debug, Clone, DeriveMallocSizeOf)]
+pub enum MiningType {
+    Stratum,
+    CPU,
+    Disable,
+}
+
+impl FromStr for MiningType {
+    type Err = String;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let mining_type = match s {
+            "stratum" => Self::Stratum,
+            "cpu" => Self::CPU,
+            "disable" => Self::Disable,
+            _ => return Err("invalid mining type".into()),
+        };
+        Ok(mining_type)
+    }
+}
+
 #[derive(Debug, Clone, DeriveMallocSizeOf)]
 pub struct ProofOfWorkConfig {
     pub test_mode: bool,
     pub use_octopus_in_test_mode: bool,
-    pub use_stratum: bool,
+    pub mining_type: MiningType,
     pub initial_difficulty: u64,
     pub block_generation_period: u64,
     pub difficulty_adjustment_epoch_period: u64,
@@ -75,7 +97,7 @@ pub struct ProofOfWorkConfig {
 
 impl ProofOfWorkConfig {
     pub fn new(
-        test_mode: bool, use_octopus_in_test_mode: bool, use_stratum: bool,
+        test_mode: bool, use_octopus_in_test_mode: bool, mining_type: &str,
         initial_difficulty: Option<u64>, stratum_listen_addr: String,
         stratum_port: u16, stratum_secret: Option<H256>,
     ) -> Self
@@ -84,7 +106,7 @@ impl ProofOfWorkConfig {
             ProofOfWorkConfig {
                 test_mode,
                 use_octopus_in_test_mode,
-                use_stratum,
+                mining_type: mining_type.parse().expect("Invalid mining type"),
                 initial_difficulty: initial_difficulty.unwrap_or(4),
                 block_generation_period: 1000000,
                 difficulty_adjustment_epoch_period: 20,
@@ -96,7 +118,7 @@ impl ProofOfWorkConfig {
             ProofOfWorkConfig {
                 test_mode,
                 use_octopus_in_test_mode,
-                use_stratum,
+                mining_type: mining_type.parse().expect("Invalid mining type"),
                 initial_difficulty: INITIAL_DIFFICULTY,
                 block_generation_period: TARGET_AVERAGE_BLOCK_GENERATION_PERIOD,
                 difficulty_adjustment_epoch_period:
@@ -112,6 +134,14 @@ impl ProofOfWorkConfig {
         !self.test_mode || self.use_octopus_in_test_mode
     }
 
+    pub fn use_stratum(&self) -> bool {
+        matches!(self.mining_type, MiningType::Stratum)
+    }
+
+    pub fn enable_mining(&self) -> bool {
+        !matches!(self.mining_type, MiningType::Disable)
+    }
+
     pub fn target_difficulty(
         &self, block_count: u64, timespan: u64, cur_difficulty: &U256,
     ) -> U256 {

core/src/sync/utils.rs

@@ -174,9 +174,9 @@ pub fn initialize_synchronization_graph_with_data_manager(
 
     let vm = VmFactory::new(1024 * 32);
     let pow_config = ProofOfWorkConfig::new(
-        true,  /* test_mode */
-        false, /* use_octopus_in_test_mode */
-        false, /* use_stratum */
+        true,      /* test_mode */
+        false,     /* use_octopus_in_test_mode */
+        "disable", /* mining_type */
         Some(10),
         String::from(""), /* stratum_listen_addr */
         0,                /* stratum_port */

network/src/node_table.rs

@@ -305,17 +305,24 @@ impl FromStr for Node {
     type Err = Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let (id, endpoint) = if s.len() > 138
-            && &s[0..10] == "cfxnode://"
-            && &s[138..139] == "@"
-        {
-            (
-                s[10..138].parse().map_err(|_| ErrorKind::InvalidNodeId)?,
-                NodeEndpoint::from_str(&s[139..])?,
-            )
-        } else {
-            (NodeId::default(), NodeEndpoint::from_str(s)?)
-        };
+        let (id, endpoint) =
+            if let Some(id_and_address_str) = s.strip_prefix("cfxnode://") {
+                // A node url with format "cfxnode://ID@IP:PORT"
+                let delimiter_index = id_and_address_str
+                    .find("@")
+                    .ok_or(ErrorKind::AddressParse)?;
+                (
+                    id_and_address_str[..delimiter_index]
+                        .parse()
+                        .map_err(|_| ErrorKind::InvalidNodeId)?,
+                    NodeEndpoint::from_str(
+                        &id_and_address_str[delimiter_index + 1..],
+                    )?,
+                )
+            } else {
+                // A simple address without node id.
+                (NodeId::default(), NodeEndpoint::from_str(s)?)
+            };
 
         Ok(Node {
             id,

run/default.toml

@@ -25,7 +25,7 @@ bootnodes="cfxnode://25265e1aa470d9d8667947820c4830a64e9f9678d6cb23ecde91e044752
 #     * Open port 12535 for ws rpc if `jsonrpc_ws_port` is not provided.
 #     * Open port 12536 for tcp rpc if `jsonrpc_tcp_port` is not provided.
 #     * Open port 12537 for http rpc if `jsonrpc_http_port` is not provided.
-#     * generate blocks automatically without PoW if `start_mining` is false
+#     * generate blocks automatically without PoW
 #     * Skip catch-up mode even there is no peer
 #
 # mode = ""
@@ -41,21 +41,20 @@ bootnodes="cfxnode://25265e1aa470d9d8667947820c4830a64e9f9678d6cb23ecde91e044752
 
 # ----------------- Mining Configuration -----------------
 
-# `start_mining` controls whether a thread is started to mine new blocks.
-# If not set, the node will not start mining blocks.
-# If the value is true, `mining_author` must be set.
-#
-# start_mining=true
-
 # `mining_author` is the address to receive mining rewards.
-# The value is a 40-digit hex string without 0x prefix.
+# If set, `mining_type` will be "stratum" by default.
+# The value is a 40-digit hex string.
 # By default, the value is not set.
 #
 # mining_author="1aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
 
-# `use_stratum` controls whether the mining process goes through the
-# stratum protocol.
-# use_stratum=true
+# `mining_type` controls whether the mining process goes through the
+# stratum protocol, uses CPU-mining, or disable mining.
+# Possible values are "stratum", "cpu", and "disable".
+# The default value is "stratum" if `mining_author` is set.
+# If the value is set and not "disable", `mining_author` must be set.
+#
+# mining_type = "stratum"
 
 # Listen address for stratum
 #
@@ -66,7 +65,7 @@ bootnodes="cfxnode://25265e1aa470d9d8667947820c4830a64e9f9678d6cb23ecde91e044752
 # stratum_port = 32525
 
 # Secret key for stratum.
-# The value is 64-digit hex string without 0x prefix.
+# The value is 64-digit hex string.
 # If not set, the RPC subscription will not check the authorization.
 #
 # stratum_secret = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
@@ -296,7 +295,7 @@ jsonrpc_local_http_port=12539
 # netconf_dir="./net_config"
 
 # `net_key` is the 256-bit private key to generate a unique node id for this node.
-# The value is a 64-digit hex string without 0x prefix.
+# The value is a 64-digit hex string.
 # If not set, the node will try to read from the file "key" under the directory `netconf_dir`.
 # If the file is not found, the node will generate a random key.
 # By default, the value is not set.