Draft
Conversation
…lock-sync src: dynamic block sync size
FFI: remove all unwraps and asserts, use err ints
FFI: clippy+fmt
…len-out FFI: rm extra fcmp_proof_out_len param
See monero-project/research-lab#142 for context. https://elligator.org/design, a third-party website commenting on/promoting Elligator due to its adoptance by standardization organizations, includes a section on the uniformity of Elligator. They note adding two samples is sufficient to cover the curve, with the relevant paragraph claiming to be feedback from an author (LoupVaillant/elligator@feca3da). An author also published a paper on acheiving uniformity (https://eprint.iacr.org/2020/1513) which notes the sum of two invocations has bias bounded to ~10/sqrt(q). This would suggest a few bits of bias are still technically present, which further argues for this as it'll have _less_ bias than the original (for which we've been primarily discussing the 1 bit of _explicit_ bias). The unbiased hash-to-curve invokes a 64-byte hashing algorithm (Blake2b-512, as in-tree and preferable) to obtain two separate inputs for the traditional Elligator map (`hash[.. 32]` and `hash[32 ..]`). Both are mapped, then the results are summed. Two successive calls to `keccak256` could be done _if done carefully_. Defining the second keccak to have an input of the result of the first hash-to-point would be _critically flawed_. Using a 64-byte hash should be quicker, Keccak-512 doesn't appear to be in-tree, and I prefer Blake2b-512 anyways (which is already in the protocol due to its usage within FCMP++s). Required now for the TUV generators, which use this as their methodology. They arguably don't need to, yet it's good practice and gets this in the codebase now. The existing test that the TUV generators match Rust's prove this C++ function is equivalent to the Rust function. Long-term, we should use this for at least all new key images' generators (which require a collision-resistance property). @jeffro256 has volunteered there. There may be incentive to further change the hash-to-curve. This one attempts to be as minimal as possible, keeping all of the existing hash-to-point. We could instead follow the [IRTF specification](https://www.rfc-editor.org/rfc/rfc9380.html). The primary change would be how the Cryptonote developers used which candidate `u` coordinate was chosen to determine if the resulting `y` coordinate was even/odd. The RFC used which candidate `u` coordinate was chosen to determine the resulting `v` coordinate was even/odd. A Curve25519 implementation of the RFC can be used, if the choice of candidate is recovered from if `v` is even/odd, then the result is mapped to Ed25519 and the sign is fixed? It should also be noted this is likely inefficient, as we sample two Curve25519 points, map them to Ed25519, and sum them. Performing the addition on Curve25519 would avoid one invocation of the map. This is just a further change from the existing code.
Fix compile errors
Author
|
Converted to draft as I can't in good faith recommended this PR due to the multiple sets of patches it depends on. |
Causes build times to minimize due to use of these dependencies at build time.
…9 from curve25519-dalek Specifically, this uses the fiat-crypto backend for curve25519-dalek which has been formally verified. Notably, since we're now resolving to solely a single backend, we could alternatively not use a patch of curve25519-dalek yet add a direct dependency of fiat-crypto to the dalek-ff-group fork to achieve (almost) the same result. Currently, we'll only benefit from curve25519-dalek providing some more tooling around fiat-crypto and switching between the 32- and 64-bit variants. This has been tested on 64-bit platforms. This has yet to be tested on 32-bit platforms.
I originally patched to 0.2.1 by accident.
jeffro256
force-pushed
the
fcmp++-stage
branch
from
c20ec30 to
a579b72
Compare
j-berman
force-pushed
the
fcmp++-stage
branch
from
a579b72 to
64190fe
Compare
jeffro256
force-pushed
the
fcmp++-stage
branch
from
29f12ff to
7bfffa9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on #93.
Uses a commit for
crypto-bigintinstead of a branch specification.Forks
dalek-ff-groupto use aFieldElementtype exposed fromcurve25519-dalek, as proposed in dalek-cryptography/curve25519-dalek#787. This backports those changes onto the 4.x release branch, as required by our MSRV, and adds in a FFI-compatible representation.Additionally, the fork of
curve25519-dalekexplicitly and solely uses thefiatbackend which has been formally verified due to a preference expressed by @j-berman. eece472 provides commentary in that regard.Untested on 32-bit platforms. The CI should be updated to
cdinto the newly addeddalek-ff-groupfolder and runcargo test --all-featureson 32/64-bit platforms.