From 064e87a347feb3cce8286732ee7c85922ed01500 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Mon, 15 Oct 2018 12:05:36 +1100 Subject: [PATCH 01/13] Add SpecialRecord struct --- beacon_chain/types/src/lib.rs | 2 + beacon_chain/types/src/special_record.rs | 89 ++++++++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 beacon_chain/types/src/special_record.rs diff --git a/beacon_chain/types/src/lib.rs b/beacon_chain/types/src/lib.rs index 4b2d91f9d..c7cc3ac12 100644 --- a/beacon_chain/types/src/lib.rs +++ b/beacon_chain/types/src/lib.rs @@ -10,6 +10,7 @@ pub mod chain_config; pub mod block; pub mod crosslink_record; pub mod shard_and_committee; +pub mod special_record; pub mod validator_record; use self::ethereum_types::{ @@ -27,6 +28,7 @@ pub use chain_config::ChainConfig; pub use block::Block; pub use crosslink_record::CrosslinkRecord; pub use shard_and_committee::ShardAndCommittee; +pub use special_record::{ SpecialRecord, SpecialRecordKind }; pub use validator_record::ValidatorRecord; pub type Hash256 = H256; diff --git a/beacon_chain/types/src/special_record.rs b/beacon_chain/types/src/special_record.rs new file mode 100644 index 000000000..62a5fdc36 --- /dev/null +++ b/beacon_chain/types/src/special_record.rs @@ -0,0 +1,89 @@ +use super::ssz::{ Encodable, SszStream }; + + +/// The value of the "type" field of SpecialRecord. +/// +/// Note: this value must serialize to a u8 and therefore must not be greater than 255. +#[derive(Debug, PartialEq, Clone, Copy)] +pub enum SpecialRecordKind { + Logout = 0, + CasperSlashing = 1, + RandaoChange = 2, +} + + +/// The structure used in the `BeaconBlock.specials` field. +#[derive(Debug, PartialEq, Clone)] +pub struct SpecialRecord { + pub kind: SpecialRecordKind, + pub data: Vec, +} + +impl SpecialRecord { + pub fn logout(data: &[u8]) -> Self { + Self { + kind: SpecialRecordKind::Logout, + data: data.to_vec(), + } + } + + pub fn casper_slashing(data: &[u8]) -> Self { + Self { + kind: SpecialRecordKind::CasperSlashing, + data: data.to_vec(), + } + } + + pub fn randao_change(data: &[u8]) -> Self { + Self { + kind: SpecialRecordKind::RandaoChange, + data: data.to_vec(), + } + } +} + +impl Encodable for SpecialRecord { + fn ssz_append(&self, s: &mut SszStream) { + s.append(&self.kind); + s.append_vec(&self.data); + } +} + +impl Encodable for SpecialRecordKind { + fn ssz_append(&self, s: &mut SszStream) { + s.append(&(*self as u8)); + } +} + + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + pub fn test_special_record_ssz() { + let s = SpecialRecord::logout(&vec![]); + let mut ssz_stream = SszStream::new(); + ssz_stream.append(&s); + let ssz = ssz_stream.drain(); + assert_eq!(ssz, vec![0, 0, 0, 0, 0]); + + let s = SpecialRecord::casper_slashing(&vec![]); + let mut ssz_stream = SszStream::new(); + ssz_stream.append(&s); + let ssz = ssz_stream.drain(); + assert_eq!(ssz, vec![1, 0, 0, 0, 0]); + + let s = SpecialRecord::randao_change(&vec![]); + let mut ssz_stream = SszStream::new(); + ssz_stream.append(&s); + let ssz = ssz_stream.drain(); + assert_eq!(ssz, vec![2, 0, 0, 0, 0]); + + let s = SpecialRecord::randao_change(&vec![42, 43, 44]); + let mut ssz_stream = SszStream::new(); + ssz_stream.append(&s); + let ssz = ssz_stream.drain(); + assert_eq!(ssz, vec![2, 0, 0, 0, 3, 42, 43, 44]); + } +} From 561167fa1ef85a34d34c8026c8ba88411431507b Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Mon, 15 Oct 2018 14:10:43 +1100 Subject: [PATCH 02/13] Fix panic in ssz decode --- beacon_chain/utils/ssz/src/decode.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_chain/utils/ssz/src/decode.rs b/beacon_chain/utils/ssz/src/decode.rs index bb51610ee..1168b0a16 100644 --- a/beacon_chain/utils/ssz/src/decode.rs +++ b/beacon_chain/utils/ssz/src/decode.rs @@ -73,7 +73,7 @@ pub fn decode_ssz_list(ssz_bytes: &[u8], index: usize) pub fn decode_length(bytes: &[u8], index: usize, length_bytes: usize) -> Result { - if bytes.len() < length_bytes { + if bytes.len() < index + length_bytes { return Err(DecodeError::TooShort); }; let mut len: usize = 0; From a862c82b3728e2d993b8eba4c93c1c03f93cb453 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Mon, 15 Oct 2018 14:57:14 +1100 Subject: [PATCH 03/13] Rename Block -> BeaconBlock - Update types::block - Update ssz_helpers::SszBlock - Update db::stores::block_store - Add new fields to types::Block - Update SszBlock as per new Block fields --- beacon_chain/types/src/beacon_block.rs | 105 ++++ beacon_chain/types/src/block.rs | 80 --- beacon_chain/types/src/lib.rs | 4 +- beacon_chain/utils/ssz_helpers/src/lib.rs | 2 +- .../utils/ssz_helpers/src/ssz_beacon_block.rs | 470 ++++++++++++++++++ .../utils/ssz_helpers/src/ssz_block.rs | 358 ------------- .../{block_store.rs => beacon_block_store.rs} | 65 +-- lighthouse/db/src/stores/mod.rs | 8 +- 8 files changed, 616 insertions(+), 476 deletions(-) create mode 100644 beacon_chain/types/src/beacon_block.rs delete mode 100644 beacon_chain/types/src/block.rs create mode 100644 beacon_chain/utils/ssz_helpers/src/ssz_beacon_block.rs delete mode 100644 beacon_chain/utils/ssz_helpers/src/ssz_block.rs rename lighthouse/db/src/stores/{block_store.rs => beacon_block_store.rs} (75%) diff --git a/beacon_chain/types/src/beacon_block.rs b/beacon_chain/types/src/beacon_block.rs new file mode 100644 index 000000000..e75a3e6bb --- /dev/null +++ b/beacon_chain/types/src/beacon_block.rs @@ -0,0 +1,105 @@ +use super::Hash256; +use super::attestation_record::AttestationRecord; +use super::special_record::SpecialRecord; +use super::ssz::{ Encodable, SszStream }; + +pub const MIN_SSZ_BLOCK_LENGTH: usize = { + 8 + // slot + 32 + // randao_reveal + 32 + // pow_chain_reference + 4 + // ancestor hashes (assuming empty) + 32 + // active_state_root + 32 + // crystallized_state_root + 4 + // attestations (assuming empty) + 4 // specials (assuming empty) +}; +pub const MAX_SSZ_BLOCK_LENGTH: usize = MIN_SSZ_BLOCK_LENGTH + (1 << 24); + +#[derive(Debug, PartialEq, Clone)] +pub struct BeaconBlock { + pub slot: u64, + pub randao_reveal: Hash256, + pub pow_chain_reference: Hash256, + pub ancestor_hashes: Vec, + pub active_state_root: Hash256, + pub crystallized_state_root: Hash256, + pub attestations: Vec, + pub specials: Vec, +} + +impl BeaconBlock { + pub fn zero() -> Self { + Self { + slot: 0, + randao_reveal: Hash256::zero(), + pow_chain_reference: Hash256::zero(), + ancestor_hashes: vec![], + active_state_root: Hash256::zero(), + crystallized_state_root: Hash256::zero(), + attestations: vec![], + specials: vec![], + } + } + + /// Return a reference to `ancestor_hashes[0]`. + /// + /// The first hash in `ancestor_hashes` is the parent of the block. + pub fn parent_hash(&self) -> Option<&Hash256> { + self.ancestor_hashes.get(0) + } +} + +impl Encodable for BeaconBlock { + fn ssz_append(&self, s: &mut SszStream) { + s.append(&self.slot); + s.append(&self.randao_reveal); + s.append(&self.pow_chain_reference); + s.append_vec(&self.ancestor_hashes.to_vec()); + s.append(&self.active_state_root); + s.append(&self.crystallized_state_root); + s.append_vec(&self.attestations); + s.append_vec(&self.specials); + } +} + + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_block_zero() { + let b = BeaconBlock::zero(); + assert_eq!(b.slot, 0); + assert!(b.randao_reveal.is_zero()); + assert!(b.pow_chain_reference.is_zero()); + assert_eq!(b.ancestor_hashes, vec![Hash256::zero()]); + assert!(b.active_state_root.is_zero()); + assert!(b.crystallized_state_root.is_zero()); + assert_eq!(b.attestations.len(), 0); + assert_eq!(b.specials.len(), 0); + } + + #[test] + pub fn test_block_min_ssz_length() { + let b = BeaconBlock::zero(); + + let mut ssz_stream = SszStream::new(); + ssz_stream.append(&b); + let ssz = ssz_stream.drain(); + + assert_eq!(ssz.len(), MIN_SSZ_BLOCK_LENGTH); + } + + #[test] + pub fn test_block_parent_hash() { + let mut b = BeaconBlock::zero(); + b.ancestor_hashes = vec![ + Hash256::from("cats".as_bytes()), + Hash256::from("dogs".as_bytes()), + Hash256::from("birds".as_bytes()), + ]; + + assert_eq!(b.parent_hash().unwrap(), &Hash256::from("cats".as_bytes())); + } +} diff --git a/beacon_chain/types/src/block.rs b/beacon_chain/types/src/block.rs deleted file mode 100644 index ee84773d0..000000000 --- a/beacon_chain/types/src/block.rs +++ /dev/null @@ -1,80 +0,0 @@ -use super::Hash256; -use super::attestation_record::AttestationRecord; -use super::ssz::{ Encodable, SszStream }; - -pub const MIN_SSZ_BLOCK_LENGTH: usize = { - 32 + // parent_hash - 8 + // slot_number - 32 + // randao_reveal - 4 + // attestations (assuming zero) - 32 + // pow_chain_ref - 32 + // active_state_root - 32 // crystallized_state_root -}; -pub const MAX_SSZ_BLOCK_LENGTH: usize = MIN_SSZ_BLOCK_LENGTH + (1 << 24); - -#[derive(Debug, PartialEq, Clone)] -pub struct Block { - pub parent_hash: Hash256, - pub slot_number: u64, - pub randao_reveal: Hash256, - pub attestations: Vec, - pub pow_chain_ref: Hash256, - pub active_state_root: Hash256, - pub crystallized_state_root: Hash256, -} - -impl Block { - pub fn zero() -> Self { - Self { - parent_hash: Hash256::zero(), - slot_number: 0, - randao_reveal: Hash256::zero(), - attestations: vec![], - pow_chain_ref: Hash256::zero(), - active_state_root: Hash256::zero(), - crystallized_state_root: Hash256::zero(), - } - } -} - -impl Encodable for Block { - fn ssz_append(&self, s: &mut SszStream) { - s.append(&self.parent_hash); - s.append(&self.slot_number); - s.append(&self.randao_reveal); - s.append_vec(&self.attestations); - s.append(&self.pow_chain_ref); - s.append(&self.active_state_root); - s.append(&self.crystallized_state_root); - } -} - - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_block_zero() { - let b = Block::zero(); - assert!(b.parent_hash.is_zero()); - assert_eq!(b.slot_number, 0); - assert!(b.randao_reveal.is_zero()); - assert_eq!(b.attestations.len(), 0); - assert!(b.pow_chain_ref.is_zero()); - assert!(b.active_state_root.is_zero()); - assert!(b.crystallized_state_root.is_zero()); - } - - #[test] - pub fn test_block_min_ssz_length() { - let b = Block::zero(); - - let mut ssz_stream = SszStream::new(); - ssz_stream.append(&b); - let ssz = ssz_stream.drain(); - - assert_eq!(ssz.len(), MIN_SSZ_BLOCK_LENGTH); - } -} diff --git a/beacon_chain/types/src/lib.rs b/beacon_chain/types/src/lib.rs index c7cc3ac12..14dfa792a 100644 --- a/beacon_chain/types/src/lib.rs +++ b/beacon_chain/types/src/lib.rs @@ -7,7 +7,7 @@ pub mod active_state; pub mod attestation_record; pub mod crystallized_state; pub mod chain_config; -pub mod block; +pub mod beacon_block; pub mod crosslink_record; pub mod shard_and_committee; pub mod special_record; @@ -25,7 +25,7 @@ pub use active_state::ActiveState; pub use attestation_record::AttestationRecord; pub use crystallized_state::CrystallizedState; pub use chain_config::ChainConfig; -pub use block::Block; +pub use beacon_block::BeaconBlock; pub use crosslink_record::CrosslinkRecord; pub use shard_and_committee::ShardAndCommittee; pub use special_record::{ SpecialRecord, SpecialRecordKind }; diff --git a/beacon_chain/utils/ssz_helpers/src/lib.rs b/beacon_chain/utils/ssz_helpers/src/lib.rs index 6f313e8a0..64ee3b1b9 100644 --- a/beacon_chain/utils/ssz_helpers/src/lib.rs +++ b/beacon_chain/utils/ssz_helpers/src/lib.rs @@ -4,4 +4,4 @@ extern crate types; extern crate ssz; pub mod attestation_ssz_splitter; -pub mod ssz_block; +pub mod ssz_beacon_block; diff --git a/beacon_chain/utils/ssz_helpers/src/ssz_beacon_block.rs b/beacon_chain/utils/ssz_helpers/src/ssz_beacon_block.rs new file mode 100644 index 000000000..b46b1f2db --- /dev/null +++ b/beacon_chain/utils/ssz_helpers/src/ssz_beacon_block.rs @@ -0,0 +1,470 @@ +use super::ssz::decode::{ + decode_length, + Decodable, +}; +use super::hashing::canonical_hash; +use super::types::beacon_block::{ + MIN_SSZ_BLOCK_LENGTH, + MAX_SSZ_BLOCK_LENGTH, +}; +use super::types::attestation_record::MIN_SSZ_ATTESTION_RECORD_LENGTH; + +#[derive(Debug, PartialEq)] +pub enum SszBeaconBlockError { + TooShort, + TooLong, +} + +const LENGTH_BYTES: usize = 4; + +/// Allows for reading of block values directly from serialized ssz bytes. +/// +/// The purpose of this struct is to provide the functionality to read block fields directly from +/// some serialized SSZ slice allowing us to read the block without fully +/// de-serializing it. +/// +/// This struct should be as "zero-copy" as possible. The `ssz` field is a reference to some slice +/// and each function reads from that slice. +/// +/// Use this to perform intial checks before we fully de-serialize a block. It should only really +/// be used to verify blocks that come in from the network, for internal operations we should use a +/// full `BeaconBlock`. +#[derive(Debug, PartialEq)] +pub struct SszBeaconBlock<'a> { + ssz: &'a [u8], + // Ancestors + ancestors_position: usize, + ancestors_len: usize, + // Attestations + attestations_position: usize, + attestations_len: usize, + // Specials + specials_position: usize, + specials_len: usize, +} + +impl<'a> SszBeaconBlock<'a> { + /// Create a new instance from a slice reference. + /// + /// This function will validate the length of the ssz string, however it will not validate the + /// contents. + /// + /// The returned `SszBeaconBlock` instance will contain a `len` field which can be used to determine + /// how many bytes were read from the slice. In the case of multiple, sequentually serialized + /// blocks `len` can be used to assume the location of the next serialized block. + pub fn from_slice(vec: &'a [u8]) + -> Result + { + let untrimmed_ssz = &vec[..]; + + /* + * Ensure the SSZ is long enough to be a block + */ + if vec.len() < MIN_SSZ_BLOCK_LENGTH { + return Err(SszBeaconBlockError::TooShort); + } + + /* + * Ensure the SSZ slice isn't longer than is possible for a block. + */ + if vec.len() > MAX_SSZ_BLOCK_LENGTH { + return Err(SszBeaconBlockError::TooLong); + } + + /* + * Determine how many bytes are used to store ancestor hashes. + */ + let ancestors_position = + 8 + // slot + 32 + // randao_reveal + 32; // pow_chain_reference + let ancestors_len = decode_length(untrimmed_ssz, ancestors_position, LENGTH_BYTES) + .map_err(|_| SszBeaconBlockError::TooShort)?; + + /* + * Determine how many bytes are used to store attestation records. + */ + let attestations_position = + ancestors_position + LENGTH_BYTES + ancestors_len + // end of ancestor bytes + 32 + // active_state_root + 32; // crystallized_state_root + let attestations_len = decode_length(untrimmed_ssz, attestations_position, LENGTH_BYTES) + .map_err(|_| SszBeaconBlockError::TooShort)?; + + /* + * Determine how many bytes are used to store specials. + */ + let specials_position = + attestations_position + // location of attestations + LENGTH_BYTES + // attestations length prefix + attestations_len; // # of attestation bytes + let specials_len = decode_length(untrimmed_ssz, specials_position, LENGTH_BYTES) + .map_err(|_| SszBeaconBlockError::TooShort)?; + + /* + * Now that all variable field lengths are known (ancestors, attestations, specials) we can + * know the exact length of the block and reject it if the slice is too short. + */ + let block_ssz_len = { + MIN_SSZ_BLOCK_LENGTH + ancestors_len + attestations_len + specials_len + }; + if vec.len() < block_ssz_len { + println!("block_ssz_len: {:?}, len: {:?}", block_ssz_len, vec.len()); + return Err(SszBeaconBlockError::TooShort); + } + + Ok(Self{ + ssz: &untrimmed_ssz[0..block_ssz_len], + ancestors_position, + ancestors_len, + attestations_position, + attestations_len, + specials_position, + specials_len, + }) + } + + pub fn len(&self) -> usize { self.ssz.len() } + + /// Return the canonical hash for this block. + pub fn block_hash(&self) -> Vec { + canonical_hash(self.ssz) + } + + /// Return the bytes representing `ancestor_hashes[0]`. + /// + /// The first hash in `ancestor_hashes` is the parent of the block. + pub fn parent_hash(&self) -> Option<&[u8]> { + let ancestor_ssz = self.ancestor_hashes(); + if ancestor_ssz.len() >= 32 { + Some(&ancestor_ssz[0..32]) + } else { + None + } + } + + /// Return the `slot` field. + pub fn slot(&self) -> u64 { + /* + * An error should be unreachable from this decode + * because we checked the length of the array at + * the initalization of this struct. + * + * If you can make this function panic, please report + * it to paul@sigmaprime.io + */ + if let Ok((n, _)) = u64::ssz_decode(&self.ssz, 0) { + n + } else { + unreachable!(); + } + } + + /// Return the `randao_reveal` field. + pub fn randao_reveal(&self) -> &[u8] { + let start = 8; // slot is 8 bytes + &self.ssz[start..start + 32] + } + + /// Return the `pow_chain_reference` field. + pub fn pow_chain_reference(&self) -> &[u8] { + let start = + 8 + // slot + 32; // randao_reveal + &self.ssz[start..start + 32] + } + + /// Return the serialized `ancestor_hashes` bytes. + pub fn ancestor_hashes(&self) -> &[u8] { + let start = self.ancestors_position + LENGTH_BYTES; + &self.ssz[start..start + self.ancestors_len] + } + + /// Return the `active_state_root` field. + pub fn act_state_root(&self) -> &[u8] { + let start = self.ancestors_position + LENGTH_BYTES + self.ancestors_len; + &self.ssz[start..(start + 32)] + } + + /// Return the `active_state_root` field. + pub fn cry_state_root(&self) -> &[u8] { + let start = + self.ancestors_position + LENGTH_BYTES + self.ancestors_len + // ancestors + 32; // active_state_root; + &self.ssz[start..(start + 32)] + } + + /// Return the serialized `attestations` bytes. + pub fn attestations(&self) -> &[u8] { + let start = self.attestations_position + LENGTH_BYTES; + &self.ssz[start..(start + self.attestations_len)] + } + + /// Return the serialized `specials` bytes. + pub fn specials(&self) -> &[u8] { + let start = self.specials_position + LENGTH_BYTES; + &self.ssz[start..(start + self.specials_len)] + } +} + + +#[cfg(test)] +mod tests { + use super::*; + use super::super::types::{ + AttestationRecord, + BeaconBlock, + SpecialRecord, + }; + use super::super::ssz::SszStream; + use super::super::types::Hash256; + + fn get_block_ssz(b: &BeaconBlock) -> Vec { + let mut ssz_stream = SszStream::new(); + ssz_stream.append(b); + ssz_stream.drain() + } + + fn get_special_record_ssz(sr: &SpecialRecord) -> Vec { + let mut ssz_stream = SszStream::new(); + ssz_stream.append(sr); + ssz_stream.drain() + } + + fn get_attestation_record_ssz(ar: &AttestationRecord) -> Vec { + let mut ssz_stream = SszStream::new(); + ssz_stream.append(ar); + ssz_stream.drain() + } + + #[test] + fn test_ssz_block_zero_attestation_records() { + let mut b = BeaconBlock::zero(); + b.attestations = vec![]; + let ssz = get_block_ssz(&b); + + + assert!(SszBeaconBlock::from_slice(&ssz[..]).is_ok()); + } + + #[test] + fn test_ssz_block_single_attestation_record_one_byte_short() { + let mut b = BeaconBlock::zero(); + b.attestations = vec![AttestationRecord::zero()]; + let ssz = get_block_ssz(&b); + + assert_eq!( + SszBeaconBlock::from_slice(&ssz[0..(ssz.len() - 1)]), + Err(SszBeaconBlockError::TooShort) + ); + } + + #[test] + fn test_ssz_block_single_attestation_record_one_byte_long() { + let mut b = BeaconBlock::zero(); + b.attestations = vec![AttestationRecord::zero()]; + let mut ssz = get_block_ssz(&b); + let original_len = ssz.len(); + ssz.push(42); + + let ssz_block = SszBeaconBlock::from_slice(&ssz[..]).unwrap(); + + assert_eq!(ssz_block.len(), original_len); + } + + #[test] + fn test_ssz_block_single_attestation_record() { + let mut b = BeaconBlock::zero(); + b.attestations = vec![AttestationRecord::zero()]; + let ssz = get_block_ssz(&b); + + assert!(SszBeaconBlock::from_slice(&ssz[..]).is_ok()); + } + + #[test] + fn test_ssz_block_attestations_length() { + let mut block = BeaconBlock::zero(); + block.attestations.push(AttestationRecord::zero()); + + let serialized = get_block_ssz(&block); + let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); + + assert_eq!(ssz_block.attestations_len, MIN_SSZ_ATTESTION_RECORD_LENGTH); + } + + #[test] + fn test_ssz_block_block_hash() { + let mut block = BeaconBlock::zero(); + block.attestations.push(AttestationRecord::zero()); + let serialized = get_block_ssz(&block); + let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); + let hash = ssz_block.block_hash(); + // Note: this hash was not generated by some external program, + // it was simply printed then copied into the code. This test + // will tell us if the hash changes, not that it matches some + // canonical reference. + let expected_hash = [ + 11, 181, 149, 114, 248, 15, 46, 0, 106, 135, 158, 31, 15, 194, 149, 176, + 43, 110, 154, 26, 253, 67, 18, 139, 250, 84, 144, 219, 3, 208, 50, 145 + ]; + assert_eq!(hash, expected_hash); + + /* + * Test if you give the SszBeaconBlock too many ssz bytes + */ + let mut too_long = serialized.clone(); + too_long.push(42); + let ssz_block = SszBeaconBlock::from_slice(&too_long).unwrap(); + let hash = ssz_block.block_hash(); + assert_eq!(hash, expected_hash); + } + + #[test] + fn test_ssz_block_slot() { + let mut block = BeaconBlock::zero(); + block.attestations.push(AttestationRecord::zero()); + block.slot = 42; + + let serialized = get_block_ssz(&block); + let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); + + assert_eq!(ssz_block.slot(), 42); + } + + #[test] + fn test_ssz_block_randao_reveal() { + let mut block = BeaconBlock::zero(); + block.attestations.push(AttestationRecord::zero()); + let reference_hash = Hash256::from([42_u8; 32]); + block.randao_reveal = reference_hash.clone(); + + let serialized = get_block_ssz(&block); + let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); + + assert_eq!(ssz_block.randao_reveal(), &reference_hash.to_vec()[..]); + } + + #[test] + fn test_ssz_block_ancestor_hashes() { + let mut block = BeaconBlock::zero(); + let h = Hash256::from(&vec![42_u8; 32][..]); + block.ancestor_hashes.push(h); + + let serialized = get_block_ssz(&block); + let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); + + assert_eq!(ssz_block.ancestor_hashes(), &h.to_vec()[..]); + } + + #[test] + fn test_ssz_block_parent_hash() { + let mut block = BeaconBlock::zero(); + block.ancestor_hashes = vec![ + Hash256::from("cats".as_bytes()), + Hash256::from("dogs".as_bytes()), + Hash256::from("birds".as_bytes()), + ]; + + let serialized = get_block_ssz(&block); + let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); + + assert_eq!(ssz_block.parent_hash().unwrap(), &Hash256::from("cats".as_bytes()).to_vec()[..]); + } + + #[test] + fn test_ssz_block_specials() { + /* + * Without data + */ + let mut block = BeaconBlock::zero(); + let s = SpecialRecord::logout(&[]); + block.specials.push(s.clone()); + + let serialized = get_block_ssz(&block); + let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); + let sr_ssz = get_special_record_ssz(&s); + + assert_eq!(ssz_block.specials(), &sr_ssz[..]); + + /* + * With data + */ + let mut block = BeaconBlock::zero(); + let s = SpecialRecord::randao_change(&[16, 17, 18]); + block.specials.push(s.clone()); + + let serialized = get_block_ssz(&block); + let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); + let sr_ssz = get_special_record_ssz(&s); + + assert_eq!(ssz_block.specials(), &sr_ssz[..]); + } + + #[test] + fn test_ssz_block_attestations() { + /* + * Single AttestationRecord + */ + let mut block = BeaconBlock::zero(); + block.attestations.push(AttestationRecord::zero()); + + let serialized = get_block_ssz(&block); + let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); + let ssz_ar = get_attestation_record_ssz(&AttestationRecord::zero()); + + assert_eq!(ssz_block.attestations(), &ssz_ar[..]); + + /* + * Multiple AttestationRecords + */ + let mut block = BeaconBlock::zero(); + block.attestations.push(AttestationRecord::zero()); + block.attestations.push(AttestationRecord::zero()); + + let serialized = get_block_ssz(&block); + let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); + let mut ssz_ar = get_attestation_record_ssz(&AttestationRecord::zero()); + ssz_ar.append(&mut get_attestation_record_ssz(&AttestationRecord::zero())); + + assert_eq!(ssz_block.attestations(), &ssz_ar[..]); + } + + #[test] + fn test_ssz_block_pow_chain_reference() { + let mut block = BeaconBlock::zero(); + block.attestations.push(AttestationRecord::zero()); + let reference_hash = Hash256::from([42_u8; 32]); + block.pow_chain_reference = reference_hash.clone(); + + let serialized = get_block_ssz(&block); + let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); + + assert_eq!(ssz_block.pow_chain_reference(), &reference_hash.to_vec()[..]); + } + + #[test] + fn test_ssz_block_act_state_root() { + let mut block = BeaconBlock::zero(); + block.attestations.push(AttestationRecord::zero()); + let reference_hash = Hash256::from([42_u8; 32]); + block.active_state_root = reference_hash.clone(); + + let serialized = get_block_ssz(&block); + let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); + + assert_eq!(ssz_block.act_state_root(), &reference_hash.to_vec()[..]); + } + + #[test] + fn test_ssz_block_cry_state_root() { + let mut block = BeaconBlock::zero(); + block.attestations.push(AttestationRecord::zero()); + let reference_hash = Hash256::from([42_u8; 32]); + block.crystallized_state_root = reference_hash.clone(); + + let serialized = get_block_ssz(&block); + let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); + + assert_eq!(ssz_block.cry_state_root(), &reference_hash.to_vec()[..]); + } +} diff --git a/beacon_chain/utils/ssz_helpers/src/ssz_block.rs b/beacon_chain/utils/ssz_helpers/src/ssz_block.rs deleted file mode 100644 index 7dd2ddc6d..000000000 --- a/beacon_chain/utils/ssz_helpers/src/ssz_block.rs +++ /dev/null @@ -1,358 +0,0 @@ -use super::ssz::decode::{ - decode_length, - Decodable, -}; -use super::hashing::canonical_hash; -use super::types::block::{ - MIN_SSZ_BLOCK_LENGTH, - MAX_SSZ_BLOCK_LENGTH, -}; -use super::types::attestation_record::MIN_SSZ_ATTESTION_RECORD_LENGTH; - -#[derive(Debug, PartialEq)] -pub enum SszBlockError { - TooShort, - TooLong, -} - -const LENGTH_BYTES: usize = 4; - -/// Allows for reading of block values directly from serialized ssz bytes. -/// -/// The purpose of this struct is to provide the functionality to read block fields directly from -/// some serialized SSZ slice allowing us to read the block without fully -/// de-serializing it. -/// -/// This struct should be as "zero-copy" as possible. The `ssz` field is a reference to some slice -/// and each function reads from that slice. -/// -/// Use this to perform intial checks before we fully de-serialize a block. It should only really -/// be used to verify blocks that come in from the network, for internal operations we should use a -/// full `Block`. -#[derive(Debug, PartialEq)] -pub struct SszBlock<'a> { - ssz: &'a [u8], - attestation_len: usize, - pub len: usize, -} - -impl<'a> SszBlock<'a> { - /// Create a new instance from a slice reference. - /// - /// This function will validate the length of the ssz string, however it will not validate the - /// contents. - /// - /// The returned `SszBlock` instance will contain a `len` field which can be used to determine - /// how many bytes were read from the slice. In the case of multiple, sequentually serialized - /// blocks `len` can be used to assume the location of the next serialized block. - pub fn from_slice(vec: &'a [u8]) - -> Result - { - let untrimmed_ssz = &vec[..]; - /* - * Ensure the SSZ is long enough to be a block with - * one attestation record (not necessarily a valid - * attestation record). - */ - if vec.len() < MIN_SSZ_BLOCK_LENGTH + MIN_SSZ_ATTESTION_RECORD_LENGTH { - return Err(SszBlockError::TooShort); - } - /* - * Ensure the SSZ slice isn't longer than is possible for a block. - */ - if vec.len() > MAX_SSZ_BLOCK_LENGTH { - return Err(SszBlockError::TooLong); - } - /* - * Determine how many bytes are used to store attestation records. - */ - let attestation_len = decode_length(untrimmed_ssz, 72, LENGTH_BYTES) - .map_err(|_| SszBlockError::TooShort)?; - /* - * The block only has one variable field, `attestations`, therefore - * the size of the block must be the minimum size, plus the length - * of the attestations. - */ - let block_ssz_len = { - MIN_SSZ_BLOCK_LENGTH + attestation_len - }; - if vec.len() < block_ssz_len { - return Err(SszBlockError::TooShort); - } - Ok(Self{ - ssz: &untrimmed_ssz[0..block_ssz_len], - attestation_len, - len: block_ssz_len, - }) - } - - /// Return the canonical hash for this block. - pub fn block_hash(&self) -> Vec { - canonical_hash(self.ssz) - } - - /// Return the `parent_hash` field. - pub fn parent_hash(&self) -> &[u8] { - &self.ssz[0..32] - } - - /// Return the `slot_number` field. - pub fn slot_number(&self) -> u64 { - /* - * An error should be unreachable from this decode - * because we checked the length of the array at - * the initalization of this struct. - * - * If you can make this function panic, please report - * it to paul@sigmaprime.io - */ - if let Ok((n, _)) = u64::ssz_decode(&self.ssz, 32) { - n - } else { - unreachable!(); - } - } - - /// Return the `randao_reveal` field. - pub fn randao_reveal(&self) -> &[u8] { - &self.ssz[40..72] - } - - /// Return the `attestations` field. - pub fn attestations(&self) -> &[u8] { - let start = 72 + LENGTH_BYTES; - &self.ssz[start..(start + self.attestation_len)] - } - - /// Return the `pow_chain_ref` field. - pub fn pow_chain_ref(&self) -> &[u8] { - let start = self.len - (32 * 3); - &self.ssz[start..(start + 32)] - } - - /// Return the `active_state_root` field. - pub fn act_state_root(&self) -> &[u8] { - let start = self.len - (32 * 2); - &self.ssz[start..(start + 32)] - } - - /// Return the `active_state_root` field. - pub fn cry_state_root(&self) -> &[u8] { - let start = self.len - 32; - &self.ssz[start..(start + 32)] - } -} - -#[cfg(test)] -mod tests { - use super::*; - use super::super::types::{ - AttestationRecord, - Block, - }; - use super::super::ssz::SszStream; - use super::super::types::Hash256; - - fn get_block_ssz(b: &Block) -> Vec { - let mut ssz_stream = SszStream::new(); - ssz_stream.append(b); - ssz_stream.drain() - } - - fn get_attestation_record_ssz(ar: &AttestationRecord) -> Vec { - let mut ssz_stream = SszStream::new(); - ssz_stream.append(ar); - ssz_stream.drain() - } - - #[test] - fn test_ssz_block_zero_attestation_records() { - let mut b = Block::zero(); - b.attestations = vec![]; - let ssz = get_block_ssz(&b); - - assert_eq!( - SszBlock::from_slice(&ssz[..]), - Err(SszBlockError::TooShort) - ); - } - - #[test] - fn test_ssz_block_single_attestation_record_one_byte_short() { - let mut b = Block::zero(); - b.attestations = vec![AttestationRecord::zero()]; - let ssz = get_block_ssz(&b); - - assert_eq!( - SszBlock::from_slice(&ssz[0..(ssz.len() - 1)]), - Err(SszBlockError::TooShort) - ); - } - - #[test] - fn test_ssz_block_single_attestation_record_one_byte_long() { - let mut b = Block::zero(); - b.attestations = vec![AttestationRecord::zero()]; - let mut ssz = get_block_ssz(&b); - let original_len = ssz.len(); - ssz.push(42); - - let ssz_block = SszBlock::from_slice(&ssz[..]).unwrap(); - - assert_eq!(ssz_block.len, original_len); - } - - #[test] - fn test_ssz_block_single_attestation_record() { - let mut b = Block::zero(); - b.attestations = vec![AttestationRecord::zero()]; - let ssz = get_block_ssz(&b); - - assert!(SszBlock::from_slice(&ssz[..]).is_ok()); - } - - #[test] - fn test_ssz_block_attestation_length() { - let mut block = Block::zero(); - block.attestations.push(AttestationRecord::zero()); - - let serialized = get_block_ssz(&block); - let ssz_block = SszBlock::from_slice(&serialized).unwrap(); - - assert_eq!(ssz_block.attestation_len, MIN_SSZ_ATTESTION_RECORD_LENGTH); - } - - #[test] - fn test_ssz_block_block_hash() { - let mut block = Block::zero(); - block.attestations.push(AttestationRecord::zero()); - let serialized = get_block_ssz(&block); - let ssz_block = SszBlock::from_slice(&serialized).unwrap(); - let hash = ssz_block.block_hash(); - // Note: this hash was not generated by some external program, - // it was simply printed then copied into the code. This test - // will tell us if the hash changes, not that it matches some - // canonical reference. - let expected_hash = [ - 64, 176, 117, 210, 228, 229, 237, 100, 66, 66, 98, - 252, 31, 111, 218, 27, 160, 57, 164, 12, 15, 164, - 66, 102, 142, 36, 2, 196, 121, 54, 242, 3 - ]; - assert_eq!(hash, expected_hash); - - /* - * Test if you give the SszBlock too many ssz bytes - */ - let mut too_long = serialized.clone(); - too_long.push(42); - let ssz_block = SszBlock::from_slice(&too_long).unwrap(); - let hash = ssz_block.block_hash(); - assert_eq!(hash, expected_hash); - } - - #[test] - fn test_ssz_block_parent_hash() { - let mut block = Block::zero(); - block.attestations.push(AttestationRecord::zero()); - let reference_hash = Hash256::from([42_u8; 32]); - block.parent_hash = reference_hash.clone(); - - let serialized = get_block_ssz(&block); - let ssz_block = SszBlock::from_slice(&serialized).unwrap(); - - assert_eq!(ssz_block.parent_hash(), &reference_hash.to_vec()[..]); - } - - #[test] - fn test_ssz_block_slot_number() { - let mut block = Block::zero(); - block.attestations.push(AttestationRecord::zero()); - block.slot_number = 42; - - let serialized = get_block_ssz(&block); - let ssz_block = SszBlock::from_slice(&serialized).unwrap(); - - assert_eq!(ssz_block.slot_number(), 42); - } - - #[test] - fn test_ssz_block_randao_reveal() { - let mut block = Block::zero(); - block.attestations.push(AttestationRecord::zero()); - let reference_hash = Hash256::from([42_u8; 32]); - block.randao_reveal = reference_hash.clone(); - - let serialized = get_block_ssz(&block); - let ssz_block = SszBlock::from_slice(&serialized).unwrap(); - - assert_eq!(ssz_block.randao_reveal(), &reference_hash.to_vec()[..]); - } - - #[test] - fn test_ssz_block_attestations() { - /* - * Single AttestationRecord - */ - let mut block = Block::zero(); - block.attestations.push(AttestationRecord::zero()); - - let serialized = get_block_ssz(&block); - let ssz_block = SszBlock::from_slice(&serialized).unwrap(); - let ssz_ar = get_attestation_record_ssz(&AttestationRecord::zero()); - - assert_eq!(ssz_block.attestations(), &ssz_ar[..]); - - /* - * Multiple AttestationRecords - */ - let mut block = Block::zero(); - block.attestations.push(AttestationRecord::zero()); - block.attestations.push(AttestationRecord::zero()); - - let serialized = get_block_ssz(&block); - let ssz_block = SszBlock::from_slice(&serialized).unwrap(); - let mut ssz_ar = get_attestation_record_ssz(&AttestationRecord::zero()); - ssz_ar.append(&mut get_attestation_record_ssz(&AttestationRecord::zero())); - - assert_eq!(ssz_block.attestations(), &ssz_ar[..]); - } - - #[test] - fn test_ssz_block_pow_chain_ref() { - let mut block = Block::zero(); - block.attestations.push(AttestationRecord::zero()); - let reference_hash = Hash256::from([42_u8; 32]); - block.pow_chain_ref = reference_hash.clone(); - - let serialized = get_block_ssz(&block); - let ssz_block = SszBlock::from_slice(&serialized).unwrap(); - - assert_eq!(ssz_block.pow_chain_ref(), &reference_hash.to_vec()[..]); - } - - #[test] - fn test_ssz_block_act_state_root() { - let mut block = Block::zero(); - block.attestations.push(AttestationRecord::zero()); - let reference_hash = Hash256::from([42_u8; 32]); - block.active_state_root = reference_hash.clone(); - - let serialized = get_block_ssz(&block); - let ssz_block = SszBlock::from_slice(&serialized).unwrap(); - - assert_eq!(ssz_block.act_state_root(), &reference_hash.to_vec()[..]); - } - - #[test] - fn test_ssz_block_cry_state_root() { - let mut block = Block::zero(); - block.attestations.push(AttestationRecord::zero()); - let reference_hash = Hash256::from([42_u8; 32]); - block.crystallized_state_root = reference_hash.clone(); - - let serialized = get_block_ssz(&block); - let ssz_block = SszBlock::from_slice(&serialized).unwrap(); - - assert_eq!(ssz_block.cry_state_root(), &reference_hash.to_vec()[..]); - } -} diff --git a/lighthouse/db/src/stores/block_store.rs b/lighthouse/db/src/stores/beacon_block_store.rs similarity index 75% rename from lighthouse/db/src/stores/block_store.rs rename to lighthouse/db/src/stores/beacon_block_store.rs index eb4c11a9c..932675e41 100644 --- a/lighthouse/db/src/stores/block_store.rs +++ b/lighthouse/db/src/stores/beacon_block_store.rs @@ -1,8 +1,7 @@ extern crate ssz_helpers; -use self::ssz_helpers::ssz_block::{ - SszBlock, - SszBlockError, +use self::ssz_helpers::ssz_beacon_block::{ + SszBeaconBlock, }; use std::sync::Arc; use super::{ @@ -12,19 +11,19 @@ use super::{ use super::BLOCKS_DB_COLUMN as DB_COLUMN; #[derive(Clone, Debug, PartialEq)] -pub enum BlockAtSlotError { - UnknownBlock, - InvalidBlock, +pub enum BeaconBlockAtSlotError { + UnknownBeaconBlock, + InvalidBeaconBlock, DBError(String), } -pub struct BlockStore +pub struct BeaconBlockStore where T: ClientDB { db: Arc, } -impl BlockStore { +impl BeaconBlockStore { pub fn new(db: Arc) -> Self { Self { db, @@ -66,26 +65,31 @@ impl BlockStore { /// /// If a block is found, a tuple of (block_hash, serialized_block) is returned. pub fn block_at_slot(&self, head_hash: &[u8], slot: u64) - -> Result, Vec)>, BlockAtSlotError> + -> Result, Vec)>, BeaconBlockAtSlotError> { match self.get_serialized_block(head_hash)? { - None => Err(BlockAtSlotError::UnknownBlock), + None => Err(BeaconBlockAtSlotError::UnknownBeaconBlock), Some(ssz) => { - let block = SszBlock::from_slice(&ssz) - .map_err(|_| BlockAtSlotError::InvalidBlock)?; - match block.slot_number() { + let block = SszBeaconBlock::from_slice(&ssz) + .map_err(|_| BeaconBlockAtSlotError::InvalidBeaconBlock)?; + match block.slot() { s if s == slot => Ok(Some((head_hash.to_vec(), ssz.to_vec()))), s if s < slot => Ok(None), - _ => self.block_at_slot(block.parent_hash(), slot) + _ => { + match block.parent_hash() { + Some(parent_hash) => self.block_at_slot(parent_hash, slot), + None => Err(BeaconBlockAtSlotError::UnknownBeaconBlock) + } + } } } } } } -impl From for BlockAtSlotError { +impl From for BeaconBlockAtSlotError { fn from(e: DBError) -> Self { - BlockAtSlotError::DBError(e.message) + BeaconBlockAtSlotError::DBError(e.message) } } @@ -94,7 +98,7 @@ mod tests { extern crate ssz; extern crate types; - use self::types::block::Block; + use self::types::beacon_block::BeaconBlock; use self::types::attestation_record::AttestationRecord; use self::types::Hash256; use self::ssz::SszStream; @@ -107,7 +111,7 @@ mod tests { #[test] fn test_block_store_on_memory_db() { let db = Arc::new(MemoryDB::open()); - let bs = Arc::new(BlockStore::new(db.clone())); + let bs = Arc::new(BeaconBlockStore::new(db.clone())); let thread_count = 10; let write_count = 10; @@ -146,11 +150,11 @@ mod tests { #[test] fn test_block_at_slot() { let db = Arc::new(MemoryDB::open()); - let bs = Arc::new(BlockStore::new(db.clone())); + let bs = Arc::new(BeaconBlockStore::new(db.clone())); let blocks = (0..5).into_iter() .map(|_| { - let mut block = Block::zero(); + let mut block = BeaconBlock::zero(); let ar = AttestationRecord::zero(); block.attestations.push(ar); block @@ -175,8 +179,7 @@ mod tests { let slots = [0, 1, 3, 4, 5]; for (i, mut block) in blocks.enumerate() { - block.parent_hash = parent_hashes[i]; - block.slot_number = slots[i]; + block.slot = slots[i]; let mut s = SszStream::new(); s.append(&block); let ssz = s.drain(); @@ -184,23 +187,23 @@ mod tests { } let tuple = bs.block_at_slot(&hashes[4], 5).unwrap().unwrap(); - let block = SszBlock::from_slice(&tuple.1).unwrap(); - assert_eq!(block.slot_number(), 5); + let block = SszBeaconBlock::from_slice(&tuple.1).unwrap(); + assert_eq!(block.slot(), 5); assert_eq!(tuple.0, hashes[4].to_vec()); let tuple = bs.block_at_slot(&hashes[4], 4).unwrap().unwrap(); - let block = SszBlock::from_slice(&tuple.1).unwrap(); - assert_eq!(block.slot_number(), 4); + let block = SszBeaconBlock::from_slice(&tuple.1).unwrap(); + assert_eq!(block.slot(), 4); assert_eq!(tuple.0, hashes[3].to_vec()); let tuple = bs.block_at_slot(&hashes[4], 3).unwrap().unwrap(); - let block = SszBlock::from_slice(&tuple.1).unwrap(); - assert_eq!(block.slot_number(), 3); + let block = SszBeaconBlock::from_slice(&tuple.1).unwrap(); + assert_eq!(block.slot(), 3); assert_eq!(tuple.0, hashes[2].to_vec()); let tuple = bs.block_at_slot(&hashes[4], 0).unwrap().unwrap(); - let block = SszBlock::from_slice(&tuple.1).unwrap(); - assert_eq!(block.slot_number(), 0); + let block = SszBeaconBlock::from_slice(&tuple.1).unwrap(); + assert_eq!(block.slot(), 0); assert_eq!(tuple.0, hashes[0].to_vec()); let ssz = bs.block_at_slot(&hashes[4], 2).unwrap(); @@ -210,6 +213,6 @@ mod tests { assert_eq!(ssz, None); let ssz = bs.block_at_slot(&Hash256::from("unknown".as_bytes()), 2); - assert_eq!(ssz, Err(BlockAtSlotError::UnknownBlock)); + assert_eq!(ssz, Err(BeaconBlockAtSlotError::UnknownBeaconBlock)); } } diff --git a/lighthouse/db/src/stores/mod.rs b/lighthouse/db/src/stores/mod.rs index e65803714..aa49489f7 100644 --- a/lighthouse/db/src/stores/mod.rs +++ b/lighthouse/db/src/stores/mod.rs @@ -3,13 +3,13 @@ use super::{ DBError, }; -mod block_store; +mod beacon_block_store; mod pow_chain_store; mod validator_store; -pub use self::block_store::{ - BlockStore, - BlockAtSlotError, +pub use self::beacon_block_store::{ + BeaconBlockStore, + BeaconBlockAtSlotError, }; pub use self::pow_chain_store::PoWChainStore; pub use self::validator_store::{ From e289d8b5fd3759845f383cc0fefd033fbf2b723d Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Mon, 15 Oct 2018 15:08:43 +1100 Subject: [PATCH 04/13] Fix comment in attestation_validation --- beacon_chain/validation/src/attestation_validation.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beacon_chain/validation/src/attestation_validation.rs b/beacon_chain/validation/src/attestation_validation.rs index bb458ebc7..62c37d029 100644 --- a/beacon_chain/validation/src/attestation_validation.rs +++ b/beacon_chain/validation/src/attestation_validation.rs @@ -102,8 +102,8 @@ impl AttestationValidationContext } /* - * The attestation must indicate that its last justified slot is the same as the last justified - * slot known to us. + * The attestation justified slot must not be higher than the last_justified_slot of the + * context. */ if a.justified_slot > self.last_justified_slot { return Err(AttestationValidationError::JustifiedSlotIncorrect); From e91317ca27b7caaa35ff52bc4d7b9dc727bda8e3 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Mon, 15 Oct 2018 16:26:40 +1100 Subject: [PATCH 05/13] Change SpecialRecord to use u8 instead of enum --- beacon_chain/types/src/special_record.rs | 69 ++++++++++++++++++++---- 1 file changed, 60 insertions(+), 9 deletions(-) diff --git a/beacon_chain/types/src/special_record.rs b/beacon_chain/types/src/special_record.rs index 62a5fdc36..8a8ada0bd 100644 --- a/beacon_chain/types/src/special_record.rs +++ b/beacon_chain/types/src/special_record.rs @@ -1,4 +1,9 @@ -use super::ssz::{ Encodable, SszStream }; +use super::ssz::{ + Encodable, + Decodable, + DecodeError, + SszStream, +}; /// The value of the "type" field of SpecialRecord. @@ -15,31 +20,46 @@ pub enum SpecialRecordKind { /// The structure used in the `BeaconBlock.specials` field. #[derive(Debug, PartialEq, Clone)] pub struct SpecialRecord { - pub kind: SpecialRecordKind, + pub kind: u8, pub data: Vec, } impl SpecialRecord { pub fn logout(data: &[u8]) -> Self { Self { - kind: SpecialRecordKind::Logout, + kind: SpecialRecordKind::Logout as u8, data: data.to_vec(), } } pub fn casper_slashing(data: &[u8]) -> Self { Self { - kind: SpecialRecordKind::CasperSlashing, + kind: SpecialRecordKind::CasperSlashing as u8, data: data.to_vec(), } } pub fn randao_change(data: &[u8]) -> Self { Self { - kind: SpecialRecordKind::RandaoChange, + kind: SpecialRecordKind::RandaoChange as u8, data: data.to_vec(), } } + + /// Match `self.kind` to a `SpecialRecordKind`. + /// + /// Returns `None` if `self.kind` is an unknown value. + fn resolve_kind(&self) -> Option { + match self.kind { + x if x == SpecialRecordKind::Logout as u8 + => Some(SpecialRecordKind::Logout), + x if x == SpecialRecordKind::CasperSlashing as u8 + => Some(SpecialRecordKind::CasperSlashing), + x if x == SpecialRecordKind::RandaoChange as u8 + => Some(SpecialRecordKind::RandaoChange), + _ => None + } + } } impl Encodable for SpecialRecord { @@ -49,19 +69,25 @@ impl Encodable for SpecialRecord { } } -impl Encodable for SpecialRecordKind { - fn ssz_append(&self, s: &mut SszStream) { - s.append(&(*self as u8)); +impl Decodable for SpecialRecord { + fn ssz_decode(bytes: &[u8], i: usize) + -> Result<(Self, usize), DecodeError> + { + let (kind, i) = u8::ssz_decode(bytes, i)?; + let (data, i) = Decodable::ssz_decode(bytes, i)?; + Ok((SpecialRecord{kind, data}, i)) } } + + #[cfg(test)] mod tests { use super::*; #[test] - pub fn test_special_record_ssz() { + pub fn test_special_record_ssz_encode() { let s = SpecialRecord::logout(&vec![]); let mut ssz_stream = SszStream::new(); ssz_stream.append(&s); @@ -86,4 +112,29 @@ mod tests { let ssz = ssz_stream.drain(); assert_eq!(ssz, vec![2, 0, 0, 0, 3, 42, 43, 44]); } + + #[test] + pub fn test_special_record_ssz_encode_decode() { + let s = SpecialRecord::randao_change(&vec![13, 16, 14]); + let mut ssz_stream = SszStream::new(); + ssz_stream.append(&s); + let ssz = ssz_stream.drain(); + let (s_decoded, _) = SpecialRecord::ssz_decode(&ssz, 0).unwrap(); + assert_eq!(s, s_decoded); + } + + #[test] + pub fn test_special_record_resolve_kind() { + let s = SpecialRecord::logout(&vec![]); + assert_eq!(s.resolve_kind(), Some(SpecialRecordKind::Logout)); + + let s = SpecialRecord::casper_slashing(&vec![]); + assert_eq!(s.resolve_kind(), Some(SpecialRecordKind::CasperSlashing)); + + let s = SpecialRecord::randao_change(&vec![]); + assert_eq!(s.resolve_kind(), Some(SpecialRecordKind::RandaoChange)); + + let s = SpecialRecord { kind: 88, data: vec![] }; + assert_eq!(s.resolve_kind(), None); + } } From 1621901f0de79fc05063f7efbb3f4c8be2247171 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 16 Oct 2018 13:44:26 +1100 Subject: [PATCH 06/13] Update SSZ - Implement generic list decoding> - Expose `encode` mod. - Add convenience encoding function. --- beacon_chain/utils/ssz/src/impl_decode.rs | 11 +++++++++++ beacon_chain/utils/ssz/src/lib.rs | 12 +++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/beacon_chain/utils/ssz/src/impl_decode.rs b/beacon_chain/utils/ssz/src/impl_decode.rs index 2833037f3..617585fd6 100644 --- a/beacon_chain/utils/ssz/src/impl_decode.rs +++ b/beacon_chain/utils/ssz/src/impl_decode.rs @@ -1,4 +1,5 @@ use super::ethereum_types::H256; +use super::decode::decode_ssz_list; use super::{ DecodeError, Decodable, @@ -65,6 +66,16 @@ impl Decodable for H256 { } } +impl Decodable for Vec + where T: Decodable +{ + fn ssz_decode(bytes: &[u8], index: usize) + -> Result<(Self, usize), DecodeError> + { + decode_ssz_list(bytes, index) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/beacon_chain/utils/ssz/src/lib.rs b/beacon_chain/utils/ssz/src/lib.rs index bf0de9262..47ee3cd96 100644 --- a/beacon_chain/utils/ssz/src/lib.rs +++ b/beacon_chain/utils/ssz/src/lib.rs @@ -11,8 +11,8 @@ extern crate bytes; extern crate ethereum_types; pub mod decode; +pub mod encode; -mod encode; mod impl_encode; mod impl_decode; @@ -29,3 +29,13 @@ pub use encode::{ pub const LENGTH_BYTES: usize = 4; pub const MAX_LIST_SIZE : usize = 1 << (4 * 8); + + +/// Convenience function to SSZ encode an object supporting ssz::Encode. +pub fn ssz_encode(val: &T) -> Vec + where T: Encodable +{ + let mut ssz_stream = SszStream::new(); + ssz_stream.append(val); + ssz_stream.drain() +} From 1207421d70b4888734e45b05abcf874249fb7d9a Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 16 Oct 2018 13:46:18 +1100 Subject: [PATCH 07/13] Fix broken block_store test --- lighthouse/db/src/stores/beacon_block_store.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/lighthouse/db/src/stores/beacon_block_store.rs b/lighthouse/db/src/stores/beacon_block_store.rs index 932675e41..80c29c5a7 100644 --- a/lighthouse/db/src/stores/beacon_block_store.rs +++ b/lighthouse/db/src/stores/beacon_block_store.rs @@ -179,6 +179,7 @@ mod tests { let slots = [0, 1, 3, 4, 5]; for (i, mut block) in blocks.enumerate() { + block.ancestor_hashes.push(parent_hashes[i]); block.slot = slots[i]; let mut s = SszStream::new(); s.append(&block); From f31d41e123485bd1301c0fe2c0ddcd02138ca048 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 16 Oct 2018 13:47:28 +1100 Subject: [PATCH 08/13] Implement SSZ decode for BeaconBlock, fix encode --- beacon_chain/types/src/beacon_block.rs | 51 ++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/beacon_chain/types/src/beacon_block.rs b/beacon_chain/types/src/beacon_block.rs index e75a3e6bb..fa833b149 100644 --- a/beacon_chain/types/src/beacon_block.rs +++ b/beacon_chain/types/src/beacon_block.rs @@ -1,7 +1,12 @@ use super::Hash256; use super::attestation_record::AttestationRecord; use super::special_record::SpecialRecord; -use super::ssz::{ Encodable, SszStream }; +use super::ssz::{ + Encodable, + Decodable, + DecodeError, + SszStream, +}; pub const MIN_SSZ_BLOCK_LENGTH: usize = { 8 + // slot @@ -54,7 +59,7 @@ impl Encodable for BeaconBlock { s.append(&self.slot); s.append(&self.randao_reveal); s.append(&self.pow_chain_reference); - s.append_vec(&self.ancestor_hashes.to_vec()); + s.append_vec(&self.ancestor_hashes); s.append(&self.active_state_root); s.append(&self.crystallized_state_root); s.append_vec(&self.attestations); @@ -62,6 +67,32 @@ impl Encodable for BeaconBlock { } } +impl Decodable for BeaconBlock { + fn ssz_decode(bytes: &[u8], i: usize) + -> Result<(Self, usize), DecodeError> + { + let (slot, i) = u64::ssz_decode(bytes, i)?; + let (randao_reveal, i) = Hash256::ssz_decode(bytes, i)?; + let (pow_chain_reference, i) = Hash256::ssz_decode(bytes, i)?; + let (ancestor_hashes, i) = Decodable::ssz_decode(bytes, i)?; + let (active_state_root, i) = Hash256::ssz_decode(bytes, i)?; + let (crystallized_state_root, i) = Hash256::ssz_decode(bytes, i)?; + let (attestations, i) = Decodable::ssz_decode(bytes, i)?; + let (specials, i) = Decodable::ssz_decode(bytes, i)?; + let block = BeaconBlock { + slot, + randao_reveal, + pow_chain_reference, + ancestor_hashes, + active_state_root, + crystallized_state_root, + attestations, + specials + }; + Ok((block, i)) + } +} + #[cfg(test)] mod tests { @@ -73,13 +104,27 @@ mod tests { assert_eq!(b.slot, 0); assert!(b.randao_reveal.is_zero()); assert!(b.pow_chain_reference.is_zero()); - assert_eq!(b.ancestor_hashes, vec![Hash256::zero()]); + assert_eq!(b.ancestor_hashes, vec![]); assert!(b.active_state_root.is_zero()); assert!(b.crystallized_state_root.is_zero()); assert_eq!(b.attestations.len(), 0); assert_eq!(b.specials.len(), 0); } + #[test] + pub fn test_block_ssz_encode_decode() { + let mut b = BeaconBlock::zero(); + b.ancestor_hashes = vec![Hash256::zero(); 32]; + + let mut ssz_stream = SszStream::new(); + ssz_stream.append(&b); + let ssz = ssz_stream.drain(); + + let (b_decoded, _) = BeaconBlock::ssz_decode(&ssz, 0).unwrap(); + + assert_eq!(b, b_decoded); + } + #[test] pub fn test_block_min_ssz_length() { let b = BeaconBlock::zero(); From c45e05ca0285a0e30796aeb3bded0beb7c5172de Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 16 Oct 2018 13:59:26 +1100 Subject: [PATCH 09/13] Update SszBeaconBlock as per new spec --- .../utils/ssz_helpers/src/ssz_beacon_block.rs | 65 +++++++++++-------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/beacon_chain/utils/ssz_helpers/src/ssz_beacon_block.rs b/beacon_chain/utils/ssz_helpers/src/ssz_beacon_block.rs index b46b1f2db..7b2c2862b 100644 --- a/beacon_chain/utils/ssz_helpers/src/ssz_beacon_block.rs +++ b/beacon_chain/utils/ssz_helpers/src/ssz_beacon_block.rs @@ -7,7 +7,6 @@ use super::types::beacon_block::{ MIN_SSZ_BLOCK_LENGTH, MAX_SSZ_BLOCK_LENGTH, }; -use super::types::attestation_record::MIN_SSZ_ATTESTION_RECORD_LENGTH; #[derive(Debug, PartialEq)] pub enum SszBeaconBlockError { @@ -136,8 +135,9 @@ impl<'a> SszBeaconBlock<'a> { /// The first hash in `ancestor_hashes` is the parent of the block. pub fn parent_hash(&self) -> Option<&[u8]> { let ancestor_ssz = self.ancestor_hashes(); + let start = LENGTH_BYTES; if ancestor_ssz.len() >= 32 { - Some(&ancestor_ssz[0..32]) + Some(&ancestor_ssz[start..start + 32]) } else { None } @@ -174,10 +174,10 @@ impl<'a> SszBeaconBlock<'a> { &self.ssz[start..start + 32] } - /// Return the serialized `ancestor_hashes` bytes. + /// Return the serialized `ancestor_hashes` bytes, including length prefix. pub fn ancestor_hashes(&self) -> &[u8] { - let start = self.ancestors_position + LENGTH_BYTES; - &self.ssz[start..start + self.ancestors_len] + let start = self.ancestors_position; + &self.ssz[start..(start + self.ancestors_len + LENGTH_BYTES)] } /// Return the `active_state_root` field. @@ -194,16 +194,22 @@ impl<'a> SszBeaconBlock<'a> { &self.ssz[start..(start + 32)] } - /// Return the serialized `attestations` bytes. + /// Return the serialized `attestations` bytes, including length prefix. pub fn attestations(&self) -> &[u8] { - let start = self.attestations_position + LENGTH_BYTES; - &self.ssz[start..(start + self.attestations_len)] + let start = self.attestations_position; + &self.ssz[start..(start + self.attestations_len + LENGTH_BYTES)] } - /// Return the serialized `specials` bytes. + /// Return the serialized `attestations` bytes _without_ the length prefix. + pub fn attestations_without_length(&self) -> &[u8] { + let start = self.attestations_position + LENGTH_BYTES; + &self.ssz[start..start + self.attestations_len] + } + + /// Return the serialized `specials` bytes, including length prefix. pub fn specials(&self) -> &[u8] { - let start = self.specials_position + LENGTH_BYTES; - &self.ssz[start..(start + self.specials_len)] + let start = self.specials_position; + &self.ssz[start..(start + self.specials_len + LENGTH_BYTES)] } } @@ -218,6 +224,7 @@ mod tests { }; use super::super::ssz::SszStream; use super::super::types::Hash256; + use super::super::ssz::encode::encode_length; fn get_block_ssz(b: &BeaconBlock) -> Vec { let mut ssz_stream = SszStream::new(); @@ -281,17 +288,6 @@ mod tests { assert!(SszBeaconBlock::from_slice(&ssz[..]).is_ok()); } - #[test] - fn test_ssz_block_attestations_length() { - let mut block = BeaconBlock::zero(); - block.attestations.push(AttestationRecord::zero()); - - let serialized = get_block_ssz(&block); - let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); - - assert_eq!(ssz_block.attestations_len, MIN_SSZ_ATTESTION_RECORD_LENGTH); - } - #[test] fn test_ssz_block_block_hash() { let mut block = BeaconBlock::zero(); @@ -353,7 +349,10 @@ mod tests { let serialized = get_block_ssz(&block); let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); - assert_eq!(ssz_block.ancestor_hashes(), &h.to_vec()[..]); + let mut expected = encode_length(32, LENGTH_BYTES); + expected.append(&mut h.to_vec()); + + assert_eq!(ssz_block.ancestor_hashes(), &expected[..]); } #[test] @@ -384,7 +383,10 @@ mod tests { let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); let sr_ssz = get_special_record_ssz(&s); - assert_eq!(ssz_block.specials(), &sr_ssz[..]); + let mut expected = encode_length(sr_ssz.len(), LENGTH_BYTES); + expected.append(&mut sr_ssz.to_vec()); + + assert_eq!(ssz_block.specials(), &expected[..]); /* * With data @@ -397,7 +399,10 @@ mod tests { let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); let sr_ssz = get_special_record_ssz(&s); - assert_eq!(ssz_block.specials(), &sr_ssz[..]); + let mut expected = encode_length(sr_ssz.len(), LENGTH_BYTES); + expected.append(&mut sr_ssz.to_vec()); + + assert_eq!(ssz_block.specials(), &expected[..]); } #[test] @@ -412,7 +417,10 @@ mod tests { let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); let ssz_ar = get_attestation_record_ssz(&AttestationRecord::zero()); - assert_eq!(ssz_block.attestations(), &ssz_ar[..]); + let mut expected = encode_length(ssz_ar.len(), LENGTH_BYTES); + expected.append(&mut ssz_ar.to_vec()); + + assert_eq!(ssz_block.attestations(), &expected[..]); /* * Multiple AttestationRecords @@ -426,7 +434,10 @@ mod tests { let mut ssz_ar = get_attestation_record_ssz(&AttestationRecord::zero()); ssz_ar.append(&mut get_attestation_record_ssz(&AttestationRecord::zero())); - assert_eq!(ssz_block.attestations(), &ssz_ar[..]); + let mut expected = encode_length(ssz_ar.len(), LENGTH_BYTES); + expected.append(&mut ssz_ar.to_vec()); + + assert_eq!(ssz_block.attestations(), &expected[..]); } #[test] From c3d88a7e80c64f08fb7219479375702ab75374db Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 16 Oct 2018 13:59:45 +1100 Subject: [PATCH 10/13] Update validation as per new spec - Block -> BeaconBlock - Updates to SszBeaconBlock --- .../validation/src/attestation_validation.rs | 12 +- .../validation/src/block_validation.rs | 119 ++++++++++-------- .../tests/attestation_validation/helpers.rs | 16 +-- .../tests/block_validation/helpers.rs | 56 +++++---- .../tests/block_validation/tests.rs | 69 +++++----- 5 files changed, 143 insertions(+), 129 deletions(-) diff --git a/beacon_chain/validation/src/attestation_validation.rs b/beacon_chain/validation/src/attestation_validation.rs index 62c37d029..94e3fcac9 100644 --- a/beacon_chain/validation/src/attestation_validation.rs +++ b/beacon_chain/validation/src/attestation_validation.rs @@ -13,8 +13,8 @@ use super::db::{ DBError }; use super::db::stores::{ - BlockStore, - BlockAtSlotError, + BeaconBlockStore, + BeaconBlockAtSlotError, ValidatorStore, }; use super::types::{ @@ -64,7 +64,7 @@ pub struct AttestationValidationContext /// A vec of the hashes of the blocks preceeding the present slot. pub parent_hashes: Arc>, /// The store containing block information. - pub block_store: Arc>, + pub block_store: Arc>, /// The store containing validator information. pub validator_store: Arc>, /// A map of (slot, shard_id) to the attestation set of validation indices. @@ -223,10 +223,10 @@ impl From for AttestationValidationError { } } -impl From for AttestationValidationError { - fn from(e: BlockAtSlotError) -> Self { +impl From for AttestationValidationError { + fn from(e: BeaconBlockAtSlotError) -> Self { match e { - BlockAtSlotError::DBError(s) => AttestationValidationError::DBError(s), + BeaconBlockAtSlotError::DBError(s) => AttestationValidationError::DBError(s), _ => AttestationValidationError::InvalidJustifiedBlockHash } diff --git a/beacon_chain/validation/src/block_validation.rs b/beacon_chain/validation/src/block_validation.rs index 08aae7c0e..43c7138a6 100644 --- a/beacon_chain/validation/src/block_validation.rs +++ b/beacon_chain/validation/src/block_validation.rs @@ -13,7 +13,7 @@ use super::attestation_validation::{ use super::types::{ AttestationRecord, AttesterMap, - Block, + BeaconBlock, ProposerMap, }; use super::ssz_helpers::attestation_ssz_splitter::{ @@ -21,16 +21,16 @@ use super::ssz_helpers::attestation_ssz_splitter::{ split_all_attestations, AttestationSplitError, }; -use super::ssz_helpers::ssz_block::{ - SszBlock, - SszBlockError, +use super::ssz_helpers::ssz_beacon_block::{ + SszBeaconBlock, + SszBeaconBlockError, }; use super::db::{ ClientDB, DBError, }; use super::db::stores::{ - BlockStore, + BeaconBlockStore, PoWChainStore, ValidatorStore, }; @@ -41,18 +41,20 @@ use super::ssz::{ use super::types::Hash256; #[derive(Debug, PartialEq)] -pub enum BlockStatus { +pub enum BeaconBlockStatus { NewBlock, KnownBlock, } #[derive(Debug, PartialEq)] -pub enum SszBlockValidationError { +pub enum SszBeaconBlockValidationError { FutureSlot, SlotAlreadyFinalized, UnknownPoWChainRef, UnknownParentHash, BadAttestationSsz, + BadAncestorHashesSsz, + BadSpecialsSsz, ParentSlotHigherThanBlockSlot, AttestationValidationError(AttestationValidationError), AttestationSignatureFailed, @@ -64,7 +66,7 @@ pub enum SszBlockValidationError { } /// The context against which a block should be validated. -pub struct BlockValidationContext +pub struct BeaconBlockValidationContext where T: ClientDB + Sized { /// The slot as determined by the system time. @@ -84,20 +86,20 @@ pub struct BlockValidationContext /// A map of (slot, shard_id) to the attestation set of validation indices. pub attester_map: Arc, /// The store containing block information. - pub block_store: Arc>, + pub block_store: Arc>, /// The store containing validator information. pub validator_store: Arc>, /// The store containing information about the proof-of-work chain. pub pow_store: Arc>, } -impl BlockValidationContext +impl BeaconBlockValidationContext where T: ClientDB { - /// Validate some SszBlock against a block validation context. An SszBlock varies from a Block in + /// Validate some SszBeaconBlock against a block validation context. An SszBeaconBlock varies from a BeaconBlock 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 + /// The reason to validate an SzzBeaconBlock 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. /// @@ -107,8 +109,8 @@ impl BlockValidationContext /// Note: this function does not implement randao_reveal checking as it is not in the /// specification. #[allow(dead_code)] - pub fn validate_ssz_block(&self, b: &SszBlock) - -> Result<(BlockStatus, Option), SszBlockValidationError> + pub fn validate_ssz_block(&self, b: &SszBeaconBlock) + -> Result<(BeaconBlockStatus, Option), SszBeaconBlockValidationError> where T: ClientDB + Sized { @@ -118,7 +120,7 @@ impl BlockValidationContext */ let block_hash = &b.block_hash(); if self.block_store.block_exists(&block_hash)? { - return Ok((BlockStatus::KnownBlock, None)); + return Ok((BeaconBlockStatus::KnownBlock, None)); } /* @@ -127,9 +129,9 @@ impl BlockValidationContext * It is up to the calling fn to determine what should be done with "future" blocks (e.g., * cache or discard). */ - let block_slot = b.slot_number(); + let block_slot = b.slot(); if block_slot > self.present_slot { - return Err(SszBlockValidationError::FutureSlot); + return Err(SszBeaconBlockValidationError::FutureSlot); } /* @@ -143,7 +145,7 @@ impl BlockValidationContext * `last_finalized_block` in it's chain. */ if block_slot <= self.last_finalized_slot { - return Err(SszBlockValidationError::SlotAlreadyFinalized); + return Err(SszBeaconBlockValidationError::SlotAlreadyFinalized); } /* @@ -155,15 +157,15 @@ impl BlockValidationContext * "sufficienty deep in the canonical PoW chain". This should be clarified as the spec * crystallizes. */ - let pow_chain_ref = b.pow_chain_ref(); - if !self.pow_store.block_hash_exists(b.pow_chain_ref())? { - return Err(SszBlockValidationError::UnknownPoWChainRef); + let pow_chain_reference = b.pow_chain_reference(); + if !self.pow_store.block_hash_exists(b.pow_chain_reference())? { + return Err(SszBeaconBlockValidationError::UnknownPoWChainRef); } /* * Store a slice of the serialized attestations from the block SSZ. */ - let attestations_ssz = &b.attestations(); + let attestations_ssz = &b.attestations_without_length(); /* * Get a slice of the first serialized attestation (the 0'th) and decode it into @@ -185,7 +187,7 @@ impl BlockValidationContext * of the previous block is attesting to some other block than the one they produced. */ if first_attestation.oblique_parent_hashes.len() > 0 { - return Err(SszBlockValidationError::ProposerAttestationHasObliqueHashes); + return Err(SszBeaconBlockValidationError::ProposerAttestationHasObliqueHashes); } /* @@ -196,12 +198,13 @@ impl BlockValidationContext * * Also, read the slot from the parent block for later use. */ - let parent_hash = b.parent_hash(); + let parent_hash = b.parent_hash() + .ok_or(SszBeaconBlockValidationError::BadAncestorHashesSsz)?; let parent_block_slot = match self.block_store.get_serialized_block(&parent_hash)? { - None => return Err(SszBlockValidationError::UnknownParentHash), + None => return Err(SszBeaconBlockValidationError::UnknownParentHash), Some(ssz) => { - let parent_block = SszBlock::from_slice(&ssz[..])?; - parent_block.slot_number() + let parent_block = SszBeaconBlock::from_slice(&ssz[..])?; + parent_block.slot() } }; @@ -211,7 +214,7 @@ impl BlockValidationContext * In other words, the parent must come before the child. */ if parent_block_slot >= block_slot { - return Err(SszBlockValidationError::ParentSlotHigherThanBlockSlot); + return Err(SszBeaconBlockValidationError::ParentSlotHigherThanBlockSlot); } /* @@ -242,9 +245,9 @@ impl BlockValidationContext * attestation of this block, reject the block. */ let parent_block_proposer = self.proposer_map.get(&parent_block_slot) - .ok_or(SszBlockValidationError::BadProposerMap)?; + .ok_or(SszBeaconBlockValidationError::BadProposerMap)?; if !attestation_voters.contains(&parent_block_proposer) { - return Err(SszBlockValidationError::NoProposerSignature); + return Err(SszBeaconBlockValidationError::NoProposerSignature); } /* @@ -265,7 +268,7 @@ impl BlockValidationContext * validation. This is so all attestation validation is halted if a single bad attestation * is found. */ - let failure: RwLock> = RwLock::new(None); + let failure: RwLock> = RwLock::new(None); let mut deserialized_attestations: Vec = other_attestations .par_iter() .filter_map(|attestation_ssz| { @@ -286,7 +289,7 @@ impl BlockValidationContext */ Err(e) => { let mut failure = failure.write().unwrap(); - *failure = Some(SszBlockValidationError::from(e)); + *failure = Some(SszBeaconBlockValidationError::from(e)); None } /* @@ -299,7 +302,7 @@ impl BlockValidationContext */ Err(e) => { let mut failure = failure.write().unwrap(); - *failure = Some(SszBlockValidationError::from(e)); + *failure = Some(SszBeaconBlockValidationError::from(e)); None } /* @@ -313,7 +316,7 @@ impl BlockValidationContext .collect(); match failure.into_inner() { - Err(_) => return Err(SszBlockValidationError::RwLockPoisoned), + Err(_) => return Err(SszBeaconBlockValidationError::RwLockPoisoned), Ok(failure) => { match failure { Some(error) => return Err(error), @@ -329,63 +332,69 @@ impl BlockValidationContext */ deserialized_attestations.insert(0, first_attestation); + let (ancestor_hashes, _) = Decodable::ssz_decode(&b.ancestor_hashes(), 0) + .map_err(|_| SszBeaconBlockValidationError::BadAncestorHashesSsz)?; + let (specials, _) = Decodable::ssz_decode(&b.specials(), 0) + .map_err(|_| SszBeaconBlockValidationError::BadSpecialsSsz)?; + /* * If we have reached this point, the block is a new valid block that is worthy of * processing. */ - let block = Block { - parent_hash: Hash256::from(parent_hash), - slot_number: block_slot, + let block = BeaconBlock { + slot: block_slot, randao_reveal: Hash256::from(b.randao_reveal()), - attestations: deserialized_attestations, - pow_chain_ref: Hash256::from(pow_chain_ref), + pow_chain_reference: Hash256::from(pow_chain_reference), + ancestor_hashes, active_state_root: Hash256::from(b.act_state_root()), crystallized_state_root: Hash256::from(b.cry_state_root()), + attestations: deserialized_attestations, + specials, }; - Ok((BlockStatus::NewBlock, Some(block))) + Ok((BeaconBlockStatus::NewBlock, Some(block))) } } -impl From for SszBlockValidationError { +impl From for SszBeaconBlockValidationError { fn from(e: DBError) -> Self { - SszBlockValidationError::DBError(e.message) + SszBeaconBlockValidationError::DBError(e.message) } } -impl From for SszBlockValidationError { +impl From for SszBeaconBlockValidationError { fn from(e: AttestationSplitError) -> Self { match e { AttestationSplitError::TooShort => - SszBlockValidationError::BadAttestationSsz + SszBeaconBlockValidationError::BadAttestationSsz } } } -impl From for SszBlockValidationError { - fn from(e: SszBlockError) -> Self { +impl From for SszBeaconBlockValidationError { + fn from(e: SszBeaconBlockError) -> Self { match e { - SszBlockError::TooShort => - SszBlockValidationError::DBError("Bad parent block in db.".to_string()), - SszBlockError::TooLong => - SszBlockValidationError::DBError("Bad parent block in db.".to_string()), + SszBeaconBlockError::TooShort => + SszBeaconBlockValidationError::DBError("Bad parent block in db.".to_string()), + SszBeaconBlockError::TooLong => + SszBeaconBlockValidationError::DBError("Bad parent block in db.".to_string()), } } } -impl From for SszBlockValidationError { +impl From for SszBeaconBlockValidationError { fn from(e: DecodeError) -> Self { match e { DecodeError::TooShort => - SszBlockValidationError::BadAttestationSsz, + SszBeaconBlockValidationError::BadAttestationSsz, DecodeError::TooLong => - SszBlockValidationError::BadAttestationSsz, + SszBeaconBlockValidationError::BadAttestationSsz, } } } -impl From for SszBlockValidationError { +impl From for SszBeaconBlockValidationError { fn from(e: AttestationValidationError) -> Self { - SszBlockValidationError::AttestationValidationError(e) + SszBeaconBlockValidationError::AttestationValidationError(e) } } diff --git a/beacon_chain/validation/tests/attestation_validation/helpers.rs b/beacon_chain/validation/tests/attestation_validation/helpers.rs index df2bfdcc7..0bb0bb77d 100644 --- a/beacon_chain/validation/tests/attestation_validation/helpers.rs +++ b/beacon_chain/validation/tests/attestation_validation/helpers.rs @@ -5,13 +5,13 @@ use super::db::{ }; use super::db::stores::{ ValidatorStore, - BlockStore, + BeaconBlockStore, }; use super::types::{ AttestationRecord, AttesterMap, Bitfield, - Block, + BeaconBlock, Hash256, }; use super::validation::attestation_validation::{ @@ -32,14 +32,14 @@ use super::hashing::{ pub struct TestStore { pub db: Arc, pub validator: Arc>, - pub block: Arc>, + pub block: Arc>, } impl TestStore { pub fn new() -> Self { let db = Arc::new(MemoryDB::open()); let validator = Arc::new(ValidatorStore::new(db.clone())); - let block = Arc::new(BlockStore::new(db.clone())); + let block = Arc::new(BeaconBlockStore::new(db.clone())); Self { db, validator, @@ -81,7 +81,7 @@ pub fn generate_attestation(shard_id: u16, cycle_length: u8, parent_hashes: &[Hash256], signing_keys: &[Option], - block_store: &BlockStore) + block_store: &BeaconBlockStore) -> AttestationRecord { let mut attester_bitfield = Bitfield::new(); @@ -136,10 +136,10 @@ pub fn generate_attestation(shard_id: u16, /// Create a minimum viable block at some slot. /// /// Allows the validation function to read the block and verify its slot. -pub fn create_block_at_slot(block_store: &BlockStore, hash: &Hash256, slot: u64) { - let mut justified_block = Block::zero(); +pub fn create_block_at_slot(block_store: &BeaconBlockStore, hash: &Hash256, slot: u64) { + let mut justified_block = BeaconBlock::zero(); justified_block.attestations.push(AttestationRecord::zero()); - justified_block.slot_number = slot; + justified_block.slot = slot; let mut s = SszStream::new(); s.append(&justified_block); let justified_block_ssz = s.drain(); diff --git a/beacon_chain/validation/tests/block_validation/helpers.rs b/beacon_chain/validation/tests/block_validation/helpers.rs index 83c806af9..3e367fa96 100644 --- a/beacon_chain/validation/tests/block_validation/helpers.rs +++ b/beacon_chain/validation/tests/block_validation/helpers.rs @@ -11,29 +11,29 @@ use super::db::{ MemoryDB, }; use super::db::stores::{ - BlockStore, + BeaconBlockStore, PoWChainStore, ValidatorStore, }; use super::types::{ AttestationRecord, AttesterMap, - Block, + BeaconBlock, Hash256, ProposerMap, }; -use super::ssz_helpers::ssz_block::SszBlock; +use super::ssz_helpers::ssz_beacon_block::SszBeaconBlock; use super::validation::block_validation::{ - BlockValidationContext, - SszBlockValidationError, - BlockStatus, + BeaconBlockValidationContext, + SszBeaconBlockValidationError, + BeaconBlockStatus, }; use super::ssz::{ SszStream, }; #[derive(Debug)] -pub struct BlockTestParams { +pub struct BeaconBlockTestParams { pub total_validators: usize, pub cycle_length: u8, pub shard_count: u16, @@ -50,7 +50,7 @@ pub struct BlockTestParams { pub struct TestStore { pub db: Arc, - pub block: Arc>, + pub block: Arc>, pub pow_chain: Arc>, pub validator: Arc>, } @@ -58,7 +58,7 @@ pub struct TestStore { impl TestStore { pub fn new() -> Self { let db = Arc::new(MemoryDB::open()); - let block = Arc::new(BlockStore::new(db.clone())); + let block = Arc::new(BeaconBlockStore::new(db.clone())); let pow_chain = Arc::new(PoWChainStore::new(db.clone())); let validator = Arc::new(ValidatorStore::new(db.clone())); Self { @@ -74,8 +74,8 @@ type ParentHashes = Vec; /// Setup for a block validation function, without actually executing the /// block validation function. -pub fn setup_block_validation_scenario(params: &BlockTestParams) - -> (Block, ParentHashes, AttesterMap, ProposerMap, TestStore) +pub fn setup_block_validation_scenario(params: &BeaconBlockTestParams) + -> (BeaconBlock, ParentHashes, AttesterMap, ProposerMap, TestStore) { let stores = TestStore::new(); @@ -89,6 +89,7 @@ pub fn setup_block_validation_scenario(params: &BlockTestParams) .map(|i| Hash256::from(i as u64)) .collect(); let parent_hash = Hash256::from("parent_hash".as_bytes()); + let ancestor_hashes = vec![parent_hash.clone(); 32]; let randao_reveal = Hash256::from("randao_reveal".as_bytes()); let justified_block_hash = Hash256::from("justified_hash".as_bytes()); let pow_chain_ref = Hash256::from("pow_chain".as_bytes()); @@ -104,16 +105,16 @@ pub fn setup_block_validation_scenario(params: &BlockTestParams) /* * Generate a minimum viable parent block and store it in the database. */ - let mut parent_block = Block::zero(); + let mut parent_block = BeaconBlock::zero(); let parent_attestation = AttestationRecord::zero(); - parent_block.slot_number = block_slot - 1; + parent_block.slot = block_slot - 1; parent_block.attestations.push(parent_attestation); let parent_block_ssz = serialize_block(&parent_block); stores.block.put_serialized_block(parent_hash.as_ref(), &parent_block_ssz).unwrap(); let proposer_map = { let mut proposer_map = ProposerMap::new(); - proposer_map.insert(parent_block.slot_number, params.parent_proposer_index); + proposer_map.insert(parent_block.slot, params.parent_proposer_index); proposer_map }; @@ -168,14 +169,15 @@ pub fn setup_block_validation_scenario(params: &BlockTestParams) (attester_map, attestations, keypairs) }; - let block = Block { - parent_hash, - slot_number: block_slot, + let block = BeaconBlock { + slot: block_slot, randao_reveal, - attestations, - pow_chain_ref, + pow_chain_reference: pow_chain_ref, + ancestor_hashes, active_state_root, crystallized_state_root, + attestations, + specials: vec![], }; (block, @@ -185,8 +187,8 @@ pub fn setup_block_validation_scenario(params: &BlockTestParams) stores) } -/// Helper function to take some Block and SSZ serialize it. -pub fn serialize_block(b: &Block) -> Vec { +/// Helper function to take some BeaconBlock and SSZ serialize it. +pub fn serialize_block(b: &BeaconBlock) -> Vec { let mut stream = SszStream::new(); stream.append(b); stream.drain() @@ -196,11 +198,11 @@ pub fn serialize_block(b: &Block) -> Vec { /// /// Returns the Result returned from the block validation function. pub fn run_block_validation_scenario( - params: &BlockTestParams, + params: &BeaconBlockTestParams, mutator_func: F) - -> Result<(BlockStatus, Option), SszBlockValidationError> - where F: FnOnce(Block, AttesterMap, ProposerMap, TestStore) - -> (Block, AttesterMap, ProposerMap, TestStore) + -> Result<(BeaconBlockStatus, Option), SszBeaconBlockValidationError> + where F: FnOnce(BeaconBlock, AttesterMap, ProposerMap, TestStore) + -> (BeaconBlock, AttesterMap, ProposerMap, TestStore) { let (block, parent_hashes, @@ -214,10 +216,10 @@ pub fn run_block_validation_scenario( stores) = mutator_func(block, attester_map, proposer_map, stores); let ssz_bytes = serialize_block(&block); - let ssz_block = SszBlock::from_slice(&ssz_bytes[..]) + let ssz_block = SszBeaconBlock::from_slice(&ssz_bytes[..]) .unwrap(); - let context = BlockValidationContext { + let context = BeaconBlockValidationContext { present_slot: params.validation_context_slot, cycle_length: params.cycle_length, last_justified_slot: params.validation_context_justified_slot, diff --git a/beacon_chain/validation/tests/block_validation/tests.rs b/beacon_chain/validation/tests/block_validation/tests.rs index 3046998a9..0c0cba0f3 100644 --- a/beacon_chain/validation/tests/block_validation/tests.rs +++ b/beacon_chain/validation/tests/block_validation/tests.rs @@ -2,27 +2,27 @@ use super::bls::{ AggregateSignature, }; use super::helpers::{ - BlockTestParams, + BeaconBlockTestParams, TestStore, run_block_validation_scenario, serialize_block, }; use super::types::{ - Block, + BeaconBlock, Hash256, ProposerMap, }; -use super::ssz_helpers::ssz_block::SszBlock; +use super::ssz_helpers::ssz_beacon_block::SszBeaconBlock; use super::validation::block_validation::{ - SszBlockValidationError, - BlockStatus, + SszBeaconBlockValidationError, + BeaconBlockStatus, }; use super::validation::attestation_validation::{ AttestationValidationError, }; use super::hashing::canonical_hash; -fn get_simple_params() -> BlockTestParams { +fn get_simple_params() -> BeaconBlockTestParams { let validators_per_shard: usize = 5; let cycle_length: u8 = 2; let shard_count: u16 = 4; @@ -37,7 +37,7 @@ fn get_simple_params() -> BlockTestParams { let validation_context_justified_block_hash = Hash256::from("justified_hash".as_bytes()); let validation_context_finalized_slot = 0; - BlockTestParams { + BeaconBlockTestParams { total_validators, cycle_length, shard_count, @@ -59,7 +59,7 @@ fn get_simple_params() -> BlockTestParams { fn test_block_validation_valid() { let params = get_simple_params(); - let mutator = |block: Block, attester_map, proposer_map, stores| { + let mutator = |block: BeaconBlock, attester_map, proposer_map, stores| { /* * Do not mutate */ @@ -70,14 +70,14 @@ fn test_block_validation_valid() { ¶ms, mutator); - assert_eq!(status.unwrap().0, BlockStatus::NewBlock); + assert_eq!(status.unwrap().0, BeaconBlockStatus::NewBlock); } #[test] fn test_block_validation_valid_known_block() { let params = get_simple_params(); - let mutator = |block: Block, attester_map, proposer_map, stores: TestStore| { + let mutator = |block: BeaconBlock, attester_map, proposer_map, stores: TestStore| { /* * Pre-store the block in the database */ @@ -91,15 +91,15 @@ fn test_block_validation_valid_known_block() { ¶ms, mutator); - assert_eq!(status.unwrap(), (BlockStatus::KnownBlock, None)); + assert_eq!(status.unwrap(), (BeaconBlockStatus::KnownBlock, None)); } #[test] fn test_block_validation_parent_slot_too_high() { let params = get_simple_params(); - let mutator = |mut block: Block, attester_map, proposer_map, stores| { - block.slot_number = params.validation_context_justified_slot + 1; + let mutator = |mut block: BeaconBlock, attester_map, proposer_map, stores| { + block.slot = params.validation_context_justified_slot + 1; (block, attester_map, proposer_map, stores) }; @@ -107,15 +107,15 @@ fn test_block_validation_parent_slot_too_high() { ¶ms, mutator); - assert_eq!(status, Err(SszBlockValidationError::ParentSlotHigherThanBlockSlot)); + assert_eq!(status, Err(SszBeaconBlockValidationError::ParentSlotHigherThanBlockSlot)); } #[test] fn test_block_validation_invalid_future_slot() { let params = get_simple_params(); - let mutator = |mut block: Block, attester_map, proposer_map, stores| { - block.slot_number = block.slot_number + 1; + let mutator = |mut block: BeaconBlock, attester_map, proposer_map, stores| { + block.slot = block.slot + 1; (block, attester_map, proposer_map, stores) }; @@ -123,7 +123,7 @@ fn test_block_validation_invalid_future_slot() { ¶ms, mutator); - assert_eq!(status, Err(SszBlockValidationError::FutureSlot)); + assert_eq!(status, Err(SszBeaconBlockValidationError::FutureSlot)); } #[test] @@ -145,15 +145,15 @@ fn test_block_validation_invalid_slot_already_finalized() { ¶ms, mutator); - assert_eq!(status, Err(SszBlockValidationError::SlotAlreadyFinalized)); + assert_eq!(status, Err(SszBeaconBlockValidationError::SlotAlreadyFinalized)); } #[test] fn test_block_validation_invalid_unknown_pow_hash() { let params = get_simple_params(); - let mutator = |mut block: Block, attester_map, proposer_map, stores| { - block.pow_chain_ref = Hash256::from("unknown pow hash".as_bytes()); + let mutator = |mut block: BeaconBlock, attester_map, proposer_map, stores| { + block.pow_chain_reference = Hash256::from("unknown pow hash".as_bytes()); (block, attester_map, proposer_map, stores) }; @@ -161,15 +161,15 @@ fn test_block_validation_invalid_unknown_pow_hash() { ¶ms, mutator); - assert_eq!(status, Err(SszBlockValidationError::UnknownPoWChainRef)); + assert_eq!(status, Err(SszBeaconBlockValidationError::UnknownPoWChainRef)); } #[test] fn test_block_validation_invalid_unknown_parent_hash() { let params = get_simple_params(); - let mutator = |mut block: Block, attester_map, proposer_map, stores| { - block.parent_hash = Hash256::from("unknown parent block".as_bytes()); + let mutator = |mut block: BeaconBlock, attester_map, proposer_map, stores| { + block.ancestor_hashes[0] = Hash256::from("unknown parent block".as_bytes()); (block, attester_map, proposer_map, stores) }; @@ -177,14 +177,14 @@ fn test_block_validation_invalid_unknown_parent_hash() { ¶ms, mutator); - assert_eq!(status, Err(SszBlockValidationError::UnknownParentHash)); + assert_eq!(status, Err(SszBeaconBlockValidationError::UnknownParentHash)); } #[test] fn test_block_validation_invalid_1st_attestation_signature() { let params = get_simple_params(); - let mutator = |mut block: Block, attester_map, proposer_map, stores| { + let mutator = |mut block: BeaconBlock, attester_map, proposer_map, stores| { /* * Set the second attestaion record to have an invalid signature. */ @@ -196,7 +196,7 @@ fn test_block_validation_invalid_1st_attestation_signature() { ¶ms, mutator); - assert_eq!(status, Err(SszBlockValidationError::AttestationValidationError( + assert_eq!(status, Err(SszBeaconBlockValidationError::AttestationValidationError( AttestationValidationError::BadAggregateSignature))); } @@ -204,12 +204,15 @@ fn test_block_validation_invalid_1st_attestation_signature() { fn test_block_validation_invalid_no_parent_proposer_signature() { let params = get_simple_params(); - let mutator = |block: Block, attester_map, mut proposer_map: ProposerMap, stores: TestStore| { + let mutator = |block: BeaconBlock, attester_map, mut proposer_map: ProposerMap, stores: TestStore| { /* * Set the proposer for this slot to be a validator that does not exist. */ - let ssz = stores.block.get_serialized_block(&block.parent_hash.as_ref()).unwrap().unwrap(); - let parent_block_slot = SszBlock::from_slice(&ssz[..]).unwrap().slot_number(); + let ssz = { + let parent_hash = block.parent_hash().unwrap().as_ref(); + stores.block.get_serialized_block(parent_hash).unwrap().unwrap() + }; + let parent_block_slot = SszBeaconBlock::from_slice(&ssz[..]).unwrap().slot(); proposer_map.insert(parent_block_slot, params.total_validators + 1); (block, attester_map, proposer_map, stores) }; @@ -218,7 +221,7 @@ fn test_block_validation_invalid_no_parent_proposer_signature() { ¶ms, mutator); - assert_eq!(status, Err(SszBlockValidationError::NoProposerSignature)); + assert_eq!(status, Err(SszBeaconBlockValidationError::NoProposerSignature)); } #[test] @@ -237,14 +240,14 @@ fn test_block_validation_invalid_bad_proposer_map() { ¶ms, mutator); - assert_eq!(status, Err(SszBlockValidationError::BadProposerMap)); + assert_eq!(status, Err(SszBeaconBlockValidationError::BadProposerMap)); } #[test] fn test_block_validation_invalid_2nd_attestation_signature() { let params = get_simple_params(); - let mutator = |mut block: Block, attester_map, proposer_map, stores| { + let mutator = |mut block: BeaconBlock, attester_map, proposer_map, stores| { /* * Set the second attestaion record to have an invalid signature. */ @@ -256,6 +259,6 @@ fn test_block_validation_invalid_2nd_attestation_signature() { ¶ms, mutator); - assert_eq!(status, Err(SszBlockValidationError::AttestationValidationError( + assert_eq!(status, Err(SszBeaconBlockValidationError::AttestationValidationError( AttestationValidationError::BadAggregateSignature))); } From fa705229aaa214c5ee52e76dba8aab665d524010 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 16 Oct 2018 15:24:50 +1100 Subject: [PATCH 11/13] Fix clippy lints --- beacon_chain/types/src/special_record.rs | 2 -- .../validation/src/block_validation.rs | 26 +++++++++++++------ .../db/src/stores/beacon_block_store.rs | 5 +++- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/beacon_chain/types/src/special_record.rs b/beacon_chain/types/src/special_record.rs index 8a8ada0bd..9189728df 100644 --- a/beacon_chain/types/src/special_record.rs +++ b/beacon_chain/types/src/special_record.rs @@ -80,8 +80,6 @@ impl Decodable for SpecialRecord { } - - #[cfg(test)] mod tests { use super::*; diff --git a/beacon_chain/validation/src/block_validation.rs b/beacon_chain/validation/src/block_validation.rs index 43c7138a6..8c2c28b1a 100644 --- a/beacon_chain/validation/src/block_validation.rs +++ b/beacon_chain/validation/src/block_validation.rs @@ -186,7 +186,7 @@ impl BeaconBlockValidationContext * The presence of oblique hashes in the first attestation would indicate that the proposer * of the previous block is attesting to some other block than the one they produced. */ - if first_attestation.oblique_parent_hashes.len() > 0 { + if !first_attestation.oblique_parent_hashes.is_empty() { return Err(SszBeaconBlockValidationError::ProposerAttestationHasObliqueHashes); } @@ -274,10 +274,12 @@ impl BeaconBlockValidationContext .filter_map(|attestation_ssz| { /* * If some thread has set the `failure` variable to `Some(error)` the abandon - * attestation serialization and validation. + * attestation serialization and validation. Also, fail early if the lock has been + * poisoned. */ - if let Some(_) = *failure.read().unwrap() { - return None; + match failure.read() { + Ok(ref option) if option.is_none() => (), + _ => return None } /* * If there has not been a failure yet, attempt to serialize and validate the @@ -288,8 +290,12 @@ impl BeaconBlockValidationContext * Deserialization failed, therefore the block is invalid. */ Err(e) => { - let mut failure = failure.write().unwrap(); - *failure = Some(SszBeaconBlockValidationError::from(e)); + /* + * If the failure lock isn't poisoned, set it to some error. + */ + if let Ok(mut f) = failure.write() { + *f = Some(SszBeaconBlockValidationError::from(e)); + } None } /* @@ -301,8 +307,12 @@ impl BeaconBlockValidationContext * Attestation validation failed with some error. */ Err(e) => { - let mut failure = failure.write().unwrap(); - *failure = Some(SszBeaconBlockValidationError::from(e)); + /* + * If the failure lock isn't poisoned, set it to some error. + */ + if let Ok(mut f) = failure.write() { + *f = Some(SszBeaconBlockValidationError::from(e)); + } None } /* diff --git a/lighthouse/db/src/stores/beacon_block_store.rs b/lighthouse/db/src/stores/beacon_block_store.rs index 80c29c5a7..2caf225a0 100644 --- a/lighthouse/db/src/stores/beacon_block_store.rs +++ b/lighthouse/db/src/stores/beacon_block_store.rs @@ -10,6 +10,9 @@ use super::{ }; use super::BLOCKS_DB_COLUMN as DB_COLUMN; +type BeaconBlockHash = Vec; +type BeaconBlockSsz = Vec; + #[derive(Clone, Debug, PartialEq)] pub enum BeaconBlockAtSlotError { UnknownBeaconBlock, @@ -65,7 +68,7 @@ impl BeaconBlockStore { /// /// If a block is found, a tuple of (block_hash, serialized_block) is returned. pub fn block_at_slot(&self, head_hash: &[u8], slot: u64) - -> Result, Vec)>, BeaconBlockAtSlotError> + -> Result, BeaconBlockAtSlotError> { match self.get_serialized_block(head_hash)? { None => Err(BeaconBlockAtSlotError::UnknownBeaconBlock), From 6ee3ad10da432909a0eec3aaf40279a0de09a3b7 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Sun, 21 Oct 2018 20:07:57 +1100 Subject: [PATCH 12/13] Change integer literals to constants --- .../utils/ssz_helpers/src/ssz_beacon_block.rs | 73 ++++++++++--------- 1 file changed, 38 insertions(+), 35 deletions(-) diff --git a/beacon_chain/utils/ssz_helpers/src/ssz_beacon_block.rs b/beacon_chain/utils/ssz_helpers/src/ssz_beacon_block.rs index 7b2c2862b..d244fbb19 100644 --- a/beacon_chain/utils/ssz_helpers/src/ssz_beacon_block.rs +++ b/beacon_chain/utils/ssz_helpers/src/ssz_beacon_block.rs @@ -14,7 +14,16 @@ pub enum SszBeaconBlockError { TooLong, } -const LENGTH_BYTES: usize = 4; +/* + * Constants used for navigating the SSZ bytes. + */ +const LENGTH_PREFIX_BYTES: usize = 4; +const SLOT_BYTES: usize = 8; +const HASH_SIZE: usize = 32; +const RANDAO_REVEAL_BYTES: usize = HASH_SIZE; +const POW_CHAIN_REF_BYTES: usize = HASH_SIZE; +const ACTIVE_STATE_BYTES: usize = HASH_SIZE; +const CRYSTALLIZED_STATE_BYTES: usize = HASH_SIZE; /// Allows for reading of block values directly from serialized ssz bytes. /// @@ -74,41 +83,35 @@ impl<'a> SszBeaconBlock<'a> { * Determine how many bytes are used to store ancestor hashes. */ let ancestors_position = - 8 + // slot - 32 + // randao_reveal - 32; // pow_chain_reference - let ancestors_len = decode_length(untrimmed_ssz, ancestors_position, LENGTH_BYTES) + SLOT_BYTES + + RANDAO_REVEAL_BYTES + + POW_CHAIN_REF_BYTES; + let ancestors_len = decode_length(untrimmed_ssz, ancestors_position, LENGTH_PREFIX_BYTES) .map_err(|_| SszBeaconBlockError::TooShort)?; /* * Determine how many bytes are used to store attestation records. */ let attestations_position = - ancestors_position + LENGTH_BYTES + ancestors_len + // end of ancestor bytes - 32 + // active_state_root - 32; // crystallized_state_root - let attestations_len = decode_length(untrimmed_ssz, attestations_position, LENGTH_BYTES) + ancestors_position + LENGTH_PREFIX_BYTES + ancestors_len + // end of ancestor bytes + ACTIVE_STATE_BYTES + + CRYSTALLIZED_STATE_BYTES; + let attestations_len = decode_length(untrimmed_ssz, attestations_position, LENGTH_PREFIX_BYTES) .map_err(|_| SszBeaconBlockError::TooShort)?; /* * Determine how many bytes are used to store specials. */ - let specials_position = - attestations_position + // location of attestations - LENGTH_BYTES + // attestations length prefix - attestations_len; // # of attestation bytes - let specials_len = decode_length(untrimmed_ssz, specials_position, LENGTH_BYTES) + let specials_position = attestations_position + LENGTH_PREFIX_BYTES + attestations_len; + let specials_len = decode_length(untrimmed_ssz, specials_position, LENGTH_PREFIX_BYTES) .map_err(|_| SszBeaconBlockError::TooShort)?; /* * Now that all variable field lengths are known (ancestors, attestations, specials) we can * know the exact length of the block and reject it if the slice is too short. */ - let block_ssz_len = { - MIN_SSZ_BLOCK_LENGTH + ancestors_len + attestations_len + specials_len - }; + let block_ssz_len = MIN_SSZ_BLOCK_LENGTH + ancestors_len + attestations_len + specials_len; if vec.len() < block_ssz_len { - println!("block_ssz_len: {:?}, len: {:?}", block_ssz_len, vec.len()); return Err(SszBeaconBlockError::TooShort); } @@ -162,54 +165,54 @@ impl<'a> SszBeaconBlock<'a> { /// Return the `randao_reveal` field. pub fn randao_reveal(&self) -> &[u8] { - let start = 8; // slot is 8 bytes - &self.ssz[start..start + 32] + let start = SLOT_BYTES; + &self.ssz[start..start + RANDAO_REVEAL_BYTES] } /// Return the `pow_chain_reference` field. pub fn pow_chain_reference(&self) -> &[u8] { let start = - 8 + // slot - 32; // randao_reveal - &self.ssz[start..start + 32] + SLOT_BYTES + + RANDAO_REVEAL_BYTES; + &self.ssz[start..start + POW_CHAIN_REF_BYTES] } /// Return the serialized `ancestor_hashes` bytes, including length prefix. pub fn ancestor_hashes(&self) -> &[u8] { let start = self.ancestors_position; - &self.ssz[start..(start + self.ancestors_len + LENGTH_BYTES)] + &self.ssz[start..(start + self.ancestors_len + LENGTH_PREFIX_BYTES)] } /// Return the `active_state_root` field. pub fn act_state_root(&self) -> &[u8] { - let start = self.ancestors_position + LENGTH_BYTES + self.ancestors_len; + let start = self.ancestors_position + LENGTH_PREFIX_BYTES + self.ancestors_len; &self.ssz[start..(start + 32)] } /// Return the `active_state_root` field. pub fn cry_state_root(&self) -> &[u8] { let start = - self.ancestors_position + LENGTH_BYTES + self.ancestors_len + // ancestors - 32; // active_state_root; + self.ancestors_position + LENGTH_PREFIX_BYTES + self.ancestors_len + + ACTIVE_STATE_BYTES; &self.ssz[start..(start + 32)] } /// Return the serialized `attestations` bytes, including length prefix. pub fn attestations(&self) -> &[u8] { let start = self.attestations_position; - &self.ssz[start..(start + self.attestations_len + LENGTH_BYTES)] + &self.ssz[start..(start + self.attestations_len + LENGTH_PREFIX_BYTES)] } /// Return the serialized `attestations` bytes _without_ the length prefix. pub fn attestations_without_length(&self) -> &[u8] { - let start = self.attestations_position + LENGTH_BYTES; + let start = self.attestations_position + LENGTH_PREFIX_BYTES; &self.ssz[start..start + self.attestations_len] } /// Return the serialized `specials` bytes, including length prefix. pub fn specials(&self) -> &[u8] { let start = self.specials_position; - &self.ssz[start..(start + self.specials_len + LENGTH_BYTES)] + &self.ssz[start..(start + self.specials_len + LENGTH_PREFIX_BYTES)] } } @@ -349,7 +352,7 @@ mod tests { let serialized = get_block_ssz(&block); let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); - let mut expected = encode_length(32, LENGTH_BYTES); + let mut expected = encode_length(32, LENGTH_PREFIX_BYTES); expected.append(&mut h.to_vec()); assert_eq!(ssz_block.ancestor_hashes(), &expected[..]); @@ -383,7 +386,7 @@ mod tests { let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); let sr_ssz = get_special_record_ssz(&s); - let mut expected = encode_length(sr_ssz.len(), LENGTH_BYTES); + let mut expected = encode_length(sr_ssz.len(), LENGTH_PREFIX_BYTES); expected.append(&mut sr_ssz.to_vec()); assert_eq!(ssz_block.specials(), &expected[..]); @@ -399,7 +402,7 @@ mod tests { let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); let sr_ssz = get_special_record_ssz(&s); - let mut expected = encode_length(sr_ssz.len(), LENGTH_BYTES); + let mut expected = encode_length(sr_ssz.len(), LENGTH_PREFIX_BYTES); expected.append(&mut sr_ssz.to_vec()); assert_eq!(ssz_block.specials(), &expected[..]); @@ -417,7 +420,7 @@ mod tests { let ssz_block = SszBeaconBlock::from_slice(&serialized).unwrap(); let ssz_ar = get_attestation_record_ssz(&AttestationRecord::zero()); - let mut expected = encode_length(ssz_ar.len(), LENGTH_BYTES); + let mut expected = encode_length(ssz_ar.len(), LENGTH_PREFIX_BYTES); expected.append(&mut ssz_ar.to_vec()); assert_eq!(ssz_block.attestations(), &expected[..]); @@ -434,7 +437,7 @@ mod tests { let mut ssz_ar = get_attestation_record_ssz(&AttestationRecord::zero()); ssz_ar.append(&mut get_attestation_record_ssz(&AttestationRecord::zero())); - let mut expected = encode_length(ssz_ar.len(), LENGTH_BYTES); + let mut expected = encode_length(ssz_ar.len(), LENGTH_PREFIX_BYTES); expected.append(&mut ssz_ar.to_vec()); assert_eq!(ssz_block.attestations(), &expected[..]); From 694db90b8cdb166eff7bf26b756487c48f4d1ed7 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Sun, 21 Oct 2018 20:12:17 +1100 Subject: [PATCH 13/13] Simplify parent_hashes code --- beacon_chain/utils/ssz_helpers/src/ssz_beacon_block.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/beacon_chain/utils/ssz_helpers/src/ssz_beacon_block.rs b/beacon_chain/utils/ssz_helpers/src/ssz_beacon_block.rs index d244fbb19..3066f5679 100644 --- a/beacon_chain/utils/ssz_helpers/src/ssz_beacon_block.rs +++ b/beacon_chain/utils/ssz_helpers/src/ssz_beacon_block.rs @@ -138,12 +138,8 @@ impl<'a> SszBeaconBlock<'a> { /// The first hash in `ancestor_hashes` is the parent of the block. pub fn parent_hash(&self) -> Option<&[u8]> { let ancestor_ssz = self.ancestor_hashes(); - let start = LENGTH_BYTES; - if ancestor_ssz.len() >= 32 { - Some(&ancestor_ssz[start..start + 32]) - } else { - None - } + let start = LENGTH_PREFIX_BYTES; + ancestor_ssz.get(start..start + HASH_SIZE) } /// Return the `slot` field.