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

Merge main-v0.13.4 into main #3178

Merged

Conversation

meship-starkware
Copy link
Contributor

No description provided.

dorimedini-starkware and others added 30 commits October 22, 2024 18:31
…blish-branch

Dori/merge main blockifier publish branch
* chore(blockifier): impl deserialize for versioned_constants

* chore(blockifier): remove fee_transfer_gas_cost (#2858)
- No reason to run cargo on release in the ci, and this busts cache
- Cargo doc doesn't pick up the usual place we pass -Dwarnings, which is
  RUSTFLAGS, it needs a special flag RUSTDOCFLAGS (it cannot reference
  the usual RUSTFLAGS so we have to duplicate)

Co-Authored-By: Gilad Chase <[email protected]>
* chore(deployment): cdk8s refactor (#2468)

* chore(deployment): get values from sequencer config

* chore(deployment): removed set functions

* chore(deployment): preparation to remove objects.py

* chore(deployment): fixed wrong assert usage, added return type annotations
Copy link
Contributor Author

@meship-starkware meship-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 31 files at r2.
Reviewable status: 145 of 178 files reviewed, all discussions resolved

Copy link

github-actions bot commented Jan 8, 2025

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [34.867 ms 34.989 ms 35.182 ms]
change: [+1.9730% +2.3476% +2.9416%] (p = 0.00 < 0.05)
Performance has regressed.
Found 5 outliers among 100 measurements (5.00%)
1 (1.00%) high mild
4 (4.00%) high severe

full_committer_flow performance regressed!
full_committer_flow time: [30.724 ms 30.768 ms 30.816 ms]
change: [+1.9177% +2.4759% +2.9184%] (p = 0.00 < 0.05)
Performance has regressed.
Found 5 outliers among 100 measurements (5.00%)
2 (2.00%) high mild
3 (3.00%) high severe

Copy link
Contributor Author

@meship-starkware meship-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 7 files at r3.
Reviewable status: 146 of 178 files reviewed, all discussions resolved (waiting on @dorimedini-starkware, @Itay-Tsabary-Starkware, and @noaov1)

@meship-starkware meship-starkware force-pushed the meshi/merge-main-v0.13.4-into-main-1736338547 branch from 5740a25 to e17ffad Compare January 8, 2025 14:16
Copy link

github-actions bot commented Jan 8, 2025

Benchmark movements:
full_committer_flow performance regressed!
full_committer_flow time: [30.733 ms 30.782 ms 30.833 ms]
change: [+2.6102% +2.8823% +3.1347%] (p = 0.00 < 0.05)
Performance has regressed.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild

@meship-starkware meship-starkware force-pushed the meshi/merge-main-v0.13.4-into-main-1736338547 branch from e17ffad to b110df4 Compare January 8, 2025 15:33
Copy link

github-actions bot commented Jan 8, 2025

Benchmark movements:
full_committer_flow performance regressed!
full_committer_flow time: [30.598 ms 30.651 ms 30.714 ms]
change: [+2.0932% +2.6788% +3.1570%] (p = 0.00 < 0.05)
Performance has regressed.
Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severe

@meship-starkware meship-starkware force-pushed the meshi/merge-main-v0.13.4-into-main-1736338547 branch from b110df4 to 231b191 Compare January 8, 2025 15:53
Copy link

github-actions bot commented Jan 8, 2025

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [35.100 ms 35.574 ms 36.140 ms]
change: [+2.5668% +4.0739% +5.8909%] (p = 0.00 < 0.05)
Performance has regressed.
Found 10 outliers among 100 measurements (10.00%)
3 (3.00%) high mild
7 (7.00%) high severe

full_committer_flow performance regressed!
full_committer_flow time: [30.865 ms 30.914 ms 30.968 ms]
change: [+3.2343% +3.4786% +3.7340%] (p = 0.00 < 0.05)
Performance has regressed.
Found 5 outliers among 100 measurements (5.00%)
1 (1.00%) high mild
4 (4.00%) high severe

@meship-starkware meship-starkware force-pushed the meshi/merge-main-v0.13.4-into-main-1736338547 branch from 98036c8 to 6a397db Compare January 9, 2025 07:33
Copy link

github-actions bot commented Jan 9, 2025

Benchmark movements:
full_committer_flow performance regressed!
full_committer_flow time: [30.612 ms 30.721 ms 30.852 ms]
change: [+2.8491% +3.2623% +3.7804%] (p = 0.00 < 0.05)
Performance has regressed.
Found 7 outliers among 100 measurements (7.00%)
3 (3.00%) high mild
4 (4.00%) high severe

Copy link
Contributor Author

@meship-starkware meship-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 31 files at r2.
Reviewable status: 144 of 180 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware, @Itay-Tsabary-Starkware, and @noaov1)


crates/mempool_test_utils/src/starknet_api_test_utils.rs line 404 at r6 (raw file):

        let sender_address = self.sender_address();
        self.nonce_manager.borrow_mut().next(sender_address)
    }

I talked with @ArniStarkware about this. The reason for this change is because of a style error in the documentation. We document this method as a public function docstring, meaning that we intend to make this method public

Code quote:

    pub fn next_nonce(&mut self) -> Nonce {
        let sender_address = self.sender_address();
        self.nonce_manager.borrow_mut().next(sender_address)
    }

Copy link
Contributor

@ArniStarkware ArniStarkware 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: 144 of 180 files reviewed, all discussions resolved (waiting on @dorimedini-starkware, @Itay-Tsabary-Starkware, @meship-starkware, and @noaov1)


crates/mempool_test_utils/src/starknet_api_test_utils.rs line 404 at r6 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

I talked with @ArniStarkware about this. The reason for this change is because of a style error in the documentation. We document this method as a public function docstring, meaning that we intend to make this method public

Also, the said documentation refers to using this method as part of the public API of AccountTransactionGenerator.

Copy link
Collaborator

@dorimedini-starkware dorimedini-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: 144 of 180 files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware, @meship-starkware, and @noaov1)

Copy link
Collaborator

@dorimedini-starkware dorimedini-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 17 of 31 files at r2, 6 of 13 files at r5, 6 of 7 files at r6.
Reviewable status: 173 of 180 files reviewed, 1 unresolved discussion (waiting on @Itay-Tsabary-Starkware, @meship-starkware, and @noaov1)


crates/blockifier/cairo_native line 0 at r6 (raw file):
is this equal to the commit on main-v0.13.4?

Copy link

github-actions bot commented Jan 9, 2025

Benchmark movements:
full_committer_flow performance regressed!
full_committer_flow time: [30.888 ms 30.986 ms 31.129 ms]
change: [+2.6739% +3.4132% +4.0252%] (p = 0.00 < 0.05)
Performance has regressed.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severe

Copy link
Contributor

@Itay-Tsabary-Starkware Itay-Tsabary-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 143 of 174 files at r1, 21 of 31 files at r2, 9 of 13 files at r5, 7 of 7 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @meship-starkware and @noaov1)

Copy link
Collaborator

@dorimedini-starkware dorimedini-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: all files reviewed, 1 unresolved discussion (waiting on @meship-starkware and @noaov1)

Copy link
Contributor Author

@meship-starkware meship-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: all files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware and @noaov1)


crates/blockifier/cairo_native line at r6 (raw file):

Previously, dorimedini-starkware wrote…

is this equal to the commit on main-v0.13.4?

yes it was changed and I change it back to the right commit

Copy link
Collaborator

@dorimedini-starkware dorimedini-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 @noaov1)

@dorimedini-starkware dorimedini-starkware added this pull request to the merge queue Jan 9, 2025
Merged via the queue into main with commit caf0deb Jan 9, 2025
22 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 11, 2025
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.