diff --git a/lighthouse/state/attestation_record/attestation_record.rs b/lighthouse/state/attestation_record/attestation_record.rs index 3fb1c8b0d..9cfa53516 100644 --- a/lighthouse/state/attestation_record/attestation_record.rs +++ b/lighthouse/state/attestation_record/attestation_record.rs @@ -61,7 +61,7 @@ impl Decodable for AttestationRecord { // Do aggregate sig decoding properly. let (agg_sig_bytes, i) = decode_ssz_list(bytes, i)?; let aggregate_sig = AggregateSignature::from_bytes(&agg_sig_bytes) - .map_err(|_| DecodeError::OutOfBounds)?; + .map_err(|_| DecodeError::TooShort)?; // also could be TooLong let attestation_record = Self { slot, diff --git a/lighthouse/state/attestation_record/mod.rs b/lighthouse/state/attestation_record/mod.rs index 74a5d8c34..d96a451fd 100644 --- a/lighthouse/state/attestation_record/mod.rs +++ b/lighthouse/state/attestation_record/mod.rs @@ -11,7 +11,7 @@ pub use self::attestation_record::{ MIN_SSZ_ATTESTION_RECORD_LENGTH, }; pub use self::ssz_splitter::{ - split_all, - split_one, + split_all_attestations, + split_one_attestation, AttestationSplitError, }; diff --git a/lighthouse/state/attestation_record/ssz_splitter.rs b/lighthouse/state/attestation_record/ssz_splitter.rs index 4562b1fb3..72b191cf3 100644 --- a/lighthouse/state/attestation_record/ssz_splitter.rs +++ b/lighthouse/state/attestation_record/ssz_splitter.rs @@ -9,13 +9,13 @@ pub enum AttestationSplitError { /// Given some ssz slice, find the bounds of each serialized AttestationRecord and return a vec of /// slices point to each. -pub fn split_all<'a>(full_ssz: &'a [u8], index: usize) +pub fn split_all_attestations<'a>(full_ssz: &'a [u8], index: usize) -> Result, AttestationSplitError> { let mut v = vec![]; let mut index = index; while index < full_ssz.len() - 1 { - let (slice, i) = split_one(full_ssz, index)?; + let (slice, i) = split_one_attestation(full_ssz, index)?; v.push(slice); index = i; } @@ -24,27 +24,27 @@ pub fn split_all<'a>(full_ssz: &'a [u8], index: usize) /// Given some ssz slice, find the bounds of one serialized AttestationRecord /// and return a slice pointing to that. -pub fn split_one<'a>(full_ssz: &'a [u8], index: usize) +pub fn split_one_attestation<'a>(full_ssz: &'a [u8], index: usize) -> Result<(&'a [u8], usize), AttestationSplitError> { if full_ssz.len() < MIN_LENGTH { return Err(AttestationSplitError::TooShort); } - let hashes_len = decode_length(full_ssz, 10, LENGTH_BYTES) + let hashes_len = decode_length(full_ssz, index + 10, LENGTH_BYTES) .map_err(|_| AttestationSplitError::TooShort)?; let bitfield_len = decode_length( - full_ssz, hashes_len + 46, + full_ssz, index + hashes_len + 46, LENGTH_BYTES) .map_err(|_| AttestationSplitError::TooShort)?; - // Subtract one because the min length assume 1 byte of bitfield - let len = MIN_LENGTH + // Subtract one because the min length assumes 1 byte of bitfield + let len = MIN_LENGTH - 1 + hashes_len - + bitfield_len.saturating_sub(1); + + bitfield_len; - if full_ssz.len() < len { + if index + full_ssz.len() < len { return Err(AttestationSplitError::TooShort); } @@ -102,7 +102,7 @@ mod tests { let mut ssz_stream = SszStream::new(); ssz_stream.append(&a); let ssz = ssz_stream.drain(); - let (a_ssz, i) = split_one(&ssz, 0).unwrap(); + let (a_ssz, i) = split_one_attestation(&ssz, 0).unwrap(); assert_eq!(i, ssz.len()); let (decoded_a, _) = AttestationRecord::ssz_decode(a_ssz, 0) .unwrap(); @@ -115,7 +115,7 @@ mod tests { ssz_stream.append(&a); ssz_stream.append(&b); let ssz = ssz_stream.drain(); - let ssz_vec = split_all(&ssz, 0).unwrap(); + let ssz_vec = split_all_attestations(&ssz, 0).unwrap(); let (decoded_a, _) = AttestationRecord::ssz_decode(ssz_vec[0], 0) .unwrap(); @@ -133,7 +133,7 @@ mod tests { ssz_stream.append(&b); let ssz = ssz_stream.drain(); let ssz = &ssz[0..ssz.len() - 1]; - assert!(split_all(&ssz, 0).is_err()); + assert!(split_all_attestations(&ssz, 0).is_err()); } } diff --git a/lighthouse/state/validation/attestation/attestation_validation.rs b/lighthouse/state/validation/attestation/attestation_validation.rs index db19db0bc..482285125 100644 --- a/lighthouse/state/validation/attestation/attestation_validation.rs +++ b/lighthouse/state/validation/attestation/attestation_validation.rs @@ -1,9 +1,10 @@ use std::collections::HashSet; -use super::AttestationRecord; +use super::attestation_record::AttestationRecord; use super::attestation_parent_hashes::{ attestation_parent_hashes, ParentHashesError, }; +use super::AttesterMap; use super::db::{ ClientDB, DBError @@ -17,7 +18,6 @@ use super::utils::hash::canonical_hash; use super::utils::types::{ Hash256, }; -use std::collections::HashMap; use std::sync::Arc; use super::signatures::{ verify_aggregate_signature_for_indices, @@ -44,10 +44,6 @@ pub enum AttestationValidationError { DBError(String), } -type Slot = u64; -type ShardId = u64; -type AttesterMap = HashMap<(Slot, ShardId), Vec>; - fn bytes_for_bits(bits: usize) -> usize { (bits.saturating_sub(1) / 8) + 1 } @@ -62,12 +58,14 @@ pub fn validate_attestation(a: &AttestationRecord, cycle_length: u8, known_last_justified_slot: u64, known_parent_hashes: Arc>, - block_store: BlockStore, - validator_store: ValidatorStore, + block_store: Arc>, + validator_store: Arc>, attester_map: Arc) - -> Result<(bool, Option>), AttestationValidationError> + -> Result>, AttestationValidationError> where T: ClientDB + Sized { + // TODO: assert attestion isn't already known + /* * The attesation slot must not be higher than the block that contained it. */ @@ -156,7 +154,7 @@ pub fn validate_attestation(a: &AttestationRecord, a.justified_slot) }; - let (signature_valid, voted_hashmap) = + let voted_hashmap = verify_aggregate_signature_for_indices( &signed_message, &a.aggregate_sig, @@ -164,7 +162,7 @@ pub fn validate_attestation(a: &AttestationRecord, &a.attester_bitfield, &validator_store)?; - Ok((signature_valid, voted_hashmap)) + Ok(voted_hashmap) } /// Generates the message used to validate the signature provided with an AttestationRecord. diff --git a/lighthouse/state/validation/attestation/mod.rs b/lighthouse/state/validation/attestation/mod.rs index b4b782a0a..a28f3c6ac 100644 --- a/lighthouse/state/validation/attestation/mod.rs +++ b/lighthouse/state/validation/attestation/mod.rs @@ -1,6 +1,7 @@ +use super::AttesterMap; use super::db; use super::bls; -use super::AttestationRecord; +use super::attestation_record; use super::ssz; use super::attestation_parent_hashes; use super::utils; @@ -8,4 +9,7 @@ use super::utils; mod attestation_validation; mod signatures; -pub use self::attestation_validation::validate_attestation; +pub use self::attestation_validation::{ + validate_attestation, + AttestationValidationError, +}; diff --git a/lighthouse/state/validation/attestation/signatures.rs b/lighthouse/state/validation/attestation/signatures.rs index 302c13f63..fa17eba5a 100644 --- a/lighthouse/state/validation/attestation/signatures.rs +++ b/lighthouse/state/validation/attestation/signatures.rs @@ -24,7 +24,7 @@ pub fn verify_aggregate_signature_for_indices( attestation_indices: &[usize], bitfield: &Bitfield, validator_store: &ValidatorStore) - -> Result<(bool, Option>), SignatureVerificationError> + -> Result>, SignatureVerificationError> where T: ClientDB + Sized { let mut voters = HashSet::new(); @@ -41,9 +41,9 @@ pub fn verify_aggregate_signature_for_indices( } } if agg_sig.verify(&message, &agg_pub_key) { - Ok((true, Some(voters))) + Ok(Some(voters)) } else { - Ok((false, None)) + Ok(None) } } @@ -126,7 +126,7 @@ mod tests { /* * Test using all valid parameters. */ - let (is_valid, voters) = verify_aggregate_signature_for_indices( + let voters = verify_aggregate_signature_for_indices( &message, &agg_sig, &attestation_indices, @@ -134,7 +134,6 @@ mod tests { &store).unwrap(); let voters = voters.unwrap(); - assert_eq!(is_valid, true); (0..signing_keypairs.len()) .for_each(|i| assert!(voters.contains(&i))); (signing_keypairs.len()..non_signing_keypairs.len()) @@ -145,14 +144,13 @@ mod tests { * parameters the same and assert that it fails. */ bitfield.set_bit(signing_keypairs.len() + 1, true); - let (is_valid, voters) = verify_aggregate_signature_for_indices( + let voters = verify_aggregate_signature_for_indices( &message, &agg_sig, &attestation_indices, &bitfield, &store).unwrap(); - assert_eq!(is_valid, false); assert_eq!(voters, None); } } diff --git a/lighthouse/state/validation/mod.rs b/lighthouse/state/validation/mod.rs index b09805d6c..8ad367f4a 100644 --- a/lighthouse/state/validation/mod.rs +++ b/lighthouse/state/validation/mod.rs @@ -1,18 +1,17 @@ -/* -use super::crystallized_state::CrystallizedState; -use super::active_state::ActiveState; -use super::attestation_record::AttestationRecord; -use super::block::Block; -use super::chain_config::ChainConfig; -*/ +use std::collections::HashMap; use super::block; use super::bls; use super::Logger; use super::db; -use super::attestation_record::AttestationRecord; +use super::attestation_record; use super::ssz; use super::transition::attestation_parent_hashes; use super::utils; mod attestation; mod ssz_block; + +type Slot = u64; +type ShardId = u64; +type AttesterMap = HashMap<(Slot, ShardId), Vec>; +type ProposerMap = HashMap; diff --git a/lighthouse/state/validation/ssz_block.rs b/lighthouse/state/validation/ssz_block.rs index c94f25294..35c590d42 100644 --- a/lighthouse/state/validation/ssz_block.rs +++ b/lighthouse/state/validation/ssz_block.rs @@ -1,14 +1,34 @@ +use std::sync::Arc; +use super::attestation::{ + validate_attestation, + AttestationValidationError, +}; +use super::attestation_record::{ + AttestationRecord, + split_one_attestation, + split_all_attestations, + AttestationSplitError, +}; +use super::{ + AttesterMap, + ProposerMap, +}; use super::block::SszBlock; -use super::Logger; use super::db::{ ClientDB, DBError, }; +use super::Logger; use super::db::stores::{ BlockStore, PoWChainStore, ValidatorStore, }; +use super::ssz::{ + Decodable, + DecodeError, +}; +use super::utils::types::Hash256; pub enum BlockStatus { NewBlock, @@ -18,21 +38,25 @@ pub enum BlockStatus { pub enum SszBlockValidationError { FutureSlot, UnknownPoWChainRef, + BadAttestationSsz, + AttestationValidationError(AttestationValidationError), + InvalidAttestation, + NoProposerSignature, + BadProposerMap, DatabaseError(String), } -impl From for SszBlockValidationError { - fn from(e: DBError) -> SszBlockValidationError { - SszBlockValidationError::DatabaseError(e.message) - } -} - #[allow(dead_code)] pub fn validate_ssz_block(b: &SszBlock, expected_slot: u64, - block_store: &BlockStore, pow_store: &PoWChainStore, - _validator_store: &ValidatorStore, + cycle_length: u8, + last_justified_slot: u64, + parent_hashes: Arc>, + proposer_map: Arc, + attester_map: Arc, + block_store: Arc>, + validator_store: Arc>, _log: &Logger) -> Result where T: ClientDB + Sized @@ -41,7 +65,8 @@ pub fn validate_ssz_block(b: &SszBlock, return Ok(BlockStatus::KnownBlock); } - if b.slot_number() > expected_slot { + let block_slot = b.slot_number(); + if block_slot > expected_slot { return Err(SszBlockValidationError::FutureSlot); } @@ -49,6 +74,84 @@ pub fn validate_ssz_block(b: &SszBlock, return Err(SszBlockValidationError::UnknownPoWChainRef); } - // Do validation here + let attestations_ssz = &b.attestations(); + + let (first_attestation_ssz, next_index) = split_one_attestation( + &attestations_ssz, + 0)?; + let (first_attestation, _) = AttestationRecord::ssz_decode( + &first_attestation_ssz, 0)?; + + let attestation_voters = validate_attestation( + &first_attestation, + block_slot, + cycle_length, + last_justified_slot, + parent_hashes.clone(), + block_store.clone(), + validator_store.clone(), + attester_map.clone())?; + + let attestation_voters = attestation_voters + .ok_or(SszBlockValidationError::InvalidAttestation)?; + + let proposer = proposer_map.get(&block_slot) + .ok_or(SszBlockValidationError::BadProposerMap)?; + + if !attestation_voters.contains(&proposer) { + return Err(SszBlockValidationError::NoProposerSignature); + } + + let other_attestations = split_all_attestations(attestations_ssz, + next_index)?; + + for attestation in other_attestations { + let (a, _) = AttestationRecord::ssz_decode(&attestation, 0)?; + let attestation_voters = validate_attestation( + &a, + block_slot, + cycle_length, + last_justified_slot, + parent_hashes.clone(), + block_store.clone(), + validator_store.clone(), + attester_map.clone())?; + if attestation_voters.is_none() { + return Err(SszBlockValidationError::InvalidAttestation); + } + } + Ok(BlockStatus::NewBlock) } + +impl From for SszBlockValidationError { + fn from(e: DBError) -> Self { + SszBlockValidationError::DatabaseError(e.message) + } +} + +impl From for SszBlockValidationError { + fn from(e: AttestationSplitError) -> Self { + match e { + AttestationSplitError::TooShort => + SszBlockValidationError::BadAttestationSsz + } + } +} + +impl From for SszBlockValidationError { + fn from(e: DecodeError) -> Self { + match e { + DecodeError::TooShort => + SszBlockValidationError::BadAttestationSsz, + DecodeError::TooLong => + SszBlockValidationError::BadAttestationSsz, + } + } +} + +impl From for SszBlockValidationError { + fn from(e: AttestationValidationError) -> Self { + SszBlockValidationError::AttestationValidationError(e) + } +} diff --git a/ssz/src/decode.rs b/ssz/src/decode.rs index 9d4132be8..bb51610ee 100644 --- a/ssz/src/decode.rs +++ b/ssz/src/decode.rs @@ -4,7 +4,6 @@ use super::{ #[derive(Debug, PartialEq)] pub enum DecodeError { - OutOfBounds, TooShort, TooLong, } @@ -22,7 +21,7 @@ pub fn decode_ssz(ssz_bytes: &[u8], index: usize) where T: Decodable { if index >= ssz_bytes.len() { - return Err(DecodeError::OutOfBounds) + return Err(DecodeError::TooShort) } T::ssz_decode(ssz_bytes, index) }