Skip to content

Commit

Permalink
Refuse use of unwrap in the codebase\; convert existing use of unwrap…
Browse files Browse the repository at this point in the history
… to expect/error results
  • Loading branch information
zbuc committed Sep 8, 2023
1 parent 82a1447 commit 5062307
Show file tree
Hide file tree
Showing 56 changed files with 196 additions and 72 deletions.
3 changes: 2 additions & 1 deletion clippy.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
disallowed-types = [
# Don't permit the use of `HashMap` because it's not ordered stably
{ path = "std::collections::HashMap", reason = "prefer `std::collections::BTreeMap` to provide a stable ordering" },
]
]
allow-unwrap-in-tests = true
1 change: 1 addition & 0 deletions crates/bin/pcli/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(clippy::unwrap_used)]
#![allow(clippy::clone_on_copy)]
use std::fs;

Expand Down
1 change: 1 addition & 0 deletions crates/bin/pclientd/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(clippy::unwrap_used)]
use anyhow::Result;
use clap::Parser;
use tracing_subscriber::{prelude::*, EnvFilter};
Expand Down
1 change: 1 addition & 0 deletions crates/bin/pd/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Source code for the Penumbra node software.
#![allow(clippy::clone_on_copy)]
#![deny(clippy::unwrap_used)]
#![recursion_limit = "512"]

mod consensus;
Expand Down
1 change: 1 addition & 0 deletions crates/bin/pd/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![allow(clippy::clone_on_copy)]
#![deny(clippy::unwrap_used)]
#![recursion_limit = "512"]
use std::{net::SocketAddr, path::PathBuf};

Expand Down
1 change: 1 addition & 0 deletions crates/core/app/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(clippy::unwrap_used)]
mod action_handler;
mod mock_client;
mod temp_storage_ext;
Expand Down
6 changes: 4 additions & 2 deletions crates/core/asset/src/asset/denom_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::{
sync::Arc,
};

use anyhow::ensure;
use anyhow::{ensure, Context};
use ark_ff::fields::PrimeField;
use decaf377::Fq;
use penumbra_num::Amount;
Expand Down Expand Up @@ -460,7 +460,9 @@ impl Unit {
}

let v2_power_of_ten = 10u128.pow((self.exponent() - right.len() as u8).into());
v2 = v2.checked_mul(v2_power_of_ten).unwrap();
v2 = v2
.checked_mul(v2_power_of_ten)
.context("multiplication overflowed when applying right hand side exponent")?;

let v = v1
.checked_mul(v1_power_of_ten)
Expand Down
14 changes: 10 additions & 4 deletions crates/core/asset/src/asset/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,23 +146,29 @@ impl Builder {
for _d in displays.iter() {
display_to_base.push(base_index);
}
display_regexes.push(displays.iter().map(|d| Regex::new(d).unwrap()).collect());
display_regexes.push(
displays
.iter()
.map(|d| Regex::new(d).expect("unable to parse display regex"))
.collect(),
);
}

Registry {
base_set: RegexSet::new(self.base_regexes.iter()).unwrap(),
base_set: RegexSet::new(self.base_regexes.iter())
.expect("unable to parse base regexes"),
base_regexes: self
.base_regexes
.iter()
.map(|r| Regex::new(r).unwrap())
.map(|r| Regex::new(r).expect("unable to parse base regex"))
.collect(),
constructors: self.constructors,
display_set: RegexSet::new(
self.unit_regexes
.iter()
.flat_map(|displays| displays.iter()),
)
.unwrap(),
.expect("unable to parse display regexes"),
display_to_base,
display_regexes,
}
Expand Down
3 changes: 2 additions & 1 deletion crates/core/asset/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(clippy::unwrap_used)]
use once_cell::sync::Lazy;

pub mod asset;
Expand All @@ -10,7 +11,7 @@ pub use value::{Value, ValueVar, ValueView};
pub static STAKING_TOKEN_DENOM: Lazy<asset::DenomMetadata> = Lazy::new(|| {
asset::Cache::with_known_assets()
.get_unit("upenumbra")
.unwrap()
.expect("unable to get upenumbra denom, which should be hardcoded")
.base()
});
pub static STAKING_TOKEN_ASSET_ID: Lazy<asset::Id> = Lazy::new(|| STAKING_TOKEN_DENOM.id());
36 changes: 27 additions & 9 deletions crates/core/asset/src/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use std::{
str::FromStr,
};

use anyhow::Context;
use penumbra_num::{Amount, AmountVar};
use penumbra_proto::{core::crypto::v1alpha1 as pb, DomainType, TypeUrl};
use regex::Regex;
Expand Down Expand Up @@ -297,23 +298,40 @@ impl FromStr for Value {
type Err = anyhow::Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let asset_id_re = Regex::new(r"^([0-9.]+)(passet[0-9].*)$").unwrap();
let denom_re = Regex::new(r"^([0-9.]+)([^0-9.].*)$").unwrap();
let asset_id_re =
Regex::new(r"^([0-9.]+)(passet[0-9].*)$").context("unable to parse asset ID regex")?;
let denom_re =
Regex::new(r"^([0-9.]+)([^0-9.].*)$").context("unable to parse denom regex")?;

if let Some(captures) = asset_id_re.captures(s) {
let numeric_str = captures.get(1).expect("matched regex").as_str();
let asset_id_str = captures.get(2).expect("matched regex").as_str();

let asset_id = Id::from_str(asset_id_str).expect("able to parse asset ID");
let amount = numeric_str.parse::<u64>().unwrap();
let numeric_str = captures
.get(1)
.context("string value should have numeric part")?
.as_str();
let asset_id_str = captures
.get(2)
.context("string value should have asset ID part")?
.as_str();

let asset_id =
Id::from_str(asset_id_str).context("unable to parse string value's asset ID")?;
let amount = numeric_str
.parse::<u64>()
.context("unable to parse string value's numeric amount")?;

Ok(Value {
amount: amount.into(),
asset_id,
})
} else if let Some(captures) = denom_re.captures(s) {
let numeric_str = captures.get(1).expect("matched regex").as_str();
let denom_str = captures.get(2).expect("matched regex").as_str();
let numeric_str = captures
.get(1)
.context("string value should have numeric part")?
.as_str();
let denom_str = captures
.get(2)
.context("string value should have denom part")?
.as_str();

let display_denom = REGISTRY.parse_unit(denom_str);
let amount = display_denom.parse_value(numeric_str)?;
Expand Down
1 change: 1 addition & 0 deletions crates/core/component/dao/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(clippy::unwrap_used)]
#![cfg_attr(docsrs, feature(doc_cfg))]

#[cfg_attr(docsrs, doc(cfg(feature = "component")))]
Expand Down
1 change: 1 addition & 0 deletions crates/core/component/dex/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(clippy::unwrap_used)]
#![cfg_attr(docsrs, feature(doc_cfg))]

#[cfg_attr(docsrs, doc(cfg(feature = "component")))]
Expand Down
1 change: 1 addition & 0 deletions crates/core/component/distributions/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(clippy::unwrap_used)]
#![cfg_attr(docsrs, feature(doc_cfg))]

#[cfg_attr(docsrs, doc(cfg(feature = "component")))]
Expand Down
1 change: 1 addition & 0 deletions crates/core/component/fee/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(clippy::unwrap_used)]
#![cfg_attr(docsrs, feature(doc_cfg))]

#[cfg_attr(docsrs, doc(cfg(feature = "component")))]
Expand Down
1 change: 1 addition & 0 deletions crates/core/component/governance/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(clippy::unwrap_used)]
mod delegator_vote;
pub use delegator_vote::proof::{DelegatorVoteCircuit, DelegatorVoteProof};

Expand Down
1 change: 1 addition & 0 deletions crates/core/component/ibc/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Many of the IBC message types are enums, where the number of variants differs
// depending on the build configuration, meaning that the fallthrough case gets
// marked as unreachable only when not building in test configuration.
#![deny(clippy::unwrap_used)]
#![allow(unreachable_patterns)]
#![cfg_attr(docsrs, feature(doc_cfg))]

Expand Down
1 change: 1 addition & 0 deletions crates/core/component/sct/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(clippy::unwrap_used)]
#![cfg_attr(docsrs, feature(doc_cfg))]

#[cfg_attr(docsrs, doc(cfg(feature = "component")))]
Expand Down
1 change: 1 addition & 0 deletions crates/core/component/shielded-pool/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(clippy::unwrap_used)]
#![cfg_attr(docsrs, feature(doc_cfg))]

#[cfg_attr(docsrs, doc(cfg(feature = "component")))]
Expand Down
1 change: 1 addition & 0 deletions crates/core/component/stake/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![cfg_attr(docsrs, feature(doc_cfg))]
#![deny(clippy::unwrap_used)]
#![allow(clippy::clone_on_copy)]

mod changes;
Expand Down
1 change: 1 addition & 0 deletions crates/core/keys/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(clippy::unwrap_used)]
use decaf377_fmd as fmd;
use decaf377_ka as ka;
use decaf377_rdsa as rdsa;
Expand Down
12 changes: 6 additions & 6 deletions crates/core/num/src/fixpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ impl U128x128 {
/// Decode this number from a 32-byte array.
pub fn from_bytes(bytes: [u8; 32]) -> Self {
// See above.
let hi = u128::from_be_bytes(bytes[0..16].try_into().unwrap());
let lo = u128::from_be_bytes(bytes[16..32].try_into().unwrap());
let hi = u128::from_be_bytes(bytes[0..16].try_into().expect("slice is 16 bytes"));
let lo = u128::from_be_bytes(bytes[16..32].try_into().expect("slice is 16 bytes"));
Self(U256::from_words(hi, lo))
}

Expand Down Expand Up @@ -204,10 +204,10 @@ impl AllocVar<U128x128, Fq> for U128x128Var {
// Now construct the bit constraints out of thin air ...
let bytes = inner.to_bytes();
// The U128x128 type uses a big-endian encoding
let limb_3 = u64::from_be_bytes(bytes[0..8].try_into().unwrap());
let limb_2 = u64::from_be_bytes(bytes[8..16].try_into().unwrap());
let limb_1 = u64::from_be_bytes(bytes[16..24].try_into().unwrap());
let limb_0 = u64::from_be_bytes(bytes[24..32].try_into().unwrap());
let limb_3 = u64::from_be_bytes(bytes[0..8].try_into().expect("slice is 8 bytes"));
let limb_2 = u64::from_be_bytes(bytes[8..16].try_into().expect("slice is 8 bytes"));
let limb_1 = u64::from_be_bytes(bytes[16..24].try_into().expect("slice is 8 bytes"));
let limb_0 = u64::from_be_bytes(bytes[24..32].try_into().expect("slice is 8 bytes"));

let limb_0_var = UInt64::new_variable(cs.clone(), || Ok(limb_0), AllocationMode::Witness)?;
let limb_1_var = UInt64::new_variable(cs.clone(), || Ok(limb_1), AllocationMode::Witness)?;
Expand Down
1 change: 1 addition & 0 deletions crates/core/num/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(clippy::unwrap_used)]
mod amount;
pub mod fixpoint;

Expand Down
1 change: 1 addition & 0 deletions crates/core/transaction/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
//! [`TransactionPerspective`] (e.g., the sender or receiver) of the cleartext
//! contents of a shielded transaction after it has been created.
#![deny(clippy::unwrap_used)]
#![allow(clippy::clone_on_copy)]

mod auth_data;
Expand Down
1 change: 1 addition & 0 deletions crates/crypto/decaf377-fmd/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! An implementation of [Fuzzy Message Detection][fmd].
//!
//! [fmd]: https://protocol.penumbra.zone/main/crypto/fmd.html
#![deny(clippy::unwrap_used)]

mod clue;
mod clue_key;
Expand Down
1 change: 1 addition & 0 deletions crates/crypto/decaf377-ka/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(clippy::unwrap_used)]
use std::convert::{TryFrom, TryInto};

use ark_ff::UniformRand;
Expand Down
5 changes: 4 additions & 1 deletion crates/crypto/eddy/src/decryption_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,10 @@ mod tests {
let now = Instant::now();

let table = MockDecryptionTable::default();
table.initialize(bitsize).await.unwrap();
table
.initialize(bitsize)
.await
.expect("unable to initialize test table");

let elapsed = now.elapsed();

Expand Down
1 change: 1 addition & 0 deletions crates/crypto/eddy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
//! - [ ] Encryption Proofs
//!
//! [protocol-batching]: https://protocol.penumbra.zone/main/concepts/batching_flows.html
#![deny(clippy::unwrap_used)]

mod ciphertext;
mod decryption_share;
Expand Down
8 changes: 4 additions & 4 deletions crates/crypto/eddy/src/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ mod tests {
fn limb_value_addition_roundtrip(value1: u64, value2: u64) {
let value = Value::from(value1);
let value2 = Value::from(value2);
let limbs = value.to_limbs().unwrap();
let limbs2 = value2.to_limbs().unwrap();
let limbs = value.to_limbs().expect("unable to convert to limbs");
let limbs2 = value2.to_limbs().expect("unable to convert to limbs");
let limbs3 = [
limbs[0].0 + limbs2[0].0,
limbs[1].0 + limbs2[1].0,
Expand All @@ -102,7 +102,7 @@ mod tests {
#[test]
fn limb_value_roundtrip(value: u64) {
let value = Value::from(value);
let limbs = value.to_limbs().unwrap();
let limbs = value.to_limbs().expect("unable to convert to limbs");
let value2 = Value::from_limbs(limbs[0], limbs[1], limbs[2], limbs[3]);
assert_eq!(value.0, value2.0);
}
Expand All @@ -114,7 +114,7 @@ mod tests {
let value = Value::from(value);
let (ciphertext, proof) = value
.transparent_encrypt(&encryption_key, &mut rng)
.unwrap();
.expect("unable to encrypt");

assert!(proof.verify(&ciphertext, &encryption_key).is_ok());
}
Expand Down
1 change: 1 addition & 0 deletions crates/crypto/proof-params/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(clippy::unwrap_used)]
use ark_groth16::{PreparedVerifyingKey, ProvingKey, VerifyingKey};
use ark_serialize::CanonicalDeserialize;
use decaf377::Bls12_377;
Expand Down
1 change: 1 addition & 0 deletions crates/crypto/proof-setup/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(clippy::unwrap_used)]
// Todo: prune public interface once we know exactly what's needed.
mod dlog;
mod group;
Expand Down
22 changes: 15 additions & 7 deletions crates/crypto/proof-setup/src/phase2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,8 @@ impl Contribution {
old: &CRSElements,
) -> Self {
let delta = F::rand(rng);
// e.w. negligible probability this will not panic
let delta_inv = delta.inverse().unwrap();
// e.w. negligible probability this will panic (1 / 2^256)
let delta_inv = delta.inverse().expect("unable to inverse delta");

let mut new = old.clone();
new.raw.delta_1 *= delta;
Expand Down Expand Up @@ -290,7 +290,7 @@ mod test {
fn non_trivial_crs() -> (CRSElements, RawCRSElements) {
let delta = F::rand(&mut OsRng);
// Won't panic e.w. negligible probability
let delta_inv = delta.inverse().unwrap();
let delta_inv = delta.inverse().expect("unable to inverse delta");

make_crs(delta, delta_inv)
}
Expand Down Expand Up @@ -340,7 +340,9 @@ mod test {
#[test]
fn test_contribution_produces_valid_crs() {
let (root, start) = non_trivial_crs();
let start = start.validate(&mut OsRng, &root).unwrap();
let start = start
.validate(&mut OsRng, &root)
.expect("unable to validate start");
let contribution = Contribution::make(
&mut OsRng,
ContributionHash([0u8; CONTRIBUTION_HASH_SIZE]),
Expand All @@ -356,7 +358,9 @@ mod test {
#[test]
fn test_can_calculate_contribution_hash() {
let (root, start) = non_trivial_crs();
let start = start.validate(&mut OsRng, &root).unwrap();
let start = start
.validate(&mut OsRng, &root)
.expect("unable to validate start");
let contribution = Contribution::make(
&mut OsRng,
ContributionHash([0u8; CONTRIBUTION_HASH_SIZE]),
Expand All @@ -368,7 +372,9 @@ mod test {
#[test]
fn test_contribution_is_linked_to_parent() {
let (root, start) = non_trivial_crs();
let start = start.validate(&mut OsRng, &root).unwrap();
let start = start
.validate(&mut OsRng, &root)
.expect("unable to validate start");
let contribution = Contribution::make(
&mut OsRng,
ContributionHash([0u8; CONTRIBUTION_HASH_SIZE]),
Expand All @@ -380,7 +386,9 @@ mod test {
#[test]
fn test_contribution_is_not_linked_to_itself() {
let (root, start) = non_trivial_crs();
let start = start.validate(&mut OsRng, &root).unwrap();
let start = start
.validate(&mut OsRng, &root)
.expect("unable to validate start");
let contribution = Contribution::make(
&mut OsRng,
ContributionHash([0u8; CONTRIBUTION_HASH_SIZE]),
Expand Down
2 changes: 1 addition & 1 deletion crates/crypto/tct/src/commitment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ mod arbitrary {
let rng = runner.rng();
Ok(if !self.0.is_empty() {
proptest::strategy::Just(
*rng.sample(rand::distributions::Slice::new(&self.0).unwrap()),
*rng.sample(rand::distributions::Slice::new(&self.0).expect("empty vector")),
)
} else {
let parts = [
Expand Down
Loading

0 comments on commit 5062307

Please sign in to comment.