Adds a threshold rule for DARC expressions.

nkcr commented 3 years ago

What this PR does

This PR adds a new expression in DARC to support a threshold of identities. It comes in the following form:

threshold<n/d, id1, id2, ...>

nkcr commented 3 years ago

I found some erronous test cases

* `threshold<x/0,...>` yields a `NaN/Inf` threshold which is accepted

* `threshold<x/y>` (no ids) yields a `NaN/Inf` actual threshold which is accepted

I didn't want to force users to behave correctly. If one wants to use a threshold that doesn't make sense, it is his responsibility, as long as it doesn't make the system panic.

If we want to go that path I would do the following:

A provided threshold of 0/0 is undefined,
A provided threshold of 1/0 will always make the validation fail,
A provided threshold of 0/N will always make the validation pass.

Plus, we always assume that there is at least one identity in the threshold function, by flooring the computed denominator to 1.

Also, you need to check that the denominator is equal to the number of ids.

That's not the idea. I should be able to put a threshold saying "whatever identities there are, I want a threshold of 50%", using 1/2, or 50/100, or whatever fraction.

You should check that numerator <= denominator and that none are zero.

Again, we should let users do stupid things. If the denominator is greater than the numerator, that means the threshold will never be reached. Which is fine.

tharvik commented 3 years ago

I found some erronous test cases
* `threshold<x/0,...>` yields a `NaN/Inf` threshold which is accepted

* `threshold<x/y>` (no ids) yields a `NaN/Inf` actual threshold which is accepted
I didn't want to force users to behave correctly. If one wants to use a threshold that doesn't make sense, it is his responsibility, as long as it doesn't make the system panic.

If we want to go that path

Your call then, I was thinking that cothority was a bit more insisting on a correct format for rules but if we want to allow for potential extensions, go for it.

Also, you need to check that the denominator is equal to the number of ids.

That's not the idea. I should be able to put a threshold saying "whatever identities there are, I want a threshold of 50%", using 1/2, or 50/100, or whatever fraction.

Ho, I see the use case, keep it as it is then.

ineiti commented 3 years ago

If we want to go that path I would do the following:

A provided threshold of 0/0 is undefined, A provided threshold of 1/0 will always make the validation fail, A provided threshold of 0/N will always make the validation pass.

Having undefined behaviour is not what you want in a consensus system... So I propose the partially wrong interpretation (with a nice comment saying that it's partially wrong):