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

Update 0x303-I3-Oracle.md #11

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

CDSecurity
Copy link

Added 7 security considerations related to Chainlink Oracle as well as a link to the article which explains these problems more in depth.

Added 7 security considerations related to Chainlink Oracle as well as a link to the article which explains these problems more in depth.
2.0/0x300-Integrations/0x303-I3-Oracle.md Outdated Show resolved Hide resolved
2.0/0x300-Integrations/0x303-I3-Oracle.md Show resolved Hide resolved
2.0/0x300-Integrations/0x303-I3-Oracle.md Show resolved Hide resolved
2.0/0x300-Integrations/0x303-I3-Oracle.md Show resolved Hide resolved
damianrusinek added a commit to damianrusinek/SCSVS that referenced this pull request Sep 8, 2023
| **I3.9** | Verify that NOT the same heartbeat is used for multiple price feeds when using Chainlink Oracle |
| **I3.10** | Verify that the code deals with different price feeds having different decimal precision when using Chainlink Oracle |
| **I3.11** | Verify that the price feed address wherever it is located(hardcoded, deployment script) is pointing to the correct oracle price feed |
| **I3.12** | Verify that the code handles calls to the oracle if they potentially revert when using Chainlink Oracle |
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mean a situation when call to price feed reverts? If so, what is the risk? The whole tx would revert right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @CDSecurity, just a quick reminder. Can you elaborate on that?

@damianrusinek
Copy link
Contributor

Thanks @CDSecurity for update. I have addressed your comments.

We are also thinking about how to present the checks for specific protocols (like Chainlink). One option is to separate them as I proposed here: https://github.com/ComposableSecurity/SCSVS/pull/12/files, but that is not yet decided.

I am mentioning that because if we decide to separate them, these check will also have to be re-organized.

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.

2 participants