Skip to content
This repository was archived by the owner on May 6, 2025. It is now read-only.

Refactor the interface #26

Merged
merged 2 commits into from
Aug 24, 2022
Merged

Refactor the interface #26

merged 2 commits into from
Aug 24, 2022

Conversation

ali-behjati
Copy link
Contributor

@ali-behjati ali-behjati commented Aug 23, 2022

This change has two purpose:

  1. add priceFeedExists to handle the case when a price does not exist
    in updatePriceIfNecessary.
    try-catch doesn't work on local function call in solidity and we cannot
    catch the revert there.
  2. getValidTimePeriod(): There are some cases that the previous
    getCurrentPrice implementation does not
    cover and it is better to use the latestAvailablePrice.
    However, we need to know the validity period and this is why it is added
    here. It uses the unsafe function to have better revert message if it
    occurs.

This change has two purpose:
1) add priceFeedExists to handle the case when a price does not exist
in `updatePriceIfNecessary`.
try-catch doesn't work on local function call in solidity and we cannot
catch the revert there.
2) getValidTimePeriod(): There are some cases that the previous
getCurrentPrice implementation does not
cover and it is better to use the latestAvailablePrice.
However, we need to know the validity period and this is why it is added
here. It uses the unsafe function to have better revert message if it
occurs.
@ali-behjati ali-behjati requested review from tompntn, jayantk and drozdziak1 and removed request for tompntn and jayantk August 23, 2022 17:44
@ali-behjati ali-behjati requested a review from jayantk August 23, 2022 17:45
Copy link
Contributor

@jayantk jayantk left a comment

Choose a reason for hiding this comment

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

nice

@ali-behjati ali-behjati merged commit 11abe0d into main Aug 24, 2022
@ali-behjati ali-behjati deleted the abehjati/refactor-interface branch August 24, 2022 10:13
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.

2 participants