search

vegaprotocol / specs

Specs, designs and requirements đŸ¦”
MIT License
7 stars 2 forks source link

Implement the time triggered data source modifier as originally specified #1092

Open davidsiska-vega opened 2 years ago

davidsiska-vega commented 2 years ago

From @VegaJakeg "We have an automated test which is flaky because it uses the "Equals" operator for the termination trigger on a timebound oracle. The issue is that sometimes vega doesn't ever register a time which matches the oracle's specified end time, and so we miss it. I'd argue allowing the equals condition to be used for the termination trigger is a bug (in the spec/vega), since we're allowing people to pick a flaky end condition. (Either that, or we should guarantee to check intervening seconds if ever a block skips several seconds at once). Any thoughts? (I haven't raised a bug for this yet, but if people agree, I will)."

  1. A solution would be that you have to specify a range - though if the range is narrow there is no guarantee we will see a time in the range.
  2. Another solution would be that when core evaluates the trigger it checks that "at some point between the previous evaluation and now we've passed the time specified which implies that the condition was met at some point in the block and the trigger should be activated" and we'll update the spect to make this clear.
barnabee commented 1 year ago

The solution is to use greater than or equals.

We shouldn't check if people do stupid stuff here for the same reason that when we have WASM we don't want to analyse their code and refuse to do things we think might be bad or flaky: we will never catch all bad code and it is not the job of the protocol to do so.

If you use a dumb oracle spec you will hopefully get found out during review. We could even create a market "linter" to flag things like this outside of Vega core. If you don't get found out then, you will need to use governance to fix it.

barnabee commented 1 year ago

The other solution here is to implement the time triggered data source modifier as originally specified and use this, which would be the "best practice" way of triggering an action or data at a time.

The internal timestamp data source can remain (it may be useful in future especially with WASM / Smart Products) but it is not the intended time trigger functionality and it exists only because what was built did not meet the spec. and we decided to live with it for the MVP.

IMO we should definitely not add weird/bespoke validation logic to arbitrary combinations of filters and data sources to paper over the lack of the time trigger modifier.