Minimize the number of panics in the codebase

CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - [#1948](https://github.com/FuelLabs/fuel-core/pull/1948): Add new `AlgorithmV1` and `AlgorithmUpdaterV1` for the gas price. Include tools for analysis
 
 ### Changed
+- [#1999](https://github.com/FuelLabs/fuel-core/pull/1999): Minimize the number of panics in the codebase.
 - [#1990](https://github.com/FuelLabs/fuel-core/pull/1990): Use latest view for mutate GraphQL queries after modification of the node.
 - [#1992](https://github.com/FuelLabs/fuel-core/pull/1992): Parse multiple relayer contracts, `RELAYER-V2-LISTENING-CONTRACTS` env variable using a `,` delimiter.
 

Cargo.toml

@@ -40,7 +40,7 @@ exclude = ["version-compatibility"]
 [profile.release]
 codegen-units = 1
 lto = "fat"
-panic = "abort"
+panic = "unwind"
 
 [workspace.package]
 authors = ["Fuel Labs <contact@fuel.sh>"]

crates/fuel-core/src/graphql_api/api_service.rs

@@ -278,7 +278,7 @@ async fn graphql_subscription_handler(
 ) -> Sse<impl Stream<Item = anyhow::Result<Event, serde_json::Error>>> {
     let stream = schema
         .execute_stream(req.0)
-        .map(|r| Ok(Event::default().json_data(r).unwrap()));
+        .map(|r| Event::default().json_data(r));
     Sse::new(stream)
         .keep_alive(axum::response::sse::KeepAlive::new().text("keep-alive-text"))
 }

crates/fuel-core/src/graphql_api/worker_service.rs

@@ -28,6 +28,7 @@ use fuel_core_services::{
     StateWatcher,
 };
 use fuel_core_storage::{
+    Error as StorageError,
     Result as StorageResult,
     StorageAsMut,
 };
@@ -46,11 +47,13 @@ use fuel_core_types::{
             Inputs,
             Outputs,
             Salt,
+            StorageSlots,
         },
         input::coin::{
             CoinPredicate,
             CoinSigned,
         },
+        Contract,
         Input,
         Output,
         Transaction,
@@ -328,15 +331,31 @@ where
 {
     for tx in transactions {
         match tx {
-            Transaction::Create(create) => {
-                let contract_id = create
+            Transaction::Create(tx) => {
+                let contract_id = tx
                     .outputs()
                     .iter()
                     .filter_map(|output| output.contract_id().cloned())
                     .next()
-                    .expect("Committed `Create` transaction should have a contract id");
-
-                let salt = *create.salt();
+                    .map(Ok::<_, StorageError>)
+                    .unwrap_or_else(|| {
+                        // TODO: Reuse `CreateMetadata` when it will be exported
+                        //  from the `fuel-tx` crate.
+                        let salt = tx.salt();
+                        let storage_slots = tx.storage_slots();
+                        let contract = Contract::try_from(tx)
+                            .map_err(|e| anyhow::anyhow!("{:?}", e))?;
+                        let contract_root = contract.root();
+                        let state_root =
+                            Contract::initial_state_root(storage_slots.iter());
+                        Ok::<_, StorageError>(contract.id(
+                            salt,
+                            &contract_root,
+                            &state_root,
+                        ))
+                    })?;
+
+                let salt = *tx.salt();
 
                 db.storage::<ContractsInfo>()
                     .insert(&contract_id, &(salt.into()))?;

crates/fuel-core/src/query/balance/asset_query.rs

@@ -95,7 +95,7 @@ impl<'a> AssetsQuery<'a> {
                     let id = if let CoinId::Utxo(id) = id {
                         id
                     } else {
-                        unreachable!("We've checked it above")
+                        return Err(anyhow::anyhow!("The coin is not UTXO").into());
                     };
                     let coin = self.database.coin(id)?;
 
@@ -127,7 +127,7 @@ impl<'a> AssetsQuery<'a> {
                     let id = if let CoinId::Message(id) = id {
                         id
                     } else {
-                        unreachable!("We've checked it above")
+                        return Err(anyhow::anyhow!("The coin is not a message").into());
                     };
                     let message = self.database.message(&id)?;
                     Ok(message)

crates/fuel-core/src/schema.rs

@@ -115,7 +115,7 @@ where
             )
             .into())
         }
-        (None, None, None, None) => {
+        (_, _, None, None) => {
             return Err(anyhow!("The queries for the whole range is not supported").into())
         }
         (_, _, _, _) => { /* Other combinations are allowed */ }
@@ -132,8 +132,7 @@ where
             } else if let Some(last) = last {
                 (last, IterDirection::Reverse)
             } else {
-                // Unreachable because of the check `(None, None, None, None)` above
-                unreachable!()
+                return Err(anyhow!("Either `first` or `last` should be provided"))
             };
 
             let start;

crates/services/consensus_module/poa/src/service.rs

@@ -505,7 +505,8 @@ where
                     }
                     should_continue = true;
                 } else {
-                    unreachable!("The task is the holder of the `Sender` too")
+                    tracing::error!("The PoA task should be the holder of the `Sender`");
+                    should_continue = false;
                 }
             }
             // TODO: This should likely be refactored to use something like tokio::sync::Notify.

crates/services/executor/src/executor.rs

@@ -1293,10 +1293,14 @@ where
             storage_tx,
         )?;
         let Input::Contract(input) = core::mem::take(input) else {
-            unreachable!()
+            return Err(ExecutorError::Other(
+                "Input of the `Mint` transaction is not a contract".to_string(),
+            ))
         };
         let Output::Contract(output) = outputs[0] else {
-            unreachable!()
+            return Err(ExecutorError::Other(
+                "The output of the `Mint` transaction is not a contract".to_string(),
+            ))
         };
         Ok((input, output))
     }
@@ -1533,10 +1537,7 @@ where
                 Input::CoinSigned(CoinSigned { utxo_id, .. })
                 | Input::CoinPredicate(CoinPredicate { utxo_id, .. }) => {
                     if let Some(coin) = db.storage::<Coins>().get(utxo_id)? {
-                        if !coin
-                            .matches_input(input)
-                            .expect("The input is a coin above")
-                        {
+                        if !coin.matches_input(input).unwrap_or_default() {
                             return Err(
                                 TransactionValidityError::CoinMismatch(*utxo_id).into()
                             )
@@ -1570,10 +1571,7 @@ where
                             .into())
                         }
 
-                        if !message
-                            .matches_input(input)
-                            .expect("The input is message above")
-                        {
+                        if !message.matches_input(input).unwrap_or_default() {
                             return Err(
                                 TransactionValidityError::MessageMismatch(*nonce).into()
                             )
@@ -1694,9 +1692,7 @@ where
         // if there's no script result (i.e. create) then fee == base amount
         Ok((
             total_used_gas,
-            max_fee
-                .checked_sub(fee)
-                .expect("Refunded fee can't be more than `max_fee`."),
+            max_fee.checked_sub(fee).ok_or(ExecutorError::FeeOverflow)?,
         ))
     }
 
@@ -1870,8 +1866,8 @@ where
     {
         let tx_idx = execution_data.tx_count;
         for (output_index, output) in outputs.iter().enumerate() {
-            let index = u16::try_from(output_index)
-                .expect("Transaction can have only up to `u16::MAX` outputs");
+            let index =
+                u16::try_from(output_index).map_err(|_| ExecutorError::TooManyOutputs)?;
             let utxo_id = UtxoId::new(*tx_id, index);
             match output {
                 Output::Coin {

crates/services/p2p/src/service.rs

@@ -497,7 +497,7 @@ where
 {
     async fn run(&mut self, watcher: &mut StateWatcher) -> anyhow::Result<bool> {
         tracing::debug!("P2P task is running");
-        let should_continue;
+        let mut should_continue;
 
         tokio::select! {
             biased;
@@ -549,7 +549,8 @@ where
                         let _ = channel.send(peers);
                     }
                     None => {
-                        unreachable!("The `Task` is holder of the `Sender`, so it should not be possible");
+                        tracing::error!("The P2P `Task` should be holder of the `Sender`");
+                        should_continue = false;
                     }
                 }
             }

crates/services/txpool/src/service.rs

@@ -452,9 +452,9 @@ where
             .into_iter()
             .map(|check_result| match check_result {
                 None => insertion.next().unwrap_or_else(|| {
-                    unreachable!(
-                        "the number of inserted txs matches the number of `None` results"
-                    )
+                    Err(Error::Other(
+                        anyhow::anyhow!("Insertion result is missing").to_string(),
+                    ))
                 }),
                 Some(err) => Err(err),
             })

crates/types/src/services/executor.rs

@@ -297,6 +297,9 @@ pub enum Error {
     /// It is possible to occur untyped errors in the case of the upgrade.
     #[display(fmt = "Occurred untyped error: {_0}")]
     Other(String),
+    /// Number of outputs is more than `u16::MAX`.
+    #[display(fmt = "Number of outputs is more than `u16::MAX`")]
+    TooManyOutputs,
 }
 
 impl From<Error> for anyhow::Error {

tests/test-helpers/Cargo.toml

@@ -32,4 +32,5 @@ fuel-core-txpool = { path = "../../crates/services/txpool", features = [
 fuel-core-types = { path = "../../crates/types", features = ["test-helpers"] }
 itertools = { workspace = true }
 rand = { workspace = true }
+reqwest = { workspace = true }
 tempfile = { workspace = true }

tests/test-helpers/src/lib.rs

@@ -1,2 +1,11 @@
 pub mod builder;
 pub mod fuel_core_driver;
+
+pub async fn send_graph_ql_query(url: &str, query: &str) -> String {
+    let client = reqwest::Client::new();
+    let mut map = std::collections::HashMap::new();
+    map.insert("query", query);
+    let response = client.post(url).json(&map).send().await.unwrap();
+
+    response.text().await.unwrap()
+}

tests/tests/blocks.rs

@@ -45,6 +45,7 @@ use std::{
     ops::Deref,
     time::Duration,
 };
+use test_helpers::send_graph_ql_query;
 
 #[tokio::test]
 async fn block() {
@@ -322,6 +323,23 @@ async fn block_connection_5(
     };
 }
 
+#[tokio::test]
+async fn missing_first_and_last_parameters_returns_an_error() {
+    let query = r#"
+        query {
+          transactions(before: "00000000#0x00"){
+            __typename
+          }
+        }
+    "#;
+
+    let node = FuelService::new_node(Config::local_node()).await.unwrap();
+    let url = format!("http://{}/v1/graphql", node.bound_address);
+
+    let result = send_graph_ql_query(&url, query).await;
+    assert!(result.contains("The queries for the whole range is not supported"));
+}
+
 mod full_block {
     use super::*;
     use cynic::QueryBuilder;

tests/tests/dos.rs

@@ -4,15 +4,7 @@ use fuel_core::service::{
     Config,
     FuelService,
 };
-
-async fn send_query(url: &str, query: &str) -> String {
-    let client = reqwest::Client::new();
-    let mut map = std::collections::HashMap::new();
-    map.insert("query", query);
-    let response = client.post(url).json(&map).send().await.unwrap();
-
-    response.text().await.unwrap()
-}
+use test_helpers::send_graph_ql_query;
 
 #[tokio::test]
 async fn complex_queries__recursion() {
@@ -81,7 +73,7 @@ async fn complex_queries__recursion() {
     let node = FuelService::new_node(config).await.unwrap();
     let url = format!("http://{}/v1/graphql", node.bound_address);
 
-    let result = send_query(&url, query).await;
+    let result = send_graph_ql_query(&url, query).await;
     assert!(result.contains("The recursion depth of the query cannot be greater than"));
 }
 
@@ -102,7 +94,7 @@ async fn complex_queries__10_blocks__works() {
     let node = FuelService::new_node(Config::local_node()).await.unwrap();
     let url = format!("http://{}/v1/graphql", node.bound_address);
 
-    let result = send_query(&url, query).await;
+    let result = send_graph_ql_query(&url, query).await;
     assert!(result.contains("transactions"));
 }
 
@@ -123,6 +115,6 @@ async fn complex_queries__50_block__query_to_complex() {
     let node = FuelService::new_node(Config::local_node()).await.unwrap();
     let url = format!("http://{}/v1/graphql", node.bound_address);
 
-    let result = send_query(&url, query).await;
+    let result = send_graph_ql_query(&url, query).await;
     assert!(result.contains("Query is too complex."));
 }