Skip to content

Conversation

@asmaastarkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Contributor Author

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

@asmaastarkware asmaastarkware marked this pull request as ready for review January 22, 2026 14:00
@asmaastarkware asmaastarkware force-pushed the asmaa/support_real_virtual_leader_election branch from 6a971da to 01b9d49 Compare January 22, 2026 14:05
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 PR is way too big. For me, it's impossible to follow it through.
Suggestion:

  1. Create a struct containing the existing LeaderFn and pass it everywhere instead of the current direct usage of the LeaderFn. (Refactor - no change in logic).
  2. Add to the struct also the virtual_leader_fn (Refactor - no change in logic).
  3. Specifically, use the virtual_leader_fn only in the place where deciding upon PreCommit count. Should be very minor and co-located.

@matanl-starkware made 4 comments.
Reviewable status: 0 of 10 files reviewed, 3 unresolved discussions (waiting on @asmaastarkware and @dafnamatsry).


crates/apollo_consensus/src/single_height_consensus.rs line 107 at r1 (raw file):

            return VecDeque::new();
        }
        let proposer_id = virtual_leader_fn(block_info.round);

This should remain unchanged

Suggestion:

leader_fn

crates/apollo_consensus/src/manager.rs line 889 at r1 (raw file):

                    height,
                    round,
                    proposer: context.virtual_proposer(height, round),

The proposer should not change.
The virtual_proposer should only come into effect when counting PreCommit votes.

Code quote:

proposer: context.virtual_proposer(height, round),

crates/apollo_consensus/src/state_machine.rs line 534 at r1 (raw file):

                        height: self.height,
                        round: self.round,
                        proposer: virtual_leader_fn(self.round),

Keep as is

Code quote:

virtual_leader_fn(self.round),

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