Skip to content

refactor(circuit_common): bound seq_of_component_size by MAX_SEQUENCE_LOG_SIZE - #672

Open
ilyalesokhin-starkware wants to merge 1 commit into
mainfrom
ilya/seq-max-sequence-log-size
Open

ilyalesokhin-starkware wants to merge 1 commit into
mainfrom
ilya/seq-max-sequence-log-size

Conversation

@ilyalesokhin-starkware

@ilyalesokhin-starkware ilyalesokhin-starkware commented Jul 23, 2026 •

Copy link
Copy Markdown
Contributor

refactor(circuit_common): bound seq_of_component_size by MAX_SEQUENCE_LOG_SIZE
This keeps the seq columns consistent with MAX_SEQUENCE_LOG_SIZE, which the
memory_id_to_big component relies on: it reserves ID_TO_BIG_MAX_ROWS ==
2^MAX_SEQUENCE_LOG_SIZE ids per instance and places consecutive instances at id
offsets that assume no instance exceeds that size. If MAX_SEQUENCE_LOG_SIZE is
inconsistent with the preprocessed columns, the id_to_big components can overlap.

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

@cursor

cursor Bot commented Jul 23, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes STARK constraint construction for component height selection; misalignment with memory_id_to_big id spacing would be serious, but the change is intended to enforce that alignment.

Overview
seq_of_component_size now sums bit_k · seq_k only over log sizes 0..=MAX_SEQUENCE_LOG_SIZE, matching which seq_k columns the preprocessed trace defines, instead of using the per-proof max_component_size_bits() (trace log size + 1).

The loop still skips missing seq_{log_size} entries via preprocessed_columns.get, and get_n_instances_bit is called with the index converted from the u32 loop variable.

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

@reviewable-StarkWare

Copy link
Copy Markdown
Collaborator

This change is Reviewable

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1c0fd5a. Configure here.

};

let bit = component_data.get_n_instances_bit(context, log_size);
let bit = component_data.get_n_instances_bit(context, log_size.try_into().unwrap());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Out-of-bounds instance bit index

High Severity

The loop now runs through MAX_SEQUENCE_LOG_SIZE, but when a matching seq_k column exists in preprocessed_columns for k at or above max_component_size_bits(), get_n_instances_bit indexes past the end of n_instances_bits (length log_trace_size + 1) and panics. The previous loop stopped at max_component_size_bits() and never hit those indices.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1c0fd5a. Configure here.

…_LOG_SIZE

This keeps the seq columns consistent with MAX_SEQUENCE_LOG_SIZE, which the
memory_id_to_big component relies on: it reserves ID_TO_BIG_MAX_ROWS ==
2^MAX_SEQUENCE_LOG_SIZE ids per instance and places consecutive instances at id
offsets that assume no instance exceeds that size. If MAX_SEQUENCE_LOG_SIZE is
inconsistent with the preprocessed columns, the id_to_big components can overlap.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ilyalesokhin-starkware
ilyalesokhin-starkware force-pushed the ilya/seq-max-sequence-log-size branch 2 times, most recently from 1c0fd5a to 9788d22 Compare July 23, 2026 08:28
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.

2 participants