-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Make the block header struct's internals private #2000
Conversation
299360e
to
d05a504
Compare
Bam Pull Request number 2000! |
|
||
// TODO: consider removing these lines. | ||
let min_difficulty = self.ethash_params.minimum_difficulty; | ||
if header.difficulty < min_difficulty { | ||
return Err(From::from(BlockError::DifficultyOutOfBounds(OutOfBounds { min: Some(min_difficulty), max: None, found: header.difficulty }))) | ||
if header.difficulty().clone() < min_difficulty { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be able to just do if header.difficulty() < &min_difficulty
a lot of the |
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.
2db05c7
to
bb63b1c
Compare
bb63b1c
to
8678e5e
Compare
69aa867
to
18c36f7
Compare
Great point. I went through and found places where we could eliminate clone() by comparing references. Unfortunately, it seems like you can't compare a &H256 with an H256 directly because of rust-lang/rfcs#1332, but by making both sides references, it compiles nicely as @rphmeier suggested. I also went through and fixed up the tests and tried to minimize explicit cloning. Any time + or * is used for math, I had to do some cloning. I think the llvm compiler will do a good job of optimizing those situations. |
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.
This begins work on #1267