open-runtime-module-library icon indicating copy to clipboard operation
open-runtime-module-library copied to clipboard

Oracle should use the `TimestampedValue` from `orml_traits`

Open lemunozm opened this issue 2 years ago • 4 comments

Hi!

Both orml_oracle and orml_traits define the same TimestampedValue struct. This is not an issue at all. Nevertheless, orml_oracle is using its own TimestampedValue as a requirement in CombineData and DataProviderExtended traits. This means that if from my pallet I want to have a Config parameter with any of these types, I need to have a hard dependency against the orml_oracle because of that TimestampedValue inside of it, when ideally I only need orml_traits as a dependency, losing the power that abstracting by traits gives.

It seems like the TimestampedValue from orml_oracle can be safely removed, and instead, using the one found in orml_traits.

lemunozm avatar Apr 19 '23 07:04 lemunozm

Yeah that's likely caused by a bad refactor

xlc avatar Apr 19 '23 07:04 xlc

@xlc I just remembered this issue, if you want, I can open another PR, taking advance of the changes in the oracles to fix it.

lemunozm avatar Jun 02 '23 05:06 lemunozm

Please do!

xlc avatar Jun 02 '23 22:06 xlc

Go deeper with it. I'm not sure the change is "so" fast-forward.

I feel like if after this change, orml-oracle uses TimestampedValue from orml-traits, then DataProvider should also use that specific TimestampedValue instead of letting it generic.

If not, the only thing we would be doing with this change is "moving" oracle-related implementation to orml-trait which should not be ok.

Having said that, I prefer not to touch anything regarding this now, or maybe only remove the TimestampedValue from orml-traits to avoid confusion with the internal oracle TimestampedValue.

lemunozm avatar Jun 05 '23 08:06 lemunozm