Properly return the prototype in case of error (paritytech#600)

bin/wasm-node/rust/src/runtime_service.rs

@@ -412,13 +412,19 @@ impl RuntimeService {
             }
 
             // Perform the actual runtime call locally.
-            let mut runtime_call =
-                executor::read_only_runtime_host::run(executor::read_only_runtime_host::Config {
+            let mut runtime_call = match executor::read_only_runtime_host::run(
+                executor::read_only_runtime_host::Config {
                     virtual_machine: runtime.virtual_machine.take().unwrap(),
                     function_to_call: method,
                     parameter: parameter_vectored,
-                })
-                .map_err(RuntimeCallError::StartError)?; // TODO: must put back virtual machine /!\
+                },
+            ) {
+                Ok(vm) => vm,
+                Err((err, prototype)) => {
+                    runtime.virtual_machine = Some(prototype);
+                    return Err(RuntimeCallError::StartError(err));
+                }
+            };
 
             loop {
                 match runtime_call {
@@ -436,8 +442,8 @@ impl RuntimeService {
                         return Ok((return_value, latest_known_runtime_lock));
                     }
                     executor::read_only_runtime_host::RuntimeHostVm::Finished(Err(error)) => {
-                        // TODO: put back virtual_machine /!\
-                        return Err(RuntimeCallError::CallError(error));
+                        runtime.virtual_machine = Some(error.prototype);
+                        return Err(RuntimeCallError::CallError(error.detail));
                     }
                     executor::read_only_runtime_host::RuntimeHostVm::StorageGet(get) => {
                         let requested_key = get.key_as_vec(); // TODO: optimization: don't use as_vec
@@ -516,7 +522,7 @@ impl RuntimeService {
 pub enum RuntimeCallError {
     /// Error during the runtime call.
     #[display(fmt = "{}", _0)]
-    CallError(executor::read_only_runtime_host::Error),
+    CallError(executor::read_only_runtime_host::ErrorDetail),
     /// Error initializing the runtime call.
     #[display(fmt = "{}", _0)]
     StartError(executor::host::StartErr),

src/author/runtime.rs

@@ -119,7 +119,7 @@ pub enum Error {
     WasmVm(runtime_host::Error),
     /// Error while initializing the Wasm virtual machine.
     #[display(fmt = "{}", _0)]
-    VmInit(host::StartErr),
+    VmInit(host::StartErr, host::HostVmPrototype),
     /// Overflow when incrementing block height.
     BlockHeightOverflow,
     /// `Core_initialize_block` has returned a non-empty output.
@@ -185,7 +185,7 @@ pub fn build_block(config: Config) -> BlockBuild {
 
     let vm = match init_result {
         Ok(vm) => vm,
-        Err(err) => return BlockBuild::Finished(Err(Error::VmInit(err))),
+        Err((err, proto)) => return BlockBuild::Finished(Err(Error::VmInit(err, proto))),
     };
 
     let shared = Shared {
@@ -329,7 +329,9 @@ impl BlockBuild {
 
                     inner = Inner::Runtime(match init_result {
                         Ok(vm) => vm,
-                        Err(err) => return BlockBuild::Finished(Err(Error::VmInit(err))),
+                        Err((err, proto)) => {
+                            return BlockBuild::Finished(Err(Error::VmInit(err, proto)))
+                        }
                     });
                 }
 
@@ -533,7 +535,7 @@ impl InherentExtrinsics {
 
         let vm = match init_result {
             Ok(vm) => vm,
-            Err(err) => return BlockBuild::Finished(Err(Error::VmInit(err))),
+            Err((err, proto)) => return BlockBuild::Finished(Err(Error::VmInit(err, proto))),
         };
 
         BlockBuild::from_inner(vm, self.shared)
@@ -625,7 +627,7 @@ impl ApplyExtrinsic {
 
         let vm = match init_result {
             Ok(vm) => vm,
-            Err(err) => return BlockBuild::Finished(Err(Error::VmInit(err))),
+            Err((err, proto)) => return BlockBuild::Finished(Err(Error::VmInit(err, proto))),
         };
 
         BlockBuild::from_inner(vm, self.shared)
@@ -646,7 +648,7 @@ impl ApplyExtrinsic {
 
         let vm = match init_result {
             Ok(vm) => vm,
-            Err(err) => return BlockBuild::Finished(Err(Error::VmInit(err))),
+            Err((err, proto)) => return BlockBuild::Finished(Err(Error::VmInit(err, proto))),
         };
 
         BlockBuild::from_inner(vm, self.shared)

src/chain/blocks_tree/verify.rs

@@ -449,12 +449,15 @@ impl<T> VerifyContext<T> {
                     },
                 }
             }
-            verify::header_body::Verify::Finished(Err(error)) => BodyVerifyStep2::Error {
-                chain: NonFinalizedTree {
-                    inner: Some(self.chain),
-                },
-                error,
-            },
+            verify::header_body::Verify::Finished(Err((error, parent_runtime))) => {
+                BodyVerifyStep2::Error {
+                    chain: NonFinalizedTree {
+                        inner: Some(self.chain),
+                    },
+                    error,
+                    parent_runtime,
+                }
+            }
             verify::header_body::Verify::StorageGet(inner) => {
                 BodyVerifyStep2::StorageGet(StorageGet {
                     context: self,
@@ -694,6 +697,8 @@ pub enum BodyVerifyStep2<T> {
         chain: NonFinalizedTree<T>,
         /// Error that happened during the verification.
         error: verify::header_body::Error, // TODO: BodyVerifyError, or rename the error to be common
+        /// Value that was passed to [`BodyVerifyRuntimeRequired::resume`].
+        parent_runtime: host::HostVmPrototype,
     },
     /// Loading a storage value is required in order to continue.
     StorageGet(StorageGet<T>),

src/chain/chain_information/aura_config.rs

@@ -67,7 +67,7 @@ impl AuraGenesisConfiguration {
     ) -> Result<(Self, host::HostVmPrototype), FromVmPrototypeError> {
         let mut vm: host::HostVm = vm
             .run_no_param("AuraApi_slot_duration")
-            .map_err(FromVmPrototypeError::VmStart)?
+            .map_err(|(err, proto)| FromVmPrototypeError::VmStart(err, proto))?
             .into();
 
         let (slot_duration, vm_prototype) = loop {
@@ -96,7 +96,7 @@ impl AuraGenesisConfiguration {
 
         let mut vm: host::HostVm = vm_prototype
             .run_no_param("AuraApi_authorities")
-            .map_err(FromVmPrototypeError::VmStart)?
+            .map_err(|(err, proto)| FromVmPrototypeError::VmStart(err, proto))?
             .into();
 
         let (authorities_list, vm_prototype) = loop {
@@ -159,7 +159,8 @@ impl FromGenesisStorageError {
 #[derive(Debug, derive_more::Display)]
 pub enum FromVmPrototypeError {
     /// Error when starting the virtual machine.
-    VmStart(host::StartErr),
+    #[display(fmt = "{}", _0)]
+    VmStart(host::StartErr, host::HostVmPrototype),
     /// Crash while running the virtual machine.
     Trapped,
     /// Virtual machine tried to call a host function that isn't valid in this context.
@@ -175,11 +176,13 @@ impl FromVmPrototypeError {
     pub fn is_function_not_found(&self) -> bool {
         matches!(
             self,
-            FromVmPrototypeError::VmStart(host::StartErr::VirtualMachine(
-                vm::StartErr::FunctionNotFound,
-            )) | FromVmPrototypeError::VmStart(host::StartErr::VirtualMachine(
-                vm::StartErr::NotAFunction,
-            ))
+            FromVmPrototypeError::VmStart(
+                host::StartErr::VirtualMachine(vm::StartErr::FunctionNotFound,),
+                _
+            ) | FromVmPrototypeError::VmStart(
+                host::StartErr::VirtualMachine(vm::StartErr::NotAFunction,),
+                _
+            )
         )
     }
 }

src/chain/chain_information/babe_config.rs

@@ -66,7 +66,7 @@ impl BabeGenesisConfiguration {
     ) -> Result<(Self, host::HostVmPrototype), FromVmPrototypeError> {
         let mut vm: host::HostVm = vm
             .run_no_param("BabeApi_configuration")
-            .map_err(FromVmPrototypeError::VmStart)?
+            .map_err(|(err, proto)| FromVmPrototypeError::VmStart(err, proto))?
             .into();
 
         let (inner, vm_prototype) = loop {
@@ -146,7 +146,8 @@ impl FromGenesisStorageError {
 #[derive(Debug, derive_more::Display)]
 pub enum FromVmPrototypeError {
     /// Error when starting the virtual machine.
-    VmStart(host::StartErr),
+    #[display(fmt = "{}", _0)]
+    VmStart(host::StartErr, host::HostVmPrototype),
     /// Crash while running the virtual machine.
     Trapped,
     /// Virtual machine tried to call a host function that isn't valid in this context.
@@ -160,11 +161,13 @@ impl FromVmPrototypeError {
     pub fn is_function_not_found(&self) -> bool {
         matches!(
             self,
-            FromVmPrototypeError::VmStart(host::StartErr::VirtualMachine(
-                vm::StartErr::FunctionNotFound,
-            )) | FromVmPrototypeError::VmStart(host::StartErr::VirtualMachine(
-                vm::StartErr::NotAFunction,
-            ))
+            FromVmPrototypeError::VmStart(
+                host::StartErr::VirtualMachine(vm::StartErr::FunctionNotFound,),
+                _
+            ) | FromVmPrototypeError::VmStart(
+                host::StartErr::VirtualMachine(vm::StartErr::NotAFunction,),
+                _
+            )
         )
     }
 }

src/chain/chain_information/babe_fetch_epoch.rs

@@ -41,11 +41,11 @@ pub struct Config {
 }
 
 /// Problem encountered during a call to [`babe_fetch_epoch`].
-#[derive(Debug, Clone, derive_more::Display)]
+#[derive(Debug, derive_more::Display)]
 pub enum Error {
     /// Error while starting the Wasm virtual machine.
     #[display(fmt = "{}", _0)]
-    WasmStart(host::StartErr),
+    WasmStart(host::StartErr, host::HostVmPrototype),
     /// Error while running the Wasm virtual machine.
     #[display(fmt = "{}", _0)]
     WasmVm(read_only_runtime_host::Error),
@@ -70,7 +70,7 @@ pub fn babe_fetch_epoch(config: Config) -> Query {
 
     match vm {
         Ok(vm) => Query::from_inner(vm),
-        Err(err) => Query::Finished(Err(Error::WasmStart(err))),
+        Err((err, proto)) => Query::Finished(Err(Error::WasmStart(err, proto))),
     }
 }
 

src/executor/host.rs

@@ -270,39 +270,49 @@ impl HostVmPrototype {
     }
 
     /// Starts the VM, calling the function passed as parameter.
-    pub fn run(self, function_to_call: &str, data: &[u8]) -> Result<ReadyToRun, StartErr> {
+    pub fn run(self, function_to_call: &str, data: &[u8]) -> Result<ReadyToRun, (StartErr, Self)> {
         self.run_vectored(function_to_call, iter::once(data))
     }
 
     /// Same as [`HostVmPrototype::run`], except that the function desn't need any parameter.
-    pub fn run_no_param(self, function_to_call: &str) -> Result<ReadyToRun, StartErr> {
+    pub fn run_no_param(self, function_to_call: &str) -> Result<ReadyToRun, (StartErr, Self)> {
         self.run_vectored(function_to_call, iter::empty::<Vec<u8>>())
     }
 
     /// Same as [`HostVmPrototype::run`], except that the function parameter can be passed as
     /// a list of buffers. All the buffers will be concatenated in memory.
     pub fn run_vectored(
-        self,
+        mut self,
         function_to_call: &str,
         data: impl Iterator<Item = impl AsRef<[u8]>> + Clone,
-    ) -> Result<ReadyToRun, StartErr> {
+    ) -> Result<ReadyToRun, (StartErr, Self)> {
         let mut data_len_u32: u32 = 0;
         for data in data.clone() {
-            let len = u32::try_from(data.as_ref().len()).map_err(|_| StartErr::DataSizeOverflow)?;
-            data_len_u32 = data_len_u32
-                .checked_add(len)
-                .ok_or(StartErr::DataSizeOverflow)?;
+            let len = match u32::try_from(data.as_ref().len()) {
+                Ok(v) => v,
+                Err(_) => return Err((StartErr::DataSizeOverflow, self)),
+            };
+            data_len_u32 = match data_len_u32.checked_add(len) {
+                Some(v) => v,
+                None => return Err((StartErr::DataSizeOverflow, self)),
+            };
         }
 
         // Now create the actual virtual machine. We pass as parameter `heap_base` as the location
         // of the input data.
-        let mut vm = self.vm_proto.start(
+        let mut vm = match self.vm_proto.start(
             function_to_call,
             &[
                 vm::WasmValue::I32(i32::from_ne_bytes(self.heap_base.to_ne_bytes())),
                 vm::WasmValue::I32(i32::from_ne_bytes(data_len_u32.to_ne_bytes())),
             ],
-        )?;
+        ) {
+            Ok(vm) => vm,
+            Err((error, vm_proto)) => {
+                self.vm_proto = vm_proto;
+                return Err((error.into(), self));
+            }
+        };
 
         // Now writing the input data into the VM.
         let mut after_input_data = self.heap_base;
@@ -419,6 +429,29 @@ pub enum HostVm {
     LogEmit(LogEmit),
 }
 
+impl HostVm {
+    /// Cancels execution of the virtual machine and returns back the prototype.
+    pub fn into_prototype(self) -> HostVmPrototype {
+        match self {
+            HostVm::ReadyToRun(inner) => inner.inner.into_prototype(),
+            HostVm::Finished(inner) => inner.inner.into_prototype(),
+            HostVm::Error { prototype, .. } => prototype,
+            HostVm::ExternalStorageGet(inner) => inner.inner.into_prototype(),
+            HostVm::ExternalStorageSet(inner) => inner.inner.into_prototype(),
+            HostVm::ExternalStorageAppend(inner) => inner.inner.into_prototype(),
+            HostVm::ExternalStorageClearPrefix(inner) => inner.inner.into_prototype(),
+            HostVm::ExternalStorageRoot(inner) => inner.inner.into_prototype(),
+            HostVm::ExternalStorageChangesRoot(inner) => inner.inner.into_prototype(),
+            HostVm::ExternalStorageNextKey(inner) => inner.inner.into_prototype(),
+            HostVm::ExternalOffchainStorageSet(inner) => inner.inner.into_prototype(),
+            HostVm::CallRuntimeVersion(inner) => inner.inner.into_prototype(),
+            HostVm::StartStorageTransaction(inner) => inner.inner.into_prototype(),
+            HostVm::EndStorageTransaction { resume, .. } => resume.inner.into_prototype(),
+            HostVm::LogEmit(inner) => inner.inner.into_prototype(),
+        }
+    }
+}
+
 /// Virtual machine is ready to run.
 pub struct ReadyToRun {
     inner: Inner,