Skip to content

Commit 1beb6c2

Browse files
committed
remove addtional state tracking until we have a better soln
We currently track if the proposal was local as its own flag, derived from the leader. Right now this doesn't buy us much. The original intention was to enable a more fool-proof setup where the proposer's node would keep track even in the event of crashes, but for now, this re-centers the PR on a solution that relies on the leader being known.
1 parent db36fd0 commit 1beb6c2

4 files changed

Lines changed: 23 additions & 196 deletions

File tree

consensus/src/simplex/actors/voter/actor.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -858,10 +858,13 @@ impl<
858858
let round = proposal.round;
859859
let view = round.view();
860860
debug!(%view, "attempting certification");
861-
let result = if self.state.proposed_locally(view) {
862-
// The proposer should always be willing to certify their own
863-
// proposals. Reaching out to the automaton is unnecessary and
864-
// creates duplicate work.
861+
let leader_is_local = self
862+
.state
863+
.leader_index(view)
864+
.is_some_and(|leader| self.state.is_me(leader));
865+
let result = if leader_is_local {
866+
// Once we know the local participant led this view, reaching out to the
867+
// automaton is unnecessary and creates duplicate work.
865868
Either::Left(ready(Ok(true)))
866869
} else {
867870
let receiver = self.automaton.certify(round, proposal.payload).await;

consensus/src/simplex/actors/voter/round.rs

Lines changed: 8 additions & 154 deletions
Original file line numberDiff line numberDiff line change
@@ -181,13 +181,6 @@ impl<S: Scheme, D: Digest> Round<S, D> {
181181
self.scheme.me().is_some_and(|me| me == signer)
182182
}
183183

184-
/// Returns true if the leader is the local participant.
185-
fn is_leader(&self) -> bool {
186-
self.leader()
187-
.as_ref()
188-
.is_some_and(|leader| self.is_signer(leader.idx))
189-
}
190-
191184
/// Removes the leader and certification deadlines so timeouts stop firing.
192185
pub const fn clear_deadlines(&mut self) {
193186
self.leader_deadline = None;
@@ -204,9 +197,6 @@ impl<S: Scheme, D: Digest> Round<S, D> {
204197
.expect("leader index comes from elector, must be within bounds");
205198
debug!(round=?self.round, %leader, ?key, "leader elected");
206199
self.leader = Some(Leader { idx: leader, key });
207-
if self.proposal().is_some() {
208-
self.proposal.set_proposed_locally(self.is_leader());
209-
}
210200
}
211201

212202
/// Returns the notarization certificate if we already reconstructed one.
@@ -229,11 +219,6 @@ impl<S: Scheme, D: Digest> Round<S, D> {
229219
matches!(self.certify, CertifyState::Certified(true))
230220
}
231221

232-
/// Returns whether this round belongs to a view led by the local participant.
233-
pub const fn proposed_locally(&self) -> bool {
234-
self.proposal.proposed_locally()
235-
}
236-
237222
/// Returns true if certification was aborted due to finalization.
238223
#[cfg(test)]
239224
pub const fn is_certify_aborted(&self) -> bool {
@@ -250,8 +235,7 @@ impl<S: Scheme, D: Digest> Round<S, D> {
250235
if self.broadcast_nullify {
251236
return false;
252237
}
253-
self.proposal
254-
.set_verified_proposal(proposal, self.is_leader());
238+
self.proposal.built(proposal);
255239
self.leader_deadline = None;
256240
true
257241
}
@@ -371,8 +355,7 @@ impl<S: Scheme, D: Digest> Round<S, D> {
371355
///
372356
/// Returns the leader's public key if equivocation is detected (conflicting proposals).
373357
pub fn add_recovered_proposal(&mut self, proposal: Proposal<D>) -> Option<S::PublicKey> {
374-
let proposed_locally = self.is_leader();
375-
let leader = match self.proposal.update(&proposal, true) {
358+
match self.proposal.update(&proposal, true) {
376359
ProposalChange::New => {
377360
debug!(?proposal, "setting proposal from certificate");
378361
self.leader_deadline = None;
@@ -393,9 +376,7 @@ impl<S: Scheme, D: Digest> Round<S, D> {
393376
equivocator
394377
}
395378
ProposalChange::Skipped => None,
396-
};
397-
self.proposal.set_proposed_locally(proposed_locally);
398-
leader
379+
}
399380
}
400381

401382
/// Adds a verified notarization certificate to the round.
@@ -552,8 +533,10 @@ impl<S: Scheme, D: Digest> Round<S, D> {
552533
"replaying notarize from another signer"
553534
);
554535

555-
self.proposal
556-
.set_verified_proposal(notarize.proposal.clone(), self.is_leader());
536+
// While we may not be the leader here, we still call
537+
// built because the effect is the same (there is a proposal
538+
// and it is verified).
539+
self.proposal.built(notarize.proposal.clone());
557540
self.broadcast_notarize = true;
558541
}
559542
Artifact::Nullify(nullify) => {
@@ -1100,72 +1083,10 @@ mod tests {
11001083
// Has notarization and proposal came from certificate
11011084
// try_certify returns the proposal from the certificate
11021085
assert!(round.try_certify().is_some());
1103-
assert!(!round.proposed_locally());
1104-
}
1105-
1106-
#[test]
1107-
fn recovered_local_leader_proposal_stays_local() {
1108-
let mut rng = test_rng();
1109-
let namespace = b"ns";
1110-
let Fixture {
1111-
schemes, verifier, ..
1112-
} = ed25519::fixture(&mut rng, namespace, 4);
1113-
let local_scheme = schemes[0].clone();
1114-
1115-
let now = SystemTime::UNIX_EPOCH;
1116-
let round_info = Rnd::new(Epoch::new(1), View::new(1));
1117-
let proposal = Proposal::new(round_info, View::new(0), Sha256Digest::from([8u8; 32]));
1118-
1119-
let mut round = Round::new(local_scheme, round_info, now);
1120-
round.set_leader(Participant::new(0));
1121-
1122-
let notarization_votes: Vec<_> = schemes
1123-
.iter()
1124-
.map(|scheme| Notarize::sign(scheme, proposal.clone()).unwrap())
1125-
.collect();
1126-
let notarization =
1127-
Notarization::from_notarizes(&verifier, notarization_votes.iter(), &Sequential)
1128-
.unwrap();
1129-
let (added, _) = round.add_notarization(notarization);
1130-
assert!(added);
1131-
1132-
assert!(round.proposed_locally());
1133-
assert!(round.try_certify().is_some());
1134-
}
1135-
1136-
#[test]
1137-
fn delayed_leader_assignment_marks_recovered_proposal_local() {
1138-
let mut rng = test_rng();
1139-
let namespace = b"ns";
1140-
let Fixture {
1141-
schemes, verifier, ..
1142-
} = ed25519::fixture(&mut rng, namespace, 4);
1143-
let local_scheme = schemes[0].clone();
1144-
1145-
let now = SystemTime::UNIX_EPOCH;
1146-
let round_info = Rnd::new(Epoch::new(1), View::new(1));
1147-
let proposal = Proposal::new(round_info, View::new(0), Sha256Digest::from([10u8; 32]));
1148-
1149-
let mut round = Round::new(local_scheme, round_info, now);
1150-
1151-
let notarization_votes: Vec<_> = schemes
1152-
.iter()
1153-
.map(|scheme| Notarize::sign(scheme, proposal.clone()).unwrap())
1154-
.collect();
1155-
let notarization =
1156-
Notarization::from_notarizes(&verifier, notarization_votes.iter(), &Sequential)
1157-
.unwrap();
1158-
let (added, _) = round.add_notarization(notarization);
1159-
assert!(added);
1160-
assert!(!round.proposed_locally());
1161-
1162-
round.set_leader(Participant::new(0));
1163-
1164-
assert!(round.proposed_locally());
11651086
}
11661087

11671088
#[test]
1168-
fn replayed_notarize_does_not_mark_follower_view_local() {
1089+
fn replayed_notarize_keeps_proposal_verified() {
11691090
let mut rng = test_rng();
11701091
let namespace = b"ns";
11711092
let Fixture { schemes, .. } = ed25519::fixture(&mut rng, namespace, 4);
@@ -1183,73 +1104,6 @@ mod tests {
11831104

11841105
assert_eq!(round.proposal(), Some(&proposal));
11851106
assert_eq!(round.proposal.status(), ProposalStatus::Verified);
1186-
assert!(!round.proposed_locally());
1187-
}
1188-
1189-
#[test]
1190-
fn locally_built_proposals_stay_marked_local_for_certification() {
1191-
let mut rng = test_rng();
1192-
let namespace = b"ns";
1193-
let Fixture {
1194-
schemes, verifier, ..
1195-
} = ed25519::fixture(&mut rng, namespace, 4);
1196-
let local_scheme = schemes[0].clone();
1197-
1198-
let now = SystemTime::UNIX_EPOCH;
1199-
let round_info = Rnd::new(Epoch::new(1), View::new(1));
1200-
let proposal = Proposal::new(round_info, View::new(0), Sha256Digest::from([7u8; 32]));
1201-
1202-
let mut round = Round::new(local_scheme, round_info, now);
1203-
round.set_leader(Participant::new(0));
1204-
assert!(round.proposed(proposal.clone()));
1205-
assert!(round.proposed_locally());
1206-
1207-
let notarization_votes: Vec<_> = schemes
1208-
.iter()
1209-
.map(|scheme| Notarize::sign(scheme, proposal.clone()).unwrap())
1210-
.collect();
1211-
let notarization =
1212-
Notarization::from_notarizes(&verifier, notarization_votes.iter(), &Sequential)
1213-
.unwrap();
1214-
let (added, _) = round.add_notarization(notarization);
1215-
assert!(added);
1216-
1217-
assert!(round.try_certify().is_some());
1218-
assert!(round.proposed_locally());
1219-
}
1220-
1221-
#[test]
1222-
fn recovered_conflicting_certificate_keeps_leader_local_marker() {
1223-
let mut rng = test_rng();
1224-
let namespace = b"ns";
1225-
let Fixture {
1226-
schemes, verifier, ..
1227-
} = ed25519::fixture(&mut rng, namespace, 4);
1228-
let local_scheme = schemes[0].clone();
1229-
1230-
let now = SystemTime::UNIX_EPOCH;
1231-
let round_info = Rnd::new(Epoch::new(1), View::new(1));
1232-
let local_proposal = Proposal::new(round_info, View::new(0), Sha256Digest::from([5u8; 32]));
1233-
let conflicting = Proposal::new(round_info, View::new(0), Sha256Digest::from([6u8; 32]));
1234-
1235-
let mut round = Round::new(local_scheme, round_info, now);
1236-
round.set_leader(Participant::new(0));
1237-
assert!(round.proposed(local_proposal));
1238-
assert!(round.proposed_locally());
1239-
1240-
let notarization_votes: Vec<_> = schemes
1241-
.iter()
1242-
.map(|scheme| Notarize::sign(scheme, conflicting.clone()).unwrap())
1243-
.collect();
1244-
let notarization =
1245-
Notarization::from_notarizes(&verifier, notarization_votes.iter(), &Sequential)
1246-
.unwrap();
1247-
let (added, equivocator) = round.add_notarization(notarization);
1248-
assert!(added);
1249-
assert!(equivocator.is_some());
1250-
1251-
assert_eq!(round.proposal.status(), ProposalStatus::Equivocated);
1252-
assert!(round.proposed_locally());
12531107
}
12541108

12551109
#[test]

0 commit comments

Comments
 (0)