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

change unwrap to map_err in hyperdrive python bindings #64

Closed
dpaiton opened this issue May 3, 2024 · 0 comments · Fixed by #113
Closed

change unwrap to map_err in hyperdrive python bindings #64

dpaiton opened this issue May 3, 2024 · 0 comments · Fixed by #113
Assignees

Comments

@dpaiton
Copy link
Contributor

dpaiton commented May 3, 2024

The best example is in calculate_targeted_long_with_budget, where we do:

            .map_err(|err| {
                PyErr::new::<PyValueError, _>(format!(
                    "Calculate_targeted_long_with_budget returned the error: {:?}",
                    err
                ))
            })?;

But that's the only place we do it.

In calculate_spot_price_after_short we do something like that but throw away the actual err. In most other fns we use unwrap(), which throws a cryptic panic message. We should follow the targeted long example everywhere.

@dpaiton dpaiton self-assigned this May 3, 2024
@dpaiton dpaiton linked a pull request May 24, 2024 that will close this issue
dpaiton added a commit that referenced this issue May 27, 2024
# 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant