-
Notifications
You must be signed in to change notification settings - Fork 122
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
[CT-1002] functions to move collateral in and out of perpetual positions #1889
Conversation
WalkthroughThe updates introduce several new functions and methods across different files to enhance the handling and calculation of perpetual market positions and collateral management. Key changes include the addition of a function to create Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PerpetualMarket
participant SubaccountManager
participant CollateralManager
User->>PerpetualMarket: Request for PerpInfo creation
PerpetualMarket-->>User: Returns PerpInfo
User->>SubaccountManager: Update subaccount with new PerpInfo
SubaccountManager->>CollateralManager: Rebalance collateral
CollateralManager-->>SubaccountManager: Collateral rebalanced
SubaccountManager-->>User: Subaccount updated with new collateral
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
@@ -132,3 +133,36 @@ func SetUpDefaultPerpOIsForTest( | |||
} | |||
} | |||
} | |||
|
|||
func CreatePerpInfo( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just moving here, no change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- protocol/testutil/perpetuals/perpetuals.go (2 hunks)
- protocol/x/subaccounts/keeper/margining.go (1 hunks)
- protocol/x/subaccounts/keeper/margining_test.go (1 hunks)
- protocol/x/subaccounts/lib/updates.go (1 hunks)
- protocol/x/subaccounts/lib/updates_test.go (4 hunks)
- protocol/x/subaccounts/types/position_size.go (1 hunks)
- protocol/x/subaccounts/types/settled_update.go (1 hunks)
- protocol/x/subaccounts/types/update.go (1 hunks)
Files skipped from review due to trivial changes (1)
- protocol/x/subaccounts/keeper/margining_test.go
Additional comments not posted (16)
protocol/x/subaccounts/types/settled_update.go (2)
16-22
: LGTM! Function implementation is correct.The
GetAssetUpdates
function correctly converts theAssetUpdates
slice to a map and returns deep copies of each update.
24-30
: LGTM! Function implementation is correct.The
GetPerpetualUpdates
function correctly converts thePerpetualUpdates
slice to a map and returns deep copies of each update.protocol/x/subaccounts/types/position_size.go (1)
127-132
: LGTM! Function implementation is correct.The
GetBigQuoteBalance
function correctly returns theBigQuoteBalanceDelta
if it exists, otherwise a newbig.Int
.protocol/x/subaccounts/types/update.go (3)
95-100
: LGTM! Function implementation is correct.The
DeepCopy
function forAssetUpdate
correctly returns a deep copy of theAssetUpdate
struct.
112-122
: LGTM! Function implementation is correct.The
DeepCopy
function forPerpetualUpdate
correctly returns a deep copy of thePerpetualUpdate
struct.
108-109
: LGTM! Struct modification is correct.The
BigQuoteBalanceDelta
field is correctly added to thePerpetualUpdate
struct.protocol/testutil/perpetuals/perpetuals.go (1)
137-168
: LGTM! Function implementation is correct.The
CreatePerpInfo
function correctly constructs and returns aperptypes.PerpInfo
struct with the provided parameters.protocol/x/subaccounts/lib/updates_test.go (3)
8-8
: LGTM! Imports are clean and necessary.The import changes, including the addition of
perp_testutil
and the removal of unused imports, improve the codebase.
131-131
: LGTM! Function call updated correctly.The function call has been correctly updated to use
perp_testutil.CreatePerpInfo
.
150-151
: LGTM! Function calls updated correctly.The function calls have been correctly updated to use
perp_testutil.CreatePerpInfo
.protocol/x/subaccounts/keeper/margining.go (5)
15-30
: LGTM! Function implementation is correct.The function correctly calculates quote balance updates for the given settled updates.
110-160
: LGTM! Function implementation is correct.The function correctly rebalances collateral across all positions.
162-195
: LGTM! Function implementation is correct.The function correctly withdraws all extra collateral from all perpetual positions associated with the given subaccount.
197-230
: LGTM! Function implementation is correct.The function correctly moves collateral to a specified position.
32-108
: EnsureCalculateUpdatedSubaccount
handles edge cases.The function calculates quote balance updates for a single settled update. Ensure that
CalculateUpdatedSubaccount
handles edge cases correctly.protocol/x/subaccounts/lib/updates.go (1)
290-299
: LGTM! Position calculation updates are correct.The updates to the position calculation, including the new
QuoteBalance
field, are correct and consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- protocol/x/subaccounts/keeper/margining.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- protocol/x/subaccounts/keeper/margining.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- protocol/x/subaccounts/keeper/margining.go (1 hunks)
- protocol/x/subaccounts/keeper/margining_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- protocol/x/subaccounts/keeper/margining.go
- protocol/x/subaccounts/keeper/margining_test.go
Changelist
[Describe or list the changes made in this PR]
Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breaking
label.indexer-postgres-breaking
label.PrepareProposal
orProcessProposal
, manually add the labelproposal-breaking
.feature:[feature-name]
.backport/[branch-name]
.refactor
,chore
,bug
.Summary by CodeRabbit
New Features
Enhancements
Refactor
Testing
Documentation