Skip to content

Commit 404d244

Browse files
committed
fix(consensus): correct weighted round-robin proposer selection
1 parent 3c86d2f commit 404d244

1 file changed

Lines changed: 31 additions & 18 deletions

File tree

crates/consensus/src/proposer_selector.rs

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ pub struct ProposerSelector {
3030
total_voting_power: RwLock<u64>,
3131
/// Last (height, round) where priorities were synchronized
3232
last_synced: RwLock<(ConsensusHeight, Round)>,
33+
/// Proposer from the last advance (for idempotent queries)
34+
last_proposer: RwLock<Option<ConsensusAddress>>,
3335
}
3436

3537
impl ProposerSelector {
@@ -53,6 +55,7 @@ impl ProposerSelector {
5355
priorities: RwLock::new(priorities),
5456
total_voting_power: RwLock::new(total_voting_power),
5557
last_synced: RwLock::new((initial_height, Round::Nil)),
58+
last_proposer: RwLock::new(None),
5659
}
5760
}
5861

@@ -69,30 +72,32 @@ impl ProposerSelector {
6972
let mut priorities = self.priorities.write();
7073
let total_power = *self.total_voting_power.read();
7174
let mut last_synced = self.last_synced.write();
75+
let mut last_proposer = self.last_proposer.write();
7276

7377
let (last_height, last_round) = *last_synced;
7478

7579
// Calculate total advances needed from last synced point to (height, round)
7680
let advances = Self::compute_advances(last_height, last_round, height, round);
7781

82+
// Track the proposer from advances (the proposer is determined DURING advance,
83+
// not after, because advance_one_round penalizes the proposer)
84+
let mut proposer_addr = None;
7885
for _ in 0..advances {
79-
Self::advance_one_round(&mut priorities, total_power);
86+
proposer_addr = Self::advance_one_round(&mut priorities, total_power);
8087
}
8188

89+
// If no advances needed, use the cached last_proposer (idempotent query)
90+
let proposer_addr = if advances == 0 {
91+
last_proposer.expect("last_proposer should be set after first advance")
92+
} else {
93+
let addr = proposer_addr.expect("should have proposer after advances");
94+
*last_proposer = Some(addr);
95+
addr
96+
};
97+
8298
// Update sync point
8399
*last_synced = (height, round);
84100

85-
// Find proposer (highest priority, tie-break by address for determinism)
86-
let proposer_addr = priorities
87-
.iter()
88-
.max_by(|a, b| {
89-
a.priority
90-
.cmp(&b.priority)
91-
.then_with(|| a.address.cmp(&b.address))
92-
})
93-
.map(|p| p.address)
94-
.expect("validator set must not be empty");
95-
96101
// Return reference from the validator set
97102
validator_set
98103
.as_slice()
@@ -137,17 +142,20 @@ impl ProposerSelector {
137142
}
138143

139144
// Different heights:
140-
// Each height transition + target round advances
141-
// height_diff accounts for moving through heights, to_r for rounds in target
142-
// Add 1 because entering a new height's round 0 is itself an advance
145+
// Each height transition counts as one advance, plus any rounds within the target height.
146+
// Example: (1,0) -> (2,0) = 1 advance, (1,0) -> (2,1) = 2 advances, (1,0) -> (3,0) = 2 advances
143147
let height_diff = to_h - from_h;
144-
height_diff + (to_r as u64) + 1
148+
height_diff + (to_r as u64)
145149
}
146150

147151
/// Single round advancement of the Tendermint algorithm.
148-
fn advance_one_round(priorities: &mut [ValidatorPriority], total_power: u64) {
152+
/// Returns the proposer address for this round.
153+
fn advance_one_round(
154+
priorities: &mut [ValidatorPriority],
155+
total_power: u64,
156+
) -> Option<ConsensusAddress> {
149157
if priorities.is_empty() || total_power == 0 {
150-
return;
158+
return None;
151159
}
152160

153161
// Step 1: Increase all priorities by their voting power
@@ -167,10 +175,14 @@ impl ProposerSelector {
167175
.map(|(idx, _)| idx)
168176
.unwrap();
169177

178+
let proposer_addr = priorities[proposer_idx].address;
179+
170180
// Step 3: Decrease proposer's priority by total voting power
171181
priorities[proposer_idx].priority = priorities[proposer_idx]
172182
.priority
173183
.saturating_sub(total_power as i64);
184+
185+
Some(proposer_addr)
174186
}
175187

176188
/// Center priorities around zero to prevent overflow.
@@ -264,6 +276,7 @@ impl Clone for ProposerSelector {
264276
priorities: RwLock::new(self.priorities.read().clone()),
265277
total_voting_power: RwLock::new(*self.total_voting_power.read()),
266278
last_synced: RwLock::new(*self.last_synced.read()),
279+
last_proposer: RwLock::new(*self.last_proposer.read()),
267280
}
268281
}
269282
}

0 commit comments

Comments
 (0)