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

WIP: Ethereum oracles #1644

Merged
merged 22 commits into from
Jul 17, 2023
Merged

WIP: Ethereum oracles #1644

merged 22 commits into from
Jul 17, 2023

Conversation

barnabee
Copy link
Member

@barnabee barnabee commented Mar 29, 2023

close #881 .

@gordsport gordsport changed the base branch from master to cosmicelevator May 31, 2023 12:25
protocol/0000-ETHD-ethereum-data-source.md Outdated Show resolved Hide resolved
protocol/0000-ETHD-ethereum-data-source.md Outdated Show resolved Hide resolved
protocol/0000-ETHD-ethereum-data-source.md Outdated Show resolved Hide resolved
protocol/0000-ETHD-ethereum-data-source.md Outdated Show resolved Hide resolved
protocol/0000-ETHD-ethereum-data-source.md Outdated Show resolved Hide resolved
@AnExsomnis AnExsomnis self-requested a review May 31, 2023 12:53
ettec
ettec previously approved these changes May 31, 2023
Copy link
Contributor

@ettec ettec left a comment

Choose a reason for hiding this comment

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

Fine for now

protocol/0081-ETHD-ethereum-data-source.md Outdated Show resolved Hide resolved
protocol/0081-ETHD-ethereum-data-source.md Outdated Show resolved Hide resolved
protocol/0081-ETHD-ethereum-data-source.md Outdated Show resolved Hide resolved
protocol/0081-ETHD-ethereum-data-source.md Outdated Show resolved Hide resolved
@AnExsomnis AnExsomnis self-requested a review June 21, 2023 12:09
AnExsomnis
AnExsomnis previously approved these changes Jun 21, 2023
@AnExsomnis AnExsomnis self-requested a review July 13, 2023 10:12
AnExsomnis
AnExsomnis previously approved these changes Jul 13, 2023
Copy link
Member Author

@barnabee barnabee left a comment

Choose a reason for hiding this comment

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

Some questions/comments on the changes to this. I can only "Comment" not "Request changes" so I've done that.

Would appreciate some guidance @gordsport on how finished the ACs should be. They seem high level/unfinished in places.

Also the changes/ACs appear to suggest additional or different functionality to that implied and required purely by the spec and existing overriding data sourcing principles that are currently expected to apply to all data sources, for instance around tracking active data sources.

3. Create more than spam.protection.max.proposals oracle data source proposals in an epoch - proposal rejected with error message (<a name="0081-ETHD-003" href="#0081-ETHD-003">0081-ETHD-003</a>)
4. Create ethereum oracles based on calling a read method of a smart contract by supplying incorrect ABI (Phase 2 - oracle based on listening for events) (<a name="0081-ETHD-004" href="#0081-ETHD-004">0081-ETHD-004</a>)
1. Create ethereum oracles based on calling a read method of a smart contract (Phase 2 - oracle based on listening for events) (<a name="0082-ETHD-001" href="#0082-ETHD-001">0082-ETHD-001</a>)
2. All current governance rules that apply to propose / submit / vote on a proposal should be applicable for the ethereum oracle data source creation / amendment (<a name="0082-ETHD-002" href="#0082-ETHD-002">0082-ETHD-002</a>)
Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a specific data source creation proposal or is this talking about proposals such as market creation/amendment that introduce a new Eth oracle?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the governance path that creates/updates a market is the intended way to handle this kind of data source, in the same way as the other ones.

1. Ability to delete a data source if and only if the data source is NOT used by any active markets (<a name="0081-ETHD-008" href="#0081-ETHD-008">0081-ETHD-008</a>)
2. Should NOT be able to delete a data source if its being actively used by a market (<a name="0081-ETHD-009" href="#0081-ETHD-009">0081-ETHD-009</a>)
3. If a single data source is used by multiple markets, then should NOT be able to delete the data source even if one of those markets is actively using the data source (<a name="0081-ETHD-010" href="#0081-ETHD-010">0081-ETHD-010</a>)
1. Ability to delete a data source if and only if the data source is NOT used by any active markets (<a name="0082-ETHD-008" href="#0082-ETHD-008">0082-ETHD-008</a>)
Copy link
Member Author

Choose a reason for hiding this comment

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

Can you explain the intended mechanics for creating/deleting data sources if it does not happen automatically when they are first reference (create) and no longer reference (delete)? If it is automatic I think the AC could be made clearer.

Copy link
Contributor

Choose a reason for hiding this comment

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

The intended mechanism to deactivate and delete a data source is the one we have at the moment.
Seems a little rewording will help, I'll suggest changes.

1. Validate if the smart contract address is valid (<a name="0082-ETHD-011" href="#0082-ETHD-011">0082-ETHD-011</a>)
2. Validate if the data elements of the oracle data source is valid - e.g. source for a value that's returned as boolean but have a filter / condition for greater than 0 (<a name="0082-ETHD-012" href="#0082-ETHD-012">0082-ETHD-012</a>)
3. Validations for min / max frequency of listening for events / read a smart contract (<a name="0082-ETHD-013" href="#0082-ETHD-013">0082-ETHD-013</a>)
4. Create a new market with an inactive external oracle data source, system should throw an error (<a name="0082-ETHD-014" href="#0082-ETHD-014">0082-ETHD-014</a>)
Copy link
Member Author

Choose a reason for hiding this comment

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

What does this mean? If the contract address, ABI, etc. are valid why would it error?

2. Validate if the data elements of the oracle data source is valid - e.g. source for a value that's returned as boolean but have a filter / condition for greater than 0 (<a name="0082-ETHD-012" href="#0082-ETHD-012">0082-ETHD-012</a>)
3. Validations for min / max frequency of listening for events / read a smart contract (<a name="0082-ETHD-013" href="#0082-ETHD-013">0082-ETHD-013</a>)
4. Create a new market with an inactive external oracle data source, system should throw an error (<a name="0082-ETHD-014" href="#0082-ETHD-014">0082-ETHD-014</a>)
5. Validations to be applied - need to be expanded (<a name="0082-ETHD-015" href="#0082-ETHD-015">0082-ETHD-015</a>)
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this an unfinished AC?

1. Test min / max values / validations for any new network parameters that are added (<a name="0081-ETHD-017" href="#0081-ETHD-017">0081-ETHD-017</a>)
2. Test the impact / behaviour of the system, after the changes to the new network parameters are enacted (<a name="0081-ETHD-018" href="#0081-ETHD-018">0081-ETHD-018</a>)
1. Test min / max values / validations for any new network parameters that are added (<a name="0082-ETHD-017" href="#0082-ETHD-017">0082-ETHD-017</a>)
2. Test the impact / behaviour of the system, after the changes to the new network parameters are enacted (<a name="0082-ETHD-018" href="#0082-ETHD-018">0082-ETHD-018</a>)
Copy link
Member Author

Choose a reason for hiding this comment

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

I assume this (and probably the previous AC) would need to be expanded to multiple ACs?

4. If an oracle data source is inactive - then any events / any data received from that oracle data source is NOT processed (<a name="0082-ETHD-030" href="#0082-ETHD-030">0082-ETHD-030</a>)
5. SPAM rules if any defined should be tested for (<a name="0082-ETHD-031" href="#0082-ETHD-031">0082-ETHD-031</a>)
6. NOT all data sourced should be stored on chain - invalid / incorrect data is filtered out and is NOT processed / stored on chain - understand what the rules are and design the AC's / test accordingly (<a name="0082-ETHD-032" href="#0082-ETHD-032">0082-ETHD-032</a>)
7. Any active data sources that aren't used by any markets should not source data until they are being actively used by a market (<a name="0082-ETHD-033" href="#0082-ETHD-033">0082-ETHD-033</a>)
Copy link
Member Author

Choose a reason for hiding this comment

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

What is the difference between "active" and "actively [used by a market/referenced by something within Vega core]? Why does this difference exist?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this point should be removed, there is no such difference that it is implying

2. Create a market to use an internal data source to terminate a market and an ethereum oracle to settle the market (<a name="0081-ETHD-035" href="#0081-ETHD-035">0081-ETHD-035</a>)
3. Create a market to use an external data source to terminate a market and an internal / manual oracle to settle the market (<a name="0081-ETHD-036" href="#0081-ETHD-036">0081-ETHD-036</a>)
4. Data sourcing should be completely decoupled from data filtering (<a name="0081-ETHD-037" href="#0081-ETHD-037">0081-ETHD-037</a>)
1. It should be possible to use only ethereum oracle data sources or internal data sources or use a combination of both types of oracles (<a name="0082-ETHD-034" href="#0082-ETHD-034">0082-ETHD-034</a>)
Copy link
Member Author

Choose a reason for hiding this comment

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

Clarify. I assume this means e.g. using one type for trading termination and the other for settlement, the equivalent for perps?

@AnExsomnis
Copy link
Contributor

Hello @barnabee , I addressed your comments and did some rewording in this PR #1834
It is opened from this branch.

AnExsomnis
AnExsomnis previously approved these changes Jul 17, 2023
@AnExsomnis AnExsomnis merged commit b74df11 into cosmicelevator Jul 17, 2023
4 checks passed
@AnExsomnis AnExsomnis deleted the feat-ethereum-data-source branch July 17, 2023 15:06
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.

Specification for Ethereum Oracles
5 participants