Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Oracle should use the TimestampedValue from orml_traits #904

Open
lemunozm opened this issue Apr 19, 2023 · 4 comments
Open

Oracle should use the TimestampedValue from orml_traits #904

lemunozm opened this issue Apr 19, 2023 · 4 comments

Comments

@lemunozm
Copy link
Contributor

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.

@xlc
Copy link
Member

xlc commented Apr 19, 2023

Yeah that's likely caused by a bad refactor

@lemunozm
Copy link
Contributor Author

lemunozm commented Jun 2, 2023

@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.

@xlc
Copy link
Member

xlc commented Jun 2, 2023

Please do!

@lemunozm
Copy link
Contributor Author

lemunozm commented Jun 5, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants