Skip to content

Commit

Permalink
Rust 1.54.0 lints (#2483)
Browse files Browse the repository at this point in the history
## Issue Addressed

N/A

## Proposed Changes

- Removing a bunch of unnecessary references
- Updated `Error::VariantError` to `Error::Variant`
- There were additional enum variant lints that I ignored, because I thought our variant names were fine
- removed `MonitoredValidator`'s `pubkey` field, because I couldn't find it used anywhere. It looks like we just use the string version of the pubkey (the `id` field) if there is no index

## Additional Info



Co-authored-by: realbigsean <[email protected]>
  • Loading branch information
realbigsean and realbigsean committed Jul 30, 2021
1 parent 8efd9fc commit 303deb9
Show file tree
Hide file tree
Showing 104 changed files with 302 additions and 308 deletions.
2 changes: 1 addition & 1 deletion account_manager/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub fn read_mnemonic_from_cli(
.map_err(|e| format!("Unable to read {:?}: {:?}", path, e))
.and_then(|bytes| {
let bytes_no_newlines: PlainText = strip_off_newlines(bytes).into();
let phrase = from_utf8(&bytes_no_newlines.as_ref())
let phrase = from_utf8(bytes_no_newlines.as_ref())
.map_err(|e| format!("Unable to derive mnemonic: {:?}", e))?;
Mnemonic::from_phrase(phrase, Language::English).map_err(|e| {
format!(
Expand Down
2 changes: 1 addition & 1 deletion account_manager/src/validator/exit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.long(BEACON_SERVER_FLAG)
.value_name("NETWORK_ADDRESS")
.help("Address to a beacon node HTTP API")
.default_value(&DEFAULT_BEACON_NODE)
.default_value(DEFAULT_BEACON_NODE)
.takes_value(true),
)
.arg(
Expand Down
8 changes: 4 additions & 4 deletions account_manager/src/validator/slashing_protection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ pub fn cli_run<T: EthSpec>(

match matches.subcommand() {
(IMPORT_CMD, Some(matches)) => {
let import_filename: PathBuf = clap_utils::parse_required(&matches, IMPORT_FILE_ARG)?;
let minify: bool = clap_utils::parse_required(&matches, MINIFY_FLAG)?;
let import_filename: PathBuf = clap_utils::parse_required(matches, IMPORT_FILE_ARG)?;
let minify: bool = clap_utils::parse_required(matches, MINIFY_FLAG)?;
let import_file = File::open(&import_filename).map_err(|e| {
format!(
"Unable to open import file at {}: {:?}",
Expand Down Expand Up @@ -199,8 +199,8 @@ pub fn cli_run<T: EthSpec>(
Ok(())
}
(EXPORT_CMD, Some(matches)) => {
let export_filename: PathBuf = clap_utils::parse_required(&matches, EXPORT_FILE_ARG)?;
let minify: bool = clap_utils::parse_required(&matches, MINIFY_FLAG)?;
let export_filename: PathBuf = clap_utils::parse_required(matches, EXPORT_FILE_ARG)?;
let minify: bool = clap_utils::parse_required(matches, MINIFY_FLAG)?;

if !slashing_protection_db_path.exists() {
return Err(format!(
Expand Down
6 changes: 3 additions & 3 deletions account_manager/src/wallet/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ pub fn cli_run(matches: &ArgMatches, wallet_base_dir: PathBuf) -> Result<(), Str
Language::English,
);

let wallet = create_wallet_from_mnemonic(matches, &wallet_base_dir.as_path(), &mnemonic)?;
let wallet = create_wallet_from_mnemonic(matches, wallet_base_dir.as_path(), &mnemonic)?;

if let Some(path) = mnemonic_output_path {
create_with_600_perms(&path, mnemonic.phrase().as_bytes())
Expand Down Expand Up @@ -168,7 +168,7 @@ pub fn create_wallet_from_mnemonic(
if !path.exists() {
// To prevent users from accidentally supplying their password to the PASSWORD_FLAG and
// create a file with that name, we require that the password has a .pass suffix.
if path.extension() != Some(&OsStr::new("pass")) {
if path.extension() != Some(OsStr::new("pass")) {
return Err(format!(
"Only creates a password file if that file ends in .pass: {:?}",
path
Expand All @@ -189,7 +189,7 @@ pub fn create_wallet_from_mnemonic(
.create_wallet(
wallet_name,
wallet_type,
&mnemonic,
mnemonic,
wallet_password.as_bytes(),
)
.map_err(|e| format!("Unable to create wallet: {:?}", e))?;
Expand Down
2 changes: 1 addition & 1 deletion account_manager/src/wallet/recover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub fn cli_run(matches: &ArgMatches, wallet_base_dir: PathBuf) -> Result<(), Str

let mnemonic = read_mnemonic_from_cli(mnemonic_path, stdin_inputs)?;

let wallet = create_wallet_from_mnemonic(matches, &wallet_base_dir.as_path(), &mnemonic)
let wallet = create_wallet_from_mnemonic(matches, wallet_base_dir.as_path(), &mnemonic)
.map_err(|e| format!("Unable to create wallet: {:?}", e))?;

println!("Your wallet has been successfully recovered.");
Expand Down
20 changes: 10 additions & 10 deletions beacon_node/beacon_chain/src/attestation_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ impl<T: BeaconChainTypes> VerifiedAggregatedAttestation<T> {
//
// Attestations must be for a known block. If the block is unknown, we simply drop the
// attestation and do not delay consideration for later.
let head_block = verify_head_block_is_known(chain, &attestation, None)?;
let head_block = verify_head_block_is_known(chain, attestation, None)?;

// Check the attestation target root is consistent with the head root.
//
Expand All @@ -457,7 +457,7 @@ impl<T: BeaconChainTypes> VerifiedAggregatedAttestation<T> {
//
// Whilst this attestation *technically* could be used to add value to a block, it is
// invalid in the spirit of the protocol. Here we choose safety over profit.
verify_attestation_target_root::<T::EthSpec>(&head_block, &attestation)?;
verify_attestation_target_root::<T::EthSpec>(&head_block, attestation)?;

// Ensure that the attestation has participants.
if attestation.aggregation_bits.is_zero() {
Expand Down Expand Up @@ -628,7 +628,7 @@ impl<T: BeaconChainTypes> VerifiedUnaggregatedAttestation<T> {
// MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance).
//
// We do not queue future attestations for later processing.
verify_propagation_slot_range(chain, &attestation)?;
verify_propagation_slot_range(chain, attestation)?;

// Check to ensure that the attestation is "unaggregated". I.e., it has exactly one
// aggregation bit set.
Expand All @@ -642,10 +642,10 @@ impl<T: BeaconChainTypes> VerifiedUnaggregatedAttestation<T> {
//
// Enforce a maximum skip distance for unaggregated attestations.
let head_block =
verify_head_block_is_known(chain, &attestation, chain.config.import_max_skip_slots)?;
verify_head_block_is_known(chain, attestation, chain.config.import_max_skip_slots)?;

// Check the attestation target root is consistent with the head root.
verify_attestation_target_root::<T::EthSpec>(&head_block, &attestation)?;
verify_attestation_target_root::<T::EthSpec>(&head_block, attestation)?;

Ok(())
}
Expand Down Expand Up @@ -927,7 +927,7 @@ pub fn verify_attestation_signature<T: BeaconChainTypes>(
let signature_set = indexed_attestation_signature_set_from_pubkeys(
|validator_index| pubkey_cache.get(validator_index).map(Cow::Borrowed),
&indexed_attestation.signature,
&indexed_attestation,
indexed_attestation,
&fork,
chain.genesis_validators_root,
&chain.spec,
Expand Down Expand Up @@ -1031,15 +1031,15 @@ pub fn verify_signed_aggregate_signatures<T: BeaconChainTypes>(
let signature_sets = vec![
signed_aggregate_selection_proof_signature_set(
|validator_index| pubkey_cache.get(validator_index).map(Cow::Borrowed),
&signed_aggregate,
signed_aggregate,
&fork,
chain.genesis_validators_root,
&chain.spec,
)
.map_err(BeaconChainError::SignatureSetError)?,
signed_aggregate_signature_set(
|validator_index| pubkey_cache.get(validator_index).map(Cow::Borrowed),
&signed_aggregate,
signed_aggregate,
&fork,
chain.genesis_validators_root,
&chain.spec,
Expand All @@ -1048,7 +1048,7 @@ pub fn verify_signed_aggregate_signatures<T: BeaconChainTypes>(
indexed_attestation_signature_set_from_pubkeys(
|validator_index| pubkey_cache.get(validator_index).map(Cow::Borrowed),
&indexed_attestation.signature,
&indexed_attestation,
indexed_attestation,
&fork,
chain.genesis_validators_root,
&chain.spec,
Expand All @@ -1069,7 +1069,7 @@ fn obtain_indexed_attestation_and_committees_per_slot<T: BeaconChainTypes>(
attestation: &Attestation<T::EthSpec>,
) -> Result<(IndexedAttestation<T::EthSpec>, CommitteesPerSlot), Error> {
map_attestation_committee(chain, attestation, |(committee, committees_per_slot)| {
get_indexed_attestation(committee.committee, &attestation)
get_indexed_attestation(committee.committee, attestation)
.map(|attestation| (attestation, committees_per_slot))
.map_err(Error::Invalid)
})
Expand Down
11 changes: 5 additions & 6 deletions beacon_node/beacon_chain/src/beacon_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1372,7 +1372,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
attester_cache_key,
request_slot,
request_index,
&self,
self,
)?
};
drop(cache_timer);
Expand Down Expand Up @@ -1729,7 +1729,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
self.shuffling_is_compatible(
&att.data.beacon_block_root,
att.data.target.epoch,
&state,
state,
)
})
}
Expand Down Expand Up @@ -2182,9 +2182,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
for attestation in signed_block.message().body().attestations() {
let committee =
state.get_beacon_committee(attestation.data.slot, attestation.data.index)?;
let indexed_attestation =
get_indexed_attestation(&committee.committee, attestation)
.map_err(|e| BlockError::BeaconChainError(e.into()))?;
let indexed_attestation = get_indexed_attestation(committee.committee, attestation)
.map_err(|e| BlockError::BeaconChainError(e.into()))?;
slasher.accept_attestation(indexed_attestation);
}
}
Expand Down Expand Up @@ -3267,7 +3266,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {

metrics::stop_timer(committee_building_timer);

map_fn(&committee_cache, shuffling_decision_block)
map_fn(committee_cache, shuffling_decision_block)
}
}

Expand Down
7 changes: 4 additions & 3 deletions beacon_node/beacon_chain/src/block_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ impl<T: EthSpec> From<DBError> for BlockError<T> {
}

/// Information about invalid blocks which might still be slashable despite being invalid.
#[allow(clippy::enum_variant_names)]
pub enum BlockSlashInfo<TErr> {
/// The block is invalid, but its proposer signature wasn't checked.
SignatureNotChecked(SignedBeaconBlockHeader, TErr),
Expand Down Expand Up @@ -837,7 +838,7 @@ impl<T: BeaconChainTypes> IntoFullyVerifiedBlock<T> for SignedBeaconBlock<T::Eth
}

fn block(&self) -> &SignedBeaconBlock<T::EthSpec> {
&self
self
}
}

Expand Down Expand Up @@ -996,7 +997,7 @@ impl<'a, T: BeaconChainTypes> FullyVerifiedBlock<'a, T> {
for (i, summary) in summaries.iter().enumerate() {
let epoch = state.current_epoch() - Epoch::from(summaries.len() - i);
if let Err(e) =
validator_monitor.process_validator_statuses(epoch, &summary, &chain.spec)
validator_monitor.process_validator_statuses(epoch, summary, &chain.spec)
{
error!(
chain.log,
Expand Down Expand Up @@ -1204,7 +1205,7 @@ pub fn check_block_relevancy<T: BeaconChainTypes>(
// Do not process a block from a finalized slot.
check_block_against_finalized_slot(block, chain)?;

let block_root = block_root.unwrap_or_else(|| get_block_root(&signed_block));
let block_root = block_root.unwrap_or_else(|| get_block_root(signed_block));

// Check if the block is already known. We know it is post-finalization, so it is
// sufficient to check the fork choice.
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/beacon_chain/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,7 @@ fn genesis_block<T: EthSpec>(
genesis_state: &mut BeaconState<T>,
spec: &ChainSpec,
) -> Result<SignedBeaconBlock<T>, String> {
let mut genesis_block = BeaconBlock::empty(&spec);
let mut genesis_block = BeaconBlock::empty(spec);
*genesis_block.state_root_mut() = genesis_state
.update_tree_hash_cache()
.map_err(|e| format!("Error hashing genesis state: {:?}", e))?;
Expand Down
16 changes: 8 additions & 8 deletions beacon_node/beacon_chain/src/eth1_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ mod test {
"test should not use dummy backend"
);

let mut state: BeaconState<E> = BeaconState::new(0, get_eth1_data(0), &spec);
let mut state: BeaconState<E> = BeaconState::new(0, get_eth1_data(0), spec);
*state.eth1_deposit_index_mut() = 0;
state.eth1_data_mut().deposit_count = 0;

Expand Down Expand Up @@ -815,7 +815,7 @@ mod test {
"cache should store all logs"
);

let mut state: BeaconState<E> = BeaconState::new(0, get_eth1_data(0), &spec);
let mut state: BeaconState<E> = BeaconState::new(0, get_eth1_data(0), spec);
*state.eth1_deposit_index_mut() = 0;
state.eth1_data_mut().deposit_count = 0;

Expand Down Expand Up @@ -877,10 +877,10 @@ mod test {
"test should not use dummy backend"
);

let state: BeaconState<E> = BeaconState::new(0, get_eth1_data(0), &spec);
let state: BeaconState<E> = BeaconState::new(0, get_eth1_data(0), spec);

let a = eth1_chain
.eth1_data_for_block_production(&state, &spec)
.eth1_data_for_block_production(&state, spec)
.expect("should produce default eth1 data vote");
assert_eq!(
a,
Expand All @@ -902,11 +902,11 @@ mod test {
"test should not use dummy backend"
);

let mut state: BeaconState<E> = BeaconState::new(0, get_eth1_data(0), &spec);
let mut state: BeaconState<E> = BeaconState::new(0, get_eth1_data(0), spec);

*state.slot_mut() = Slot::from(slots_per_eth1_voting_period * 10);
let follow_distance_seconds = eth1_follow_distance * spec.seconds_per_eth1_block;
let voting_period_start = get_voting_period_start_seconds(&state, &spec);
let voting_period_start = get_voting_period_start_seconds(&state, spec);
let start_eth1_block = voting_period_start - follow_distance_seconds * 2;
let end_eth1_block = voting_period_start - follow_distance_seconds;

Expand All @@ -926,7 +926,7 @@ mod test {
});

let vote = eth1_chain
.eth1_data_for_block_production(&state, &spec)
.eth1_data_for_block_production(&state, spec)
.expect("should produce default eth1 data vote");

assert_eq!(
Expand Down Expand Up @@ -956,7 +956,7 @@ mod test {
get_votes_to_consider(
blocks.iter(),
get_voting_period_start_seconds(&state, spec),
&spec,
spec,
),
HashMap::new()
);
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/beacon_chain/src/naive_aggregation_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ mod tests {
}

fn key_from_sync_contribution(a: &SyncCommitteeContribution<E>) -> SyncContributionData {
SyncContributionData::from_contribution(&a)
SyncContributionData::from_contribution(a)
}

macro_rules! test_suite {
Expand Down
4 changes: 2 additions & 2 deletions beacon_node/beacon_chain/src/sync_committee_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -573,15 +573,15 @@ pub fn verify_signed_aggregate_signatures<T: BeaconChainTypes>(
let signature_sets = vec![
signed_sync_aggregate_selection_proof_signature_set(
|validator_index| pubkey_cache.get(validator_index).map(Cow::Borrowed),
&signed_aggregate,
signed_aggregate,
&fork,
chain.genesis_validators_root,
&chain.spec,
)
.map_err(BeaconChainError::SignatureSetError)?,
signed_sync_aggregate_signature_set(
|validator_index| pubkey_cache.get(validator_index).map(Cow::Borrowed),
&signed_aggregate,
signed_aggregate,
&fork,
chain.genesis_validators_root,
&chain.spec,
Expand Down
12 changes: 6 additions & 6 deletions beacon_node/beacon_chain/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -698,8 +698,8 @@ where
slot: Slot,
) -> HarnessAttestations<E> {
let unaggregated_attestations = self.make_unaggregated_attestations(
&attesting_validators,
&state,
attesting_validators,
state,
state_root,
block_hash,
slot,
Expand Down Expand Up @@ -785,7 +785,7 @@ where
relative_sync_committee: RelativeSyncCommittee,
) -> HarnessSyncContributions<E> {
let sync_messages =
self.make_sync_committee_messages(&state, block_hash, slot, relative_sync_committee);
self.make_sync_committee_messages(state, block_hash, slot, relative_sync_committee);

let sync_contributions: Vec<Option<SignedContributionAndProof<E>>> = sync_messages
.iter()
Expand Down Expand Up @@ -825,7 +825,7 @@ where
})?;

let default = SyncCommitteeContribution::from_message(
&sync_message,
sync_message,
subnet_id as u64,
*subcommittee_position,
)
Expand Down Expand Up @@ -989,7 +989,7 @@ where
let mut signed_block_headers = vec![block_header_1, block_header_2]
.into_iter()
.map(|block_header| {
block_header.sign::<E>(&sk, &fork, genesis_validators_root, &self.chain.spec)
block_header.sign::<E>(sk, &fork, genesis_validators_root, &self.chain.spec)
})
.collect::<Vec<_>>();

Expand Down Expand Up @@ -1199,7 +1199,7 @@ where
validators: &[usize],
) {
let attestations =
self.make_attestations(validators, &state, state_root, block_hash, block.slot());
self.make_attestations(validators, state, state_root, block_hash, block.slot());
self.process_attestations(attestations);
}

Expand Down
3 changes: 0 additions & 3 deletions beacon_node/beacon_chain/src/validator_monitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,6 @@ type SummaryMap = HashMap<Epoch, EpochSummary>;
struct MonitoredValidator {
/// A human-readable identifier for the validator.
pub id: String,
/// The validator voting pubkey.
pub pubkey: PublicKeyBytes,
/// The validator index in the state.
pub index: Option<u64>,
/// A history of the validator over time.
Expand All @@ -140,7 +138,6 @@ impl MonitoredValidator {
id: index
.map(|i| i.to_string())
.unwrap_or_else(|| pubkey.to_string()),
pubkey,
index,
summaries: <_>::default(),
}
Expand Down
Loading

0 comments on commit 303deb9

Please sign in to comment.