Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 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
1 change: 1 addition & 0 deletions .gitlab/pipeline/check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ cargo-clippy:
RUSTFLAGS: "-D warnings"
script:
- SKIP_WASM_BUILD=1 cargo clippy --all-targets --locked --workspace
- SKIP_WASM_BUILD=1 cargo clippy --all-targets --all-features --locked --workspace
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have a different step for this? Or modify the one we already have?
Or does clippy do some magic and not re-do the work after the first command?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought I'd add it here to hijack the previous job since it has already downloaded the packages and only has to recompile workspace packages which have other features (no other deps). You can actually see it in the clippy job that has run on this PR. Happy to make it a separate step if the CI folks are happy with the extra cost (if there is any - I don't know the ins and outs of forklift).
The features are not purely additive, and can actually remove code from the main build so I don't think we should replace the previous step.


check-try-runtime:
stage: check
Expand Down
12 changes: 7 additions & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -482,25 +482,27 @@ suspicious_double_ref_op = { level = "allow", priority = 2 }

[workspace.lints.clippy]
all = { level = "allow", priority = 0 }
correctness = { level = "deny", priority = 1 }
correctness = { level = "warn", priority = 1 }
complexity = { level = "warn", priority = 1 }
if-same-then-else = { level = "allow", priority = 2 }
complexity = { level = "deny", priority = 1 }
zero-prefixed-literal = { level = "allow", priority = 2 } # 00_1000_000
type_complexity = { level = "allow", priority = 2 } # raison d'etre
nonminimal-bool = { level = "allow", priority = 2 } # maybe
borrowed-box = { level = "allow", priority = 2 } # Reasonable to fix this one
too-many-arguments = { level = "allow", priority = 2 } # (Turning this on would lead to)
needless-lifetimes = { level = "allow", priority = 2 } # generated code
unnecessary_cast = { level = "allow", priority = 2 } # Types may change
identity-op = { level = "allow", priority = 2 } # One case where we do 0 +
useless_conversion = { level = "allow", priority = 2 } # Types may change
unit_arg = { level = "allow", priority = 2 } # styalistic.
option-map-unit-fn = { level = "allow", priority = 2 } # styalistic
bind_instead_of_map = { level = "allow", priority = 2 } # styalistic
unit_arg = { level = "allow", priority = 2 } # stylistic
option-map-unit-fn = { level = "allow", priority = 2 } # stylistic
bind_instead_of_map = { level = "allow", priority = 2 } # stylistic
erasing_op = { level = "allow", priority = 2 } # E.g. 0 * DOLLARS
eq_op = { level = "allow", priority = 2 } # In tests we test equality.
while_immutable_condition = { level = "allow", priority = 2 } # false positives
needless_option_as_deref = { level = "allow", priority = 2 } # false positives
derivable_impls = { level = "allow", priority = 2 } # false positives
clone_on_copy = { level = "allow", priority = 2 } # false positives
stable_sort_primitive = { level = "allow", priority = 2 } # prefer stable sort
extra-unused-type-parameters = { level = "allow", priority = 2 } # stylistic
default_constructed_unit_structs = { level = "allow", priority = 2 } # stylistic
Expand Down
2 changes: 1 addition & 1 deletion cumulus/pallets/xcmp-queue/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ mod benchmarks {
}

assert!(
OutboundXcmpStatus::<T>::get().iter().find(|p| p.recipient == para).is_none(),
!OutboundXcmpStatus::<T>::get().iter().any(|p| p.recipient == para),
"No messages in the channel; therefore removed."
);
}
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/bounties/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ benchmarks_instance_pallet! {
);
}
verify {
ensure!(missed_any == false, "Missed some");
ensure!(!missed_any, "Missed some");
if b > 0 {
ensure!(budget_remaining < BalanceOf::<T, I>::max_value(), "Budget not used");
assert_last_event::<T, I>(Event::BountyBecameActive { index: b - 1 }.into())
Expand Down
3 changes: 1 addition & 2 deletions substrate/frame/contracts/src/benchmarking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2246,13 +2246,12 @@ benchmarks! {
let message_len = message.len() as i32;
let key_type = sp_core::crypto::KeyTypeId(*b"code");
let sig_params = (0..r)
.map(|i| {
.flat_map(|i| {
let pub_key = sp_io::crypto::sr25519_generate(key_type, None);
let sig = sp_io::crypto::sr25519_sign(key_type, &pub_key, &message).expect("Generates signature");
let data: [u8; 96] = [AsRef::<[u8]>::as_ref(&sig), AsRef::<[u8]>::as_ref(&pub_key)].concat().try_into().unwrap();
data
})
.flatten()
.collect::<Vec<_>>();
let sig_params_len = sig_params.len() as i32;

Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/fast-unstake/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ benchmarks! {
fast_unstake_events::<T>().last(),
Some(Event::BatchChecked { .. })
));
assert!(stashes.iter().all(|(s, _)| request.stashes.iter().find(|(ss, _)| ss == s).is_some()));
assert!(stashes.iter().all(|(s, _)| request.stashes.iter().any(|(ss, _)| ss == s)));
}

register_fast_unstake {
Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/scheduler/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ pub mod v3 {
// Check that no agenda overflows `MaxScheduledPerBlock`.
let max_scheduled_per_block = T::MaxScheduledPerBlock::get() as usize;
for (block_number, agenda) in Agenda::<T>::iter() {
if agenda.iter().cloned().filter_map(|s| s).count() > max_scheduled_per_block {
if agenda.iter().cloned().flatten().count() > max_scheduled_per_block {
log::error!(
target: TARGET,
"Would truncate agenda of block {:?} from {} items to {} items.",
Expand All @@ -119,7 +119,7 @@ pub mod v3 {
// Check that bounding the calls will not overflow `MAX_LENGTH`.
let max_length = T::Preimages::MAX_LENGTH as usize;
for (block_number, agenda) in Agenda::<T>::iter() {
for schedule in agenda.iter().cloned().filter_map(|s| s) {
for schedule in agenda.iter().cloned().flatten() {
match schedule.call {
frame_support::traits::schedule::MaybeHashed::Value(call) => {
let l = call.using_encoded(|c| c.len());
Expand Down
2 changes: 1 addition & 1 deletion substrate/primitives/core/src/paired_crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ pub mod ecdsa_bls377 {
let Ok(right_sig) = sig.0[ecdsa::SIGNATURE_SERIALIZED_SIZE..].try_into() else {
return false
};
bls377::Pair::verify(&right_sig, message.as_ref(), &right_pub)
bls377::Pair::verify(&right_sig, message, &right_pub)
}
}
}
Expand Down