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

feat: simplify special asset pairs calculations #2315

Merged
merged 47 commits into from
Nov 15, 2023
Merged

feat: simplify special asset pairs calculations #2315

merged 47 commits into from
Nov 15, 2023

Conversation

toteki
Copy link
Member

@toteki toteki commented Nov 6, 2023

No description provided.

x/leverage/keeper/limits.go Fixed Show fixed Hide fixed
x/leverage/types/position.go Dismissed Show dismissed Hide dismissed
@toteki toteki marked this pull request as ready for review November 13, 2023 07:43
@toteki toteki requested a review from a team as a code owner November 13, 2023 07:43
Copy link

codecov bot commented Nov 13, 2023

Codecov Report

Merging #2315 (7740d40) into main (7f05ad4) will decrease coverage by 5.11%.
Report is 294 commits behind head on main.
The diff coverage is 66.08%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2315      +/-   ##
==========================================
- Coverage   75.38%   70.28%   -5.11%     
==========================================
  Files         100      170      +70     
  Lines        8025    12659    +4634     
==========================================
+ Hits         6050     8897    +2847     
- Misses       1589     3158    +1569     
- Partials      386      604     +218     
Files Coverage Δ
ante/ante.go 66.66% <100.00%> (+18.45%) ⬆️
ante/fee.go 80.00% <100.00%> (+1.64%) ⬆️
ante/spam_prevention.go 75.92% <ø> (ø)
app/inflation/inflation.go 100.00% <100.00%> (ø)
app/upgradev3/migrations.go 84.21% <ø> (+5.94%) ⬆️
util/coin/utoken.go 100.00% <100.00%> (ø)
util/ibc/ibc.go 58.82% <ø> (ø)
util/sdkutil/events.go 0.00% <ø> (ø)
util/store/store.go 52.06% <ø> (+10.65%) ⬆️
util/store/unmarshal.go 42.85% <ø> (ø)
... and 40 more

... and 103 files with indirect coverage changes

@toteki
Copy link
Member Author

toteki commented Nov 13, 2023

Seeing some wierd go mod errors on builds and tests - started appearing 3 commits ago at 497e5be

Copy link
Member

@robert-zaremba robert-zaremba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you point to the place where was the bug

x/leverage/EXAMPLES.md Show resolved Hide resolved
x/leverage/EXAMPLES.md Outdated Show resolved Hide resolved
x/leverage/EXAMPLES.md Outdated Show resolved Hide resolved
x/leverage/keeper/limits.go Show resolved Hide resolved
x/leverage/keeper/limits.go Outdated Show resolved Hide resolved
x/leverage/keeper/limits.go Outdated Show resolved Hide resolved
x/leverage/keeper/limits.go Outdated Show resolved Hide resolved
toteki and others added 6 commits November 14, 2023 06:28
Co-authored-by: Robert Zaremba <robert@zaremba.ch>
Co-authored-by: Robert Zaremba <robert@zaremba.ch>
Co-authored-by: Robert Zaremba <robert@zaremba.ch>
Co-authored-by: Robert Zaremba <robert@zaremba.ch>
Co-authored-by: Robert Zaremba <robert@zaremba.ch>
@toteki
Copy link
Member Author

toteki commented Nov 14, 2023

Note: The original bug was in the logic for pairing normal assets durign max withdrawal, in which the position object was mutated by subtracting borrowed and collateral values from special asset pairs. The amount subtracted was too high, resulting in a negative coin amount.

This logic no longer exists in the refactor

Copy link
Member

@robert-zaremba robert-zaremba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved

@toteki toteki enabled auto-merge November 15, 2023 13:54
@toteki toteki added this pull request to the merge queue Nov 15, 2023
Merged via the queue into main with commit 5648999 Nov 15, 2023
25 of 27 checks passed
@toteki toteki deleted the adam/fixsp branch November 15, 2023 14:06
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 this pull request may close these issues.

2 participants