Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding the actual mining process to the recipes (basic-pow, hybrid-consensus) #433

Merged
merged 11 commits into from
May 11, 2021
4 changes: 2 additions & 2 deletions consensus/sha3pow/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::sync::Arc;
/// The test is done by multiplying the two together. If the product
/// overflows the bounds of U256, then the product (and thus the hash)
/// was too high.
fn hash_meets_difficulty(hash: &H256, difficulty: U256) -> bool {
pub fn hash_meets_difficulty(hash: &H256, difficulty: U256) -> bool {
let num_hash = U256::from(&hash[..]);
let (_, overflowed) = num_hash.overflowing_mul(difficulty);

Expand Down Expand Up @@ -60,7 +60,7 @@ impl<B: BlockT<Hash = H256>> PowAlgorithm<B> for MinimalSha3Algorithm {

fn difficulty(&self, _parent: B::Hash) -> Result<Self::Difficulty, Error<B>> {
// Fixed difficulty hardcoded here
Ok(U256::from(1_000_000))
Ok(U256::from(10))
}

fn verify(
Expand Down
32 changes: 31 additions & 1 deletion nodes/basic-pow/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,14 @@ use sc_client_api::{ExecutorProvider, RemoteBackend};
use sc_executor::native_executor_instance;
pub use sc_executor::NativeExecutor;
use sc_service::{error::Error as ServiceError, Configuration, PartialComponents, TaskManager};
use sha3pow::MinimalSha3Algorithm;
use sha3pow::*;
use sp_api::TransactionFor;
use sp_consensus::import_queue::BasicQueue;
use sp_inherents::InherentDataProviders;
use std::{sync::Arc, time::Duration};
use std::thread;
use sp_core::{H256, Encode};
use sha3::{Digest, Sha3_256};

// Our native executor instance.
native_executor_instance!(
Expand Down Expand Up @@ -193,6 +196,33 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
task_manager
.spawn_essential_handle()
.spawn_blocking("pow", worker_task);

// Start Mining
let mut numb: u8 = 0;
thread::spawn(move || {
loop {
let worker = _worker.clone();
let metadata = worker.lock().metadata();
if let Some(metadata) = metadata {
let nonce = H256::from_slice(Sha3_256::digest(&[numb]).as_slice());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I am not familiar with writing consensus, so I am also learning here. A few questions here:

Why is nonce a hash, instead of simply an incrementing number? I see that numb pretty much act as an incrementing nonce. But then this number should probably be of type U256 instead of u8.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jimmychu0807 for the review!

The nonce as a Hash is mimicking the Kulupu implementation that uses the RandomX algorithm, as one of it's security goals that the Miners should not be able to identify "weaker" authorised parameters (such as specific nonce values) that make the PoW computation significantly more efficient, thus giving an advantage to a miner who would know such parameters, and in Kulupu implementation the nonce is actully the Hash of a random value
let nonce = H256::random_using(&mut rng);
In this implementation it doesn't really need to be a Hash at all, I think an U256 would be a better fit, as we really do increment the nonce by 1 every unsuccessful attempt.
the u8 was used cuz i didn't change anything in the rest of the code, where the nonce is a Hash, and the Sha3_256::digest() takes a &[u8] as input, so i found it a faster way.
Bottom line using the U256 for the nonce would eliminate the need for the numb variable. It was just quick and dirty implementation to make it work as discussed in #432
I updated the code to make it uses the type U256 for the nonce.

let compute = Compute {
difficulty: metadata.difficulty,
pre_hash: metadata.pre_hash,
nonce,
};
let seal = compute.compute();
if hash_meets_difficulty(&seal.work, seal.difficulty) {
let mut worker = worker.lock();
worker.submit(seal.encode());
}
numb = numb.saturating_add(1u8);
if numb == 255u8 {
numb = 0;
}
thread::sleep(Duration::new(0, 500_000_000));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the logic here should be something like this?

Suggested change
if hash_meets_difficulty(&seal.work, seal.difficulty) {
let mut worker = worker.lock();
worker.submit(seal.encode());
}
numb = numb.saturating_add(1u8);
if numb == 255u8 {
numb = 0;
}
thread::sleep(Duration::new(0, 500_000_000));
if hash_meets_difficulty(&seal.work, seal.difficulty) {
let mut worker = worker.lock();
worker.submit(seal.encode());
} else {
numb = numb.saturating_add(1u8);
if numb == 255u8 {
numb = 0;
}
}

The last line of thread::sleep is in the else branch of if let Some(metadata) = metadata {..} else { thread::sleep }, based on code structure in https://github.com/kulupu/kulupu/blob/master/src/service.rs#L330-L377

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

putting the thread::sleep in the else branch is for producing a delay in case no Block is yet ready to start mining, but i added it to create a delay in the mining process itself, since the difficulty was reduced due to the usage of the u8 numb variable to generate a nonce.
i updated this, the nonce uses the type U256 now.

}
}
});
}

network_starter.start_network();
Expand Down
32 changes: 31 additions & 1 deletion nodes/hybrid-consensus/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@ use sc_executor::native_executor_instance;
pub use sc_executor::NativeExecutor;
use sc_finality_grandpa::GrandpaBlockImport;
use sc_service::{error::Error as ServiceError, Configuration, PartialComponents, TaskManager};
use sha3pow::MinimalSha3Algorithm;
use sha3pow::*;
use sp_api::TransactionFor;
use sp_consensus::import_queue::BasicQueue;
use sp_inherents::InherentDataProviders;
use std::sync::Arc;
use std::time::Duration;
use std::thread;
use sp_core::{H256, Encode};
jimmychu0807 marked this conversation as resolved.
Show resolved Hide resolved
use sha3::{Digest, Sha3_256};

// Our native executor instance.
native_executor_instance!(
Expand Down Expand Up @@ -206,6 +209,33 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
task_manager
.spawn_essential_handle()
.spawn_blocking("pow", worker_task);

// Start Mining
let mut numb: u8 = 0;
thread::spawn(move || {
loop {
let worker = _worker.clone();
let metadata = worker.lock().metadata();
if let Some(metadata) = metadata {
let nonce = H256::from_slice(Sha3_256::digest(&[numb]).as_slice());
let compute = Compute {
difficulty: metadata.difficulty,
pre_hash: metadata.pre_hash,
nonce,
};
let seal = compute.compute();
if hash_meets_difficulty(&seal.work, seal.difficulty) {
let mut worker = worker.lock();
worker.submit(seal.encode());
}
numb = numb.saturating_add(1u8);
if numb == 255u8 {
numb = 0;
}
thread::sleep(Duration::new(0, 500_000_000));
}
}
});
}

let grandpa_config = sc_finality_grandpa::Config {
Expand Down