-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Split execute_and_import_block in client into two functions #1455
Conversation
|
It looks like @joepetrowski signed our Contributor License Agreement. 👍 Many thanks, Parity Technologies CLA Bot |
core/client/src/client.rs
Outdated
| hash: Block::Hash, | ||
| body: Option<Vec<Block::Extrinsic>>, | ||
| transaction: &B::BlockImportOperation, | ||
| ) -> error::Result<(Option<StorageUpdate<B, Block>>, Option<Option<ChangesUpdate>>, Option<Vec<(Vec<u8>, Option<Vec<u8>>)>>)> where |
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.
style: could split the tuple args each onto their own line with another level of indentation
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.
resolved
core/client/src/client.rs
Outdated
| transaction_state, | ||
| &mut overlay, | ||
| "Core_execute_block", | ||
| &<Block as BlockT>::new(import_headers.pre().clone(), body.clone().unwrap_or_default()).encode(), |
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 don't think body needs to be cloned here.
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.
Good catch, will fix
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.
resolved
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 we should open an issue for that. I would like to see some function Block::encode_from_parts(header: &Header, ext: &Vec<Extrensics>) or something. This could be used here and would not require to clone the stuff.
core/client/src/client.rs
Outdated
| None => (None, None, None) | ||
| }; | ||
|
|
||
| // TODO: correct path logic |
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.
Every TODO should be a github issue and the link should be added to the comment.
core/client/src/client.rs
Outdated
| /// A stream of block finality notifications. | ||
| pub type FinalityNotifications<Block> = mpsc::UnboundedReceiver<FinalityNotification<Block>>; | ||
|
|
||
| type StorageUpdate<B, Block> = <<<B as backend::Backend<Block, Blake2Hasher>>::BlockImportOperation as backend::BlockImportOperation<Block, Blake2Hasher>>::State as state_machine::Backend<Blake2Hasher>>::Transaction; |
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.
| type StorageUpdate<B, Block> = <<<B as backend::Backend<Block, Blake2Hasher>>::BlockImportOperation as backend::BlockImportOperation<Block, Blake2Hasher>>::State as state_machine::Backend<Blake2Hasher>>::Transaction; | |
| type StorageUpdate<B, Block> = <<<B as backend::Backend<Block, Blake2Hasher>>::BlockImportOperation as BlockImportOperation<Block, Blake2Hasher>>::State as state_machine::Backend<Blake2Hasher>>::Transaction; |
core/client/src/client.rs
Outdated
| transaction_state, | ||
| &mut overlay, | ||
| "Core_execute_block", | ||
| &<Block as BlockT>::new(import_headers.pre().clone(), body.clone().unwrap_or_default()).encode(), |
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 we should open an issue for that. I would like to see some function Block::encode_from_parts(header: &Header, ext: &Vec<Extrensics>) or something. This could be used here and would not require to clone the stuff.
…ch#1455) * type error * borrow problem * compiles but run error * tests pass ready for review * tests pass ready for review * fix comments by rob * removed clone * style fix * updating * resolved conflicts * fixed comments
Reduce slot time back to 1 second
This is step one in addressing Issue #1232
I have taken block execution from
execute_and_import_blockand put it into a new functionblock_execution, which returns a tuple of(storage_update,changes_update,storage_changes). The function always executes, so behavior should be exactly as before.