Skip to content

Commit 02cb2d6

Browse files
authored
Enable lints for tests only running optimized (#6664)
* enable linting optimized-only tests * fix automatically fixable or obvious lints * fix suspicious_open_options by removing manual options * fix `await_holding_lock`s * avoid failing lint due to now disabled `#[cfg(debug_assertions)]` * reduce future sizes in tests * fix accidently flipped assert logic * restore holding lock for web3signer download * Merge branch 'unstable' into lint-opt-tests
1 parent 847c801 commit 02cb2d6

File tree

34 files changed

+576
-578
lines changed

34 files changed

+576
-578
lines changed

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ test-full: cargo-fmt test-release test-debug test-ef test-exec-engine
204204
# Lints the code for bad style and potentially unsafe arithmetic using Clippy.
205205
# Clippy lints are opt-in per-crate for now. By default, everything is allowed except for performance and correctness lints.
206206
lint:
207-
cargo clippy --workspace --benches --tests $(EXTRA_CLIPPY_OPTS) --features "$(TEST_FEATURES)" -- \
207+
RUSTFLAGS="-C debug-assertions=no $(RUSTFLAGS)" cargo clippy --workspace --benches --tests $(EXTRA_CLIPPY_OPTS) --features "$(TEST_FEATURES)" -- \
208208
-D clippy::fn_to_numeric_cast_any \
209209
-D clippy::manual_let_else \
210210
-D clippy::large_stack_frames \

account_manager/src/validator/exit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,6 @@ mod tests {
409409
)
410410
.unwrap();
411411

412-
assert_eq!(expected_pk, kp.pk.into());
412+
assert_eq!(expected_pk, kp.pk);
413413
}
414414
}

beacon_node/beacon_chain/src/shuffling_cache.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ mod test {
512512
}
513513

514514
assert!(
515-
!cache.contains(&shuffling_id_and_committee_caches.get(0).unwrap().0),
515+
!cache.contains(&shuffling_id_and_committee_caches.first().unwrap().0),
516516
"should not contain oldest epoch shuffling id"
517517
);
518518
assert_eq!(

beacon_node/beacon_chain/tests/attestation_production.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,12 @@ async fn produces_attestations_from_attestation_simulator_service() {
7070
}
7171

7272
// Compare the prometheus metrics that evaluates the performance of the unaggregated attestations
73-
let hit_prometheus_metrics = vec![
73+
let hit_prometheus_metrics = [
7474
metrics::VALIDATOR_MONITOR_ATTESTATION_SIMULATOR_HEAD_ATTESTER_HIT_TOTAL,
7575
metrics::VALIDATOR_MONITOR_ATTESTATION_SIMULATOR_TARGET_ATTESTER_HIT_TOTAL,
7676
metrics::VALIDATOR_MONITOR_ATTESTATION_SIMULATOR_SOURCE_ATTESTER_HIT_TOTAL,
7777
];
78-
let miss_prometheus_metrics = vec![
78+
let miss_prometheus_metrics = [
7979
metrics::VALIDATOR_MONITOR_ATTESTATION_SIMULATOR_HEAD_ATTESTER_MISS_TOTAL,
8080
metrics::VALIDATOR_MONITOR_ATTESTATION_SIMULATOR_TARGET_ATTESTER_MISS_TOTAL,
8181
metrics::VALIDATOR_MONITOR_ATTESTATION_SIMULATOR_SOURCE_ATTESTER_MISS_TOTAL,

beacon_node/beacon_chain/tests/attestation_verification.rs

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -431,10 +431,12 @@ impl GossipTester {
431431
.chain
432432
.verify_aggregated_attestation_for_gossip(&aggregate)
433433
.err()
434-
.expect(&format!(
435-
"{} should error during verify_aggregated_attestation_for_gossip",
436-
desc
437-
));
434+
.unwrap_or_else(|| {
435+
panic!(
436+
"{} should error during verify_aggregated_attestation_for_gossip",
437+
desc
438+
)
439+
});
438440
inspect_err(&self, err);
439441

440442
/*
@@ -449,10 +451,12 @@ impl GossipTester {
449451
.unwrap();
450452

451453
assert_eq!(results.len(), 2);
452-
let batch_err = results.pop().unwrap().err().expect(&format!(
453-
"{} should error during batch_verify_aggregated_attestations_for_gossip",
454-
desc
455-
));
454+
let batch_err = results.pop().unwrap().err().unwrap_or_else(|| {
455+
panic!(
456+
"{} should error during batch_verify_aggregated_attestations_for_gossip",
457+
desc
458+
)
459+
});
456460
inspect_err(&self, batch_err);
457461

458462
self
@@ -475,10 +479,12 @@ impl GossipTester {
475479
.chain
476480
.verify_unaggregated_attestation_for_gossip(&attn, Some(subnet_id))
477481
.err()
478-
.expect(&format!(
479-
"{} should error during verify_unaggregated_attestation_for_gossip",
480-
desc
481-
));
482+
.unwrap_or_else(|| {
483+
panic!(
484+
"{} should error during verify_unaggregated_attestation_for_gossip",
485+
desc
486+
)
487+
});
482488
inspect_err(&self, err);
483489

484490
/*
@@ -496,10 +502,12 @@ impl GossipTester {
496502
)
497503
.unwrap();
498504
assert_eq!(results.len(), 2);
499-
let batch_err = results.pop().unwrap().err().expect(&format!(
500-
"{} should error during batch_verify_unaggregated_attestations_for_gossip",
501-
desc
502-
));
505+
let batch_err = results.pop().unwrap().err().unwrap_or_else(|| {
506+
panic!(
507+
"{} should error during batch_verify_unaggregated_attestations_for_gossip",
508+
desc
509+
)
510+
});
503511
inspect_err(&self, batch_err);
504512

505513
self
@@ -816,7 +824,7 @@ async fn aggregated_gossip_verification() {
816824
let (index, sk) = tester.non_aggregator();
817825
*a = SignedAggregateAndProof::from_aggregate(
818826
index as u64,
819-
tester.valid_aggregate.message().aggregate().clone(),
827+
tester.valid_aggregate.message().aggregate(),
820828
None,
821829
&sk,
822830
&chain.canonical_head.cached_head().head_fork(),

beacon_node/beacon_chain/tests/bellatrix.rs

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ async fn merge_with_terminal_block_hash_override() {
8282

8383
let block = &harness.chain.head_snapshot().beacon_block;
8484

85-
let execution_payload = block.message().body().execution_payload().unwrap().clone();
85+
let execution_payload = block.message().body().execution_payload().unwrap();
8686
if i == 0 {
8787
assert_eq!(execution_payload.block_hash(), genesis_pow_block_hash);
8888
}
@@ -133,7 +133,7 @@ async fn base_altair_bellatrix_with_terminal_block_after_fork() {
133133
* Do the Bellatrix fork, without a terminal PoW block.
134134
*/
135135

136-
harness.extend_to_slot(bellatrix_fork_slot).await;
136+
Box::pin(harness.extend_to_slot(bellatrix_fork_slot)).await;
137137

138138
let bellatrix_head = &harness.chain.head_snapshot().beacon_block;
139139
assert!(bellatrix_head.as_bellatrix().is_ok());
@@ -207,15 +207,7 @@ async fn base_altair_bellatrix_with_terminal_block_after_fork() {
207207
harness.extend_slots(1).await;
208208

209209
let block = &harness.chain.head_snapshot().beacon_block;
210-
execution_payloads.push(
211-
block
212-
.message()
213-
.body()
214-
.execution_payload()
215-
.unwrap()
216-
.clone()
217-
.into(),
218-
);
210+
execution_payloads.push(block.message().body().execution_payload().unwrap().into());
219211
}
220212

221213
verify_execution_payload_chain(execution_payloads.as_slice());

beacon_node/beacon_chain/tests/capella.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ async fn base_altair_bellatrix_capella() {
5454
/*
5555
* Do the Altair fork.
5656
*/
57-
harness.extend_to_slot(altair_fork_slot).await;
57+
Box::pin(harness.extend_to_slot(altair_fork_slot)).await;
5858

5959
let altair_head = &harness.chain.head_snapshot().beacon_block;
6060
assert!(altair_head.as_altair().is_ok());
@@ -63,7 +63,7 @@ async fn base_altair_bellatrix_capella() {
6363
/*
6464
* Do the Bellatrix fork, without a terminal PoW block.
6565
*/
66-
harness.extend_to_slot(bellatrix_fork_slot).await;
66+
Box::pin(harness.extend_to_slot(bellatrix_fork_slot)).await;
6767

6868
let bellatrix_head = &harness.chain.head_snapshot().beacon_block;
6969
assert!(bellatrix_head.as_bellatrix().is_ok());
@@ -81,7 +81,7 @@ async fn base_altair_bellatrix_capella() {
8181
/*
8282
* Next Bellatrix block shouldn't include an exec payload.
8383
*/
84-
harness.extend_slots(1).await;
84+
Box::pin(harness.extend_slots(1)).await;
8585

8686
let one_after_bellatrix_head = &harness.chain.head_snapshot().beacon_block;
8787
assert!(
@@ -112,7 +112,7 @@ async fn base_altair_bellatrix_capella() {
112112
terminal_block.timestamp = timestamp;
113113
}
114114
});
115-
harness.extend_slots(1).await;
115+
Box::pin(harness.extend_slots(1)).await;
116116

117117
let two_after_bellatrix_head = &harness.chain.head_snapshot().beacon_block;
118118
assert!(

beacon_node/beacon_chain/tests/payload_invalidation.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ async fn invalid_payload_invalidates_parent() {
413413
rig.import_block(Payload::Valid).await; // Import a valid transition block.
414414
rig.move_to_first_justification(Payload::Syncing).await;
415415

416-
let roots = vec![
416+
let roots = [
417417
rig.import_block(Payload::Syncing).await,
418418
rig.import_block(Payload::Syncing).await,
419419
rig.import_block(Payload::Syncing).await,
@@ -1052,7 +1052,7 @@ async fn invalid_parent() {
10521052

10531053
// Ensure the block built atop an invalid payload is invalid for gossip.
10541054
assert!(matches!(
1055-
rig.harness.chain.clone().verify_block_for_gossip(block.clone().into()).await,
1055+
rig.harness.chain.clone().verify_block_for_gossip(block.clone()).await,
10561056
Err(BlockError::ParentExecutionPayloadInvalid { parent_root: invalid_root })
10571057
if invalid_root == parent_root
10581058
));

beacon_node/beacon_chain/tests/store_tests.rs

Lines changed: 48 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ async fn long_skip() {
330330
final_blocks as usize,
331331
BlockStrategy::ForkCanonicalChainAt {
332332
previous_slot: Slot::new(initial_blocks),
333-
first_slot: Slot::new(initial_blocks + skip_slots as u64 + 1),
333+
first_slot: Slot::new(initial_blocks + skip_slots + 1),
334334
},
335335
AttestationStrategy::AllValidators,
336336
)
@@ -381,8 +381,7 @@ async fn randao_genesis_storage() {
381381
.beacon_state
382382
.randao_mixes()
383383
.iter()
384-
.find(|x| **x == genesis_value)
385-
.is_some());
384+
.any(|x| *x == genesis_value));
386385

387386
// Then upon adding one more block, it isn't
388387
harness.advance_slot();
@@ -393,14 +392,13 @@ async fn randao_genesis_storage() {
393392
AttestationStrategy::AllValidators,
394393
)
395394
.await;
396-
assert!(harness
395+
assert!(!harness
397396
.chain
398397
.head_snapshot()
399398
.beacon_state
400399
.randao_mixes()
401400
.iter()
402-
.find(|x| **x == genesis_value)
403-
.is_none());
401+
.any(|x| *x == genesis_value));
404402

405403
check_finalization(&harness, num_slots);
406404
check_split_slot(&harness, store);
@@ -1062,7 +1060,7 @@ fn check_shuffling_compatible(
10621060
let current_epoch_shuffling_is_compatible = harness.chain.shuffling_is_compatible(
10631061
&block_root,
10641062
head_state.current_epoch(),
1065-
&head_state,
1063+
head_state,
10661064
);
10671065

10681066
// Check for consistency with the more expensive shuffling lookup.
@@ -1102,7 +1100,7 @@ fn check_shuffling_compatible(
11021100
let previous_epoch_shuffling_is_compatible = harness.chain.shuffling_is_compatible(
11031101
&block_root,
11041102
head_state.previous_epoch(),
1105-
&head_state,
1103+
head_state,
11061104
);
11071105
harness
11081106
.chain
@@ -1130,14 +1128,11 @@ fn check_shuffling_compatible(
11301128

11311129
// Targeting two epochs before the current epoch should always return false
11321130
if head_state.current_epoch() >= 2 {
1133-
assert_eq!(
1134-
harness.chain.shuffling_is_compatible(
1135-
&block_root,
1136-
head_state.current_epoch() - 2,
1137-
&head_state
1138-
),
1139-
false
1140-
);
1131+
assert!(!harness.chain.shuffling_is_compatible(
1132+
&block_root,
1133+
head_state.current_epoch() - 2,
1134+
head_state
1135+
));
11411136
}
11421137
}
11431138
}
@@ -1559,14 +1554,13 @@ async fn prunes_fork_growing_past_youngest_finalized_checkpoint() {
15591554
.map(Into::into)
15601555
.collect();
15611556
let canonical_state_root = canonical_state.update_tree_hash_cache().unwrap();
1562-
let (canonical_blocks, _, _, _) = rig
1563-
.add_attested_blocks_at_slots(
1564-
canonical_state,
1565-
canonical_state_root,
1566-
&canonical_slots,
1567-
&honest_validators,
1568-
)
1569-
.await;
1557+
let (canonical_blocks, _, _, _) = Box::pin(rig.add_attested_blocks_at_slots(
1558+
canonical_state,
1559+
canonical_state_root,
1560+
&canonical_slots,
1561+
&honest_validators,
1562+
))
1563+
.await;
15701564

15711565
// Postconditions
15721566
let canonical_blocks: HashMap<Slot, SignedBeaconBlockHash> = canonical_blocks_zeroth_epoch
@@ -1939,7 +1933,7 @@ async fn prune_single_block_long_skip() {
19391933
2 * slots_per_epoch,
19401934
1,
19411935
2 * slots_per_epoch,
1942-
2 * slots_per_epoch as u64,
1936+
2 * slots_per_epoch,
19431937
1,
19441938
)
19451939
.await;
@@ -1961,31 +1955,45 @@ async fn prune_shared_skip_states_mid_epoch() {
19611955
#[tokio::test]
19621956
async fn prune_shared_skip_states_epoch_boundaries() {
19631957
let slots_per_epoch = E::slots_per_epoch();
1964-
pruning_test(slots_per_epoch - 1, 1, slots_per_epoch, 2, slots_per_epoch).await;
1965-
pruning_test(slots_per_epoch - 1, 2, slots_per_epoch, 1, slots_per_epoch).await;
1966-
pruning_test(
1958+
Box::pin(pruning_test(
1959+
slots_per_epoch - 1,
1960+
1,
1961+
slots_per_epoch,
1962+
2,
1963+
slots_per_epoch,
1964+
))
1965+
.await;
1966+
Box::pin(pruning_test(
1967+
slots_per_epoch - 1,
1968+
2,
1969+
slots_per_epoch,
1970+
1,
1971+
slots_per_epoch,
1972+
))
1973+
.await;
1974+
Box::pin(pruning_test(
19671975
2 * slots_per_epoch + slots_per_epoch / 2,
1968-
slots_per_epoch as u64 / 2,
1976+
slots_per_epoch / 2,
19691977
slots_per_epoch,
1970-
slots_per_epoch as u64 / 2 + 1,
1978+
slots_per_epoch / 2 + 1,
19711979
slots_per_epoch,
1972-
)
1980+
))
19731981
.await;
1974-
pruning_test(
1982+
Box::pin(pruning_test(
19751983
2 * slots_per_epoch + slots_per_epoch / 2,
1976-
slots_per_epoch as u64 / 2,
1984+
slots_per_epoch / 2,
19771985
slots_per_epoch,
1978-
slots_per_epoch as u64 / 2 + 1,
1986+
slots_per_epoch / 2 + 1,
19791987
slots_per_epoch,
1980-
)
1988+
))
19811989
.await;
1982-
pruning_test(
1990+
Box::pin(pruning_test(
19831991
2 * slots_per_epoch - 1,
1984-
slots_per_epoch as u64,
1992+
slots_per_epoch,
19851993
1,
19861994
0,
19871995
2 * slots_per_epoch,
1988-
)
1996+
))
19891997
.await;
19901998
}
19911999

@@ -2094,7 +2102,7 @@ async fn pruning_test(
20942102
);
20952103
check_chain_dump(
20962104
&harness,
2097-
(num_initial_blocks + num_canonical_middle_blocks + num_finalization_blocks + 1) as u64,
2105+
num_initial_blocks + num_canonical_middle_blocks + num_finalization_blocks + 1,
20982106
);
20992107

21002108
let all_canonical_states = harness
@@ -2613,8 +2621,7 @@ async fn process_blocks_and_attestations_for_unaligned_checkpoint() {
26132621
harness.advance_slot();
26142622
}
26152623
harness.extend_to_slot(finalizing_slot - 1).await;
2616-
harness
2617-
.add_block_at_slot(finalizing_slot, harness.get_current_state())
2624+
Box::pin(harness.add_block_at_slot(finalizing_slot, harness.get_current_state()))
26182625
.await
26192626
.unwrap();
26202627

0 commit comments

Comments
 (0)