Refactor to return errors instead of panic

[dpaiton]

TL;DR

Our use of both panic and result results in a worse developer experience, and should be changed to using Result everywhere.

The problem

Per the Rust docs:

You could call panic! for any error situation, whether there’s a possible way to recover or not, but then you’re making the decision that a situation is unrecoverable on behalf of the calling code.

While it was reasonable to set up panics in the MVP stage of this repo, it has matured enough now that we should take care to consider developer experience with error handling. The current hybrid workflow uses panic and Result, which is difficult to manage.

Examples

We have stuff like this:

            let max_long = match panic::catch_unwind(|| {
                state.calculate_max_long(U256::MAX, checkpoint_exposure, None)
            }) {
                Ok(max_long) => match max_long {
                    Ok(max_long) => max_long,
                    Err(_) => continue,
                },
                Err(_) => continue,
            };

Where we have to catch both a panic and a result to handle a type <Result, Result<FixedPoint, Report>, Report>.

Another situation is here:

            // Need to catch panics because of FixedPoint.
            let actual = panic::catch_unwind(|| {
                state.calculate_max_long(
                    U256::MAX,
                    checkpoint_exposure,
                    Some(max_iterations.into()),
                )
            });
            match chain
                .mock_hyperdrive_math()
                .calculate_max_long(
                    MaxTradeParams {
                        share_reserves: state.info.share_reserves,
                        bond_reserves: state.info.bond_reserves,
                        longs_outstanding: state.info.longs_outstanding,
                        long_exposure: state.info.long_exposure,
                        share_adjustment: state.info.share_adjustment,
                        time_stretch: state.config.time_stretch,
                        vault_share_price: state.info.vault_share_price,
                        initial_vault_share_price: state.config.initial_vault_share_price,
                        minimum_share_reserves: state.config.minimum_share_reserves,
                        curve_fee: state.config.fees.curve,
                        flat_fee: state.config.fees.flat,
                        governance_lp_fee: state.config.fees.governance_lp,
                    },
                    checkpoint_exposure,
                    max_iterations.into(),
                )
                .call()
                .await
            {
                Ok((expected_base_amount, ..)) => {
                    assert_eq!(
                        actual.unwrap().unwrap(),
                        FixedPoint::from(expected_base_amount)
                    );
                }
                Err(_) => assert!(actual.is_err() || actual.unwrap().is_err()),
            }

Where we have to check if either the FixedPoint or the hyperdrive-math function errored to compare against a single (flat) error from Solidity.

Solution

We should use return Err(eyre!("message")); for all errors from the lowest level (FixedPointMath) on up. Again, quoting the docs:

However, when failure is expected, it’s more appropriate to return a Result than to make a panic! call.

Since e.g. underflow & overflow errors are expected to happen often (e.g. when you are producing a negative FixedPoint) we should return a Result and let the user decide how to handle it.