Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 55 additions & 34 deletions core/client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use state_machine::{
DBValue, Backend as StateBackend, CodeExecutor, ChangesTrieAnchorBlockId,
ExecutionStrategy, ExecutionManager, prove_read,
ChangesTrieRootsStorage, ChangesTrieStorage,
key_changes, key_changes_proof, OverlayedChanges
key_changes, key_changes_proof, OverlayedChanges,
};

use crate::backend::{self, BlockImportOperation};
Expand All @@ -66,6 +66,9 @@ pub type ImportNotifications<Block> = mpsc::UnboundedReceiver<BlockImportNotific
/// 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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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;

type ChangesUpdate = trie::MemoryDB<Blake2Hasher>;

/// Substrate Client
pub struct Client<B, E, Block, RA> where Block: BlockT {
backend: Arc<B>,
Expand Down Expand Up @@ -588,39 +591,9 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
}

let mut transaction = self.backend.begin_operation(BlockId::Hash(parent_hash))?;
let (storage_update, changes_update, storage_changes) = match transaction.state()? {
Some(transaction_state) => {
let mut overlay = Default::default();
let r = self.executor.call_at_state(
transaction_state,
&mut overlay,
"Core_execute_block",
&<Block as BlockT>::new(import_headers.pre().clone(), body.clone().unwrap_or_default()).encode(),
match (origin, self.block_execution_strategy) {
(BlockOrigin::NetworkInitialSync, _) | (_, ExecutionStrategy::NativeWhenPossible) =>
ExecutionManager::NativeWhenPossible,
(_, ExecutionStrategy::AlwaysWasm) => ExecutionManager::AlwaysWasm,
_ => ExecutionManager::Both(|wasm_result, native_result| {
let header = import_headers.post();
warn!("Consensus error between wasm and native block execution at block {}", hash);
warn!(" Header {:?}", header);
warn!(" Native result {:?}", native_result);
warn!(" Wasm result {:?}", wasm_result);
telemetry!("block.execute.consensus_failure";
"hash" => ?hash,
"origin" => ?origin,
"header" => ?header
);
wasm_result
}),
},
);
let (_, storage_update, changes_update) = r?;
overlay.commit_prospective();
(Some(storage_update), Some(changes_update), Some(overlay.into_committed().collect()))
},
None => (None, None, None)
};

// TODO: correct path logic
Copy link
Member

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.

let (storage_update,changes_update,storage_changes) = self.block_execution(&import_headers, origin, hash, body.clone(), &transaction)?;

// TODO: non longest-chain rule.
let is_new_best = finalized || match fork_choice {
Expand Down Expand Up @@ -691,6 +664,54 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
Ok(ImportResult::Queued)
}

fn block_execution(
&self,
import_headers: &PrePostHeader<Block::Header>,
origin: BlockOrigin,
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
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved

E: CallExecutor<Block, Blake2Hasher> + Send + Sync + Clone,
{
match transaction.state()? {
Some(transaction_state) => {
let mut overlay = Default::default();
let mut r = self.executor.call_at_state(
transaction_state,
&mut overlay,
"Core_execute_block",
&<Block as BlockT>::new(import_headers.pre().clone(), body.clone().unwrap_or_default()).encode(),
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, will fix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved

Copy link
Member

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.

match (origin, self.block_execution_strategy) {
(BlockOrigin::NetworkInitialSync, _) | (_, ExecutionStrategy::NativeWhenPossible) =>
ExecutionManager::NativeWhenPossible,
(_, ExecutionStrategy::AlwaysWasm) => ExecutionManager::AlwaysWasm,
_ => ExecutionManager::Both(|wasm_result, native_result| {
let header = import_headers.post();
warn!("Consensus error between wasm and native block execution at block {}", hash);
warn!(" Header {:?}", header);
warn!(" Native result {:?}", native_result);
warn!(" Wasm result {:?}", wasm_result);
telemetry!("block.execute.consensus_failure";
"hash" => ?hash,
"origin" => ?origin,
"header" => ?header
);
wasm_result
}),
},
);
let (_, storage_update, changes_update) = r?;
overlay.commit_prospective();

Ok((Some(storage_update), Some(changes_update), Some(overlay.into_committed().collect())))
},

None => Ok((None, None, None))
}

}

/// Finalizes all blocks up to given. If a justification is provided it is
/// stored with the given finalized block (any other finalized blocks are
/// left unjustified).
Expand Down