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.

Sign up for GitHub

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

Jump to bottom

Cross margin futures initial setup #1139

Merged
merged 19 commits into from
Aug 4, 2022
Merged

Cross margin futures initial setup #1139

merged 19 commits into from
Aug 4, 2022

Conversation

avclarke
Copy link
Contributor

@avclarke avclarke commented Jul 19, 2022 •
edited
Loading

Description

Add support for cross margin future accounts

Related issue

https://github.com/Kwenta/kwenta-private/issues/357

Motivation and Context

Initial setup and refactor of futures components to allow for users to use their cross margin account as well as isolated margin account for futures trades.

Current functionality

  • There is a button at the top right of the futures trade panel which will allow you to switch between account types, opening an onboarding modal if you need to create a cross margin account.
  • After account creation you can deposit sUSD
  • Open and close cross margin position (temporarily uses all margin)
  • Cross margin trade history

Missing functionality

  • Withdraw margin
  • Distributing margin
  • Aggregated trade history

@vercel
Copy link

vercel bot commented Jul 19, 2022 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
kwenta ✅ Ready (Inspect) Visit Preview Aug 4, 2022 at 8:16PM (UTC)

@vercel vercel bot temporarily deployed to Preview July 19, 2022 18:30 Inactive
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 19, 2022
View reviewed changes
hooks/useQueryCrossMarginAccount.ts Fixed Show fixed Hide fixed
hooks/useQueryCrossMarginAccount.ts Fixed Show fixed Hide fixed
hooks/useQueryCrossMarginAccount.ts Fixed Show fixed Hide fixed
@vercel vercel bot temporarily deployed to Preview July 20, 2022 10:18 Inactive
@vercel vercel bot temporarily deployed to Preview July 26, 2022 09:14 Inactive
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 28, 2022
View reviewed changes
sections/futures/PositionCard/ClosePositionModal.tsx
}, [closeTxn.hash]);
}, [closeTxn?.hash]);

const closePosition = async () => {

Check notice

Code scanning

'closePosition' is assigned a value but never used.

'closePosition' is assigned a value but never used.
@vercel vercel bot temporarily deployed to Preview July 28, 2022 09:09 Inactive
@vercel vercel bot temporarily deployed to Preview August 1, 2022 09:40 Inactive
@vercel vercel bot temporarily deployed to Preview August 1, 2022 11:15 Inactive
@vercel vercel bot temporarily deployed to Preview August 2, 2022 12:59 Inactive
@vercel vercel bot temporarily deployed to Preview August 2, 2022 18:14 Inactive
@avclarke avclarke changed the title Cross margin futures (WIP) Cross margin futures initial setup Aug 2, 2022
@avclarke avclarke marked this pull request as ready for review August 2, 2022 18:57
@vercel vercel bot temporarily deployed to Preview August 2, 2022 18:59 Inactive
@Tburm
Copy link
Contributor

Tburm commented Aug 2, 2022

The sUSD balance on this component drops to 0 when I swap to my cross margin account:
image

@avclarke
Copy link
Contributor Author

avclarke commented Aug 2, 2022

The sUSD balance on this component drops to 0 when I swap to my cross margin account: image

That should be fixed in one of the more recent commits, you might need to pull the latest.

@vercel vercel bot temporarily deployed to Preview August 3, 2022 11:29 Inactive
@vercel vercel bot temporarily deployed to Preview August 3, 2022 13:27 Inactive
@platschi platschi requested review from platschi and Tburm August 3, 2022 13:31
@vercel vercel bot temporarily deployed to Preview August 3, 2022 13:41 Inactive
Tburm
Tburm previously approved these changes Aug 3, 2022
View reviewed changes
Copy link
Contributor

@Tburm Tburm left a comment

Choose a reason for hiding this comment

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

Testing worked as expected, allowing me to deposit to cross margin and open positions using all of my free margin against a market. Trade and transfer history also look accurate depending on the selected account.

I am approving even though I'm leaving a few comments where we can improve and fix some bugs. As long as these are documented in issues I see no reason why we should block merging and continue making improvements.

queries/synths/useSynthBalances.ts Outdated Show resolved Hide resolved
contexts/RefetchContext.tsx Outdated Show resolved Hide resolved
queries/futures/useGetCurrentPortfolioValue.ts Show resolved Hide resolved
sections/futures/MarketInfoBox/MarketInfoBox.tsx Show resolved Hide resolved
queries/futures/useGetCrossMarginTradePreview.ts Outdated Show resolved Hide resolved
sections/futures/Trade/CrossMarginAccountActions.tsx Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview August 4, 2022 10:51 Inactive
@avclarke
Copy link
Contributor Author

avclarke commented Aug 4, 2022

Thanks for the review @Tburm I've addresses a few of the things and left comments about what we can leave until updating the trade panel.

@avclarke avclarke requested a review from Tburm August 4, 2022 13:08
@vercel vercel bot temporarily deployed to Preview August 4, 2022 13:09 Inactive
Tburm
Tburm previously approved these changes Aug 4, 2022
View reviewed changes
platschi
platschi previously approved these changes Aug 4, 2022
View reviewed changes
@avclarke avclarke dismissed stale reviews from platschi and Tburm via 1275e83 August 4, 2022 20:13
@vercel vercel bot temporarily deployed to Preview August 4, 2022 20:17 Inactive
@platschi platschi merged commit 07a56cc into dev Aug 4, 2022
@KngZhi KngZhi mentioned this pull request Aug 11, 2022
Closed
@platschi platschi deleted the feat/cross-margin branch August 18, 2022 20:21
@KngZhi KngZhi mentioned this pull request Oct 5, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@platschi platschi platschi approved these changes

@Tburm Tburm Tburm left review comments

Assignees
No one assigned
Labels
cross margin
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@avclarke @Tburm @platschi