feat: implement Pedersen hash function

[xJonathanLEI]

Resolves #15.

[milancermak]

Why is the repo called starkware-crypto-rs but the crate is starkware-crypto-sys?

[xJonathanLEI]

Why is the repo called starkware-crypto-rs but the crate is starkware-crypto-sys?

The naming convention in Rust is to append -sys to FFI binding crates. As an example, rust-openssl's crate is named openssl-sys.

[milancermak]

Sorry for the delay here. I got a new computer and I'm currently waiting for it to download some dev packages to see if I can build this branch, but at the moment, I'm at a place with a rather slow connection so it takes a while 😔

[xJonathanLEI]

Haha no rush.

[milancermak]

So I can't get this to build because I'm on a M1 Mac and getting the CPP dependency to compile is hell. What system are you running btw?

I don't want my problems to block the merge, but there's a significant number of folks in the dev community who will have the same setup and will run into the same problem. There's value in having a native Rust version of the Pedersen hash. It looks to me we can use the one from pathfinder or am I mistaken? I'm sure if we asked them, they'd publish a standalone package for easy integration.

[milancermak]

Also, @maxgillett, I got the impression you're pretty knowledgeable about cryptography, do you have any inputs on the above?

[xJonathanLEI]

I was building on Linux so didn't get any issue (CI works well too). But I got an M1 MacBook too, so lemme try on it later today to see if I can get it to work.

And yes, a native Rust implementation would be super helpful. I couldn't do it because I'm totally not a cryptographer myself and I respect the "don't roll your own crypto" rule.

As for pathfinder, as noted in the Telegram group, they do have a Pedersen hash implementation, but:

• there are no ECDSA functionalities, which we really need for the signer
• initial benchmark shows that their hash function is 10x slower than the crypto-cpp wrapper

The 2nd item is probably less relevant but the lack of ECDSA is the deal breaker here IMO.

But yes input from @maxgillett would be great.

[xJonathanLEI]

Looks like there's a conflict now. Let me rebase this branch.

[xJonathanLEI]

@milancermak You're right. A default installation of Rust and Xcode command line tools doesn't seem to compile this branch. The exact error I'm getting is:

  cargo:warning=lib/starkware-crypto/src/starkware/crypto/ffi/utils.cc:1:10: fatal error: 'endian.h' file not found
  cargo:warning=#include <endian.h>
  cargo:warning=         ^~~~~~~~~~
  cargo:warning=1 error generated.
  exit status: 1
  exit status: 0
  exit status: 0
  exit status: 0
  exit status: 0
  exit status: 0
  exit status: 0

I'm looking into how this can be resolved. Once we figure it out we can add it to README for troubleshooting.

(I feel like this is not an M1 specific issue, but macOS in general. Let me do some more tests).

[xJonathanLEI]

OK I just tried using GitHub Actions to build the branch and got the exact same error. I can confirm this is a macOS issue, not specific to M1.

It's now tracked upstream here. We need to wait until the upstream issue is fixed before we can proceed with this PR.

In the meantime, I think it's a good chance for us to add macOS to our CI pipelines for running tests.

[milancermak]

I got the exact same error (no surprise 😄). Got different one when I tried CXXFLAGS=-I/Library/Developer/CommandLineTools/SDKs/MacOSX11.1.sdk/usr/include/arm cargo build but didn't get much further than that.

I 100% agree with the "don't roll your own crypto" adage, but we don't have to. ECDSA is a standard and a lib we can just use. In my view, the Pedersen hash was the hard part. At least from my understanding, pluging it in shouldn't be that hard (famous last words, right? 🤣 ).

I can try and put together a PoC, but unfortunately my day is very fragmented today, I think I'll be able to get to it only late in the evening.

In the mean time, I've created issue #23 to add different platforms to the test pipeline.

[xJonathanLEI]

Yep. I also tried building on elliptic-curve but didn't get very far. The deepest I've ever gone with crypto stuff was building on top of predefined curves haha.

I guess we can still try porting stuff over from the JavaScript library. Meanwhile, I'll also continue to look into the upstream compilation issue.

[maxgillett]

Hi all, I created a pull request in the crypto-cpp repo that fixes the macOS compilation issue. Funny enough, I realized when I went to submit the request that @lastagile had already opened and closed a solution to this problem (without merging), but it looks like their solution breaks Linux compatibility.

[xJonathanLEI]

@maxgillett Great job! Let's push them to merge it (as it seems the repo isn't really monitored lol).

[maxgillett]

Regarding reimplementation of the Pedersen hash in Rust, I'm not knowledgable enough to add anything meaningful to what's already been discussed. I would be in favor of using the FFI bindings for now and opening a lower priority issue to switch both this component and the ECDSA signing over to Rust.

[xJonathanLEI]

The upstream issue on macOS is now fixed thanks to @maxgillett's PR. So the bindings now works on both Linux and macOS.

However... I just tried on my Windows box and it seems that it won't compile there lol. I'm looking into it right now...

[xJonathanLEI]

Windows compilation issue tracked here.

[milancermak]

Just how many dev machines do you have @xJonathanLEI 😆

Nice work all btw 💪

[xJonathanLEI]

Lolll I got a few devices to test stuff on.

I've published starkware-crypto-sys v0.1.3 after fixing macOS and Windows (GNU) compilation issues. This branch has also been updated to use v0.1.3 instead. So it should work everywhere EXCEPT Windows (MSVC).

So this is still not good enough, since MSVC is the default installation option for Windows.

[xJonathanLEI]

Superseded by #33.