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

Plonk: transcript hasher #61

Merged
merged 25 commits into from
Sep 16, 2022
Merged

Plonk: transcript hasher #61

merged 25 commits into from
Sep 16, 2022

Conversation

vesselinux
Copy link
Collaborator

Addresses Issue #56

@vesselinux vesselinux changed the title Plonk transcript hasher Plonk: transcript hasher Jul 26, 2022
@vesselinux vesselinux requested a review from dtebbs July 29, 2022 08:07
@dtebbs
Copy link
Contributor

dtebbs commented Jul 29, 2022

@vesselinux please rebase onto origin/develop

@vesselinux
Copy link
Collaborator Author

@vesselinux please rebase onto origin/develop

Sorry, do you mean rebase onto origin/plonk?

@dtebbs
Copy link
Contributor

dtebbs commented Jul 29, 2022

@vesselinux please rebase onto origin/develop

Sorry, do you mean rebase onto origin/plonk?

Oh yes. origin/plonk please. Thanks.
I noticed that the error handling commit still appears in the history of this PR, even though that is merged to plonk now. Rebasing should remove it (or you may have to skip it).

@vesselinux vesselinux force-pushed the plonk-transcript-hasher-issue-56 branch 2 times, most recently from ef3297b to ee34c38 Compare July 29, 2022 13:45
@vesselinux
Copy link
Collaborator Author

vesselinux commented Jul 29, 2022

@vesselinux please rebase onto origin/develop

Sorry, do you mean rebase onto origin/plonk?

Oh yes. origin/plonk please. Thanks. I noticed that the error handling commit still appears in the history of this PR, even though that is merged to plonk now. Rebasing should remove it (or you may have to skip it).

Rebased on plonk and dropped the error handling commit.

@vesselinux vesselinux force-pushed the plonk-transcript-hasher-issue-56 branch from 793e649 to 2fbad61 Compare August 17, 2022 07:14
@vesselinux vesselinux force-pushed the plonk-transcript-hasher-issue-56 branch from 2fbad61 to 78dd119 Compare August 17, 2022 07:48
vesselinux pushed a commit that referenced this pull request Aug 17, 2022
…to the transcript hasher constructor (cf. #61 (comment)); added exception handling in case the buffer length is invalid (cf. #61 (comment))
@vesselinux vesselinux requested a review from dtebbs August 17, 2022 09:54
vesselinux pushed a commit that referenced this pull request Aug 18, 2022
…r process since u is not used by the prover anyway. removed the automatic clearing of the hasher buffer inside get_hash. the caller is now responsible to clear the buffer when reusing the same hasher object. see also PR comment #61 (comment) .
vesselinux pushed a commit that referenced this pull request Aug 19, 2022
vesselinux pushed a commit that referenced this pull request Aug 19, 2022
@dtebbs
Copy link
Contributor

dtebbs commented Aug 23, 2022

Hasher should either keep a reference (more dangerous), or just allocate it's own internal buffer. Latter is probably preferable.

vesselinux pushed a commit that referenced this pull request Aug 26, 2022
…e last round of the prover. we need to compute u even though we are not using it in the prover in order to make sure that the prover and verifier make exactly the same number of calls to transcript_hasher.get_hash(). addresses PR #61 comment #61 (comment)
vesselinux pushed a commit that referenced this pull request Aug 26, 2022
… input parameter to the constructor. only the object has access to the buffer now. addresses PR #61 comment #61 (comment)
@vesselinux
Copy link
Collaborator Author

Hasher should either keep a reference (more dangerous), or just allocate it's own internal buffer. Latter is probably preferable.

Removed the private buffer variable as an input parameter to the constructor. Only the object has access to the buffer now.

@vesselinux vesselinux requested a review from dtebbs August 26, 2022 12:01
vesselinux pushed a commit that referenced this pull request Oct 13, 2022
…ion parameter to the prover and verifier classes. moved the current transcript_hasher class from files srs.* to its own files under tests, the reason being that its implementation is specific to the bls12-381 curve. set the prover and verifier to be instantiated with this bls12-381-specific implementation of transcript_hasher. this commit addresses steps 2. and 4. from comment #61 (comment) of PR#61.
vesselinux pushed a commit that referenced this pull request Oct 13, 2022
…hat describes the interface of a common transcript hasher. addresses step 1. from comment #61 (comment) of PR#61.
vesselinux pushed a commit that referenced this pull request Oct 13, 2022
…cript hasher with a while loop directly finding the correct length; addresses #61 (comment) .
vesselinux pushed a commit that referenced this pull request Oct 13, 2022
vesselinux pushed a commit that referenced this pull request Oct 13, 2022
…bls12_381_test_vector_transcript_hasher initialized in the constructor. addresses #61 (comment) .
vesselinux pushed a commit that referenced this pull request Oct 13, 2022
…length and challenge arrays are of same length in transcript hasher constructor; minor edits. addresses lates comments in PR #61 .
dtebbs pushed a commit that referenced this pull request Oct 26, 2022
…to the transcript hasher constructor (cf. #61 (comment)); added exception handling in case the buffer length is invalid (cf. #61 (comment))
dtebbs pushed a commit that referenced this pull request Oct 26, 2022
…r process since u is not used by the prover anyway. removed the automatic clearing of the hasher buffer inside get_hash. the caller is now responsible to clear the buffer when reusing the same hasher object. see also PR comment #61 (comment) .
dtebbs pushed a commit that referenced this pull request Oct 26, 2022
dtebbs pushed a commit that referenced this pull request Oct 26, 2022
dtebbs pushed a commit that referenced this pull request Oct 26, 2022
…e last round of the prover. we need to compute u even though we are not using it in the prover in order to make sure that the prover and verifier make exactly the same number of calls to transcript_hasher.get_hash(). addresses PR #61 comment #61 (comment)
dtebbs pushed a commit that referenced this pull request Oct 26, 2022
… input parameter to the constructor. only the object has access to the buffer now. addresses PR #61 comment #61 (comment)
dtebbs pushed a commit that referenced this pull request Oct 26, 2022
dtebbs pushed a commit that referenced this pull request Oct 26, 2022
dtebbs pushed a commit that referenced this pull request Oct 26, 2022
…ion parameter to the prover and verifier classes. moved the current transcript_hasher class from files srs.* to its own files under tests, the reason being that its implementation is specific to the bls12-381 curve. set the prover and verifier to be instantiated with this bls12-381-specific implementation of transcript_hasher. this commit addresses steps 2. and 4. from comment #61 (comment) of PR#61.
dtebbs pushed a commit that referenced this pull request Oct 26, 2022
…hat describes the interface of a common transcript hasher. addresses step 1. from comment #61 (comment) of PR#61.
dtebbs pushed a commit that referenced this pull request Oct 26, 2022
…cript hasher with a while loop directly finding the correct length; addresses #61 (comment) .
dtebbs pushed a commit that referenced this pull request Oct 26, 2022
dtebbs pushed a commit that referenced this pull request Oct 26, 2022
dtebbs pushed a commit that referenced this pull request Oct 26, 2022
…bls12_381_test_vector_transcript_hasher initialized in the constructor. addresses #61 (comment) .
dtebbs pushed a commit that referenced this pull request Oct 26, 2022
…length and challenge arrays are of same length in transcript hasher constructor; minor edits. addresses lates comments in PR #61 .
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.

None yet

2 participants