Make the block header struct's internals private (#2000)

* Make the block header struct's internals private

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.

* Drop some unnecessary cloning by comparing references

* Fix compiler errors from callsites in tests.
This commit is contained in:
Nipunn Koorapati
2016-08-29 02:35:24 -07:00
committed by Arkadiy Paronyan
parent 2d883c43c9
commit 4389742ca3
21 changed files with 336 additions and 336 deletions

View File

@@ -82,17 +82,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);
}
@@ -123,9 +122,9 @@ 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(())
@@ -133,7 +132,7 @@ impl Engine for BasicAuthority {
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 = public_to_address(&try!(recover(&sig.into(), &header.bare_hash())));
if !self.our_params.authorities.contains(&signer) {
return try!(Err(BlockError::InvalidSeal));
@@ -152,10 +151,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() <= &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().clone() })));
}
Ok(())
}

View File

@@ -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