Replace panic with Result (#113)

# Resolved Issues
#99
#64
#65
#66

# Description
Our use of both panic and result results in a worse developer
experience, and should be changed to using Result everywhere possible.
This PR removes almost all `panic` uses in the core libs; where
functions once would panic but return a type `T` they now return an
error with the type`Result<T>`. I also removed `unwrap()` statements,
which convert `Result` to `panic`, in most places around the repo.

Exceptions were made for core (anything with sugar like `+`, `-`, `*`)
FixedPoint arithmetic (thus keeping any U256 panics), certain
test-related cases, and macros.

I also improved some error messaging (in bindings mostly, but also
elsewhere), added comments, & added a lint ignore.

I ran the fuzz testing with 10k `FUZZ_RUNS` and 50k `FAST_FUZZ_RUNS`
twice without error locally.

Cargo.toml

@@ -21,3 +21,6 @@ authors = [
     "Ryan Goree <ryan@delv.tech>",
     "Sheng Lundquist <sheng@delv.tech>",
 ]
+
+[workspace.lints.clippy]
+comparison_chain = "allow"

bindings/hyperdrivepy/src/hyperdrive_state.rs

@@ -1,5 +1,5 @@
 use hyperdrive_math::State;
-use pyo3::prelude::*;
+use pyo3::{exceptions::PyAssertionError, prelude::*};
 
 use crate::{PyPoolConfig, PyPoolInfo};
 
@@ -13,22 +13,27 @@ impl HyperdriveState {
         HyperdriveState { state }
     }
 
-    pub(crate) fn new_from_pool(pool_config: &PyAny, pool_info: &PyAny) -> Self {
+    pub(crate) fn new_from_pool(pool_config: &PyAny, pool_info: &PyAny) -> PyResult<Self> {
         let rust_pool_config = match PyPoolConfig::extract(pool_config) {
             Ok(py_pool_config) => py_pool_config.pool_config,
             Err(err) => {
-                panic!("Error extracting pool config: {:?}", err);
+                return Err(PyErr::new::<PyAssertionError, _>(format!(
+                    "Error extracting pool config {:?}: {}",
+                    pool_config, err
+                )));
             }
         };
         let rust_pool_info = match PyPoolInfo::extract(pool_info) {
             Ok(py_pool_info) => py_pool_info.pool_info,
             Err(err) => {
-                // Handle the error, e.g., printing an error message or panicking
-                panic!("Error extracting pool info: {:?}", err);
+                return Err(PyErr::new::<PyAssertionError, _>(format!(
+                    "Error extracting pool info {:?}: {}",
+                    pool_info, err
+                )));
             }
         };
         let state = State::new(rust_pool_config, rust_pool_info);
-        HyperdriveState::new(state)
+        Ok(HyperdriveState::new(state))
     }
 }
 
@@ -38,8 +43,10 @@ impl From<State> for HyperdriveState {
     }
 }
 
-impl From<(&PyAny, &PyAny)> for HyperdriveState {
-    fn from(args: (&PyAny, &PyAny)) -> Self {
+impl TryFrom<(&PyAny, &PyAny)> for HyperdriveState {
+    type Error = PyErr;
+
+    fn try_from(args: (&PyAny, &PyAny)) -> PyResult<Self> {
         HyperdriveState::new_from_pool(args.0, args.1)
     }
 }

bindings/hyperdrivepy/src/hyperdrive_state_methods.rs

@@ -4,25 +4,25 @@ mod short;
 mod yield_space;
 
 use ethers::core::types::U256;
-use hyperdrive_math::State;
 use pyo3::{exceptions::PyValueError, prelude::*};
 
+pub use crate::utils::*;
 use crate::HyperdriveState;
-pub use crate::{utils::*, PyPoolConfig, PyPoolInfo};
 
 #[pymethods]
 impl HyperdriveState {
     #[new]
     pub fn __init__(pool_config: &PyAny, pool_info: &PyAny) -> PyResult<Self> {
-        let rust_pool_config = PyPoolConfig::extract(pool_config)?.pool_config;
-        let rust_pool_info = PyPoolInfo::extract(pool_info)?.pool_info;
-        let state = State::new(rust_pool_config, rust_pool_info);
-        Ok(HyperdriveState::new(state))
+        HyperdriveState::new_from_pool(pool_config, pool_info)
     }
 
     pub fn to_checkpoint(&self, time: &str) -> PyResult<String> {
-        let time_int = U256::from_dec_str(time)
-            .map_err(|_| PyErr::new::<PyValueError, _>("Failed to convert time string to U256"))?;
+        let time_int = U256::from_dec_str(time).map_err(|err| {
+            PyErr::new::<PyValueError, _>(format!(
+                "Failed to convert time string {} to U256: {}",
+                time, err
+            ))
+        })?;
         let result_int = self.state.to_checkpoint(time_int);
         let result = result_int.to_string();
         Ok(result)
@@ -35,19 +35,25 @@ impl HyperdriveState {
     }
 
     pub fn calculate_spot_price(&self) -> PyResult<String> {
-        let result_fp = self.state.calculate_spot_price();
+        let result_fp = self.state.calculate_spot_price().map_err(|err| {
+            PyErr::new::<PyValueError, _>(format!("calculate_spot_price: {}", err))
+        })?;
         let result = U256::from(result_fp).to_string();
         Ok(result)
     }
 
     pub fn calculate_spot_rate(&self) -> PyResult<String> {
-        let result_fp = self.state.calculate_spot_rate();
+        let result_fp = self.state.calculate_spot_rate().map_err(|err| {
+            PyErr::new::<PyValueError, _>(format!("calculate_spot_rate: {}", err))
+        })?;
         let result = U256::from(result_fp).to_string();
         Ok(result)
     }
 
     pub fn calculate_max_spot_price(&self) -> PyResult<String> {
-        let result_fp = self.state.calculate_max_spot_price();
+        let result_fp = self.state.calculate_max_spot_price().map_err(|err| {
+            PyErr::new::<PyValueError, _>(format!("calculate_max_spot_price: {}", err))
+        })?;
         let result = U256::from(result_fp).to_string();
         Ok(result)
     }

bindings/hyperdrivepy/src/hyperdrive_state_methods/long/close.rs

@@ -12,19 +12,31 @@ impl HyperdriveState {
         maturity_time: &str,
         current_time: &str,
     ) -> PyResult<String> {
-        let bond_amount_fp = FixedPoint::from(U256::from_dec_str(bond_amount).map_err(|_| {
-            PyErr::new::<PyValueError, _>("Failed to convert bond_amount string to U256")
+        let bond_amount_fp = FixedPoint::from(U256::from_dec_str(bond_amount).map_err(|err| {
+            PyErr::new::<PyValueError, _>(format!(
+                "Failed to convert bond_amount string {} to U256: {}",
+                bond_amount, err
+            ))
         })?);
-        let maturity_time = U256::from_dec_str(maturity_time).map_err(|_| {
-            PyErr::new::<PyValueError, _>("Failed to convert maturity_time string to U256")
+        let maturity_time = U256::from_dec_str(maturity_time).map_err(|err| {
+            PyErr::new::<PyValueError, _>(format!(
+                "Failed to convert maturity_time string {} to U256: {}",
+                maturity_time, err
+            ))
         })?;
-        let current_time = U256::from_dec_str(current_time).map_err(|_| {
-            PyErr::new::<PyValueError, _>("Failed to convert current_time string to U256")
+        let current_time = U256::from_dec_str(current_time).map_err(|err| {
+            PyErr::new::<PyValueError, _>(format!(
+                "Failed to convert current_time string {} to U256: {}",
+                current_time, err
+            ))
         })?;
 
-        let result_fp =
-            self.state
-                .calculate_close_long(bond_amount_fp, maturity_time, current_time);
+        let result_fp = self
+            .state
+            .calculate_close_long(bond_amount_fp, maturity_time, current_time)
+            .map_err(|err| {
+                PyErr::new::<PyValueError, _>(format!("calculate_close_long: {}", err))
+            })?;
         let result = U256::from(result_fp).to_string();
         Ok(result)
     }

bindings/hyperdrivepy/src/hyperdrive_state_methods/long/max.rs

@@ -12,18 +12,22 @@ impl HyperdriveState {
         checkpoint_exposure: &str,
         maybe_max_iterations: Option<usize>,
     ) -> PyResult<String> {
-        let budget_fp = FixedPoint::from(U256::from_dec_str(budget).map_err(|_| {
-            PyErr::new::<PyValueError, _>("Failed to convert budget string to U256")
+        let budget_fp = FixedPoint::from(U256::from_dec_str(budget).map_err(|err| {
+            PyErr::new::<PyValueError, _>(format!(
+                "Failed to convert budget string {} to U256: {}",
+                budget, err
+            ))
         })?);
-        let checkpoint_exposure_i = I256::from_dec_str(checkpoint_exposure).map_err(|_| {
-            PyErr::new::<PyValueError, _>("Failed to convert checkpoint_exposure string to I256")
+        let checkpoint_exposure_i = I256::from_dec_str(checkpoint_exposure).map_err(|err| {
+            PyErr::new::<PyValueError, _>(format!(
+                "Failed to convert checkpoint_exposure string {} to I256: {}",
+                checkpoint_exposure, err
+            ))
         })?;
         let result_fp = self
             .state
             .calculate_max_long(budget_fp, checkpoint_exposure_i, maybe_max_iterations)
-            .map_err(|e| {
-                PyErr::new::<PyValueError, _>(format!("Failed to calculate max long: {}", e))
-            })?;
+            .map_err(|err| PyErr::new::<PyValueError, _>(format!("calculate_max_long: {}", err)))?;
         let result = U256::from(result_fp).to_string();
         Ok(result)
     }

bindings/hyperdrivepy/src/hyperdrive_state_methods/long/open.rs

@@ -7,24 +7,35 @@ use crate::HyperdriveState;
 #[pymethods]
 impl HyperdriveState {
     pub fn calculate_open_long(&self, base_amount: &str) -> PyResult<String> {
-        let base_amount_fp = FixedPoint::from(U256::from_dec_str(base_amount).map_err(|_| {
-            PyErr::new::<PyValueError, _>("Failed to convert base_amount string to U256")
+        let base_amount_fp = FixedPoint::from(U256::from_dec_str(base_amount).map_err(|err| {
+            PyErr::new::<PyValueError, _>(format!(
+                "Failed to convert base_amount string {} to U256: {}",
+                base_amount, err
+            ))
         })?);
-        let result_fp = self.state.calculate_open_long(base_amount_fp).unwrap();
+        let result_fp = self
+            .state
+            .calculate_open_long(base_amount_fp)
+            .map_err(|err| {
+                PyErr::new::<PyValueError, _>(format!("calculate_open_long: {}", err))
+            })?;
         let result = U256::from(result_fp).to_string();
         Ok(result)
     }
 
     pub fn calculate_pool_deltas_after_open_long(&self, base_amount: &str) -> PyResult<String> {
-        let base_amount_fp = FixedPoint::from(U256::from_dec_str(base_amount).map_err(|_| {
-            PyErr::new::<PyValueError, _>("Failed to convert base_amount string to U256")
+        let base_amount_fp = FixedPoint::from(U256::from_dec_str(base_amount).map_err(|err| {
+            PyErr::new::<PyValueError, _>(format!(
+                "Failed to convert base_amount string {} to U256: {}",
+                base_amount, err
+            ))
         })?);
         let result_fp = self
             .state
             .calculate_pool_deltas_after_open_long(base_amount_fp)
             .map_err(|err| {
                 PyErr::new::<PyValueError, _>(format!(
-                    "calculate_pool_deltas_after_open_long returned the error: {:?}",
+                    "calculate_pool_deltas_after_open_long: {:?}",
                     err
                 ))
             })?;
@@ -37,15 +48,19 @@ impl HyperdriveState {
         base_amount: &str,
         maybe_bond_amount: Option<&str>,
     ) -> PyResult<String> {
-        let base_amount_fp = FixedPoint::from(U256::from_dec_str(base_amount).map_err(|_| {
-            PyErr::new::<PyValueError, _>("Failed to convert base_amount string to U256")
+        let base_amount_fp = FixedPoint::from(U256::from_dec_str(base_amount).map_err(|err| {
+            PyErr::new::<PyValueError, _>(format!(
+                "Failed to convert base_amount string {} to U256: {}",
+                base_amount, err
+            ))
         })?);
         let maybe_bond_amount_fp = if let Some(bond_amount) = maybe_bond_amount {
             Some(FixedPoint::from(U256::from_dec_str(bond_amount).map_err(
-                |_| {
-                    PyErr::new::<PyValueError, _>(
-                        "Failed to convert maybe_bond_amount string to U256",
-                    )
+                |err| {
+                    PyErr::new::<PyValueError, _>(format!(
+                        "Failed to convert maybe_bond_amount string {} to U256: {}",
+                        bond_amount, err
+                    ))
                 },
             )?))
         } else {
@@ -54,7 +69,9 @@ impl HyperdriveState {
         let result_fp = self
             .state
             .calculate_spot_price_after_long(base_amount_fp, maybe_bond_amount_fp)
-            .unwrap();
+            .map_err(|err| {
+                PyErr::new::<PyValueError, _>(format!("calculate_spot_price_after_long: {}", err))
+            })?;
         let result = U256::from(result_fp).to_string();
         Ok(result)
     }
@@ -64,15 +81,19 @@ impl HyperdriveState {
         base_amount: &str,
         maybe_bond_amount: Option<&str>,
     ) -> PyResult<String> {
-        let base_amount_fp = FixedPoint::from(U256::from_dec_str(base_amount).map_err(|_| {
-            PyErr::new::<PyValueError, _>("Failed to convert base_amount string to U256")
+        let base_amount_fp = FixedPoint::from(U256::from_dec_str(base_amount).map_err(|err| {
+            PyErr::new::<PyValueError, _>(format!(
+                "Failed to convert base_amount string {} to U256: {}",
+                base_amount, err
+            ))
         })?);
         let maybe_bond_amount_fp = if let Some(bond_amount) = maybe_bond_amount {
             Some(FixedPoint::from(U256::from_dec_str(bond_amount).map_err(
-                |_| {
-                    PyErr::new::<PyValueError, _>(
-                        "Failed to convert maybe_bond_amount string to U256",
-                    )
+                |err| {
+                    PyErr::new::<PyValueError, _>(format!(
+                        "Failed to convert maybe_bond_amount string {} to U256: {}",
+                        bond_amount, err
+                    ))
                 },
             )?))
         } else {
@@ -81,7 +102,7 @@ impl HyperdriveState {
         let result_fp = self
             .state
             .calculate_spot_rate_after_long(base_amount_fp, maybe_bond_amount_fp)
-            .unwrap();
+            .map_err(|err| PyErr::new::<PyValueError, _>(format!("{}", err)))?;
         let result = U256::from(result_fp).to_string();
         Ok(result)
     }

bindings/hyperdrivepy/src/hyperdrive_state_methods/long/targeted.rs

@@ -14,21 +14,31 @@ impl HyperdriveState {
         maybe_max_iterations: Option<usize>,
         maybe_allowable_error: Option<&str>,
     ) -> PyResult<String> {
-        let budget_fp = FixedPoint::from(U256::from_dec_str(budget).map_err(|_| {
-            PyErr::new::<PyValueError, _>("Failed to convert budget string to U256")
+        let budget_fp = FixedPoint::from(U256::from_dec_str(budget).map_err(|err| {
+            PyErr::new::<PyValueError, _>(format!(
+                "Failed to convert budget string {} to U256: {}",
+                budget, err
+            ))
         })?);
-        let target_rate_fp = FixedPoint::from(U256::from_dec_str(target_rate).map_err(|_| {
-            PyErr::new::<PyValueError, _>("Failed to convert target_rate string to U256")
+        let target_rate_fp = FixedPoint::from(U256::from_dec_str(target_rate).map_err(|err| {
+            PyErr::new::<PyValueError, _>(format!(
+                "Failed to convert target_rate string {} to U256: {}",
+                target_rate, err
+            ))
         })?);
-        let checkpoint_exposure_i = I256::from_dec_str(checkpoint_exposure).map_err(|_| {
-            PyErr::new::<PyValueError, _>("Failed to convert checkpoint_exposure string to I256")
+        let checkpoint_exposure_i = I256::from_dec_str(checkpoint_exposure).map_err(|err| {
+            PyErr::new::<PyValueError, _>(format!(
+                "Failed to convert checkpoint_exposure string {} to I256: {}",
+                checkpoint_exposure, err
+            ))
         })?;
         let maybe_allowable_error_fp = if let Some(allowable_error) = maybe_allowable_error {
             Some(FixedPoint::from(
-                U256::from_dec_str(allowable_error).map_err(|_| {
-                    PyErr::new::<PyValueError, _>(
-                        "Failed to convert maybe_allowable_error string to U256",
-                    )
+                U256::from_dec_str(allowable_error).map_err(|err| {
+                    PyErr::new::<PyValueError, _>(format!(
+                        "Failed to convert maybe_allowable_error string {} to U256: {}",
+                        allowable_error, err
+                    ))
                 })?,
             ))
         } else {
@@ -45,7 +55,7 @@ impl HyperdriveState {
             )
             .map_err(|err| {
                 PyErr::new::<PyValueError, _>(format!(
-                    "Calculate_targeted_long_with_budget returned the error: {:?}",
+                    "calculate_targeted_long_with_budget: {:?}",
                     err
                 ))
             })?;