ExWeb3 / ex_keccak

Elixir library for computing Keccak SHA3-256 hashes using a NIF built tiny-keccak Rust crate.
Apache License 2.0
25 stars 20 forks source link

Rustler precompiled fails to verify NIF checksum #35

Closed inaniyants closed 1 year ago

inaniyants commented 1 year ago

Hello and thanks for delivering such useful update as precompiled NIFs!

It looks like 0.7.0 release on hex contains wrong checksums file. You can see, that some dependent packages are failing to update to 0.7.0 , like in this PR https://github.com/poanetwork/ex_abi/pull/122

I have few suggestions: 1) add checksums file to .gitignore, cause it doesn't need to be tracked in repo 2) add release script , which will ensure update of checksums before publish to hex. It can be simple Makefile

release:
    mix rustler_precompiled.download ExKeccak --all --print
    mix hex.publish --yes

then just make release.

In such way won't be needed to commit checksums on each release and won't forget to update it before publishing to hex.

inaniyants commented 1 year ago

And one side question. It is still stated in Readme, that "ex_keccak requires Rust to be installed."

I believe with precompiled NIFs it is not required for library users, since rust code is already compiled. Am I correct ?

tzumby commented 1 year ago

Hey @inaniyants,

Thanks for opening this and pointing the problem out! I missed that line in the README.md, I'll update, you definitely don't need Rust unless you want to force compile.

Let me do some digging here on the checksum. I definitely think publishing to hex would be nice as part of the action.

tzumby commented 1 year ago

It looks like the authors of the rustler-precompiled did intend for everyone to check-in the checksum file in the repos. That's what I saw in all the libraries that I checked as well. The mismatch is a problem for sure, I'm going to figure out a solution to make sure this doesn't happen in the future.

inaniyants commented 1 year ago

the authors of the rustler-precompiled did intend for everyone to check-in the checksum file in the repos'

I think there it is meant to include file into hex repo, but not into source code repo.

Here is ref, where I originally found recommendation to not include it to source code repo https://github.com/philss/rustler_precompiled/blob/main/PRECOMPILATION_GUIDE.md

Note: you don't need to track the checksum file in your version control system (git or other).

inaniyants commented 1 year ago

I definitely think publishing to hex would be nice as part of the action.

Probably this will help https://hex.pm/docs/publish#publishing-from-ci but your hex key should be somewhere stored . Does github provide this storage for secret keys ?

tzumby commented 1 year ago

I just pushed a 0.7.1 update with the right checksums, do you mind re-running your CI with the version bump to see if that fixes it ?

inaniyants commented 1 year ago

Yeah, new build is ok https://github.com/poanetwork/ex_abi/pull/123