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: implement poseidon transcript #107

Merged
merged 7 commits into from
Oct 25, 2023

Conversation

TomTaehoonKim
Copy link
Contributor

Description

This PR implements Poseidon Transcript (Poseidon Read/Write) along with a base class Transcript (Transcript, Transcript Read/Write) which will later also be used implementing Sha256 Transcript.

Note that Halo2 Poseidon Sponge is implemented for two variants compared to Poseidon Sponge from arkworks.

  • Initializes state's 0 index element with 1<<64.
  • Append F::One to the state before squeezing.

For the halo2 rust implementation, see link

@chokobole
Copy link
Contributor

Could you add a reason for why you rename PoseidonDefaultConfigEntry to PoseidonConfigEntry?

@chokobole
Copy link
Contributor

chokobole commented Oct 24, 2023

  1. Could you rename PoseidonUtilTest to PoseidonConfigTest in posiedon_config_unittest.cc?
  2. Could you rename TYPED_TEST(PoseidonUtilTest, PoseidonConfig_CreateDefault) { to TYPED_TEST(PoseidonConfigTest, CreateDefault) {?
  3. Could you add a test for CreateCustom?
  4. Pleaes use TEST_F instead of TYPED_TEST

@chokobole
Copy link
Contributor

Could you update the commit body to the last commit?

See [PoseidonRead](https://github.com/kroma-network/halo2/blob/7d0a36990452c8e7ebd600de258420781a9b7917/halo2_proofs/src/transcript/poseidon.rs#L12-L100),
[PoseidonWrite](https://github.com/kroma-network/halo2/blob/7d0a36990452c8e7ebd600de258420781a9b7917/halo2_proofs/src/transcript/poseidon.rs#L102-L174),
[Transcript](https://github.com/kroma-network/halo2/blob/7d0a36990452c8e7ebd600de258420781a9b7917/halo2_proofs/src/transcript.rs#L43-L63),
[TranscriptRead](https://github.com/kroma-network/halo2/blob/7d0a36990452c8e7ebd600de258420781a9b7917/halo2_proofs/src/transcript.rs#L65-L73)
and [TranscriptWirte](https://github.com/kroma-network/halo2/blob/7d0a36990452c8e7ebd600de258420781a9b7917/halo2_proofs/src/transcript.rs#L75-L83).

tachyon/crypto/hashes/sponge/poseidon/poseidon_config.h Outdated Show resolved Hide resolved
tachyon/crypto/hashes/sponge/poseidon/halo2_poseidon.h Outdated Show resolved Hide resolved
tachyon/crypto/hashes/sponge/poseidon/halo2_poseidon.h Outdated Show resolved Hide resolved
tachyon/crypto/hashes/sponge/poseidon/BUILD.bazel Outdated Show resolved Hide resolved
tachyon/zk/transcript/poseidon_transcript.h Outdated Show resolved Hide resolved
tachyon/crypto/hashes/sponge/poseidon/halo2_poseidon.h Outdated Show resolved Hide resolved
tachyon/zk/transcript/poseidon_transcript_unittest.cc Outdated Show resolved Hide resolved
tachyon/zk/transcript/poseidon_transcript_unittest.cc Outdated Show resolved Hide resolved
tachyon/zk/transcript/poseidon_transcript_unittest.cc Outdated Show resolved Hide resolved
@TomTaehoonKim TomTaehoonKim force-pushed the feat/implement-poseidon-transcript branch 3 times, most recently from 244b601 to 70f9bdc Compare October 25, 2023 00:41
@chokobole
Copy link
Contributor

fix(crypto): provide default constructor for PoseidonSponge Is it bugfix commit?

tachyon/zk/transcript/poseidon_transcript.h Outdated Show resolved Hide resolved
tachyon/zk/transcript/transcript.h Outdated Show resolved Hide resolved
tachyon/zk/transcript/transcript.h Outdated Show resolved Hide resolved
tachyon/zk/transcript/poseidon_transcript.h Outdated Show resolved Hide resolved
tachyon/zk/transcript/poseidon_transcript_unittest.cc Outdated Show resolved Hide resolved
tachyon/zk/transcript/transcript.h Outdated Show resolved Hide resolved
tachyon/zk/transcript/transcript.h Outdated Show resolved Hide resolved
tachyon/zk/transcript/transcript.h Outdated Show resolved Hide resolved
tachyon/zk/transcript/transcript.h Outdated Show resolved Hide resolved
@TomTaehoonKim TomTaehoonKim force-pushed the feat/implement-poseidon-transcript branch 2 times, most recently from d6334c4 to 7f28f5e Compare October 25, 2023 02:42
@TomTaehoonKim TomTaehoonKim force-pushed the feat/implement-poseidon-transcript branch from 7f28f5e to e905450 Compare October 25, 2023 03:50
tachyon/zk/transcript/poseidon_transcript.h Outdated Show resolved Hide resolved
tachyon/zk/transcript/poseidon_transcript.h Outdated Show resolved Hide resolved
tachyon/zk/transcript/poseidon_transcript.h Outdated Show resolved Hide resolved
tachyon/zk/transcript/poseidon_transcript.h Outdated Show resolved Hide resolved
tachyon/zk/transcript/poseidon_transcript.h Outdated Show resolved Hide resolved
tachyon/zk/transcript/poseidon_transcript.h Outdated Show resolved Hide resolved
tachyon/zk/transcript/poseidon_transcript.h Outdated Show resolved Hide resolved
tachyon/zk/transcript/poseidon_transcript.h Outdated Show resolved Hide resolved
tachyon/zk/transcript/transcript.h Outdated Show resolved Hide resolved
tachyon/zk/transcript/transcript.h Show resolved Hide resolved
@TomTaehoonKim TomTaehoonKim force-pushed the feat/implement-poseidon-transcript branch from e905450 to c6de27d Compare October 25, 2023 05:27
@chokobole chokobole force-pushed the feat/implement-poseidon-transcript branch from c6de27d to 36a9429 Compare October 25, 2023 05:46
@TomTaehoonKim TomTaehoonKim force-pushed the feat/implement-poseidon-transcript branch from 36a9429 to 8825d63 Compare October 25, 2023 06:16
Copy link
Contributor

@chokobole chokobole left a comment

Choose a reason for hiding this comment

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

LGTM

@TomTaehoonKim TomTaehoonKim force-pushed the feat/implement-poseidon-transcript branch from 8825d63 to b733f83 Compare October 25, 2023 09:47
Copy link
Contributor

@Leegwangwoon Leegwangwoon left a comment

Choose a reason for hiding this comment

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

LGTM

@TomTaehoonKim TomTaehoonKim force-pushed the feat/implement-poseidon-transcript branch from b733f83 to abf97b8 Compare October 25, 2023 11:57
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

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

@TomTaehoonKim TomTaehoonKim merged commit 433e827 into main Oct 25, 2023
3 checks passed
@TomTaehoonKim TomTaehoonKim deleted the feat/implement-poseidon-transcript branch October 25, 2023 13:05
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

5 participants