Update solvency_after_[long/short] to return Result<T> instead of Result<Option<T>>#143
Merged
Update solvency_after_[long/short] to return Result<T> instead of Result<Option<T>>#143
solvency_after_[long/short] to return Result<T> instead of Result<Option<T>>#143Conversation
solvency_after_[long/short] to return Result<T> instead of Result<Option<T>>solvency_after_[long/short] to return Result<T> instead of Result<Option<T>>
c12a11b to
ee76b47
Compare
dpaiton
added a commit
that referenced
this pull request
Jun 19, 2024
Updates the hyperdrive dependency to version `1.0.12` & makes required rust changes. A test is failing with this update, so I am ignoring it for now. It will pass once #143 lands.
2cd9ff9 to
d5a2c39
Compare
sentilesdal
approved these changes
Jun 20, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolved Issues
working towards #29
syncs changes with hyperdrive when the version bumped from
1.0.11to1.0.12.Description
The solvency checkers used to return
Option<T>, but were later updated to returnResult<Option<T>>so they could handle underlying function calls that returnedResault<T>. This leads to downstream checks that are confusing at best, and wrong at worst. For example, something like this inshort/max.rs:is meant as a sanity check on
max_bond_amountand should not cause the calling function to fail. However the?operator would indeed cause failure even though the call is wrapped in a match statement. Updating it tomeans that we don't care why or where the solvency check failed. Any underlying failure leads to assigning the value to 0, and a complete success leads to assigning it to the variable being checked.
Note: I had to increase the tolerance for the trade amount delta in
fuzz_error_open_short_max_txn_amount. The previous amount was already absurdly high, though, so either the test or the thing it's testing (max short & open short) is not working. I am actively investigating this in the process of completing #29. Given this, I think the tolerance increase is acceptible.