From 06c4def6911878e7ad5d46921521485068abd314 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 6 Jun 2025 14:26:29 +0000 Subject: [PATCH 1/2] voter: Save the context waker of precommited voting rounds Signed-off-by: Alexandru Vasile --- src/voter/mod.rs | 33 ++++++++++++++++++++++----------- src/voter/voting_round.rs | 20 +++++++++++++++++++- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/src/voter/mod.rs b/src/voter/mod.rs index 39c6d4f..953e31f 100644 --- a/src/voter/mod.rs +++ b/src/voter/mod.rs @@ -775,20 +775,31 @@ where { let mut inner = self.inner.lock(); - let should_start_next = { - let completable = match inner.best_round.poll(cx)? { - Poll::Ready(()) => true, - Poll::Pending => false, - }; + // Best round must be completable before we can start a new one. + match inner.best_round.poll(cx)? { + Poll::Ready(()) => {}, + Poll::Pending => return Poll::Pending, + }; - // start when we've cast all votes. - let precommitted = - matches!(inner.best_round.state(), Some(&VotingRoundState::Precommitted)); + // The state of the best round must advance to the point where we can + // start a new round. + // We can effectively start a new round when: + // - the best round future completed + // - the best round state is precommitted, which means we have cast all votes + let precommitted = + matches!(inner.best_round.state(), Some(&VotingRoundState::Precommitted)); + if !precommitted { + trace!( + target: LOG_TARGET, + "Best round at {} is not precommitted. Waiting for precommit in state {:?}", + inner.best_round.round_number(), + inner.best_round.state(), + ); - completable && precommitted - }; + // This ensures we are not causing delays in the voting process. Previously, + // we relied on polling the future again which would save the waker via `process_incoming`. + inner.best_round.set_waker(cx.waker().clone()); - if !should_start_next { return Poll::Pending } diff --git a/src/voter/voting_round.rs b/src/voter/voting_round.rs index f728b6b..f5cff5d 100644 --- a/src/voter/voting_round.rs +++ b/src/voter/voting_round.rs @@ -74,6 +74,11 @@ where primary_block: Option<(H, N)>, // a block posted by primary as a hint. finalized_sender: UnboundedSender>, best_finalized: Option>, + + /// Waker to notify that we have switched to the precommited state + /// if we have returned `Poll::Pending` in the previous poll of + /// the `Voter::process_best_round`. + waker: Option, } /// Whether we should vote in the current round (i.e. push votes to the sink.) @@ -138,6 +143,7 @@ where env, last_round_state, finalized_sender, + waker: None, } } @@ -163,6 +169,7 @@ where last_round_state, finalized_sender, best_finalized: None, + waker: None, } } @@ -653,6 +660,11 @@ where self.votes.set_precommitted_index(); self.outgoing.push(Message::Precommit(precommit)); } + + if let Some(ref waker) = self.waker { + waker.wake_by_ref(); + } + self.state = Some(State::Precommitted); } else { self.state = Some(State::Prevoted(precommit_timer)); @@ -666,7 +678,13 @@ where Ok(()) } - // construct a prevote message based on local state. + /// Sets the internal waker to be used when we return `Poll::Pending` in the + /// `Voter::process_best_round` method. + pub(super) fn set_waker(&mut self, waker: std::task::Waker) { + self.waker = Some(waker); + } + + /// Construct a prevote message based on local state. fn construct_prevote(&self, last_round_state: &RoundState) -> (H, E::BestChain) { let last_round_estimate = last_round_state .estimate From e64034c29054b75581e6769077cf917f886fcdbc Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 6 Jun 2025 14:30:39 +0000 Subject: [PATCH 2/2] voter: Inline completed_best_round to avoid double lock Signed-off-by: Alexandru Vasile --- src/voter/mod.rs | 50 +++++++++++++++++++++--------------------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/src/voter/mod.rs b/src/voter/mod.rs index 953e31f..71ca87a 100644 --- a/src/voter/mod.rs +++ b/src/voter/mod.rs @@ -809,40 +809,34 @@ where inner.best_round.round_number(), inner.best_round.round_number() + 1, ); - } - self.completed_best_round()?; + // Complete the best round and start a new one. + self.env.completed( + inner.best_round.round_number(), + inner.best_round.round_state(), + inner.best_round.dag_base(), + inner.best_round.historical_votes(), + )?; + + let old_round_number = inner.best_round.round_number(); + + let next_round = VotingRound::new( + old_round_number + 1, + self.voters.clone(), + self.last_finalized_in_rounds.clone(), + Some(inner.best_round.bridge_state()), + inner.best_round.finalized_sender(), + self.env.clone(), + ); + + let old_round = ::std::mem::replace(&mut inner.best_round, next_round); + inner.past_rounds.push(&*self.env, old_round); + } // round has been updated. so we need to re-poll. self.poll_unpin(cx) } - fn completed_best_round(&mut self) -> Result<(), E::Error> { - let mut inner = self.inner.lock(); - - self.env.completed( - inner.best_round.round_number(), - inner.best_round.round_state(), - inner.best_round.dag_base(), - inner.best_round.historical_votes(), - )?; - - let old_round_number = inner.best_round.round_number(); - - let next_round = VotingRound::new( - old_round_number + 1, - self.voters.clone(), - self.last_finalized_in_rounds.clone(), - Some(inner.best_round.bridge_state()), - inner.best_round.finalized_sender(), - self.env.clone(), - ); - - let old_round = ::std::mem::replace(&mut inner.best_round, next_round); - inner.past_rounds.push(&*self.env, old_round); - Ok(()) - } - fn set_last_finalized_number(&mut self, finalized_number: N) -> bool { let last_finalized_number = &mut self.last_finalized_number; if finalized_number > *last_finalized_number {