Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Never panic during execution proof check #3504

Merged
merged 20 commits into from
Sep 11, 2019
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions core/client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ env_logger = "0.6"
tempfile = "3.1"
test-client = { package = "substrate-test-runtime-client", path = "../test-runtime/client" }
kvdb-memorydb = { git = "https://github.com/paritytech/parity-common", rev="b0317f649ab2c665b7987b8475878fc4d2e1f81d" }
panic-handler = { package = "substrate-panic-handler", path = "../panic-handler" }

[features]
default = ["std"]
Expand Down
3 changes: 2 additions & 1 deletion core/client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ use state_machine::{
ExecutionStrategy, ExecutionManager, prove_read, prove_child_read,
ChangesTrieRootsStorage, ChangesTrieStorage, ChangesTrieConfigurationRange,
key_changes, key_changes_proof, OverlayedChanges, NeverOffchainExt,
BackendTrustLevel,
};
use executor::{RuntimeVersion, RuntimeInfo};
use consensus::{
Expand Down Expand Up @@ -1051,7 +1052,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
let get_execution_manager = |execution_strategy: ExecutionStrategy| {
match execution_strategy {
ExecutionStrategy::NativeElseWasm => ExecutionManager::NativeElseWasm,
ExecutionStrategy::AlwaysWasm => ExecutionManager::AlwaysWasm,
ExecutionStrategy::AlwaysWasm => ExecutionManager::AlwaysWasm(BackendTrustLevel::Trusted),
ExecutionStrategy::NativeWhenPossible => ExecutionManager::NativeWhenPossible,
ExecutionStrategy::Both => ExecutionManager::Both(|wasm_result, native_result| {
let header = import_headers.post();
Expand Down
84 changes: 75 additions & 9 deletions core/client/src/light/call_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,26 +459,46 @@ pub fn check_execution_proof<Header, E, H>(
E: CodeExecutor<H>,
H: Hasher,
H::Out: Ord + 'static,
{
check_execution_proof_with_make_header(
executor,
request,
remote_proof,
|header| <Header as HeaderT>::new(
*header.number() + One::one(),
Default::default(),
Default::default(),
header.hash(),
Default::default(),
),
)
}

fn check_execution_proof_with_make_header<Header, E, H, MakeNextHeader: Fn(&Header) -> Header>(
executor: &E,
request: &RemoteCallRequest<Header>,
remote_proof: Vec<Vec<u8>>,
make_next_header: MakeNextHeader,
) -> ClientResult<Vec<u8>>
where
Header: HeaderT,
E: CodeExecutor<H>,
H: Hasher,
H::Out: Ord + 'static,
{
let local_state_root = request.header.state_root();
let root: H::Out = convert_hash(&local_state_root);

// prepare execution environment + check preparation proof
let mut changes = OverlayedChanges::default();
let trie_backend = create_proof_check_backend(root, remote_proof)?;
let next_block = <Header as HeaderT>::new(
*request.header.number() + One::one(),
Default::default(),
Default::default(),
request.header.hash(),
Default::default(),
);
let next_header = make_next_header(&request.header);
execution_proof_check_on_trie_backend::<H, _>(
&trie_backend,
&mut changes,
executor,
"Core_initialize_block",
&next_block.encode(),
&next_header.encode(),
None,
)?;

Expand Down Expand Up @@ -531,6 +551,43 @@ mod tests {
(remote_result, local_result)
}

fn execute_with_proof_failure(remote_client: &TestClient, at: u64, method: &'static str) {
let remote_block_id = BlockId::Number(at);
let remote_header = remote_client.header(&remote_block_id).unwrap().unwrap();

// 'fetch' execution proof from remote node
let (_, remote_execution_proof) = remote_client.execution_proof(
&remote_block_id,
method,
&[]
).unwrap();

// check remote execution proof locally
let local_executor = NativeExecutor::<test_client::LocalExecutor>::new(None);
let execution_result = check_execution_proof_with_make_header(
&local_executor,
&RemoteCallRequest {
block: test_client::runtime::Hash::default(),
header: remote_header,
method: method.into(),
call_data: vec![],
retry_count: None,
},
remote_execution_proof,
|header| <Header as HeaderT>::new(
at + 1,
Default::default(),
Default::default(),
header.hash(),
header.digest().clone(), // this makes next header wrong
),
);
match execution_result {
Err(crate::error::Error::Execution(_)) => (),
_ => panic!("Unexpected execution result: {:?}", execution_result),
}
}

// prepare remote client
let remote_client = test_client::new();
for i in 1u32..3u32 {
Expand All @@ -547,15 +604,24 @@ mod tests {
let (remote, local) = execute(&remote_client, 0, "Core_version");
assert_eq!(remote, local);

let (remote, local) = execute(&remote_client, 2, "Core_version");
assert_eq!(remote, local);

// check method that requires environment
let (_, block) = execute(&remote_client, 0, "BlockBuilder_finalize_block");
let local_block: Header = Decode::decode(&mut &block[..]).unwrap();
assert_eq!(local_block.number, 1);

// check method that requires environment
let (_, block) = execute(&remote_client, 2, "BlockBuilder_finalize_block");
let local_block: Header = Decode::decode(&mut &block[..]).unwrap();
assert_eq!(local_block.number, 3);

// check that proof check doesn't panic even if proof is incorrect AND no panic handler is set
execute_with_proof_failure(&remote_client, 2, "Core_version");

// check that proof check doesn't panic even if proof is incorrect AND panic handler is set
panic_handler::set("TEST");
execute_with_proof_failure(&remote_client, 2, "Core_version");
}

#[test]
Expand Down
Loading