Skip to content

Commit b0db814

Browse files
committed
fix(starfish): skip block proposal when behind quorum commit round (#11384)
# Description of change Under `consensus_block_restrictions`, Core's `try_new_block` requires the candidate round to exceed an approximation of the network's quorum commit round (`local_commit_round + quorum_commit_index - local_commit_index`). Threads `commit_vote_monitor` through `Core::new`. Blocks at or below the quorum commit round cannot improve the commit rule and waste peer bandwidth, so skipping them is a pure efficiency improvement when behind. Stacked on top of #11383. ## Links to any relevant issues Fixes #11190 Part of #11323 ## How the change has been tested - [x] Basic tests (linting, compilation, formatting, unit/integration tests) - [ ] Patch-specific tests (correctness, functionality coverage) - [ ] I have added tests that prove my fix is effective or that my feature works - [x] I have checked that new and existing unit tests pass locally with my changes
1 parent 71739ba commit b0db814

4 files changed

Lines changed: 44 additions & 4 deletions

File tree

crates/starfish/core/src/authority_node.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,8 @@ impl ConsensusAuthority {
166166

167167
let fast_sync_ongoing = dag_state.read().fast_sync_ongoing();
168168

169+
let commit_vote_monitor = Arc::new(CommitVoteMonitor::new(context.clone()));
170+
169171
let core = Core::new(
170172
context.clone(),
171173
leader_schedule,
@@ -180,6 +182,7 @@ impl ConsensusAuthority {
180182
protocol_keypair,
181183
dag_state.clone(),
182184
sync_last_known_own_block,
185+
commit_vote_monitor.clone(),
183186
);
184187

185188
let (core_dispatcher, core_thread_handle) =
@@ -203,8 +206,6 @@ impl ConsensusAuthority {
203206
let shard_reconstructor =
204207
ShardReconstructor::start(context.clone(), dag_state.clone(), core_dispatcher.clone());
205208

206-
let commit_vote_monitor = Arc::new(CommitVoteMonitor::new(context.clone()));
207-
208209
// `fast_sync_active` is a shared flag used by the fast syncer to
209210
// signal when it has any work in flight. The regular commit syncer
210211
// and the header synchronizer both read it to pause their

crates/starfish/core/src/authority_service.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2458,6 +2458,7 @@ mod tests {
24582458
key_pairs[context.own_index.value()].1.clone(),
24592459
dag_state.clone(),
24602460
true,
2461+
Arc::new(CommitVoteMonitor::new(context.clone())),
24612462
);
24622463
core.set_last_known_proposed_round(rounds + 5);
24632464

@@ -2624,6 +2625,7 @@ mod tests {
26242625
key_pairs[context.own_index.value()].1.clone(),
26252626
dag_state.clone(),
26262627
true,
2628+
Arc::new(CommitVoteMonitor::new(context.clone())),
26272629
);
26282630
core.set_last_known_proposed_round(rounds + 5);
26292631

@@ -2803,6 +2805,7 @@ mod tests {
28032805
key_pairs[context.own_index.value()].1.clone(),
28042806
dag_state.clone(),
28052807
true,
2808+
Arc::new(CommitVoteMonitor::new(context.clone())),
28062809
);
28072810

28082811
let core_dispatcher = Arc::new(FakeCoreThreadDispatcher {
@@ -3132,6 +3135,7 @@ mod tests {
31323135
key_pairs[context.own_index.value()].1.clone(),
31333136
dag_state.clone(),
31343137
true,
3138+
Arc::new(CommitVoteMonitor::new(context.clone())),
31353139
);
31363140

31373141
let core_dispatcher = Arc::new(FakeCoreThreadDispatcher {
@@ -3273,6 +3277,7 @@ mod tests {
32733277
key_pairs[context.own_index.value()].1.clone(),
32743278
dag_state.clone(),
32753279
true,
3280+
Arc::new(CommitVoteMonitor::new(context.clone())),
32763281
);
32773282

32783283
let core_dispatcher = Arc::new(FakeCoreThreadDispatcher {
@@ -3439,6 +3444,7 @@ mod tests {
34393444
key_pairs[context.own_index.value()].1.clone(),
34403445
dag_state.clone(),
34413446
true,
3447+
Arc::new(CommitVoteMonitor::new(context.clone())),
34423448
);
34433449
core.set_last_known_proposed_round(rounds + 5);
34443450

@@ -3632,6 +3638,7 @@ mod tests {
36323638
key_pairs[context.own_index.value()].1.clone(),
36333639
dag_state.clone(),
36343640
true,
3641+
Arc::new(CommitVoteMonitor::new(context.clone())),
36353642
);
36363643

36373644
let core_dispatcher = Arc::new(FakeCoreThreadDispatcher {
@@ -3857,6 +3864,7 @@ mod tests {
38573864
key_pairs[context.own_index.value()].1.clone(),
38583865
dag_state.clone(),
38593866
true,
3867+
Arc::new(CommitVoteMonitor::new(context.clone())),
38603868
);
38613869

38623870
let core_dispatcher = Arc::new(FakeCoreThreadDispatcher {

crates/starfish/core/src/core.rs

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ use crate::{
4242
commit::{CertifiedCommits, CommitAPI, PendingSubDag},
4343
commit_observer::{CommitObserver, CommittedSubDagSource},
4444
commit_syncer::fast::FastSyncOutput,
45+
commit_vote_monitor::CommitVoteMonitor,
4546
context::Context,
4647
dag_state::{DagState, DataSource},
4748
encoder::{ShardEncoder, create_encoder},
@@ -108,6 +109,7 @@ pub(crate) struct Core {
108109
last_known_proposed_round: Option<Round>,
109110
/// Encoder is used to encode transactions into a longer vector of shards
110111
encoder: Box<dyn ShardEncoder + Send + Sync>,
112+
commit_vote_monitor: Arc<CommitVoteMonitor>,
111113
}
112114

113115
#[derive(Eq, PartialEq, Copy, Clone, Debug)]
@@ -165,6 +167,7 @@ impl Core {
165167
block_signer: ProtocolKeyPair,
166168
dag_state: Arc<RwLock<DagState>>,
167169
sync_last_known_own_block: bool,
170+
commit_vote_monitor: Arc<CommitVoteMonitor>,
168171
) -> Self {
169172
let last_decided_leader = dag_state.read().last_commit_leader();
170173
let committer = UniversalCommitterBuilder::new(
@@ -220,6 +223,7 @@ impl Core {
220223
dag_state,
221224
last_known_proposed_round: min_propose_round,
222225
encoder,
226+
commit_vote_monitor,
223227
}
224228
.recover()
225229
}
@@ -747,11 +751,27 @@ impl Core {
747751
.with_label_values(&["Core::try_new_block"])
748752
.start_timer();
749753

750-
// Ensure the new block has a higher round than the last proposed block.
754+
// Ensure the new block has a higher round than the last proposed block
755+
// and, under `consensus_block_restrictions`, also the approximate quorum commit
756+
// round. Blocks at or below the quorum commit round will not improve
757+
// the commit rule.
751758
let clock_round = {
752759
let dag_state = self.dag_state.read();
753760
let clock_round = dag_state.threshold_clock_round();
754-
if clock_round <= dag_state.get_last_proposed_block_header().round() {
761+
let last_proposed_round = dag_state.get_last_proposed_block_header().round();
762+
let min_round = if self.context.protocol_config.consensus_block_restrictions() {
763+
let quorum_commit_index = self.commit_vote_monitor.quorum_commit_index();
764+
let local_commit_index = dag_state.last_commit_index();
765+
let local_commit_round = dag_state.last_commit_round();
766+
// Lower bound on the quorum commit round: at least 1 round per
767+
// commit, so the gap in indices maps to at least that many rounds.
768+
let approx_quorum_round =
769+
local_commit_round + quorum_commit_index.saturating_sub(local_commit_index);
770+
last_proposed_round.max(approx_quorum_round)
771+
} else {
772+
last_proposed_round
773+
};
774+
if clock_round <= min_round {
755775
return None;
756776
}
757777
clock_round
@@ -1497,6 +1517,7 @@ impl CoreTextFixture {
14971517

14981518
let block_signer = signers.remove(own_index.value()).1;
14991519

1520+
let commit_vote_monitor = Arc::new(CommitVoteMonitor::new(context.clone()));
15001521
let core = Core::new(
15011522
context,
15021523
leader_schedule,
@@ -1508,6 +1529,7 @@ impl CoreTextFixture {
15081529
block_signer,
15091530
dag_state,
15101531
sync_last_known_own_block,
1532+
commit_vote_monitor,
15111533
);
15121534

15131535
Self {
@@ -1643,6 +1665,7 @@ mod test {
16431665
key_pairs.remove(context.own_index.value()).1,
16441666
dag_state.clone(),
16451667
false,
1668+
Arc::new(CommitVoteMonitor::new(context.clone())),
16461669
);
16471670

16481671
// New round should be num_round + 1
@@ -1772,6 +1795,7 @@ mod test {
17721795
key_pairs.remove(context.own_index.value()).1,
17731796
dag_state.clone(),
17741797
false,
1798+
Arc::new(CommitVoteMonitor::new(context.clone())),
17751799
);
17761800

17771801
// Clock round should have advanced to 5 during recovery because
@@ -1916,6 +1940,7 @@ mod test {
19161940
key_pairs.remove(context.own_index.value()).1,
19171941
dag_state.clone(),
19181942
false,
1943+
Arc::new(CommitVoteMonitor::new(context.clone())),
19191944
);
19201945

19211946
// Manually check the transaction commitment that is expected to be computed in
@@ -2016,6 +2041,7 @@ mod test {
20162041
key_pairs.remove(context.own_index.value()).1,
20172042
dag_state.clone(),
20182043
false,
2044+
Arc::new(CommitVoteMonitor::new(context.clone())),
20192045
);
20202046

20212047
let mut expected_ancestors = BTreeSet::new();
@@ -2177,6 +2203,7 @@ mod test {
21772203
key_pairs.remove(context.own_index.value()).1,
21782204
dag_state,
21792205
true,
2206+
Arc::new(CommitVoteMonitor::new(context.clone())),
21802207
);
21812208

21822209
// No new block should have been produced
@@ -2394,6 +2421,7 @@ mod test {
23942421
key_pairs.remove(context.own_index.value()).1,
23952422
dag_state,
23962423
false,
2424+
Arc::new(CommitVoteMonitor::new(context.clone())),
23972425
);
23982426

23992427
// There is no proposal during recovery because there is no subscriber.
@@ -3310,6 +3338,7 @@ mod test {
33103338
key_pairs.remove(context.own_index.value()).1,
33113339
dag_state.clone(),
33123340
false,
3341+
Arc::new(CommitVoteMonitor::new(context.clone())),
33133342
);
33143343

33153344
let last_commit = store

crates/starfish/core/src/core_thread.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,7 @@ pub(crate) mod tests {
555555
CommitConsumer, VerifiedBlockHeader,
556556
block_manager::BlockManager,
557557
commit_observer::CommitObserver,
558+
commit_vote_monitor::CommitVoteMonitor,
558559
context::Context,
559560
core::CoreSignals,
560561
dag_state::DagState,
@@ -766,6 +767,7 @@ pub(crate) mod tests {
766767
key_pairs.remove(context.own_index.value()).1,
767768
dag_state.clone(),
768769
false,
770+
Arc::new(CommitVoteMonitor::new(context.clone())),
769771
);
770772

771773
let (core_dispatcher, handle) = ChannelCoreThreadDispatcher::start(context, core, false);

0 commit comments

Comments
 (0)