Add random oracle #32
Conversation
|
I think it's fine to keep them separate but I'd put these two traits in independent subfolders - similar to the ones for the "main" oracle - so that people wanting to depend on only one side of it can add the single dependency (like here). |
|
Thanks for the changes! What do you think about renaming Random -> Randomness? |
contracts/oracle-random/lib.rs
Outdated
| Self::env().emit_event(OwnershipTransferred { | ||
| previous_owner: None, | ||
| new_owner: caller, | ||
| }); | ||
| Self::env().emit_event(UpdaterChanged { | ||
| old: None, | ||
| new: caller, | ||
| }); |
There was a problem hiding this comment.
Just FYI - these events will be difficult to catch with any of the existing indexers (Subsquid or Subquery) as these indexers "subscribe" to AccountId of the contract while this new constructor will emit events for which address? The new one (most probably) but no one yet knows its address as it's just being created.
We can leave it, maybe someone will find a way to use it.
There was a problem hiding this comment.
would it make sense if we keep previous_owner and old same as caller ?
contracts/oracle-random/lib.rs
Outdated
| )] pub struct RandomData { | ||
| randomness: Vec<u8>, | ||
| signature: Vec<u8>, | ||
| previous_signature: Vec<u8>, | ||
| } |
There was a problem hiding this comment.
Can you answer these questions from my previous comment?
- How big can the data/signature be? I.e. how many bytes?
- Why do we need prev_signature?
- Do you expect users of the oracle to do something with signature/prev_signature? If yes, what would that be?
There was a problem hiding this comment.
1 signature size is around 192 bytes
2 prev_signature is not required, I am removing it.
3 signature will be used for auditing purpose to make sure the randomness value is coming from drand only.
There was a problem hiding this comment.
You're not posting signature in the (previous) Oracle, won't those price points not be "audited"?
There was a problem hiding this comment.
Yes, I added one getter to retrieve signature if required
There was a problem hiding this comment.
So the data AND signature will be both 192 bytes each?
There was a problem hiding this comment.
data is 64bytes
| } | ||
|
|
||
| #[ink(message)] | ||
| fn set_random_value(&mut self, round: u64, data: RandomData) { |
There was a problem hiding this comment.
In your docs it says that if the new randomness is for a round that is older than self.latest_round then such update will be rejected. Why did you decide to change the behavior here?
| [req] | ||
| ) | ||
|
|
||
| const txnHash = result.result?.toHex() |
There was a problem hiding this comment.
What happens if this fails? i.e. the transaction isn't finalized/included in a block? Would the whole function return w/ an error and some upper layers retry it?
|
What happens if this fails? i.e. the transaction isn't finalized/included in a block? Would the whole function return w/ an error and some upper layers retry it? If tx fail next round will update all pending rounds Why these two ink(message)s are in a separate impl ? I like the idea of separating ink-related functions (like contructors and messages) from some internal details (_set_random_value) that you're doing here but let's make it consistent - i.e. move code_hash, set_code_hash and new to the previous impl .... previous impl is for Default, do you want me to add code_hash and set_code in getter setter as this cannot be moved in default and aprt from those there are no impl **In the docs, the Solidity contract rejects updates that are for "old rounds": Why are you not doing it here?** In our next version we are going to remove that check, as if somehow feeder updates newer round than that condition will restrict update of older round if it is skipped by any chance. |
RandomOracle contract for storing random values associated with different rounds, which can be updated by the owner of the contract.
this oracles used RandomOracleSetter and RandomOracleGetter traits which has common methods with Price oracle Setter and Getter traits. @deuszx what do you suggest, creating a separate trait or splitting Price oracle traits so that same can be used for this Random oracle