Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Replace static str errors #613

Merged
merged 2 commits into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions cli/polka-storage-provider/server/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{
sync::atomic::{AtomicU32, Ordering},
};

use primitives_proofs::SectorNumber;
use primitives_proofs::{SectorNumber, SectorNumberError};
use rocksdb::{ColumnFamily, ColumnFamilyDescriptor, Options as DBOptions, DB as RocksDB};
use serde::{de::DeserializeOwned, Serialize};
use storagext::types::market::{ConversionError, DealProposal};
Expand Down Expand Up @@ -184,7 +184,7 @@ impl DealDB {

/// Atomically increments sector_id counter, so it can be used as an identifier by a sector.
/// Prior to all of the calls to this function, `initialize_biggest_sector_id` must be called at the node start-up.
pub fn next_sector_number(&self) -> Result<SectorNumber, &'static str> {
pub fn next_sector_number(&self) -> Result<SectorNumber, SectorNumberError> {
// [`Ordering::Relaxed`] can be used here, as it's an update on a single variable.
// It does not depend on other Atomic variables and it does not matter which thread makes it first.
// We just need it to be different on every thread that calls it concurrently, so the ids are not duplicated.
Expand Down
18 changes: 13 additions & 5 deletions primitives/commitment/src/piece.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@ impl From<PieceInfo> for filecoin_proofs::PieceInfo {
}
}

#[derive(PartialEq, Debug, Eq, Clone, Copy, thiserror::Error)]
pub enum UnpaddedPieceError {
#[error("minimum piece size is 127 bytes")]
TooSmall,
#[error("unpadded piece size must be a power of 2 multiple of 127")]
WrongSize,
}

/// Size of a piece in bytes. Unpadded piece size should be power of two
/// multiple of 127.
#[cfg_attr(feature = "serde", derive(::serde::Deserialize, ::serde::Serialize))]
Expand All @@ -47,14 +55,14 @@ impl UnpaddedPieceSize {

/// Initialize new unpadded piece size. Error is returned if the size is
/// invalid.
pub fn new(size: u64) -> Result<Self, &'static str> {
pub fn new(size: u64) -> Result<Self, UnpaddedPieceError> {
if size < 127 {
return Err("minimum piece size is 127 bytes");
return Err(UnpaddedPieceError::TooSmall);
}

// is 127 * 2^n
if size >> size.trailing_zeros() != 127 {
return Err("unpadded piece size must be a power of 2 multiple of 127");
return Err(UnpaddedPieceError::WrongSize);
}

Ok(Self(size))
Expand Down Expand Up @@ -236,15 +244,15 @@ mod tests {
);
assert_eq!(
UnpaddedPieceSize::new(126),
Err("minimum piece size is 127 bytes")
Err(UnpaddedPieceError::TooSmall)
);
assert_eq!(
PaddedPieceSize::new(0b10000001),
Err(PaddedPieceSizeError::SizeNotPowerOfTwo)
);
assert_eq!(
UnpaddedPieceSize::new(0b1110111000),
Err("unpadded piece size must be a power of 2 multiple of 127")
Err(UnpaddedPieceError::WrongSize)
);
assert!(UnpaddedPieceSize::new(0b1111111000).is_ok());
}
Expand Down
2 changes: 1 addition & 1 deletion primitives/proofs/Cargo.toml
aidan46 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ codec = { workspace = true, default-features = false, features = ["derive"] }
scale-decode = { workspace = true, default-features = false, features = ["derive"] }
scale-encode = { workspace = true, default-features = false, features = ["derive"] }
scale-info = { workspace = true, default-features = false, features = ["derive"] }

sp-core = { workspace = true, default-features = false }
sp-runtime = { workspace = true, default-features = false }
sp-std = { workspace = true, default-features = false }
thiserror = { workspace = true, default-features = false }

clap = { workspace = true, features = ["derive"], optional = true }
serde = { workspace = true, features = ["derive"], optional = true }
Expand Down
25 changes: 22 additions & 3 deletions primitives/proofs/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ impl SectorNumber {
///
/// Returns a `Result` containing the new `SectorNumber` if valid,
/// or an error message if the sector number exceeds `MAX_SECTORS`.
pub fn new(sector_number: u32) -> Result<Self, &'static str> {
pub fn new(sector_number: u32) -> Result<Self, SectorNumberError> {
if sector_number > MAX_SECTORS {
return Err("Sector number is too large");
return Err(SectorNumberError::NumberTooLarge);
}

Ok(Self(sector_number))
Expand Down Expand Up @@ -69,6 +69,25 @@ impl Decode for SectorNumber {
}
}

#[derive(
Clone,
Copy,
PartialEq,
Ord,
PartialOrd,
Eq,
Encode,
EncodeAsType,
TypeInfo,
RuntimeDebug,
MaxEncodedLen,
thiserror::Error,
)]
pub enum SectorNumberError {
#[error("Sector number is too large")]
NumberTooLarge,
}

// Implement the `Visitor` trait to define how to go from SCALE
// values into this type.
pub struct SectorNumberVisitor<R>(PhantomData<R>);
Expand Down Expand Up @@ -130,7 +149,7 @@ impl From<u16> for SectorNumber {
}

impl TryFrom<u32> for SectorNumber {
type Error = &'static str;
type Error = SectorNumberError;

fn try_from(value: u32) -> Result<Self, Self::Error> {
Self::new(value)
Expand Down