From 17d57492f45761e88b99e4b027544abbea9044ef Mon Sep 17 00:00:00 2001 From: Mr-kay-cloud2 Date: Thu, 21 May 2026 16:04:08 +0100 Subject: [PATCH] fix: resolve panic docs and storage options issues --- CHANGELOG.md | 1 + crates/db/src/conv.rs | 4 ++ crates/db/src/lib.rs | 23 +++++++++++ crates/db/src/manager.rs | 4 +- .../src/remote_prover/batch_prover.rs | 3 ++ .../src/remote_prover/block_prover.rs | 3 ++ .../src/remote_prover/tx_prover.rs | 3 ++ crates/utils/src/clap.rs | 38 ++++++++++--------- crates/utils/src/clap/rocksdb.rs | 2 +- crates/utils/src/fee.rs | 11 +++++- crates/utils/src/fifo_cache.rs | 8 ++++ crates/utils/src/fs.rs | 4 ++ crates/utils/src/grpc.rs | 3 ++ crates/utils/src/grpc/layers.rs | 4 ++ crates/utils/src/lib.rs | 4 ++ crates/utils/src/limiter.rs | 4 ++ crates/utils/src/logging.rs | 15 ++++++++ crates/utils/src/panic.rs | 4 ++ 18 files changed, 117 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7ab38e17c..1141a1fec0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ - [BREAKING] Renamed `SubmitProvenBatch` RPC endpoint to `SubmitProvenTxBatch` ([#2094](https://github.com/0xMiden/node/pull/2094)). - Fixed block producer mempool panic when selecting transactions that depend on notes created by pruned committed transactions ([#2097](https://github.com/0xMiden/node/pull/2097)). - Implemented filtering based on the network account note script root allowlist in ntx-builder ([#2042](https://github.com/0xMiden/node/issues/2042)). +- Documented additional public error and panic cases in utility crates ([#2114](https://github.com/0xMiden/node/pull/2114)). - [BREAKING] Renamed `ExplorerStatusDetails` fields in the network monitor's `/status` payload from `number_of_*` to `total_*` (`total_transactions`, `total_nullifiers`, `total_notes`, `total_account_updates`). The values now represent network-wide cumulative totals from the explorer's `overviewStats` query instead of last-block counts. - [BREAKING] Removed `--wallet-filepath` / `--counter-filepath` flags and the `MIDEN_MONITOR_WALLET_FILEPATH` / `MIDEN_MONITOR_COUNTER_FILEPATH` env vars from the network monitor. The monitor now keeps wallet and counter accounts fully in memory and regenerates them on every startup; the dashboard's counter value resets to zero on restart. diff --git a/crates/db/src/conv.rs b/crates/db/src/conv.rs index 8da9b50d38..f14e5b6315 100644 --- a/crates/db/src/conv.rs +++ b/crates/db/src/conv.rs @@ -52,6 +52,10 @@ pub trait SqlTypeConvert: Sized { type Raw: Sized; fn to_raw_sql(self) -> Self::Raw; + + /// # Errors + /// + /// Returns an error if conversion from raw SQL fails. fn from_raw_sql(_raw: Self::Raw) -> Result; fn map_err( diff --git a/crates/db/src/lib.rs b/crates/db/src/lib.rs index d1a3b81b37..0faaab097a 100644 --- a/crates/db/src/lib.rs +++ b/crates/db/src/lib.rs @@ -18,6 +18,10 @@ pub type Result = std::result::Result; /// /// Defaults to twice the available CPU parallelism. If the OS cannot report the available /// parallelism, fall back to two connections. +/// +/// # Panics +/// +/// Panics if the computed connection count is zero. pub fn default_connection_pool_size() -> NonZeroUsize { let available_cores = std::thread::available_parallelism().map_or(1, NonZeroUsize::get); let connection_count = available_cores.saturating_mul(2); @@ -33,11 +37,22 @@ pub struct Db { impl Db { /// Creates a new database instance with the provided connection pool. + /// # Errors + /// + /// Returns an error if the database connection pool cannot be created. pub fn new(database_filepath: &Path) -> Result { Self::new_with_pool_size(database_filepath, default_connection_pool_size()) } /// Creates a new database instance with the provided connection pool size. + /// + /// # Errors + /// + /// Returns an error if the database connection pool cannot be created. + /// + /// # Panics + /// + /// Panics if the database file path is not valid UTF-8. pub fn new_with_pool_size( database_filepath: &Path, connection_pool_size: NonZeroUsize, @@ -50,6 +65,10 @@ impl Db { } /// Create and commit a transaction with the queries added in the provided closure + /// + /// # Errors + /// + /// Returns an error if database access fails. pub async fn transact(&self, msg: M, query: Q) -> std::result::Result where Q: Send @@ -78,6 +97,10 @@ impl Db { } /// Run the query _without_ a transaction + /// + /// # Errors + /// + /// Returns an error if acquiring a database connection fails or if the query fails. pub async fn query(&self, msg: M, query: Q) -> std::result::Result where Q: Send + FnOnce(&mut SqliteConnection) -> std::result::Result + 'static, diff --git a/crates/db/src/manager.rs b/crates/db/src/manager.rs index c34e7a15e9..b845948320 100644 --- a/crates/db/src/manager.rs +++ b/crates/db/src/manager.rs @@ -74,7 +74,9 @@ impl deadpool::managed::Manager for ConnectionManager { Ok(()) } } - +/// # Errors +/// +/// Returns an error if configuring the database connection fails. pub fn configure_connection_on_creation( conn: &mut SqliteConnection, ) -> Result<(), ConnectionManagerError> { diff --git a/crates/remote-prover-client/src/remote_prover/batch_prover.rs b/crates/remote-prover-client/src/remote_prover/batch_prover.rs index 9514fdeb10..d63107b371 100644 --- a/crates/remote-prover-client/src/remote_prover/batch_prover.rs +++ b/crates/remote-prover-client/src/remote_prover/batch_prover.rs @@ -106,6 +106,9 @@ impl RemoteBatchProver { } impl RemoteBatchProver { + /// # Errors + /// + /// Returns an error if remote proof generation fails. pub async fn prove( &self, proposed_batch: ProposedBatch, diff --git a/crates/remote-prover-client/src/remote_prover/block_prover.rs b/crates/remote-prover-client/src/remote_prover/block_prover.rs index ee45aef67e..0663de78c8 100644 --- a/crates/remote-prover-client/src/remote_prover/block_prover.rs +++ b/crates/remote-prover-client/src/remote_prover/block_prover.rs @@ -102,6 +102,9 @@ impl RemoteBlockProver { } impl RemoteBlockProver { + /// # Errors + /// + /// Returns an error if remote proof generation fails. pub async fn prove( &self, tx_batches: OrderedBatches, diff --git a/crates/remote-prover-client/src/remote_prover/tx_prover.rs b/crates/remote-prover-client/src/remote_prover/tx_prover.rs index be213904c2..d14db192c8 100644 --- a/crates/remote-prover-client/src/remote_prover/tx_prover.rs +++ b/crates/remote-prover-client/src/remote_prover/tx_prover.rs @@ -102,6 +102,9 @@ impl RemoteTransactionProver { } impl RemoteTransactionProver { + /// # Errors + /// + /// Returns an error if transaction proof generation fails. pub fn prove( &self, tx_inputs: &TransactionInputs, diff --git a/crates/utils/src/clap.rs b/crates/utils/src/clap.rs index d8fa718a05..ceee53d792 100644 --- a/crates/utils/src/clap.rs +++ b/crates/utils/src/clap.rs @@ -118,6 +118,22 @@ impl Default for GrpcOptionsExternal { } } +/// Collection of per usage storage backend configurations. +/// +/// Note: Currently only contains `rocksdb` related configuration. +#[derive(clap::Args, Clone, Debug, Default, PartialEq, Eq)] +pub struct StorageOptions { + #[cfg(feature = "rocksdb")] + #[clap(flatten)] + pub account_tree: AccountTreeRocksDbOptions, + #[cfg(feature = "rocksdb")] + #[clap(flatten)] + pub nullifier_tree: NullifierTreeRocksDbOptions, + #[cfg(feature = "rocksdb")] + #[clap(flatten)] + pub account_state_forest: AccountStateForestRocksDbOptions, +} + impl GrpcOptionsExternal { pub fn test() -> Self { Self { @@ -127,6 +143,10 @@ impl GrpcOptionsExternal { } /// Return a gRPC config for benchmarking. + /// + /// # Panics + /// + /// Panics if the hard-coded non-zero benchmark limits are changed to zero. pub fn bench() -> Self { Self { request_timeout: Duration::from_hours(24), @@ -138,26 +158,10 @@ impl GrpcOptionsExternal { } } -/// Collection of per usage storage backend configurations. -/// -/// Note: Currently only contains `rocksdb` related configuration. -#[derive(clap::Args, Clone, Debug, Default, PartialEq, Eq)] -pub struct StorageOptions { - #[cfg(feature = "rocksdb")] - #[clap(flatten)] - pub account_tree: AccountTreeRocksDbOptions, - #[cfg(feature = "rocksdb")] - #[clap(flatten)] - pub nullifier_tree: NullifierTreeRocksDbOptions, - #[cfg(feature = "rocksdb")] - #[clap(flatten)] - pub account_state_forest: AccountStateForestRocksDbOptions, -} - impl StorageOptions { /// Benchmark setup. /// - /// These values were determined during development of `LargeSmt` + /// These values were determined during development of `LargeSmt`. pub fn bench() -> Self { #[cfg(feature = "rocksdb")] { diff --git a/crates/utils/src/clap/rocksdb.rs b/crates/utils/src/clap/rocksdb.rs index 0cdca501d0..beac91b85b 100644 --- a/crates/utils/src/clap/rocksdb.rs +++ b/crates/utils/src/clap/rocksdb.rs @@ -23,7 +23,7 @@ impl From for RocksDbDurabilityMode { } } -/// Per usage options for rocksdb configuration +/// Per usage options for rocksdb configuration. #[derive(clap::Args, Clone, Debug, PartialEq, Eq)] pub struct NullifierTreeRocksDbOptions { #[arg( diff --git a/crates/utils/src/fee.rs b/crates/utils/src/fee.rs index 33c4472bad..d6ae9c514a 100644 --- a/crates/utils/src/fee.rs +++ b/crates/utils/src/fee.rs @@ -4,12 +4,19 @@ use miden_protocol::testing::account_id::ACCOUNT_ID_FEE_FAUCET; /// Derive a default, zero valued fee, payable to /// [`miden_protocol::testing::account_id::ACCOUNT_ID_FEE_FAUCET`]. +/// +/// # Panics +/// +/// Panics if the test faucet account ID is invalid or if fee construction fails. pub fn test_fee() -> FungibleAsset { let faucet = ACCOUNT_ID_FEE_FAUCET.try_into().unwrap(); FungibleAsset::new(faucet, 0).unwrap() } - -/// Derive the default fee parameters, compatible with [`fn test_fee`]. +/// Derive the default fee parameters, compatible with [`test_fee`]. +/// +/// # Panics +/// +/// Panics if the test faucet account ID is invalid or if fee parameter construction fails. pub fn test_fee_params() -> FeeParameters { let faucet = ACCOUNT_ID_FEE_FAUCET.try_into().unwrap(); FeeParameters::new(faucet, 0).unwrap() diff --git a/crates/utils/src/fifo_cache.rs b/crates/utils/src/fifo_cache.rs index df10793d11..20b3f3e976 100644 --- a/crates/utils/src/fifo_cache.rs +++ b/crates/utils/src/fifo_cache.rs @@ -34,11 +34,19 @@ where } /// Returns a clone of the value associated with `key`, or `None` if not present. + /// + /// # Panics + /// + /// Panics if the cache lock is poisoned. pub fn get(&self, key: &K) -> Option { self.0.lock().expect("fifo cache lock poisoned").map.get(key).cloned() } /// Inserts a key-value pair, evicting the oldest entry if the cache is at capacity. + /// + /// # Panics + /// + /// Panics if the cache lock is poisoned. pub fn push(&self, key: K, value: V) { let mut inner = self.0.lock().expect("fifo cache lock poisoned"); if inner.eviction.len() >= inner.capacity.get() { diff --git a/crates/utils/src/fs.rs b/crates/utils/src/fs.rs index a0a57bfeb2..979a4a33b3 100644 --- a/crates/utils/src/fs.rs +++ b/crates/utils/src/fs.rs @@ -1,6 +1,10 @@ use std::path::Path; /// Validates that a directory either does not exist (and creates it) or exists and is empty. +/// +/// # Errors +/// +/// Returns an error if the directory cannot be removed or created. pub fn ensure_empty_directory(directory: &Path) -> anyhow::Result<()> { if fs_err::exists(directory)? { let is_empty = fs_err::read_dir(directory)?.next().is_none(); diff --git a/crates/utils/src/grpc.rs b/crates/utils/src/grpc.rs index 3dc42adf8b..3b453b0132 100644 --- a/crates/utils/src/grpc.rs +++ b/crates/utils/src/grpc.rs @@ -5,6 +5,9 @@ use anyhow::Context; /// A sealed extension trait for [`url::Url`] that adds convenience functions for binding and /// connecting to the url. pub trait UrlExt: private::Sealed { + /// # Errors + /// + /// Returns an error if the URL cannot be converted to a socket address. fn to_socket(&self) -> anyhow::Result; } diff --git a/crates/utils/src/grpc/layers.rs b/crates/utils/src/grpc/layers.rs index c94912bcee..c53b01b43e 100644 --- a/crates/utils/src/grpc/layers.rs +++ b/crates/utils/src/grpc/layers.rs @@ -18,6 +18,10 @@ pub fn rate_limit_concurrent_connections( } /// Creates a per-IP rate limit layer using the configured governor settings. +/// +/// # Errors +/// +/// Returns an error if the gRPC rate limit layer cannot be created. pub fn rate_limit_per_ip( grpc_options: GrpcOptionsExternal, ) -> anyhow::Result< diff --git a/crates/utils/src/lib.rs b/crates/utils/src/lib.rs index 8732c72bf6..02036d12d6 100644 --- a/crates/utils/src/lib.rs +++ b/crates/utils/src/lib.rs @@ -42,7 +42,11 @@ impl ErrorReport for T {} pub trait FlattenResult where InnerError: Into, + OuterError: From, { + /// # Errors + /// + /// Returns an error if flattening the nested result fails. fn flatten_result(self) -> Result; } diff --git a/crates/utils/src/limiter.rs b/crates/utils/src/limiter.rs index c68b9cf53f..4198f56765 100644 --- a/crates/utils/src/limiter.rs +++ b/crates/utils/src/limiter.rs @@ -30,6 +30,10 @@ pub trait QueryParamLimiter { /// Limit that causes a bail if exceeded. const LIMIT: usize; /// Do the actual check. + /// + /// # Errors + /// + /// Returns an error if the request exceeds the rate limit. fn check(size: usize) -> Result<(), QueryLimitError> { if size > Self::LIMIT { Err(QueryLimitError { diff --git a/crates/utils/src/logging.rs b/crates/utils/src/logging.rs index 8e38ac7928..a1ed64925d 100644 --- a/crates/utils/src/logging.rs +++ b/crates/utils/src/logging.rs @@ -56,6 +56,14 @@ impl Drop for OtelGuard { /// /// Returns an [`OtelGuard`] if open-telemetry is enabled, otherwise `None`. When this guard is /// dropped, the tracer provider is shutdown. +/// +/// # Errors +/// +/// Returns an error if logging initialization fails. +/// +/// # Panics +/// +/// Panics if tracing is initialized more than once. pub fn setup_tracing(otel: OpenTelemetry) -> anyhow::Result> { if otel.is_enabled() { opentelemetry::global::set_text_map_propagator(TraceContextPropagator::new()); @@ -112,6 +120,9 @@ pub fn setup_tracing(otel: OpenTelemetry) -> anyhow::Result> { Ok(tracer_provider.map(|tracer_provider| OtelGuard { tracer_provider })) } +/// # Errors +/// +/// Returns an error if logging initialization fails. fn init_tracer_provider() -> anyhow::Result { let builder = opentelemetry_otlp::SpanExporter::builder().with_tonic(); @@ -130,6 +141,10 @@ fn init_tracer_provider() -> anyhow::Result { /// This forces serialization of all such tests. Otherwise, the tested spans could /// be interleaved during runtime. Also, the global exporter could be re-initialized in /// the middle of a concurrently running test. +/// +/// # Errors +/// +/// Returns an error if logging initialization fails. #[cfg(feature = "testing")] pub fn setup_test_tracing() -> anyhow::Result<( tokio::sync::mpsc::UnboundedReceiver, diff --git a/crates/utils/src/panic.rs b/crates/utils/src/panic.rs index c330fe362a..bffd70bc62 100644 --- a/crates/utils/src/panic.rs +++ b/crates/utils/src/panic.rs @@ -10,6 +10,10 @@ use crate::tracing::OpenTelemetrySpanExt; /// [`tower_http::catch_panic::ResponseForPanic`] trait. /// /// This should be added to tonic server builder as a layer via [`CatchPanicLayer::custom()`]. +/// +/// # Panics +/// +/// Panics if building the panic response fails. #[track_caller] pub fn catch_panic_layer_fn(err: Box) -> Response> { // Log the panic error details.