Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add generic witness reader #11

Merged
merged 3 commits into from
Feb 25, 2025
Merged

feat: add generic witness reader #11

merged 3 commits into from
Feb 25, 2025

Conversation

erhant
Copy link
Contributor

@erhant erhant commented Feb 21, 2025

hi! I am trying to read witness files in another project of mine (circomkit-ffi) and I came across your witness reader function. I have made the following changes:

  • a bit of docs added / refactored
  • have the witness parser take a chunk-mapper argument as well, of type impl Fn(&[u8]) -> T
  • implement parse_witness_to_bigints by simply calling the generic function with chunk-mapper as BigInt::from_le_bytes
  • use match instead of if-else; maybe opinionated but I think it looks more clear here and especially fixes the hidden case of "section 1 pos mutations happen within the else branch of section_id == 2 check" case

with this generic function we are able to read raw witness files into other backends like:

  • Arkworks: parse_witness_to_elems(&wtns_data, F::from_le_bytes_mod_order)
  • Lambdaworks: parse_witness_to_elems(&wtns_data, |bytes| FrElement::from_bytes_le(bytes).unwrap())

@erhant
Copy link
Contributor Author

erhant commented Feb 22, 2025

changes added! 🙏🏻

@alxkzmn alxkzmn requested a review from vivianjeng February 25, 2025 03:06
Copy link
Contributor

@vivianjeng vivianjeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution!
It looks good to me

Could you please format the code with

cargo fmt --all

Thank you

@erhant
Copy link
Contributor Author

erhant commented Feb 25, 2025

hey! fixed with cargo fmt --all and also fixed a few lint issues from cargo clippy 🙏🏻

@vivianjeng vivianjeng merged commit 51bb3dc into zkmopro:main Feb 25, 2025
9 checks passed
@vivianjeng
Copy link
Contributor

@erhant I published a new version
please check: https://crates.io/crates/witnesscalc-adapter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants