Skip to content

Refactor Statement to keep track of component names#455

Merged
ilyalesokhin-starkware merged 1 commit intomainfrom
ilya/named
Apr 15, 2026
Merged

Refactor Statement to keep track of component names#455
ilyalesokhin-starkware merged 1 commit intomainfrom
ilya/named

Conversation

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor

Change get_components() on the Statement trait to return an
IndexMap<&'static str, Box<dyn CircuitEval>> so each component
carries a stable name alongside its constraint evaluator. Update all
implementors (CircuitStatement, CairoStatement, SimpleStatement) and
call sites accordingly.

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 15, 2026

PR Summary

Medium Risk
Touches core verifier/statement plumbing by changing Statement::get_components() and propagating IndexMap ordering through proof config and verification loops; a mismatch in component ordering/naming could break proof verification even though logic is mostly refactor.

Overview
Refactors the Statement interface to carry a stable component name alongside each CircuitEval by returning an IndexMap<&'static str, Box<dyn CircuitEval<_>>> instead of a Vec/slice.

Updates all statement implementations and call sites (Cairo verifier, circuit AIR statement, and examples) to build/iterate components via IndexMap (typically values()), and adjusts indexing-based logic to use IndexMap lookups by name.

Adds indexmap as a dependency where needed and updates ProofConfig::from_components and verifier composition evaluation loops to consume components from the map deterministically.

Reviewed by Cursor Bugbot for commit c8a1958. Bugbot is set up for automated code reviews on this repo. Configure here.

@reviewable-StarkWare
Copy link
Copy Markdown
Collaborator

This change is Reviewable

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor

@Gali-StarkWare Gali-StarkWare left a comment

Choose a reason for hiding this comment

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

@Gali-StarkWare reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: 1 of 13 files reviewed, 1 unresolved discussion (waiting on ilyalesokhin-starkware and leo-starkware).


Cargo.lock line 1277 at r1 (raw file):

 "circuits-stark-verifier",
 "expect-test",
 "indexmap",

Is it related? I don't see the toml change

Code quote:

 "indexmap",

Copy link
Copy Markdown
Contributor

@Gali-StarkWare Gali-StarkWare left a comment

Choose a reason for hiding this comment

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

@Gali-StarkWare made 1 comment.
Reviewable status: 1 of 13 files reviewed, 1 unresolved discussion (waiting on ilyalesokhin-starkware and leo-starkware).


Cargo.lock line 1277 at r1 (raw file):

Previously, Gali-StarkWare wrote…

Is it related? I don't see the toml change

nvm

Copy link
Copy Markdown
Contributor

@Gali-StarkWare Gali-StarkWare left a comment

Choose a reason for hiding this comment

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

@Gali-StarkWare resolved 1 discussion.
Reviewable status: 1 of 13 files reviewed, all discussions resolved (waiting on leo-starkware).

Copy link
Copy Markdown
Contributor

@Gali-StarkWare Gali-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:

@Gali-StarkWare reviewed 12 files and made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ilyalesokhin-starkware and leo-starkware).


crates/circuit_air/src/statement.rs line 145 at r1 (raw file):

{
    IndexMap::from([
        ("eq", Box::new(CircuitEqComponent {}) as Box<dyn CircuitEval<Value>>),

Maybe eq_gate is better? Or does is require changes elsewhere?

Code quote:

"eq"

Copy link
Copy Markdown
Contributor

@Gali-StarkWare Gali-StarkWare left a comment

Choose a reason for hiding this comment

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

So why do we need the names?

@Gali-StarkWare made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ilyalesokhin-starkware and leo-starkware).

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

I need it in the next PR, here:

let Some(index) = self.components.get_index_of(name) else {

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

crates/circuit_air/src/statement.rs line 145 at r1 (raw file):

Previously, Gali-StarkWare wrote…

Maybe eq_gate is better? Or does is require changes elsewhere?

it is called eq in the infra:

)

Copy link
Copy Markdown
Contributor Author

@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 resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on leo-starkware).

Copy link
Copy Markdown
Contributor

@Gali-StarkWare Gali-StarkWare 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: :shipit: complete! all files reviewed, all discussions resolved (waiting on leo-starkware).

Copy link
Copy Markdown
Contributor

@Gali-StarkWare Gali-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:

@Gali-StarkWare made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on leo-starkware).

Change get_components() on the Statement trait to return an
IndexMap<&'static str, Box<dyn CircuitEval<Value>>> so each component
carries a stable name alongside its constraint evaluator. Update all
implementors (CircuitStatement, CairoStatement, SimpleStatement) and
call sites accordingly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ilyalesokhin-starkware ilyalesokhin-starkware merged commit 5ef6793 into main Apr 15, 2026
11 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.

3 participants