Skip to content

Conversation

@leo-starkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Contributor Author

leo-starkware commented Dec 23, 2025

@codecov-commenter
Copy link

codecov-commenter commented Dec 23, 2025

Codecov Report

❌ Patch coverage is 98.85057% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 91.95%. Comparing base (6dea951) to head (a442e4a).
⚠️ Report is 1 commits behind head on lifted.

Files with missing lines Patch % Lines
crates/stwo/src/core/vcs_lifted/verifier.rs 96.55% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           lifted    #1293      +/-   ##
==========================================
- Coverage   91.97%   91.95%   -0.02%     
==========================================
  Files         119      119              
  Lines       14959    14974      +15     
  Branches    14959    14974      +15     
==========================================
+ Hits        13759    13770      +11     
- Misses       1110     1114       +4     
  Partials       90       90              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@leo-starkware leo-starkware force-pushed the leo/layout_of_queried_values branch from 2fd9f95 to e5b9911 Compare December 23, 2025 13:07
@leo-starkware leo-starkware force-pushed the leo/layout_of_queried_values branch from e5b9911 to 68fdcc2 Compare December 23, 2025 13:10
@leo-starkware leo-starkware force-pushed the leo/layout_of_queried_values branch from 68fdcc2 to 27315a2 Compare December 23, 2025 13:11
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: cf74d17 Previous: 2a2c27f Ratio
cpu polynomial commitment 2^20 3539673391 ns/iter (± 59228169) 1502751321 ns/iter (± 2911465) 2.36

This comment was automatically generated by workflow using github-action-benchmark.

CC: @gilbens-starkware

@leo-starkware leo-starkware force-pushed the leo/layout_of_queried_values branch 2 times, most recently from c9cd915 to 0a59d76 Compare December 23, 2025 13:46
@leo-starkware leo-starkware force-pushed the leo/layout_of_queried_values branch from 0a59d76 to fd7a4d9 Compare December 24, 2025 09:45
@leo-starkware leo-starkware force-pushed the leo/layout_of_queried_values branch from fd7a4d9 to cf74d17 Compare December 24, 2025 09:49
@leo-starkware leo-starkware force-pushed the leo/layout_of_queried_values branch from cf74d17 to c340d00 Compare December 24, 2025 13:14
@leo-starkware leo-starkware force-pushed the leo/layout_of_queried_values branch from c340d00 to 9eb95de Compare December 24, 2025 13:25
@leo-starkware leo-starkware marked this pull request as ready for review December 24, 2025 13:28
@leo-starkware leo-starkware force-pushed the leo/layout_of_queried_values branch from 9eb95de to e015b6c Compare December 25, 2025 08:25
@ilyalesokhin-starkware
Copy link
Collaborator

crates/stwo/src/core/fri.rs line 463 at r2 (raw file):

        // The correct type for `decommitmented_values` is `SecureColumnByCoords<CpuBackend>`,
        // however since this is verifier code, we cannot use types from the prover module.

Suggestion:

        // A QM31 column is committed as 4 M31 columns.

@ilyalesokhin-starkware
Copy link
Collaborator

crates/stwo/src/core/vcs_lifted/verifier.rs line 133 at r2 (raw file):

            let row: Vec<_> = sorted_queries_iter
                .iter_mut()
                .map(|col_iter| *col_iter.next().unwrap())

Do we need to check that the iterators are empty after the last iteration?

Code quote:

*col_iter.next()

Copy link
Collaborator

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

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

@ilyalesokhin-starkware reviewed 4 files and all commit messages.
Reviewable status: 4 of 7 files reviewed, 2 unresolved discussions (waiting on @leo-starkware).

Copy link
Contributor Author

@leo-starkware leo-starkware left a comment

Choose a reason for hiding this comment

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

@leo-starkware made 1 comment.
Reviewable status: 4 of 7 files reviewed, 2 unresolved discussions (waiting on @ilyalesokhin-starkware).


crates/stwo/src/core/vcs_lifted/verifier.rs line 133 at r2 (raw file):

Previously, ilyalesokhin-starkware wrote…

Do we need to check that the iterators are empty after the last iteration?

This is checked in crates/stwo/src/core/pcs/quotients.rs:125, but maybe it's indeed better here. WDYT?

@leo-starkware leo-starkware force-pushed the leo/layout_of_queried_values branch from e015b6c to 929f0d6 Compare December 25, 2025 14:06
Copy link
Contributor Author

@leo-starkware leo-starkware left a comment

Choose a reason for hiding this comment

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

@leo-starkware made 2 comments.
Reviewable status: 4 of 7 files reviewed, 2 unresolved discussions (waiting on @ilyalesokhin-starkware).


crates/stwo/src/core/vcs_lifted/verifier.rs line 133 at r2 (raw file):

Previously, leo-starkware wrote…

This is checked in crates/stwo/src/core/pcs/quotients.rs:125, but maybe it's indeed better here. WDYT?

Added


crates/stwo/src/core/fri.rs line 463 at r2 (raw file):

        // The correct type for `decommitmented_values` is `SecureColumnByCoords<CpuBackend>`,
        // however since this is verifier code, we cannot use types from the prover module.

Done. Also changed the type from vec to array

Copy link
Collaborator

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

@ilyalesokhin-starkware reviewed 2 files, made 1 comment, and resolved 2 discussions.
Reviewable status: 6 of 7 files reviewed, all discussions resolved.

Copy link
Contributor Author

@leo-starkware leo-starkware left a comment

Choose a reason for hiding this comment

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

@leo-starkware reviewed 2 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @leo-starkware).

@leo-starkware leo-starkware changed the base branch from leo/order_of_alphas to lifted December 29, 2025 14:47
@leo-starkware leo-starkware force-pushed the leo/layout_of_queried_values branch from 929f0d6 to a442e4a Compare December 29, 2025 14:48
@leo-starkware leo-starkware merged commit 2f93b37 into lifted Dec 29, 2025
23 checks passed
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.

5 participants