From bb81bacb4e448c36659df16d1ba6bc7dc67f10ed Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 25 Sep 2018 09:29:56 +1000 Subject: [PATCH] Add comments to attestation fns, fix bug --- .../validation/attestation/signatures.rs | 28 +++++++- lighthouse/state/validation/ssz_block.rs | 67 +++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/lighthouse/state/validation/attestation/signatures.rs b/lighthouse/state/validation/attestation/signatures.rs index fa17eba5a..853456401 100644 --- a/lighthouse/state/validation/attestation/signatures.rs +++ b/lighthouse/state/validation/attestation/signatures.rs @@ -18,6 +18,13 @@ pub enum SignatureVerificationError { DBError(String), } +/// Verify an aggregate signature across the supplied message. +/// +/// The public keys used for verification are collected by mapping +/// each true bitfield bit to canonical ValidatorRecord index through +/// the attestation_indicies map. +/// +/// Each public key is loaded from the store on-demand. pub fn verify_aggregate_signature_for_indices( message: &[u8], agg_sig: &AggregateSignature, @@ -33,13 +40,32 @@ pub fn verify_aggregate_signature_for_indices( for i in 0..attestation_indices.len() { let voted = bitfield.get_bit(i); if voted { - let validator = attestation_indices[i]; + /* + * De-reference the attestation index into a canonical ValidatorRecord index. + */ + let validator = attestation_indices.get(i) + .ok_or(SignatureVerificationError::BadValidatorIndex)?; + /* + * Load the validators public key from our store. + */ let pub_key = validator_store.get_public_key_by_index(i)? .ok_or(SignatureVerificationError::NoPublicKeyForValidator)?; + /* + * Add the validators public key to the aggregate public key. + */ agg_pub_key.add(&pub_key); + /* + * Add to the validator to the set of voters for this attestation record. + */ voters.insert(validator); } } + /* + * Verify the aggregate public key against the aggregate signature. + * + * This verification will only succeed if the exact set of public keys + * were added to the aggregate public key as those that signed the aggregate signature. + */ if agg_sig.verify(&message, &agg_pub_key) { Ok(Some(voters)) } else { diff --git a/lighthouse/state/validation/ssz_block.rs b/lighthouse/state/validation/ssz_block.rs index 35c590d42..90c2e2261 100644 --- a/lighthouse/state/validation/ssz_block.rs +++ b/lighthouse/state/validation/ssz_block.rs @@ -46,6 +46,19 @@ pub enum SszBlockValidationError { DatabaseError(String), } +/// Validate some SszBlock. An SszBlock varies from a Block in that is a read-only structure +/// that reads directly from encoded SSZ. +/// +/// The reason to validate an SzzBlock is to avoid decoding it in its entirety if there is +/// a suspicion that the block might be invalid. Such a suspicion should be applied to +/// all blocks coming from the network. +/// +/// Of course, this function will only be more efficient if a block is already serialized. +/// Serializing a complete block and then validating with this function will be less +/// efficient than just validating the original block. +/// +/// This function will determine if the block is new, already known or invalid (either +/// intrinsically or due to some application error.) #[allow(dead_code)] pub fn validate_ssz_block(b: &SszBlock, expected_slot: u64, @@ -61,27 +74,59 @@ pub fn validate_ssz_block(b: &SszBlock, -> Result where T: ClientDB + Sized { + /* + * If this block is already known, return immediately. + */ if block_store.block_exists(&b.block_hash())? { return Ok(BlockStatus::KnownBlock); } + /* + * Copy the block slot (will be used multiple times) + */ let block_slot = b.slot_number(); + + /* + * If the block slot corresponds to a slot in the future (according to the local time), + * drop it. + */ if block_slot > expected_slot { return Err(SszBlockValidationError::FutureSlot); } + /* + * If the PoW chain hash is not known to us, drop it. + * + * We only accept blocks that reference a known PoW hash. + * + * Note: it is not clear what a "known" PoW chain ref is. Likely, + * it means "sufficienty deep in the canonical PoW chain". + */ if pow_store.block_hash_exists(b.pow_chain_ref())? == false { return Err(SszBlockValidationError::UnknownPoWChainRef); } + /* + * Store a reference to the serialized attestations from the block. + */ let attestations_ssz = &b.attestations(); + /* + * Get a slice of the first serialized attestation (the 0th) and decode it into + * a full AttestationRecord object. + */ let (first_attestation_ssz, next_index) = split_one_attestation( &attestations_ssz, 0)?; let (first_attestation, _) = AttestationRecord::ssz_decode( &first_attestation_ssz, 0)?; + /* + * Validate this first attestation. + * + * It is a requirement that the block proposer for this slot + * must have signed the 0th attestation record. + */ let attestation_voters = validate_attestation( &first_attestation, block_slot, @@ -92,19 +137,37 @@ pub fn validate_ssz_block(b: &SszBlock, validator_store.clone(), attester_map.clone())?; + /* + * If the set of voters is None, the attestation was invalid. + */ let attestation_voters = attestation_voters .ok_or(SszBlockValidationError::InvalidAttestation)?; + /* + * Read the proposer from the map of slot -> validator index. + */ let proposer = proposer_map.get(&block_slot) .ok_or(SszBlockValidationError::BadProposerMap)?; + /* + * If the proposer for this slot was not a voter, reject the block. + */ if !attestation_voters.contains(&proposer) { return Err(SszBlockValidationError::NoProposerSignature); } + /* + * Split the remaining attestations into a vector of slices, each containing + * a single serialized attestation record. + */ let other_attestations = split_all_attestations(attestations_ssz, next_index)?; + /* + * Verify each other AttestationRecord. + * + * TODO: make this parallelized. + */ for attestation in other_attestations { let (a, _) = AttestationRecord::ssz_decode(&attestation, 0)?; let attestation_voters = validate_attestation( @@ -121,6 +184,10 @@ pub fn validate_ssz_block(b: &SszBlock, } } + /* + * If we have reached this point, the block is a new valid block that is worthy of + * processing. + */ Ok(BlockStatus::NewBlock) }