From 599948b26b5a430fd971c458dd10af99da7066b7 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 6 Mar 2019 16:24:56 +1100 Subject: [PATCH] Add comments to block_processing code --- eth2/state_processing/src/errors.rs | 148 +++++++++++++++--- .../src/per_block_processing.rs | 20 +++ .../validate_attestation.rs | 74 ++++++--- .../verify_attester_slashing.rs | 8 +- .../per_block_processing/verify_deposit.rs | 16 +- .../src/per_block_processing/verify_exit.rs | 11 +- .../verify_proposer_slashing.rs | 28 +++- .../verify_slashable_attestation.rs | 19 ++- .../per_block_processing/verify_transfer.rs | 15 +- 9 files changed, 258 insertions(+), 81 deletions(-) diff --git a/eth2/state_processing/src/errors.rs b/eth2/state_processing/src/errors.rs index 89cd82b24..6c9719fc2 100644 --- a/eth2/state_processing/src/errors.rs +++ b/eth2/state_processing/src/errors.rs @@ -1,4 +1,4 @@ -use types::BeaconStateError; +use types::*; macro_rules! impl_from_beacon_state_error { ($type: ident) => { @@ -39,23 +39,31 @@ macro_rules! impl_into_with_index_without_beacon_error { }; } +/// A conversion that consumes `self` and adds an `index` variable to resulting struct. +/// +/// Used here to allow converting an error into an upstream error that points to the object that +/// caused the error. For example, pointing to the index of an attestation that caused the +/// `AttestationInvalid` error. pub trait IntoWithIndex: Sized { - fn into_with_index(self, i: usize) -> T; + fn into_with_index(self, index: usize) -> T; } /* * Block Validation */ +/// The object is invalid or validation failed. #[derive(Debug, PartialEq)] pub enum BlockProcessingError { - /// The `BeaconBlock` is invalid. + /// Validation completed successfully and the object is invalid. Invalid(BlockInvalid), + /// Encountered a `BeaconStateError` whilst attempting to determine validity. BeaconStateError(BeaconStateError), } impl_from_beacon_state_error!(BlockProcessingError); +/// Describes why an object is invalid. #[derive(Debug, PartialEq)] pub enum BlockInvalid { StateSlotMismatch, @@ -87,28 +95,61 @@ impl Into for BlockInvalid { * Attestation Validation */ +/// The object is invalid or validation failed. #[derive(Debug, PartialEq)] pub enum AttestationValidationError { - /// The `Attestation` is invalid. + /// Validation completed successfully and the object is invalid. Invalid(AttestationInvalid), /// Encountered a `BeaconStateError` whilst attempting to determine validity. BeaconStateError(BeaconStateError), } +/// Describes why an object is invalid. #[derive(Debug, PartialEq)] pub enum AttestationInvalid { - PreGenesis, - IncludedTooEarly, - IncludedTooLate, - WrongJustifiedSlot, - WrongJustifiedRoot, + /// Attestation references a pre-genesis slot. + /// + /// (genesis_slot, attestation_slot) + PreGenesis(Slot, Slot), + /// Attestation included before the inclusion delay. + /// + /// (state_slot, inclusion_delay, attestation_slot) + IncludedTooEarly(Slot, u64, Slot), + /// Attestation slot is too far in the past to be included in a block. + /// + /// (state_slot, attestation_slot) + IncludedTooLate(Slot, Slot), + /// Attestation justified epoch does not match the states current or previous justified epoch. + /// + /// (attestation_justified_epoch, state_epoch, used_previous_epoch) + WrongJustifiedEpoch(Epoch, Epoch, bool), + /// Attestation justified epoch root does not match root known to the state. + /// + /// (state_justified_root, attestation_justified_root) + WrongJustifiedRoot(Hash256, Hash256), + /// Attestation crosslink root does not match the state crosslink root for the attestations + /// slot. BadLatestCrosslinkRoot, + /// The custody bitfield has some bits set `true`. This is not allowed in phase 0. CustodyBitfieldHasSetBits, + /// There are no set bits on the attestation -- an attestation must be signed by at least one + /// validator. AggregationBitfieldIsEmpty, - BadAggregationBitfieldLength, - BadCustodyBitfieldLength, - NoCommitteeForShard, + /// The custody bitfield length is not the smallest possible size to represent the committee. + /// + /// (committee_len, bitfield_len) + BadCustodyBitfieldLength(usize, usize), + /// The aggregation bitfield length is not the smallest possible size to represent the committee. + /// + /// (committee_len, bitfield_len) + BadAggregationBitfieldLength(usize, usize), + /// There was no known committee for the given shard in the given slot. + /// + /// (attestation_data_shard, attestation_data_slot) + NoCommitteeForShard(u64, Slot), + /// The attestation signature verification failed. BadSignature, + /// The shard block root was not set to zero. This is a phase 0 requirement. ShardBlockRootNotZero, } @@ -119,21 +160,29 @@ impl_into_with_index_with_beacon_error!(AttestationValidationError, AttestationI * `AttesterSlashing` Validation */ +/// The object is invalid or validation failed. #[derive(Debug, PartialEq)] pub enum AttesterSlashingValidationError { - /// The `SlashableAttestation` is invalid. + /// Validation completed successfully and the object is invalid. Invalid(AttesterSlashingInvalid), /// Encountered a `BeaconStateError` whilst attempting to determine validity. BeaconStateError(BeaconStateError), } +/// Describes why an object is invalid. #[derive(Debug, PartialEq)] pub enum AttesterSlashingInvalid { + /// The attestation data is identical, an attestation cannot conflict with itself. AttestationDataIdentical, + /// The attestations were not in conflict. NotSlashable, + /// The first `SlashableAttestation` was invalid. SlashableAttestation1Invalid(SlashableAttestationInvalid), + /// The second `SlashableAttestation` was invalid. SlashableAttestation2Invalid(SlashableAttestationInvalid), - UnknownValidator, + /// The validator index is unknown. One cannot slash one who does not exist. + UnknownValidator(u64), + /// There were no indices able to be slashed. NoSlashableIndices, } @@ -144,19 +193,35 @@ impl_into_with_index_with_beacon_error!(AttesterSlashingValidationError, Atteste * `SlashableAttestation` Validation */ +/// The object is invalid or validation failed. #[derive(Debug, PartialEq)] pub enum SlashableAttestationValidationError { + /// Validation completed successfully and the object is invalid. Invalid(SlashableAttestationInvalid), } +/// Describes why an object is invalid. #[derive(Debug, PartialEq)] pub enum SlashableAttestationInvalid { + /// The custody bitfield has some bits set `true`. This is not allowed in phase 0. CustodyBitfieldHasSetBits, + /// No validator indices were specified. NoValidatorIndices, - BadValidatorIndicesOrdering, - BadCustodyBitfieldLength, - MaxIndicesExceed, - UnknownValidator, + /// The validator indices were not in increasing order. + /// + /// The error occured between the given `index` and `index + 1` + BadValidatorIndicesOrdering(usize), + /// The custody bitfield length is not the smallest possible size to represent the validators. + /// + /// (validators_len, bitfield_len) + BadCustodyBitfieldLength(usize, usize), + /// The number of slashable indices exceed the global maximum. + /// + /// (max_indices, indices_given) + MaxIndicesExceed(usize, usize), + /// The validator index is unknown. One cannot slash one who does not exist. + UnknownValidator(u64), + /// The slashable attestation aggregate signature was not valid. BadSignature, } @@ -172,19 +237,35 @@ impl Into for SlashableAttestationValidationError { * `ProposerSlashing` Validation */ +/// The object is invalid or validation failed. #[derive(Debug, PartialEq)] pub enum ProposerSlashingValidationError { + /// Validation completed successfully and the object is invalid. Invalid(ProposerSlashingInvalid), } +/// Describes why an object is invalid. #[derive(Debug, PartialEq)] pub enum ProposerSlashingInvalid { - ProposerUnknown, - ProposalSlotMismatch, - ProposalShardMismatch, - ProposalBlockRootMismatch, + /// The proposer index is not a known validator. + ProposerUnknown(u64), + /// The two proposal have different slots. + /// + /// (proposal_1_slot, proposal_2_slot) + ProposalSlotMismatch(Slot, Slot), + /// The two proposal have different shards. + /// + /// (proposal_1_shard, proposal_2_shard) + ProposalShardMismatch(u64, u64), + /// The two proposal have different block roots. + /// + /// (proposal_1_root, proposal_2_root) + ProposalBlockRootMismatch(Hash256, Hash256), + /// The specified proposer has already been slashed. ProposerAlreadySlashed, + /// The first proposal signature was invalid. BadProposal1Signature, + /// The second proposal signature was invalid. BadProposal2Signature, } @@ -197,14 +278,20 @@ impl_into_with_index_without_beacon_error!( * `Deposit` Validation */ +/// The object is invalid or validation failed. #[derive(Debug, PartialEq)] pub enum DepositValidationError { + /// Validation completed successfully and the object is invalid. Invalid(DepositInvalid), } +/// Describes why an object is invalid. #[derive(Debug, PartialEq)] pub enum DepositInvalid { - BadIndex, + /// The deposit index does not match the state index. + /// + /// (state_index, deposit_index) + BadIndex(u64, u64), } impl_into_with_index_without_beacon_error!(DepositValidationError, DepositInvalid); @@ -213,16 +300,24 @@ impl_into_with_index_without_beacon_error!(DepositValidationError, DepositInvali * `Exit` Validation */ +/// The object is invalid or validation failed. #[derive(Debug, PartialEq)] pub enum ExitValidationError { + /// Validation completed successfully and the object is invalid. Invalid(ExitInvalid), } +/// Describes why an object is invalid. #[derive(Debug, PartialEq)] pub enum ExitInvalid { - ValidatorUnknown, + /// The specified validator is not in the state's validator registry. + ValidatorUnknown(u64), AlreadyExited, - FutureEpoch, + /// The exit is for a future epoch. + /// + /// (state_epoch, exit_epoch) + FutureEpoch(Epoch, Epoch), + /// The exit signature was not signed by the validator. BadSignature, } @@ -232,11 +327,14 @@ impl_into_with_index_without_beacon_error!(ExitValidationError, ExitInvalid); * `Transfer` Validation */ +/// The object is invalid or validation failed. #[derive(Debug, PartialEq)] pub enum TransferValidationError { + /// Validation completed successfully and the object is invalid. Invalid(TransferInvalid), } +/// Describes why an object is invalid. #[derive(Debug, PartialEq)] pub enum TransferInvalid {} diff --git a/eth2/state_processing/src/per_block_processing.rs b/eth2/state_processing/src/per_block_processing.rs index 06fe3b556..b9006d6f9 100644 --- a/eth2/state_processing/src/per_block_processing.rs +++ b/eth2/state_processing/src/per_block_processing.rs @@ -19,6 +19,12 @@ mod verify_proposer_slashing; mod verify_slashable_attestation; mod verify_transfer; +/// Updates the state for a new block, whilst validating that the block is valid. +/// +/// Returns `Ok(())` if the block is valid and the state was successfully updated. Otherwise +/// returns an error describing why the block was invalid or how the function failed to execute. +/// +/// Spec v0.4.0 pub fn per_block_processing( state: &mut BeaconState, block: &BeaconBlock, @@ -27,6 +33,13 @@ pub fn per_block_processing( per_block_processing_signature_optional(state, block, true, spec) } +/// Updates the state for a new block, whilst validating that the block is valid, without actually +/// checking the block proposer signature. +/// +/// Returns `Ok(())` if the block is valid and the state was successfully updated. Otherwise +/// returns an error describing why the block was invalid or how the function failed to execute. +/// +/// Spec v0.4.0 pub fn per_block_processing_without_verifying_block_signature( state: &mut BeaconState, block: &BeaconBlock, @@ -35,6 +48,13 @@ pub fn per_block_processing_without_verifying_block_signature( per_block_processing_signature_optional(state, block, false, spec) } +/// Updates the state for a new block, whilst validating that the block is valid, optionally +/// checking the block proposer signature. +/// +/// Returns `Ok(())` if the block is valid and the state was successfully updated. Otherwise +/// returns an error describing why the block was invalid or how the function failed to execute. +/// +/// Spec v0.4.0 fn per_block_processing_signature_optional( mut state: &mut BeaconState, block: &BeaconBlock, diff --git a/eth2/state_processing/src/per_block_processing/validate_attestation.rs b/eth2/state_processing/src/per_block_processing/validate_attestation.rs index 8214e8d9a..8749bc5ea 100644 --- a/eth2/state_processing/src/per_block_processing/validate_attestation.rs +++ b/eth2/state_processing/src/per_block_processing/validate_attestation.rs @@ -3,7 +3,10 @@ use ssz::TreeHash; use types::beacon_state::helpers::*; use types::*; -/// Validate an attestation, checking the aggregate signature. +/// Indicates if an `Attestation` is valid to be included in a block in the current epoch of the +/// given state. +/// +/// Returns `Ok(())` if the `Attestation` is valid, otherwise indicates the reason for invalidity. /// /// Spec v0.4.0 pub fn validate_attestation( @@ -14,7 +17,10 @@ pub fn validate_attestation( validate_attestation_signature_optional(state, attestation, spec, true) } -/// Validate an attestation, without checking the aggregate signature. +/// Indicates if an `Attestation` is valid to be included in a block in the current epoch of the +/// given state, without validating the aggregate signature. +/// +/// Returns `Ok(())` if the `Attestation` is valid, otherwise indicates the reason for invalidity. /// /// Spec v0.4.0 pub fn validate_attestation_without_signature( @@ -25,7 +31,9 @@ pub fn validate_attestation_without_signature( validate_attestation_signature_optional(state, attestation, spec, false) } -/// Validate an attestation, optionally checking the aggregate signature. +/// Indicates if an `Attestation` is valid to be included in a block in the current epoch of the +/// given state, optionally validating the aggregate signature. +/// /// /// Spec v0.4.0 fn validate_attestation_signature_optional( @@ -37,19 +45,23 @@ fn validate_attestation_signature_optional( // Verify that `attestation.data.slot >= GENESIS_SLOT`. verify!( attestation.data.slot >= spec.genesis_slot, - Invalid::PreGenesis + Invalid::PreGenesis(spec.genesis_slot, attestation.data.slot) ); // Verify that `attestation.data.slot + MIN_ATTESTATION_INCLUSION_DELAY <= state.slot`. verify!( attestation.data.slot + spec.min_attestation_inclusion_delay <= state.slot, - Invalid::IncludedTooEarly + Invalid::IncludedTooEarly( + state.slot, + spec.min_attestation_inclusion_delay, + attestation.data.slot + ) ); // Verify that `state.slot < attestation.data.slot + SLOTS_PER_EPOCH`. verify!( state.slot < attestation.data.slot + spec.slots_per_epoch, - Invalid::IncludedTooLate + Invalid::IncludedTooLate(state.slot, attestation.data.slot) ); // Verify that `attestation.data.justified_epoch` is equal to `state.justified_epoch` if @@ -58,29 +70,37 @@ fn validate_attestation_signature_optional( if (attestation.data.slot + 1).epoch(spec.slots_per_epoch) >= state.current_epoch(spec) { verify!( attestation.data.justified_epoch == state.justified_epoch, - Invalid::WrongJustifiedSlot + Invalid::WrongJustifiedEpoch( + attestation.data.justified_epoch, + state.justified_epoch, + false + ) ); } else { verify!( attestation.data.justified_epoch == state.previous_justified_epoch, - Invalid::WrongJustifiedSlot + Invalid::WrongJustifiedEpoch( + attestation.data.justified_epoch, + state.previous_justified_epoch, + true + ) ); } // Verify that `attestation.data.justified_block_root` is equal to `get_block_root(state, // get_epoch_start_slot(attestation.data.justified_epoch))`. + let justified_block_root = *state + .get_block_root( + attestation + .data + .justified_epoch + .start_slot(spec.slots_per_epoch), + &spec, + ) + .ok_or(BeaconStateError::InsufficientBlockRoots)?; verify!( - attestation.data.justified_block_root - == *state - .get_block_root( - attestation - .data - .justified_epoch - .start_slot(spec.slots_per_epoch), - &spec - ) - .ok_or(BeaconStateError::InsufficientBlockRoots)?, - Invalid::WrongJustifiedRoot + attestation.data.justified_block_root == justified_block_root, + Invalid::WrongJustifiedRoot(justified_block_root, attestation.data.justified_block_root) ); // Verify that either: @@ -106,7 +126,12 @@ fn validate_attestation_signature_optional( .get_crosslink_committees_at_slot(attestation.data.slot, spec)? .iter() .find(|(_committee, shard)| *shard == attestation.data.shard) - .ok_or_else(|| Error::Invalid(Invalid::NoCommitteeForShard))?; + .ok_or_else(|| { + Error::Invalid(Invalid::NoCommitteeForShard( + attestation.data.shard, + attestation.data.slot, + )) + })?; // Custody bitfield is all zeros (phase 0 requirement). verify!( @@ -115,8 +140,8 @@ fn validate_attestation_signature_optional( ); // Custody bitfield length is correct. verify!( - verify_bitfield_length(&attestation.aggregation_bitfield, committee.len()), - Invalid::BadCustodyBitfieldLength + verify_bitfield_length(&attestation.custody_bitfield, committee.len()), + Invalid::BadCustodyBitfieldLength(committee.len(), attestation.custody_bitfield.len()) ); // Aggregation bitfield isn't empty. verify!( @@ -126,7 +151,10 @@ fn validate_attestation_signature_optional( // Aggregation bitfield length is correct. verify!( verify_bitfield_length(&attestation.aggregation_bitfield, committee.len()), - Invalid::BadAggregationBitfieldLength + Invalid::BadAggregationBitfieldLength( + committee.len(), + attestation.aggregation_bitfield.len() + ) ); if verify_signature { diff --git a/eth2/state_processing/src/per_block_processing/verify_attester_slashing.rs b/eth2/state_processing/src/per_block_processing/verify_attester_slashing.rs index b12ffa5ae..aef09943a 100644 --- a/eth2/state_processing/src/per_block_processing/verify_attester_slashing.rs +++ b/eth2/state_processing/src/per_block_processing/verify_attester_slashing.rs @@ -2,10 +2,10 @@ use super::verify_slashable_attestation::verify_slashable_attestation; use crate::errors::{AttesterSlashingInvalid as Invalid, AttesterSlashingValidationError as Error}; use types::*; -/// Returns `Ok(())` if some `AttesterSlashing` is valid to be included in some `BeaconState`, -/// otherwise returns an `Err`. +/// Indicates if an `AttesterSlashing` is valid to be included in a block in the current epoch of the given +/// state. /// -/// Returns the slashable indices from the `AttesterSlashing`. +/// Returns `Ok(())` if the `AttesterSlashing` is valid, otherwise indicates the reason for invalidity. /// /// Spec v0.4.0 pub fn verify_attester_slashing( @@ -36,7 +36,7 @@ pub fn verify_attester_slashing( let validator = state .validator_registry .get(*i as usize) - .ok_or_else(|| Error::Invalid(Invalid::UnknownValidator))?; + .ok_or_else(|| Error::Invalid(Invalid::UnknownValidator(*i)))?; if slashable_attestation_1.validator_indices.contains(&i) & !validator.slashed { slashable_indices.push(*i); diff --git a/eth2/state_processing/src/per_block_processing/verify_deposit.rs b/eth2/state_processing/src/per_block_processing/verify_deposit.rs index 20ed2f0b2..59a2a66c4 100644 --- a/eth2/state_processing/src/per_block_processing/verify_deposit.rs +++ b/eth2/state_processing/src/per_block_processing/verify_deposit.rs @@ -1,22 +1,26 @@ use crate::errors::{DepositInvalid as Invalid, DepositValidationError as Error}; -use ssz::TreeHash; -use types::beacon_state::helpers::verify_bitfield_length; use types::*; -/// Verify validity of ``slashable_attestation`` fields. +/// Indicates if a `Deposit` is valid to be included in a block in the current epoch of the given +/// state. /// -/// Returns `Ok(())` if all fields are valid. +/// Returns `Ok(())` if the `Deposit` is valid, otherwise indicates the reason for invalidity. +/// +/// Note: this function is incomplete. /// /// Spec v0.4.0 pub fn verify_deposit( state: &BeaconState, deposit: &Deposit, - spec: &ChainSpec, + _spec: &ChainSpec, ) -> Result<(), Error> { // TODO: verify serialized deposit data. // TODO: verify deposit index. - verify!(deposit.index == state.deposit_index, Invalid::BadIndex); + verify!( + deposit.index == state.deposit_index, + Invalid::BadIndex(state.deposit_index, deposit.index) + ); // TODO: verify merkle branch. diff --git a/eth2/state_processing/src/per_block_processing/verify_exit.rs b/eth2/state_processing/src/per_block_processing/verify_exit.rs index d4c2f7baa..34c55c55e 100644 --- a/eth2/state_processing/src/per_block_processing/verify_exit.rs +++ b/eth2/state_processing/src/per_block_processing/verify_exit.rs @@ -2,9 +2,10 @@ use crate::errors::{ExitInvalid as Invalid, ExitValidationError as Error}; use ssz::SignedRoot; use types::*; -/// Verify validity of ``slashable_attestation`` fields. +/// Indicates if an `Exit` is valid to be included in a block in the current epoch of the given +/// state. /// -/// Returns `Ok(())` if all fields are valid. +/// Returns `Ok(())` if the `Exit` is valid, otherwise indicates the reason for invalidity. /// /// Spec v0.4.0 pub fn verify_exit( @@ -15,7 +16,9 @@ pub fn verify_exit( let validator = state .validator_registry .get(exit.validator_index as usize) - .ok_or(Error::Invalid(Invalid::ValidatorUnknown))?; + .ok_or(Error::Invalid(Invalid::ValidatorUnknown( + exit.validator_index, + )))?; verify!( validator.exit_epoch @@ -25,7 +28,7 @@ pub fn verify_exit( verify!( state.current_epoch(spec) >= exit.epoch, - Invalid::FutureEpoch + Invalid::FutureEpoch(state.current_epoch(spec), exit.epoch) ); let message = exit.signed_root(); diff --git a/eth2/state_processing/src/per_block_processing/verify_proposer_slashing.rs b/eth2/state_processing/src/per_block_processing/verify_proposer_slashing.rs index a7b344b2c..c4e51b1a8 100644 --- a/eth2/state_processing/src/per_block_processing/verify_proposer_slashing.rs +++ b/eth2/state_processing/src/per_block_processing/verify_proposer_slashing.rs @@ -2,8 +2,10 @@ use crate::errors::{ProposerSlashingInvalid as Invalid, ProposerSlashingValidati use ssz::SignedRoot; use types::*; -/// Returns `Ok(())` if some `ProposerSlashing` is valid to be included in some `BeaconState`, -/// otherwise returns an `Err`. +/// Indicates if a `ProposerSlashing` is valid to be included in a block in the current epoch of the given +/// state. +/// +/// Returns `Ok(())` if the `ProposerSlashing` is valid, otherwise indicates the reason for invalidity. /// /// Spec v0.4.0 pub fn verify_proposer_slashing( @@ -14,21 +16,32 @@ pub fn verify_proposer_slashing( let proposer = state .validator_registry .get(proposer_slashing.proposer_index as usize) - .ok_or(Error::Invalid(Invalid::ProposerUnknown))?; + .ok_or(Error::Invalid(Invalid::ProposerUnknown( + proposer_slashing.proposer_index, + )))?; verify!( proposer_slashing.proposal_1.slot == proposer_slashing.proposal_2.slot, - Invalid::ProposalSlotMismatch + Invalid::ProposalSlotMismatch( + proposer_slashing.proposal_1.slot, + proposer_slashing.proposal_2.slot + ) ); verify!( proposer_slashing.proposal_1.shard == proposer_slashing.proposal_2.shard, - Invalid::ProposalShardMismatch + Invalid::ProposalShardMismatch( + proposer_slashing.proposal_1.shard, + proposer_slashing.proposal_2.shard + ) ); verify!( proposer_slashing.proposal_1.block_root != proposer_slashing.proposal_2.block_root, - Invalid::ProposalBlockRootMismatch + Invalid::ProposalBlockRootMismatch( + proposer_slashing.proposal_1.block_root, + proposer_slashing.proposal_2.block_root + ) ); verify!(!proposer.slashed, Invalid::ProposerAlreadySlashed); @@ -55,6 +68,9 @@ pub fn verify_proposer_slashing( Ok(()) } +/// Verifies the signature of a proposal. +/// +/// Returns `true` if the signature is valid. fn verify_proposal_signature( proposal: &Proposal, pubkey: &PublicKey, diff --git a/eth2/state_processing/src/per_block_processing/verify_slashable_attestation.rs b/eth2/state_processing/src/per_block_processing/verify_slashable_attestation.rs index cd0f9a201..0b948ad89 100644 --- a/eth2/state_processing/src/per_block_processing/verify_slashable_attestation.rs +++ b/eth2/state_processing/src/per_block_processing/verify_slashable_attestation.rs @@ -5,9 +5,10 @@ use ssz::TreeHash; use types::beacon_state::helpers::verify_bitfield_length; use types::*; -/// Verify validity of ``slashable_attestation`` fields. +/// Indicates if a `SlashableAttestation` is valid to be included in a block in the current epoch of the given +/// state. /// -/// Returns `Ok(())` if all fields are valid. +/// Returns `Ok(())` if the `SlashableAttestation` is valid, otherwise indicates the reason for invalidity. /// /// Spec v0.4.0 pub fn verify_slashable_attestation( @@ -27,7 +28,7 @@ pub fn verify_slashable_attestation( if slashable_attestation.validator_indices[i] >= slashable_attestation.validator_indices[i + 1] { - invalid!(Invalid::BadValidatorIndicesOrdering); + invalid!(Invalid::BadValidatorIndicesOrdering(i)); } } @@ -35,12 +36,18 @@ pub fn verify_slashable_attestation( &slashable_attestation.custody_bitfield, slashable_attestation.validator_indices.len(), ) { - invalid!(Invalid::BadCustodyBitfieldLength); + invalid!(Invalid::BadCustodyBitfieldLength( + slashable_attestation.validator_indices.len(), + slashable_attestation.custody_bitfield.len() + )); } if slashable_attestation.validator_indices.len() > spec.max_indices_per_slashable_vote as usize { - invalid!(Invalid::MaxIndicesExceed); + invalid!(Invalid::MaxIndicesExceed( + spec.max_indices_per_slashable_vote as usize, + slashable_attestation.validator_indices.len() + )); } // TODO: this signature verification could likely be replaced with: @@ -62,7 +69,7 @@ pub fn verify_slashable_attestation( Some(validator) => { aggregate_pubs[custody_bit as usize].add(&validator.pubkey); } - None => invalid!(Invalid::UnknownValidator), + None => invalid!(Invalid::UnknownValidator(*v)), }; } diff --git a/eth2/state_processing/src/per_block_processing/verify_transfer.rs b/eth2/state_processing/src/per_block_processing/verify_transfer.rs index 0dee9a7d2..220f5e496 100644 --- a/eth2/state_processing/src/per_block_processing/verify_transfer.rs +++ b/eth2/state_processing/src/per_block_processing/verify_transfer.rs @@ -1,17 +1,18 @@ use crate::errors::{TransferInvalid as Invalid, TransferValidationError as Error}; -use ssz::TreeHash; -use types::beacon_state::helpers::verify_bitfield_length; use types::*; -/// Verify validity of ``slashable_attestation`` fields. +/// Indicates if a `Transfer` is valid to be included in a block in the current epoch of the given +/// state. /// -/// Returns `Ok(())` if all fields are valid. +/// Returns `Ok(())` if the `Transfer` is valid, otherwise indicates the reason for invalidity. +/// +/// Note: this function is incomplete. /// /// Spec v0.4.0 pub fn verify_transfer( - state: &BeaconState, - transfer: &Transfer, - spec: &ChainSpec, + _state: &BeaconState, + _transfer: &Transfer, + _spec: &ChainSpec, ) -> Result<(), Error> { // TODO: verify transfer.