Skip to content

apollo_consensus: add faulty nodes to the sim test #11016

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.

Sign up for GitHub

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

Jump to bottom
Closed
asmaastarkware wants to merge 1 commit into from
Closed

apollo_consensus: add faulty nodes to the sim test #11016

asmaastarkware wants to merge 1 commit into from

Conversation

@asmaastarkware
Copy link
Contributor

@asmaastarkware asmaastarkware commented Dec 23, 2025

No description provided.

@reviewable-StarkWare
Copy link

reviewable-StarkWare commented Dec 23, 2025

This change is Reviewable

This was referenced Dec 23, 2025
Merged
Merged
@asmaastarkware Graphite App
Copy link
Contributor Author

asmaastarkware commented Dec 23, 2025 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@asmaastarkware asmaastarkware mentioned this pull request Dec 23, 2025
Merged
@asmaastarkware asmaastarkware marked this pull request as ready for review December 23, 2025 13:24
@asmaastarkware asmaastarkware force-pushed the asmaa/sim_test/add_drop_rate branch from d6d2d07 to 900acc4 Compare December 23, 2025 13:41
@asmaastarkware asmaastarkware force-pushed the asmaa/sim_test/add_faulty_nodes_with_should_count_flag branch from 39f4361 to b26a6f6 Compare December 23, 2025 13:41
matanl-starkware
matanl-starkware reviewed Dec 23, 2025
View reviewed changes
Copy link
Collaborator

@matanl-starkware matanl-starkware left a comment

Choose a reason for hiding this comment

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

This appears to be a continuation of PR 10964.
I'll review it after you apply the comment fixes for the original.

@matanl-starkware made 1 comment.
Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @dafnamatsry).

@asmaastarkware asmaastarkware force-pushed the asmaa/sim_test/add_faulty_nodes_with_should_count_flag branch from b26a6f6 to eafd514 Compare December 24, 2025 08:15
asmaastarkware
asmaastarkware commented Dec 24, 2025
View reviewed changes
Copy link
Contributor Author

@asmaastarkware asmaastarkware left a comment

Choose a reason for hiding this comment

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

I will close this PR; the design in #10964 is better than this.
Here, we assume that the first vote should be counted and the second should not, but that assumption is incorrect. We might drop the first vote, then the “second” becomes the first and should be counted.
Yes, we could move the keep/drop logic inside the generate function, but that would make the code ugly.
Let’s continue with the other design.

@asmaastarkware made 1 comment.
Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @dafnamatsry).

@asmaastarkware asmaastarkware mentioned this pull request Dec 24, 2025
Merged
@asmaastarkware asmaastarkware mentioned this pull request Dec 25, 2025
Merged
@github-actions github-actions bot locked and limited conversation to collaborators Dec 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@matanl-starkware matanl-starkware matanl-starkware left review comments

@dafnamatsry dafnamatsry Awaiting requested review from dafnamatsry

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@asmaastarkware @reviewable-StarkWare @matanl-starkware