diff --git a/Cargo.lock b/Cargo.lock index 9bbde61f6a..cac2cd9832 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5686,7 +5686,7 @@ dependencies = [ [[package]] name = "redux" version = "0.1.0" -source = "git+https://github.com/openmina/redux-rs.git?rev=741a01d#741a01df74b84715391651438c1b1ff60d117901" +source = "git+https://github.com/openmina/redux-rs.git?rev=ab14890c#ab14890c68fa478ccfec9d499a76d25f20759619" dependencies = [ "enum_dispatch", "linkme", diff --git a/Cargo.toml b/Cargo.toml index 17198539e4..acaccb5bf5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,6 +35,11 @@ members = [ resolver = "2" +[workspace.lints.clippy] +#unwrap_used = "warn" +arithmetic_side_effects = "warn" +indexing_slicing = "warn" + [workspace.dependencies] mina-p2p-messages = { path = "mina-p2p-messages" } ledger = { path = "ledger", package = "mina-tree" } @@ -48,7 +53,7 @@ poly-commitment = {git = "https://github.com/openmina/proof-systems", rev = "2fd libp2p = { git = "https://github.com/openmina/rust-libp2p", rev = "5c44c7d9", default-features = false } vrf = { path = "vrf" } openmina-node-account = { path = "node/account" } -redux = { git = "https://github.com/openmina/redux-rs.git", rev = "741a01d", features = ["serde"] } +redux = { git = "https://github.com/openmina/redux-rs.git", rev = "ab14890c", features = ["serde"] } serde = "1.0.190" serde_json = "1.0.107" serde_with = { version = "3.7.0", features = ["hex"] } diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 0000000000..c592911cd6 --- /dev/null +++ b/clippy.toml @@ -0,0 +1 @@ +allow-unwrap-in-tests=true \ No newline at end of file diff --git a/node/Cargo.toml b/node/Cargo.toml index e969552580..99ec0ed539 100644 --- a/node/Cargo.toml +++ b/node/Cargo.toml @@ -4,6 +4,9 @@ version = "0.11.0" edition = "2021" license = "Apache-2.0" +[lints] +workspace = true + [dependencies] rand = "0.8.0" serde = "1.0.147" diff --git a/node/build.rs b/node/build.rs index f12d405c19..dc9ae8de89 100644 --- a/node/build.rs +++ b/node/build.rs @@ -126,7 +126,7 @@ fn main() -> Result<(), Box> { visit_dirs(&node_dir, &mut |file| { let mut path = file.path(); - let path_str = path.to_str().unwrap(); + let path_str = path.to_str().expect("path"); if !path_str.ends_with("_actions.rs") && !path_str.ends_with("action.rs") { return; } @@ -156,7 +156,8 @@ fn main() -> Result<(), Box> { if let Some(action_name) = matches.get(2) { let action_name = action_name.as_str().to_owned(); // Without 'Action' suffix - let action_name_base = action_name[..(action_name.len() - 6)].to_string(); + let action_name_base = + action_name[..(action_name.len().saturating_sub(6))].to_string(); let mut variant_lines = vec![]; loop { let Some(line) = lines.next() else { break }; @@ -252,8 +253,7 @@ fn main() -> Result<(), Box> { .map(|(k, v)| { let mut s = "use crate::".to_owned(); if !k.is_empty() { - s += &k.join("::"); - s += "::"; + s.push_str(&format!("{}::", k.join("::"))); } s += &format!("{{{}}};", v.join(", ")); s @@ -268,8 +268,7 @@ fn main() -> Result<(), Box> { if meta.is_inlined() { meta.action_kinds() } else { - // Remove suffix `Action` from action name. - vec![name[..(name.len() - 6)].to_string()] + vec![name[..name.len().saturating_sub(6)].to_string()] } }); let action_kinds = std::iter::once("None".to_owned()) @@ -304,7 +303,7 @@ fn main() -> Result<(), Box> { while let Some(action_name) = queue.pop_front() { let fn_body = match actions.get(dbg!(&action_name)).unwrap() { ActionMeta::Struct => { - let action_kind = &action_name[..(action_name.len() - 6)]; + let action_kind = &action_name[..(action_name.len().saturating_sub(6))]; format!("ActionKind::{action_kind}") } ActionMeta::Enum(variants) => { diff --git a/node/common/src/service/p2p.rs b/node/common/src/service/p2p.rs index 8a1c7cc197..328960253e 100644 --- a/node/common/src/service/p2p.rs +++ b/node/common/src/service/p2p.rs @@ -44,7 +44,7 @@ impl webrtc::P2pServiceWebrtc for NodeService { &mut self, other_pk: &PublicKey, message: &T, - ) -> Result { + ) -> Result> { let rng = &mut self.rng; self.p2p.sec_key.encrypt(other_pk, rng, message) } @@ -53,7 +53,7 @@ impl webrtc::P2pServiceWebrtc for NodeService { &mut self, other_pk: &PublicKey, encrypted: &T::Encrypted, - ) -> Result { + ) -> Result> { self.p2p.sec_key.decrypt(other_pk, encrypted) } diff --git a/node/src/block_producer/block_producer_reducer.rs b/node/src/block_producer/block_producer_reducer.rs index 87ec1a0fa2..f9cca60e45 100644 --- a/node/src/block_producer/block_producer_reducer.rs +++ b/node/src/block_producer/block_producer_reducer.rs @@ -145,9 +145,10 @@ impl BlockProducerEnabled { if let Some(won_slot) = state.current.won_slot() { if let Some(chain) = best_chain.last().map(|best_tip| { if best_tip.global_slot() == won_slot.global_slot() { - // We are producing block which replaces current best tip - // instead of extending it. - best_chain[..(best_chain.len() - 1)].to_vec() + best_chain + .get(..best_chain.len().saturating_sub(1)) + .unwrap_or(&[]) + .to_vec() } else { best_chain.to_vec() } @@ -430,14 +431,24 @@ impl BlockProducerEnabled { epoch_length: v2::UnsignedExtendedUInt32StableV1(1.into()), }; let epoch_count = v2::UnsignedExtendedUInt32StableV1( - (pred_consensus_state.epoch_count.as_u32() + 1).into(), + (pred_consensus_state + .epoch_count + .as_u32() + .checked_add(1) + .expect("overflow")) + .into(), ); (staking_data, next_data, epoch_count) } else { assert_eq!(pred_epoch, next_epoch); let mut next_data = pred_consensus_state.next_epoch_data.clone(); next_data.epoch_length = v2::UnsignedExtendedUInt32StableV1( - (next_data.epoch_length.as_u32() + 1).into(), + (next_data + .epoch_length + .as_u32() + .checked_add(1) + .expect("overflow")) + .into(), ); ( pred_consensus_state.staking_epoch_data.clone(), @@ -475,7 +486,8 @@ impl BlockProducerEnabled { let is_same_global_sub_window = pred_global_sub_window == next_global_sub_window; let are_windows_overlapping = pred_global_sub_window - + constraint_constants().sub_windows_per_window as u32 + .checked_add(constraint_constants().sub_windows_per_window as u32) + .expect("overflow") >= next_global_sub_window; let current_sub_window_densities = pred_sub_window_densities @@ -525,7 +537,7 @@ impl BlockProducerEnabled { 0 }; if incr_window { - density + 1 + density.saturating_add(1) } else { density } @@ -545,7 +557,9 @@ impl BlockProducerEnabled { &global_slot_since_genesis, ); let consensus_state = v2::ConsensusProofOfStakeDataConsensusStateValueStableV2 { - blockchain_length: v2::UnsignedExtendedUInt32StableV1((pred_block.height() + 1).into()), + blockchain_length: v2::UnsignedExtendedUInt32StableV1( + (pred_block.height().checked_add(1).expect("overflow")).into(), + ), epoch_count, min_window_density, sub_window_densities, @@ -592,7 +606,11 @@ impl BlockProducerEnabled { 0 => (pred_block.hash().clone(), List::new()), chain_proof_len => { // TODO(binier): test - let mut iter = chain.iter().rev().take(chain_proof_len + 1).rev(); + let mut iter = chain + .iter() + .rev() + .take(chain_proof_len.saturating_add(1)) + .rev(); if let Some(first_block) = iter.next() { let first_hash = first_block.hash().clone(); let body_hashes = iter diff --git a/node/src/block_producer/block_producer_state.rs b/node/src/block_producer/block_producer_state.rs index 69c7405a03..384d1ad8c2 100644 --- a/node/src/block_producer/block_producer_state.rs +++ b/node/src/block_producer/block_producer_state.rs @@ -302,8 +302,13 @@ impl BlockProducerCurrentState { match self { Self::WonSlot { won_slot, .. } | Self::WonSlotWait { won_slot, .. } => { // Make sure to only producer blocks when in the slot interval - let slot_upper_bound = won_slot.slot_time + slot_interval; - let estimated_produced_time = now + BLOCK_PRODUCTION_ESTIMATE; + let slot_upper_bound = won_slot + .slot_time + .checked_add(slot_interval) + .expect("overflow"); + let estimated_produced_time = now + .checked_add(BLOCK_PRODUCTION_ESTIMATE) + .expect("overflow"); estimated_produced_time >= won_slot.slot_time && now < slot_upper_bound } _ => false, diff --git a/node/src/block_producer/mod.rs b/node/src/block_producer/mod.rs index e0fd494eb1..e9630f9ba2 100644 --- a/node/src/block_producer/mod.rs +++ b/node/src/block_producer/mod.rs @@ -78,7 +78,13 @@ impl BlockProducerWonSlot { fn calculate_slot_time(genesis_timestamp: redux::Timestamp, slot: u32) -> redux::Timestamp { // FIXME: this calculation must use values from the protocol constants, // now it assumes 3 minutes blocks. - genesis_timestamp + (slot as u64) * 3 * 60 * 1_000_000_000_u64 + genesis_timestamp + .checked_add( + (slot as u64) + .checked_mul(3u64.saturating_mul(60).saturating_mul(1_000_000_000_u64)) + .expect("overflow"), + ) + .expect("overflow") } pub fn global_slot(&self) -> u32 { @@ -86,14 +92,16 @@ impl BlockProducerWonSlot { } pub fn epoch(&self) -> u32 { - self.global_slot() / self.global_slot.slots_per_epoch.as_u32() + self.global_slot() + .checked_div(self.global_slot.slots_per_epoch.as_u32()) + .expect("division by 0") } pub fn global_slot_since_genesis( &self, slot_diff: u32, ) -> v2::MinaNumbersGlobalSlotSinceGenesisMStableV1 { - let slot = self.global_slot() + slot_diff; + let slot = self.global_slot().checked_add(slot_diff).expect("overflow"); v2::MinaNumbersGlobalSlotSinceGenesisMStableV1::SinceGenesis(slot.into()) } @@ -105,7 +113,9 @@ impl BlockProducerWonSlot { } pub fn next_slot_time(&self) -> redux::Timestamp { - self.slot_time + 3 * 60 * 1_000_000_000_u64 + self.slot_time + .checked_add(3u64.saturating_mul(60).saturating_mul(1_000_000_000_u64)) + .expect("overflow") } } @@ -145,14 +155,24 @@ impl PartialOrd for BlockProducerWonSlot { } pub fn to_epoch_and_slot(global_slot: &v2::ConsensusGlobalSlotStableV1) -> (u32, u32) { - let epoch = global_slot.slot_number.as_u32() / global_slot.slots_per_epoch.as_u32(); - let slot = global_slot.slot_number.as_u32() % global_slot.slots_per_epoch.as_u32(); + let epoch = global_slot + .slot_number + .as_u32() + .checked_div(global_slot.slots_per_epoch.as_u32()) + .expect("division by 0"); + let slot = global_slot + .slot_number + .as_u32() + .checked_rem(global_slot.slots_per_epoch.as_u32()) + .expect("division by 0"); (epoch, slot) } pub fn next_epoch_first_slot(global_slot: &v2::ConsensusGlobalSlotStableV1) -> u32 { let (epoch, _) = to_epoch_and_slot(global_slot); - (epoch + 1) * global_slot.slots_per_epoch.as_u32() + (epoch.saturating_add(1)) + .checked_mul(global_slot.slots_per_epoch.as_u32()) + .expect("overflow") } // Returns the epoch number and whether it is the last slot of the epoch diff --git a/node/src/block_producer/vrf_evaluator/block_producer_vrf_evaluator_actions.rs b/node/src/block_producer/vrf_evaluator/block_producer_vrf_evaluator_actions.rs index 9a1afb2c6d..b5ed604638 100644 --- a/node/src/block_producer/vrf_evaluator/block_producer_vrf_evaluator_actions.rs +++ b/node/src/block_producer/vrf_evaluator/block_producer_vrf_evaluator_actions.rs @@ -133,7 +133,11 @@ impl redux::EnablingCondition for BlockProducerVrfEvaluatorAction } => state.block_producer.with(false, |this| { if this.vrf_evaluator.is_slot_requested() { if let Some(current_evaluation) = this.vrf_evaluator.current_evaluation() { - current_evaluation.latest_evaluated_slot + 1 == vrf_output.global_slot() + current_evaluation + .latest_evaluated_slot + .checked_add(1) + .expect("overflow") + == vrf_output.global_slot() && current_evaluation.epoch_data.ledger == *staking_ledger_hash } else { false diff --git a/node/src/block_producer/vrf_evaluator/block_producer_vrf_evaluator_reducer.rs b/node/src/block_producer/vrf_evaluator/block_producer_vrf_evaluator_reducer.rs index 93e789260a..880db568c9 100644 --- a/node/src/block_producer/vrf_evaluator/block_producer_vrf_evaluator_reducer.rs +++ b/node/src/block_producer/vrf_evaluator/block_producer_vrf_evaluator_reducer.rs @@ -191,7 +191,8 @@ impl BlockProducerVrfEvaluatorState { best_tip_epoch, root_block_epoch: *root_block_epoch, is_current_epoch_evaluated: state.is_epoch_evaluated(best_tip_epoch), - is_next_epoch_evaluated: state.is_epoch_evaluated(best_tip_epoch + 1), + is_next_epoch_evaluated: state + .is_epoch_evaluated(best_tip_epoch.checked_add(1).expect("overflow")), last_evaluated_epoch: state.last_evaluated_epoch(), staking_epoch_data: staking_epoch_data.clone(), next_epoch_data: next_epoch_data.clone(), @@ -235,7 +236,8 @@ impl BlockProducerVrfEvaluatorState { time: meta.time(), best_tip_epoch: *best_tip_epoch, is_current_epoch_evaluated: state.is_epoch_evaluated(*best_tip_epoch), - is_next_epoch_evaluated: state.is_epoch_evaluated(best_tip_epoch + 1), + is_next_epoch_evaluated: state + .is_epoch_evaluated(best_tip_epoch.checked_add(1).expect("overflow")), best_tip_slot: *best_tip_slot, best_tip_global_slot: *best_tip_global_slot, next_epoch_first_slot: *next_epoch_first_slot, @@ -425,7 +427,10 @@ impl BlockProducerVrfEvaluatorState { } => { let (epoch_number, initial_slot) = match state.epoch_context() { super::EpochContext::Current(_) => (*best_tip_epoch, *current_global_slot), - super::EpochContext::Next(_) => (best_tip_epoch + 1, next_epoch_first_slot - 1), + super::EpochContext::Next(_) => ( + best_tip_epoch.checked_add(1).expect("overflow"), + next_epoch_first_slot.checked_sub(1).expect("underflow"), + ), super::EpochContext::Waiting => todo!(), }; state.status = BlockProducerVrfEvaluatorStatus::InitialSlotSelection { diff --git a/node/src/block_producer/vrf_evaluator/block_producer_vrf_evaluator_state.rs b/node/src/block_producer/vrf_evaluator/block_producer_vrf_evaluator_state.rs index 78c0b34b32..12bf6dd7f9 100644 --- a/node/src/block_producer/vrf_evaluator/block_producer_vrf_evaluator_state.rs +++ b/node/src/block_producer/vrf_evaluator/block_producer_vrf_evaluator_state.rs @@ -46,7 +46,7 @@ impl BlockProducerVrfEvaluatorState { pub fn evaluate_epoch_bounds(global_slot: &u32) -> SlotPositionInEpoch { if global_slot % SLOTS_PER_EPOCH == 0 { SlotPositionInEpoch::Beginning - } else if (global_slot + 1) % SLOTS_PER_EPOCH == 0 { + } else if (global_slot.checked_add(1).expect("overflow")) % SLOTS_PER_EPOCH == 0 { SlotPositionInEpoch::End } else { SlotPositionInEpoch::Within @@ -111,7 +111,7 @@ impl BlockProducerVrfEvaluatorState { if !self.is_epoch_evaluated(best_tip_epoch) { self.epoch_context = EpochContext::Current((*staking_epoch_data).into()) - } else if !self.is_epoch_evaluated(best_tip_epoch + 1) { + } else if !self.is_epoch_evaluated(best_tip_epoch.checked_add(1).expect("overflow")) { if root_block_epoch == best_tip_epoch && is_next_epoch_seed_finalized { self.epoch_context = EpochContext::Next((*next_epoch_data).into()) } else { @@ -284,7 +284,9 @@ impl BlockProducerVrfEvaluatorState { { match self.epoch_context { EpochContext::Current(_) => self.last_evaluated_epoch = Some(*epoch_number), - EpochContext::Next(_) => self.last_evaluated_epoch = Some(epoch_number + 1), + EpochContext::Next(_) => { + self.last_evaluated_epoch = Some(epoch_number.checked_add(1).expect("overflow")) + } EpochContext::Waiting => {} } } @@ -333,7 +335,10 @@ impl BlockProducerVrfEvaluatorState { Some(VrfEvaluatorInput::new( pending_evaluation.epoch_data.seed, pending_evaluation.epoch_data.delegator_table, - pending_evaluation.latest_evaluated_slot + 1, + pending_evaluation + .latest_evaluated_slot + .checked_add(1) + .expect("overflow"), pending_evaluation.epoch_data.total_currency, pending_evaluation.epoch_data.ledger, )) @@ -345,7 +350,7 @@ impl BlockProducerVrfEvaluatorState { pub fn retention_slot(&self, current_epoch_number: &u32) -> u32 { const PAST_EPOCHS_TO_KEEP: u32 = 2; let cutoff_epoch = current_epoch_number.saturating_sub(PAST_EPOCHS_TO_KEEP); - (cutoff_epoch * SLOTS_PER_EPOCH).saturating_sub(1) + (cutoff_epoch.saturating_mul(SLOTS_PER_EPOCH)).saturating_sub(1) } pub fn cleanup_old_won_slots(&mut self, current_epoch_number: &u32) { diff --git a/node/src/ledger/ledger_service.rs b/node/src/ledger/ledger_service.rs index 7e9846e4fb..89366f6d88 100644 --- a/node/src/ledger/ledger_service.rs +++ b/node/src/ledger/ledger_service.rs @@ -856,7 +856,7 @@ impl LedgerCtx { let num_accounts = mask.num_accounts() as u64; let first_node_addr = ledger::Address::first( - LEDGER_DEPTH - super::tree_height_for_num_accounts(num_accounts), + LEDGER_DEPTH.saturating_sub(super::tree_height_for_num_accounts(num_accounts)), ); let hash = LedgerHash::from_fp(mask.get_hash(first_node_addr)?); Some((num_accounts, hash)) diff --git a/node/src/ledger/mod.rs b/node/src/ledger/mod.rs index c6ccf614eb..137fc97c98 100644 --- a/node/src/ledger/mod.rs +++ b/node/src/ledger/mod.rs @@ -40,7 +40,7 @@ lazy_static::lazy_static! { use ledger::TreeVersion; std::array::from_fn(|i| { - let hash = ledger::V2::empty_hash_at_height(LEDGER_DEPTH - i); + let hash = ledger::V2::empty_hash_at_height(LEDGER_DEPTH.saturating_sub(i)); v2::MinaBaseLedgerHash0StableV1(hash.into()).into() }) }; @@ -61,7 +61,7 @@ pub fn complete_height_tree_with_empties( let content_hash = content_hash.0.to_field()?; let computed_hash = (subtree_height..LEDGER_DEPTH).fold(content_hash, |prev_hash, height| { - let depth = LEDGER_DEPTH - height; + let depth = LEDGER_DEPTH.saturating_sub(height); let empty_right = ledger_empty_hash_at_depth(depth).0.to_field().unwrap(); // We know empties are valid ledger::V2::hash_node(height, prev_hash, empty_right) }); @@ -109,7 +109,7 @@ pub fn hash_node_at_depth( left: mina_hasher::Fp, right: mina_hasher::Fp, ) -> mina_hasher::Fp { - let height = LEDGER_DEPTH - depth - 1; + let height = LEDGER_DEPTH.saturating_sub(depth).saturating_sub(1); ledger::V2::hash_node(height, left, right) } diff --git a/node/src/recorder/recorder.rs b/node/src/recorder/recorder.rs index eacd8560c3..081c88b011 100644 --- a/node/src/recorder/recorder.rs +++ b/node/src/recorder/recorder.rs @@ -38,7 +38,7 @@ impl Recorder { actions_files.push(Some(file)); Self::OnlyInputActions { - recorder_i: actions_files.len() - 1, + recorder_i: actions_files.len().saturating_sub(1), recorder_path: path, actions_f_bytes_written: 0, actions_f_index, @@ -90,12 +90,14 @@ impl Recorder { }; let mut files = ACTIONS_F.try_lock().unwrap(); - let cur_f = &mut files[*recorder_i]; + let cur_f = files.get_mut(*recorder_i).unwrap(); // TODO: error propagation let file = if *actions_f_bytes_written > 64 * 1024 * 1024 { cur_f.take().unwrap().sync_all().unwrap(); *actions_f_bytes_written = 0; - *actions_f_index += 1; + *actions_f_index = actions_f_index + .checked_add(1) + .expect("overflow in actions_f_index"); cur_f.insert( fs::File::create(super::actions_path(recorder_path, *actions_f_index)) .unwrap(), @@ -113,7 +115,12 @@ impl Recorder { writer.write_all(&encoded).unwrap(); writer.flush().unwrap(); - *actions_f_bytes_written += 8 + encoded.len() as u64; + *actions_f_bytes_written = actions_f_bytes_written + .checked_add( + 8u64.checked_add(encoded.len() as u64) + .expect("overflow in encoded len"), + ) + .expect("overflow in actions_f_bytes_written"); } } } diff --git a/node/src/rpc/mod.rs b/node/src/rpc/mod.rs index 8d70d1a9b7..cbd511863e 100644 --- a/node/src/rpc/mod.rs +++ b/node/src/rpc/mod.rs @@ -653,7 +653,7 @@ pub mod discovery { peer_id: value.peer_id, libp2p: value.peer_id.try_into()?, key: value.key, - dist: this_key - value.key, + dist: this_key.distance(&value.key), addrs: value.addresses().clone(), connection: value.connection, }) diff --git a/node/src/rpc_effectful/rpc_effectful_effects.rs b/node/src/rpc_effectful/rpc_effectful_effects.rs index c1419215fb..9fe4058a7d 100644 --- a/node/src/rpc_effectful/rpc_effectful_effects.rs +++ b/node/src/rpc_effectful/rpc_effectful_effects.rs @@ -79,8 +79,10 @@ pub fn rpc_effects(store: &mut Store, action: ActionWithMeta(store: &mut Store, action: ActionWithMeta(store: &mut Store, action: ActionWithMeta Option { @@ -194,7 +194,7 @@ impl SnarkPoolState { pub fn available_jobs_with_highest_priority(&self, n: usize) -> Vec<&JobState> { // find `n` jobs with lowest order (highest priority). self.available_jobs_iter() - .fold(Vec::with_capacity(n + 1), |mut jobs, job| { + .fold(Vec::with_capacity(n.saturating_add(1)), |mut jobs, job| { jobs.push(job); if jobs.len() > n { jobs.sort_by_key(|job| job.order); @@ -267,7 +267,9 @@ impl JobState { }; let account_updates = match &self.job { OneOrTwo::One(job) => account_updates(job), - OneOrTwo::Two((job1, job2)) => account_updates(job1) + account_updates(job2), + OneOrTwo::Two((job1, job2)) => account_updates(job1) + .checked_add(account_updates(job2)) + .expect("overflow"), }; if matches!( @@ -292,7 +294,7 @@ impl JobSummary { const MAX_LATENCY: Duration = Duration::from_secs(10); let (JobSummary::Tx(n) | JobSummary::Merge(n)) = self; - BASE * (*n as u32) + MAX_LATENCY + BASE.saturating_mul(*n as u32).saturating_add(MAX_LATENCY) } } diff --git a/node/src/state.rs b/node/src/state.rs index a0b4735aed..9b5049af1b 100644 --- a/node/src/state.rs +++ b/node/src/state.rs @@ -280,7 +280,7 @@ impl State { /// and only there! pub fn action_applied(&mut self, action: &ActionWithMeta) { self.last_action = action.meta().clone(); - self.applied_actions_count += 1; + self.applied_actions_count = self.applied_actions_count.checked_add(1).expect("overflow"); } pub fn genesis_block(&self) -> Option { @@ -294,8 +294,14 @@ impl State { let initial_ms = u64::from(genesis.timestamp()) / 1_000_000; let now_ms = u64::from(self.time()) / 1_000_000; let ms = now_ms.saturating_sub(initial_ms); - let slots = ms / constraint_constants().block_window_duration_ms; - Some(initial_slot(&genesis) + slots as u32) + let slots = ms + .checked_div(constraint_constants().block_window_duration_ms) + .expect("division by 0"); + Some( + initial_slot(&genesis) + .checked_add(slots as u32) + .expect("overflow"), + ) } /// Current global slot based on constants and current time. @@ -307,7 +313,11 @@ impl State { pub fn current_slot(&self) -> Option { let slots_per_epoch = self.genesis_block()?.constants().slots_per_epoch.as_u32(); - Some(self.cur_global_slot()? % slots_per_epoch) + Some( + self.cur_global_slot()? + .checked_rem(slots_per_epoch) + .expect("division by 0"), + ) } pub fn cur_global_slot_since_genesis(&self) -> Option { @@ -316,14 +326,22 @@ impl State { pub fn current_epoch(&self) -> Option { let slots_per_epoch = self.genesis_block()?.constants().slots_per_epoch.as_u32(); - Some(self.cur_global_slot()? / slots_per_epoch) + Some( + self.cur_global_slot()? + .checked_div(slots_per_epoch) + .expect("division by 0"), + ) } pub fn should_produce_blocks_after_genesis(&self) -> bool { self.block_producer.is_enabled() && self.genesis_block().map_or(false, |b| { let slot = &b.consensus_state().curr_global_slot_since_hard_fork; - let epoch = slot.slot_number.as_u32() / slot.slots_per_epoch.as_u32(); + let epoch = slot + .slot_number + .as_u32() + .checked_div(slot.slots_per_epoch.as_u32()) + .expect("division by 0"); self.current_epoch() <= Some(epoch) }) } diff --git a/node/src/stats/stats_actions.rs b/node/src/stats/stats_actions.rs index 3c4d3b82a4..6a81a70088 100644 --- a/node/src/stats/stats_actions.rs +++ b/node/src/stats/stats_actions.rs @@ -21,7 +21,10 @@ impl ActionStats { while self.per_block.len() >= 20000 { self.per_block.pop_back(); } - let id = self.per_block.back().map_or(0, |v| v.id + 1); + let id = self + .per_block + .back() + .map_or(0, |v| v.id.checked_add(1).expect("overflow")); self.per_block.push_back(ActionStatsForBlock { id, time, @@ -54,7 +57,7 @@ impl ActionStats { }; let i = id .checked_add(blocks.len() as u64)? - .checked_sub(last.id + 1)?; + .checked_sub(last.id.checked_add(1)?)?; blocks.get(i as usize).cloned() } } @@ -69,16 +72,22 @@ impl ActionStatsSnapshot { } let kind_i = *prev_action.action() as usize; - let duration = action.meta().time_as_nanos() - prev_action.meta().time_as_nanos(); + let duration = action + .meta() + .time_as_nanos() + .saturating_sub(prev_action.meta().time_as_nanos()); // TODO(binier): add constant len in ActionKind instead and use // that for constant vec length. let len = self.0.len(); - let need_len = kind_i + 1; + let need_len = kind_i.checked_add(1).expect("overflow"); if len < need_len { self.0.resize(need_len, Default::default()); } - self.0[kind_i].add(duration); + self.0 + .get_mut(kind_i) + .expect("kind_i out of bounds") + .add(duration); } } @@ -128,11 +137,16 @@ pub struct ActionStatsForBlock { impl ActionStatsForBlock { fn new_action(&mut self, action: &ActionKindWithMeta, prev_action: &ActionKindWithMeta) { - let duration = action.meta().time_as_nanos() - prev_action.meta().time_as_nanos(); + let duration = action + .meta() + .time_as_nanos() + .saturating_sub(prev_action.meta().time_as_nanos()); match prev_action.action() { ActionKind::None => {} - ActionKind::EventSourceWaitForEvents => self.cpu_idle += duration, - _ => self.cpu_busy += duration, + ActionKind::EventSourceWaitForEvents => { + self.cpu_idle = self.cpu_idle.saturating_add(duration) + } + _ => self.cpu_busy = self.cpu_busy.saturating_add(duration), } self.stats.add(action, prev_action); } @@ -182,8 +196,8 @@ impl ActionStatsForRanges { } else { &mut self.above_50_ms }; - stats.total_calls += 1; - stats.total_duration += duration; + stats.total_calls = stats.total_calls.saturating_add(1); + stats.total_duration = stats.total_duration.saturating_add(duration); stats.max_duration = std::cmp::max(stats.max_duration, duration); } } diff --git a/node/src/stats/stats_block_producer.rs b/node/src/stats/stats_block_producer.rs index 2a413ea22e..10cd97ed7c 100644 --- a/node/src/stats/stats_block_producer.rs +++ b/node/src/stats/stats_block_producer.rs @@ -361,13 +361,15 @@ impl From<&BlockWithoutProof> for ProducedBlockTransactions { match &cmd.data { v2::MinaBaseUserCommandStableV2::SignedCommand(v) => match &v.payload.body { v2::MinaBaseSignedCommandPayloadBodyStableV2::Payment(_) => { - res.payments += 1 + res.payments = res.payments.checked_add(1).expect("overflow") } v2::MinaBaseSignedCommandPayloadBodyStableV2::StakeDelegation(_) => { - res.delegations += 1 + res.delegations = res.delegations.checked_add(1).expect("overflow") } }, - v2::MinaBaseUserCommandStableV2::ZkappCommand(_) => res.zkapps += 1, + v2::MinaBaseUserCommandStableV2::ZkappCommand(_) => { + res.zkapps = res.zkapps.checked_add(1).expect("overflow") + } } res }) diff --git a/node/src/stats/stats_sync.rs b/node/src/stats/stats_sync.rs index 596a1fdd57..04a323288c 100644 --- a/node/src/stats/stats_sync.rs +++ b/node/src/stats/stats_sync.rs @@ -309,12 +309,23 @@ impl SyncStats { // }) .enumerate() .map(|(i, s)| { - let height = best_tip_height - i as u32; + let height = best_tip_height.checked_sub(i as u32).expect("underflow"); let hash = s.block_hash().clone(); let pred_hash = s .block() .map(|b| b.pred_hash()) - .unwrap_or_else(|| states[states.len() - i - 2].block_hash()) + .unwrap_or_else(|| { + states + .get( + states + .len() + .saturating_sub(i) + .checked_sub(2) + .expect("underflow"), + ) + .unwrap() // can't fail because index is less than states.len() + .block_hash() + }) .clone(); let mut stats = SyncBlock::new(height, hash, pred_hash); stats.update_with_block_state(s); diff --git a/node/src/transition_frontier/genesis/transition_frontier_genesis_config.rs b/node/src/transition_frontier/genesis/transition_frontier_genesis_config.rs index 238b6425fb..4ae9444824 100644 --- a/node/src/transition_frontier/genesis/transition_frontier_genesis_config.rs +++ b/node/src/transition_frontier/genesis/transition_frontier_genesis_config.rs @@ -78,7 +78,7 @@ pub struct GenesisConfigLoaded { } fn bp_num_delegators(i: usize) -> usize { - (i + 1) * 2 + (i.saturating_add(1)).checked_mul(2).expect("overflow") } #[derive(Debug, thiserror::Error)] @@ -146,7 +146,8 @@ impl GenesisConfig { constants, } => { let (whales, fish) = (*whales, *fish); - let delegator_balance = |balance: u64| move |i| balance / i as u64; + let delegator_balance = + |balance: u64| move |i| balance.checked_div(i as u64).expect("division by 0"); let whales = (0..whales).map(|i| { let balance = 8333_u64; let delegators = (1..=bp_num_delegators(i)).map(delegator_balance(50_000_000)); @@ -423,8 +424,8 @@ impl GenesisConfig { let mut account = ledger::Account::create_with(account_id, Balance::from_mina(balance).unwrap()); account.delegate = delegate; - *total_balance += balance; - *counter += 1; + *total_balance = total_balance.checked_add(balance).expect("overflow"); + *counter = counter.checked_add(1).expect("overflow"); // println!("Created account with balance: {}, total_balance: {}", balance, *total_balance); // Debug print account } @@ -458,12 +459,10 @@ impl GenesisConfig { NonStakers::Count(count) => *std::cmp::min(count, &remaining_accounts), }; - let non_staker_total = total_balance * 20 / 80; - let non_staker_balance = if non_staker_count > 0 { - non_staker_total / non_staker_count as u64 - } else { - 0 - }; + let non_staker_total = total_balance.checked_mul(20).expect("overflow") / 80; + let non_staker_balance = non_staker_total + .checked_div(non_staker_count as u64) + .unwrap_or(0); println!("Non staker total balance: {}", non_staker_total); @@ -486,10 +485,12 @@ impl GenesisConfig { let mask = ledger::Mask::new_root(db); let (mask, total_currency) = accounts.into_iter().try_fold( (mask, 0), - |(mut mask, mut total_currency), account| { + |(mut mask, mut total_currency): (ledger::Mask, u64), account| { let account = account?; let account_id = account.id(); - total_currency += account.balance.as_u64(); + total_currency = total_currency + .checked_add(account.balance.as_u64()) + .expect("overflow"); mask.get_or_create_account(account_id, account).unwrap(); Ok((mask, total_currency)) }, diff --git a/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_reducer.rs b/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_reducer.rs index 0a8bcd0951..bfa153b397 100644 --- a/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_reducer.rs +++ b/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_reducer.rs @@ -316,7 +316,7 @@ impl TransitionFrontierSyncLedgerSnarkedState { // We know at which node to begin querying, so we skip all the intermediary depths let first_node_address = ledger::Address::first( - LEDGER_DEPTH - tree_height_for_num_accounts(*num_accounts), + LEDGER_DEPTH.saturating_sub(tree_height_for_num_accounts(*num_accounts)), ); let expected_hash = contents_hash.clone(); let first_query = (first_node_address, expected_hash); @@ -528,8 +528,16 @@ impl TransitionFrontierSyncLedgerSnarkedState { // from that subtree will be skipped and add them to the count. // Empty node hashes are not counted in the stats. - let empty = ledger_empty_hash_at_depth(address.length() + 1); - *num_hashes_accepted += (*left != empty) as u64 + (*right != empty) as u64; + let empty = + ledger_empty_hash_at_depth(address.length().checked_add(1).expect("overflow")); + + if *left != empty { + *num_hashes_accepted = num_hashes_accepted.checked_add(1).expect("overflow") + } + + if *right != empty { + *num_hashes_accepted = num_hashes_accepted.checked_add(1).expect("overflow") + } if left != previous_left { let previous = queue.insert(address.child_left(), left.clone()); @@ -572,7 +580,8 @@ impl TransitionFrontierSyncLedgerSnarkedState { return; }; - *synced_accounts_count += count; + *synced_accounts_count = + synced_accounts_count.checked_add(*count).expect("overflow"); pending.remove(address); // Dispatch diff --git a/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_state.rs b/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_state.rs index 557e6e2faa..ea85c186be 100644 --- a/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_state.rs +++ b/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_state.rs @@ -229,10 +229,12 @@ impl TransitionFrontierSyncLedgerSnarkedState { // would be more accurate (accounts are fetched in groups of 64, hashes of 2). let tree_height = tree_height_for_num_accounts(*total_accounts_expected); let fill_ratio = (*total_accounts_expected as f64) / 2f64.powf(tree_height as f64); - let num_hashes_estimate = 2u64.pow((tree_height - ACCOUNT_SUBTREE_HEIGHT) as u32); + let num_hashes_estimate = 2u64 + .saturating_pow((tree_height.saturating_sub(ACCOUNT_SUBTREE_HEIGHT)) as u32); let num_hashes_estimate = (num_hashes_estimate as f64 * fill_ratio).ceil() as u64; - let fetched = *synced_accounts_count + synced_hashes_count; - let estimation = fetched.max(*total_accounts_expected + num_hashes_estimate); + let fetched = synced_accounts_count.saturating_add(*synced_hashes_count); + let estimation = + fetched.max(total_accounts_expected.saturating_add(num_hashes_estimate)); Some(LedgerSyncProgress { fetched, diff --git a/node/src/transition_frontier/sync/transition_frontier_sync_reducer.rs b/node/src/transition_frontier/sync/transition_frontier_sync_reducer.rs index 029a2b7667..7f8f742b65 100644 --- a/node/src/transition_frontier/sync/transition_frontier_sync_reducer.rs +++ b/node/src/transition_frontier/sync/transition_frontier_sync_reducer.rs @@ -439,7 +439,7 @@ impl TransitionFrontierSyncState { best_chain.iter().map(|b| (b.hash(), b)).collect(); let k = best_tip.constants().k.as_u32() as usize; - let mut chain = Vec::with_capacity(k + root_block_updates.len()); + let mut chain = Vec::with_capacity(k.saturating_add(root_block_updates.len())); // TODO(binier): maybe time should be when we originally // applied this block? Same for below. @@ -654,7 +654,7 @@ impl TransitionFrontierSyncState { return; }; let mut needed_protocol_states = std::mem::take(needed_protocol_states); - let start_i = chain.len().saturating_sub(k + 1); + let start_i = chain.len().saturating_sub(k.saturating_add(1)); let mut iter = std::mem::take(chain) .into_iter() .filter_map(|v| v.take_applied_block()); diff --git a/node/src/transition_frontier/transition_frontier_effects.rs b/node/src/transition_frontier/transition_frontier_effects.rs index 07428a03ae..9597d639f9 100644 --- a/node/src/transition_frontier/transition_frontier_effects.rs +++ b/node/src/transition_frontier/transition_frontier_effects.rs @@ -231,7 +231,9 @@ pub fn transition_frontier_effects( best_tip.height().saturating_sub(b1.height()) as usize; if height_diff == 0 { best_tip.hash() != b1.hash() - } else if let Some(index) = chain.len().checked_sub(height_diff + 1) { + } else if let Some(index) = + chain.len().checked_sub(height_diff.saturating_add(1)) + { chain.get(index).map_or(true, |b2| b1.hash() != b2.hash()) } else { true diff --git a/node/src/transition_frontier/transition_frontier_reducer.rs b/node/src/transition_frontier/transition_frontier_reducer.rs index 284846ff3e..420a447c83 100644 --- a/node/src/transition_frontier/transition_frontier_reducer.rs +++ b/node/src/transition_frontier/transition_frontier_reducer.rs @@ -86,7 +86,10 @@ impl TransitionFrontierState { // into transition frontier anymore due to consensus // reasons. let tip = new_chain.last().unwrap(); - *height + tip.constants().k.as_u32() > tip.height() + height + .checked_add(tip.constants().k.as_u32()) + .expect("overflow") + > tip.height() }); state.chain_diff = state.maybe_make_chain_diff(&new_chain); state.best_chain = new_chain; diff --git a/node/src/transition_frontier/transition_frontier_state.rs b/node/src/transition_frontier/transition_frontier_state.rs index 69811b0c0d..9fdde1e3b5 100644 --- a/node/src/transition_frontier/transition_frontier_state.rs +++ b/node/src/transition_frontier/transition_frontier_state.rs @@ -6,6 +6,7 @@ use mina_p2p_messages::v2::{ TransactionHash, }; use openmina_core::block::{AppliedBlock, ArcBlockWithHash}; +use openmina_core::bug_condition; use serde::{Deserialize, Serialize}; use super::genesis::TransitionFrontierGenesisState; @@ -128,22 +129,39 @@ impl TransitionFrontierState { } Some((new_chain_start_at, _)) => { // `new_chain_start_at` is the index of `new_root` in `old_chain` - let old_chain_advanced = &old_chain[new_chain_start_at..]; + let Some(old_chain_advanced) = &old_chain.get(new_chain_start_at..) else { + bug_condition!("old_chain[{}] out of bounds", new_chain_start_at); + return None; + }; // Common length let len = old_chain_advanced.len().min(new_chain.len()); // Find the first different block, search from the end - let diff_start_at = old_chain_advanced[..len] + let diff_start_at = old_chain_advanced + .get(..len) + .unwrap() // can't fail because len is the minimum len .iter() .rev() - .zip(new_chain[..len].iter().rev()) + .zip( + new_chain + .get(..len) + .unwrap() // can't fail because len is the minimum len + .iter() + .rev(), + ) .position(|(old_block, new_block)| old_block == new_block) - .map(|index| len - index) // we started from the end + .map(|index| len.saturating_sub(index)) // we started from the end .unwrap(); // Never panics because we know there is the common root block - let diff_old_chain = &old_chain_advanced[diff_start_at..]; - let diff_new_chain = &new_chain[diff_start_at..]; + let Some(diff_old_chain) = old_chain_advanced.get(diff_start_at..) else { + bug_condition!("old_chain[{}] out of bounds", diff_start_at); + return None; + }; + let Some(diff_new_chain) = new_chain.get(diff_start_at..) else { + bug_condition!("new_chain[{}] out of bounds", diff_start_at); + return None; + }; (diff_old_chain, diff_new_chain) } diff --git a/node/testing/src/service/mod.rs b/node/testing/src/service/mod.rs index e5cae724ce..d8597880dc 100644 --- a/node/testing/src/service/mod.rs +++ b/node/testing/src/service/mod.rs @@ -364,7 +364,7 @@ impl P2pServiceWebrtc for NodeTestingService { &mut self, other_pk: &node::p2p::identity::PublicKey, message: &T, - ) -> Result { + ) -> Result> { self.real.encrypt(other_pk, message) } @@ -372,7 +372,7 @@ impl P2pServiceWebrtc for NodeTestingService { &mut self, other_pub_key: &node::p2p::identity::PublicKey, encrypted: &T::Encrypted, - ) -> Result { + ) -> Result> { self.real.decrypt(other_pub_key, encrypted) } diff --git a/p2p/src/channels/p2p_channels_service.rs b/p2p/src/channels/p2p_channels_service.rs index 38a6eae8d7..faf4be6663 100644 --- a/p2p/src/channels/p2p_channels_service.rs +++ b/p2p/src/channels/p2p_channels_service.rs @@ -12,10 +12,10 @@ pub trait P2pChannelsService: redux::Service { &mut self, other_pk: &PublicKey, message: &T, - ) -> Result; + ) -> Result>; fn decrypt( &mut self, other_pk: &PublicKey, encrypted: &T::Encrypted, - ) -> Result; + ) -> Result>; } diff --git a/p2p/src/channels/signaling/discovery_effectful/p2p_channels_signaling_discovery_effectful_effects.rs b/p2p/src/channels/signaling/discovery_effectful/p2p_channels_signaling_discovery_effectful_effects.rs index b95e028cfd..9580ec90b4 100644 --- a/p2p/src/channels/signaling/discovery_effectful/p2p_channels_signaling_discovery_effectful_effects.rs +++ b/p2p/src/channels/signaling/discovery_effectful/p2p_channels_signaling_discovery_effectful_effects.rs @@ -32,7 +32,7 @@ impl P2pChannelsSignalingDiscoveryEffectfulAction { pub_key, offer, } => match store.service().encrypt(&pub_key, &*offer) { - Err(()) => { + Err(_) => { // todo!("Failed to encrypt webrtc offer. Handle it.") } Ok(offer) => { @@ -49,7 +49,7 @@ impl P2pChannelsSignalingDiscoveryEffectfulAction { .service() .decrypt::(&pub_key, &answer) { - Err(()) => { + Err(_) => { store.dispatch(P2pChannelsSignalingDiscoveryAction::AnswerDecrypted { peer_id, answer: P2pConnectionResponse::SignalDecryptionFailed, diff --git a/p2p/src/channels/signaling/exchange_effectful/p2p_channels_signaling_exchange_effectful_effects.rs b/p2p/src/channels/signaling/exchange_effectful/p2p_channels_signaling_exchange_effectful_effects.rs index d7fa962bc4..58116b1fcc 100644 --- a/p2p/src/channels/signaling/exchange_effectful/p2p_channels_signaling_exchange_effectful_effects.rs +++ b/p2p/src/channels/signaling/exchange_effectful/p2p_channels_signaling_exchange_effectful_effects.rs @@ -34,7 +34,7 @@ impl P2pChannelsSignalingExchangeEffectfulAction { offer, } => { match store.service().decrypt::(&pub_key, &offer) { - Err(()) => { + Err(_) => { store.dispatch(P2pChannelsSignalingExchangeAction::OfferDecryptError { peer_id, }); @@ -67,7 +67,7 @@ impl P2pChannelsSignalingExchangeEffectfulAction { Some(v) => v, }; match store.service().encrypt(&pub_key, &answer) { - Err(()) => bug_condition!("Failed to encrypt webrtc answer. Shouldn't happen since we managed to decrypt sent offer."), + Err(_) => bug_condition!("Failed to encrypt webrtc answer. Shouldn't happen since we managed to decrypt sent offer."), Ok(answer) => { answer_message_send(store.service(), peer_id, Some(answer)); } diff --git a/p2p/src/identity/secret_key.rs b/p2p/src/identity/secret_key.rs index 5237391cc6..30254371c1 100644 --- a/p2p/src/identity/secret_key.rs +++ b/p2p/src/identity/secret_key.rs @@ -56,11 +56,24 @@ use aes_gcm::{ aead::{Aead, AeadCore}, Aes256Gcm, KeyInit, }; + +// TODO: provide more detailed errors +#[derive(Debug, Clone)] +pub struct EncryptError(); + +impl std::fmt::Display for EncryptError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Encryption error occurred") + } +} + +impl std::error::Error for EncryptError {} + impl SecretKey { - fn shared_key(&self, other_pk: &PublicKey) -> Result { + fn shared_key(&self, other_pk: &PublicKey) -> Result { let key = self.to_x25519().diffie_hellman(&other_pk.to_x25519()); if !key.was_contributory() { - return Err(()); + return Err(EncryptError()); } let key = key.to_bytes(); // eprintln!("[shared_key] {} & {} = {}", self.public_key(), other_pk, hex::encode(&key)); @@ -73,11 +86,15 @@ impl SecretKey { other_pk: &PublicKey, rng: impl Rng + CryptoRng, data: &[u8], - ) -> Result, ()> { + ) -> Result, Box> { let shared_key = self.shared_key(other_pk)?; let nonce = Aes256Gcm::generate_nonce(rng); let mut buffer = Vec::from(AsRef::<[u8]>::as_ref(&nonce)); - buffer.extend(shared_key.encrypt(&nonce, data).or(Err(()))?); + buffer.extend( + shared_key + .encrypt(&nonce, data) + .or(Err(Box::new(EncryptError())))?, + ); Ok(buffer) } @@ -86,24 +103,30 @@ impl SecretKey { other_pk: &PublicKey, rng: impl Rng + CryptoRng, data: &M, - ) -> Result { - let data = serde_json::to_vec(data).or(Err(()))?; + ) -> Result> { + let data = serde_json::to_vec(data).map_err(|_| EncryptError())?; self.encrypt_raw(other_pk, rng, &data).map(Into::into) } - pub fn decrypt_raw(&self, other_pk: &PublicKey, ciphertext: &[u8]) -> Result, ()> { + pub fn decrypt_raw( + &self, + other_pk: &PublicKey, + ciphertext: &[u8], + ) -> Result, EncryptError> { let shared_key = self.shared_key(other_pk)?; - let (nonce, ciphertext) = ciphertext.split_at_checked(12).ok_or(())?; - shared_key.decrypt(nonce.into(), ciphertext).or(Err(())) + let (nonce, ciphertext) = ciphertext.split_at_checked(12).ok_or(EncryptError())?; + shared_key + .decrypt(nonce.into(), ciphertext) + .or(Err(EncryptError())) } pub fn decrypt( &self, other_pk: &PublicKey, ciphertext: &M::Encrypted, - ) -> Result { - let data = self.decrypt_raw(other_pk, ciphertext.as_ref())?; - serde_json::from_slice(&data).or(Err(())) + ) -> Result> { + let data: Vec = self.decrypt_raw(other_pk, ciphertext.as_ref())?; + serde_json::from_slice(&data).map_err(Box::::from) } } diff --git a/p2p/src/network/kad/p2p_network_kad_internals.rs b/p2p/src/network/kad/p2p_network_kad_internals.rs index 3246db4126..5c520af141 100644 --- a/p2p/src/network/kad/p2p_network_kad_internals.rs +++ b/p2p/src/network/kad/p2p_network_kad_internals.rs @@ -66,6 +66,12 @@ mod u256_serde { #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] pub struct P2pNetworkKadKey(#[serde(with = "u256_serde")] U256); +impl P2pNetworkKadKey { + pub fn distance(self, rhs: &Self) -> P2pNetworkKadDist { + P2pNetworkKadDist(self.0 ^ rhs.0) + } +} + #[derive(Clone, Debug, Serialize, PartialEq, Deserialize, thiserror::Error)] pub enum P2pNetworkKadKeyError { #[error("decoding error")] @@ -110,7 +116,7 @@ impl Debug for P2pNetworkKadKey { impl Add<&P2pNetworkKadDist> for &P2pNetworkKadKey { type Output = P2pNetworkKadKey; - #[allow(clippy::suspicious_arithmetic_impl)] + #[allow(clippy::suspicious_arithmetic_impl, clippy::arithmetic_side_effects)] fn add(self, rhs: &P2pNetworkKadDist) -> Self::Output { P2pNetworkKadKey(self.0 ^ rhs.0) } @@ -119,36 +125,36 @@ impl Add<&P2pNetworkKadDist> for &P2pNetworkKadKey { impl Sub for P2pNetworkKadKey { type Output = P2pNetworkKadDist; - #[allow(clippy::suspicious_arithmetic_impl)] + #[allow(clippy::suspicious_arithmetic_impl, clippy::arithmetic_side_effects)] fn sub(self, rhs: Self) -> Self::Output { - P2pNetworkKadDist(self.0 ^ rhs.0) + self.distance(&rhs) } } impl Sub<&P2pNetworkKadKey> for &P2pNetworkKadKey { type Output = P2pNetworkKadDist; - #[allow(clippy::suspicious_arithmetic_impl)] + #[allow(clippy::suspicious_arithmetic_impl, clippy::arithmetic_side_effects)] fn sub(self, rhs: &P2pNetworkKadKey) -> Self::Output { - P2pNetworkKadDist(self.0 ^ rhs.0) + self.distance(rhs) } } impl Sub for &P2pNetworkKadKey { type Output = P2pNetworkKadDist; - #[allow(clippy::suspicious_arithmetic_impl)] + #[allow(clippy::suspicious_arithmetic_impl, clippy::arithmetic_side_effects)] fn sub(self, rhs: P2pNetworkKadKey) -> Self::Output { - P2pNetworkKadDist(self.0 ^ rhs.0) + self.distance(&rhs) } } impl Sub<&P2pNetworkKadKey> for P2pNetworkKadKey { type Output = P2pNetworkKadDist; - #[allow(clippy::suspicious_arithmetic_impl)] + #[allow(clippy::suspicious_arithmetic_impl, clippy::arithmetic_side_effects)] fn sub(self, rhs: &P2pNetworkKadKey) -> Self::Output { - P2pNetworkKadDist(self.0 ^ rhs.0) + self.distance(rhs) } } diff --git a/p2p/src/service_impl/mod.rs b/p2p/src/service_impl/mod.rs index bd5b14f0ed..c6cbf390f3 100644 --- a/p2p/src/service_impl/mod.rs +++ b/p2p/src/service_impl/mod.rs @@ -77,13 +77,13 @@ pub mod webrtc { &mut self, other_pk: &PublicKey, message: &T, - ) -> Result; + ) -> Result>; fn decrypt( &mut self, other_pub_key: &PublicKey, encrypted: &T::Encrypted, - ) -> Result; + ) -> Result>; fn auth_encrypt_and_send( &mut self, diff --git a/p2p/src/service_impl/webrtc/mod.rs b/p2p/src/service_impl/webrtc/mod.rs index c363f39b6b..8704d4396f 100644 --- a/p2p/src/service_impl/webrtc/mod.rs +++ b/p2p/src/service_impl/webrtc/mod.rs @@ -808,13 +808,13 @@ pub trait P2pServiceWebrtc: redux::Service { &mut self, other_pk: &PublicKey, message: &T, - ) -> Result; + ) -> Result>; fn decrypt( &mut self, other_pub_key: &PublicKey, encrypted: &T::Encrypted, - ) -> Result; + ) -> Result>; fn auth_encrypt_and_send( &mut self, diff --git a/p2p/src/service_impl/webrtc_with_libp2p.rs b/p2p/src/service_impl/webrtc_with_libp2p.rs index 7c16c32141..2bab626dcf 100644 --- a/p2p/src/service_impl/webrtc_with_libp2p.rs +++ b/p2p/src/service_impl/webrtc_with_libp2p.rs @@ -172,7 +172,7 @@ impl P2pChannelsService for T { &mut self, other_pk: &crate::identity::PublicKey, message: &M, - ) -> Result { + ) -> Result> { P2pServiceWebrtc::encrypt(self, other_pk, message) } @@ -180,7 +180,7 @@ impl P2pChannelsService for T { &mut self, other_pk: &crate::identity::PublicKey, encrypted: &M::Encrypted, - ) -> Result { + ) -> Result> { P2pServiceWebrtc::decrypt(self, other_pk, encrypted) } } diff --git a/p2p/testing/src/service.rs b/p2p/testing/src/service.rs index 1a25065c54..8ffe1f1a30 100644 --- a/p2p/testing/src/service.rs +++ b/p2p/testing/src/service.rs @@ -104,7 +104,7 @@ impl P2pServiceWebrtc for ClusterService { &mut self, _other_pk: &p2p::identity::PublicKey, _message: &T, - ) -> Result { + ) -> Result> { unreachable!("this is webrtc only and this crate tests libp2p only") } @@ -112,7 +112,7 @@ impl P2pServiceWebrtc for ClusterService { &mut self, _other_pub_key: &p2p::identity::PublicKey, _encrypted: &T::Encrypted, - ) -> Result { + ) -> Result> { unreachable!("this is webrtc only and this crate tests libp2p only") }