Add market insurance pool balance to marketData type and APIs #640

Closed ashleyvega closed 4 years ago

In GitLab by @edd on Jan 14, 2020, 11:19

We can query for the current balance of the insurance pool of a market by looking at the accounts on a market. @tamlyn10 is interested in adding a view that shows the recent history of the market insurance pool: https://www.notion.so/vegaprotocol/Insurance-pool-data-cf78c263a33a40e7af9dd2d17d936585

To be able to use this value somewhere we'd need a few things:

Add insurance pool balance as a value on the marketData type
Add a historical insurance pool value that we can use to produce a sparkline, similar to the candle data [future work see #659 ]

Our sparklines in the Futures component: Screenshot_2020-01-14_at_11.14.49

uses the Candles data to generate the 24h charts, so data in that same shape would be great.

In GitLab by @cdm on Jan 14, 2020, 11:26

Two thoughts on this one:

Is that marketData struct the correct place for insurance pool data balance? Do we have a spec for 'marketData' as IIRC it was a bit ugly when it arrived?
A historical insurance pool data API is non-core and we could create a plugin in the future

In GitLab by @edd on Jan 14, 2020, 11:36

We currently get the market account balance from a nested account property on the market. The radionale for adding it to marketData is that we already have a marketData subscription. We should be able to subscribe to a market account with the below query, but I've never seen itwork. Although maybe that's just because the balance has never changed from 0 :thinking:. However if we're keeping the marketData type, I still think it is a logical place for it.
I'm aware it's non core. It's still something we want!

subscription {
  accounts(marketId:"LBXRA65PN4FN5HBWRI2YBCOYDG2PBGYU", type:Insurance ){
    balance
  }

}

In GitLab by @edd on Jan 14, 2020, 11:47

@jeremyletang just corrected me, there is a spec: https://gitlab.com/vega-protocol/product/blob/master/specs/0021-market-data-spec.md

In GitLab by @edd on Jan 14, 2020, 11:48

@tamlyn10 do you think it makes sense to add the insurance account balance here? I think it does - if it's as important as you say. This is just the current balance, so it doesn't satisfy all the current requirements. If it does, I'll add a spec MR.

In GitLab by @jeremyletang on Jan 15, 2020, 19:41

Ok, so if needed we can add it very easily to the market data. It’s another thing to do an historic of it. We can do it indeed, as a plugin it will be a matter of minutes/hours to implement. But that’s not here at the moment

In GitLab by @tamlyn10 on Jan 16, 2020, 11:26

@eddhannay yes adding current balance to market date makes sense to me. While the historical is desirable, I see it as a task for the future... I can add an implement ticket to the product board if that's helpful?

In GitLab by @edd on Jan 16, 2020, 12:41

changed title from GraphQL API: Add market insurance pool data{- to a-} to GraphQL API: Add market insurance pool data

In GitLab by @edd on Jan 16, 2020, 12:41

assigned to @cdm

In GitLab by @edd on Jan 16, 2020, 12:41

@cdm this is now over to you - feel free to break out the historical data to a separate issue.

In GitLab by @cdm on Jan 20, 2020, 17:43

changed title from {-GraphQL API: Add market insurance pool data-} to {+Add market insurance pool balance to marketData type and APIs+}

In GitLab by @cdm on Jan 20, 2020, 17:43

changed the description

In GitLab by @cdm on Jan 20, 2020, 17:47

changed the description

In GitLab by @cdm on Jan 20, 2020, 17:47

Broken historical account balance API PLUGIN ticket out

In GitLab by @cdm on Jan 20, 2020, 17:48

mentioned in merge request !559

In GitLab by @cdm on Jan 20, 2020, 17:48

created merge request !559 to address this issue

In GitLab by @cdm on Jan 20, 2020, 19:39

@eddhannay @tamlyn10 for the task of adding INSURANCE pool account balance value to the marketData struct.

Some questions:

Do you want INSURANCE pool ASSET value in a field too?
How can we cope with the future of multiple assets?
Should we return Account data types here instead of single value fields, could allow point 2)

In GitLab by @cdm on Jan 21, 2020, 15:27

:up: @tamlyn10 @eddhannay

In GitLab by @cdm on Jan 21, 2020, 15:27

If this is not needed for nicenet I can put it down and move on...

In GitLab by @jeremyletang on Jan 21, 2020, 15:35

First thing is already available via the API. Second one (historical data) will be implemented later as a non-core api / plugin.

Closing that as we have an issue covering the historical data feature request already (https://gitlab.com/vega-protocol/trading-core/issues/659)

In GitLab by @jeremyletang on Jan 21, 2020, 15:36

closed

In GitLab by @cdm on Jan 21, 2020, 16:01

Please don't close my ticket :(

This task is about adding this to marketData type, which isn't added yet, and maybe shouldn't be added if we're being pure about this.

@tamlyn10 requested the feature so I'd like to get her opinion...

In GitLab by @cdm on Jan 21, 2020, 16:01

reopened

In GitLab by @jeremyletang on Jan 21, 2020, 16:04

oh fair enough, We were discussing it with @eddhannay here, and it decided it was non-necessary, but I don't mind.

In GitLab by @cdm on Jan 21, 2020, 16:06

@eddhannay @tamlyn10 sounds like there's been some IRL chat to close this ticket (from statements above)

In GitLab by @tamlyn10 on Jan 21, 2020, 17:30

Hey guys, so the historical insurance pool data definitely isn't a priority but would be good to get at some point. I am OK with the single data point as it is now, though my preference is to display it on the top level of the FX Futures window (alongside best bid / offer etc. ).

How you guys choose to implement that, I leave in your capable hands!

In GitLab by @cdm on Jan 21, 2020, 17:46

@eddhannay @jeremyletang any thoughts? I'm perhaps confused about this task now (we're all agreed on the ticket I created about the future historical account plugin though AFAIK)

In GitLab by @cdm on Jan 21, 2020, 17:47

display it on the top level of the FX Futures window

Sounds like this is a UI/Console issue ?

In GitLab by @jeremyletang on Jan 21, 2020, 17:53

Well to me the understanding was:

we already can have the the insurance pool balance through the API on demand, (and even inside the marketData as you can compose the graph as you wich through graphql, or call the expected request for the account you wish)
we need to implement historical data, through a plugin, and we have a ticket for this.

This ticket does not need to be anymore from the core point of view as we have 1), and decided to implement 2) later on. Anything else (fx future windows stuff, etc) is all about how the FE will use the data, and is out of subject.

In GitLab by @jeremyletang on Jan 21, 2020, 17:54

hence me closing that earlier on.

In GitLab by @edd on Jan 21, 2020, 18:07

Yep. I'm taking over here.

This issue can be closed. https://gitlab.com/vega-protocol/client/issues/767 is the ticket to add the current insurance pool balance to the FX Futures table.

When #659 is done we will add a new ticket.

In GitLab by @cdm on Jan 21, 2020, 18:09

@eddhannay @jeremyletang thank you this makes more sense for me now, good to get the background/why it was closed to help the remote team - sorry if this has been a bit of extra back and forward

In GitLab by @cdm on Jan 21, 2020, 18:09

closed

In GitLab by @cdm on Jan 21, 2020, 18:13

I think I just needed the context that I got - after the fact - remote first growing pains eh?

vegaprotocol / vega

Add market insurance pool balance to marketData type and APIs #640