Skip to content

Commit

Permalink
Enable arithmetic_side_effects Clippy lint
Browse files Browse the repository at this point in the history
  • Loading branch information
stanislav-tkach committed Jan 26, 2025
1 parent ffe23e3 commit b0564f4
Show file tree
Hide file tree
Showing 30 changed files with 247 additions and 121 deletions.
3 changes: 2 additions & 1 deletion rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ members = [
"cbork-cddl-parser",
"cbork-utils",
"catalyst-voting",
"catalyst-types",
"catalyst-types",
"immutable-ledger",
"vote-tx-v1",
"vote-tx-v2",
Expand Down Expand Up @@ -60,3 +60,4 @@ string_slice = "deny"
unchecked_duration_subtraction = "deny"
unreachable = "deny"
missing_docs_in_private_items = "deny"
arithmetic_side_effects = "deny"
2 changes: 1 addition & 1 deletion rust/Earthfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
VERSION 0.8

IMPORT github.com/input-output-hk/catalyst-ci/earthly/rust:v3.2.27 AS rust-ci
IMPORT github.com/input-output-hk/catalyst-ci/earthly/rust:v3.2.28 AS rust-ci

COPY_SRC:
FUNCTION
Expand Down
7 changes: 6 additions & 1 deletion rust/c509-certificate/src/attributes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,12 @@ impl Encode<()> for Attributes {
));
}
// The attribute type should be included in array too
encode_array_len(e, "Attributes", self.0.len() as u64 * 2)?;
let Some(len) = (self.0.len() as u64).checked_mul(2) else {
return Err(minicbor::encode::Error::message(
"Attributes length overflow",
));
};
encode_array_len(e, "Attributes", len)?;
for attribute in &self.0 {
attribute.encode(e, ctx)?;
}
Expand Down
6 changes: 5 additions & 1 deletion rust/c509-certificate/src/extensions/extension/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,11 @@ impl Encode<()> for Extension {
{
// Determine encoded OID value based on critical flag
let encoded_oid = if self.critical {
-mapped_oid
mapped_oid.checked_neg().ok_or_else(|| {
minicbor::encode::Error::message(format!(
"Invalid OID value (will overflow during negation): {mapped_oid}"
))
})?
} else {
mapped_oid
};
Expand Down
8 changes: 7 additions & 1 deletion rust/c509-certificate/src/extensions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,13 @@ impl Encode<()> for Extensions {
if self.0.len() == 1 && extension.registered_oid().c509_oid().oid() == &KEY_USAGE_OID {
match extension.value() {
ExtensionValue::Int(value) => {
let ku_value = if extension.critical() { -value } else { *value };
let ku_value = if extension.critical() {
value
.checked_neg()
.ok_or_else(|| minicbor::encode::Error::message(format!("Invalid key usage value (will overflow during negation): {value}")))?
} else {
*value
};
encode_helper(e, "Extensions KeyUsage", ctx, &ku_value)?;
return Ok(());
},
Expand Down
7 changes: 6 additions & 1 deletion rust/c509-certificate/src/general_names/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,12 @@ impl Encode<()> for GeneralNames {
));
}
// The general name type should be included in array too
encode_array_len(e, "General Names", self.0.len() as u64 * 2)?;
let Some(len) = (self.0.len() as u64).checked_mul(2) else {
return Err(minicbor::encode::Error::message(
"General Names length overflow",
));
};
encode_array_len(e, "General Names", len)?;
for gn in &self.0 {
gn.encode(e, ctx)?;
}
Expand Down
14 changes: 12 additions & 2 deletions rust/c509-certificate/src/name/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,24 @@ impl Encode<()> for NameValue {

encode_cn_value(e, cn_value)?;
} else {
encode_array_len(e, "Attributes", attrs.len() as u64 * 2)?;
let Some(len) = (attrs.len() as u64).checked_mul(2) else {
return Err(minicbor::encode::Error::message(
"Attribute length overflow",
));
};
encode_array_len(e, "Attributes", len)?;
for attribute in attrs {
attribute.encode(e, ctx)?;
}
}
} else {
// If is okay if the attributes is empty
encode_array_len(e, "Attributes", attrs.len() as u64 * 2)?;
let Some(len) = (attrs.len() as u64).checked_mul(2) else {
return Err(minicbor::encode::Error::message(
"Attribute length overflow",
));
};
encode_array_len(e, "Attributes", len)?;
for attribute in attrs {
attribute.encode(e, ctx)?;
}
Expand Down
2 changes: 1 addition & 1 deletion rust/cardano-blockchain-types/src/auxdata/scripts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ impl TryFrom<u64> for ScriptType {
match value {
0 => Err(anyhow!("Invalid script type: {}", value)),
1 => Ok(Self::Native),
_ => Ok(Self::Plutus(value - 1)),
_ => Ok(Self::Plutus(value.saturating_sub(1))),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion rust/cardano-blockchain-types/src/fork.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl fmt::Display for Fork {
0 => write!(f, "IMMUTABLE"),
1 => write!(f, "BACKFILL"),
// For live forks: 2 maps to LIVE:1, 3 maps to LIVE:2 etc.
2..=u64::MAX => write!(f, "LIVE:{}", self.0 - 1),
2..=u64::MAX => write!(f, "LIVE:{}", self.0.saturating_sub(1)),
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,11 @@ fn check_is_key_exist(
if found_keys.contains(key) {
err_report.duplicate_field(
format!("{key:?}").as_str(),
format!("Redundant key found in item {} in RBAC map", index + 1).as_str(),
format!(
"Redundant key found in item {} in RBAC map",
index.saturating_add(1)
)
.as_str(),
format!("CIP36 Key Registration {key:?}").as_str(),
);
return true;
Expand Down
10 changes: 6 additions & 4 deletions rust/cardano-blockchain-types/src/multi_era_block_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,8 +487,10 @@ pub(crate) mod tests {
let pallas_block =
pallas::ledger::traverse::MultiEraBlock::decode(test_block.raw.as_slice())?;

let previous_point =
Point::new((pallas_block.slot() - 1).into(), vec![0; 32].try_into()?);
let previous_point = Point::new(
(pallas_block.slot().checked_sub(1).unwrap()).into(),
vec![0; 32].try_into()?,
);

let block = MultiEraBlock::new(
Network::Preprod,
Expand All @@ -511,7 +513,7 @@ pub(crate) mod tests {
pallas::ledger::traverse::MultiEraBlock::decode(test_block.raw.as_slice())?;

let previous_point = Point::new(
(pallas_block.slot() - 1).into(),
(pallas_block.slot().checked_sub(1).unwrap()).into(),
pallas_block
.header()
.previous_hash()
Expand Down Expand Up @@ -540,7 +542,7 @@ pub(crate) mod tests {
let prev_point = pallas::ledger::traverse::MultiEraBlock::decode(block.as_slice())
.map(|block| {
Point::new(
(block.slot() - 1).into(),
(block.slot().saturating_sub(1)).into(),
block
.header()
.previous_hash()
Expand Down
24 changes: 12 additions & 12 deletions rust/cardano-blockchain-types/src/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,23 +190,23 @@ impl Network {

let (elapsed_slots, era_known_slot) = if time < shelley_start_time {
// Byron era
let time_diff = time - byron_start_time;
(
time_diff.num_seconds() / i64::from(genesis.byron_slot_length),
genesis.byron_known_slot,
)
let time_diff = time.signed_duration_since(byron_start_time);
let elapsed_slots = time_diff
.num_seconds()
.checked_div(i64::from(genesis.byron_slot_length))?;
(elapsed_slots, genesis.byron_known_slot)
} else {
// Shelley era
let time_diff = time - shelley_start_time;
(
time_diff.num_seconds() / i64::from(genesis.shelley_slot_length),
genesis.shelley_known_slot,
)
let time_diff = time.signed_duration_since(shelley_start_time);
let elapsed_slots = time_diff
.num_seconds()
.checked_div(i64::from(genesis.shelley_slot_length))?;
(elapsed_slots, genesis.shelley_known_slot)
};

let era_known_slot = i64::try_from(era_known_slot).ok()?;

Some(Slot::from_saturating(elapsed_slots + era_known_slot))
let slot = elapsed_slots.checked_add(era_known_slot)?;
Some(Slot::from_saturating(slot))
}
}

Expand Down
10 changes: 7 additions & 3 deletions rust/cardano-chain-follower/examples/follow_chains.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,11 @@ async fn start_sync_for(network: &Network, matches: ArgMatches) -> Result<(), Bo
let mithril_dl_workers = format!("{}", dl_config.workers);

if let Some(chunk_size) = matches.get_one::<u16>("mithril-sync-chunk-size") {
dl_config = dl_config.with_chunk_size(*chunk_size as usize * 1024 * 1024);
let chunk_size = chunk_size
.checked_mul(1024)
.and_then(|v| v.checked_mul(1024))
.ok_or_else(|| "Chunk size overflow")?;
dl_config = dl_config.with_chunk_size(chunk_size);
}
let mithril_dl_chunk_size = format!("{} MBytes", dl_config.chunk_size / (1024 * 1024));

Expand Down Expand Up @@ -176,7 +180,7 @@ async fn follow_for(network: Network, matches: ArgMatches) {
let mut largest_aux_size: usize = 0;

while let Some(chain_update) = follower.next().await {
updates += 1;
updates = updates.saturating_add(1);

if chain_update.tip {
reached_tip = true;
Expand All @@ -196,7 +200,7 @@ async fn follow_for(network: Network, matches: ArgMatches) {
info!(
chain = network.to_string(),
"Chain Update {}:{}",
updates - 1,
updates.saturating_sub(1),
last_update
);
}
Expand Down
5 changes: 3 additions & 2 deletions rust/cardano-chain-follower/src/chain_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ async fn retry_connect(
match peer {
Ok(peer) => return Ok(peer),
Err(err) => {
retries -= 1;
retries = retries.saturating_sub(1);
if retries == 0 {
return Err(err);
}
Expand All @@ -72,7 +72,7 @@ async fn retry_connect(
}
},
Err(error) => {
retries -= 1;
retries = retries.saturating_sub(1);
if retries == 0 {
return Err(pallas::network::facades::Error::ConnectFailure(
tokio::io::Error::new(
Expand Down Expand Up @@ -193,6 +193,7 @@ async fn process_rollback(
let head_slot = previous_point.slot_or_default();
debug!("Head slot: {head_slot:?}");
debug!("Rollback slot: {rollback_slot:?}");
#[allow(clippy::arithmetic_side_effects)]
let slot_rollback_size = head_slot - rollback_slot;

// We actually do the work here...
Expand Down
12 changes: 7 additions & 5 deletions rust/cardano-chain-follower/src/chain_sync_live_chains.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl ProtectedLiveChainBlockList {
chain.get(point)?
} else if advance < 0 {
// This is a fuzzy lookup backwards.
advance += 1;
advance = advance.saturating_add(1);
chain.upper_bound(Bound::Excluded(point))?
} else {
// This is a fuzzy lookup forwards.
Expand All @@ -89,13 +89,13 @@ impl ProtectedLiveChainBlockList {

// If we are stepping backwards, look backwards.
while advance < 0 {
advance += 1;
advance = advance.saturating_add(1);
this = this.prev()?;
}

// If we are stepping forwards, look forwards.
while advance > 0 {
advance -= 1;
advance = advance.saturating_sub(1);
this = this.next()?;
}

Expand Down Expand Up @@ -234,7 +234,7 @@ impl ProtectedLiveChainBlockList {
// We search backwards because a rollback is more likely in the newest blocks than
// the oldest.
while let Some(popped) = live_chain.pop_back() {
rollback_size += 1;
rollback_size = rollback_size.saturating_add(1);
if previous_point.strict_eq(&popped.value().previous()) {
// We are now contiguous, so stop purging.
break;
Expand Down Expand Up @@ -352,6 +352,8 @@ impl ProtectedLiveChainBlockList {
let mut previous_point = entry.value().point();

// Loop until we exhaust probe slots, OR we would step past genesis.
// It is ok because slot implement saturating subtraction.
#[allow(clippy::arithmetic_side_effects)]
while slot_age < reference_slot {
let ref_point = Point::fuzzy(reference_slot - slot_age);
let Some(entry) = chain.lower_bound(Bound::Included(&ref_point)) else {
Expand Down Expand Up @@ -391,7 +393,7 @@ impl ProtectedLiveChainBlockList {

// Search backwards for a fork smaller than or equal to the one we know.
while this_block.fork() > fork {
rollback_depth += 1;
rollback_depth = rollback_depth.saturating_add(1);
entry = match entry.prev() {
Some(entry) => entry,
None => return None,
Expand Down
2 changes: 1 addition & 1 deletion rust/cardano-chain-follower/src/follow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ mod tests {
.expect("cannot decode block");

let previous_point = Point::new(
(pallas_block.slot() - 1).into(),
(pallas_block.slot().checked_sub(1).unwrap()).into(),
pallas_block
.header()
.previous_hash()
Expand Down
3 changes: 2 additions & 1 deletion rust/cardano-chain-follower/src/mithril_snapshot_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ pub(crate) fn probe_point(point: &Point, distance: u64) -> Point {
const STEP_BACK_ORIGIN: u64 = 0;
// Now that we have the tip, step back about 4 block intervals from tip, and do a fuzzy
// iteration to find the exact two blocks at the end of the immutable chain.
#[allow(clippy::arithmetic_side_effects)]
let step_back_search = point.slot_or_default() - distance.into();

// We stepped back to the origin, so just return Origin
Expand Down Expand Up @@ -143,7 +144,7 @@ impl MithrilSnapshotIterator {
return iterator;
}

backwards_search *= 2;
backwards_search = backwards_search.saturating_mul(2);
}
}

Expand Down
17 changes: 13 additions & 4 deletions rust/cardano-chain-follower/src/mithril_snapshot_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,9 @@ fn calculate_sleep_duration(
let mut next_sleep = MINIMUM_MITHRIL_UPDATE_CHECK_INTERVAL;

// How long between snapshots,
let mut snapshot_interval = (latest_snapshot.created_at - previous_snapshot.created_at)
let mut snapshot_interval = latest_snapshot
.created_at
.signed_duration_since(previous_snapshot.created_at)
.max(MAXIMUM_MITHRIL_UPDATE_CHECK_INTERVAL);

// We should never be negative, but we CAN be zero if there was no chronologically
Expand All @@ -139,11 +141,18 @@ fn calculate_sleep_duration(
snapshot_interval = EXPECTED_MITHRIL_UPDATE_CHECK_INTERVAL;
}

let next_expected_snapshot = latest_snapshot.created_at + snapshot_interval;
let next_expected_snapshot = latest_snapshot
.created_at
.checked_add_signed(snapshot_interval)
.unwrap_or_else(|| {
// Realistically this should never happen.
error!("latest_snapshot.created_at + snapshot_interval overflow");
latest_snapshot.created_at
});

if next_expected_snapshot > now {
// We are behind schedule. Sleep until the next expected snapshot should be published.
next_sleep = next_expected_snapshot - now;
next_sleep = next_expected_snapshot.signed_duration_since(now);
}

next_sleep
Expand Down Expand Up @@ -359,7 +368,7 @@ async fn get_latest_validated_mithril_snapshot(

// IF the mithril data we have is NOT the current latest (or the immediately previous), it
// may as well be invalid.
if latest_mithril < actual_latest.beacon.immutable_file_number - 1 {
if latest_mithril < actual_latest.beacon.immutable_file_number.saturating_sub(1) {
return None;
}

Expand Down
4 changes: 3 additions & 1 deletion rust/cardano-chain-follower/src/mithril_turbo_downloader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,9 @@ impl SnapshotDownloader for MithrilTurboDownloader {
self.inner.cfg.chain,
Some(self.inner.ext_size.load(Ordering::SeqCst)),
self.inner.dedup_size.load(Ordering::SeqCst),
tot_files - (chg_files + new_files),
tot_files
.saturating_sub(chg_files)
.saturating_sub(new_files),
chg_files,
new_files,
);
Expand Down
Loading

0 comments on commit b0564f4

Please sign in to comment.