diff --git a/changelog.d/clarity-runtime-tx-error.changed b/changelog.d/clarity-runtime-tx-error.changed new file mode 100644 index 00000000000..6818cae1a10 --- /dev/null +++ b/changelog.d/clarity-runtime-tx-error.changed @@ -0,0 +1 @@ +Moved Clarity transaction-error classification from `stackslib` to `clarity`, with authoritative included/rejected dispositions that can be shared with WASM consumers diff --git a/clarity/src/vm/clarity.rs b/clarity/src/vm/clarity.rs index ce31e146160..2f3f131ea48 100644 --- a/clarity/src/vm/clarity.rs +++ b/clarity/src/vm/clarity.rs @@ -107,6 +107,189 @@ impl std::error::Error for ClarityError { } } +/// An execution-phase failure for a transaction that remains included in its block. +// Left unboxed, as it was in `stackslib`. +#[allow(clippy::large_enum_variant)] +pub enum IncludedRuntimeTxError { + /// An error raised by the interpreter while executing the transaction. + #[non_exhaustive] + Runtime { + /// The underlying interpreter error: always a runtime error or an early return. + error: VmExecutionError, + /// A short description used when logging the failure. + err_type: &'static str, + }, + /// Execution stopped by an abort callback, such as a failed post-condition check. + #[non_exhaustive] + AbortedByCallback { + /// What the output value of the transaction would have been. + output: Option, + /// The asset map evaluated by the abort callback. + assets_modified: AssetMap, + /// Events emitted while processing the transaction. + tx_events: Vec, + /// A human-readable explanation for aborting the transaction. + reason: String, + }, + /// A non-rejectable runtime analysis error in Epoch 2.1 or later. + #[non_exhaustive] + Analysis { error: RuntimeCheckErrorKind }, +} + +/// An execution-phase failure that prevents a transaction from being included in a block. +pub enum RejectedRuntimeTxError { + /// The execution cost and the budget it exceeded. + #[non_exhaustive] + Cost { + cost: ExecutionCost, + budget: ExecutionCost, + }, + /// Execution exceeded a non-consensus resource budget. + #[non_exhaustive] + ExecutionResourceBudgetExceeded { message: String }, + /// A Clarity error with no more specific variant in this enum. + #[non_exhaustive] + Clarity { error: ClarityError }, +} + +/// The authoritative block-inclusion disposition of an execution-phase failure. +// Size is inherited from `IncludedRuntimeTxError`, which is left unboxed as it was in `stackslib`. +#[allow(clippy::large_enum_variant)] +#[must_use] +pub enum ClarityRuntimeTxError { + /// The transaction remains included, charges its fee, and advances its nonces. + Included(IncludedRuntimeTxError), + /// Transaction processing fails and its state changes are not committed. + Rejected(RejectedRuntimeTxError), +} + +impl ClarityRuntimeTxError { + /// Whether the classified failure still results in an included transaction. + pub fn is_included_in_block(&self) -> bool { + matches!(self, Self::Included(_)) + } +} + +/// Classify an execution-phase [`ClarityError`] by its block-inclusion disposition. +pub fn handle_clarity_runtime_error( + error: ClarityError, + epoch_id: StacksEpochId, +) -> ClarityRuntimeTxError { + let included_error = match error { + ClarityError::Interpreter(error @ VmExecutionError::Runtime(..)) => { + IncludedRuntimeTxError::Runtime { + error, + err_type: "runtime error", + } + } + ClarityError::Interpreter(error @ VmExecutionError::EarlyReturn(_)) => { + IncludedRuntimeTxError::Runtime { + error, + err_type: "short return/panic", + } + } + ClarityError::Interpreter(VmExecutionError::RuntimeCheck(runtime_check_err)) => { + if runtime_check_err.rejectable() || epoch_id < StacksEpochId::Epoch21 { + return ClarityRuntimeTxError::Rejected(RejectedRuntimeTxError::Clarity { + error: ClarityError::Interpreter(VmExecutionError::RuntimeCheck( + runtime_check_err, + )), + }); + } + IncludedRuntimeTxError::Analysis { + error: runtime_check_err, + } + } + ClarityError::AbortedByCallback { + output, + assets_modified, + tx_events, + reason, + } => IncludedRuntimeTxError::AbortedByCallback { + output: output.map(|v| *v), + assets_modified: *assets_modified, + tx_events, + reason, + }, + ClarityError::CostError(cost, budget) => { + return ClarityRuntimeTxError::Rejected(RejectedRuntimeTxError::Cost { cost, budget }); + } + ClarityError::ExecutionResourceBudgetExceeded(s) => { + return ClarityRuntimeTxError::Rejected( + RejectedRuntimeTxError::ExecutionResourceBudgetExceeded { message: s }, + ); + } + unhandled_error => { + return ClarityRuntimeTxError::Rejected(RejectedRuntimeTxError::Clarity { + error: unhandled_error, + }); + } + }; + ClarityRuntimeTxError::Included(included_error) +} + +/// The authoritative block-inclusion disposition of a deployment-analysis failure. +#[must_use] +pub enum ClarityAnalysisTxError { + /// The failed deployment remains included and produces a failure receipt. + #[non_exhaustive] + Included { error: ClarityError }, + /// The failed deployment prevents the transaction from being included. + #[non_exhaustive] + Rejected { error: ClarityError }, +} + +impl ClarityAnalysisTxError { + /// Whether the classified analysis failure still results in an included transaction. + pub fn is_included_in_block(&self) -> bool { + matches!(self, Self::Included { .. }) + } +} + +/// Classify a contract-deployment transaction whose analysis phase failed. +pub fn handle_clarity_analysis_error( + error: ClarityError, + epoch_id: StacksEpochId, +) -> ClarityAnalysisTxError { + let is_included = match &error { + ClarityError::CostError(..) | ClarityError::AnalysisResourceBudgetExceeded(_) => false, + ClarityError::Parse(err) => !err.rejectable_in_epoch(epoch_id), + ClarityError::StaticCheck(err) => !err.err.rejectable_in_epoch(epoch_id), + // `analyze_smart_contract` only produces the variants above. Reject any + // unexpected variant instead of manufacturing an invalid included state. + _ => false, + }; + if is_included { + ClarityAnalysisTxError::Included { error } + } else { + ClarityAnalysisTxError::Rejected { error } + } +} + +impl From for ClarityError { + /// Recover the error that was classified. Inverse of the `Included` half of + /// [`handle_clarity_runtime_error`], less the logging label. + fn from(included: IncludedRuntimeTxError) -> Self { + match included { + IncludedRuntimeTxError::Runtime { error, .. } => ClarityError::Interpreter(error), + IncludedRuntimeTxError::AbortedByCallback { + output, + assets_modified, + tx_events, + reason, + } => ClarityError::AbortedByCallback { + output: output.map(Box::new), + assets_modified: Box::new(assets_modified), + tx_events, + reason, + }, + IncludedRuntimeTxError::Analysis { error } => { + ClarityError::Interpreter(VmExecutionError::RuntimeCheck(error)) + } + } + } +} + impl From for ClarityError { fn from(e: StaticCheckError) -> Self { match *e.err { @@ -494,3 +677,251 @@ pub trait TransactionConnection: ClarityConnection { } } } + +#[cfg(test)] +mod unit_tests { + use super::*; + use crate::vm::analysis::errors::StaticCheckErrorKind; + use crate::vm::ast::errors::ParseErrorKind; + use crate::vm::errors::{EarlyReturnError, RuntimeError}; + use crate::vm::events::{STXBurnEventData, STXEventType}; + use crate::vm::types::StandardPrincipalData; + + #[test] + fn runtime_error_disposition_is_authoritative() { + let epoch = StacksEpochId::latest(); + + let runtime = ClarityError::Interpreter(VmExecutionError::Runtime( + RuntimeError::ArithmeticOverflow, + None, + )); + assert!(handle_clarity_runtime_error(runtime, epoch).is_included_in_block()); + + let aborted = ClarityError::AbortedByCallback { + output: None, + assets_modified: Box::new(AssetMap::new()), + tx_events: vec![], + reason: "post-condition".into(), + }; + assert!(handle_clarity_runtime_error(aborted, epoch).is_included_in_block()); + + let cost = ClarityError::CostError(ExecutionCost::ZERO, ExecutionCost::max_value()); + assert!(!handle_clarity_runtime_error(cost, epoch).is_included_in_block()); + let resource = ClarityError::ExecutionResourceBudgetExceeded("too slow".into()); + assert!(!handle_clarity_runtime_error(resource, epoch).is_included_in_block()); + let rejectable = ClarityError::BadTransaction("nope".into()); + assert!(!handle_clarity_runtime_error(rejectable, epoch).is_included_in_block()); + } + + #[test] + fn early_return_is_included_with_its_logging_label() { + let error = ClarityError::Interpreter(VmExecutionError::EarlyReturn( + EarlyReturnError::UnwrapFailed(Box::new(Value::Int(42))), + )); + + match handle_clarity_runtime_error(error, StacksEpochId::latest()) { + ClarityRuntimeTxError::Included(IncludedRuntimeTxError::Runtime { + error, + err_type, + .. + }) => { + assert_eq!(err_type, "short return/panic"); + assert!(matches!( + error, + VmExecutionError::EarlyReturn(EarlyReturnError::UnwrapFailed(value)) + if *value == Value::Int(42) + )); + } + _ => panic!("early returns must be included as acceptable runtime errors"), + } + } + + #[test] + fn aborted_by_callback_payload_round_trips() { + let sender = PrincipalData::Standard(StandardPrincipalData::transient()); + let mut assets_modified = AssetMap::new(); + assets_modified + .add_stx_burn(&sender, 123) + .expect("valid STX burn"); + let tx_events = vec![StacksTransactionEvent::STXEvent( + STXEventType::STXBurnEvent(STXBurnEventData { + sender, + amount: 123, + }), + )]; + + let error = ClarityError::AbortedByCallback { + output: Some(Box::new(Value::Int(42))), + assets_modified: Box::new(assets_modified.clone()), + tx_events: tx_events.clone(), + reason: "post-condition failed".into(), + }; + + match handle_clarity_runtime_error(error, StacksEpochId::latest()) { + ClarityRuntimeTxError::Included(IncludedRuntimeTxError::AbortedByCallback { + output, + assets_modified: classified_assets, + tx_events: classified_events, + reason, + .. + }) => { + assert_eq!(output, Some(Value::Int(42))); + assert_eq!(classified_assets, assets_modified); + assert_eq!(classified_events, tx_events); + assert_eq!(reason, "post-condition failed"); + } + _ => panic!("callback aborts must preserve their payload and remain included"), + } + } + + #[test] + fn analysis_error_inclusion_is_gated_on_epoch_21() { + let err = || { + ClarityError::Interpreter(VmExecutionError::RuntimeCheck( + RuntimeCheckErrorKind::ValueTooLarge, + )) + }; + + assert!( + !handle_clarity_runtime_error(err(), StacksEpochId::Epoch20).is_included_in_block() + ); + assert!( + !handle_clarity_runtime_error(err(), StacksEpochId::Epoch2_05).is_included_in_block() + ); + assert!(handle_clarity_runtime_error(err(), StacksEpochId::Epoch21).is_included_in_block()); + assert!( + handle_clarity_runtime_error(err(), StacksEpochId::latest()).is_included_in_block() + ); + } + + /// Runtime-check errors are classified by `rejectable() || epoch_id < Epoch21`. The test + /// above pins the epoch half; this pins the `rejectable()` half. Every epoch used here is + /// >= 2.1, so the epoch half is false and only `rejectable()` can reject. + #[test] + fn rejectable_runtime_checks_are_rejected_in_every_epoch() { + for epoch in [ + StacksEpochId::Epoch21, + StacksEpochId::Epoch33, + StacksEpochId::latest(), + ] { + // `RuntimeCheckErrorKind` is not `Clone`, so rebuild the set for each epoch. + let rejectable = [ + RuntimeCheckErrorKind::Unreachable("bug".into()), + RuntimeCheckErrorKind::RestrictAssetsMemoryExceeded(1, 0), + RuntimeCheckErrorKind::PoxStxAssetMapOverwrite, + ]; + + for kind in rejectable { + // Pin the premise: if a kind stops being rejectable, fail here rather than below. + assert!(kind.rejectable(), "{kind:?} is expected to be rejectable"); + + let label = format!("{kind:?}"); + let error = ClarityError::Interpreter(VmExecutionError::RuntimeCheck(kind)); + assert!( + !handle_clarity_runtime_error(error, epoch).is_included_in_block(), + "{label} must never be included in a block, even in {epoch}" + ); + } + } + } + + #[test] + fn analysis_failure_excludes_resource_exhaustion() { + let epoch = StacksEpochId::latest(); + + assert!( + !handle_clarity_analysis_error( + ClarityError::CostError(ExecutionCost::ZERO, ExecutionCost::max_value()), + epoch + ) + .is_included_in_block() + ); + assert!( + !handle_clarity_analysis_error( + ClarityError::AnalysisResourceBudgetExceeded("too slow".into()), + epoch + ) + .is_included_in_block() + ); + assert!( + !handle_clarity_analysis_error( + ClarityError::BadTransaction("not an analysis error".into()), + epoch + ) + .is_included_in_block() + ); + } + + #[test] + fn analysis_failure_excludes_rejectable_errors() { + let epoch = StacksEpochId::latest(); + + // Rejectable in every epoch. + assert!( + !handle_clarity_analysis_error( + ClarityError::Parse(ParseError::new(ParseErrorKind::InterpreterFailure)), + epoch + ) + .is_included_in_block() + ); + assert!( + !handle_clarity_analysis_error( + ClarityError::StaticCheck(StaticCheckError::new( + StaticCheckErrorKind::TraitReferenceChainTooDeep + )), + epoch + ) + .is_included_in_block() + ); + } + + #[test] + fn analysis_failure_includes_ordinary_type_errors() { + let epoch = StacksEpochId::latest(); + + assert!( + handle_clarity_analysis_error( + ClarityError::StaticCheck(StaticCheckError::new( + StaticCheckErrorKind::UnknownFunction("no-such-fn".into()) + )), + epoch + ) + .is_included_in_block() + ); + } + + #[test] + fn analysis_failure_includes_ordinary_parse_errors() { + let epoch = StacksEpochId::latest(); + let parse_error = ParseError::new(ParseErrorKind::SeparatorExpected("token".into())); + assert!(!parse_error.rejectable_in_epoch(epoch)); + + match handle_clarity_analysis_error(ClarityError::Parse(parse_error), epoch) { + ClarityAnalysisTxError::Included { + error: ClarityError::Parse(error), + .. + } => assert!(matches!( + *error.err, + ParseErrorKind::SeparatorExpected(ref token) if token == "token" + )), + _ => panic!("ordinary parse errors must produce included analysis failures"), + } + } + + /// `SupertypeTooLarge` stops being rejectable at 3.4. + #[test] + fn analysis_failure_rejectability_can_change_with_epoch() { + let err = || { + ClarityError::StaticCheck(StaticCheckError::new( + StaticCheckErrorKind::SupertypeTooLarge, + )) + }; + + assert!( + !handle_clarity_analysis_error(err(), StacksEpochId::Epoch33).is_included_in_block() + ); + assert!( + handle_clarity_analysis_error(err(), StacksEpochId::Epoch34).is_included_in_block() + ); + } +} diff --git a/stackslib/src/chainstate/stacks/db/transactions.rs b/stackslib/src/chainstate/stacks/db/transactions.rs index 5b4884d83ae..27ee82ad7fa 100644 --- a/stackslib/src/chainstate/stacks/db/transactions.rs +++ b/stackslib/src/chainstate/stacks/db/transactions.rs @@ -18,6 +18,11 @@ use std::collections::{HashMap, HashSet}; use clarity::vm::analysis::types::ContractAnalysis; use clarity::vm::clarity::TransactionConnection; +// Re-exported to keep the old import paths working. +pub use clarity::vm::clarity::{ + handle_clarity_analysis_error, handle_clarity_runtime_error, ClarityAnalysisTxError, + ClarityRuntimeTxError, IncludedRuntimeTxError, RejectedRuntimeTxError, +}; use clarity::vm::contexts::{AssetMap, AssetMapEntry, ExecutionState, InvocationContext}; use clarity::vm::costs::cost_functions::ClarityCostFunction; use clarity::vm::costs::{runtime_cost, CostTracker, ExecutionCost}; @@ -387,71 +392,6 @@ impl From for MemPoolRejection { } } -pub enum ClarityRuntimeTxError { - Acceptable { - error: ClarityError, - err_type: &'static str, - }, - AbortedByCallback { - /// What the output value of the transaction would have been. - /// This will be a Some for contract-calls, and None for contract initialization txs. - output: Option, - /// The asset map which was evaluated by the abort callback - assets_modified: AssetMap, - /// The events from the transaction processing - tx_events: Vec, - /// A human-readable explanation for aborting the transaction - reason: String, - }, - CostError(ExecutionCost, ExecutionCost), - AnalysisError(RuntimeCheckErrorKind), - ExecutionResourceBudgetExceeded(String), - Rejectable(ClarityError), -} - -pub fn handle_clarity_runtime_error(error: ClarityError) -> ClarityRuntimeTxError { - match error { - // runtime errors are okay - ClarityError::Interpreter(VmExecutionError::Runtime(_, _)) => { - ClarityRuntimeTxError::Acceptable { - error, - err_type: "runtime error", - } - } - ClarityError::Interpreter(VmExecutionError::EarlyReturn(_)) => { - ClarityRuntimeTxError::Acceptable { - error, - err_type: "short return/panic", - } - } - ClarityError::Interpreter(VmExecutionError::RuntimeCheck(runtime_check_err)) => { - if runtime_check_err.rejectable() { - ClarityRuntimeTxError::Rejectable(ClarityError::Interpreter( - VmExecutionError::RuntimeCheck(runtime_check_err), - )) - } else { - ClarityRuntimeTxError::AnalysisError(runtime_check_err) - } - } - ClarityError::AbortedByCallback { - output, - assets_modified, - tx_events, - reason, - } => ClarityRuntimeTxError::AbortedByCallback { - output: output.map(|v| *v), - assets_modified: *assets_modified, - tx_events, - reason, - }, - ClarityError::CostError(cost, budget) => ClarityRuntimeTxError::CostError(cost, budget), - ClarityError::ExecutionResourceBudgetExceeded(s) => { - ClarityRuntimeTxError::ExecutionResourceBudgetExceeded(s) - } - unhandled_error => ClarityRuntimeTxError::Rejectable(unhandled_error), - } -} - /// Log and count unreachable ClarityError variants that should never occur in production. fn log_unreachable_error(error: &ClarityError, txid: &Txid) { match error { @@ -1383,8 +1323,13 @@ impl StacksChainState { } Err(e) => { log_unreachable_error(&e, &tx.txid()); - match handle_clarity_runtime_error(e) { - ClarityRuntimeTxError::Acceptable { error, err_type } => { + let runtime_err = handle_clarity_runtime_error(e, epoch_id); + match runtime_err { + ClarityRuntimeTxError::Included(IncludedRuntimeTxError::Runtime { + error, + err_type, + .. + }) => { info!("Contract-call processed with {}", err_type; "txid" => %tx.txid(), "origin" => %origin_account.principal, @@ -1400,12 +1345,15 @@ impl StacksChainState { Some(error.to_string()), ) } - ClarityRuntimeTxError::AbortedByCallback { - output, - assets_modified, - tx_events, - reason, - } => { + ClarityRuntimeTxError::Included( + IncludedRuntimeTxError::AbortedByCallback { + output, + assets_modified, + tx_events, + reason, + .. + }, + ) => { info!("Contract-call aborted by post-condition"; "txid" => %tx.txid(), "origin" => %origin_account.principal, @@ -1423,7 +1371,11 @@ impl StacksChainState { ); return Ok(receipt); } - ClarityRuntimeTxError::CostError(cost_after, budget) => { + ClarityRuntimeTxError::Rejected(RejectedRuntimeTxError::Cost { + cost: cost_after, + budget, + .. + }) => { warn!("Block compute budget exceeded: if included, this will invalidate a block"; "txid" => %tx.txid(), "cost" => %cost_after, "budget" => %budget); return Err(Error::CostOverflowError( cost_before, @@ -1431,11 +1383,11 @@ impl StacksChainState { budget, )); } - ClarityRuntimeTxError::AnalysisError(runtime_check_err) => { - if epoch_id >= StacksEpochId::Epoch21 { - // in 2.1 and later, this is a permitted runtime error. take the - // fee from the payer and keep the tx. - info!("Contract-call encountered an analysis error at runtime"; + ClarityRuntimeTxError::Included(IncludedRuntimeTxError::Analysis { + error: runtime_check_err, + .. + }) => { + info!("Contract-call encountered an analysis error at runtime"; "txid" => %tx.txid(), "origin" => %origin_account.principal, "origin_nonce" => %origin_account.nonce, @@ -1444,29 +1396,20 @@ impl StacksChainState { "function_args" => %VecDisplay(&contract_call.function_args), "error" => %runtime_check_err); - let receipt = - StacksTransactionReceipt::from_runtime_failure_contract_call( - tx.clone(), - total_cost, - runtime_check_err, - ); - return Ok(receipt); - } else { - // prior to 2.1, this is not permitted in a block. - warn!("Unexpected analysis error invalidating transaction: if included, this will invalidate a block"; - "txid" => %tx.txid(), - "origin" => %origin_account.principal, - "origin_nonce" => %origin_account.nonce, - "contract_name" => %contract_id, - "function_name" => %contract_call.function_name, - "function_args" => %VecDisplay(&contract_call.function_args), - "error" => %runtime_check_err); - return Err(Error::ClarityError(ClarityError::Interpreter( - VmExecutionError::RuntimeCheck(runtime_check_err), - ))); - } + let receipt = + StacksTransactionReceipt::from_runtime_failure_contract_call( + tx.clone(), + total_cost, + runtime_check_err, + ); + return Ok(receipt); } - ClarityRuntimeTxError::ExecutionResourceBudgetExceeded(s) => { + ClarityRuntimeTxError::Rejected( + RejectedRuntimeTxError::ExecutionResourceBudgetExceeded { + message: s, + .. + }, + ) => { warn!("Transaction exceeded miner execution resource limit; will be dropped from mempool"; "error" => s.clone(), "txid" => %tx.txid(), @@ -1477,7 +1420,10 @@ impl StacksChainState { "function_args" => %VecDisplay(&contract_call.function_args)); return Err(Error::ExecutionResourceBudgetExceeded(s)); } - ClarityRuntimeTxError::Rejectable(e) => { + ClarityRuntimeTxError::Rejected(RejectedRuntimeTxError::Clarity { + error: e, + .. + }) => { error!("Unexpected error in validating transaction: if included, this will invalidate a block"; "txid" => %tx.txid(), "origin" => %origin_account.principal, @@ -1547,49 +1493,43 @@ impl StacksChainState { Ok(x) => x, Err(e) => { log_unreachable_error(&e, &tx.txid()); - match e { - ClarityError::CostError(ref cost_after, ref budget) => { - warn!( - "Block compute budget exceeded on {}: cost before={}, after={}, budget={}", - tx.txid(), - &cost_before, - cost_after, - budget - ); - return Err(Error::CostOverflowError( - cost_before, - cost_after.clone(), - budget.clone(), - )); - } - ClarityError::AnalysisResourceBudgetExceeded(s) => { - // The analysis phase exceeded its wall-clock deadline or allocation limit (on a voting path only). - warn!("Contract analysis exceeded the analysis resource budget; tx will be dropped from the mempool"; - "error" => s.clone(), - "txid" => %tx.txid(), - "contract_name" => %contract_id, - ); - return Err(Error::AnalysisResourceBudgetExceeded(s)); - } - other_error => { - if let ClarityError::Parse(err) = &other_error { - if err.rejectable_in_epoch(clarity_tx.get_epoch()) { - info!( - "Transaction {} is problematic and should have prevented this block from being relayed", - tx.txid() + match handle_clarity_analysis_error(e, clarity_tx.get_epoch()) { + ClarityAnalysisTxError::Rejected { + error: rejected, .. + } => match rejected { + ClarityError::CostError(cost_after, budget) => { + warn!( + "Block compute budget exceeded on {}: cost before={}, after={}, budget={}", + tx.txid(), + &cost_before, + &cost_after, + &budget ); - return Err(Error::ClarityError(other_error)); - } + return Err(Error::CostOverflowError( + cost_before, + cost_after, + budget, + )); + } + ClarityError::AnalysisResourceBudgetExceeded(s) => { + warn!("Contract analysis exceeded the analysis resource budget; tx will be dropped from the mempool"; + "error" => s.clone(), + "txid" => %tx.txid(), + "contract_name" => %contract_id, + ); + return Err(Error::AnalysisResourceBudgetExceeded(s)); } - if let ClarityError::StaticCheck(err) = &other_error { - if err.err.rejectable_in_epoch(clarity_tx.get_epoch()) { - info!( + other_error => { + info!( "Transaction {} is problematic and should have prevented this block from being relayed", tx.txid() ); - return Err(Error::ClarityError(other_error)); - } + return Err(Error::ClarityError(other_error)); } + }, + ClarityAnalysisTxError::Included { + error: other_error, .. + } => { // this analysis isn't free -- convert to runtime error let mut analysis_cost = clarity_tx.cost_so_far(); analysis_cost @@ -1656,8 +1596,13 @@ impl StacksChainState { } Err(e) => { log_unreachable_error(&e, &tx.txid()); - match handle_clarity_runtime_error(e) { - ClarityRuntimeTxError::Acceptable { error, err_type } => { + let runtime_err = handle_clarity_runtime_error(e, epoch_id); + match runtime_err { + ClarityRuntimeTxError::Included(IncludedRuntimeTxError::Runtime { + error, + err_type, + .. + }) => { info!("Smart-contract processed with {}", err_type; "txid" => %tx.txid(), "contract" => %contract_id, @@ -1681,12 +1626,14 @@ impl StacksChainState { }; return Ok(receipt); } - ClarityRuntimeTxError::AbortedByCallback { - assets_modified, - tx_events, - reason, - .. - } => { + ClarityRuntimeTxError::Included( + IncludedRuntimeTxError::AbortedByCallback { + assets_modified, + tx_events, + reason, + .. + }, + ) => { let receipt = StacksTransactionReceipt::from_condition_aborted_smart_contract( tx.clone(), @@ -1698,7 +1645,11 @@ impl StacksChainState { ); return Ok(receipt); } - ClarityRuntimeTxError::CostError(cost_after, budget) => { + ClarityRuntimeTxError::Rejected(RejectedRuntimeTxError::Cost { + cost: cost_after, + budget, + .. + }) => { warn!("Block compute budget exceeded: if included, this will invalidate a block"; "txid" => %tx.txid(), "cost" => %cost_after, @@ -1709,42 +1660,40 @@ impl StacksChainState { budget, )); } - ClarityRuntimeTxError::AnalysisError(runtime_check_err) => { - if epoch_id >= StacksEpochId::Epoch21 { - // in 2.1 and later, this is a permitted runtime error. take the - // fee from the payer and keep the tx. - info!("Smart-contract encountered an analysis error at runtime"; + ClarityRuntimeTxError::Included(IncludedRuntimeTxError::Analysis { + error: runtime_check_err, + .. + }) => { + info!("Smart-contract encountered an analysis error at runtime"; "txid" => %tx.txid(), "contract" => %contract_id, "error" => %runtime_check_err); - let receipt = - StacksTransactionReceipt::from_runtime_failure_smart_contract( - tx.clone(), - total_cost, - contract_analysis, - runtime_check_err, - ); - return Ok(receipt); - } else { - // prior to 2.1, this is not permitted in a block. - warn!("Unexpected analysis error invalidating transaction: if included, this will invalidate a block"; - "txid" => %tx.txid(), - "contract" => %contract_id, - "error" => %runtime_check_err); - return Err(Error::ClarityError(ClarityError::Interpreter( - VmExecutionError::RuntimeCheck(runtime_check_err), - ))); - } + let receipt = + StacksTransactionReceipt::from_runtime_failure_smart_contract( + tx.clone(), + total_cost, + contract_analysis, + runtime_check_err, + ); + return Ok(receipt); } - ClarityRuntimeTxError::ExecutionResourceBudgetExceeded(s) => { + ClarityRuntimeTxError::Rejected( + RejectedRuntimeTxError::ExecutionResourceBudgetExceeded { + message: s, + .. + }, + ) => { warn!("Transaction exceeded miner execution resource limit; will be dropped from mempool"; "error" => s.clone(), "txid" => %tx.txid(), "contract" => %contract_id); return Err(Error::ExecutionResourceBudgetExceeded(s)); } - ClarityRuntimeTxError::Rejectable(e) => { + ClarityRuntimeTxError::Rejected(RejectedRuntimeTxError::Clarity { + error: e, + .. + }) => { error!("Unexpected error invalidating transaction: if included, this will invalidate a block"; "txid" => %tx.txid(), "contract_name" => %contract_id, diff --git a/stackslib/src/chainstate/stacks/miner.rs b/stackslib/src/chainstate/stacks/miner.rs index f7dafc55f21..dd7dcfa6626 100644 --- a/stackslib/src/chainstate/stacks/miner.rs +++ b/stackslib/src/chainstate/stacks/miner.rs @@ -23,7 +23,6 @@ use std::thread::ThreadId; use std::time::Instant; use clarity::vm::database::BurnStateDB; -use clarity::vm::errors::VmExecutionError; use clarity::vm::resource_limiter::ResourceBudget; use serde::Deserialize; use stacks_common::codec::StacksMessageCodec; @@ -44,6 +43,7 @@ use crate::chainstate::stacks::address::StacksAddressExtensions; use crate::chainstate::stacks::db::blocks::SetupBlockResult; use crate::chainstate::stacks::db::transactions::{ finalize_failed_transaction, handle_clarity_runtime_error, ClarityRuntimeTxError, + RejectedRuntimeTxError, }; use crate::chainstate::stacks::db::unconfirmed::UnconfirmedState; use crate::chainstate::stacks::db::{ChainstateTx, ClarityTx, StacksChainState}; @@ -665,50 +665,25 @@ impl TransactionResult { epoch_id: StacksEpochId, ) -> (bool, Error) { let error = match error { - Error::ClarityError(e) => match handle_clarity_runtime_error(e) { - ClarityRuntimeTxError::Rejectable(e) => { + Error::ClarityError(e) => match handle_clarity_runtime_error(e, epoch_id) { + ClarityRuntimeTxError::Rejected(RejectedRuntimeTxError::Clarity { + error: e, + .. + }) => { // this transaction would invalidate the whole block, so don't re-consider it info!("Problematic transaction would invalidate the block, so dropping from mempool"; "txid" => %tx.txid(), "error" => %e); return (true, Error::ClarityError(e)); } - // recover original ClarityError - ClarityRuntimeTxError::Acceptable { error, .. } => { - if let ClarityError::Parse(ref parse_err) = error { - info!("Parse error: {}", parse_err; "txid" => %tx.txid()); - if parse_err.rejectable_in_epoch(epoch_id) { - info!("Problematic transaction failed parse checks"; "txid" => %tx.txid()); - return (true, Error::ClarityError(error)); - } - } - Error::ClarityError(error) - } - ClarityRuntimeTxError::CostError(cost, budget) => { - Error::ClarityError(ClarityError::CostError(cost, budget)) - } - ClarityRuntimeTxError::AnalysisError(e) => { - let clarity_err = Error::ClarityError(ClarityError::Interpreter( - VmExecutionError::RuntimeCheck(e), - )); - if epoch_id < StacksEpochId::Epoch21 { - // this would invalidate the block, so it's problematic - return (true, clarity_err); - } else { - // in 2.1 and later, this can be mined - clarity_err - } - } - ClarityRuntimeTxError::AbortedByCallback { - output, - assets_modified, - tx_events, - reason, - } => Error::ClarityError(ClarityError::AbortedByCallback { - output: output.map(Box::new), - assets_modified: Box::new(assets_modified), - tx_events, - reason, - }), - ClarityRuntimeTxError::ExecutionResourceBudgetExceeded(s) => { + // An included failure is still mineable: recover the original `ClarityError`. + ClarityRuntimeTxError::Included(included) => Error::ClarityError(included.into()), + ClarityRuntimeTxError::Rejected(RejectedRuntimeTxError::Cost { + cost, + budget, + .. + }) => Error::ClarityError(ClarityError::CostError(cost, budget)), + ClarityRuntimeTxError::Rejected( + RejectedRuntimeTxError::ExecutionResourceBudgetExceeded { message: s, .. }, + ) => { // This transaction took too long to execute or used too much heap memory. Consider it problematic. info!("Problematic transaction caused ExecutionResourceBudgetExceeded"; "error" => s.clone(), diff --git a/stackslib/src/chainstate/tests/madhouse/commands/postcond.rs b/stackslib/src/chainstate/tests/madhouse/commands/postcond.rs index 1841fb06be4..1d670506ce1 100644 --- a/stackslib/src/chainstate/tests/madhouse/commands/postcond.rs +++ b/stackslib/src/chainstate/tests/madhouse/commands/postcond.rs @@ -220,7 +220,7 @@ impl Command for CallRes /// but the combined total exceeds it: `max(transfer, burn) <= allowance < /// transfer + burn`. /// -/// Pre-Epoch34: `VmInternalError::Expect` -> `Rejectable` -> block rejected. +/// Pre-Epoch34: `VmInternalError::Expect` -> `RejectedRuntimeTxError::Clarity` -> block rejected. /// /// Epoch34: clean `(err u0)` Clarity response, effects rolled back. pub struct CallRestrictWithStxCombinedExceeds { diff --git a/stackslib/src/chainstate/tests/madhouse/mod.rs b/stackslib/src/chainstate/tests/madhouse/mod.rs index 516644d9ab6..b5d56ae6beb 100644 --- a/stackslib/src/chainstate/tests/madhouse/mod.rs +++ b/stackslib/src/chainstate/tests/madhouse/mod.rs @@ -25,7 +25,7 @@ use proptest::prelude::Strategy; use self::commands::*; use self::context::Epoch33ToEpoch34TestContext; -/// Pre-Epoch34 returns a block-invalidating `Rejectable` error; Epoch34 +/// Pre-Epoch34 returns a block-invalidating error; Epoch34 /// returns `(err u0)` with effects rolled back. Both "safe" (within allowance) /// and "combined-exceeds" (each op passes individually, combined total exceeds) /// are exercised in each epoch. @@ -37,7 +37,7 @@ fn scenario_with_stx_postconditions() { ctx, // -- Deploy (Epoch33) -- DeployContractLvlPostCondContract, - // -- Epoch33: combined-exceeds -> Rejectable -- + // -- Epoch33: combined-exceeds -> rejected -- CallRestrictWithStxSafe, CallRestrictWithStxCombinedExceeds, CallAsContractWithStxSafe,