Replace all Rlp usages with UntrustedRlp except for ethcore views #8233
Conversation
All Rlp methods return Result<_,DecoderError> now, so for this first
pass each will be marked with `expect("TODO")`. In the next pass we can
categorise figure out how to handle each case.
Rlp should be valid since created manually in tests
This can be done again in the next phase, in order that we can leave the ethcore views unchanged
Use legacy Rlp for ethcore views. Will redo replacing Rlp with UntrustedRlp in a subsequent PR
|
It looks like @ascjones signed our Contributor License Agreement. 👍 Many thanks, Parity Technologies CLA Bot |
| @@ -74,6 +74,38 @@ pub struct BlockDescriptor { | |||
| pub total_difficulty: U256, | |||
| } | |||
|
|
|||
| // best block data | |||
| struct BestAndLatest { | |||
There was a problem hiding this comment.
handwritten implementations of Encodable and Decodable can be replaced with
#[derive(RlpEncodable, RlpDecodable)]
struct {
// ..
}
ethcore/src/client/test_client.rs
Outdated
| @@ -469,7 +469,7 @@ impl ChainInfo for TestBlockChainClient { | |||
| impl BlockInfo for TestBlockChainClient { | |||
| fn block_header(&self, id: BlockId) -> Option<encoded::Header> { | |||
| self.block_hash(id) | |||
| .and_then(|hash| self.blocks.read().get(&hash).map(|r| Rlp::new(r).at(0).as_raw().to_vec())) | |||
| .and_then(|hash| self.blocks.read().get(&hash).map(|r| UntrustedRlp::new(r).at(0).unwrap().as_raw().to_vec())) | |||
There was a problem hiding this comment.
We fought hard to get rid of unwraps() in the code base and this PR intorudces it back, can we please replace them with expects()?
There was a problem hiding this comment.
Yes sorry that unwrap snuck in. Removed.
There was a problem hiding this comment.
There seems to be way more than that one. It's fine in tests code, but would avoid it in regular code.
There was a problem hiding this comment.
I've removed the unwraps I added in this PR. There are plenty in there still from before - like you say probably okay for tests. Which is why I was lazy in this case and just added them. Broken windows and all!
sync/src/blocks.rs
Outdated
| @@ -293,9 +294,9 @@ impl BlockCollection { | |||
| let mut block_rlp = RlpStream::new_list(3); | |||
| block_rlp.append_raw(&block.header, 1); | |||
There was a problem hiding this comment.
new_from_header_and_body()?
There was a problem hiding this comment.
Good idea. See latest commit.
tomusdrw
added
A8-looksgood 🦄
Part of #8033, replacing Rlp with UntrustedRlp.
Now everywhere (except ethcore views) are using
UntrustedRlpand handling theResult<_,DecoderError>.This paves the way for the next phase, which would be to replace Rlp with UntrustedRlp in the ethcore views. Once that is done we can remove the legacy Rlp implementation.