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

Make _openTrove and _adjustTrove revert when new oracle failure detected #357

Merged
merged 7 commits into from
Aug 28, 2024

Conversation

RickGriff
Copy link
Collaborator

@RickGriff RickGriff commented Aug 23, 2024

Closes #336.

_openTrove and _adjustTrove check whether an oracle failed, and revert if so.

@RickGriff RickGriff requested a review from bingen August 24, 2024 13:31
@RickGriff RickGriff marked this pull request as ready for review August 24, 2024 13:32
@RickGriff RickGriff changed the title Make openTrove and adjustTrove revert when new oracle failure detected Make _openTrove and _adjustTrove revert when new oracle failure detected Aug 24, 2024
Copy link
Collaborator

@bingen bingen left a comment

Choose a reason for hiding this comment

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

Nice!
It seems the new Chanlink mock oracle contract throws some errors. It would nice to get rid of them.

@@ -1499,6 +1506,12 @@ contract BorrowerOperations is LiquityBase, AddRemoveManagers, IBorrowerOperatio
}
}

function _requireOraclesLive(bool _newOracleFailureDetected) internal view {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn’t this pure?

if (priceFeedDisabled) return lastGoodPrice;
// fetchPrice returns:
// - The price
// - A bool indicating whether a new oracle failure was detected in the call
Copy link
Collaborator

Choose a reason for hiding this comment

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

So, if the failure is not new (i.e., the oracle and the branch were already down), this would return false, right?
(or at least that seems to happen in the first line of this function)
Why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right. The intent is to only detect new failures in this call, so that openTrove and adjustTrove revert if they are called in the transaction where the oracle first fails.

If the oracle failed in a previous fetchPrice call, the branch will already be shut down and using the lastGoodPrice

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, yes, of course. Thanks for clarifying!

@RickGriff RickGriff merged commit a448493 into main Aug 28, 2024
5 of 7 checks passed
@danielattilasimon danielattilasimon deleted the dedaub_12 branch January 19, 2025 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants