Skip to content

Commit

Permalink
fix: avoid using floats when generating shares and added PoW validati…
Browse files Browse the repository at this point in the history
…on (#24)

Description
---
When calculated shares floats were used, but since float rounding can be
different across machines and systems, added integers only to avoid
different shares generated.

Motivation and Context
---

How Has This Been Tested?
---
By running the normal mining process through p2pool (using 2 miners and
2 p2pool instances).

What process can a PR reviewer use to test or verify this change?
---
By running the normal mining process through p2pool (using 2 miners and
2 p2pool instances) and everything seems to work as before.

Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->
  • Loading branch information
ksrichard authored Jul 26, 2024
1 parent 787d2be commit 94404b4
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 15 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ jobs:
# on stable Rust.
name: cargo check with stable
runs-on: [ self-hosted, ubuntu-high-cpu ]
env:
RUSTUP_PERMIT_COPY_RENAME: true
steps:
- name: checkout
uses: actions/checkout@v4
Expand Down
35 changes: 20 additions & 15 deletions src/sharechain/in_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use async_trait::async_trait;
use log::{debug, info, warn};
use minotari_app_grpc::tari_rpc::{NewBlockCoinbase, SubmitBlockRequest};
use tari_common_types::tari_address::TariAddress;
use tari_common_types::types::BlockHash;
use tari_core::blocks::BlockHeader;
use tari_core::proof_of_work::sha3x_difficulty;
use tari_utilities::{epoch_time::EpochTime, hex::Hex};
Expand Down Expand Up @@ -46,7 +47,10 @@ impl BlockLevel {
}

fn genesis_block() -> Block {
Block::builder().with_height(0).build()
Block::builder()
.with_height(0)
.with_prev_hash(BlockHash::zero())
.build()
}

impl Default for InMemoryShareChain {
Expand Down Expand Up @@ -112,19 +116,16 @@ impl InMemoryShareChain {
result
}

// TODO: use integers instead of floats
/// Generating number of shares for all the miners.
async fn miners_with_shares(&self) -> HashMap<String, f64> {
let mut result: HashMap<String, f64> = HashMap::new(); // target wallet address -> number of shares
let block_levels = self.block_levels.read().await;
let chain = self.chain(block_levels.iter());
chain.iter().for_each(|block| {
fn miners_with_shares(&self, chain_iter: Iter<'_, Block>) -> HashMap<String, u64> {
let mut result: HashMap<String, u64> = HashMap::new(); // target wallet address -> number of shares
chain_iter.for_each(|block| {
if let Some(miner_wallet_address) = block.miner_wallet_address() {
let addr = miner_wallet_address.to_base58();
if let Some(curr_hash_rate) = result.get(&addr) {
result.insert(addr, curr_hash_rate + 1.0);
result.insert(addr, curr_hash_rate + 1);
} else {
result.insert(addr, 1.0);
result.insert(addr, 1);
}
}
});
Expand Down Expand Up @@ -158,6 +159,13 @@ impl InMemoryShareChain {
return Ok(ValidateBlockResult::new(false, false));
}

// validate PoW
if let Err(error) = sha3x_difficulty(block.original_block_header()) {
warn!(target: LOG_TARGET, "❌ Invalid PoW!");
debug!(target: LOG_TARGET, "Failed to calculate difficulty: {error:?}");
return Ok(ValidateBlockResult::new(false, false));
}

// TODO: check here for miners
// TODO: (send merkle tree root hash and generate here, then compare the two from miners list and shares)
} else {
Expand Down Expand Up @@ -274,19 +282,16 @@ impl ShareChain for InMemoryShareChain {
Ok(last_block.height())
}

// TODO: use integers only instead of floats
#[allow(clippy::cast_possible_truncation)]
async fn generate_shares(&self, reward: u64) -> Vec<NewBlockCoinbase> {
let mut result = vec![];
let miners = self.miners_with_shares().await;
let miners = self.miners_with_shares(self.chain(self.block_levels.read().await.iter()).iter());

// calculate full hash rate and shares
miners
.iter()
.map(|(addr, rate)| (addr, rate / SHARE_COUNT as f64))
.filter(|(_, share)| *share > 0.0)
.filter(|(_, share)| **share > 0)
.for_each(|(addr, share)| {
let curr_reward = ((reward as f64) * share) as u64;
let curr_reward = (reward / SHARE_COUNT) * share;
debug!(target: LOG_TARGET, "{addr} -> SHARE: {share:?}, REWARD: {curr_reward:?}");
result.push(NewBlockCoinbase {
address: addr.clone(),
Expand Down

0 comments on commit 94404b4

Please sign in to comment.