Skip to content

Commit

Permalink
refactor: replace u64 with usize internally
Browse files Browse the repository at this point in the history
Signed-off-by: Marin Veršić <[email protected]>
  • Loading branch information
mversic committed Jun 10, 2024
1 parent 6cf6378 commit 795c868
Show file tree
Hide file tree
Showing 26 changed files with 220 additions and 192 deletions.
9 changes: 7 additions & 2 deletions client/tests/integration/events/pipeline.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use std::thread::{self, JoinHandle};
use std::{
num::NonZeroUsize,
thread::{self, JoinHandle},
};

use eyre::Result;
use iroha::{
Expand Down Expand Up @@ -129,6 +132,8 @@ fn applied_block_must_be_available_in_kura() {
.as_ref()
.expect("Must be some")
.kura()
.get_block_by_height(event.header().height())
.get_block_by_height(
NonZeroUsize::new(event.header().height().try_into().unwrap()).unwrap(),
)
.expect("Block applied event was received earlier");
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ mod isi {

impl From<CustomInstructionExpr> for Custom {
fn from(isi: CustomInstructionExpr) -> Self {
let payload =
JsonString::serialize(&isi).expect("Couldn't serialize custom instruction");
let payload = serde_json::to_value(&isi)
.expect("INTERNAL BUG: Couldn't serialize custom instruction");

Self::new(payload)
}
}
Expand All @@ -44,7 +45,7 @@ mod isi {
type Error = serde_json::Error;

fn try_from(payload: &JsonString) -> serde_json::Result<Self> {
payload.deserialize()
serde_json::from_str::<Self>(payload.as_ref())
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ pub struct MintAssetForAllAccounts {

impl From<CustomInstructionBox> for Custom {
fn from(isi: CustomInstructionBox) -> Self {
let payload = JsonString::serialize(&isi).expect("Couldn't serialize custom instruction");
let payload = serde_json::to_value(&isi)
.expect("INTERNAL BUG: Couldn't serialize custom instruction");

Self::new(payload)
}
}
Expand All @@ -41,6 +43,6 @@ impl TryFrom<&JsonString> for CustomInstructionBox {
type Error = serde_json::Error;

fn try_from(payload: &JsonString) -> serde_json::Result<Self> {
payload.deserialize()
serde_json::from_str::<Self>(payload.as_ref())
}
}
4 changes: 2 additions & 2 deletions client_cli/pytests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ The test model has the following structure:

```shell
CLIENT_CLI_BINARY=/path/to/iroha_client_cli
CLIENT_CLI_CONFIG=/path/to/config.toml
CLIENT_CLI_CONFIG=/path/to/client.toml
TORII_API_PORT_MIN=8080
TORII_API_PORT_MAX=8083
```
Expand Down Expand Up @@ -156,7 +156,7 @@ Tests are configured via environment variables. These variables can be optionall

The variables:

- `CLIENT_CLI_DIR` — Specifies a path to a directory containing the `iroha` binary and its `config.json` configuration file.\
- `CLIENT_CLI_DIR` — Specifies a path to a directory containing the `iroha` binary and its `client.toml` configuration file.\
Set to `/client_cli`, by default.
- `TORII_API_PORT_MIN`/`TORII_API_PORT_MAX` — This pair specifies the range of local ports through which the Iroha 2 peers are deployed. A randomly selected port from the specified range is used for each test.\
Set to `8080` and `8083` respectively, by default.
Expand Down
38 changes: 24 additions & 14 deletions core/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ pub enum BlockValidationError {
/// Mismatch between the actual and expected height of the latest block. Expected: {expected}, actual: {actual}
LatestBlockHeightMismatch {
/// Expected value
expected: u64,
expected: usize,
/// Actual value
actual: u64,
actual: usize,
},
/// Mismatch between the actual and expected hashes of the current block. Expected: {expected:?}, actual: {actual:?}
IncorrectHash {
Expand Down Expand Up @@ -145,30 +145,36 @@ mod pending {
}

fn make_header(
previous_height: u64,
prev_height: usize,
prev_block_hash: Option<HashOf<SignedBlock>>,
view_change_index: u64,
view_change_index: usize,
transactions: &[CommittedTransaction],
) -> BlockHeader {
BlockHeader {
height: previous_height + 1,
previous_block_hash: prev_block_hash,
height: prev_height
.checked_add(1)
.expect("INTERNAL BUG: Blockchain height exceeds usize::MAX")
.try_into()
.expect("INTERNAL BUG: Number of blocks exceeds u64::MAX"),
prev_block_hash,
transactions_hash: transactions
.iter()
.map(|value| value.as_ref().hash())
.collect::<MerkleTree<_>>()
.hash(),
timestamp_ms: SystemTime::now()
.duration_since(SystemTime::UNIX_EPOCH)
.expect("Failed to get the current system time")
.expect("INTERNAL BUG: Failed to get the current system time")
.as_millis()
.try_into()
.expect("Time should fit into u64"),
view_change_index,
.expect("INTERNAL BUG: Time should fit into u64"),
view_change_index: view_change_index
.try_into()
.expect("INTERNAL BUG: Number of view changes exceeds u32::MAX"),
consensus_estimation_ms: DEFAULT_CONSENSUS_ESTIMATION
.as_millis()
.try_into()
.expect("Time should fit into u64"),
.expect("INTERNAL BUG: Time should fit into u64"),
}
}

Expand Down Expand Up @@ -205,7 +211,7 @@ mod pending {
/// Upon executing this method current timestamp is stored in the block header.
pub fn chain(
self,
view_change_index: u64,
view_change_index: usize,
state: &mut StateBlock<'_>,
) -> BlockBuilder<Chained> {
let transactions = Self::categorize_transactions(self.0.transactions, state);
Expand Down Expand Up @@ -273,7 +279,11 @@ mod valid {
state_block: &mut StateBlock<'_>,
) -> WithEvents<Result<ValidBlock, (SignedBlock, BlockValidationError)>> {
let expected_block_height = state_block.height() + 1;
let actual_height = block.header().height;
let actual_height = block
.header()
.height
.try_into()
.expect("INTERNAL BUG: Block height exceeds usize::MAX");

if expected_block_height != actual_height {
return WithEvents::new(Err((
Expand All @@ -286,7 +296,7 @@ mod valid {
}

let expected_prev_block_hash = state_block.latest_block_hash();
let actual_prev_block_hash = block.header().previous_block_hash;
let actual_prev_block_hash = block.header().prev_block_hash;

if expected_prev_block_hash != actual_prev_block_hash {
return WithEvents::new(Err((
Expand Down Expand Up @@ -486,7 +496,7 @@ mod valid {
let mut payload = BlockPayload {
header: BlockHeader {
height: 2,
previous_block_hash: None,
prev_block_hash: None,
transactions_hash: None,
timestamp_ms: 0,
view_change_index: 0,
Expand Down
18 changes: 13 additions & 5 deletions core/src/block_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ impl BlockSynchronizer {

pub mod message {
//! Module containing messages for [`BlockSynchronizer`](super::BlockSynchronizer).
use std::num::NonZeroUsize;

use super::*;

/// Get blocks after some block
Expand Down Expand Up @@ -204,14 +206,20 @@ pub mod message {
error!(?prev_hash, "Block hash not found");
return;
}
Some(height) => height + 1, // It's get blocks *after*, so we add 1.
// It's get blocks *after*, so we add 1.
Some(height) => height
.checked_add(1)
.expect("INTERNAL BUG: Block height exceeds usize::MAX"),
},
None => 1,
None => nonzero_ext::nonzero!(1_usize),
};

let blocks = (start_height..)
.take(1 + block_sync.gossip_max_size.get() as usize)
.map_while(|height| block_sync.kura.get_block_by_height(height))
let blocks = (start_height.get()..)
.take(block_sync.gossip_max_size.get() as usize + 1)
.map_while(|height| {
NonZeroUsize::new(height)
.and_then(|height| block_sync.kura.get_block_by_height(height))
})
.skip_while(|block| Some(block.hash()) == *latest_hash)
.map(|block| (*block).clone())
.collect::<Vec<_>>();
Expand Down
51 changes: 19 additions & 32 deletions core/src/kura.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::{
fmt::Debug,
fs,
io::{BufWriter, Read, Seek, SeekFrom, Write},
num::NonZeroUsize,
path::{Path, PathBuf},
sync::Arc,
};
Expand Down Expand Up @@ -165,7 +166,7 @@ impl Kura {
match block_store.read_block_data(block.start, &mut block_data_buffer) {
Ok(()) => match SignedBlock::decode_all_versioned(&block_data_buffer) {
Ok(decoded_block) => {
if prev_block_hash != decoded_block.header().previous_block_hash {
if prev_block_hash != decoded_block.header().prev_block_hash {
error!("Block has wrong previous block hash. Not reading any blocks beyond this height.");
break;
}
Expand Down Expand Up @@ -273,45 +274,46 @@ impl Kura {
}

/// Get the hash of the block at the provided height.
pub fn get_block_hash(&self, block_height: u64) -> Option<HashOf<SignedBlock>> {
pub fn get_block_hash(&self, block_height: NonZeroUsize) -> Option<HashOf<SignedBlock>> {
let hash_data_guard = self.block_data.lock();
if block_height == 0 || block_height > hash_data_guard.len() as u64 {

let block_height = block_height.get();
if hash_data_guard.len() < block_height {
return None;
}
let index: usize = (block_height - 1)
.try_into()
.expect("block_height fits in 32 bits or we are running on a 64 bit machine");
Some(hash_data_guard[index].0)

let block_index = block_height - 1;
Some(hash_data_guard[block_index].0)
}

/// Search through blocks for the height of the block with the given hash.
pub fn get_block_height_by_hash(&self, hash: &HashOf<SignedBlock>) -> Option<u64> {
pub fn get_block_height_by_hash(&self, hash: &HashOf<SignedBlock>) -> Option<NonZeroUsize> {
self.block_data
.lock()
.iter()
.position(|(block_hash, _block_arc)| block_hash == hash)
.map(|index| index as u64 + 1)
.and_then(|idx| idx.checked_add(1))
.and_then(NonZeroUsize::new)
}

/// Get a reference to block by height, loading it from disk if needed.
// The below lint suggests changing the code into something that does not compile due
// to the borrow checker.
pub fn get_block_by_height(&self, block_height: u64) -> Option<Arc<SignedBlock>> {
pub fn get_block_by_height(&self, block_height: NonZeroUsize) -> Option<Arc<SignedBlock>> {
let mut data_array_guard = self.block_data.lock();
if block_height == 0 || block_height > data_array_guard.len() as u64 {

if data_array_guard.len() < block_height.get() {
return None;
}
let block_number: usize = (block_height - 1)
.try_into()
.expect("Failed to cast to u32.");

if let Some(block_arc) = data_array_guard[block_number].1.as_ref() {
let block_index = block_height.get() - 1;
if let Some(block_arc) = data_array_guard[block_index].1.as_ref() {
return Some(Arc::clone(block_arc));
};

let block_store = self.block_store.lock();
let BlockIndex { start, length } = block_store
.read_block_index(block_number as u64)
.read_block_index(block_index as u64)
.expect("Failed to read block index from disk.");

let mut block_buf =
Expand All @@ -322,25 +324,10 @@ impl Kura {
let block = SignedBlock::decode_all_versioned(&block_buf).expect("Failed to decode block");

let block_arc = Arc::new(block);
data_array_guard[block_number].1 = Some(Arc::clone(&block_arc));
data_array_guard[block_index].1 = Some(Arc::clone(&block_arc));
Some(block_arc)
}

/// Get a reference to block by hash, loading it from disk if needed.
///
/// Internally this function searches linearly for the block's height and
/// then calls `get_block_by_height`. If you know the height of the block,
/// call `get_block_by_height` directly.
pub fn get_block_by_hash(&self, block_hash: &HashOf<SignedBlock>) -> Option<Arc<SignedBlock>> {
let index = self
.block_data
.lock()
.iter()
.position(|(hash, _arc)| hash == block_hash);

index.and_then(|index| self.get_block_by_height(index as u64 + 1))
}

/// Put a block in kura's in memory block store.
pub fn store_block(&self, block: CommittedBlock) {
let block = Arc::new(SignedBlock::from(block));
Expand Down
25 changes: 13 additions & 12 deletions core/src/metrics.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Metrics and status reporting
use std::{sync::Arc, time::SystemTime};
use std::{num::NonZeroUsize, sync::Arc, time::SystemTime};

use eyre::{Result, WrapErr as _};
use iroha_telemetry::metrics::Metrics;
Expand All @@ -23,7 +23,7 @@ pub struct MetricsReporter {
queue: Arc<Queue>,
metrics: Metrics,
/// Latest observed and processed height by metrics reporter
latest_block_height: Arc<Mutex<u64>>,
latest_block_height: Arc<Mutex<usize>>,
}

impl MetricsReporter {
Expand Down Expand Up @@ -53,14 +53,10 @@ impl MetricsReporter {
/// - If either mutex is poisoned
#[allow(clippy::cast_precision_loss)]
pub fn update_metrics(&self) -> Result<()> {
let online_peers_count: u64 = self
.network
.online_peers(
#[allow(clippy::disallowed_types)]
std::collections::HashSet::len,
)
.try_into()
.expect("casting usize to u64");
let online_peers_count: usize = self.network.online_peers(
#[allow(clippy::disallowed_types)]
std::collections::HashSet::len,
);

let state_view = self.state.view();

Expand All @@ -70,7 +66,12 @@ impl MetricsReporter {
{
let mut block_index = start_index;
while block_index < state_view.height() {
let Some(block) = self.kura.get_block_by_height(block_index + 1) else {
let Some(block) = NonZeroUsize::new(
block_index
.checked_add(1)
.expect("INTERNAL BUG: Blockchain height exceeds usize::MAX"),
)
.and_then(|index| self.kura.get_block_by_height(index)) else {
break;
};
block_index += 1;
Expand Down Expand Up @@ -126,7 +127,7 @@ impl MetricsReporter {
)
};

self.metrics.connected_peers.set(online_peers_count);
self.metrics.connected_peers.set(online_peers_count as u64);

self.metrics
.domains
Expand Down
5 changes: 5 additions & 0 deletions core/src/smartcontracts/isi/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ impl LazyQueryOutput<'_> {
/// - sorting
/// - pagination
/// - batching
///
/// # Errors
///
/// - if fetch size is too big
/// - defined pagination parameter for a query that returns singular result
pub fn apply_postprocessing(
self,
filter: &PredicateBox,
Expand Down
Loading

0 comments on commit 795c868

Please sign in to comment.