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

IOPattern from and to bytes for portability #29

Closed
wants to merge 2 commits into from

Conversation

alxiong
Copy link
Contributor

@alxiong alxiong commented Jan 5, 2025

Fix #28

cc @mmaker

@alxiong alxiong changed the title add basic trait impl to Merlin and IOPattern IOPattern from and to bytes for portability Jan 9, 2025
@alxiong
Copy link
Contributor Author

alxiong commented Jan 9, 2025

I have updated the PR to address my follow-up comment: #28 (comment) @mmaker

@mmaker
Copy link
Member

mmaker commented Jan 9, 2025

Hey, thank you so much. Some points:

  • I'm not convinced we need to implement equality checks for IOPattern (this seems just for the test)
  • Given that we have .as_bytes I don't think we need .to_bytes as well. Overall, given that we're just doing hashing, it would be great to rely only on stack allocations in the core library
  • I don't know if it from_bytes should rather be a AsRef<[u8]> or a Read implementation, given the length of these functions.

For now I'm leaning on thinking it's just simpler to expose from_string(): if you have already to share metadata between prover and verifier, you already have to think about encodings and strings.
I'm interested in hearing if this complicates things on your end and I misunderstood something.

For reference, your ideas on how to implement Safe::preprocess() would be super valuable.
I exposed from_string in 857289b

@alxiong
Copy link
Contributor Author

alxiong commented Jan 9, 2025

all sounds reasonable. closing this now.
thanks a lot!

@alxiong alxiong closed this Jan 9, 2025
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.

Common trait implementations for IOPattern
2 participants