Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Commit

Permalink
Make the block header struct's internals private
Browse files Browse the repository at this point in the history 
Currently, this involves a lot of explicit cloning, but we
could migrate the return types of the get_* functions to
be copies rather than references since they are mostly copy
types anyway.

I opted to eliminate the constructor in favor of using
Default::default() plus calling a bunch of setters. This
is similar to the model that a Google Protobuf client uses
and I think it looks fine.
  • Loading branch information
@nipunn1313
nipunn1313 committed Aug 24, 2016
1 parent 124a5da commit 299360e
Show file tree
Hide file tree
Showing 15 changed files with 263 additions and 267 deletions.
50 changes: 24 additions & 26 deletions ethcore/src/block.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,11 +248,11 @@ impl<'x> OpenBlock<'x> {
last_hashes: last_hashes,
};

r.block.base.header.parent_hash = parent.hash();
r.block.base.header.number = parent.number + 1;
r.block.base.header.author = author;
r.block.base.header.set_parent_hash(parent.hash());
r.block.base.header.set_number(parent.number() + 1);
r.block.base.header.set_author(author);
r.block.base.header.set_timestamp_now(parent.timestamp());
r.block.base.header.extra_data = extra_data;
r.block.base.header.set_extra_data(extra_data);
r.block.base.header.note_dirty();

engine.populate_from_parent(&mut r.block.base.header, parent, gas_range_target.0, gas_range_target.1);
Expand Down Expand Up @@ -312,13 +312,13 @@ impl<'x> OpenBlock<'x> {
pub fn env_info(&self) -> EnvInfo {
// TODO: memoise.
EnvInfo {
number: self.block.base.header.number,
author: self.block.base.header.author.clone(),
timestamp: self.block.base.header.timestamp,
difficulty: self.block.base.header.difficulty.clone(),
number: self.block.base.header.number(),
author: self.block.base.header.author().clone(),
timestamp: self.block.base.header.timestamp(),
difficulty: self.block.base.header.difficulty().clone(),
last_hashes: self.last_hashes.clone(),
gas_used: self.block.receipts.last().map_or(U256::zero(), |r| r.gas_used),
gas_limit: self.block.base.header.gas_limit.clone(),
gas_limit: self.block.base.header.gas_limit().clone(),
}
}

Expand Down Expand Up @@ -352,14 +352,13 @@ impl<'x> OpenBlock<'x> {
let unclosed_state = s.block.state.clone();

s.engine.on_close_block(&mut s.block);
s.block.base.header.transactions_root = ordered_trie_root(s.block.base.transactions.iter().map(|e| e.rlp_bytes().to_vec()).collect());
s.block.base.header.set_transactions_root(ordered_trie_root(s.block.base.transactions.iter().map(|e| e.rlp_bytes().to_vec()).collect()));
let uncle_bytes = s.block.base.uncles.iter().fold(RlpStream::new_list(s.block.base.uncles.len()), |mut s, u| {s.append_raw(&u.rlp(Seal::With), 1); s} ).out();
s.block.base.header.uncles_hash = uncle_bytes.sha3();
s.block.base.header.state_root = s.block.state.root().clone();
s.block.base.header.receipts_root = ordered_trie_root(s.block.receipts.iter().map(|r| r.rlp_bytes().to_vec()).collect());
s.block.base.header.log_bloom = s.block.receipts.iter().fold(LogBloom::zero(), |mut b, r| {b = &b | &r.log_bloom; b}); //TODO: use |= operator
s.block.base.header.gas_used = s.block.receipts.last().map_or(U256::zero(), |r| r.gas_used);
s.block.base.header.note_dirty();
s.block.base.header.set_uncles_hash(uncle_bytes.sha3());
s.block.base.header.set_state_root(s.block.state.root().clone());
s.block.base.header.set_receipts_root(ordered_trie_root(s.block.receipts.iter().map(|r| r.rlp_bytes().to_vec()).collect()));
s.block.base.header.set_log_bloom(s.block.receipts.iter().fold(LogBloom::zero(), |mut b, r| {b = &b | &r.log_bloom; b})); //TODO: use |= operator
s.block.base.header.set_gas_used(s.block.receipts.last().map_or(U256::zero(), |r| r.gas_used));

ClosedBlock {
block: s.block,
Expand All @@ -374,20 +373,19 @@ impl<'x> OpenBlock<'x> {
let mut s = self;

s.engine.on_close_block(&mut s.block);
if s.block.base.header.transactions_root.is_zero() || s.block.base.header.transactions_root == SHA3_NULL_RLP {
s.block.base.header.transactions_root = ordered_trie_root(s.block.base.transactions.iter().map(|e| e.rlp_bytes().to_vec()).collect());
if s.block.base.header.transactions_root().is_zero() || s.block.base.header.transactions_root().clone() == SHA3_NULL_RLP {
s.block.base.header.set_transactions_root(ordered_trie_root(s.block.base.transactions.iter().map(|e| e.rlp_bytes().to_vec()).collect()));
}
let uncle_bytes = s.block.base.uncles.iter().fold(RlpStream::new_list(s.block.base.uncles.len()), |mut s, u| {s.append_raw(&u.rlp(Seal::With), 1); s} ).out();
if s.block.base.header.uncles_hash.is_zero() {
s.block.base.header.uncles_hash = uncle_bytes.sha3();
if s.block.base.header.uncles_hash().is_zero() {
s.block.base.header.set_uncles_hash(uncle_bytes.sha3());
}
if s.block.base.header.receipts_root.is_zero() || s.block.base.header.receipts_root == SHA3_NULL_RLP {
s.block.base.header.receipts_root = ordered_trie_root(s.block.receipts.iter().map(|r| r.rlp_bytes().to_vec()).collect());
if s.block.base.header.receipts_root().is_zero() || s.block.base.header.receipts_root().clone() == SHA3_NULL_RLP {
s.block.base.header.set_receipts_root(ordered_trie_root(s.block.receipts.iter().map(|r| r.rlp_bytes().to_vec()).collect()));
}
s.block.base.header.state_root = s.block.state.root().clone();
s.block.base.header.log_bloom = s.block.receipts.iter().fold(LogBloom::zero(), |mut b, r| {b = &b | &r.log_bloom; b}); //TODO: use |= operator
s.block.base.header.gas_used = s.block.receipts.last().map_or(U256::zero(), |r| r.gas_used);
s.block.base.header.note_dirty();
s.block.base.header.set_state_root(s.block.state.root().clone());
s.block.base.header.set_log_bloom(s.block.receipts.iter().fold(LogBloom::zero(), |mut b, r| {b = &b | &r.log_bloom; b})); //TODO: use |= operator
s.block.base.header.set_gas_used(s.block.receipts.last().map_or(U256::zero(), |r| r.gas_used));

LockedBlock {
block: s.block,
Expand Down
6 changes: 3 additions & 3 deletions ethcore/src/block_queue.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ impl BlockQueue {
fn drain_verifying(verifying: &mut VecDeque<VerifyingBlock>, verified: &mut VecDeque<PreverifiedBlock>, bad: &mut HashSet<H256>) {
while !verifying.is_empty() && verifying.front().unwrap().block.is_some() {
let block = verifying.pop_front().unwrap().block.unwrap();
if bad.contains(&block.header.parent_hash) {
if bad.contains(block.header.parent_hash()) {
bad.insert(block.header.hash());
}
else {
Expand Down Expand Up @@ -313,7 +313,7 @@ impl BlockQueue {
return Err(ImportError::KnownBad.into());
}

if bad.contains(&header.parent_hash) {
if bad.contains(header.parent_hash()) {
bad.insert(h.clone());
return Err(ImportError::KnownBad.into());
}
Expand Down Expand Up @@ -351,7 +351,7 @@ impl BlockQueue {

let mut new_verified = VecDeque::new();
for block in verified.drain(..) {
if bad.contains(&block.header.parent_hash) {
if bad.contains(block.header.parent_hash()) {
bad.insert(block.header.hash());
processing.remove(&block.header.hash());
} else {
Expand Down
10 changes: 5 additions & 5 deletions ethcore/src/client/client.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ impl ClientReport {
pub fn accrue_block(&mut self, block: &PreverifiedBlock) {
self.blocks_imported += 1;
self.transactions_applied += block.transactions.len();
self.gas_processed = self.gas_processed + block.header.gas_used;
self.gas_processed = self.gas_processed + block.header.gas_used().clone();
}
}

Expand Down Expand Up @@ -278,15 +278,15 @@ impl Client {
};

// Check if Parent is in chain
let chain_has_parent = self.chain.block_header(&header.parent_hash);
let chain_has_parent = self.chain.block_header(header.parent_hash());
if let None = chain_has_parent {
warn!(target: "client", "Block import failed for #{} ({}): Parent not found ({}) ", header.number(), header.hash(), header.parent_hash);
warn!(target: "client", "Block import failed for #{} ({}): Parent not found ({}) ", header.number(), header.hash(), header.parent_hash());
return Err(());
};

// Enact Verified Block
let parent = chain_has_parent.unwrap();
let last_hashes = self.build_last_hashes(header.parent_hash.clone());
let last_hashes = self.build_last_hashes(header.parent_hash().clone());
let db = self.state_db.lock().boxed_clone();

let enact_result = enact_verified(block, engine, self.tracedb.tracing_enabled(), db, &parent, last_hashes, &self.vm_factory, self.trie_factory.clone());
Expand Down Expand Up @@ -346,7 +346,7 @@ impl Client {

for block in blocks {
let header = &block.header;
if invalid_blocks.contains(&header.parent_hash) {
if invalid_blocks.contains(header.parent_hash()) {
invalid_blocks.insert(header.hash());
continue;
}
Expand Down
34 changes: 17 additions & 17 deletions ethcore/src/client/test_client.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,19 +168,19 @@ impl TestBlockChainClient {
let len = self.numbers.read().len();
for n in len..(len + count) {
let mut header = BlockHeader::new();
header.difficulty = From::from(n);
header.parent_hash = self.last_hash.read().clone();
header.number = n as BlockNumber;
header.gas_limit = U256::from(1_000_000);
header.set_difficulty(From::from(n));
header.set_parent_hash(self.last_hash.read().clone());
header.set_number(n as BlockNumber);
header.set_gas_limit(U256::from(1_000_000));
let uncles = match with {
EachBlockWith::Uncle | EachBlockWith::UncleAndTransaction => {
let mut uncles = RlpStream::new_list(1);
let mut uncle_header = BlockHeader::new();
uncle_header.difficulty = From::from(n);
uncle_header.parent_hash = self.last_hash.read().clone();
uncle_header.number = n as BlockNumber;
uncle_header.set_difficulty(From::from(n));
uncle_header.set_parent_hash(self.last_hash.read().clone());
uncle_header.set_number(n as BlockNumber);
uncles.append(&uncle_header);
header.uncles_hash = uncles.as_raw().sha3();
header.set_uncles_hash(uncles.as_raw().sha3());
uncles
},
_ => RlpStream::new_list(0)
Expand Down Expand Up @@ -218,7 +218,7 @@ impl TestBlockChainClient {
pub fn corrupt_block(&mut self, n: BlockNumber) {
let hash = self.block_hash(BlockID::Number(n)).unwrap();
let mut header: BlockHeader = decode(&self.block_header(BlockID::Number(n)).unwrap());
header.extra_data = b"This extra data is way too long to be considered valid".to_vec();
header.set_extra_data(b"This extra data is way too long to be considered valid".to_vec());
let mut rlp = RlpStream::new_list(3);
rlp.append(&header);
rlp.append_raw(&rlp::NULL_RLP, 1);
Expand All @@ -230,7 +230,7 @@ impl TestBlockChainClient {
pub fn corrupt_block_parent(&mut self, n: BlockNumber) {
let hash = self.block_hash(BlockID::Number(n)).unwrap();
let mut header: BlockHeader = decode(&self.block_header(BlockID::Number(n)).unwrap());
header.parent_hash = H256::from(42);
header.set_parent_hash(H256::from(42));
let mut rlp = RlpStream::new_list(3);
rlp.append(&header);
rlp.append_raw(&rlp::NULL_RLP, 1);
Expand Down Expand Up @@ -470,39 +470,39 @@ impl BlockChainClient for TestBlockChainClient {
fn import_block(&self, b: Bytes) -> Result<H256, BlockImportError> {
let header = Rlp::new(&b).val_at::<BlockHeader>(0);
let h = header.hash();
let number: usize = header.number as usize;
let number: usize = header.number() as usize;
if number > self.blocks.read().len() {
panic!("Unexpected block number. Expected {}, got {}", self.blocks.read().len(), number);
}
if number > 0 {
match self.blocks.read().get(&header.parent_hash) {
match self.blocks.read().get(header.parent_hash()) {
Some(parent) => {
let parent = Rlp::new(parent).val_at::<BlockHeader>(0);
if parent.number != (header.number - 1) {
if parent.number() != (header.number() - 1) {
panic!("Unexpected block parent");
}
},
None => {
panic!("Unknown block parent {:?} for block {}", header.parent_hash, number);
panic!("Unknown block parent {:?} for block {}", header.parent_hash(), number);
}
}
}
let len = self.numbers.read().len();
if number == len {
{
let mut difficulty = self.difficulty.write();
*difficulty = *difficulty + header.difficulty;
*difficulty = *difficulty + header.difficulty().clone();
}
mem::replace(&mut *self.last_hash.write(), h.clone());
self.blocks.write().insert(h.clone(), b);
self.numbers.write().insert(number, h.clone());
let mut parent_hash = header.parent_hash;
let mut parent_hash = header.parent_hash().clone();
if number > 0 {
let mut n = number - 1;
while n > 0 && self.numbers.read()[&n] != parent_hash {
*self.numbers.write().get_mut(&n).unwrap() = parent_hash.clone();
n -= 1;
parent_hash = Rlp::new(&self.blocks.read()[&parent_hash]).val_at::<BlockHeader>(0).parent_hash;
parent_hash = Rlp::new(&self.blocks.read()[&parent_hash]).val_at::<BlockHeader>(0).parent_hash().clone();
}
}
}
Expand Down
23 changes: 11 additions & 12 deletions ethcore/src/engines/basic_authority.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,17 +81,16 @@ impl Engine for BasicAuthority {
}

fn populate_from_parent(&self, header: &mut Header, parent: &Header, gas_floor_target: U256, _gas_ceil_target: U256) {
header.difficulty = parent.difficulty;
header.gas_limit = {
let gas_limit = parent.gas_limit;
header.set_difficulty(parent.difficulty().clone());
header.set_gas_limit({
let gas_limit = parent.gas_limit().clone();
let bound_divisor = self.our_params.gas_limit_bound_divisor;
if gas_limit < gas_floor_target {
min(gas_floor_target, gas_limit + gas_limit / bound_divisor - 1.into())
} else {
max(gas_floor_target, gas_limit - gas_limit / bound_divisor + 1.into())
}
};
header.note_dirty();
});
// info!("ethash: populate_from_parent #{}: difficulty={} and gas_limit={}", header.number, header.difficulty, header.gas_limit);
}

Expand Down Expand Up @@ -122,17 +121,17 @@ impl Engine for BasicAuthority {
fn verify_block_basic(&self, header: &Header, _block: Option<&[u8]>) -> result::Result<(), Error> {
// check the seal fields.
// TODO: pull this out into common code.
if header.seal.len() != self.seal_fields() {
if header.seal().len() != self.seal_fields() {
return Err(From::from(BlockError::InvalidSealArity(
Mismatch { expected: self.seal_fields(), found: header.seal.len() }
Mismatch { expected: self.seal_fields(), found: header.seal().len() }
)));
}
Ok(())
}

fn verify_block_unordered(&self, header: &Header, _block: Option<&[u8]>) -> result::Result<(), Error> {
// check the signature is legit.
let sig = try!(UntrustedRlp::new(&header.seal[0]).as_val::<H520>());
let sig = try!(UntrustedRlp::new(&header.seal()[0]).as_val::<H520>());
let signer = Address::from(try!(ec::recover(&sig, &header.bare_hash())).sha3());
if !self.our_params.authorities.contains(&signer) {
return try!(Err(BlockError::InvalidSeal));
Expand All @@ -151,10 +150,10 @@ impl Engine for BasicAuthority {
return Err(From::from(BlockError::InvalidDifficulty(Mismatch { expected: *parent.difficulty(), found: *header.difficulty() })))
}
let gas_limit_divisor = self.our_params.gas_limit_bound_divisor;
let min_gas = parent.gas_limit - parent.gas_limit / gas_limit_divisor;
let max_gas = parent.gas_limit + parent.gas_limit / gas_limit_divisor;
if header.gas_limit <= min_gas || header.gas_limit >= max_gas {
return Err(From::from(BlockError::InvalidGasLimit(OutOfBounds { min: Some(min_gas), max: Some(max_gas), found: header.gas_limit })));
let min_gas = parent.gas_limit().clone() - parent.gas_limit().clone() / gas_limit_divisor;
let max_gas = parent.gas_limit().clone() + parent.gas_limit().clone() / gas_limit_divisor;
if header.gas_limit().clone() <= min_gas || header.gas_limit().clone() >= max_gas {
return Err(From::from(BlockError::InvalidGasLimit(OutOfBounds { min: Some(min_gas), max: Some(max_gas), found: header.gas_limit().clone() })));
}
Ok(())
}
Expand Down
5 changes: 2 additions & 3 deletions ethcore/src/engines/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,8 @@ pub trait Engine : Sync + Send {
/// Don't forget to call Super::populate_from_parent when subclassing & overriding.
// TODO: consider including State in the params.
fn populate_from_parent(&self, header: &mut Header, parent: &Header, _gas_floor_target: U256, _gas_ceil_target: U256) {
header.difficulty = parent.difficulty;
header.gas_limit = parent.gas_limit;
header.note_dirty();
header.set_difficulty(parent.difficulty().clone());
header.set_gas_limit(parent.gas_limit().clone());
}

// TODO: builtin contract routing - to do this properly, it will require removing the built-in configuration-reading logic
Expand Down
Loading

0 comments on commit 299360e

Please sign in to comment.