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: generalize transcript #202

Merged
merged 2 commits into from
Dec 15, 2023
Merged

feat: generalize transcript #202

merged 2 commits into from
Dec 15, 2023

Conversation

chokobole
Copy link
Contributor

@chokobole chokobole commented Dec 14, 2023

Description

Current Transcript assumes that its commitment type is an affine point. This commit breaks this assumption and it differentiates cases.

  1. Commitment and Scalar are same types, such as FRI commitment cases.
  2. Commitment and Scalar are different types, such as KZG commitment cases.

Also TranscriptReader and TranscriptWriter child classes uses Curve type as a template parameter which is different from Commitment type from a parent class. This commit unifies template parameter for consistency as well.

Other than above, it does as follows.

  • move halo2 specific things to zk/plonk/halo2.
  • remove Challenge255.
  • rename ReadScalar() and ReadPoint() to ReadFromProof() for consistency with WriteToProof() and WriteToTranscript().
  • remove unused headers from transcript implementation.

@chokobole chokobole force-pushed the feat/generalize-transcript branch 5 times, most recently from b83226f to d7314fa Compare December 15, 2023 02:11
@chokobole chokobole marked this pull request as ready for review December 15, 2023 02:11
Current `Transcript` assumes that its commitment type is an affine
point. This commit breaks this assumption and it differentiates cases.

1. Commitment and Scalar are same types, such as FRI commitment cases.
2. Commitment and Scalar are different types, such as KZG commitment cases.

Also `TranscriptReader` and `TranscriptWriter` child classes uses
`Curve` type as a template parameter which is different from
`Commitment` type from a parent class. This commit unifies template
parameter for consistency as well.

Other than above, it does as follows.

- move halo2 specific things to `zk/plonk/halo2`.
- remove `Challenge255` which leads to removing
  `SqueezeChallengeAsScalar()`.
- rename `ReadScalar()` and `ReadPoint()` to `ReadFromProof()`
  for consistency with `WriteToProof()` and `WriteToTranscript()`.
- remove unused headers from transcript implementation.
@chokobole chokobole force-pushed the feat/generalize-transcript branch from d7314fa to e6c6844 Compare December 15, 2023 04:03
tachyon/crypto/transcripts/transcript.h Outdated Show resolved Hide resolved
tachyon/crypto/transcripts/transcript.h Outdated Show resolved Hide resolved
tachyon/crypto/transcripts/transcript.h Outdated Show resolved Hide resolved
tachyon/crypto/transcripts/transcript.h Outdated Show resolved Hide resolved
@chokobole chokobole force-pushed the feat/generalize-transcript branch from b70c157 to c01b1de Compare December 15, 2023 06:48
@chokobole chokobole force-pushed the feat/generalize-transcript branch from c01b1de to 2f965fb Compare December 15, 2023 06:49
Copy link
Contributor

@fakedev9999 fakedev9999 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@dongchangYoo dongchangYoo left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@Insun35 Insun35 left a comment

Choose a reason for hiding this comment

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

LGTM

@chokobole chokobole merged commit c212219 into main Dec 15, 2023
3 checks passed
@chokobole chokobole deleted the feat/generalize-transcript branch December 15, 2023 07:44
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.

4 participants