Skip to content

Conversation

@miktam
Copy link

@miktam miktam commented May 31, 2018

Fixes #16

🚀 Description

Introduces ability to designate the oracle, with ability to fund the contract to be pulled by oracle if data was submitted according to the predefined rules: frequency of the update and amount of updates.

Flow:

  1. owner creates a contract, designating the oracle, minimum and maximum frequency of the updates, amount of data to be updated, and the reward
  2. owner funds the contract, and activates it, allowing oracle to start updating the data.
    3a) if oracle followed frequency rules, it could claim the reward
    3b) if oracle missed the maximum frequency (was late), owner could claim funds back
  • 📘 I've reviewed the OpenZeppelin Contributor Guidelines
  • ✅ I've added tests where applicable to test my new functionality.
  • 📖 I've made sure that my contracts are well-documented.
  • 🎨 I've run the JS/Solidity linters and fixed any issues (npm run lint:all:fix).

@nventuro nventuro added feature New contracts, functions, or helpers. contracts Smart contract code. labels Jul 28, 2018
@nventuro
Copy link
Contributor

nventuro commented Jul 28, 2018

Hi @miktam, thanks for your contribution! I think this is not receiving enough attention because the code may be a bit complex and unintuitive (an Oracle contract holding an oracleStorage struct, with an oracle member variable, variables with the frequency suffix that have time units, etc.).

I have two suggestions for you (feel free to follow either!):

  1. We could split this PR in smaller chunks (e.g. a simpler Oracle with no frequency restrictions that we'd add stuff on top of later) so that it's easier for people to review/comment on, or
  2. We could open a discussion issue to arrive at a design we agree on, and then do a follow-up PR (or even continue work on this one) to implement said design.

What do you think?

@miktam
Copy link
Author

miktam commented Jul 28, 2018

@nventuro, thank you for the feedback!

let's try with the splitting it to the smaller chunks (p1), so we could test ideas from practical perspective faster.

@nventuro
Copy link
Contributor

Awesome! I look forward to reviewing those :)

@nventuro nventuro added the on hold Put on hold for some reason that must be specified in a comment. label Jul 29, 2018
@nventuro nventuro self-assigned this Jul 29, 2018
@nventuro nventuro removed the on hold Put on hold for some reason that must be specified in a comment. label Jul 30, 2018
@cag
Copy link
Contributor

cag commented Aug 2, 2018

I'd just like to remark that there is a draft EIP about oracles being discussed over at ethereum/EIPs#1161

@nventuro
Copy link
Contributor

nventuro commented Aug 3, 2018

Thanks for letting us know @cag! Could you provide us with a rough estimate on how long you think that EIP might take until it's finalized?

Not sure how I feel about this PR now that I know that discussion is taking place: the contract proposed here is a bit more complex than the minimal EIP interface (e.g. it adds rewards). @frangio @shrugs care to share thoughts on this?

@shrugs
Copy link
Contributor

shrugs commented Aug 3, 2018

I would prefer that we implement as base contract that conforms to the eventual standard, as well as any enhancements via inheritance/composability.

@miktam
Copy link
Author

miktam commented Aug 4, 2018

@shrugs, @nventuro thank you for the feedback!

what would you recommend - to wait till standard will be finalized, or to start implementing it at the proposal stage?

@shrugs
Copy link
Contributor

shrugs commented Aug 4, 2018

@miktam good question, it's always a hard choice. If you don't think the interface will deviate much, I'd say go for it, otherwise wait for it to settle. Usually there just ends up being one or two small issues holding an EIP back, so you can make a judgement call on it and start implementing, eventually updating it.

But totally up to you :)

@miktam
Copy link
Author

miktam commented Aug 10, 2018

@cag could you please take a look if current implementation covers the draft version of EIP 1154?
also, how would you recommend to check:

receiveResult MAY revert if the id or result cannot be handled by the handler?

@cag
Copy link
Contributor

cag commented Aug 14, 2018

@miktam Hey, thanks for considering the EIP! As you may be aware, the proposal's still pretty unstable. I hope this is fine for you, as you may have to make some updates. For example, there is an additional metadata parameter being discussed right now.


I want to note two changes that will affect this PR:

  1. OracleHandler is being renamed to OracleConsumer
  2. The type of result is being changed from bytes32 to bytes

how would you recommend to check:

receiveResult MAY revert if the id or result cannot be handled by the handler

That statement means whether or not a report is acceptable is up to the OracleConsumer. Basically, there's no one-size-fits-all way of checking that. A contract which implements OracleConsumer can revert during receiveResult if what that contract expects for an id or result is not what it receives.

For example, lets say you have some sort of ConditionalPaymentProcessor which is an OracleConsumer. There are conditions which this payment processor may recognize given some id, and there are, let's say, two values which makes sense to this ConditionalPaymentProcessor: 0 and 1. Then, in this example, the ConditionalPaymentProcessor would revert if it received an id which does not correspond to some condition the contract has on file, or if the value of result parsed to neither 0 nor 1.

@miktam
Copy link
Author

miktam commented Aug 14, 2018

@cag great, thank you for the update, it will be reflected in the PR!

Changed result from bytes32 to bytes
@nventuro nventuro mentioned this pull request Aug 30, 2018
4 tasks
@ross-rosario
Copy link
Contributor

It seems there's an implementation for @cag's EIP here => https://github.com/levelkdev/tidbit. Shall we still aim to introduce same oracle interfaces to Open-Zeppelin too?

@nventuro
Copy link
Contributor

I think them being in tidbit makes sense, unless @cwhinfrey would rather us merge it here and handle maintenance?

@nventuro nventuro closed this Feb 28, 2019
@nventuro nventuro reopened this Feb 28, 2019
@cwhinfrey
Copy link
Contributor

@nventuro Either one works for me. We are actively maintaining the repo and are aiming to have some of the oracles deployed as an EVM package on ZOS soon. We are also considering adopting a simpler interface and having a contract that adds the EIP 1154 functions on top of the simpler interface.

@nventuro
Copy link
Contributor

I'll leave it up to you guys to decide then! Closing this for now, but feel free to open a separate PR to bring those contracts here :)

@nventuro nventuro closed this Feb 28, 2019
@nventuro nventuro mentioned this pull request Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contracts Smart contract code. feature New contracts, functions, or helpers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Better oracle interfaces

7 participants