-
Notifications
You must be signed in to change notification settings - Fork 1.7k
ethcore: cleanup after #11531 #11546
Conversation
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.
I think I understand what the idea here is: to preserve the type safety and impede calls to enact()
with unverified data. The price for that is yet another block type and somewhat less readable code.
I am biased and think that the more important property here should be readability. I think the type safety you’re after here is not 100% waterproof (constructing a PreverifiedBlock
to pass in is still possible I think) so I’m not sure it’s worth it.
That said, if other feel this is better I’m fine merging it.
One idea I had was to look at separating the RLP from the block type higher up in the call graph, at the queue level. Maybe that’s a more natura separation for the data? So draining from the queue would yield a tuple of rlp and a pre verified block struct?
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.
lgtm, minor nits.
@@ -296,7 +296,7 @@ impl Importer { | |||
trace_time!("import_verified_blocks"); | |||
let start = Instant::now(); | |||
|
|||
for mut block in blocks { | |||
for (block, block_bytes) in blocks { |
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.
I like this, it makes it quite obvious what is going on.
* master: Code cleanup in the sync module (#11552) initial cleanup (#11542) Warn if genesis constructor revert (#11550) ethcore: cleanup after #11531 (#11546) license update (#11543) Less cloning when importing blocks (#11531) Github Actions (#11528) Fix Alpine Dockerfile (#11538) Remove AuxiliaryData/AuxiliaryRequest (#11533) [journaldb]: cleanup (#11534) Remove references to parity-ethereum (#11525) Drop IPFS support (#11532) chain-supplier: fix warning reporting for GetNodeData request (#11530) Faster kill_garbage (#11514) [EngineSigner]: don't sign message with only zeroes (#11524)
Bikeshed on naming is welcome!