gnosis/lighthouse
1
0
Fork 0
mirror of https://github.com/sigp/lighthouse.git synced 2026-05-08 17:26:04 +00:00
Code Issues Packages Projects Releases Wiki Activity

Address clippy lints in tree-states (#4414)

Browse Source 
* Address some clippy lints

* Box errors to fix error size lint

* Add Default impl for Validator

* Address more clippy lints

* Re-implement `check_state_diff`

* Fix misc test compile errors
This commit is contained in:
Paul Hauner
2023-06-20 11:47:52 +10:00
committed by GitHub
parent 23db089a7a
commit d56cec83fc
26 changed files with 94 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
beacon_node/beacon_chain/src/migrate.rs
View File

@@ -400,7 +400,7 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> BackgroundMigrator<E, Ho
.max_by_key(|f| f.finalized_checkpoint.epoch);
// Do a bit of state reconstruction first if required.
if let Some(_) = reconstruction_notif {
if reconstruction_notif.is_some() {
let timer = std::time::Instant::now();
match db.reconstruct_historic_states(Some(BLOCKS_PER_RECONSTRUCTION)) {

2
beacon_node/beacon_chain/src/sync_committee_rewards.rs
View File

@@ -65,7 +65,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
.zip(sync_aggregate.sync_committee_bits.iter())
{
let participant_balance = balances
.get_mut(&validator_index)
.get_mut(validator_index)
.ok_or(BeaconChainError::SyncCommitteeRewardsSyncError)?;
if participant_bit {

2
beacon_node/beacon_chain/tests/store_tests.rs
View File

@@ -28,8 +28,6 @@ use store::{
HotColdDB, LevelDB, StoreConfig,
};
use tempfile::{tempdir, TempDir};
use tokio::time::sleep;
use tree_hash::TreeHash;
use types::test_utils::{SeedableRng, XorShiftRng};
use types::*;

8
beacon_node/execution_layer/src/lib.rs
View File

@@ -38,11 +38,11 @@ use tokio::{
};
use tokio_stream::wrappers::WatchStream;
use tree_hash::TreeHash;
use types::{AbstractExecPayload, BeaconStateError, ExecPayload, Withdrawals};
use types::{AbstractExecPayload, BeaconStateError, ExecPayload};
use types::{
BlindedPayload, BlockType, ChainSpec, Epoch, ExecutionBlockHash, ExecutionPayload,
ExecutionPayloadCapella, ExecutionPayloadMerge, ForkName, ForkVersionedResponse,
ProposerPreparationData, PublicKeyBytes, Signature, SignedBeaconBlock, Slot, Uint256,
BlindedPayload, BlockType, ChainSpec, Epoch, ExecutionPayloadCapella, ExecutionPayloadMerge,
ForkVersionedResponse, ProposerPreparationData, PublicKeyBytes, Signature, SignedBeaconBlock,
Slot,
};
mod block_hash;

3
beacon_node/genesis/src/interop.rs
View File

@@ -241,7 +241,8 @@ mod test {
}
for (index, v) in state.validators().iter().enumerate() {
let creds = v.withdrawal_credentials.as_bytes();
let withdrawal_credientials = v.withdrawal_credentials();
let creds = withdrawal_credientials.as_bytes();
if index % 2 == 0 {
assert_eq!(
creds[0], spec.bls_withdrawal_prefix_byte,

2
beacon_node/operation_pool/src/lib.rs
View File

@@ -1249,7 +1249,7 @@ mod release_tests {
// Each validator will have a multiple of 1_000_000_000 wei.
// Safe from overflow unless there are about 18B validators (2^64 / 1_000_000_000).
for i in 0..state.validators().len() {
state.validators_mut()[i].effective_balance = 1_000_000_000 * i as u64;
state.validators_mut().get_mut(i).unwrap().effective_balance = 1_000_000_000 * i as u64;
}
let num_validators = num_committees

2
beacon_node/store/src/forwards_iter.rs
View File

@@ -234,7 +234,7 @@ impl<'a, E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>>
// of the pre iterator.
None => {
let continuation_data = continuation_data.take();
let start_slot = Slot::from(iter.limit);
let start_slot = iter.limit;
*self = PostFinalizationLazy {
continuation_data,

2
beacon_node/store/src/hdiff.rs
View File

@@ -331,7 +331,7 @@ mod tests {
let xor_diff = XorDiff::compute(&x_values, &y_values).unwrap();
let mut y_from_xor = x_values.clone();
let mut y_from_xor = x_values;
xor_diff.apply(&mut y_from_xor).unwrap();
assert_eq!(y_values, y_from_xor);

2
beacon_node/store/src/hot_cold_store.rs
View File

@@ -1573,7 +1573,7 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
(start_slot.as_u64()..=end_slot.as_u64())
.map(Slot::new)
.map(|slot| self.get_cold_blinded_block_by_slot(slot)),
|iter| iter.filter_map(|x| x).collect(),
|iter| iter.flatten().collect(),
)
}

2
beacon_node/store/src/leveldb_store.rs
View File

@@ -183,7 +183,7 @@ impl<E: EthSpec> KeyValueStore<E> for LevelDB<E> {
}
fn iter_column_from<K: Key>(&self, column: DBColumn, from: &[u8]) -> ColumnIter<K> {
let start_key = BytesKey::from_vec(get_key_for_col(column.into(), &from));
let start_key = BytesKey::from_vec(get_key_for_col(column.into(), from));
let iter = self.db.iter(self.read_options());
iter.seek(&start_key);

4
consensus/fork_choice/tests/tests.rs
View File

@@ -345,7 +345,7 @@ impl ForkChoiceTest {
let state_root = harness
.chain
.store
.get_blinded_block(&fc.fc_store().justified_checkpoint().root)
.get_blinded_block(&fc.fc_store().justified_checkpoint().root, None)
.unwrap()
.unwrap()
.message()
@@ -361,7 +361,7 @@ impl ForkChoiceTest {
.into_iter()
.map(|v| {
if v.is_active_at(state.current_epoch()) {
v.effective_balance
v.effective_balance()
} else {
0
}

6
consensus/state_processing/src/per_block_processing/errors.rs
View File

@@ -329,9 +329,11 @@ pub enum AttestationInvalid {
///
/// `is_current` is `true` if the attestation was compared to the
/// `state.current_justified_checkpoint`, `false` if compared to `state.previous_justified_checkpoint`.
///
/// Checkpoints have been boxed to keep the error size down and prevent clippy failures.
WrongJustifiedCheckpoint {
state: Checkpoint,
attestation: Checkpoint,
state: Box<Checkpoint>,
attestation: Box<Checkpoint>,
is_current: bool,
},
/// The aggregation bitfield length is not the smallest possible size to represent the committee.

4
consensus/state_processing/src/per_block_processing/tests.rs
View File

@@ -451,8 +451,8 @@ async fn invalid_attestation_wrong_justified_checkpoint() {
Err(BlockProcessingError::AttestationInvalid {
index: 0,
reason: AttestationInvalid::WrongJustifiedCheckpoint {
state: old_justified_checkpoint,
attestation: new_justified_checkpoint,
state: Box::new(old_justified_checkpoint),
attestation: Box::new(new_justified_checkpoint),
is_current: true,
}
})

8
consensus/state_processing/src/per_block_processing/verify_attestation.rs
View File

@@ -93,8 +93,8 @@ fn verify_casper_ffg_vote<T: EthSpec>(
verify!(
data.source == state.current_justified_checkpoint(),
Invalid::WrongJustifiedCheckpoint {
state: state.current_justified_checkpoint(),
attestation: data.source,
state: Box::new(state.current_justified_checkpoint()),
attestation: Box::new(data.source),
is_current: true,
}
);
@@ -103,8 +103,8 @@ fn verify_casper_ffg_vote<T: EthSpec>(
verify!(
data.source == state.previous_justified_checkpoint(),
Invalid::WrongJustifiedCheckpoint {
state: state.previous_justified_checkpoint(),
attestation: data.source,
state: Box::new(state.previous_justified_checkpoint()),
attestation: Box::new(data.source),
is_current: false,
}
);

12
consensus/types/src/beacon_state/tests.rs
View File

@@ -1,13 +1,10 @@
#![cfg(test)]
use crate::{test_utils::*, ForkName};
use beacon_chain::test_utils::{
interop_genesis_state_with_eth1, test_spec, BeaconChainHarness, EphemeralHarnessType,
DEFAULT_ETH1_BLOCK_HASH,
};
use beacon_chain::test_utils::{BeaconChainHarness, EphemeralHarnessType};
use beacon_chain::types::{
test_utils::TestRandom, BeaconState, BeaconStateAltair, BeaconStateBase, BeaconStateError,
BeaconStateMerge, ChainSpec, Domain, Epoch, EthSpec, FixedVector, Hash256, Keypair,
MainnetEthSpec, MinimalEthSpec, RelativeEpoch, Slot,
test_utils::TestRandom, BeaconState, BeaconStateAltair, BeaconStateBase, BeaconStateCapella,
BeaconStateError, BeaconStateMerge, ChainSpec, Domain, Epoch, EthSpec, FixedVector, Hash256,
Keypair, MainnetEthSpec, MinimalEthSpec, RelativeEpoch, Slot,
};
use ssz::Encode;
use std::ops::Mul;
@@ -418,6 +415,7 @@ fn check_num_fields_pow2() {
ForkName::Base => BeaconStateBase::<E>::NUM_FIELDS,
ForkName::Altair => BeaconStateAltair::<E>::NUM_FIELDS,
ForkName::Merge => BeaconStateMerge::<E>::NUM_FIELDS,
ForkName::Capella => BeaconStateCapella::<E>::NUM_FIELDS,
};
assert_eq!(
num_fields.next_power_of_two(),

11
consensus/types/src/validator.rs
View File

@@ -230,7 +230,15 @@ impl Validator {
}
}
/*
impl Default for Validator {
fn default() -> Self {
Validator {
pubkey: Arc::new(PublicKeyBytes::empty()),
mutable: <_>::default(),
}
}
}
impl Default for ValidatorMutable {
fn default() -> Self {
ValidatorMutable {
@@ -244,7 +252,6 @@ impl Default for ValidatorMutable {
}
}
}
*/
impl TreeHash for Validator {
fn tree_hash_type() -> tree_hash::TreeHashType {

29
lcli/src/new_testnet.rs
View File

@@ -14,13 +14,14 @@ use std::fs::File;
use std::io::Read;
use std::path::PathBuf;
use std::str::FromStr;
use std::sync::Arc;
use std::time::{SystemTime, UNIX_EPOCH};
use types::ExecutionBlockHash;
use types::{
test_utils::generate_deterministic_keypairs, Address, BeaconState, ChainSpec, Config, Epoch,
Eth1Data, EthSpec, ExecutionPayloadHeader, ExecutionPayloadHeaderCapella,
ExecutionPayloadHeaderMerge, ExecutionPayloadHeaderRefMut, ForkName, Hash256, Keypair,
PublicKey, Validator,
PublicKey, Validator, ValidatorMutable,
};
pub fn run<T: EthSpec>(testnet_dir_path: PathBuf, matches: &ArgMatches) -> Result<(), String> {
@@ -233,7 +234,7 @@ fn initialize_state_with_validators<T: EthSpec>(
let mut state = BeaconState::new(genesis_time, eth1_data, spec);
// Seed RANDAO with Eth1 entropy
state.fill_randao_mixes_with(eth1_block_hash);
state.fill_randao_mixes_with(eth1_block_hash).unwrap();
for keypair in keypairs.iter() {
let withdrawal_credentials = |pubkey: &PublicKey| {
@@ -244,17 +245,19 @@ fn initialize_state_with_validators<T: EthSpec>(
let amount = spec.max_effective_balance;
// Create a new validator.
let validator = Validator {
pubkey: keypair.0.pk.clone().into(),
withdrawal_credentials: withdrawal_credentials(&keypair.1.pk),
activation_eligibility_epoch: spec.far_future_epoch,
activation_epoch: spec.far_future_epoch,
exit_epoch: spec.far_future_epoch,
withdrawable_epoch: spec.far_future_epoch,
effective_balance: std::cmp::min(
amount - amount % (spec.effective_balance_increment),
spec.max_effective_balance,
),
slashed: false,
pubkey: Arc::new(keypair.0.pk.clone().into()),
mutable: ValidatorMutable {
withdrawal_credentials: withdrawal_credentials(&keypair.1.pk),
activation_eligibility_epoch: spec.far_future_epoch,
activation_epoch: spec.far_future_epoch,
exit_epoch: spec.far_future_epoch,
withdrawable_epoch: spec.far_future_epoch,
effective_balance: std::cmp::min(
amount - amount % (spec.effective_balance_increment),
spec.max_effective_balance,
),
slashed: false,
},
};
state.validators_mut().push(validator).unwrap();
state.balances_mut().push(amount).unwrap();

8
lcli/src/state_diff.rs
View File

@@ -1,14 +1,12 @@
use crate::transition_blocks::load_from_ssz_with;
use clap::ArgMatches;
use clap_utils::{parse_optional, parse_required};
use clap_utils::parse_required;
use environment::Environment;
use eth2::{types::BlockId, BeaconNodeHttpClient, SensitiveUrl, Timeouts};
use std::path::PathBuf;
use std::time::{Duration, Instant};
use store::hdiff::{HDiff, HDiffBuffer};
use types::{BeaconState, EthSpec, FullPayload, SignedBeaconBlock};
use types::{BeaconState, EthSpec};
pub fn run<T: EthSpec>(env: Environment<T>, matches: &ArgMatches) -> Result<(), String> {
pub fn run<T: EthSpec>(_env: Environment<T>, matches: &ArgMatches) -> Result<(), String> {
let state1_path: PathBuf = parse_required(matches, "state1")?;
let state2_path: PathBuf = parse_required(matches, "state2")?;
let spec = &T::default_spec();

2
lcli/src/transition_blocks.rs
View File

@@ -74,7 +74,7 @@ use eth2::{
use ssz::Encode;
use state_processing::{
block_signature_verifier::BlockSignatureVerifier, per_block_processing, per_slot_processing,
BlockSignatureStrategy, ConsensusContext, StateProcessingStrategy, VerifyBlockRoot,
BlockSignatureStrategy, ConsensusContext, EpochCache, StateProcessingStrategy, VerifyBlockRoot,
};
use std::borrow::Cow;
use std::fs::File;

28
testing/ef_tests/src/case_result.rs
View File

@@ -2,7 +2,8 @@ use super::*;
use compare_fields::{CompareFields, Comparison, FieldComparison};
use std::fmt::Debug;
use std::path::{Path, PathBuf};
use types::{beacon_state::BeaconStateDiff, milhouse::diff::Diff, BeaconState};
use store::hdiff::{HDiff, HDiffBuffer};
use types::BeaconState;
pub const MAX_VALUE_STRING_LEN: usize = 500;
@@ -121,15 +122,28 @@ where
pub fn check_state_diff<T: EthSpec>(
pre_state: &BeaconState<T>,
opt_post_state: &Option<BeaconState<T>>,
spec: &ChainSpec,
) -> Result<(), Error> {
if let Some(post_state) = opt_post_state {
let diff = BeaconStateDiff::compute_diff(pre_state, post_state)
.expect("BeaconStateDiff should compute");
let mut diffed_state = pre_state.clone();
diff.apply_diff(&mut diffed_state)
.expect("BeaconStateDiff should apply");
// Produce a diff between the pre- and post-states.
let pre_state_buf = HDiffBuffer::from_state(pre_state.clone());
let post_state_buf = HDiffBuffer::from_state(post_state.clone());
let diff = HDiff::compute(&pre_state_buf, &post_state_buf).expect("HDiff should compute");
compare_result_detailed::<_, ()>(&Ok(diffed_state), opt_post_state)
// Apply the diff to the pre-state, ensuring the same post-state is
// regenerated.
let mut reconstructed_buf = HDiffBuffer::from_state(pre_state.clone());
diff.apply(&mut reconstructed_buf)
.expect("HDiff should apply");
let diffed_state = reconstructed_buf
.into_state(spec)
.expect("HDiffDiffer should convert to state");
// Drop the caches on the post-state to assist with equality checking.
let mut post_state_without_caches = post_state.clone();
post_state_without_caches.drop_all_caches().unwrap();
compare_result_detailed::<_, ()>(&Ok(diffed_state), &Some(post_state_without_caches))
} else {
Ok(())
}

2
testing/ef_tests/src/cases/epoch_processing.rs
View File

@@ -328,7 +328,7 @@ impl<E: EthSpec, T: EpochTransition<E>> Case for EpochProcessing<E, T> {
let mut result = T::run(&mut state, spec).map(|_| state);
check_state_diff(&pre_state, &expected)?;
check_state_diff(&pre_state, &expected, spec)?;
compare_beacon_state_results_without_caches(&mut result, &mut expected)
}
}

4
testing/ef_tests/src/cases/operations.rs
View File

@@ -458,7 +458,7 @@ impl<E: EthSpec, O: Operation<E>> Case for Operations<E, O> {
// NOTE: some of the withdrawals tests have 0 active validators, do not try
// to build the commitee cache in this case.
if O::handler_name() != "withdrawals" {
state.build_all_committee_caches(spec).unwrap();
pre_state.build_all_committee_caches(spec).unwrap();
}
let mut state = pre_state.clone();
@@ -475,7 +475,7 @@ impl<E: EthSpec, O: Operation<E>> Case for Operations<E, O> {
.apply_to(&mut state, spec, self)
.map(|()| state);
check_state_diff(&pre_state, &expected)?;
check_state_diff(&pre_state, &expected, spec)?;
compare_beacon_state_results_without_caches(&mut result, &mut expected)
}
}

2
testing/ef_tests/src/cases/sanity_blocks.rs
View File

@@ -137,7 +137,7 @@ impl<E: EthSpec> Case for SanityBlocks<E> {
post.build_all_committee_caches(spec).unwrap();
post
});
check_state_diff(&pre, &post)?;
check_state_diff(&pre, &post, spec)?;
Ok(())
}
}

2
testing/ef_tests/src/cases/sanity_slots.rs
View File

@@ -76,6 +76,6 @@ impl<E: EthSpec> Case for SanitySlots<E> {
post.build_all_committee_caches(spec).unwrap();
post
});
check_state_diff(&pre, &post)
check_state_diff(&pre, &post, spec)
}
}

8
testing/state_transition_vectors/src/exit.rs
View File

@@ -336,17 +336,17 @@ mod custom_tests {
fn assert_exited(state: &BeaconState<E>, validator_index: usize) {
let spec = E::default_spec();
let validator = &state.validators()[validator_index];
let validator = &state.validators().get(validator_index).unwrap();
assert_eq!(
validator.exit_epoch,
validator.exit_epoch(),
// This is correct until we exceed the churn limit. If that happens, we
// need to introduce more complex logic.
state.current_epoch() + 1 + spec.max_seed_lookahead,
"exit epoch"
);
assert_eq!(
validator.withdrawable_epoch,
validator.exit_epoch + E::default_spec().min_validator_withdrawability_delay,
validator.withdrawable_epoch(),
validator.exit_epoch() + E::default_spec().min_validator_withdrawability_delay,
"withdrawable epoch"
);
}

10
watch/src/updater/mod.rs
View File

@@ -211,20 +211,20 @@ pub async fn get_validators(bn: &BeaconNodeHttpClient) -> Result<HashSet<WatchVa
for val in validators {
// Only store `activation_epoch` if it not the `FAR_FUTURE_EPOCH`.
let activation_epoch = if val.validator.activation_epoch.as_u64() == FAR_FUTURE_EPOCH {
let activation_epoch = if val.validator.activation_epoch().as_u64() == FAR_FUTURE_EPOCH {
None
} else {
Some(val.validator.activation_epoch.as_u64() as i32)
Some(val.validator.activation_epoch().as_u64() as i32)
};
// Only store `exit_epoch` if it is not the `FAR_FUTURE_EPOCH`.
let exit_epoch = if val.validator.exit_epoch.as_u64() == FAR_FUTURE_EPOCH {
let exit_epoch = if val.validator.exit_epoch().as_u64() == FAR_FUTURE_EPOCH {
None
} else {
Some(val.validator.exit_epoch.as_u64() as i32)
Some(val.validator.exit_epoch().as_u64() as i32)
};
validator_map.insert(WatchValidator {
index: val.index as i32,
public_key: WatchPK::from_pubkey(val.validator.pubkey),
public_key: WatchPK::from_pubkey(*val.validator.pubkey),
status: val.status.to_string(),
activation_epoch,
exit_epoch,