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: verifier returns all outputs rather than their hash #57

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tdelabro
Copy link

@tdelabro tdelabro commented Dec 19, 2024

The fact that the verifier returns the hash of the output rather than the full outputs prevents a lot of use cases.
Eg. I want to accept a proof in my application if and only if the value written in the inputs is the pubkey that belong to a set I maintain. With only the hash I won't be able to check if this pubkey belong to my set.

This is a breaking change.
People who rely on the old behavior (hash of all outputs) of this lib will have to compute the hash themselves.

Sidenote:

  • There is room for code factorization between the different layouts.
  • It would theoretically be possible to return an iterator over &Felt instead of a Vec. This would avoid both collecting and cloning the returned values but at the cost of more complexity in the trait.

@Okm165 Okm165 self-assigned this Dec 20, 2024
@Okm165 Okm165 added the enhancement New feature or request label Dec 20, 2024
@Okm165
Copy link
Contributor

Okm165 commented Dec 20, 2024

We can export program hash and output hash util functions in the project for people to use (in couple flavors: pedersen, poseidon and keccak for example). Also wondering if raw program byte-code could be returned for sake of symmetry so user could just pass it to util function in his project lmk wdyt?

The byte-code is there in raw form we should have a way to extract it raw imo as we do with output.

@tdelabro
Copy link
Author

#59 provides it for the program hash
I can also factorize and export the output_hash in this one.

About the byte-code I'm not sure.
From my perspective, as a user of the verifier, I just want to provide the proof, get a bool and the output, and that is all.
But maybe I'm missing a use-case here

@tdelabro
Copy link
Author

So after checking it appears that the method to compute the hash of the output (once returned as a vec) already exists in the ecosystem:

pedersen:
https://github.com/xJonathanLEI/starknet-rs/blob/8755b68675f103ee011f1d964fcec55f1902fa09/starknet-core/src/crypto.rs#L66
poseidon:
https://github.com/xJonathanLEI/starknet-rs/blob/8755b68675f103ee011f1d964fcec55f1902fa09/starknet-crypto/src/poseidon_hash.rs#L72

Whatever method I can expose will just be a wrapper over those. Maybe it's not useful to reexpose those in swiftness.

@Okm165
Copy link
Contributor

Okm165 commented Dec 20, 2024

You are right lets have just logic for extraction of program and output to Vec<Felt> in swiftness then in examples we can present hashing these with https://github.com/xJonathanLEI/starknet-rs utils so we don't redefine anything

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

Successfully merging this pull request may close these issues.

2 participants