Fix/spread assertion #222
Merged
Fix/spread assertion #222
Conversation
This file contains 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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #222 +/- ##
=======================================
Coverage 94.50% 94.50%
=======================================
Files 217 218 +1
Lines 24822 24750 -72
=======================================
- Hits 23457 23389 -68
+ Misses 1365 1361 -4
☔ View full report in Codecov by Sentry. |
kerber0x
force-pushed
the
fix/spread-assertion
branch
from
61bd61e to
b371f74
Compare
kerber0x
commented
Oct 20, 2023
| /// If `belief_price` and `max_spread` both are given, | ||
| /// we compute new spread else we just use terraswap | ||
| /// spread to check `max_spread` | ||
| pub fn assert_max_spread( |
There was a problem hiding this comment.
moved to packages/white-whale/src/pool_network/swap.rs since this code is shared with the pair contract as well.
kerber0x
commented
Oct 20, 2023
| @@ -74,11 +74,13 @@ pub enum ExecuteMsg { | |||
| operations: Vec<SwapOperation>, | |||
| minimum_receive: Option<Uint128>, | |||
| to: Option<String>, | |||
| max_spread: Option<Decimal>, | |||
There was a problem hiding this comment.
Added max_spread to router messages
kerber0x
force-pushed
the
fix/spread-assertion
branch
from
b371f74 to
a820252
Compare
kerber0x
commented
Oct 20, 2023
| offer_asset.clone(), | ||
| return_asset.clone(), | ||
| offer_asset.amount, | ||
| return_asset.amount.checked_add(fees)?, |
There was a problem hiding this comment.
fees should be accounted for when calculating spread in the return amount, otherwise the max_spread input by the user is gonna be off
kerber0x
commented
Oct 20, 2023
| ) -> Result<(), ContractError> { | ||
| let (offer_amount, return_amount, spread_amount): (Uint256, Uint256, Uint256) = | ||
| match offer_decimal.cmp(&return_decimal) { |
There was a problem hiding this comment.
remove all the decimal things as it's not needed
kerber0x
commented
Oct 20, 2023
| @@ -289,6 +289,7 @@ pub fn aggregate_fees( | |||
| operations, | |||
| minimum_receive: None, | |||
| to: None, | |||
| max_spread: Some(Decimal::percent(50u64)), | |||
There was a problem hiding this comment.
Add the max_spread possible (50%) when doing the fees aggregation to avoid max spread assertions
kerber0x
force-pushed
the
fix/spread-assertion
branch
from
a820252 to
50b8c5d
Compare
kerber0x
force-pushed
the
fix/spread-assertion
branch
from
035bb05 to
5e57ffb
Compare
0xFable
approved these changes
Oct 20, 2023
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.
Description and Motivation
This PR fixes the spread assertion miscalculation when interacting with tokens with different decimals. This is in preparation to the upcoming injective migration.
Additionally, it adds the
max_spreadparameter to the pool router.Related Issues
Checklist:
Update index.md)cargo fmt --all --.cargo clippy -- -D warnings.cargo schema.