Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(consensus): add lints to consensus #1849

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions crates/sequencing/papyrus_consensus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,6 @@ papyrus_network_types = { workspace = true, features = ["testing"] }
papyrus_storage = { workspace = true, features = ["testing"] }
papyrus_test_utils.workspace = true
test-case.workspace = true

[lints]
workspace = true
7 changes: 5 additions & 2 deletions crates/sequencing/papyrus_consensus/src/bin/run_simulation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,10 @@ impl Node {

async fn stop(&mut self) {
let process = self.process.as_mut().expect("Process not found");
let pid = process.id().unwrap();
let pid = i32::try_from(process.id().unwrap())
.expect("Max PIDs on unix are way smaller than i32::MAX");
// Send SIGINT to the entire process group to terminate the process and its subprocesses
nix::sys::signal::killpg(Pid::from_raw(pid as i32), nix::sys::signal::Signal::SIGINT)
nix::sys::signal::killpg(Pid::from_raw(pid), nix::sys::signal::Signal::SIGINT)
.expect("Failed to kill process group");
}

Expand Down Expand Up @@ -296,7 +297,9 @@ async fn build_node(data_dir: &str, logs_dir: &str, i: usize, papyrus_args: &Pap
("invalid_probability", papyrus_args.invalid_probability),
// Convert optional parameters to f64 for consistency in the vector,
// types were validated during parsing.
#[allow(clippy::as_conversions)] // FIXME: truncation possible.
("cache_size", papyrus_args.cache_size.map(|v| v as f64)),
#[allow(clippy::as_conversions)] // FIXME: truncation possible.
("random_seed", papyrus_args.random_seed.map(|v| v as f64)),
];
for (key, value) in conditional_test_params {
Expand Down
1 change: 1 addition & 0 deletions crates/sequencing/papyrus_consensus/src/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ where
tokio::time::sleep(consensus_delay).await;
let mut current_height = start_height;
let mut manager = MultiHeightManager::new(validator_id, timeouts);
#[allow(clippy::as_conversions)] // FIXME: use int metrics so `as f64` may be removed.
loop {
metrics::gauge!(PAPYRUS_CONSENSUS_HEIGHT, current_height.0 as f64);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ impl NetworkReceiver {
}

fn should_drop_msg(&self, msg_hash: u64) -> bool {
#[allow(clippy::as_conversions)]
let prob = (msg_hash as f64) / (u64::MAX as f64);
prob <= self.drop_probability
}
Expand All @@ -104,6 +105,7 @@ impl NetworkReceiver {
mut msg: ConsensusMessage,
msg_hash: u64,
) -> ConsensusMessage {
#[allow(clippy::as_conversions)]
if (msg_hash as f64) / (u64::MAX as f64) > self.invalid_probability {
return msg;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ impl SingleHeightConsensus {
timeouts: TimeoutsConfig,
) -> Self {
// TODO(matan): Use actual weights, not just `len`.
let state_machine = StateMachine::new(id, validators.len() as u32);
let n_validators =
u32::try_from(validators.len()).expect("Should have way less than u32::MAX validators");
let state_machine = StateMachine::new(id, n_validators);
Self {
height,
validators,
Expand Down Expand Up @@ -445,7 +447,11 @@ impl SingleHeightConsensus {
})
.collect();
// TODO(matan): Check actual weights.
assert!(supporting_precommits.len() >= self.state_machine.quorum_size() as usize);
assert!(
supporting_precommits.len()
>= usize::try_from(self.state_machine.quorum_size())
.expect("u32 should fit in usize")
);
Ok(ShcReturn::Decision(Decision { precommits: supporting_precommits, block }))
}
}
Loading