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

Hacky native price workaround #3278

Merged
merged 13 commits into from
Feb 13, 2025
Merged

Hacky native price workaround #3278

merged 13 commits into from
Feb 13, 2025

Conversation

mstrug
Copy link
Contributor

@mstrug mstrug commented Feb 12, 2025

Description

Implementation of an option to set alternative token price (approximated) for any other native token.
This feature is a workaround for obtaining native price for tokens which are not supported in our currently available sources, and it is safe to use such simple token substitution (like mapping csUSDL to DAI).

Changes

  • Added hash map of the tokens map in CachingNativePriceEstimator, used DashMap for optimal performance (this will be read only hash map)
  • Added code to parse token list from arguments line in price_estimation::Arguments
  • Added tests

How to test

Newly added tests.

@mstrug mstrug marked this pull request as ready for review February 12, 2025 15:29
@mstrug mstrug requested a review from a team as a code owner February 12, 2025 15:29
Copy link
Contributor

@squadgazzz squadgazzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the comments. LGTM.

Copy link
Contributor

@sunce86 sunce86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lg.

@sunce86
Copy link
Contributor

sunce86 commented Feb 13, 2025

I guess we should prepare the infrastructure PR before merging this PR to main.

@mstrug
Copy link
Contributor Author

mstrug commented Feb 13, 2025

I guess we should prepare the infrastructure PR before merging this PR to main.

New cli argument field is not mandatory, it can be not specified at all, so I think it is ok to merge it before infra PR ready.

@squadgazzz
Copy link
Contributor

New cli argument field is not mandatory, it can be not specified at all, so I think it is ok to merge it before infra PR ready.

If that really works, then we are fine to go.

@mstrug mstrug merged commit 7333e77 into main Feb 13, 2025
11 checks passed
@mstrug mstrug deleted the hacky-native-price-workaround branch February 13, 2025 12:49
@github-actions github-actions bot locked and limited conversation to collaborators Feb 13, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants