diff --git a/script/src/error.rs b/script/src/error.rs index 8d09c54b86..09af48f4c0 100644 --- a/script/src/error.rs +++ b/script/src/error.rs @@ -2,6 +2,7 @@ use crate::types::{ScriptGroup, ScriptGroupType}; use ckb_error::{prelude::*, Error, ErrorKind}; use ckb_types::core::{Cycle, ScriptHashType}; use ckb_types::packed::{Byte32, Script}; +use ckb_vm::Error as VMInternalError; use std::{error::Error as StdError, fmt}; /// Script execution error. @@ -39,9 +40,13 @@ pub enum ScriptError { #[error("Invalid VM Version: {0}")] InvalidVmVersion(u8), - /// Known bugs are detected in transaction script outputs - #[error("VM Internal Error: {0}")] - VMInternalError(String), + /// Errors thrown by ckb-vm + #[error("VM Internal Error: {0:?}")] + VMInternalError(VMInternalError), + + /// Other errors raised in script execution process + #[error("Other Error: {0}")] + Other(String), } /// Locate the script using the first input index if possible, otherwise the first output index. @@ -83,6 +88,30 @@ pub struct TransactionScriptError { cause: ScriptError, } +impl TransactionScriptError { + /// Orginating script for the generated error + pub fn originating_script(&self) -> &TransactionScriptErrorSource { + &self.source + } + + /// Actual error generated when verifying script + pub fn script_error(&self) -> &ScriptError { + &self.cause + } +} + +/// It is a delibrate choice here to implement StdError directly, instead of +/// implementing thiserror::Error on TransactionScriptError. This way, calling +/// root_cause() on ckb_error::Error would return TransactionScriptError structure, +/// providing us enough information to inspect on all kinds of errors generated when +/// verifying a script. +/// +/// This also means calling source() or cause() from std::error::Error on +/// TransactionScriptError would return None values. One is expected to cast +/// a value of `std::error::Error` type(possibly returned from root_cause) into +/// concrete TransactionScriptError type, then use the defined methods to fetch +/// originating script, as well as the actual script error. See the unit test defined +/// at the end of this file for an example. impl StdError for TransactionScriptError {} impl fmt::Display for TransactionScriptError { @@ -156,3 +185,36 @@ impl From for Error { ErrorKind::Script.because(error) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_downcast_error_to_vm_error() { + let vm_error = VMInternalError::ElfParseError("Foo bar baz".to_string()); + let script_error = ScriptError::VMInternalError(vm_error.clone()); + let error: Error = script_error.output_type_script(177).into(); + + let recovered_transaction_error: TransactionScriptError = error + .root_cause() + .downcast_ref() + .cloned() + .expect("downcasting transaction error"); + assert_eq!( + recovered_transaction_error.originating_script(), + &TransactionScriptErrorSource::Outputs(177, ScriptGroupType::Type), + ); + + if let ScriptError::VMInternalError(recovered_vm_error) = + recovered_transaction_error.script_error() + { + assert_eq!(recovered_vm_error, &vm_error); + } else { + panic!( + "Invalid script type: {}", + recovered_transaction_error.script_error() + ); + } + } +} diff --git a/script/src/types.rs b/script/src/types.rs index 19434b7a9c..b8b625b84b 100644 --- a/script/src/types.rs +++ b/script/src/types.rs @@ -391,13 +391,13 @@ impl TryFrom for TransactionSnapshot { let mut snaps = Vec::with_capacity(vms.len()); for mut vm in vms { let snapshot = make_snapshot(&mut vm.machine_mut().machine) - .map_err(|e| ScriptError::VMInternalError(format!("{e:?}")).unknown_source())?; + .map_err(|e| ScriptError::VMInternalError(e).unknown_source())?; let cycles = vm.cycles(); let resume_point = match vm { ResumableMachine::Initial(_) => ResumePoint::Initial, ResumableMachine::Spawn(_, data) => (&data) .try_into() - .map_err(|e| ScriptError::VMInternalError(format!("{e:?}")).unknown_source())?, + .map_err(|e| ScriptError::VMInternalError(e).unknown_source())?, }; snaps.push((snapshot, cycles, resume_point)); } diff --git a/script/src/verify.rs b/script/src/verify.rs index 3c1065b5e4..207161e436 100644 --- a/script/src/verify.rs +++ b/script/src/verify.rs @@ -641,10 +641,8 @@ impl ScriptError::ExceededMaximumCycles(max_cycles), - _ => ScriptError::VMInternalError(format!("{error:?}")), + _ => ScriptError::VMInternalError(error), }; let bytes = machine @@ -1067,7 +1060,7 @@ impl ScriptError::ExceededMaximumCycles(max_cycles), - _ => ScriptError::VMInternalError(format!("{error:?}")), + _ => ScriptError::VMInternalError(error), }; let machines = if !snaps.is_empty() { @@ -1157,7 +1150,7 @@ impl ScriptError::ExceededMaximumCycles(max_cycles), - _ => ScriptError::VMInternalError(format!("{error:?}")), + _ => ScriptError::VMInternalError(error), }; while let Some(mut machine) = machines.pop() { @@ -1210,7 +1203,7 @@ fn run_vms( VMInternalError::CyclesExceeded => { let mut new_suspended_machines: Vec<_> = { let mut context = context.lock().map_err(|e| { - ScriptError::VMInternalError(format!("Failed to acquire lock: {}", e)) + ScriptError::Other(format!("Failed to acquire lock: {}", e)) })?; context.suspended_machines.drain(..).collect() }; @@ -1221,7 +1214,7 @@ fn run_vms( machines.append(&mut new_suspended_machines); return Ok(ChunkState::suspended(machines, Arc::clone(context))); } - _ => return Err(ScriptError::VMInternalError(format!("{error:?}"))), + _ => return Err(ScriptError::VMInternalError(error)), }, }; } diff --git a/script/src/verify/tests/ckb_latest/features_since_v2019.rs b/script/src/verify/tests/ckb_latest/features_since_v2019.rs index 77d224c0ae..4a25ef1602 100644 --- a/script/src/verify/tests/ckb_latest/features_since_v2019.rs +++ b/script/src/verify/tests/ckb_latest/features_since_v2019.rs @@ -1063,7 +1063,7 @@ fn load_code_to_stack_then_reuse_case1_load_and_write() { let result = verifier.verify_without_limit(script_version, &rtx); assert!(result.is_err()); let vm_error = VmError::MemWriteOnExecutablePage; - let script_error = ScriptError::VMInternalError(format!("{vm_error:?}")); + let script_error = ScriptError::VMInternalError(vm_error); assert_error_eq!(result.unwrap_err(), script_error.input_lock_script(0)); } diff --git a/script/src/verify/tests/ckb_latest/features_since_v2021.rs b/script/src/verify/tests/ckb_latest/features_since_v2021.rs index 9570c326ae..819d247443 100644 --- a/script/src/verify/tests/ckb_latest/features_since_v2021.rs +++ b/script/src/verify/tests/ckb_latest/features_since_v2021.rs @@ -55,7 +55,7 @@ fn test_hint_instructions() { pc: 65_656, instruction: 36_906, }; - let script_error = ScriptError::VMInternalError(format!("{vm_error:?}")); + let script_error = ScriptError::VMInternalError(vm_error); assert_error_eq!(result.unwrap_err(), script_error.input_lock_script(0)); } } @@ -105,7 +105,7 @@ fn test_b_extension() { pc: 0x10182, instruction: 0x60291913, }; - let script_error = ScriptError::VMInternalError(format!("{vm_error:?}")); + let script_error = ScriptError::VMInternalError(vm_error); assert_error_eq!(result.unwrap_err(), script_error.input_lock_script(0)); } } @@ -1018,7 +1018,7 @@ fn load_code_into_global() { assert_eq!(result.is_ok(), script_version >= ScriptVersion::V1,); if script_version < ScriptVersion::V1 { let vm_error = VmError::MemWriteOnFreezedPage; - let script_error = ScriptError::VMInternalError(format!("{vm_error:?}")); + let script_error = ScriptError::VMInternalError(vm_error); assert_error_eq!(result.unwrap_err(), script_error.input_lock_script(0)); } } diff --git a/tx-pool/src/chunk_process.rs b/tx-pool/src/chunk_process.rs index e893b62796..4817e0630e 100644 --- a/tx-pool/src/chunk_process.rs +++ b/tx-pool/src/chunk_process.rs @@ -357,7 +357,7 @@ fn exceeded_maximum_cycles_error< .nth(current) .map(|(_hash, group)| ScriptError::ExceededMaximumCycles(max_cycles).source(group)) .unwrap_or_else(|| { - ScriptError::VMInternalError(format!("suspended state group missing {current:?}")) + ScriptError::Other(format!("suspended state group missing {current:?}")) .unknown_source() }) .into()