Skip to content

Commit

Permalink
Stop Encode/Decode Bounds from Propagating Out
Browse files Browse the repository at this point in the history
  • Loading branch information
ethDreamer committed Apr 27, 2024
1 parent 2594274 commit 3a7b036
Show file tree
Hide file tree
Showing 11 changed files with 212 additions and 48 deletions.
10 changes: 5 additions & 5 deletions beacon_node/beacon_chain/src/beacon_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ pub struct BeaconChain<T: BeaconChainTypes> {
pub observed_proposer_slashings: Mutex<ObservedOperations<ProposerSlashing, T::EthSpec>>,
/// Maintains a record of which validators we've seen attester slashings for.
pub observed_attester_slashings:
Mutex<ObservedOperations<AttesterSlashingOnDisk<T::EthSpec>, T::EthSpec>>,
Mutex<ObservedOperations<AttesterSlashing<T::EthSpec>, T::EthSpec>>,
/// Maintains a record of which validators we've seen BLS to execution changes for.
pub observed_bls_to_execution_changes:
Mutex<ObservedOperations<SignedBlsToExecutionChange, T::EthSpec>>,
Expand Down Expand Up @@ -2443,10 +2443,10 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
pub fn verify_attester_slashing_for_gossip(
&self,
attester_slashing: AttesterSlashing<T::EthSpec>,
) -> Result<ObservationOutcome<AttesterSlashingOnDisk<T::EthSpec>, T::EthSpec>, Error> {
) -> Result<ObservationOutcome<AttesterSlashing<T::EthSpec>, T::EthSpec>, Error> {
let wall_clock_state = self.wall_clock_state()?;
Ok(self.observed_attester_slashings.lock().verify_and_observe(
attester_slashing.into(),
attester_slashing,
&wall_clock_state,
&self.spec,
)?)
Expand All @@ -2458,7 +2458,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
/// 2. Add it to the op pool.
pub fn import_attester_slashing(
&self,
attester_slashing: SigVerifiedOp<AttesterSlashingOnDisk<T::EthSpec>, T::EthSpec>,
attester_slashing: SigVerifiedOp<AttesterSlashing<T::EthSpec>, T::EthSpec>,
) {
// Add to fork choice.
self.canonical_head
Expand Down Expand Up @@ -4941,7 +4941,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
});

attester_slashings.retain(|slashing| {
AttesterSlashingOnDisk::from(slashing.clone())
AttesterSlashing::from(slashing.clone())
.validate(&state, &self.spec)
.map_err(|e| {
warn!(
Expand Down
14 changes: 6 additions & 8 deletions beacon_node/beacon_chain/src/observed_operations.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
use derivative::Derivative;
use smallvec::{smallvec, SmallVec};
use ssz::{Decode, Encode};
use state_processing::{SigVerifiedOp, VerifyOperation, VerifyOperationAt};
use state_processing::{SigVerifiedOp, TransformPersist, VerifyOperation, VerifyOperationAt};
use std::collections::HashSet;
use std::marker::PhantomData;
use types::{
AttesterSlashingOnDisk, BeaconState, ChainSpec, Epoch, EthSpec, ForkName, ProposerSlashing,
AttesterSlashing, BeaconState, ChainSpec, Epoch, EthSpec, ForkName, ProposerSlashing,
SignedBlsToExecutionChange, SignedVoluntaryExit, Slot,
};

Expand Down Expand Up @@ -34,7 +33,7 @@ pub struct ObservedOperations<T: ObservableOperation<E>, E: EthSpec> {

/// Was the observed operation new and valid for further processing, or a useless duplicate?
#[derive(Debug, PartialEq, Eq, Clone)]
pub enum ObservationOutcome<T: Encode + Decode, E: EthSpec> {
pub enum ObservationOutcome<T: TransformPersist, E: EthSpec> {
New(SigVerifiedOp<T, E>),
AlreadyKnown,
}
Expand All @@ -59,16 +58,15 @@ impl<E: EthSpec> ObservableOperation<E> for ProposerSlashing {
}
}

impl<E: EthSpec> ObservableOperation<E> for AttesterSlashingOnDisk<E> {
impl<E: EthSpec> ObservableOperation<E> for AttesterSlashing<E> {
fn observed_validators(&self) -> SmallVec<[u64; SMALL_VEC_SIZE]> {
let slashing_ref = self.to_ref();
let attestation_1_indices = slashing_ref
let attestation_1_indices = self
.attestation_1()
.attesting_indices
.iter()
.copied()
.collect::<HashSet<u64>>();
let attestation_2_indices = slashing_ref
let attestation_2_indices = self
.attestation_2()
.attesting_indices
.iter()
Expand Down
6 changes: 3 additions & 3 deletions beacon_node/beacon_chain/src/validator_monitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1799,16 +1799,16 @@ impl<E: EthSpec> ValidatorMonitor<E> {
}

fn register_attester_slashing(&self, src: &str, slashing: AttesterSlashingRef<'_, E>) {
let data = &slashing.attestation_1.data;
let data = &slashing.attestation_1().data;
let attestation_1_indices: HashSet<u64> = slashing
.attestation_1
.attestation_1()
.attesting_indices
.iter()
.copied()
.collect();

slashing
.attestation_2
.attestation_2()
.attesting_indices
.iter()
.filter(|index| attestation_1_indices.contains(index))
Expand Down
11 changes: 5 additions & 6 deletions beacon_node/operation_pool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,9 @@ use std::marker::PhantomData;
use std::ptr;
use types::{
sync_aggregate::Error as SyncAggregateError, typenum::Unsigned, AbstractExecPayload,
Attestation, AttestationData, AttesterSlashing, AttesterSlashingOnDisk, BeaconState,
BeaconStateError, ChainSpec, Epoch, EthSpec, ProposerSlashing, SignedBeaconBlock,
SignedBlsToExecutionChange, SignedVoluntaryExit, Slot, SyncAggregate,
SyncCommitteeContribution, Validator,
Attestation, AttestationData, AttesterSlashing, BeaconState, BeaconStateError, ChainSpec,
Epoch, EthSpec, ProposerSlashing, SignedBeaconBlock, SignedBlsToExecutionChange,
SignedVoluntaryExit, Slot, SyncAggregate, SyncCommitteeContribution, Validator,
};

type SyncContributions<E> = RwLock<HashMap<SyncAggregateId, Vec<SyncCommitteeContribution<E>>>>;
Expand All @@ -54,7 +53,7 @@ pub struct OperationPool<E: EthSpec + Default> {
/// Map from sync aggregate ID to the best `SyncCommitteeContribution`s seen for that ID.
sync_contributions: SyncContributions<E>,
/// Set of attester slashings, and the fork version they were verified against.
attester_slashings: RwLock<HashSet<SigVerifiedOp<AttesterSlashingOnDisk<E>, E>>>,
attester_slashings: RwLock<HashSet<SigVerifiedOp<AttesterSlashing<E>, E>>>,
/// Map from proposer index to slashing.
proposer_slashings: RwLock<HashMap<u64, SigVerifiedOp<ProposerSlashing, E>>>,
/// Map from exiting validator to their exit data.
Expand Down Expand Up @@ -367,7 +366,7 @@ impl<E: EthSpec> OperationPool<E> {
/// Insert an attester slashing into the pool.
pub fn insert_attester_slashing(
&self,
verified_slashing: SigVerifiedOp<AttesterSlashingOnDisk<E>, E>,
verified_slashing: SigVerifiedOp<AttesterSlashing<E>, E>,
) {
self.attester_slashings.write().insert(verified_slashing);
}
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/operation_pool/src/persistence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub struct PersistedOperationPool<E: EthSpec> {
/// TODO(electra): we've made a DB change here!!!
/// Attester slashings.
#[superstruct(only(V12, V14, V15))]
pub attester_slashings: Vec<SigVerifiedOp<AttesterSlashingOnDisk<E>, E>>,
pub attester_slashings: Vec<SigVerifiedOp<AttesterSlashing<E>, E>>,
/// [DEPRECATED] Proposer slashings.
#[superstruct(only(V5))]
pub proposer_slashings_v5: Vec<ProposerSlashing>,
Expand Down
2 changes: 1 addition & 1 deletion consensus/state_processing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,4 @@ pub use per_epoch_processing::{
};
pub use per_slot_processing::{per_slot_processing, Error as SlotProcessingError};
pub use types::{EpochCache, EpochCacheError, EpochCacheKey};
pub use verify_operation::{SigVerifiedOp, VerifyOperation, VerifyOperationAt};
pub use verify_operation::{SigVerifiedOp, TransformPersist, VerifyOperation, VerifyOperationAt};
Original file line number Diff line number Diff line change
Expand Up @@ -347,14 +347,14 @@ where
state,
get_pubkey.clone(),
&attester_slashing.attestation_1().signature,
&attester_slashing.attestation_1(),
attester_slashing.attestation_1(),
spec,
)?,
indexed_attestation_signature_set(
state,
get_pubkey,
&attester_slashing.attestation_2().signature,
&attester_slashing.attestation_2(),
attester_slashing.attestation_2(),
spec,
)?,
))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ fn error(reason: Invalid) -> BlockOperationError<Invalid> {
/// Returns `Ok(indices)` with `indices` being a non-empty vec of validator indices in ascending
/// order if the `AttesterSlashing` is valid. Otherwise returns `Err(e)` with the reason for
/// invalidity.
pub fn verify_attester_slashing<'a, E: EthSpec>(
pub fn verify_attester_slashing<E: EthSpec>(
state: &BeaconState<E>,
attester_slashing: AttesterSlashingRef<'a, E>,
attester_slashing: AttesterSlashingRef<'_, E>,
verify_signatures: VerifySignatures,
spec: &ChainSpec,
) -> Result<Vec<u64>> {
Expand All @@ -43,9 +43,9 @@ pub fn verify_attester_slashing<'a, E: EthSpec>(
/// For a given attester slashing, return the indices able to be slashed in ascending order.
///
/// Returns Ok(indices) if `indices.len() > 0`
pub fn get_slashable_indices<'a, E: EthSpec>(
pub fn get_slashable_indices<E: EthSpec>(
state: &BeaconState<E>,
attester_slashing: AttesterSlashingRef<'a, E>,
attester_slashing: AttesterSlashingRef<'_, E>,
) -> Result<Vec<u64>> {
get_slashable_indices_modular(state, attester_slashing, |_, validator| {
validator.is_slashable_at(state.current_epoch())
Expand All @@ -54,9 +54,9 @@ pub fn get_slashable_indices<'a, E: EthSpec>(

/// Same as `gather_attester_slashing_indices` but allows the caller to specify the criteria
/// for determining whether a given validator should be considered slashable.
pub fn get_slashable_indices_modular<'a, F, E: EthSpec>(
pub fn get_slashable_indices_modular<F, E: EthSpec>(
state: &BeaconState<E>,
attester_slashing: AttesterSlashingRef<'a, E>,
attester_slashing: AttesterSlashingRef<'_, E>,
is_slashable: F,
) -> Result<Vec<u64>>
where
Expand Down
Loading

0 comments on commit 3a7b036

Please sign in to comment.