Skip to content

Commit ea0e09b

Browse files
asmaastarkwareasmaastarkware
committed
apollo_consensus,apollo_protobuf: use BlockNumber for Vote.height
1 parent 0045f6c commit ea0e09b

File tree

13 files changed

+200
-155
lines changed

13 files changed

+200
-155
lines changed

‎crates/apollo_consensus/src/manager.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ impl<ContextT: ConsensusContext> ConsensusCache<ContextT> {
238238

239239
/// Caches a vote for a future height.
240240
fn cache_future_vote(&mut self, vote: Vote) {
241-
self.future_votes.entry(BlockNumber(vote.height)).or_default().push(vote);
241+
self.future_votes.entry(vote.height).or_default().push(vote);
242242
}
243243

244244
/// Caches a proposal for a future height.
@@ -666,7 +666,7 @@ impl<ContextT: ConsensusContext> MultiHeightManager<ContextT> {
666666
// TODO(matan): We need to figure out an actual caching strategy under 2 constraints:
667667
// 1. Malicious - must be capped so a malicious peer can't DoS us.
668668
// 2. Parallel proposals - we may send/receive a proposal for (H+1, 0).
669-
match message.height.cmp(&height.0) {
669+
match message.height.cmp(&height) {
670670
std::cmp::Ordering::Greater => {
671671
if self.should_cache_vote(&height, 0, &message) {
672672
trace!("Cache message for a future height. {:?}", message);
@@ -825,7 +825,7 @@ impl<ContextT: ConsensusContext> MultiHeightManager<ContextT> {
825825
&self,
826826
current_height: &BlockNumber,
827827
current_round: Round,
828-
msg_height: u64,
828+
msg_height: BlockNumber,
829829
msg_round: Round,
830830
msg_description: &str,
831831
) -> bool {
@@ -845,7 +845,7 @@ impl<ContextT: ConsensusContext> MultiHeightManager<ContextT> {
845845
msg_description,
846846
msg_height,
847847
msg_round,
848-
current_height.0,
848+
current_height,
849849
current_round,
850850
limits.future_height_limit,
851851
limits.future_height_round_limit,
@@ -865,7 +865,7 @@ impl<ContextT: ConsensusContext> MultiHeightManager<ContextT> {
865865
self.should_cache_msg(
866866
current_height,
867867
current_round,
868-
proposal.height.0,
868+
proposal.height,
869869
proposal.round,
870870
"proposal",
871871
)

‎crates/apollo_consensus/src/manager_test.rs‎

Lines changed: 100 additions & 99 deletions
Large diffs are not rendered by default.

‎crates/apollo_consensus/src/simulation_network_receiver_test.rs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use apollo_network_types::network_types::BroadcastedMessageMetadata;
66
use apollo_protobuf::consensus::Vote;
77
use apollo_test_utils::{get_rng, GetTestInstance};
88
use futures::{SinkExt, StreamExt};
9+
use starknet_api::block::BlockNumber;
910
use test_case::test_case;
1011

1112
use super::NetworkReceiver;
@@ -31,7 +32,10 @@ async fn test_invalid(distinct_messages: bool) {
3132
let mut invalid_messages = 0;
3233

3334
for height in 0..1000 {
34-
let msg = Vote { height: if distinct_messages { height } else { 0 }, ..Default::default() };
35+
let msg = Vote {
36+
height: if distinct_messages { BlockNumber(height) } else { BlockNumber(0) },
37+
..Default::default()
38+
};
3539
let broadcasted_message_metadata =
3640
BroadcastedMessageMetadata::get_test_instance(&mut get_rng());
3741
mock_network
@@ -62,7 +66,10 @@ async fn test_drops(distinct_messages: bool) {
6266
let mut num_received = 0;
6367

6468
for height in 0..1000 {
65-
let msg = Vote { height: if distinct_messages { height } else { 0 }, ..Default::default() };
69+
let msg = Vote {
70+
height: if distinct_messages { BlockNumber(height) } else { BlockNumber(0) },
71+
..Default::default()
72+
};
6673
let broadcasted_message_metadata =
6774
BroadcastedMessageMetadata::get_test_instance(&mut get_rng());
6875
mock_network

‎crates/apollo_consensus/src/single_height_consensus_test.rs‎

Lines changed: 63 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,12 @@ fn proposer() {
6262
));
6363

6464
// Receive two prevotes from other validators to reach prevote quorum.
65-
let _ = shc.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), 0, 0, *VALIDATOR_ID_1)).unwrap();
66-
let ret =
67-
shc.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), 0, 0, *VALIDATOR_ID_2)).unwrap();
65+
let _ = shc
66+
.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), BlockNumber(0), 0, *VALIDATOR_ID_1))
67+
.unwrap();
68+
let ret = shc
69+
.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), BlockNumber(0), 0, *VALIDATOR_ID_2))
70+
.unwrap();
6871
// Expect a precommit broadcast request present.
6972
let mut reqs = match ret {
7073
ShcReturn::Requests(r) => r,
@@ -77,10 +80,12 @@ fn proposer() {
7780
));
7881

7982
// Now provide precommit votes to reach decision.
80-
let _ =
81-
shc.handle_vote(&leader_fn, precommit(Some(BLOCK.id.0), 0, 0, *VALIDATOR_ID_1)).unwrap();
82-
let decision =
83-
shc.handle_vote(&leader_fn, precommit(Some(BLOCK.id.0), 0, 0, *VALIDATOR_ID_2)).unwrap();
83+
let _ = shc
84+
.handle_vote(&leader_fn, precommit(Some(BLOCK.id.0), BlockNumber(0), 0, *VALIDATOR_ID_1))
85+
.unwrap();
86+
let decision = shc
87+
.handle_vote(&leader_fn, precommit(Some(BLOCK.id.0), BlockNumber(0), 0, *VALIDATOR_ID_2))
88+
.unwrap();
8489
match decision {
8590
ShcReturn::Decision(d) => assert_eq!(d.block, BLOCK.id),
8691
_ => panic!("expected decision"),
@@ -135,9 +140,12 @@ fn validator(repeat_proposal: bool) {
135140
}
136141

137142
// Reach prevote quorum with two other validators.
138-
let _ = shc.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), 0, 0, *VALIDATOR_ID_2)).unwrap();
139-
let ret =
140-
shc.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), 0, 0, *VALIDATOR_ID_3)).unwrap();
143+
let _ = shc
144+
.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), BlockNumber(0), 0, *VALIDATOR_ID_2))
145+
.unwrap();
146+
let ret = shc
147+
.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), BlockNumber(0), 0, *VALIDATOR_ID_3))
148+
.unwrap();
141149
// Expect a precommit broadcast request present.
142150
let mut reqs = match ret {
143151
ShcReturn::Requests(r) => r,
@@ -149,9 +157,12 @@ fn validator(repeat_proposal: bool) {
149157
);
150158

151159
// Now provide precommit votes to reach decision.
152-
let _ = shc.handle_vote(&leader_fn, precommit(Some(BLOCK.id.0), 0, 0, *PROPOSER_ID)).unwrap();
153-
let decision =
154-
shc.handle_vote(&leader_fn, precommit(Some(BLOCK.id.0), 0, 0, *VALIDATOR_ID_2)).unwrap();
160+
let _ = shc
161+
.handle_vote(&leader_fn, precommit(Some(BLOCK.id.0), BlockNumber(0), 0, *PROPOSER_ID))
162+
.unwrap();
163+
let decision = shc
164+
.handle_vote(&leader_fn, precommit(Some(BLOCK.id.0), BlockNumber(0), 0, *VALIDATOR_ID_2))
165+
.unwrap();
155166
match decision {
156167
ShcReturn::Decision(d) => assert_eq!(d.block, BLOCK.id),
157168
_ => panic!("expected decision"),
@@ -182,10 +193,9 @@ fn vote_twice(same_vote: bool) {
182193

183194
// Duplicate prevote should be ignored.
184195
let _ =
185-
shc.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), HEIGHT.0, 0, *PROPOSER_ID)).unwrap();
186-
let res = shc
187-
.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), HEIGHT.0, 0, *VALIDATOR_ID_2))
188-
.unwrap();
196+
shc.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), HEIGHT, 0, *PROPOSER_ID)).unwrap();
197+
let res =
198+
shc.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), HEIGHT, 0, *VALIDATOR_ID_2)).unwrap();
189199
// On quorum of prevotes, expect a precommit broadcast request.
190200
let mut reqs = match res {
191201
ShcReturn::Requests(r) => r,
@@ -198,16 +208,16 @@ fn vote_twice(same_vote: bool) {
198208
));
199209

200210
// Precommit handling towards decision.
201-
let first_vote = precommit(Some(BLOCK.id.0), HEIGHT.0, 0, *PROPOSER_ID);
211+
let first_vote = precommit(Some(BLOCK.id.0), HEIGHT, 0, *PROPOSER_ID);
202212
let _ = shc.handle_vote(&leader_fn, first_vote.clone()).unwrap();
203213
let second_vote = if same_vote {
204214
first_vote.clone()
205215
} else {
206-
precommit(Some(Felt::TWO), HEIGHT.0, 0, *PROPOSER_ID)
216+
precommit(Some(Felt::TWO), HEIGHT, 0, *PROPOSER_ID)
207217
};
208218
let _ = shc.handle_vote(&leader_fn, second_vote.clone()).unwrap();
209219
let decision =
210-
shc.handle_vote(&leader_fn, precommit(Some(BLOCK.id.0), HEIGHT.0, 0, *VALIDATOR_ID_3));
220+
shc.handle_vote(&leader_fn, precommit(Some(BLOCK.id.0), HEIGHT, 0, *VALIDATOR_ID_3));
211221
match decision {
212222
Ok(ShcReturn::Decision(d)) => assert_eq!(d.block, BLOCK.id),
213223
_ => panic!("Expected decision"),
@@ -241,9 +251,12 @@ fn rebroadcast_votes() {
241251
));
242252

243253
// Receive two prevotes from other validators to reach prevote quorum at round 0.
244-
let _ = shc.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), 0, 0, *VALIDATOR_ID_1)).unwrap();
245-
let ret =
246-
shc.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), 0, 0, *VALIDATOR_ID_2)).unwrap();
254+
let _ = shc
255+
.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), BlockNumber(0), 0, *VALIDATOR_ID_1))
256+
.unwrap();
257+
let ret = shc
258+
.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), BlockNumber(0), 0, *VALIDATOR_ID_2))
259+
.unwrap();
247260
// Expect a precommit broadcast at round 0.
248261
let reqs = match ret {
249262
ShcReturn::Requests(r) => r,
@@ -256,8 +269,10 @@ fn rebroadcast_votes() {
256269

257270
// Advance with NIL precommits from peers (no decision) -> expect scheduling of precommit
258271
// timeout.
259-
let _ = shc.handle_vote(&leader_fn, precommit(None, 0, 0, *VALIDATOR_ID_1)).unwrap();
260-
let _ = shc.handle_vote(&leader_fn, precommit(None, 0, 0, *VALIDATOR_ID_2)).unwrap();
272+
let _ =
273+
shc.handle_vote(&leader_fn, precommit(None, BlockNumber(0), 0, *VALIDATOR_ID_1)).unwrap();
274+
let _ =
275+
shc.handle_vote(&leader_fn, precommit(None, BlockNumber(0), 0, *VALIDATOR_ID_2)).unwrap();
261276

262277
// Timeout at precommit(0) -> expect a prevote broadcast for round 1.
263278
let ret = shc.handle_event(&leader_fn, StateMachineEvent::TimeoutPrecommit(0)).unwrap();
@@ -271,9 +286,12 @@ fn rebroadcast_votes() {
271286
)));
272287

273288
// Reach prevote quorum at round 1 with two other validators.
274-
let _ = shc.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), 0, 1, *VALIDATOR_ID_2)).unwrap();
275-
let ret =
276-
shc.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), 0, 1, *VALIDATOR_ID_3)).unwrap();
289+
let _ = shc
290+
.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), BlockNumber(0), 1, *VALIDATOR_ID_2))
291+
.unwrap();
292+
let ret = shc
293+
.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), BlockNumber(0), 1, *VALIDATOR_ID_3))
294+
.unwrap();
277295
let reqs = match ret {
278296
ShcReturn::Requests(r) => r,
279297
_ => panic!("expected requests"),
@@ -286,15 +304,20 @@ fn rebroadcast_votes() {
286304
// Rebroadcast with older vote (round 0) - should be ignored (no broadcast, no task).
287305
let ret = shc.handle_event(
288306
&leader_fn,
289-
StateMachineEvent::VoteBroadcasted(precommit(Some(BLOCK.id.0), 0, 0, *PROPOSER_ID)),
307+
StateMachineEvent::VoteBroadcasted(precommit(
308+
Some(BLOCK.id.0),
309+
BlockNumber(0),
310+
0,
311+
*PROPOSER_ID,
312+
)),
290313
);
291314
match ret.unwrap() {
292315
ShcReturn::Requests(r) => assert!(r.is_empty()),
293316
_ => panic!("expected requests"),
294317
}
295318

296319
// Rebroadcast with current round (round 1) - should broadcast.
297-
let rebroadcast_vote = precommit(Some(BLOCK.id.0), 0, 1, *PROPOSER_ID);
320+
let rebroadcast_vote = precommit(Some(BLOCK.id.0), BlockNumber(0), 1, *PROPOSER_ID);
298321
let ret = shc
299322
.handle_event(&leader_fn, StateMachineEvent::VoteBroadcasted(rebroadcast_vote.clone()))
300323
.unwrap();
@@ -336,17 +359,19 @@ fn repropose() {
336359
_ => panic!("expected requests"),
337360
}
338361
// A single prevote from another validator does not yet cause quorum.
339-
let ret =
340-
shc.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), 0, 0, *VALIDATOR_ID_1)).unwrap();
362+
let ret = shc
363+
.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), BlockNumber(0), 0, *VALIDATOR_ID_1))
364+
.unwrap();
341365
// No new requests are expected at this point.
342366
match ret {
343367
ShcReturn::Requests(reqs) => assert!(reqs.is_empty()),
344368
_ => panic!("expected requests"),
345369
}
346370
// Reaching prevote quorum with a second external prevote; proposer will broadcast a precommit
347371
// and schedule a prevote timeout for round 0.
348-
let ret =
349-
shc.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), 0, 0, *VALIDATOR_ID_2)).unwrap();
372+
let ret = shc
373+
.handle_vote(&leader_fn, prevote(Some(BLOCK.id.0), BlockNumber(0), 0, *VALIDATOR_ID_2))
374+
.unwrap();
350375
// Expect ScheduleTimeoutPrevote{round:0} and BroadcastVote(Precommit).
351376
match ret {
352377
ShcReturn::Requests(reqs) => {
@@ -362,8 +387,10 @@ fn repropose() {
362387
}
363388
// receiving Nil precommit requests and then decision on new round; just assert no panic and
364389
// decisions arrive after quorum.
365-
let _ = shc.handle_vote(&leader_fn, precommit(None, 0, 0, *VALIDATOR_ID_1)).unwrap();
366-
let ret = shc.handle_vote(&leader_fn, precommit(None, 0, 0, *VALIDATOR_ID_2)).unwrap();
390+
let _ =
391+
shc.handle_vote(&leader_fn, precommit(None, BlockNumber(0), 0, *VALIDATOR_ID_1)).unwrap();
392+
let ret =
393+
shc.handle_vote(&leader_fn, precommit(None, BlockNumber(0), 0, *VALIDATOR_ID_2)).unwrap();
367394
// assert that ret is ScheduleTimeoutPrecommit
368395
match ret {
369396
ShcReturn::Requests(reqs) => {

‎crates/apollo_consensus/src/state_machine.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ impl StateMachine {
198198
) -> VecDeque<SMRequest> {
199199
let vote = Vote {
200200
vote_type,
201-
height: self.height.0,
201+
height: self.height,
202202
round: self.round,
203203
proposal_commitment,
204204
voter: self.id,

‎crates/apollo_consensus/src/state_machine_test.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ fn mk_vote(
2828
proposal_id: Option<ProposalCommitment>,
2929
voter: ValidatorId,
3030
) -> Vote {
31-
Vote { vote_type, height: HEIGHT.0, round, proposal_commitment: proposal_id, voter }
31+
Vote { vote_type, height: HEIGHT, round, proposal_commitment: proposal_id, voter }
3232
}
3333

3434
struct TestWrapper<LeaderFn: Fn(Round) -> ValidatorId> {

‎crates/apollo_consensus/src/test_utils.rs‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,17 +101,27 @@ mock! {
101101
}
102102
}
103103

104-
pub fn prevote(block_felt: Option<Felt>, height: u64, round: u32, voter: ValidatorId) -> Vote {
104+
pub fn prevote(
105+
block_felt: Option<Felt>,
106+
height: BlockNumber,
107+
round: u32,
108+
voter: ValidatorId,
109+
) -> Vote {
105110
let proposal_commitment = block_felt.map(ProposalCommitment);
106111
Vote { vote_type: VoteType::Prevote, height, round, proposal_commitment, voter }
107112
}
108113

109-
pub fn precommit(block_felt: Option<Felt>, height: u64, round: u32, voter: ValidatorId) -> Vote {
114+
pub fn precommit(
115+
block_felt: Option<Felt>,
116+
height: BlockNumber,
117+
round: u32,
118+
voter: ValidatorId,
119+
) -> Vote {
110120
let proposal_commitment = block_felt.map(ProposalCommitment);
111121
Vote { vote_type: VoteType::Precommit, height, round, proposal_commitment, voter }
112122
}
113-
pub fn proposal_init(height: u64, round: u32, proposer: ValidatorId) -> ProposalInit {
114-
ProposalInit { height: BlockNumber(height), round, proposer, ..Default::default() }
123+
pub fn proposal_init(height: BlockNumber, round: u32, proposer: ValidatorId) -> ProposalInit {
124+
ProposalInit { height, round, proposer, ..Default::default() }
115125
}
116126

117127
#[derive(Debug)]

‎crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,6 @@ impl ConsensusContext for SequencerConsensusContext {
494494
let transactions;
495495
let block_info;
496496
{
497-
let height = BlockNumber(height);
498497
let mut proposals = self.valid_proposals.lock().unwrap();
499498
(block_info, transactions, proposal_id) =
500499
proposals.get_proposal(&height, &block).clone();
@@ -549,7 +548,7 @@ impl ConsensusContext for SequencerConsensusContext {
549548
let sequencer = SequencerContractAddress(block_info.builder);
550549

551550
let block_header_without_hash = BlockHeaderWithoutHash {
552-
block_number: BlockNumber(height),
551+
block_number: height,
553552
l1_gas_price,
554553
l1_data_gas_price,
555554
l2_gas_price,

‎crates/apollo_consensus_orchestrator/src/sequencer_consensus_context_test.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -675,7 +675,7 @@ async fn decision_reached_sends_correct_values() {
675675

676676
let vote = Vote {
677677
// Currently this is the only field used by decision_reached.
678-
height: 0,
678+
height: BlockNumber(0),
679679
..Default::default()
680680
};
681681

‎crates/apollo_protobuf/src/consensus.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ pub enum VoteType {
4747
#[derive(Debug, Default, Hash, Clone, Eq, PartialEq, Serialize, Deserialize)]
4848
pub struct Vote {
4949
pub vote_type: VoteType,
50-
pub height: u64,
50+
pub height: BlockNumber,
5151
pub round: u32,
5252
pub proposal_commitment: Option<ProposalCommitment>,
5353
pub voter: ContractAddress,

0 commit comments

Comments
 (0)