crypto-cpp icon indicating copy to clipboard operation
crypto-cpp copied to clipboard

SignEcdsa is too slow

Open lastagile opened this issue 4 years ago • 23 comments

SignEcdsa takes couple seconds, any way to make it quicker?

lastagile avatar Sep 15 '21 11:09 lastagile

Hello! Have you found the solution for this?

alexberdreal avatar Sep 23 '21 12:09 alexberdreal

Same issue here. Calling Sign() takes more than 7 seconds on my machine.

xJonathanLEI avatar Jan 10 '22 09:01 xJonathanLEI

It seems that turning on -O3 helps. Signing time drops from 7 seconds to 180 milliseconds on my machine. It's apparently still not ideal, but better than nothing.

xJonathanLEI avatar Jan 10 '22 09:01 xJonathanLEI

was this ever solved?

rbdm-qnt avatar Mar 10 '23 18:03 rbdm-qnt

@rbdm-qnt Nope but (shameless plug) if you use Rust then you can use starknet-crypto lol.

xJonathanLEI avatar Mar 11 '23 06:03 xJonathanLEI

@rbdm-qnt Nope but (shameless plug) if you use Rust then you can use starknet-crypto lol.

And is that one faster?

rbdm-qnt avatar Mar 11 '23 22:03 rbdm-qnt

Yeah it is. See the linked page for benchmark.

xJonathanLEI avatar Mar 12 '23 04:03 xJonathanLEI

Yeah it is. See the linked page for benchmark.

1ms or less vs 7 seconds/180 ms? I don't understand, there must be something terribly wrong with the c++ version? Even the python version takes 70-80ms only. Unfortunately I don't use Rust, my application is in C++ and I need a high performance version for that language

rbdm-qnt avatar Mar 12 '23 11:03 rbdm-qnt

Well you can use the Rust lib from C++ as well. You'll need to create a wrapper crate that exposes the C api but it's possible.

Honestly that's likely your best bet imo, as this repo doesn't seem to be maintained.

xJonathanLEI avatar Mar 12 '23 11:03 xJonathanLEI

Well you can use the Rust lib from C++ as well. You'll need to create a wrapper crate that exposes the C api but it's possible.

Honestly that's likely your best bet imo, as this repo doesn't seem to be maintained.

Wouldn't that have a big overhead? Never done that

rbdm-qnt avatar Mar 12 '23 14:03 rbdm-qnt

Wouldn't that have a big overhead? Never done that

Nope it's just like linking to any other library. Rust does not have a runtime. Might be helpful:

  • https://parallel-rust-cpp.github.io/cpp_abi.html
  • https://stackoverflow.com/questions/69418715/calling-rust-from-c-using-cxx-bridge

xJonathanLEI avatar Mar 12 '23 15:03 xJonathanLEI

Wouldn't that have a big overhead? Never done that

Nope it's just like linking to any other library. Rust does not have a runtime. Might be helpful:

* https://parallel-rust-cpp.github.io/cpp_abi.html

* https://stackoverflow.com/questions/69418715/calling-rust-from-c-using-cxx-bridge

Thanks, I'll try it! Would you be interested in providing a C++ bind in the lib? Also any more documentation on how to install/operate the lib would be greatly appreciated (especially for a Rust noob :))

rbdm-qnt avatar Mar 13 '23 00:03 rbdm-qnt

I would love to have such instructions implemented as an example (specifically with cxx). Created an issue to track it: https://github.com/xJonathanLEI/starknet-rs/issues/325

However I'm currently too busy to put it together. Would be awesome if someone can help. Or if you make it work, feel free to drop a PR for that. Thanks!

xJonathanLEI avatar Mar 13 '23 03:03 xJonathanLEI

I would love to have such instructions implemented as an example (specifically with cxx). Created an issue to track it: xJonathanLEI/starknet-rs#325

However I'm currently too busy to put it together. Would be awesome if someone can help. Or if you make it work, feel free to drop a PR for that. Thanks!

Could you please at least provide an example of how to use the Rust code itself? I'm trying to understand how to create some C bindings, but can't figure out these Rust types like FieldElement and Fp256Parameters.

rbdm-qnt avatar Mar 14 '23 02:03 rbdm-qnt

@rbdm-qnt https://github.com/xJonathanLEI/starknet-rs/pull/328

xJonathanLEI avatar Mar 14 '23 16:03 xJonathanLEI

@rbdm-qnt xJonathanLEI/starknet-rs#328

I remade the whole thing in C++ from scratch. I thought the 180ms latency reported must have been a mistake. I followed python's implementation (which takes 75ms) and remade it in cpp17, and it takes around 100ms. I think the problem is possibly the handling of GMP types, or maybe some bottleneck in cryptopp algorithms, but it's quite puzzling. I can't understand how's its possible for Rust to run this in 1ms.

Anyway, now I really must understand how to use your implementation in my C++ application. Please, if you could provide any basic example of usage of the Sign method (even just in Rust, doesn't have to be in C++) it would save me a lot of time, cos I'm a total Rust noob

rbdm-qnt avatar Mar 15 '23 01:03 rbdm-qnt

Please, if you could provide any basic example of usage of the Sign method

Hey man my last comment was exactly just that. It's a CMake example:

https://github.com/xJonathanLEI/starknet-rs/tree/master/examples/starknet-cxx

xJonathanLEI avatar Mar 15 '23 02:03 xJonathanLEI

Please, if you could provide any basic example of usage of the Sign method

Hey man my last comment was exactly just that. It's a CMake example:

https://github.com/xJonathanLEI/starknet-rs/tree/master/examples/starknet-cxx

Thank you, that's great! Is this file autogenerated? Cause I can't find it in the directories "starknet_cxx_bridge/lib.h"

rbdm-qnt avatar Mar 15 '23 02:03 rbdm-qnt

I can't find it in the directories "starknet_cxx_bridge/lib.h"

Yes it's generated by cxx. It should live somewhere inside the build folder once you build it. Pretty sure you can just go into definition if you use an LSP. (Not 100% sure cuz I wrote the example without an LSP)

xJonathanLEI avatar Mar 15 '23 03:03 xJonathanLEI

I can't find it in the directories "starknet_cxx_bridge/lib.h"

Yes it's generated by cxx. It should live somewhere inside the build folder once you build it. Pretty sure you can just go into definition if you use an LSP. (Not 100% sure cuz I wrote the example without an LSP)

I was able to make the example work and integrate it into my codebase with some work, thank you! I have another question, why does the Sign method return 1 string? Canonically, ecdsa_sign methods return r and s values, so 2 strings, as far as I know

EDIT: looked into the rust code, it was just a formatting issue, i modified the lib.rs binding to return the individual elements of the signature with a delimiter in between them

rbdm-qnt avatar Mar 16 '23 13:03 rbdm-qnt

Yep yep it was just to make the bindings example simpler. Glad you figured it out! It's great that you found the lib helpful!

xJonathanLEI avatar Mar 16 '23 15:03 xJonathanLEI

Yep yep it was just to make the bindings example simpler. Glad you figured it out! It's great that you found the lib helpful!

I'm having trouble getting valid values for ecsda_signature; in the python version I'm able to generate rfc6979 key with an empty seed value (in bytes b"") or values like "\x01" or above. In Rust these values error out, as well as an empty string, and I have to use "0x1" or above. I'm thinking this might be the reason why the signatures are invalid? For instance, I'm generating a signature with the same exact values in both Python and C++/Rust, and then verifying the outputs

EDIT: one of the things I noticed is that, if I regenerate ecsda_signature, in python both r and s are always different, while in Rust r is always the same and s is different; i tried verifying either of them or a mix of r from python and s from rust and anything coming out of Rust seems invalid

EDIT 2: the issue seems to be in the ecdsa_sign method itself, im 99% sure its about the k arg. it most probably gets treated differently than in python. Could the problem be that I'm trying to compare this Rust Sign method with a RFC6979 compliant signature? Is it not? And if so, which method implements RFC6979 signature?

EDIT 3: I think my best bet is bridging over the generate_k method from rust, use that to generate k and give it to the sign method? I'm trying to add it to the lib.rs file but no luck compiling it so far, it says the method can't be found in starknet_crypto

EDIT 4: That was correct, i was able to expose it using and it matches python. Now in the final output, R matches python, S doesn't, the verify still fails, but i'm getting closer.

CONCLUSION: i ended up reimplementing the sign method in c++ following Starkware Python implementations as i couldn't make sense of this one, and couldn't get it to verify. It was still useful, but now i also understand the reason behind the speed difference: most of the latency is in this step

rbdm-qnt avatar Mar 16 '23 18:03 rbdm-qnt

@rbdm-qnt Hi. When using correctly all the Rust methods have been tested to generate exactly the same output as their Python counterparts. I've also verified that using ecdsa_sign from C++ gives the exact same output as in Rust. Everything should be fine. The lib is used in many places for the cryptographic stuff so it should work.

You mentioned an empty string doesn't work. That's because of how the wrapper itself (lib.rs) handles input. Everything gets converted into FieldElement at the end of the day and starknet-crypto only every deals with FieldElements, not strings. On the wrapper level, the parsing logic doesn't treat an empty string as a valid value while the Python version does (according to you; I haven't verified this).

I'm not sure how Python treats the empty string. Maybe it treats it as a 0 value? In that case, you can should be able to input 0x0 to the wrapper and it should parse correctly. Of course, this builds on the assumption that Python treats it as zero.

In any case I'm happy you made it work, but if you still consider it a bug after reading this comment (maybe I misunderstood something in your text), please file a bug under starknet-rs. Thanks. Let's not continue spamming the crypto-cpp folks lol

xJonathanLEI avatar Mar 17 '23 01:03 xJonathanLEI