diff --git a/executor/fuzz/shared/leader-result.rs b/executor/fuzz/shared/leader-result.rs index 57ec63ff..062c2547 100644 --- a/executor/fuzz/shared/leader-result.rs +++ b/executor/fuzz/shared/leader-result.rs @@ -1,4 +1,6 @@ -use genvm::{public_abi::VmError, rt::vm::RunOk, wasi::genlayer_sdk::parse_leader_result}; +use genvm::{ + public_abi::VmError, rt::vm::ContractOutcome, wasi::genlayer_sdk::parse_leader_result, +}; fn is_derived_namespace(code: &str) -> bool { is_code_or_space_extension(code, "leader_fault nondet_output") @@ -36,13 +38,13 @@ pub fn assert_parse_properties(data: &[u8]) { match parse_leader_result(data) { Ok(res) => { assert_eq!( - res.as_bytes(), + res.encode().into_bytes().as_ref(), data, "accepted leader result must serialize byte-identically" ); match res { - RunOk::VMError(e, _) => { + ContractOutcome::VMError(e, _) => { assert!( VmError::is_valid_(&e.0), "accepted vm_error must be a valid public ABI code: {:?}", @@ -59,10 +61,7 @@ pub fn assert_parse_properties(data: &[u8]) { e.0 ); } - RunOk::FatalVMError(..) => { - panic!("leader result parsing must reject fatal VM errors") - } - RunOk::Return(_) | RunOk::UserError(_) => { + ContractOutcome::Return(_) | ContractOutcome::UserError(_) => { assert!( data.len() > 1 && genvm::calldata::decode(&data[1..]).is_ok(), "validate-only and materializing calldata decode must agree" diff --git a/executor/src/host/mod.rs b/executor/src/host/mod.rs index 4db1bbfa..b1fcbfc3 100644 --- a/executor/src/host/mod.rs +++ b/executor/src/host/mod.rs @@ -1079,8 +1079,7 @@ mod tests { assert_ne!(one, execution_hash_with(Vec::new())); } - // Fatality reaches the host as its own code; only the payload is shared - // with an ordinary VM error. + // FullResult preserves nested fatality until the publication boundary #[test] fn fatal_run_ok_keeps_its_code_and_payload() { let code = crate::public_abi::VmError::timeout(); @@ -1090,4 +1089,29 @@ mod tests { assert_eq!(result.kind, host_fns::ResultCode::FatalVmError); assert_eq!(result.data, calldata::Value::Str(code.into()).into()); } + + #[test] + fn top_level_fatal_is_framed_and_reported_as_vm_error() { + let mut rt_result = rt::vm::FullResult::empty_from(rt::vm::RunOk::FatalVMError( + crate::public_abi::VmError::timeout(), + None, + )); + rt_result.coalesce_fatal_for_top_level(); + let result = FullResult::new( + rt_result, + Vec::new(), + None, + Vec::new(), + rt::fees::BucketsConsumed::default(), + primitive_types::U256::zero(), + Vec::new(), + ); + + let encoded = encode_result(&Ok(result)).unwrap(); + let reported: genvm_modules_interfaces::ReportedResult = + calldata::decode_obj(&encoded[1..]).unwrap(); + + assert_eq!(encoded[0], host_fns::ResultCode::VmError as u8); + assert_eq!(reported.kind, genvm_modules_interfaces::ResultCode::VmError); + } } diff --git a/executor/src/lib.rs b/executor/src/lib.rs index 7843e8f5..f3203517 100644 --- a/executor/src/lib.rs +++ b/executor/src/lib.rs @@ -285,7 +285,10 @@ fn extra_leader_nondet_output_error( return None; } - Some(rt::errors::vm_error_for_leader_extra(&run_ok.as_bytes())) + let encoded = rt::vm::ContractOutcome::try_from(run_ok.duplicate()) + .expect("fatal results return before extra leader output is checked") + .encode(); + Some(rt::errors::vm_error_for_leader_extra(encoded.as_slice())) } fn has_extra_leader_output(run_mode: rt::RunMode, executed: u32, published: u32) -> bool { @@ -521,6 +524,28 @@ pub async fn run_with_impl( }) } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ReportingBoundary { + TopLevel, + Nested, +} + +impl ReportingBoundary { + fn for_execution_data(entry_data: &genvm_modules_interfaces::ExecutionData) -> Self { + if entry_data.nested.is_some() { + Self::Nested + } else { + Self::TopLevel + } + } + + fn normalize(self, result: &mut rt::vm::FullResult) { + if self == Self::TopLevel { + result.coalesce_fatal_for_top_level(); + } + } +} + pub async fn run_with( entry_data: genvm_modules_interfaces::ExecutionData, context: ExecutionContext, @@ -532,6 +557,7 @@ pub async fn run_with( // during deploy attaches to this very entry at spawn. Dropping the pin // earlier would make that load's outcome depend on which queue processed it. let mut deploy_pin: Option = None; + let reporting_boundary = ReportingBoundary::for_execution_data(&entry_data); let supervisor = context.supervisor.clone(); @@ -588,6 +614,7 @@ pub async fn run_with( let res = match res { Ok((mut a, b)) => { a.discard_effects_unless_returned(); + reporting_boundary.normalize(&mut a); Ok(host::FullResult::new( a, @@ -630,4 +657,22 @@ mod tests { assert!(!has_extra_leader_output(rt::RunMode::Leader, 0, 0)); assert!(!has_extra_leader_output(rt::RunMode::Sync, 0, 0)); } + + #[test] + fn reporting_boundary_coalesces_only_top_level_fatality() { + let fatal = || { + rt::vm::FullResult::empty_from(rt::vm::RunOk::FatalVMError( + public_abi::VmError::timeout(), + None, + )) + }; + let mut top_level = fatal(); + let mut nested = fatal(); + + ReportingBoundary::TopLevel.normalize(&mut top_level); + ReportingBoundary::Nested.normalize(&mut nested); + + assert_eq!(top_level.kind, host::host_fns::ResultCode::VmError); + assert_eq!(nested.kind, host::host_fns::ResultCode::FatalVmError); + } } diff --git a/executor/src/rt/supervisor/mod.rs b/executor/src/rt/supervisor/mod.rs index 29aeb1de..20fa992b 100644 --- a/executor/src/rt/supervisor/mod.rs +++ b/executor/src/rt/supervisor/mod.rs @@ -264,13 +264,13 @@ pub async fn submit_nondet_vm_task(zelf: &Arc, task: NonDetVMTask) { } impl Supervisor { - pub async fn push_nondet_result(&self, call_no: u32, result: bytes::Bytes) { + pub async fn push_nondet_result(&self, call_no: u32, result: rt::vm::ContractResultBytes) { let mut vec = self.nondet_results.lock().await; let idx = u32_into_usize(call_no); while vec.len() <= idx { vec.push(bytes::Bytes::new()); } - vec[idx] = result; + vec[idx] = result.into_bytes(); } pub async fn take_nondet_results(&self) -> Vec { diff --git a/executor/src/rt/vm/mod.rs b/executor/src/rt/vm/mod.rs index 2458e142..50ced7cc 100644 --- a/executor/src/rt/vm/mod.rs +++ b/executor/src/rt/vm/mod.rs @@ -14,6 +14,93 @@ pub enum RunOk { FatalVMError(abi::consts::VmError, Option), } +#[derive(Debug)] +pub enum ContractOutcome { + Return(calldata::unparsed::Maybe), + UserError(calldata::unparsed::Maybe), + VMError(abi::consts::VmError, Option), +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ContractResultBytes(bytes::Bytes); + +impl ContractResultBytes { + pub fn as_slice(&self) -> &[u8] { + &self.0 + } + + pub fn into_bytes(self) -> bytes::Bytes { + self.0 + } +} + +impl ContractOutcome { + pub fn duplicate(&self) -> Self { + match self { + Self::Return(buf) => Self::Return(buf.clone()), + Self::UserError(buf) => Self::UserError(buf.clone()), + Self::VMError(e, _) => Self::VMError(e.clone(), None), + } + } + + pub fn encode(&self) -> ContractResultBytes { + use crate::public_abi::ResultCode; + + let bytes = match self { + Self::Return(buf) => { + let encoded = calldata::encode_obj(buf); + let mut res = Vec::with_capacity(1 + encoded.len()); + res.push(ResultCode::Return as u8); + res.extend_from_slice(&encoded); + res + } + Self::UserError(val) => { + let mut res = vec![ResultCode::UserError as u8]; + match val { + calldata::unparsed::Maybe::Materialized(value) => { + res.extend_from_slice(&calldata::encode(value)); + } + calldata::unparsed::Maybe::Checked(raw) => { + res.extend_from_slice(&raw.0); + } + } + res + } + Self::VMError(buf, _) => { + let mut res = Vec::with_capacity(1 + buf.0.len()); + res.push(ResultCode::VmError as u8); + res.extend_from_slice(buf.0.as_bytes()); + res + } + }; + + ContractResultBytes(bytes.into()) + } +} + +impl From for RunOk { + fn from(value: ContractOutcome) -> Self { + match value { + ContractOutcome::Return(buf) => Self::Return(buf), + ContractOutcome::UserError(buf) => Self::UserError(buf), + ContractOutcome::VMError(e, cause) => Self::VMError(e, cause), + } + } +} + +impl TryFrom for ContractOutcome { + type Error = rt::errors::Error; + + fn try_from(value: RunOk) -> Result { + match value { + RunOk::Return(buf) => Ok(Self::Return(buf)), + RunOk::UserError(buf) => Ok(Self::UserError(buf)), + RunOk::VMError(e, cause) => Ok(Self::VMError(e, cause)), + RunOk::FatalVMError(e, cause) => Err(rt::errors::Error::fatal_vm_cause(e, cause)), + } + } +} + impl RunOk { /// Like clone, but drops the `cause` of a VM error if present pub fn duplicate(&self) -> Self { @@ -25,15 +112,8 @@ impl RunOk { } } - pub fn into_nonfatal(self) -> rt::errors::Result { - match self { - RunOk::FatalVMError(e, cause) => Err(rt::errors::Error::fatal_vm_cause(e, cause)), - run_ok => Ok(run_ok), - } - } - pub fn into_contract_observable_bytes(self) -> rt::errors::Result> { - self.into_nonfatal().map(|run_ok| run_ok.as_bytes()) + ContractOutcome::try_from(self).map(|outcome| outcome.encode().into_bytes().to_vec()) } } @@ -96,52 +176,18 @@ impl FullResult { self.storage_changes.clear(); self.emissions.clear(); } + + pub fn coalesce_fatal_for_top_level(&mut self) { + if self.kind == host_fns::ResultCode::FatalVmError { + self.kind = host_fns::ResultCode::VmError; + } + } } impl RunOk { pub fn empty_return() -> Self { Self::Return(calldata::Value::Null.into()) } - - pub fn as_bytes(&self) -> Vec { - use crate::public_abi::ResultCode; - match self { - RunOk::Return(buf) => { - let encoded = calldata::encode_obj(buf); - let mut res = Vec::with_capacity(1 + encoded.len()); - res.push(ResultCode::Return as u8); - res.extend_from_slice(&encoded); - res - } - RunOk::UserError(val) => { - let mut res = vec![ResultCode::UserError as u8]; - match val { - calldata::unparsed::Maybe::Materialized(value) => { - res.extend_from_slice(&calldata::encode(value)); - } - calldata::unparsed::Maybe::Checked(raw) => { - res.extend_from_slice(&raw.0); - } - } - res - } - RunOk::VMError(buf, _) => { - let mut res = Vec::with_capacity(1 + buf.0.len()); - res.push(ResultCode::VmError as u8); - res.extend_from_slice(buf.0.as_bytes()); - res - } - // Fatality has to survive a sub-VM buffer that is relayed across a - // major boundary, so it keeps a code of its own -- one the host - // wire admits and the contract-facing enumeration does not. - RunOk::FatalVMError(buf, _) => { - let mut res = Vec::with_capacity(1 + buf.0.len()); - res.push(crate::host::host_fns::ResultCode::FatalVmError as u8); - res.extend_from_slice(buf.0.as_bytes()); - res - } - } - } } impl std::fmt::Display for RunOk { @@ -386,25 +432,21 @@ mod tests { } #[test] - fn fatal_result_serialization_is_total() { - let code = public_abi::VmError::timeout(); - let bytes = RunOk::FatalVMError(code.clone(), None).as_bytes(); + fn fatal_result_is_reported_as_fatal() { + let full = + FullResult::empty_from(RunOk::FatalVMError(public_abi::VmError::timeout(), None)); - assert_eq!( - bytes[0], - crate::host::host_fns::ResultCode::FatalVmError as u8 - ); - assert_eq!(&bytes[1..], code.0.as_bytes()); + assert_eq!(full.kind, host_fns::ResultCode::FatalVmError); } - // Fatality is reported truthfully to the host; degrading it to an ordinary - // VM error is the manager's job, at the outermost boundary. #[test] - fn fatal_result_is_reported_as_fatal() { - let full = + fn top_level_fatal_result_is_coalesced() { + let mut full = FullResult::empty_from(RunOk::FatalVMError(public_abi::VmError::timeout(), None)); - assert_eq!(full.kind, host_fns::ResultCode::FatalVmError); + full.coalesce_fatal_for_top_level(); + + assert_eq!(full.kind, host_fns::ResultCode::VmError); } fn with_effects(run_ok: RunOk) -> FullResult { diff --git a/executor/src/wasi/genlayer_sdk/mod.rs b/executor/src/wasi/genlayer_sdk/mod.rs index 5b60bf70..c70ae8f5 100644 --- a/executor/src/wasi/genlayer_sdk/mod.rs +++ b/executor/src/wasi/genlayer_sdk/mod.rs @@ -41,7 +41,7 @@ pub trait ExtendedMessageExt { &self, entry_kind: public_abi::EntryKind, entry_data: bytes::Bytes, - entry_leader_data: Option, + entry_leader_data: Option, ) -> ExtendedMessage; fn fork(&self, entry_kind: public_abi::EntryKind, entry_data: bytes::Bytes) -> ExtendedMessage; @@ -52,7 +52,7 @@ impl ExtendedMessageExt for ExtendedMessage { &self, entry_kind: public_abi::EntryKind, entry_data: bytes::Bytes, - entry_leader_data: Option, + entry_leader_data: Option, ) -> ExtendedMessage { use genlayer_sdk::abi::entry::MessageData; @@ -60,7 +60,7 @@ impl ExtendedMessageExt for ExtendedMessage { None => default_entry_stage_data(), Some(entry_leader_data) => calldata::Value::Map(BTreeMap::from([( "leaders_result".into(), - calldata::Value::Bytes(entry_leader_data.as_bytes()), + calldata::Value::Bytes(entry_leader_data.encode().into_bytes().to_vec()), )])), }; @@ -415,7 +415,9 @@ impl ContextVFS<'_> { } data => data, }; - let data: Vec = data.as_bytes(); + let data = data + .into_contract_observable_bytes() + .map_err(|e| generated::types::Error::trap(crate::anyhow_to_wasmtime(e.into())))?; let len = data.len(); self.place_content(vfs::FileContents::from(bytes::Bytes::from(data))) .map(|fd| (fd, len)) diff --git a/executor/src/wasi/genlayer_sdk/run.rs b/executor/src/wasi/genlayer_sdk/run.rs index e0661443..60748448 100644 --- a/executor/src/wasi/genlayer_sdk/run.rs +++ b/executor/src/wasi/genlayer_sdk/run.rs @@ -87,10 +87,10 @@ pub fn strip_vm_error_detail(code: &str) -> public_abi::VmError { } /// The one total parse of a leader-proposed non-deterministic result. `Ok` -/// means the bytes are accepted verbatim (`as_bytes()` reproduces `data`); +/// means the bytes are accepted verbatim (`encode()` reproduces `data`); /// `Err` carries the VM error the validator derives instead. Malformed input /// never traps and never bypasses the comparison stage. -pub fn parse_leader_result(data: &[u8]) -> Result { +pub fn parse_leader_result(data: &[u8]) -> Result { let Some((&code, rest)) = data.split_first() else { return Err(public_abi::VmError::leader_fault().nondet_output().absent()); }; @@ -104,20 +104,20 @@ pub fn parse_leader_result(data: &[u8]) -> Result = calldata::decode_obj(rest).map_err(|_| malformed_leader_result())?; - Ok(rt::vm::RunOk::Return(ret)) + Ok(rt::vm::ContractOutcome::Return(ret)) } public_abi::ResultCode::UserError => { let err: calldata::unparsed::Maybe = calldata::decode_obj(rest).map_err(|_| malformed_leader_result())?; - Ok(rt::vm::RunOk::UserError(err)) + Ok(rt::vm::ContractOutcome::UserError(err)) } public_abi::ResultCode::VmError => { let code = std::str::from_utf8(rest).map_err(|_| malformed_leader_result())?; validate_leader_vm_error(code)?; - Ok(rt::vm::RunOk::VMError( + Ok(rt::vm::ContractOutcome::VMError( public_abi::VmError(std::borrow::Cow::Owned(code.to_owned())), None, )) @@ -125,6 +125,34 @@ pub fn parse_leader_result(data: &[u8]) -> Result rt::errors::Result { + match rt::vm::ContractOutcome::try_from(computed_result)? { + // Publish the bare code, keep the detail as a local cause + rt::vm::ContractOutcome::VMError(err, cause) => Ok(rt::vm::ContractOutcome::VMError( + strip_vm_error_detail(&err.0), + cause, + )), + computed_result => Ok(computed_result), + } +} + +pub(super) fn leader_outcome_for_validation( + data: &[u8], +) -> (rt::vm::ContractOutcome, rt::vm::RunOk) { + match parse_leader_result(data) { + Ok(outcome) => { + let result_to_return = outcome.duplicate().into(); + (outcome, result_to_return) + } + Err(vm_error) => ( + rt::vm::ContractOutcome::VMError(vm_error.clone(), None), + rt::vm::RunOk::FatalVMError(vm_error, None), + ), + } +} + struct RunNondetGetVMTaskArgs { child_topmost_id: runners::Id, child_limiter: rt::memlimiter::Limiter, @@ -624,7 +652,7 @@ impl ContextVFS<'_> { let is_leader = self.context.data.supervisor.shared_data.run_mode == rt::RunMode::Leader; - let result_to_return = if is_leader { + let (result_to_return, encoded) = if is_leader { let vm_ext_msg = self.context.data.message_data.fork_leader( public_abi::EntryKind::ConsensusStage, data_leader, @@ -638,17 +666,11 @@ impl ContextVFS<'_> { .await .map_err(|e| generated::types::Error::trap(crate::anyhow_to_wasmtime(e)))?; - let computed_result = computed_result - .into_nonfatal() + let computed_result = leader_outcome_for_publication(computed_result) .map_err(|e| generated::types::Error::trap(crate::anyhow_to_wasmtime(e.into())))?; + let encoded = computed_result.encode(); - match computed_result { - // Publish the bare code, keep the detail as a local cause. - rt::vm::RunOk::VMError(err, cause) => { - rt::vm::RunOk::VMError(strip_vm_error_detail(&err.0), cause) - } - computed_result => computed_result, - } + (computed_result.into(), encoded) } else { let leaders_res_bytes = self .context @@ -662,10 +684,8 @@ impl ContextVFS<'_> { // A leader fault is fatal: it says this node's whole run is built on // a result no honest leader could have produced, so a caller must // not be able to carry on as if the block had merely failed. - let leaders_res = match parse_leader_result(&leaders_res_bytes.unwrap_or_default()) { - Ok(res) => res, - Err(vm_error) => rt::vm::RunOk::FatalVMError(vm_error, None), - }; + let (leaders_res, result_to_return) = + leader_outcome_for_validation(&leaders_res_bytes.unwrap_or_default()); if self.context.data.supervisor.shared_data.run_mode == rt::RunMode::Validator { let vm_ext_msg = self.context.data.message_data.fork_leader( @@ -679,13 +699,10 @@ impl ContextVFS<'_> { rt::supervisor::submit_nondet_vm_task(&self.context.data.supervisor, task).await; } - leaders_res + let encoded = leaders_res.encode(); + (result_to_return, encoded) }; - // One encoding feeds the fee, the leader's retained copy and the - // caller-visible file. - let encoded = bytes::Bytes::from(result_to_return.as_bytes()); - // Retention precedes the charge, so a validator replaying a run that // ran out of fee here sees the same result the leader charged for. if is_leader { @@ -698,7 +715,7 @@ impl ContextVFS<'_> { consume_nondet_output( &self.context.data.supervisor.shared_data, - encoded.len().into_int_comptime(), + encoded.as_slice().len().into_int_comptime(), ) .await?; diff --git a/executor/src/wasi/genlayer_sdk/tests.rs b/executor/src/wasi/genlayer_sdk/tests.rs index 7c3683ae..91bdfc9a 100644 --- a/executor/src/wasi/genlayer_sdk/tests.rs +++ b/executor/src/wasi/genlayer_sdk/tests.rs @@ -1,7 +1,8 @@ use super::message::{validate_balance_fee, FEE_PARAM_COUNT_BITS, FEE_PARAM_PRICE_BITS}; use super::run::{ - call_contract_route, derive_call_contract_permissions, nested_run_ok, parse_leader_result, - strip_vm_error_detail, CallContractRoute, + call_contract_route, derive_call_contract_permissions, leader_outcome_for_publication, + leader_outcome_for_validation, nested_run_ok, parse_leader_result, strip_vm_error_detail, + CallContractRoute, }; use super::*; use primitive_types::U256; @@ -231,6 +232,26 @@ fn leader_internal_error_code_is_malformed() { } } +#[test] +fn fatal_leader_outcome_cannot_be_published() { + let result = rt::vm::RunOk::FatalVMError(public_abi::VmError::timeout(), None); + + assert!(leader_outcome_for_publication(result).is_err()); +} + +#[test] +fn malformed_leader_outcome_is_visible_as_vm_error_but_remains_fatal() { + let (visible, returned) = + leader_outcome_for_validation(&[crate::host::host_fns::ResultCode::FatalVmError as u8]); + + assert!(matches!(&visible, rt::vm::ContractOutcome::VMError(..))); + assert!(matches!(returned, rt::vm::RunOk::FatalVMError(..))); + assert_eq!( + visible.encode().as_slice()[0], + public_abi::ResultCode::VmError as u8 + ); +} + #[test] fn leader_return_with_invalid_calldata_is_malformed() { // The hole this closes: the executor used to pass the `Return` payload @@ -269,7 +290,7 @@ fn leader_valid_return_is_preserved_bytewise() { // Validation must not re-encode: the accepted result has to round-trip to the // exact bytes the leader proposed, or validators would hash a different // value than the one they agreed to. - assert_eq!(got.as_bytes(), data); + assert_eq!(got.encode().into_bytes().as_ref(), data); } #[test] @@ -282,7 +303,14 @@ fn leader_user_error_with_invalid_calldata_is_malformed() { fn leader_valid_user_error_passes() { let payload = calldata::encode(&calldata::Value::Str("boom".to_owned())); let data = leader_bytes(public_abi::ResultCode::UserError, &payload); - assert_eq!(parse_leader_result(&data).unwrap().as_bytes(), data); + assert_eq!( + parse_leader_result(&data) + .unwrap() + .encode() + .into_bytes() + .as_ref(), + data + ); } #[test] @@ -334,7 +362,7 @@ fn leader_vm_error_on_trie_code_passes() { let data = leader_bytes(public_abi::ResultCode::VmError, code.as_bytes()); let got = parse_leader_result(&data) .unwrap_or_else(|e| panic!("code {code:?} should be accepted, got {e:?}")); - assert_eq!(got.as_bytes(), data); + assert_eq!(got.encode().into_bytes().as_ref(), data); } } @@ -594,6 +622,6 @@ fn every_stripped_leader_error_round_trips_through_acceptance() { let accepted = parse_leader_result(&data) .unwrap_or_else(|e| panic!("honest leader code {code:?} rejected as {e:?}")); - assert_eq!(accepted.as_bytes(), data); + assert_eq!(accepted.encode().into_bytes().as_ref(), data); } }