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

Updating registry #66

Merged
merged 9 commits into from
May 29, 2024
Merged

Updating registry #66

merged 9 commits into from
May 29, 2024

Conversation

Zen-Maxi
Copy link
Collaborator

No description provided.

@Zen-Maxi Zen-Maxi requested a review from franzns May 17, 2024 00:51
@Zen-Maxi
Copy link
Collaborator Author

Based on this sheet, did not add any from Fantom or below 500 USD TVL. https://docs.google.com/spreadsheets/d/1I7OsWcO__6B73Nsuukb3DErnDa5IjjKEaWwqAQiMVuE/edit#gid=0

Some rate providers are actually totally wrong and unsafe. Maybe better we dont make a habit of adding those; so new users cannot use or see them. Other option would be add every rate provider ever even if unsafe or wrong; to me that sounds bloated.

@mkflow27
Copy link
Collaborator

@Zen-Maxi I think unsafe rate providers should not ever be added to this registry. Only scenario I see to have an unsafe Rate Provider in here is if it was previously safe and a change in circumstances made it unsafe.

@franzns
Copy link
Contributor

franzns commented May 17, 2024

We should not add wrong ones.

How the API/UI will handle this is that all on this list would be flagged as a warning and the UI would disable deposit actions on the pool

@franzns
Copy link
Contributor

franzns commented May 17, 2024

@Zen-Maxi I think unsafe rate providers should not ever be added to this registry. Only scenario I see to have an unsafe Rate Provider in here is if it was previously safe and a change in circumstances made it unsafe.

Isnt that the case for the reth rateprovider on arb?

@mkflow27
Copy link
Collaborator

@franzns afaik this is the case for the reth on arbitrum, yes. So adding it as part of this pr is fine. However I think the jAURA and Bitfrost ones never were used really. Let's wait for @Zen-Maxi to clarify.

@Zen-Maxi
Copy link
Collaborator Author

I would rather not include the unsafe rate providers unless they went from safe to unsafe like you guys have signalled above. As for the unused / old ones it may be best to leave them out entirely. A handful of fixes and recent Chainlink additions are in this PR but removing the ones that were never safe is also fine by me if we all agree.

@mkflow27
Copy link
Collaborator

@Zen-Maxi Sure, if you could remove all the unsafe rateProviders that were not "safe" once and resolve conflicts I will review.

Removed unsafe, was only jAura (not wrapped) and bitfrost vETH in wrong direction (decreasing). Rest are either safe, or were and are now unsafe.
Does the order of addition matter?
@franzns
Copy link
Contributor

franzns commented May 18, 2024

Sounds good to me!

Copy link
Collaborator

@mkflow27 mkflow27 left a comment

Choose a reason for hiding this comment

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

Thanks @Zen-Maxi

@mkflow27 mkflow27 merged commit 09ed2c3 into main May 29, 2024
1 check passed
@mkflow27 mkflow27 deleted the Updating-Registry branch May 29, 2024 09:43
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

Successfully merging this pull request may close these issues.

3 participants