cowprotocol / dune-bridge

Other
4 stars 3 forks source link

AppData V4 & Quote V2 #12

Closed bh2smith closed 2 years ago

bh2smith commented 2 years ago

This change really is only for the version bump of Quote to Quote V2.

We take the approach suggested here https://github.com/cowprotocol/dune-bridge/pull/11/files#r905932280

Note that there was an option to include buy and sell amounts in the v2 struct, but then all of buy, sell and slippage would have to be optional and then additional messy validation would need to be done during deserialization. Instead we take the approach without needing any Optional fields where v1 contains the the old version with buy and sell amounts both neing not optional. There is no constraint on the version number for either type, so when serializing both types are accepted and all wind up being a Quote anyway. This is carefully noted in this test itself.

Thanks to @MartinquaXD for reminding me how to rust.

In this particular case, I am not actually sure why appData version needed to be bumped.

Test Plan:

New unit tests.

bh2smith commented 2 years ago

As far as I recall we were usually unpacking json by key name (which I assume will be there). If it's an array we would "unnest" the results. So I think we will have to include the verbose json format (including null values when they are unavailable) however I can double check and see if it would return null when the key doesn't exist.

bh2smith commented 2 years ago

@josojo This query demonstrates what happens when parsing JSON and the key is not available (check the "environment" field)

https://dune.com/queries/279621

Screenshot 2022-07-02 at 9 30 11 AM

Specifically these two results:

Screenshot 2022-07-02 at 9 32 39 AM