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

MOB462 Expose marginUsage from isolated positions #322

Merged
merged 4 commits into from
Apr 30, 2024
Merged

MOB462 Expose marginUsage from isolated positions #322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
04f53b6
MOB462 Expose marginUsage from isolated positions
johnqh Apr 29, 2024
55a4783
childSubaccountNumber is not an object
johnqh Apr 29, 2024
b4d666f
Merge branch 'main' into features/MOB-462-surface-margin-usage
johnqh Apr 29, 2024
ab0b74c
Merge branch 'main' into features/MOB-462-surface-margin-usage
johnqh Apr 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ allprojects {
}

group = "exchange.dydx.abacus"
version = "1.6.50"
version = "1.6.51"

repositories {
google()
Expand Down
208 changes: 94 additions & 114 deletions integration/iOS/Pods/Pods.xcodeproj/project.pbxproj
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions src/commonMain/kotlin/exchange.dydx.abacus/calculator/AccountCalculator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,10 @@ class AccountCalculator(val parser: ParserProtocol, private val useParentSubacco
"freeCollateral",
childSubaccount["freeCollateral"],
)
modifiedChildOpenPosition?.safeSet(
"marginUsage",
childSubaccount["marginUsage"],
)
modifiedChildOpenPosition?.safeSet("equity", childSubaccount["equity"])
modifiedOpenPositions.safeSet(market, modifiedChildOpenPosition)
}
Expand Down
10 changes: 9 additions & 1 deletion src/commonMain/kotlin/exchange.dydx.abacus/output/Account.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ data class SubaccountPosition(
val resources: SubaccountPositionResources,
val childSubaccountNumber: Int?,
val freeCollateral: TradeStatesWithDoubleValues?,
val marginUsage: TradeStatesWithDoubleValues?,
val quoteBalance: TradeStatesWithDoubleValues?, // available for isolated market position
val equity: TradeStatesWithDoubleValues?, // available for isolated market position
) {
Expand Down Expand Up @@ -365,6 +366,11 @@ data class SubaccountPosition(
parser,
parser.asMap(data["freeCollateral"]),
)
val marginUsage = TradeStatesWithDoubleValues.create(
null,
parser,
parser.asMap(data["marginUsage"]),
)
val quoteBalance = TradeStatesWithDoubleValues.create(
null,
parser,
Expand Down Expand Up @@ -398,8 +404,9 @@ data class SubaccountPosition(
existing.buyingPower !== buyingPower ||
existing.liquidationPrice !== liquidationPrice ||
existing.resources !== resources ||
existing.childSubaccountNumber !== childSubaccountNumber ||
existing.childSubaccountNumber != childSubaccountNumber ||
existing.freeCollateral !== freeCollateral ||
existing.marginUsage !== marginUsage ||
Copy link
Contributor

@prashanDYDX prashanDYDX Apr 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Why are we using referential equality here?

Also taking a step back, what is the goal of this if block? Why does returning existing vs a new object matter? Do we need to check fields one by one? Why not just construct the object and use built-in data class equality?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, childSubaccountNumber is not an object so shouldn't be referential equality. Changed.

All others are objects.

Those output objects can be larger hierarchal objects. Creating data object from IMap will be slower since it is not just a single object being created. It is important for abacus to be as fast as possible as we are dealing with three platforms.

When existing object (which enables referential equality) is returned, downstream code uses referential equality to check if additional code gets triggered. Again, this gives us much better performance than returning a data object even if there is no change.

Copy link
Contributor

Choose a reason for hiding this comment

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

i see, i'm curious if this architecture was adopted in response to actual perf problems or if it's more theoretical? it is not a common pattern and sort of fragile with new devs onboarding, i could easily see someone accidentally creating new objects instead of checking against existing.

existing.quoteBalance !== quoteBalance ||
existing.equity !== equity
) {
Expand Down Expand Up @@ -430,6 +437,7 @@ data class SubaccountPosition(
resources,
childSubaccountNumber,
freeCollateral,
marginUsage,
quoteBalance,
equity,
)
Expand Down
3 changes: 3 additions & 0 deletions src/commonTest/kotlin/exchange.dydx.abacus/payload/v4/V4ParentSubaccountTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,9 @@ class V4ParentSubaccountTests : V4BaseTests(true) {
},
"freeCollateral": {
"current": 796.244
},
"marginUsage": {
"current": 0.0397
}
}
},
Expand Down
2 changes: 1 addition & 1 deletion v4_abacus.podspec
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Pod::Spec.new do |spec|
spec.name = 'v4_abacus'
spec.version = '1.6.50'
spec.version = '1.6.51'
spec.homepage = 'https://github.com/dydxprotocol/v4-abacus'
spec.source = { :http=> ''}
spec.authors = ''
Expand Down
Loading