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

fix: add asset price to trade events #9959

Merged
merged 4 commits into from
Nov 2, 2023
Merged

fix: add asset price to trade events #9959

merged 4 commits into from
Nov 2, 2023

Conversation

EVODelavega
Copy link
Contributor

Closes #9952

@EVODelavega
fix: add asset price to trade events
98f39c8 
Signed-off-by: Elias Van Ootegem <[email protected]>
@EVODelavega EVODelavega requested review from candida-d, gordsport and a team as code owners November 2, 2023 12:32
ValentinTrinque
ValentinTrinque previously approved these changes Nov 2, 2023
View reviewed changes
@wwestgarth
Copy link
Contributor

wwestgarth commented Nov 2, 2023 •
edited
Loading

Can confirm this is fixed. Streaming from a local stagnet datanode without the fix we see positions like the below when a trade occurs where we see a unrealisedPnL jump to 1330419265814866953478 which is seen on console:

{"result":{"snapshot":{"positions":[{"marketId":"c1cb93afe500f2ce3f68ab8a083cd440bd15f037fa5a64a1e65be40975b09f4d","partyId":"0f9f5c2ec82694dbe2eedec2565199297b6ccf868a19a8e9a791e2a4d2d21355","openVolume":"-14000","realisedPnl":"-619932335000000000","unrealisedPnl":"-45954185000000000","averageEntryPrice":"9502995","updatedAt":"1698929112341578000","lossSocialisationAmount":"0"}],"lastPage":true}}}
{"result":{"updates":{"positions":[{"marketId":"c1cb93afe500f2ce3f68ab8a083cd440bd15f037fa5a64a1e65be40975b09f4d","partyId":"0f9f5c2ec82694dbe2eedec2565199297b6ccf868a19a8e9a791e2a4d2d21355","openVolume":"-14000","realisedPnl":"-619932335000000000","unrealisedPnl":"-45954185000000000","averageEntryPrice":"9502995","updatedAt":"1698929117696100000","lossSocialisationAmount":"0"}]}}}
{"result":{"updates":{"positions":[{"marketId":"c1cb93afe500f2ce3f68ab8a083cd440bd15f037fa5a64a1e65be40975b09f4d","partyId":"0f9f5c2ec82694dbe2eedec2565199297b6ccf868a19a8e9a791e2a4d2d21355","openVolume":"-15000","realisedPnl":"-619932335000000000","unrealisedPnl":"1330419265814866953478","averageEntryPrice":"9503017","updatedAt":"1698929119640775000","lossSocialisationAmount":"0"}]}}}
{"result":{"updates":{"positions":[{"marketId":"c1cb93afe500f2ce3f68ab8a083cd440bd15f037fa5a64a1e65be40975b09f4d","partyId":"0f9f5c2ec82694dbe2eedec2565199297b6ccf868a19a8e9a791e2a4d2d21355","openVolume":"-15000","realisedPnl":"-619932335000000000","unrealisedPnl":"-45954185000000000","averageEntryPrice":"9503017","updatedAt":"1698929122866875000","lossSocialisationAmount":"0"}]}}}
{"result":{"updates":{"positions":[{"marketId":"c1cb93afe500f2ce3f68ab8a083cd440bd15f037fa5a64a1e65be40975b09f4d","partyId":"0f9f5c2ec82694dbe2eedec2565199297b6ccf868a19a8e9a791e2a4d2d21355","openVolume":"-15000","realisedPnl":"-619932335000000000","unrealisedPnl":"-45954185000000000","averageEntryPrice":"9503017","updatedAt":"1698929127905587000","lossSocialisationAmount":"0"}]}}}

with the fix I see the flicker on console still (its pointing to a non-local data, so confims I've triggered the issue), but my local data-node stream has a nice constant unrealisedPnL:

{"result":{"updates":{"positions":[{"marketId":"c1cb93afe500f2ce3f68ab8a083cd440bd15f037fa5a64a1e65be40975b09f4d","partyId":"0f9f5c2ec82694dbe2eedec2565199297b6ccf868a19a8e9a791e2a4d2d21355","openVolume":"-26000","realisedPnl":"-619932335000000000","unrealisedPnl":"17549884670000000000","averageEntryPrice":"9472709","updatedAt":"1698933174141227000","lossSocialisationAmount":"0"}]}}}
{"result":{"updates":{"positions":[{"marketId":"c1cb93afe500f2ce3f68ab8a083cd440bd15f037fa5a64a1e65be40975b09f4d","partyId":"0f9f5c2ec82694dbe2eedec2565199297b6ccf868a19a8e9a791e2a4d2d21355","openVolume":"-26000","realisedPnl":"-619932335000000000","unrealisedPnl":"17549884670000000000","averageEntryPrice":"9472709","updatedAt":"1698933179176381000","lossSocialisationAmount":"0"}]}}}
{"result":{"updates":{"positions":[{"marketId":"c1cb93afe500f2ce3f68ab8a083cd440bd15f037fa5a64a1e65be40975b09f4d","partyId":"0f9f5c2ec82694dbe2eedec2565199297b6ccf868a19a8e9a791e2a4d2d21355","openVolume":"-26000","realisedPnl":"-619932335000000000","unrealisedPnl":"17549884670000000000","averageEntryPrice":"2207791728459954570","updatedAt":"1698933184271011000","lossSocialisationAmount":"0"}]}}}
{"result":{"updates":{"positions":[{"marketId":"c1cb93afe500f2ce3f68ab8a083cd440bd15f037fa5a64a1e65be40975b09f4d","partyId":"0f9f5c2ec82694dbe2eedec2565199297b6ccf868a19a8e9a791e2a4d2d21355","openVolume":"-26000","realisedPnl":"-619932335000000000","unrealisedPnl":"17549884670000000000","averageEntryPrice":"9472709","updatedAt":"1698933189535701000","lossSocialisationAmount":"0"}]}}}

wwestgarth
wwestgarth previously approved these changes Nov 2, 2023
View reviewed changes
@EVODelavega
fix: code review
6a3c572 
Signed-off-by: Elias Van Ootegem <[email protected]>
@EVODelavega
fix: unit tests
be5ee2c 
Signed-off-by: Elias Van Ootegem <[email protected]>
@EVODelavega
fix: network history
26fd755 
Signed-off-by: Elias Van Ootegem <[email protected]>
@EVODelavega EVODelavega merged commit ea65b29 into develop Nov 2, 2023
16 checks passed
@EVODelavega EVODelavega deleted the 9962-pnl branch November 2, 2023 15:35
Sohill-Patel
Sohill-Patel approved these changes Nov 2, 2023
View reviewed changes
Copy link
Contributor

@Sohill-Patel Sohill-Patel left a comment

Choose a reason for hiding this comment

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

previously approved but rebase unticked

jeremyletang pushed a commit that referenced this pull request Nov 6, 2023
@EVODelavega @jeremyletang
Merge pull request #9959 from vegaprotocol/9962-pnl
80341da 
fix: add asset price to trade events
@gordsport gordsport added the bug label Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sohill-Patel Sohill-Patel Sohill-Patel approved these changes

@candida-d candida-d candida-d approved these changes

@jeremyletang jeremyletang jeremyletang approved these changes

@ValentinTrinque ValentinTrinque ValentinTrinque left review comments

@wwestgarth wwestgarth wwestgarth left review comments

@gordsport gordsport Awaiting requested review from gordsport

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

PnL flicker present on mainnet
7 participants
@EVODelavega @wwestgarth @jeremyletang @ValentinTrinque @candida-d @Sohill-Patel @gordsport