feat(remove): Temporally remove the internal miner functionality (#8184)

* remove functionality of internal miner * fix configs * fix typo in comment * typo Co-authored-by: Arya <[email protected]> * remove internal config for tests to pass * typo --------- Co-authored-by: Arya <[email protected]>
ZcashFoundation · Jan 23, 2024 · d45864f · d45864f
1 parent 5824f85
commit d45864f
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 147 deletions.
diff --git a/Cargo.lock b/Cargo.lock
@@ -1369,16 +1369,6 @@ dependencies = [
  "byteorder",
 ]
 
-[[package]]
-name = "equihash"
-version = "0.2.0"
-source = "git+https://github.com/ZcashFoundation/librustzcash.git?branch=equihash-solver-tromp#aff6fac6cb9c7390565313733f0ba7d679166504"
-dependencies = [
- "blake2b_simd",
- "byteorder",
- "cc",
-]
-
 [[package]]
 name = "equivalent"
 version = "1.0.1"
@@ -5584,7 +5574,7 @@ dependencies = [
  "blake2s_simd",
  "bls12_381",
  "byteorder",
- "equihash 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "equihash",
  "ff",
  "fpe",
  "group",
@@ -5682,8 +5672,7 @@ dependencies = [
  "criterion",
  "displaydoc",
  "ed25519-zebra",
- "equihash 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "equihash 0.2.0 (git+https://github.com/ZcashFoundation/librustzcash.git?branch=equihash-solver-tromp)",
+ "equihash",
  "futures",
  "group",
  "halo2_proofs",

diff --git a/zebra-chain/Cargo.toml b/zebra-chain/Cargo.toml
@@ -35,12 +35,10 @@ getblocktemplate-rpcs = [
 ]
 
 # Experimental internal miner support
-internal-miner = [
- # TODO: replace with "equihash/solver" when that feature is merged and released:
- # https://github.com/zcash/librustzcash/pull/1083
- # https://github.com/zcash/librustzcash/pull/1088
- "equihash-solver",
-]
+# TODO: Internal miner feature functionality was removed at https://github.com/ZcashFoundation/zebra/issues/8180
+# See what was removed at https://github.com/ZcashFoundation/zebra/blob/v1.5.1/zebra-chain/Cargo.toml#L38-L43
+# Restore support when conditions are met. https://github.com/ZcashFoundation/zebra/issues/8183
+internal-miner = []
 
 # Experimental elasticsearch support
 elasticsearch = []
@@ -70,19 +68,10 @@ bridgetree = "0.4.0"
 bs58 = { version = "0.5.0", features = ["check"] }
 byteorder = "1.5.0"
 
+# TODO: Internal miner feature functionality was removed at https://github.com/ZcashFoundation/zebra/issues/8180
+# See what was removed at https://github.com/ZcashFoundation/zebra/blob/v1.5.1/zebra-chain/Cargo.toml#L73-L85
+# Restore support when conditions are met. https://github.com/ZcashFoundation/zebra/issues/8183
 equihash = "0.2.0"
-# Experimental internal miner support
-#
-# TODO: remove "equihash-solver" when the "equihash/solver" feature is merged and released:
-# https://github.com/zcash/librustzcash/pull/1083
-# https://github.com/zcash/librustzcash/pull/1088
-#
-# Use the solver PR:
-# - latest: branch = "equihash-solver-tromp",
-# - crashing with double-frees: rev = "da26c34772f4922eb13b4a1e7d88a969bbcf6a91",
-equihash-solver = { version = "0.2.0", git = "https://github.com/ZcashFoundation/librustzcash.git", branch = "equihash-solver-tromp", features = ["solver"], package = "equihash", optional = true }
-# or during development, use the locally checked out and modified version of equihash:
-#equihash-solver = { version = "0.2.0", path = "../../librustzcash/components/equihash", features = ["solver"], package = "equihash", optional = true }
 
 group = "0.13.0"
 incrementalmerkletree = "0.5.0"

diff --git a/zebra-chain/src/work/equihash.rs b/zebra-chain/src/work/equihash.rs
@@ -106,93 +106,22 @@ impl Solution {
  #[cfg(feature = "internal-miner")]
  #[allow(clippy::unwrap_in_result)]
  pub fn solve<F>(
- mut header: Header,
- mut cancel_fn: F,
+ mut _header: Header,
+ mut _cancel_fn: F,
  ) -> Result<AtLeastOne<Header>, SolverCancelled>
  where
  F: FnMut() -> Result<(), SolverCancelled>,
  {
- use crate::shutdown::is_shutting_down;
-
- let mut input = Vec::new();
- header
- .zcash_serialize(&mut input)
- .expect("serialization into a vec can't fail");
- // Take the part of the header before the nonce and solution.
- // This data is kept constant for this solver run.
- let input = &input[0..Solution::INPUT_LENGTH];
-
- while !is_shutting_down() {
- // Don't run the solver if we'd just cancel it anyway.
- cancel_fn()?;
-
- let solutions = equihash_solver::tromp::solve_200_9_compressed(input, || {
- // Cancel the solver if we have a new template.
- if cancel_fn().is_err() {
- return None;
- }
-
- // This skips the first nonce, which doesn't matter in practice.
- Self::next_nonce(&mut header.nonce);
- Some(*header.nonce)
- });
-
- let mut valid_solutions = Vec::new();
-
- // If we got any solutions, try submitting them, because the new template might just
- // contain some extra transactions. Mining extra transactions is optional.
- for solution in &solutions {
- header.solution = Self::from_bytes(solution)
- .expect("unexpected invalid solution: incorrect length");
-
- // TODO: work out why we sometimes get invalid solutions here
- if let Err(error) = header.solution.check(&header) {
- info!(?error, "found invalid solution for header");
- continue;
- }
-
- if Self::difficulty_is_valid(&header) {
- valid_solutions.push(header);
- }
- }
-
- match valid_solutions.try_into() {
- Ok(at_least_one_solution) => return Ok(at_least_one_solution),
- Err(_is_empty_error) => debug!(
- solutions = ?solutions.len(),
- "found valid solutions which did not pass the validity or difficulty checks"
- ),
- }
- }
+ // TODO: Function code was removed as part of https://github.com/ZcashFoundation/zebra/issues/8180
+ // Find the removed code at https://github.com/ZcashFoundation/zebra/blob/v1.5.1/zebra-chain/src/work/equihash.rs#L115-L166
+ // Restore the code when conditions are met. https://github.com/ZcashFoundation/zebra/issues/8183
 
  Err(SolverCancelled)
  }
 
- /// Modifies `nonce` to be the next integer in big-endian order.
- /// Wraps to zero if the next nonce would overflow.
- #[cfg(feature = "internal-miner")]
- fn next_nonce(nonce: &mut [u8; 32]) {
- let _ignore_overflow = crate::primitives::byte_array::increment_big_endian(&mut nonce[..]);
- }
-
- /// Returns `true` if the `nonce` and `solution` in `header` meet the difficulty threshold.
- ///
- /// Assumes that the difficulty threshold in the header is valid.
- #[cfg(feature = "internal-miner")]
- fn difficulty_is_valid(header: &Header) -> bool {
- // Simplified from zebra_consensus::block::check::difficulty_is_valid().
- let difficulty_threshold = header
- .difficulty_threshold
- .to_expanded()
- .expect("unexpected invalid header template: invalid difficulty threshold");
-
- // TODO: avoid calculating this hash multiple times
- let hash = header.hash();
-
- // Note: this comparison is a u256 integer comparison, like zcashd and bitcoin. Greater
- // values represent *less* work.
- hash <= difficulty_threshold
- }
+ // TODO: Some methods were removed as part of https://github.com/ZcashFoundation/zebra/issues/8180
+ // Find the removed code at https://github.com/ZcashFoundation/zebra/blob/v1.5.1/zebra-chain/src/work/equihash.rs#L171-L196
+ // Restore the code when conditions are met. https://github.com/ZcashFoundation/zebra/issues/8183
 }
 
 impl PartialEq<Solution> for Solution {

diff --git a/zebra-rpc/src/config/mining.rs b/zebra-rpc/src/config/mining.rs
@@ -15,28 +15,9 @@ pub struct Config {
  /// `getblocktemplate` RPC coinbase transaction.
  pub miner_address: Option<transparent::Address>,
 
- /// Mine blocks using Zebra's internal miner, without an external mining pool or equihash solver.
- ///
- /// This experimental feature is only supported on testnet.
- /// Mainnet miners should use a mining pool with GPUs or ASICs designed for efficient mining.
- ///
- /// The internal miner is off by default.
- #[cfg(feature = "internal-miner")]
- pub internal_miner: bool,
-
- /// The number of internal miner threads used by Zebra.
- /// These threads are scheduled at low priority.
- ///
- /// The number of threads is limited by the available parallelism reported by the OS.
- /// If the number of threads isn't configured, or can't be detected, Zebra uses one thread.
- /// This is different from Zebra's other parallelism configs, because mining runs constantly and
- /// uses a large amount of memory. (144 MB of RAM and 100% of a core per thread.)
- ///
- /// If the number of threads is set to zero, Zebra disables mining.
- /// This matches `zcashd`'s behaviour, but is different from Zebra's other parallelism configs.
- #[cfg(feature = "internal-miner")]
- pub internal_miner_threads: usize,
-
+ // TODO: Internal miner config code was removed as part of https://github.com/ZcashFoundation/zebra/issues/8180
+ // Find the removed code at https://github.com/ZcashFoundation/zebra/blob/v1.5.1/zebra-rpc/src/config/mining.rs#L18-L38
+ // Restore the code when conditions are met. https://github.com/ZcashFoundation/zebra/issues/8183
  /// Extra data to include in coinbase transaction inputs.
  /// Limited to around 95 bytes by the consensus rules.
  ///
@@ -58,12 +39,9 @@ impl Default for Config {
  // TODO: do we want to default to v5 transactions and Zebra coinbase data?
  extra_coinbase_data: None,
  debug_like_zcashd: true,
- // TODO: ignore and warn rather than panicking if these fields are in the config,
- // but the feature isn't enabled.
- #[cfg(feature = "internal-miner")]
- internal_miner: false,
- #[cfg(feature = "internal-miner")]
- internal_miner_threads: 1,
+ // TODO: Internal miner config code was removed as part of https://github.com/ZcashFoundation/zebra/issues/8180
+ // Find the removed code at https://github.com/ZcashFoundation/zebra/blob/v1.5.1/zebra-rpc/src/config/mining.rs#L61-L66
+ // Restore the code when conditions are met. https://github.com/ZcashFoundation/zebra/issues/8183
  }
  }
 }
@@ -80,6 +58,9 @@ impl Config {
  /// Is the internal miner enabled using at least one thread?
  #[cfg(feature = "internal-miner")]
  pub fn is_internal_miner_enabled(&self) -> bool {
- self.internal_miner && self.internal_miner_threads > 0
+ // TODO: Changed to return always false so internal miner is never started. Part of https://github.com/ZcashFoundation/zebra/issues/8180
+ // Find the removed code at https://github.com/ZcashFoundation/zebra/blob/v1.5.1/zebra-rpc/src/config/mining.rs#L83
+ // Restore the code when conditions are met. https://github.com/ZcashFoundation/zebra/issues/8183
+ false
  }
 }
diff --git a/zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs b/zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs
@@ -97,8 +97,8 @@ pub async fn test_responses<State, ReadState>(
  miner_address: Some(transparent::Address::from_script_hash(network, [0xad; 20])),
  extra_coinbase_data: None,
  debug_like_zcashd: true,
- // Use default field values when optional features are enabled in tests
- ..Default::default()
+ // TODO: Use default field values when optional features are enabled in tests #8183
+ //..Default::default()
  };
 
  // nu5 block height

diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs
@@ -1236,8 +1236,8 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) {
  miner_address,
  extra_coinbase_data: None,
  debug_like_zcashd: true,
- // Use default field values when optional features are enabled in tests
- ..Default::default()
+ // TODO: Use default field values when optional features are enabled in tests #8183
+ //..Default::default()
  };
 
  // nu5 block height
@@ -1685,8 +1685,8 @@ async fn rpc_getdifficulty() {
  miner_address: None,
  extra_coinbase_data: None,
  debug_like_zcashd: true,
- // Use default field values when optional features are enabled in tests
- ..Default::default()
+ // TODO: Use default field values when optional features are enabled in tests #8183
+ //..Default::default()
  };
 
  // nu5 block height

diff --git a/zebrad/src/components/miner.rs b/zebrad/src/components/miner.rs
@@ -106,7 +106,7 @@ where
 ///
 /// See [`run_mining_solver()`] for more details.
 pub async fn init<Mempool, State, Tip, BlockVerifierRouter, SyncStatus, AddressBook>(
- config: Config,
+ _config: Config,
  rpc: GetBlockTemplateRpcImpl<Mempool, State, Tip, BlockVerifierRouter, SyncStatus, AddressBook>,
 ) -> Result<(), Report>
 where
@@ -138,7 +138,9 @@ where
  SyncStatus: ChainSyncStatus + Clone + Send + Sync + 'static,
  AddressBook: AddressBookPeers + Clone + Send + Sync + 'static,
 {
- let configured_threads = config.internal_miner_threads;
+ // TODO: change this to `config.internal_miner_threads` when internal miner feature is added back.
+ // https://github.com/ZcashFoundation/zebra/issues/8183
+ let configured_threads = 1;
  // If we can't detect the number of cores, use the configured number.
  let available_threads = available_parallelism()
  .map(usize::from)