Test whether is a probable prime for BigInt.

[shove70]

No description provided.

[dlang-bot]

Thanks for your pull request and interest in making D better, @shove70! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + phobos#7051"

[jondegenhardt]

Since this is proposed for a public facing function, I think it is important to document the nature of the probabilistic test. Specifically, that it is the miller-rabin primality test that is being used, and the role the confidence parameter plays in the test.

Once in place, backward compatibility will become a valuable characteristic. For this reason, it might be better to expose the API with a name specific to the miller-rabin primality test rather than the more generic isProbablePrime name. That way alternate tests could be introduced if desired.

[shove70]

Since this is proposed for a public facing function, I think it is important to document the nature of the probabilistic test. Specifically, that it is the miller-rabin primality test that is being used, and the role the confidence parameter plays in the test.

Once in place, backward compatibility will become a valuable characteristic. For this reason, it might be better to expose the API with a name specific to the miller-rabin primality test rather than the more generic isProbablePrime name. That way alternate tests could be introduced if desired.

@jondegenhardt, Thank you very much for your suggestion. From the point of view of the powerful D language standard library, we really need to consider it more fully. I agree with you very much.

There's only one concern: when users use this function, will isProbablePrime be more intuitive, and millerRabinPrimeTest and so on confuse users?

I'll think it over and ask for more guidance.

[jondegenhardt]

There's only one concern: when users use this function, will isProbablePrime be more intuitive, and millerRabinPrimeTest and so on confuse users?

I'll think it over and ask for more guidance.

One possibility might be the approach taken for exposing random number generators in std.random. A "default, suggested" random number generator is provided with a general name (Random). And a number of more specific random number generators are provided, eg. MersenneTwisterEngine. The default generator uses one of these. However, there are no guarantees about backward compatibility, etc. The general version is intended for callers who just want a good outcome without worrying about the details. Callers who need backward compatibility across releases, or who care about the algorithm details use the more specific versions.

If you choose this approach you might consider dropping the confidence parameter from the isProbablePrime API, and leaving this in the millerRabinPrimeTest api.

[shove70]

If you choose this approach you might consider dropping the confidence parameter from the isProbablePrime API, and leaving this in the millerRabinPrimeTest api.

Thand you.
For every N tests, the probability of misjudgment is less than 4 ^(-N). So I think that different users may need different credibility in different use scenarios, and confidence parameters are left to users to control themselves, which may be more flexible.

[jondegenhardt]

If you choose this approach you might consider dropping the confidence parameter from the isProbablePrime API, and leaving this in the millerRabinPrimeTest api.

Thand you.
For every N tests, the probability of misjudgment is less than 4 ^(-N). So I think that different users may need different credibility in different use scenarios, and confidence parameters are left to users to control themselves, which may be more flexible.

Whatever makes sense. I'm not an expert on primality tests. My only reason for wondering about the confidence parameter was wondering if it would generalize to other forms of primality test. For example, assume the first version exposed both a isProbablePrime and millerRabinPrimeTest, with the isProbablePrime calling millerRabinPrimeTest. Later a different primality test is introduced (there are many). If isProbablePrime takes a confidence parameter, then switching it to use a the new lower level primality test requires backward compatibility with the meaning of the confidence parameter being used with millerRabinPrimeTest.

It probably a minor thing though. Whatever test you introduce in isProbablePrime is likely to be the only test for a long time.

[jondegenhardt]

For background - Here's a Perl library supporting a general is_prob_prime test and many specific primality tests: Math::Prime::Util::GMP.

[shove70]

@jondegenhardt,I see what you mean. There are two options:

1. Rename isProbablePrime() to millerRabinPrimeTest() (preferred suggestion)
2. Remove the confidence parameter of isProbablePrime() (because millerRabinPrimeTest requires users to specify the number of tests, so it cannot be deleted)

To sum up, I take your first suggestion.

[shove70]

@n8sh, excuse me, "auto-tester" hangs up for a long time, it seems that there is no work, Is this a code problem I submitted or something else?
Auto-tester Pending-Pass: 9, Pending: 1
Thanks.

[thewilsonator]

IDK why, it should say pending 10.

[n8sh]

I'll give it a push and see if it starts up again.

[shove70]

@thewilsonator, @n8sh, Thanks.

[shove70]

https://issues.dlang.org/show_bug.cgi?id=19997

(In reply to shove from comment #1)

https://github.com/dlang/phobos/blob/
35e9f2f/std/socket.d#L986

Also here, IPv6 can cause exceptions.
std/socket.d does not fully support IPv6, such as GetHostByAddr().

Perfecting std/socket.d to fully support IPv6 is certainly the best, but
it's a bit of a workload.
So it may be a short-term solution to temporarily handle unittest to make it
work properly. Otherwise, auto-tester of PR will be interrupted frequently.

[shove70]

Excuse me, Hope this PR can be approved and merged.

BigInt is not a built-in type. It is a type simulated by using uint as a "digit" and uint[]. Because of this, we can't manipulate its numbers as easily as we can control int, long, etc. BigInt.data.data is this uint[], but BigInt.data is private, and BigInt.data.data is private too.
Fortunately, BigInt provides a way to get digit: T getDigit(T = ulong)(size_t n) const, But there is no way to setDigit() in the opposite way, Similar operations can only be achieved through << and +, This is too consumptive.
This change in PR is what I need when I deal with RSA algorithm. It will help me improve the performance of RSA computing.

[n8sh]

New public symbols need to be approved by @atilaneves or @andralex, so it cannot be merged until one of them looks at it. From a technical perspective this PR seems fine.

[Geod24]

This change in PR is what I need when I deal with RSA algorithm. It will help me improve the performance of RSA computing.

You really, really should not do crypto with BigInt outside of learning projects though, otherwise you're opening yourself up to timing attacks.

It would be great if we had a fixed-size integer type. Sociomantic also had this need (https://github.com/sociomantic-tsunami/ocean/blob/v4.x.x/src/ocean/math/WideUInt.d).
It's probably not in scope of Phobos to provide branchless operations, but having the type defined in Phobos would already be a step forward.

[shove70]

You really, really should not do crypto with BigInt outside of learning projects though, otherwise you're opening yourself up to timing attacks.

It would be great if we had a fixed-size integer type. Sociomantic also had this need (https://github.com/sociomantic-tsunami/ocean/blob/v4.x.x/src/ocean/math/WideUInt.d).
It's probably not in scope of Phobos to provide branchless operations, but having the type defined in Phobos would already be a step forward.

Thanks.
struct WideUInt (size_t N) is a good idea and is suitable for specific situations.
But I don't understand the difference between WideUInt and BigInt for RSA. Can you describe it in detail? Thank you.

[thewilsonator]

WideUInt is fixed in size at compile time, i.e. backed by a static array, so all operations can be made to take the same number of primitive operations and thus the same amount of time. BigInt is dynamically sized, backed by a dynamic array, and thus operations can take a variable amount of time, which is bad because timing attacks.

[shove70]

Oh, I see! Thanks! @thewilsonator @Geod24 @n8sh...
I have to rethink RSA design in my production environment...