Skip to content

feat(starknet_mempool_p2p): add class manager to propogator#3500

Merged
noamsp-starkware merged 7 commits intonoam.s/refactor_starknet_gateway_remove_unused_gateway_compilerfrom
noam.s/feat_starknet_mempool_p2p_add_class_manager_to_propogator
Jan 22, 2025
Merged

feat(starknet_mempool_p2p): add class manager to propogator#3500
noamsp-starkware merged 7 commits intonoam.s/refactor_starknet_gateway_remove_unused_gateway_compilerfrom
noam.s/feat_starknet_mempool_p2p_add_class_manager_to_propogator

Conversation

@noamsp-starkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

@github-actions
Copy link

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [34.741 ms 34.762 ms 34.786 ms]
change: [-5.0857% -3.5345% -2.1608%] (p = 0.00 < 0.05)
Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
2 (2.00%) high mild
3 (3.00%) high severe

@github-actions
Copy link

github-actions bot commented Jan 20, 2025

@noamsp-starkware noamsp-starkware force-pushed the noam.s/refactor_starknet_gateway_remove_unused_gateway_compiler branch from 2f6cf19 to 3786f9e Compare January 21, 2025 05:34
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_mempool_p2p_add_class_manager_to_propogator branch from 54a9e74 to ffa9b3e Compare January 21, 2025 05:34
@github-actions
Copy link

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [36.200 ms 36.883 ms 37.614 ms]
change: [+1.4169% +3.8615% +6.4798%] (p = 0.00 < 0.05)
Performance has regressed.
Found 14 outliers among 100 measurements (14.00%)
11 (11.00%) high mild
3 (3.00%) high severe

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


crates/starknet_mempool_p2p/src/propagator/mod.rs line 20 at r2 (raw file):

    broadcast_topic_client: BroadcastTopicClient<RpcTransactionWrapper>,
    #[allow(dead_code)]
    transaction_converter: TransactionConverter,

As I wrote in wg slack, change to _transaction_converter and remove the allow dead code


crates/starknet_mempool_p2p/src/propagator/test.rs line 33 at r2 (raw file):

    let BroadcastNetworkMock { mut messages_to_broadcast_receiver, .. } = mock_network;
    let rpc_transaction = RpcTransaction::get_test_instance(&mut get_rng());
    // TODO(noamsp): use MockClassManagerClient

Change the TODO to "use MockTransactionConverterTrait"


crates/starknet_mempool_p2p/src/propagator/test.rs line 53 at r2 (raw file):

    let BroadcastNetworkMock { mut continue_propagation_receiver, .. } = mock_network;
    let propagation_metadata = BroadcastedMessageMetadata::get_test_instance(&mut get_rng());
    // TODO(noamsp): use MockClassManagerClient

Same here

@noamsp-starkware noamsp-starkware force-pushed the noam.s/refactor_starknet_gateway_remove_unused_gateway_compiler branch from 3786f9e to 24a9f6a Compare January 21, 2025 12:52
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_mempool_p2p_add_class_manager_to_propogator branch from ffa9b3e to 541ae00 Compare January 21, 2025 12:52
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: 4 of 6 files reviewed, 3 unresolved discussions (waiting on @ShahakShama)


crates/starknet_mempool_p2p/src/propagator/mod.rs line 20 at r2 (raw file):

Previously, ShahakShama wrote…

As I wrote in wg slack, change to _transaction_converter and remove the allow dead code

Done.


crates/starknet_mempool_p2p/src/propagator/test.rs line 33 at r2 (raw file):

Previously, ShahakShama wrote…

Change the TODO to "use MockTransactionConverterTrait"

Done.


crates/starknet_mempool_p2p/src/propagator/test.rs line 53 at r2 (raw file):

Previously, ShahakShama wrote…

Same here

Done.

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 2 of 2 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @noamsp-starkware)

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:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @noamsp-starkware)

@noamsp-starkware noamsp-starkware force-pushed the noam.s/refactor_starknet_gateway_remove_unused_gateway_compiler branch from 24a9f6a to 352a55a Compare January 21, 2025 15:33
@noamsp-starkware noamsp-starkware merged commit 7cf7c0f into noam.s/refactor_starknet_gateway_remove_unused_gateway_compiler Jan 22, 2025
13 of 17 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 25, 2025
@noamsp-starkware noamsp-starkware deleted the noam.s/feat_starknet_mempool_p2p_add_class_manager_to_propogator branch January 27, 2025 11:16
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.

3 participants