Skip to content

Commit

Permalink
feat: proof of work audit part 2 (#5495)
Browse files Browse the repository at this point in the history
Description
---
- Implemented creation, underflow and overflow protection for
`Difficulty` in `difficulty.rs`.
- Removed all methods not adhering to these patterns/traits.
- Fixed usage in the code to adhere to these.
- Consolidated and refactored repetitive code in `difficulty.rs`.

Motivation and Context
---
Preparation for code audit

How Has This Been Tested?
---
Added unit tests
All unit tests pass

What process can a PR reviewer use to test or verify this change?
---
Code walkthrough
Review the new unit test and related code

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


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
hansieodendaal committed Jun 26, 2023
1 parent 59d1d8e commit af32f96
Show file tree
Hide file tree
Showing 39 changed files with 593 additions and 373 deletions.
8 changes: 4 additions & 4 deletions applications/tari_base_node/src/grpc/hash_rate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,13 @@ mod test {

// we check that the window is not full when we insert less items than the window size
for _ in 0..window_size - 1 {
hash_rate_ma.add(0, Difficulty::from(0));
hash_rate_ma.add(0, Difficulty::min());
assert!(!hash_rate_ma.is_full());
}

// from this point onwards, the window should be always full
for _ in 0..10 {
hash_rate_ma.add(0, Difficulty::from(0));
hash_rate_ma.add(0, Difficulty::min());
assert!(hash_rate_ma.is_full());
}
}
Expand Down Expand Up @@ -175,7 +175,7 @@ mod test {
let window_size = hash_rate_ma.window_size;

for _ in 0..window_size {
hash_rate_ma.add(0, Difficulty::from(u64::MAX));
hash_rate_ma.add(0, Difficulty::max());
}
}

Expand All @@ -193,7 +193,7 @@ mod test {
difficulty: u64,
expected_hash_rate: u64,
) {
moving_average.add(height, Difficulty::from(difficulty));
moving_average.add(height, Difficulty::from_u64(difficulty).unwrap());
assert_eq!(moving_average.average(), expected_hash_rate);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ impl BlockTemplateProtocol<'_> {
);
Ok(FinalBlockTemplateData {
template: block_template_data,
target_difficulty: mining_difficulty.into(),
target_difficulty: Difficulty::from_u64(mining_difficulty)?,
blockhashing_blob,
blocktemplate_blob,
merge_mining_hash: tari_block.merge_mining_hash,
Expand Down
7 changes: 6 additions & 1 deletion applications/tari_merge_mining_proxy/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ use hex::FromHexError;
use hyper::header::InvalidHeaderValue;
// use tari_app_grpc::authentication::BasicAuthError;
use tari_common::{ConfigError, ConfigurationError};
use tari_core::{proof_of_work::monero_rx::MergeMineError, transactions::CoinbaseBuildError};
use tari_core::{
proof_of_work::{monero_rx::MergeMineError, DifficultyError},
transactions::CoinbaseBuildError,
};
use tari_wallet_grpc_client::BasicAuthError;
use thiserror::Error;
use tonic::{codegen::http::uri::InvalidUri, transport};
Expand Down Expand Up @@ -92,6 +95,8 @@ pub enum MmProxyError {
ConversionError(String),
#[error("No reachable servers in configuration")]
ServersUnavailable,
#[error("Invalid difficulty: {0}")]
DifficultyError(#[from] DifficultyError),
}

impl From<tonic::Status> for MmProxyError {
Expand Down
17 changes: 10 additions & 7 deletions applications/tari_miner/src/difficulty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@
use std::convert::TryInto;

use tari_app_grpc::tari_rpc::BlockHeader as grpc_header;
use tari_core::{blocks::BlockHeader, proof_of_work::sha3x_difficulty};
use tari_core::{
blocks::BlockHeader,
proof_of_work::{sha3x_difficulty, DifficultyError},
};
use tari_utilities::epoch_time::EpochTime;

use crate::errors::MinerError;
Expand Down Expand Up @@ -65,9 +68,9 @@ impl BlockHeaderSha3 {
}

#[inline]
pub fn difficulty(&mut self) -> Difficulty {
pub fn difficulty(&mut self) -> Result<Difficulty, DifficultyError> {
self.hashes = self.hashes.saturating_add(1);
sha3x_difficulty(&self.header).into()
Ok(sha3x_difficulty(&self.header)?.as_u64())
}

#[allow(clippy::cast_possible_wrap)]
Expand Down Expand Up @@ -112,8 +115,8 @@ pub mod test {
let mut hasher = BlockHeaderSha3::new(header).unwrap();
for _ in 0..1000 {
assert_eq!(
hasher.difficulty(),
core_sha3_difficulty(&core_header).as_u64(),
hasher.difficulty().unwrap(),
core_sha3_difficulty(&core_header).unwrap().as_u64(),
"with nonces = {}:{}",
hasher.header.nonce,
core_header.nonce
Expand All @@ -132,8 +135,8 @@ pub mod test {
let mut timestamp = core_header.timestamp;
for _ in 0..1000 {
assert_eq!(
hasher.difficulty(),
core_sha3_difficulty(&core_header).as_u64(),
hasher.difficulty().unwrap(),
core_sha3_difficulty(&core_header).unwrap().as_u64(),
"with timestamp = {}",
timestamp
);
Expand Down
19 changes: 17 additions & 2 deletions applications/tari_miner/src/miner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
//
use std::{
panic::panic_any,
pin::Pin,
task::{Context, Poll, Waker},
thread,
Expand Down Expand Up @@ -178,13 +179,27 @@ pub fn mining_task(
share_mode: bool,
) {
let start = Instant::now();
let mut hasher = BlockHeaderSha3::new(header).unwrap();
let mut hasher = match BlockHeaderSha3::new(header) {
Ok(hasher) => hasher,
Err(err) => {
let err = format!("Miner {} failed to create hasher: {:?}", miner, err);
error!(target: LOG_TARGET, "{}", err);
panic_any(err);
},
};
hasher.random_nonce();
// We're mining over here!
trace!(target: LOG_TARGET, "Mining thread {} started", miner);
// Mining work
loop {
let difficulty = hasher.difficulty();
let difficulty = match hasher.difficulty() {
Ok(difficulty) => difficulty,
Err(err) => {
let err = format!("Miner {} failed to calculate difficulty: {:?}", miner, err);
error!(target: LOG_TARGET, "{}", err);
panic_any(err);
},
};
if difficulty >= target_difficulty {
debug!(
target: LOG_TARGET,
Expand Down
12 changes: 9 additions & 3 deletions base_layer/core/src/blocks/accumulated_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use tari_utilities::hex::Hex;

use crate::{
blocks::{error::BlockError, Block, BlockHeader},
proof_of_work::{AchievedTargetDifficulty, Difficulty, PowAlgorithm},
proof_of_work::{difficulty::CheckedAdd, AchievedTargetDifficulty, Difficulty, PowAlgorithm},
transactions::aggregated_body::AggregateBody,
};

Expand Down Expand Up @@ -286,12 +286,18 @@ impl BlockHeaderAccumulatedDataBuilder<'_> {

let (monero_diff, blake_diff) = match achieved_target.pow_algo() {
PowAlgorithm::Monero => (
previous_accum.accumulated_monero_difficulty + achieved_target.achieved(),
previous_accum
.accumulated_monero_difficulty
.checked_add(achieved_target.achieved())
.ok_or(BlockError::DifficultyOverflow)?,
previous_accum.accumulated_sha_difficulty,
),
PowAlgorithm::Sha3 => (
previous_accum.accumulated_monero_difficulty,
previous_accum.accumulated_sha_difficulty + achieved_target.achieved(),
previous_accum
.accumulated_sha_difficulty
.checked_add(achieved_target.achieved())
.ok_or(BlockError::DifficultyOverflow)?,
),
};

Expand Down
2 changes: 2 additions & 0 deletions base_layer/core/src/blocks/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,6 @@ pub enum BlockError {
HistoricalBlockContainsPrunedTxos,
#[error("Chain block invariant error: {0}")]
ChainBlockInvariantError(String),
#[error("Adding difficulties overflowed")]
DifficultyOverflow,
}
34 changes: 17 additions & 17 deletions base_layer/core/src/blocks/genesis_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use tari_crypto::tari_utilities::hex::*;

use crate::{
blocks::{block::Block, BlockHeader, BlockHeaderAccumulatedData, ChainBlock},
proof_of_work::{PowAlgorithm, ProofOfWork},
proof_of_work::{Difficulty, PowAlgorithm, ProofOfWork},
transactions::{aggregated_body::AggregateBody, transaction_components::TransactionOutput},
};

Expand Down Expand Up @@ -120,11 +120,11 @@ pub fn get_stagenet_genesis_block() -> ChainBlock {
let accumulated_data = BlockHeaderAccumulatedData {
hash: block.hash(),
total_kernel_offset: block.header.total_kernel_offset.clone(),
achieved_difficulty: 1.into(),
achieved_difficulty: Difficulty::min(),
total_accumulated_difficulty: 1,
accumulated_monero_difficulty: 1.into(),
accumulated_sha_difficulty: 1.into(),
target_difficulty: 1.into(),
accumulated_monero_difficulty: Difficulty::min(),
accumulated_sha_difficulty: Difficulty::min(),
target_difficulty: Difficulty::min(),
};
ChainBlock::try_construct(Arc::new(block), accumulated_data).unwrap()
}
Expand Down Expand Up @@ -158,11 +158,11 @@ pub fn get_nextnet_genesis_block() -> ChainBlock {
let accumulated_data = BlockHeaderAccumulatedData {
hash: block.hash(),
total_kernel_offset: block.header.total_kernel_offset.clone(),
achieved_difficulty: 1.into(),
achieved_difficulty: Difficulty::min(),
total_accumulated_difficulty: 1,
accumulated_monero_difficulty: 1.into(),
accumulated_sha_difficulty: 1.into(),
target_difficulty: 1.into(),
accumulated_monero_difficulty: Difficulty::min(),
accumulated_sha_difficulty: Difficulty::min(),
target_difficulty: Difficulty::min(),
};
ChainBlock::try_construct(Arc::new(block), accumulated_data).unwrap()
}
Expand Down Expand Up @@ -204,11 +204,11 @@ pub fn get_igor_genesis_block() -> ChainBlock {
let accumulated_data = BlockHeaderAccumulatedData {
hash: block.hash(),
total_kernel_offset: block.header.total_kernel_offset.clone(),
achieved_difficulty: 1.into(),
achieved_difficulty: Difficulty::min(),
total_accumulated_difficulty: 1,
accumulated_monero_difficulty: 1.into(),
accumulated_sha_difficulty: 1.into(),
target_difficulty: 1.into(),
accumulated_monero_difficulty: Difficulty::min(),
accumulated_sha_difficulty: Difficulty::min(),
target_difficulty: Difficulty::min(),
};
ChainBlock::try_construct(Arc::new(block), accumulated_data).unwrap()
}
Expand Down Expand Up @@ -246,11 +246,11 @@ pub fn get_esmeralda_genesis_block() -> ChainBlock {
let accumulated_data = BlockHeaderAccumulatedData {
hash: block.hash(),
total_kernel_offset: block.header.total_kernel_offset.clone(),
achieved_difficulty: 1.into(),
achieved_difficulty: Difficulty::min(),
total_accumulated_difficulty: 1,
accumulated_monero_difficulty: 1.into(),
accumulated_sha_difficulty: 1.into(),
target_difficulty: 1.into(),
accumulated_monero_difficulty: Difficulty::min(),
accumulated_sha_difficulty: Difficulty::min(),
target_difficulty: Difficulty::min(),
};
ChainBlock::try_construct(Arc::new(block), accumulated_data).unwrap()
}
Expand Down
30 changes: 21 additions & 9 deletions base_layer/core/src/chain_storage/blockchain_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2514,7 +2514,7 @@ mod test {
ConsensusConstantsBuilder,
ConsensusManager,
},
proof_of_work::lwma_diff::LWMA_MAX_BLOCK_TIME_RATIO,
proof_of_work::{lwma_diff::LWMA_MAX_BLOCK_TIME_RATIO, Difficulty},
test_helpers::{
blockchain::{
create_chained_blocks,
Expand Down Expand Up @@ -2696,7 +2696,7 @@ mod test {
let (_, main_chain) = create_main_chain(&db, &[("A->GB", 1, 120)]).await;

let fork_root = main_chain.get("A").unwrap().clone();
let (_, orphan_chain) = create_chained_blocks(&[("B2->GB", 2, 120)], fork_root).await;
let (_, orphan_chain) = create_chained_blocks(&[("B2->GB", 1, 120)], fork_root).await;
let mut access = db.db_write_access().unwrap();

let block = orphan_chain.get("B2").unwrap().clone();
Expand All @@ -2710,7 +2710,7 @@ mod test {
.unwrap();
let fork_tip = access.fetch_orphan_chain_tip_by_hash(block.hash()).unwrap().unwrap();
assert_eq!(fork_tip, block.to_chain_header());
assert_eq!(fork_tip.accumulated_data().total_accumulated_difficulty, 4);
assert_eq!(fork_tip.accumulated_data().total_accumulated_difficulty, 3);
let all_tips = access.fetch_all_orphan_chain_tips().unwrap().len();
assert_eq!(all_tips, 1);

Expand All @@ -2730,6 +2730,7 @@ mod test {

mod handle_possible_reorg {
use super::*;
use crate::proof_of_work::Difficulty;

#[tokio::test]
async fn it_links_many_orphan_branches_to_main_chain() {
Expand Down Expand Up @@ -2765,7 +2766,11 @@ mod test {
}

let fork_root = orphan_chain_c.get("6c").unwrap().clone();
let (_, orphan_chain_d) = create_chained_blocks(block_specs!(["7d->GB", difficulty: 10]), fork_root).await;
let (_, orphan_chain_d) = create_chained_blocks(
block_specs!(["7d->GB", difficulty: Difficulty::from_u64(10).unwrap()]),
fork_root,
)
.await;

let block = orphan_chain_d.get("7d").unwrap().clone();
let result = test.handle_possible_reorg(block.to_arc_block()).unwrap();
Expand Down Expand Up @@ -2818,8 +2823,11 @@ mod test {
create_main_chain(&test.db, block_specs!(["1a->GB"], ["2a->1a"], ["3a->2a"], ["4a->3a"])).await;

let fork_root = main_chain.get("1a").unwrap().clone();
let (_, orphan_chain_b) =
create_chained_blocks(block_specs!(["2b->GB", height: 10, difficulty: 10]), fork_root).await;
let (_, orphan_chain_b) = create_chained_blocks(
block_specs!(["2b->GB", height: 10, difficulty: Difficulty::from_u64(10).unwrap()]),
fork_root,
)
.await;

let block = orphan_chain_b.get("2b").unwrap().clone();
let err = test.handle_possible_reorg(block.to_arc_block()).unwrap_err();
Expand All @@ -2829,7 +2837,11 @@ mod test {
#[tokio::test]
async fn it_allows_orphan_blocks_with_any_height() {
let test = TestHarness::setup();
let (_, main_chain) = create_main_chain(&test.db, block_specs!(["1a->GB", difficulty: 2])).await;
let (_, main_chain) = create_main_chain(
&test.db,
block_specs!(["1a->GB", difficulty: Difficulty::from_u64(2).unwrap()]),
)
.await;

let fork_root = main_chain.get("GB").unwrap().clone();
let (_, orphan_chain_b) =
Expand Down Expand Up @@ -3346,8 +3358,8 @@ mod test {
.clear_proof_of_work()
.add_proof_of_work(PowAlgorithm::Sha3, PowAlgorithmConstants {
max_target_time: 120 * LWMA_MAX_BLOCK_TIME_RATIO,
min_difficulty: 1.into(),
max_difficulty: 100.into(),
min_difficulty: Difficulty::min(),
max_difficulty: Difficulty::from_u64(100).expect("valid difficulty"),
target_time: 120,
})
.build(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,12 @@ mod clear_all_pending_headers {
let accum = BlockHeaderAccumulatedData::builder(&prev_accum)
.with_hash(header.hash())
.with_achieved_target_difficulty(
AchievedTargetDifficulty::try_construct(PowAlgorithm::Sha3, 0.into(), 0.into()).unwrap(),
AchievedTargetDifficulty::try_construct(
PowAlgorithm::Sha3,
Difficulty::min(),
Difficulty::min(),
)
.unwrap(),
)
.with_total_kernel_offset(Default::default())
.build()
Expand Down
Loading

0 comments on commit af32f96

Please sign in to comment.