Skip to content

Conversation

@avivg-starkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

@github-actions
Copy link

github-actions bot commented Jun 22, 2025

Copy link
Contributor Author

avivg-starkware commented Jun 22, 2025

@avivg-starkware avivg-starkware marked this pull request as ready for review June 22, 2025 17:27
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/add_fn_summarize_no_fee_builtins branch 3 times, most recently from 76d15c1 to f1c21f8 Compare June 23, 2025 08:25
@github-actions github-actions bot locked and limited conversation to collaborators Jun 25, 2025
@avivg-starkware avivg-starkware changed the base branch from main-v0.13.6 to graphite-base/7521 July 2, 2025 10:31
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/add_fn_summarize_no_fee_builtins branch from f1c21f8 to 7c8e93b Compare July 2, 2025 10:31
@avivg-starkware avivg-starkware changed the base branch from graphite-base/7521 to main-v0.14.0 July 2, 2025 10:31
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/add_fn_summarize_no_fee_builtins branch from 7c8e93b to 454a3e2 Compare July 2, 2025 11:09
@avivg-starkware avivg-starkware changed the title feat(blockifier): add fn summarize_no_fee_builtins blockifier: add fn summarize_no_fee_builtins Jul 2, 2025
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/add_fn_summarize_no_fee_builtins branch 4 times, most recently from 905e2b5 to d3bcb3e Compare July 2, 2025 13:20
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/add_fn_summarize_no_fee_builtins branch from d3bcb3e to 2a6232f Compare July 2, 2025 13:33
@avivg-starkware avivg-starkware changed the title blockifier: add fn summarize_no_fee_builtins blockifier: remove fee builtins duplicacy in get_tx_weights Jul 2, 2025
Copy link
Collaborator

@Yoni-Starkware Yoni-Starkware left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: 0 of 6 files reviewed, 1 unresolved discussion (waiting on @meship-starkware and @noaov1)


crates/blockifier/src/execution/call_info.rs line 168 at r2 (raw file):

    }

    pub fn remove_fee_builtins(&mut self, fee_info: &CallInfo) {

f2f

@avivg-starkware avivg-starkware changed the base branch from main-v0.14.0 to graphite-base/7521 July 3, 2025 06:50
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/add_fn_summarize_no_fee_builtins branch from 2a6232f to fb0eb1e Compare July 3, 2025 06:50
@avivg-starkware avivg-starkware changed the base branch from graphite-base/7521 to avivg/blockifier/split_execution_summary_to_builtins_sum July 3, 2025 06:50
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/add_fn_summarize_no_fee_builtins branch 2 times, most recently from b9755d5 to bf1b642 Compare July 3, 2025 06:54
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/split_execution_summary_to_builtins_sum branch from 6fc1e5a to 2d00ab0 Compare July 3, 2025 06:54
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/add_fn_summarize_no_fee_builtins branch from bf1b642 to d30be77 Compare July 3, 2025 07:21
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/split_execution_summary_to_builtins_sum branch from 2d00ab0 to 89ed6d8 Compare July 3, 2025 07:23
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/add_fn_summarize_no_fee_builtins branch 2 times, most recently from 725122a to b29dad2 Compare July 3, 2025 07:54
Copy link
Contributor Author

@avivg-starkware avivg-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: 0 of 6 files reviewed, 1 unresolved discussion (waiting on @meship-starkware, @noaov1, and @Yoni-Starkware)


crates/blockifier/src/execution/call_info.rs line 168 at r2 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

f2f

Done.

@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/split_execution_summary_to_builtins_sum branch from 89ed6d8 to 703f66a Compare July 3, 2025 08:03
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/add_fn_summarize_no_fee_builtins branch from b29dad2 to 1df300a Compare July 3, 2025 08:03
Copy link
Collaborator

@Yoni-Starkware Yoni-Starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 3 files at r5, all commit messages.
Reviewable status: 1 of 6 files reviewed, 2 unresolved discussions (waiting on @meship-starkware and @noaov1)


crates/blockifier/src/transaction/objects.rs line 217 at r4 (raw file):

    /// Returns call infos excluding fee transfer (to avoid double-counting in bouncer calculations)
    pub fn call_infos_without_fee_transfer(&self) -> impl Iterator<Item = &CallInfo> {

Suggestion:

    /// Returns call infos excluding fee transfer (to avoid double-counting in bouncer calculations).
    pub fn non_optional_call_infos_without_fee_transfer(&self) -> impl Iterator<Item = &CallInfo> {

Copy link
Collaborator

@Yoni-Starkware Yoni-Starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 3 files at r5.
Reviewable status: 3 of 6 files reviewed, 1 unresolved discussion (waiting on @avivg-starkware, @meship-starkware, and @noaov1)

@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/add_fn_summarize_no_fee_builtins branch from 1df300a to 58bf09f Compare July 3, 2025 08:37
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/split_execution_summary_to_builtins_sum branch from 703f66a to 0ecd71e Compare July 3, 2025 08:37
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/add_fn_summarize_no_fee_builtins branch from 58bf09f to 7863e52 Compare July 3, 2025 08:42
Copy link
Collaborator

@Yoni-Starkware Yoni-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:

Reviewed 1 of 2 files at r6, 1 of 1 files at r7, all commit messages.
Reviewable status: 3 of 6 files reviewed, all discussions resolved (waiting on @meship-starkware and @noaov1)

@avivg-starkware avivg-starkware changed the base branch from avivg/blockifier/split_execution_summary_to_builtins_sum to graphite-base/7521 July 3, 2025 13:11
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/add_fn_summarize_no_fee_builtins branch from 7863e52 to 78f052a Compare July 3, 2025 13:11
@avivg-starkware avivg-starkware changed the base branch from graphite-base/7521 to main-v0.14.0 July 3, 2025 13:11
@avivg-starkware avivg-starkware force-pushed the avivg/blockifier/add_fn_summarize_no_fee_builtins branch from 78f052a to 775593d Compare July 3, 2025 15:04
Copy link
Contributor Author

@avivg-starkware avivg-starkware left a comment

Choose a reason for hiding this comment

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

merged #7766 becausenative-blockifier-artifacts-push keeps failing

Reviewable status: 2 of 6 files reviewed, all discussions resolved (waiting on @meship-starkware, @noaov1, and @Yoni-Starkware)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants