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: create settle position function #1632

Merged
merged 12 commits into from
Oct 18, 2023
Merged

feat: create settle position function #1632

merged 12 commits into from
Oct 18, 2023

Conversation

matthiasmatt
Copy link
Contributor

@matthiasmatt matthiasmatt commented Oct 10, 2023

Description

  • waiting to review codecov report

Purpose

Why is this PR important?

@matthiasmatt matthiasmatt marked this pull request as ready for review October 10, 2023 13:28
@matthiasmatt matthiasmatt requested a review from a team as a code owner October 10, 2023 13:28
@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

Merging #1632 (2616741) into master (fd58bc3) will decrease coverage by 0.21%.
Report is 1 commits behind head on master.
The diff coverage is 54.87%.

❗ Current head 2616741 differs from pull request most recent head 38d0a9e. Consider uploading reports for the commit 38d0a9e to get more accurate results

@@            Coverage Diff             @@
##           master    #1632      +/-   ##
==========================================
- Coverage   74.52%   74.32%   -0.21%     
==========================================
  Files         177      177              
  Lines       14652    14814     +162     
==========================================
+ Hits        10919    11010      +91     
- Misses       3112     3176      +64     
- Partials      621      628       +7     
Files Coverage Δ
x/perp/v2/client/cli/gen_market.go 81.95% <ø> (-0.27%) ⬇️
x/perp/v2/types/msgs.go 91.71% <88.23%> (-0.39%) ⬇️
x/perp/v2/keeper/msg_server.go 77.08% <0.00%> (-14.28%) ⬇️
x/perp/v2/keeper/settlement.go 76.72% <78.12%> (+21.72%) ⬆️
x/perp/v2/client/cli/tx.go 63.90% <0.00%> (-8.65%) ⬇️

... and 2 files with indirect coverage changes

@matthiasmatt matthiasmatt marked this pull request as draft October 11, 2023 14:33
@matthiasmatt matthiasmatt marked this pull request as ready for review October 11, 2023 19:08
Copy link
Member

@Unique-Divine Unique-Divine left a comment

Choose a reason for hiding this comment

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

LGTM, I am a bit curious about how we can handle the fact that there's a variable part that could be unexpectedly large for the funding payments.

Also, when we close one market, how are the conditions for the next market selected?

@matthiasmatt
Copy link
Contributor Author

LGTM, I am a bit curious about how we can handle the fact that there's a variable part that could be unexpectedly large for the funding payments.

We have this issue for settlement and also normal behavior of the market. I think the clamp is helping us a lot in this since closing a market might have a lot to do with index/mark price divergence

@matthiasmatt matthiasmatt enabled auto-merge (squash) October 17, 2023 09:07
@matthiasmatt matthiasmatt merged commit 4561f47 into master Oct 18, 2023
@matthiasmatt matthiasmatt deleted the mat/settlement-tx branch October 18, 2023 04:18
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