plume-sig / zk-nullifier-sig Goto Github PK
View Code? Open in Web Editor NEWImplementation of PLUME: nullifier friendly signature scheme on ECDSA
License: MIT License
Implementation of PLUME: nullifier friendly signature scheme on ECDSA
License: MIT License
It might be a RustCrypto thing which just panics on certain methods when bytes yields the value larger than
It's kind of #72 (comment), but letting moving forward meanwhile.
I added a test which should be showing that hashing to curve isn't correct (at least on signing; before #84 ). As there's many conversions and different formats/endings the test isn't super readable, so it's quite probable I just messed up along the way. This is quite easy to check if anybody would add a hash_to_curve
test which a current implementation would pass.
Anyway this/such test
is a good contribution to the code. Mine was born when my implementation yielded a signature (V1) different from the current one using the same inputs which correctness I double-checked. (Yep, it's quite probable I messed up implementation and that test; it's enough to have just two errors for this. On the other hand it's god enough signal to dig into this.)
@Divide-By-0 , how much care should nullifier
receive compared with the secret key, and the random nonce (they receive different care, ofc)?
Just to add, there is a quick way to prove the PLUME security, in case anyone asks.
This is the GDH undeniable signature with the confirmation protocol replaced with NIZK, and here the NIZK is a classical Chaum-Pedersen protocol.
The GDH undeniable signature can be found here: Tatsuaki Okamoto and David Pointcheval. The gap-problems: A new class of problems for the security of cryptographic schemes. In PKC โ01
This is also discussed in the BLS signature paper. Section 2.2.
Uniqueness comes from hashing to the curve.
And that in turn blocks #43 .
See geometryxyz/ark-secp256k1#1 (comment).
Circom 2.1 allows using <==
operator for signal arrays (documentation). You can avoid writing loops just to assign inputs to a component.
For example:
-for (var i = 0; i < k; i++) {
- g_pow_s.privkey[i] <== s[i];
-}
+g_pow_s.privkey <== s
Variant should be joint/embedded with struct PlumeSignature
(aka Signature
) to avoid failing verifications when with a correct signature instance a wrong variant indicator being input.
Prover is free to choose any value forsha256_preimage_bit_length
to generate the sha256 hash for c
. Since the length of the hash pre-image is fixed, it can be removed as an input signal. The TODO
comment indicates this too:
// ...TODO: calculate internally in circom to simplify API
signal input sha256_preimage_bit_length;
Recommendation: sha256_preimage_bit_length
should always be equal to message_bits
(6*33*8
).
Context: verify_nullifier.circom#L214-L219, verify_nullifier.circom#L72
Public key is compressed twice which results in more constraints than required.
Recommendation: Use the compressed public key for calculating the hash for c
, although it makes the code complicated.
Current used DST is taken from the example, so it will collide with other projects which didn't introduce their own DST, which defies its purpose. I see it as quite a priority since if it won't be done until people start to use the thing it will be quite painful to change as it's not backward compatible in no way.
On the other hand there's quite an amount of tests alignment and small modifications due to change of the DST to proper one. Though not a horrific amount.
So the choice is better be done once and never changed, so it worth to put some effort in it. While it should be done in reasonable time. Obvious step is to start the string with "PLUME", and maybe it would be enough to be aligned with usual string, maybe some more information is nice to put there. Too long string bring small burden, btw.
PS Sorry for brevity, am writing in hurry; hope to edit for better readability, or clarify anything in discussion.
It's yet named SecretKey
, oncoming PR renames it to not give false impression of security.
here arkworks
should be checked for proper methods for keys handling
at least zeroize
should be applied
also would be nice to check that proper RNG is enforced
Currently, running serially tends to fail. Run all the tests in parallel instead.
Seems like <./rust-k256> designed only for verification of a signature, as there's no ready outlet to pub
which could sign a message. Should it be so, or does it just lacking it?
There's substantial number of different improvements since "0.3.0" which is used now. I guess it'd not only generally improve execution, but also might enable simplification of some current helpers introduced in the code.
I am recently reading into the PLUME design and the use of Geometry's map. Note that Geometry's map was designed for a different purpose. If we target at making a nullifier, there is at least one trick that we can use.
Note that for provable indifferentiability, Geometry's map does hashing to curves twice and adds the results together. This is not necessary if we use the signature as the nullifier because you don't need to reveal the signature (see #11, I am referring to the GDH undeniable signature, the signature part), and to hide the signature, just hash the final result.
This ideally should be able to reduce the cost by half (given that clearing the cofactor is not necessary for secp256k1).
I totally trust you just trying to understand.
Deterministic as "to know c
beforehand of signing"; or is it just the fact that V2 wouldn't work since it lacks the point
I mean I kind of don't see yet the essential difference if signer adds to the message point
Originally posted by @skaunov in #98 (comment)
https://github.com/plume-sig/zk-nullifier-sig/blob/main/rust-k256/src/lib.rs#L35 for easy readability.
It's important to have consistency across these impls. A few examples:
c
is returned as a plain number in js, circom vs a field element in rust.You should run a differential test on random values for message and private key (maybe pre-generate and freeze them).
I didn't check, but it seems to me that cases when c
is zero, or EC points are at infinity aren't covered by tests, and in couple of recent PRs such tests would be helpful. I guess there also maybe such important inputs which would be good to test.
(Super cool would be to have a file of test vectors.)
Circom and js use sha256.
Rust implementations use sha512.
So these implementations are not compatible.
I suggest passing all these implementations through a common test to ensure consistency.
https://github.com/miracl/core/blob/master/javascript/examples/node/TestHTP.js#L37 for https://github.com/zk-nullifier-sig/zk-nullifier-sig/blob/main/javascript/src/utils/hashToCurve.ts#L33
hashToPairing()
function is taken from htp()
function in TestHTP.js
. Best to add this information in comments. Just makes the life of the reader easier. You can add commit info to the source link.
Tests in rust_k256 use the same function while producing test data as the function they test. So actually they test wrapping (which is useful too), but c_sha256_vec_signal
is left untested in fact.
https://github.com/plume-sig/zk-nullifier-sig/blob/main/rust-k256/src/lib.rs#L429-L435
encode_pt()
is described as:
/// Format a ProjectivePoint to 64 bytes - the concatenation of the x and y values. We use 64
/// bytes instead of SEC1 encoding as our arkworks secp256k1 implementation doesn't support SEC1
/// encoding yet.
However, it converts a ProjectivePoint
to its compressed form of 33 bytes.
I'm not sure if this is the right repo for this issue, but in the demo app (https://ethbogota-2022.netlify.app/) I see the following error when trying to connect Metamask Flask.
"The method "wallet_enable" does not exist / is not available."
Context: verify_nullifier.circom#L166
Description: The following comment is incorrect as what's being calculated is a * ((b^c)^-1)
// Calculates a^s * (b^c)-1
Recommendation: Correct the comment.
The crate is quite complex and I fail to see any reason behind it. It offers a complex trait, but its type system requires further development to be really useful. (And error handling isn't well-compatible with other crates.) It's very generic over curves and its generators, but the only curve other implementations support is deprecated, and I can't find a case when another curve would be plugged into it.
I well might be just fail to see the big idea though. @weijiekoh , could you explain these since I struggled to pick-up the reasons from code but it doesn't add up well in my mind. =(
My current proposal is to ditch the defined trait, simplify few places, and decide on the level of generality. If we don't expect no other curves for PLUME in foreseeable future then further simplify to make it concrete, if do expect then [try] to preserve this part.
[try]: deprecation of the curve definition makes this more a grounding and reference work (than just use
) until the whole thing isn't maintained to be compatible with current arkworks
version
Both crates basically lacks documentation, and <./rust-k256> lacks API choices.
Also their name
are ...temporary.
PS It would be nice to think about traits used and reexported so that it would not clash with versions that downstream use. Though most likely Cargo will do its job on it, just it's better to be sure.
This one is more about releasing the crates when things will come to it.
At ark crate it's used quite suboptimal now, also see branch:43 for a further suggestion.
I noticed that both c
are produced with SHA-256 in <./javascript>. Is it ok to that crate to do it with SHA-512?
fn encode_pt(point: ProjectivePoint) -> Result<Vec<u8>, Error>
always returns Ok(..)
currently. Should be analyzed to either introduce error handling or made it infallible.
If c
overflows the Curve order, the value returned by Rust impls differ from js and circom versions.
Rust impls return c % Curve.n
, js and circom return the plain c
.
In rust_k256 crate this fn
seems to be fundamental. I'd make it pub
, redo some internals (at least better separate computed and input values), and got rid of println!
which are suitable for a bin
crate, not for a lib
.
Two possible solutions were mentioned. Both require some experience/research to implement.
cargo-test
for ?Is there any specific in that test suit which won't work better in Rust? I guess circom
crate can be just a dev dependency there (instead of external call), which potentially might enable tests parallelization. Also there's small chance of running it in GitHub action that way (though I guess it would hit some bottleneck along the way).
circom-tester
Same values are named differently across implementations. While it's not a big deal for small code size, it helps the reader if they can be named same throughout.
For example: h[m,pk]^r
is called:
Differing upto casing is fine, but arkworks impl variable name deviates the most from others.
As it's known to be leaking some information. At least was using it for the input on verification.
This would provide an expected level of interoperability.
Move circom-helper to devDependencies in https://github.com/geometryresearch/secp256k1_hash_to_curve/blob/main/circuits/package.json
From #29
EDIT: Blocked on approval in geometry research repo.
perhaps non trivial ๐
A point
hash_to_curve.rs#L45-L62 uses 320 bytes for each coordinate. Arkworks uses 40 bytes to store a coordinate.
Recommendation:
Although the test passes, you can use 40 bytes instead of 320:
-let num_field_bytes = 320;
+let num_field_bytes = 40;
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.