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

Add security_bites function to PcsConfig #998

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

Conversation

alon-f
Copy link
Contributor

@alon-f alon-f commented Jan 23, 2025

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Contributor Author

alon-f commented Jan 23, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@alon-f alon-f marked this pull request as ready for review January 23, 2025 08:41
@alon-f alon-f force-pushed the 01-23-add_security_bites_function_to_pcsconfig branch from 9a9d8cc to 687f4ea Compare January 23, 2025 08:44
@alon-f alon-f requested a review from shaharsamocha7 January 23, 2025 08:45
@codecov-commenter
Copy link

codecov-commenter commented Jan 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.92%. Comparing base (0445252) to head (70437c2).

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #998   +/-   ##
=======================================
  Coverage   92.91%   92.92%           
=======================================
  Files         105      105           
  Lines       14208    14221   +13     
  Branches    14208    14221   +13     
=======================================
+ Hits        13202    13215   +13     
  Misses        927      927           
  Partials       79       79           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alon-f alon-f changed the base branch from 01-20-add_pcsconfig_to_commitmentschemeproof to graphite-base/998 January 23, 2025 09:20
Copy link
Collaborator

@shaharsamocha7 shaharsamocha7 left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @alon-f)


crates/prover/src/core/pcs/mod.rs line 36 at r1 (raw file):

    pub fri_config: FriConfig,
}

remove empty line


crates/prover/src/core/pcs/mod.rs line 58 at r1 (raw file):

        let config = super::PcsConfig {
            pow_bits: 26,
            fri_config: super::FriConfig::new(1, 1, 70),

consider to change to a larger blowup factor

Code quote:

super::FriConfig::new(1, 1, 70)

@alon-f alon-f force-pushed the 01-23-add_security_bites_function_to_pcsconfig branch from 687f4ea to 9feb687 Compare January 23, 2025 13:52
@alon-f alon-f force-pushed the graphite-base/998 branch from f4a4f6e to 7badc8b Compare January 23, 2025 13:52
Copy link
Contributor Author

@alon-f alon-f left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 of 3 files reviewed, 2 unresolved discussions (waiting on @shaharsamocha7)


crates/prover/src/core/pcs/mod.rs line 36 at r1 (raw file):

Previously, shaharsamocha7 wrote…

remove empty line

So you like to have the struct and the impl close together?

Copy link
Collaborator

@shaharsamocha7 shaharsamocha7 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 of 3 files reviewed, 2 unresolved discussions (waiting on @alon-f)


crates/prover/src/core/pcs/mod.rs line 36 at r1 (raw file):

Previously, alon-f wrote…

So you like to have the struct and the impl close together?

For the impl of the struct itself, yes.
Do we have another convention as a company or something?

@alon-f alon-f force-pushed the graphite-base/998 branch from 7badc8b to 0445252 Compare January 26, 2025 09:18
@alon-f alon-f force-pushed the 01-23-add_security_bites_function_to_pcsconfig branch from 9feb687 to 96f267c Compare January 26, 2025 09:18
@alon-f alon-f changed the base branch from graphite-base/998 to dev January 26, 2025 09:19
@alon-f alon-f force-pushed the 01-23-add_security_bites_function_to_pcsconfig branch from 96f267c to 70437c2 Compare January 26, 2025 09:19
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