Skip to content

feat(starknet_gateway): state reader get compiled classes from class manager#3578

Merged
noamsp-starkware merged 1 commit intomainfrom
noam.s/feat_starknet_gateway_state_reader_get_compiled_classes_from_class_manager
Jan 27, 2025
Merged

feat(starknet_gateway): state reader get compiled classes from class manager#3578
noamsp-starkware merged 1 commit intomainfrom
noam.s/feat_starknet_gateway_state_reader_get_compiled_classes_from_class_manager

Conversation

@noamsp-starkware
Copy link
Contributor

@noamsp-starkware noamsp-starkware commented Jan 21, 2025

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Contributor Author

noamsp-starkware commented Jan 21, 2025

@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_state_reader_get_compiled_classes_from_class_manager branch 2 times, most recently from 103367c to 78fb37d Compare January 21, 2025 15:51
@noamsp-starkware noamsp-starkware marked this pull request as ready for review January 21, 2025 15:52
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_state_reader_get_compiled_classes_from_class_manager branch from 78fb37d to 4f7fe17 Compare January 21, 2025 16:29
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_state_reader_get_compiled_classes_from_class_manager branch 2 times, most recently from 4921fbd to 1989395 Compare January 22, 2025 11:05
@noamsp-starkware noamsp-starkware changed the base branch from noam.s/feat_starknet_mempool_handle_declare_tx_properly to noam.s/feat_starknet_gateway_add_class_manager_client January 22, 2025 11:05
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_add_class_manager_client branch 2 times, most recently from c866daf to b8fde03 Compare January 22, 2025 11:24
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_state_reader_get_compiled_classes_from_class_manager branch from 1989395 to 6f01955 Compare January 22, 2025 11:24
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_add_class_manager_client branch from b8fde03 to a001ab3 Compare January 22, 2025 12:20
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_state_reader_get_compiled_classes_from_class_manager branch from 6f01955 to 4870e47 Compare January 22, 2025 12:20
Copy link
Collaborator

@ShahakShama ShahakShama 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 6 files at r1, 4 of 5 files at r2, 65 of 65 files at r3.
Reviewable status: 70 of 72 files reviewed, all discussions resolved

Copy link
Collaborator

@ShahakShama ShahakShama 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 6 files at r1, 1 of 5 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @noamsp-starkware)


a discussion (no related file):
@alonh5 should also review this


crates/starknet_gateway/src/sync_state_reader.rs line 115 at r3 (raw file):

            Ok(value) => value,
            // TODO(noamsp): Remove this once the class manager component is ready.
            Err(ClassManagerClientError::ClassManagerError(

After talking with @alonh5, because we currently use EmptyClassManagerClient which returns default(), we should do this not when we receive a ClassNotFound, but when we receive Ok(default())

@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_add_class_manager_client branch from a001ab3 to 6590c16 Compare January 22, 2025 15:42
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_state_reader_get_compiled_classes_from_class_manager branch from 4870e47 to 8ec4b4b Compare January 22, 2025 15:43
@noamsp-starkware noamsp-starkware changed the base branch from noam.s/feat_starknet_gateway_add_class_manager_client to noam.s/fix_starknet_gateway_remove_compiled_class_hash_mismatch_test January 22, 2025 15:43
Copy link
Contributor Author

@noamsp-starkware noamsp-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: 7 of 72 files reviewed, 2 unresolved discussions (waiting on @alonh5 and @ShahakShama)


crates/starknet_gateway/src/sync_state_reader.rs line 115 at r3 (raw file):

Previously, ShahakShama wrote…

After talking with @alonh5, because we currently use EmptyClassManagerClient which returns default(), we should do this not when we receive a ClassNotFound, but when we receive Ok(default())

Done.

@noamsp-starkware noamsp-starkware force-pushed the noam.s/fix_starknet_gateway_remove_compiled_class_hash_mismatch_test branch from 3d1bacb to 4828d7f Compare January 22, 2025 15:47
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_state_reader_get_compiled_classes_from_class_manager branch from 8ec4b4b to 48df8d1 Compare January 22, 2025 15:47
@noamsp-starkware noamsp-starkware changed the base branch from noam.s/fix_starknet_gateway_remove_compiled_class_hash_mismatch_test to noam.s/remove_compiled_class_hash_mismatch January 22, 2025 15:58
@noamsp-starkware noamsp-starkware force-pushed the noam.s/remove_compiled_class_hash_mismatch branch from 4828d7f to 6cda1dc Compare January 22, 2025 16:00
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_state_reader_get_compiled_classes_from_class_manager branch from 48df8d1 to a294a76 Compare January 22, 2025 16:01
Copy link
Collaborator

@ShahakShama ShahakShama 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 65 of 65 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @noamsp-starkware)

@noamsp-starkware noamsp-starkware force-pushed the noam.s/remove_compiled_class_hash_mismatch branch from 6cda1dc to 534dbda Compare January 23, 2025 09:46
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_state_reader_get_compiled_classes_from_class_manager branch from a294a76 to baec399 Compare January 23, 2025 09:46
@noamsp-starkware noamsp-starkware changed the base branch from noam.s/remove_compiled_class_hash_mismatch to noam.s/feat_starknet_gateway_add_class_manager_client January 23, 2025 09:46
@github-actions
Copy link

Artifacts upload workflows:

@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_add_class_manager_client branch from 69749db to f3f6070 Compare January 23, 2025 09:49
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_state_reader_get_compiled_classes_from_class_manager branch from baec399 to e07c01d Compare January 23, 2025 09:49
@ShahakShama ShahakShama force-pushed the noam.s/feat_starknet_gateway_add_class_manager_client branch 2 times, most recently from 8f3af62 to 8c34d6b Compare January 26, 2025 08:22
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_state_reader_get_compiled_classes_from_class_manager branch from e07c01d to 1e797f4 Compare January 26, 2025 12:49
@noamsp-starkware noamsp-starkware changed the base branch from noam.s/feat_starknet_gateway_add_class_manager_client to main January 26, 2025 12:49
Copy link
Collaborator

@ShahakShama ShahakShama 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 24 of 24 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @noamsp-starkware)

Copy link
Contributor

@alonh5 alonh5 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @noamsp-starkware)

@noamsp-starkware noamsp-starkware added this pull request to the merge queue Jan 27, 2025
Merged via the queue into main with commit b6e4338 Jan 27, 2025
7 checks passed
@noamsp-starkware noamsp-starkware deleted the noam.s/feat_starknet_gateway_state_reader_get_compiled_classes_from_class_manager branch January 27, 2025 11:17
@github-actions github-actions bot locked and limited conversation to collaborators Jan 29, 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.

4 participants