-
Notifications
You must be signed in to change notification settings - Fork 35
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
Multi oracles with varying nb digits #186
base: master
Are you sure you want to change the base?
Multi oracles with varying nb digits #186
Conversation
9598e4d
to
a42dafb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't reviewed your Rust code but here are my thoughts on the text.
One thing that I think (?) is still missing here is covering out-of-bounds cases where all of the greater nb_digits
oracles agree on some value greater than the max for the contract (in which case the max should be executed).
The above described algorithms all assume that all oracles use the same number of digits to represent numerical event outcomes. | ||
However that might not always be the case and dealing with oracles with varying number of digits require further care. | ||
|
||
When a contract is created with multiple oracles with varying number of digits, the contract should only define payouts for outcomes up to the maximum value that can be attested by the oracle with the minimum number of digits (thereafter referred as `min_nb_digits`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably worth a note that 2^min_nb_digits - 1
represents that value or greater (as usual)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a note under this (it was also referred already in the "out of bound outcomes" section but I guess it doesn't hurt repeating).
MultiOracle.md
Outdated
#### Equal outcomes | ||
|
||
When no difference is allowed between the oracles, one simply needs to "pre-pad" the outcome prefixes of oracles with larger number of digits with `0`s. | ||
For example, for the prefix `1010`, if an oracle uses `min_nb_digits + 2` digits, the prefix `001010` should be used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the prefix here is 00
not 001010
so I think the wording could be improved a little by saying "the prefix 00
should be used yielding 001010
."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmmm so 00
is the prefix of the prefix somehow. Because 1010
is already a prefix for 1010xxxx...
, or at least that's what I was trying to convey. I updated let me know if it's clearer.
MultiOracle.md
Outdated
|
||
#### Bounded error | ||
|
||
If differences are allowed between oracles, the [same algorithm](#Algorithm) defined for oracles with equal number of digits should be used, but it should be ran with `min_nb_digits + 1`, and setting the primary oracle to be the first one in the group with `min_nb_digits`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be preferred oracle with min_nb_digits
rather than first (I think)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added that it should be first in order of preference, let me know if that's clearer.
MultiOracle.md
Outdated
The list of prefixes should then be transformed so that: | ||
- the prefixes for oracles with more than `min_nb_digits + 1` should be "pre-padded" with zeros to reach the proper length, | ||
- the first `0` should be dropped for the prefixes for oracles with `min_nb_digits` | ||
- any group of prefix that contains a prefix representing an outcome that cannot be attested to by the corresponding oracle should be discarded (e.g. in the previous example, if a group of prefix contains a prefix that starts with a `1` for oracle `D` it should be discarded). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the terminology for "group of prefix" here should be something like "if an outcome consists of oracle D
attesting to a value greater than 2^min_nb_digits - 1
then that outcome should be discarded"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree that's clearer, replaced with your wording.
MultiOracle.md
Outdated
|
||
In the case of oracles with equal number of digits, if the event outcome is greater than what oracles can attest to, the contract can still be closed if all oracles attest to the maximum value (which is [the convention](./Oracle.md#Digit-Decomposition)). | ||
To enable contracts to be closed if the event outcome is greater than what can be represented using `min_nb_digits` in the case of oracles with varying number of digits, all possible outcomes above are assigned to the maximum contract payout. | ||
To cover all the possible outcomes, all the possible combinations of prefixes that cover the interval `[base^min_nb_digits + 1; base^max_nb_digits]` are generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know what it is but max_nb_digits
hasn't been defined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point added
MultiOracle.md
Outdated
|
||
In the case of oracles with equal number of digits, if the event outcome is greater than what oracles can attest to, the contract can still be closed if all oracles attest to the maximum value (which is [the convention](./Oracle.md#Digit-Decomposition)). | ||
To enable contracts to be closed if the event outcome is greater than what can be represented using `min_nb_digits` in the case of oracles with varying number of digits, all possible outcomes above are assigned to the maximum contract payout. | ||
To cover all the possible outcomes, all the possible combinations of prefixes that cover the interval `[base^min_nb_digits + 1; base^max_nb_digits]` are generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the +1
is needed on the first entry of the interval
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point fixed
d31def7
to
8e088b1
Compare
Added a paragraph talking about the threshold case (which I think is what you were referring to?). Tbh I cut a bit the algo that I use in practice and that includes generating the combinations for thresholds to make it a bit simpler, but I can replace if you think it'd be clearer. For reference the actual algo is here: https://github.com/p2pderivatives/rust-dlc/blob/feature/multi-oracle-digit-diff/dlc-trie/src/utils.rs#L51 |
Note: test vector will need to be updated with renaming "leftEndPoint" to "endPoint" |
8e088b1
to
56b751c
Compare
Since this has not received any review on 3 months I will go ahead and merge my implementation in rust-dlc. I don't think there is any required change at this point, but I will probably be less keen on accepting change suggestions. |
56b751c
to
33680ff
Compare
33680ff
to
afa0f0c
Compare
If differences are allowed between oracles, the [same algorithm](#Algorithm) defined for oracles with equal number of digits should be used, but it should be ran with `min_nb_digits + 1`, and setting the primary oracle to be the first one (in order of preference) in the group with `min_nb_digits`. | ||
For example, if the list of oracles is `[A, B, C, D]` with respective number of digits `[12, 10, 11, 10]`, the algorithm should be run with the order `[B, A, C, D]`. | ||
The list of prefixes should then be transformed so that: | ||
- the prefixes for oracles with more than `min_nb_digits + 1` should be "pre-padded" with zeros to reach the proper length, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it min_nb_digits + 1
here instead of min_nb_digits
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the point is that you want to allow the contract to close when e.g. oracle B
and C
are at their maximum value, while C
and D
are slightly above (within the specified limit). So to enable that you need to have one more digit than min_nb_digits
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, thanks!
|
||
#### Bounded error | ||
|
||
If differences are allowed between oracles, the [same algorithm](#Algorithm) defined for oracles with equal number of digits should be used, but it should be ran with `min_nb_digits + 1`, and setting the primary oracle to be the first one (in order of preference) in the group with `min_nb_digits`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't this algorithm be ran with min_nb_digits
if the resulting digit-prefixes are padded with zeros when needed in the case of secondary oracles with higher number of digits? Why run it with min_nb_digits + 1
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same reason as above
MultiOracle.md
Outdated
|
||
In the case of oracles with equal number of digits, if the event outcome is greater than what oracles can attest to, the contract can still be closed if all oracles attest to the maximum value (which is [the convention](./Oracle.md#Digit-Decomposition)). | ||
To enable contracts to be closed if the event outcome is greater than what can be represented using `min_nb_digits` in the case of oracles with varying number of digits, all possible outcomes above are assigned to the maximum contract payout. | ||
To cover all the possible outcomes, all the possible combinations of prefixes that cover the interval `[base^min_nb_digits; base^max_nb_digits]` are generated, where `max_nb_digits` if the number of digits used by the oracle(s) with greatest `nb_digits`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're talking about outcomes that are greater than what can be represented using min_nb_digits
(strictly greater than base^min_nb_digits - 1) and smaller than or equal to what can be represented using max_nb_digits
(smaller than or equal to base^max_nb_digits - 1
), shouldn't this interval be: [base^min_nb_digits; base^max_nb_digits - 1]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right updated.
``` | ||
1, 1111111111, 1, 1111111111 | ||
01, 1111111111, 1, 1111111111 | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the example above we have: [base^min_nb_digits; base^max_nb_digits - 1] == [2^10; 2^12 - 1] == [10000000000; 111111111111]
Is it correct to say that this interval can be represented the following 2 digit prefixes (where _
stands for any digit): _1_ _ _ _ _ _ _ _ _ _
and 1_ _ _ _ _ _ _ _ _ _ _
?
Sorry I don't understand the notation above, is it equivalent to this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not exactly equivalent because in what your wrote the prefixes would have overlapping intervals. It would be more like 1 _ _ _ _ _ _ _ _ _ _ _
and 0 1 _ _ _ _ _ _ _ _ _ _
. Does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that's right, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at how to interpret the notation
[
[1, 1111111111, 1, 1111111111]
[01, 1111111111, 1, 1111111111]
]
when it comes to the describing the list of digit prefixes that cover the interval [2^10; 2^12 - 1] == [10000000000; 111111111111]
:
It seems like any of those sequences of digits at index i
should be interpreted as representing the first digits of a digit prefix of length nb_digits[i]
(where, in this case, nb_digits == [12, 10, 11, 10]
)
Padding those with _
on the right (_
representing any digit) to end up with a digit prefix of length nb_digits[i]
, then padding with 0
on the left so that all digit prefixes have the max length (12 in this case), and removing duplicates, we end up with:
1 _ _ _ _ _ _ _ _ _ _ _, 01_ _ _ _ _ _ _ _ _ _, 001111111111
Does it make sense to have 001111111111
here since 001111111111
does not belong to the [2^10; 2^12 - 1] == [10000000000; 111111111111]
interval?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand what you're saying, but the point here is that oracles B
and D
can only represent 10 digits numbers, so their maximum value is 2^10 - 1
. If the event outcome is above that, and oracles A
and C
attest to values greater than that, you still want to be able to close the contract, so this algorithm is covering the interval 2^10; 2^12 - 1
for oracle A
and 2^10; 2^11 - 1
for oracle C
, but B
and D
keep their max value (since anyway they can't attest to value greater than that). Does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that part is clear thanks!
And it is also clear to me now why the list of prefixes to cover this interval [2^10; 2^12 - 1]
does include 1111111111
which is outside the interval, as 1111111111 == 2^min_nb_digits - 1
represents that value or greater for oracles B and D.
MultiOracle.md
Outdated
.map(|x| *x - min_nb_digits) | ||
.collect::<Vec<_>>(); | ||
// Counter for the extra length of prefixes of each oracle. 0 if the oracle | ||
// has nb_digit, starts at 1 for others since at the minimum we want to generate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this comment say 0 if the oracle has min_nb_digit
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes updated
afa0f0c
to
0c9976e
Compare
0c9976e
to
73fcf67
Compare
This PR updates the multi oracle specification to deal with multi oracles that use different number of digits to represent event outcomes. It is largely based off #168 description by @nkohen. I put it on top of #163 because I added test vectors that require the changes from it to work (so the actual changes relevant for this PR are only in the last commit).
Some things that can be discussed:
0
s in the extra covering paths rather than omitting them as suggested here, mainly because it is a bit simpler to implement but also because from what I can tell the extra additions make almost no difference. It's possible that it could be a bigger problem for libraries that don't use native code though.Implemented here: p2pderivatives/rust-dlc#34