Various bug fixes in rust sdk #1005
Conversation
slundqui
requested review from
jalextowle,
jrhea,
mcclurejt,
dpaiton,
ryangoree and
sentilesdal
as code owners
slundqui
force-pushed
the
slundquist/calc-max-long-fix
branch
from
9485122 to
7590280
Compare
| max_bond_amount = absolute_max_bond_amount; | ||
| // We expect by definition for the pool to be solvent with the absolute max bond amount. | ||
| maybe_solvency = | ||
| self.solvency_after_short(max_bond_amount, spot_price, checkpoint_exposure); |
There was a problem hiding this comment.
Didn’t we already check that this isn’t insolvent on line 294? I might be missing something, but I don’t expect this to be solvent
There was a problem hiding this comment.
Ah, my mistake, I didn't realize this was in the absolute_max_short function. Is it safe to say here then that if absolute_max_short_guess returns a value that is insolvent, that the pool doesn't allow for a short at all? Or is there a better initial guess we can use here
There was a problem hiding this comment.
We might be able to use a better initial guess. We could always use 0, or we could allow “cross-overs” by allowing invalid solutions so that we can proceed with iterations. The latter sounds fine to me since Newton’s should get to a solution that is solvent after a few iterations.
This check was based on my (erroneous) thinking at implementation-time that this algorithm would never cross over the solution
…ve_share_reserves and return 0 in calc_max_long if this is the case
|
Won't merge here in favor of making a PR to the new https://github.com/delvtech/hyperdrive-rs repo, will link and close once that PR is up. |
|
Closing in favor of delvtech/hyperdrive-rs#5 |
PR originally from delvtech/hyperdrive#1005 # Resolved Issues This solves various rust crashes resulting from python fuzz testing. https://github.com/delvtech/hyperdrive/issues/1004 # Description Fixes the following issues: - Max long guesses is below the minimum transaction amount, so `calc_open_long` throws a minimum transaction amount error. We clamp the guess to be minimum transaction amount. and do a final check at the end to ensure the max amount is greater than the minimum transaction amount. - We catch a case in `absolute_max_long` where the `target_share_reserves < effective_share_reserves`. In this case, we throw a better descriptive panic. - We short circuit `calc_max_long` and return 0 if the spot price after a minimum long exceeds the max spot price. - Fixing bug with wheel build script. - Building and uploading to pypi on tag instead of push to main. # Review Checklists Please check each item **before approving** the pull request. While going through the checklist, it is recommended to leave comments on items that are referenced in the checklist to make sure that they are reviewed. If there are multiple reviewers, copy the checklists into sections titled `## [Reviewer Name]`. If the PR doesn't touch Solidity and/or Rust, the corresponding checklist can be removed. ## [[Reviewer Name]] ### Rust - [ ] **Testing** - [ ] Are there new or updated unit or integration tests? - [ ] Do the tests cover the happy paths? - [ ] Do the tests cover the unhappy paths? - [ ] Are there an adequate number of fuzz tests to ensure that we are covering the full input space? - [ ] If matching Solidity behavior, are there differential fuzz tests that ensure that Rust matches Solidity?
Resolved Issues
This solves various rust crashes resulting from python fuzz testing.
delvtech/hyperdrive-rs#41
Description
Fixes the following issues:
calc_open_longthrows a minimum transaction amount error. We clamp the guess to be minimum transaction amount. and do a final check at the end to ensure the max amount is greater than the minimum transaction amount.absolute_max_longwhere thetarget_share_reserves < effective_share_reserves. In this case, we throw an error inabsolute_max_long, and catch it and return 0 incalc_max_long.Review Checklists
Please check each item before approving the pull request. While going
through the checklist, it is recommended to leave comments on items that are
referenced in the checklist to make sure that they are reviewed. If there are
multiple reviewers, copy the checklists into sections titled
## [Reviewer Name].If the PR doesn't touch Solidity and/or Rust, the corresponding checklist can
be removed.
[[Reviewer Name]]
Rust
covering the full input space?
ensure that Rust matches Solidity?