frontend-monorepo icon indicating copy to clipboard operation
frontend-monorepo copied to clipboard

Published 20 hours ago • verified publisher icon vegaprotocol

Change diff library to better handle arrays

Open JonRay15 opened this issue 1 year ago • 4 comments

The Chore

This diff

image

Doesnt seem to match this diff in the governance

image

Tasks

  • [ ]
  • [ ]

Additional details / background info

JonRay15 avatar Feb 21 '24 13:02 JonRay15

@JonRay15 can we please get the id of the proposal in question here in order to re-validate the claim? (after my previous amendments for batch propposals)

asiaznik avatar Feb 27 '24 10:02 asiaznik

Yeh so if you look at this template JSON diff at the BTC part.

https://github.com/vegaprotocol/governance-templates/pull/17/files#diff-04b4d4340dd52fae1d55e98b2fcb942d9c4d54bd2e23777a71be3cf7a9f34e6a

And compare to the mainnet proposal diff here: https://governance.vega.xyz/proposals/59d09095ea6334e19437ab8dde340dc2a89ed6aed4eed181be98b5ffd0c992bd

You can see the issue... you can also view the full current config for that market here: https://vega-data.nodes.guru:3008/api/v2/market/4e9081e20e9e81f3e747d42cb0c9b8826454df01899e6027a22e771e19cc79fc

I suspect this is because I dont know if the price monitoring array is ordered or not. I dont think it is and I think that just logically they are applied in ascending order of the horizon.

So what this update was doing concepttually was:

  • Increasing the auction extension duration of all existing monitoring bounds
  • Adding one more especially cautious bound (the 86400 duration one)

So you can actually see on the JSON diff on github that the horizons of the existing ones have not changed (360, 1440, 4320) ... but the governance app thinks they have since its diffing them against the wrong one (4320>21600, 1440>4320, 360>1440).

I suspect there are two ways to play this:

  1. Always order price bounds by horizon before applying the diff
  2. If there is any change to price monitoring just treat it as a delete all / replace all and red out ALL bounds and show ALL new ones in green

JonRay15 avatar Feb 28 '24 16:02 JonRay15

I've time boxed this to 2h today and from my initial research I believe the best course of action would be to change the diff library to something more configurable.

I've found this https://github.com/rexskz/json-diff-kit to display the proposal diffs in a nice way.

asiaznik avatar Mar 07 '24 12:03 asiaznik

Park it until we see one change

JonRay15 avatar Mar 20 '24 16:03 JonRay15