Skip to content

Conversation

LukasDeco
Copy link
Collaborator

@LukasDeco LukasDeco commented Aug 25, 2025

  • feat: initial SDK Changes for iso pos
  • feat: unit tests

@LukasDeco LukasDeco changed the base branch from master to crispheaney/isolated-position August 28, 2025 05:55
@LukasDeco LukasDeco marked this pull request as ready for review August 28, 2025 05:55
@LukasDeco LukasDeco requested a review from crispheaney August 28, 2025 05:55
@@ -0,0 +1,351 @@
import { BN } from './';
Copy link
Member

Choose a reason for hiding this comment

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

might move this to margin file

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

you are saying... don't import BN? Or like import directly from anchor?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh I see you are saying just keep this in the margin.ts file don't make a separate marginCalculation.ts file?

sdk/src/user.ts Outdated
enterHighLeverageMode = false,
perpMarketIndex?: number
): BN {
const totalCollateral = this.getTotalCollateral(marginCategory, true);
Copy link
Member

Choose a reason for hiding this comment

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

this also needs to be using the margin type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As in you think maybe it doesn't need to be using that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

like we don't need to pass perpMarketIndex ? 'Isolated' : undefined,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think in line with your other comment, we would just keep perpMarketIndex and only use that, not add a marginType parameter which would be redundant.

sdk/src/user.ts Outdated
public getHealth(): number {
if (this.isBeingLiquidated()) {
public getHealth(perpMarketIndex?: number): number {
if (this.isBeingLiquidated() && !perpMarketIndex) {
Copy link
Member

Choose a reason for hiding this comment

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

why do you have this line?

Copy link
Member

Choose a reason for hiding this comment

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

we should also discuss if we want to change the isBeingLiquidated implementation

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added that because if we want just the health for an isolated position, then the whole account being liq'd shouldn't affect that. BUT if we change the logic of isBeingLIquidated maybe we would remove this then.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

leaving this right now since we need to discuss more specifically what we want to do

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We discussed and I've implemented something based on that.

sdk/src/user.ts Outdated
return {
perpLiabilityValue: perpLiability,
perpPnl: positionUnrealizedPnl,
spotAssetValue: perpPosition.isolatedPositionScaledBalance,
Copy link
Member

Choose a reason for hiding this comment

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

need to convert this to token amount

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added this. I think you want it converted to token value though right? Like multiplied by the price?

sdk/src/user.ts Outdated
}

public canBeLiquidated(): {
public canBeLiquidated(perpMarketIndex?: number): {
Copy link
Member

Choose a reason for hiding this comment

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

I think this fn should just return all the markets that can be liquidated instead of the caller needing to pass in a perp market index param

Copy link
Collaborator Author

@LukasDeco LukasDeco Sep 2, 2025

Choose a reason for hiding this comment

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

Would the return be like this?

 {
		canBeLiquidated: boolean;
		marginRequirement: BN;
		totalCollateral: BN;
		isolatedPositions: Array<{
                     canBeLiquidated: boolean;
		        marginRequirement: BN;
		        totalCollateral: BN}>;
	       }>;
}

@LukasDeco LukasDeco force-pushed the lukas/isolated-positions-sdk branch from 9933deb to 89272cc Compare September 2, 2025 03:41
@LukasDeco LukasDeco force-pushed the lukas/isolated-positions-sdk branch from 4e2952c to a9224f6 Compare September 2, 2025 20:57
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