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

feat(starknet_mempool): define the mempool config to be a SerializeConfig #4260

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dafnamatsry
Copy link
Collaborator

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Collaborator Author

Copy link

github-actions bot commented Feb 19, 2025

Copy link
Collaborator

@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.

Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @yair-starkware)


crates/starknet_mempool/src/config.rs line 54 at r1 (raw file):

            ),
        ]);
        vec![members].into_iter().flatten().collect()

Why do you need this line?

Code quote:

vec![members].into_iter().flatten().collect()

Copy link
Contributor

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

@dafnamatsry dafnamatsry force-pushed the mempool-config-definition branch from 91e4746 to 9375274 Compare February 20, 2025 12:52
Copy link
Collaborator Author

@dafnamatsry dafnamatsry 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: 6 of 7 files reviewed, 1 unresolved discussion (waiting on @alonh5 and @yair-starkware)


crates/starknet_mempool/src/config.rs line 54 at r1 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Why do you need this line?

Removed.
Copy pasted it from a different config file (there it is needed because it contains sub configs)

Copy link

Benchmark movements:
full_committer_flow performance regressed!
full_committer_flow time: [30.807 ms 30.860 ms 30.916 ms]
change: [+1.4816% +1.8334% +2.1354%] (p = 0.00 < 0.05)
Performance has regressed.
Found 3 outliers among 100 measurements (3.00%)
3 (3.00%) high mild

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.

4 participants