only block sui verifier on timeouts (MystenLabs#12030)

crates/sui-core/src/unit_tests/move_package_tests.rs

@@ -579,12 +579,10 @@ async fn test_metered_move_bytecode_verifier() {
     );
     let elapsed = timer_start.elapsed().as_micros() as f64 / (1000.0 * 1000.0);
 
-    assert!(
-        r.unwrap_err()
-            == SuiError::ModuleVerificationFailure {
-                error: "Verification timedout".to_string()
-            }
-    );
+    assert!(matches!(
+        r.unwrap_err(),
+        SuiError::ModuleVerificationFailure { .. }
+    ));
 
     // Some new modules might have passed
     assert!(

crates/sui-core/tests/staged/sui.yaml

@@ -356,6 +356,8 @@ ExecutionFailureStatus:
           - max_size: U64
     29:
       CertificateDenied: UNIT
+    30:
+      SuiMoveVerificationTimedout: UNIT
 ExecutionStatus:
   ENUM:
     0:

crates/sui-types/src/execution_status.rs

@@ -171,6 +171,12 @@ pub enum ExecutionFailureStatus {
 
     #[error("Certificate is on the deny list")]
     CertificateDenied,
+
+    #[error(
+        "Sui Move Bytecode Verification Timeout. \
+        Please run the Sui Move Verifier for more information."
+    )]
+    SuiMoveVerificationTimedout,
     // NOTE: if you want to add a new enum,
     // please add it at the end for Rust SDK backward compatibility.
 }

sui-execution/latest/sui-adapter/src/adapter.rs

@@ -7,14 +7,15 @@ use anyhow::Result;
 use move_binary_format::{access::ModuleAccess, file_format::CompiledModule};
 use move_bytecode_verifier::meter::Meter;
 use move_bytecode_verifier::{verify_module_with_config_metered, VerifierConfig};
-use move_core_types::{account_address::AccountAddress, vm_status::StatusCode};
+use move_core_types::account_address::AccountAddress;
 pub use move_vm_runtime::move_vm::MoveVM;
 use move_vm_runtime::{
     config::{VMConfig, VMRuntimeLimitsConfig},
     native_extensions::NativeContextExtensions,
     native_functions::NativeFunctionTable,
 };
 use sui_types::metrics::BytecodeVerifierMetrics;
+use sui_verifier::check_for_verifier_timeout;
 use tracing::instrument;
 
 use sui_move_natives::{object_runtime::ObjectRuntime, NativesCostTable};
@@ -27,7 +28,7 @@ use sui_types::{
     object::Owner,
     storage::ChildObjectResolver,
 };
-use sui_verifier::verifier::sui_verify_module_metered;
+use sui_verifier::verifier::sui_verify_module_metered_check_timeout_only;
 
 sui_macros::checked_arithmetic! {
 
@@ -212,29 +213,24 @@ pub fn run_metered_move_bytecode_verifier_impl(
 
         if let Err(e) = verify_module_with_config_metered(verifier_config, module, meter) {
             // Check that the status indicates mtering timeout
-            if [
-                StatusCode::PROGRAM_TOO_COMPLEX,
-                StatusCode::TOO_MANY_BACK_EDGES,
-            ]
-            .contains(&e.major_status())
-            {
+            if check_for_verifier_timeout(&e.major_status()) {
                 // Discard success timer, but record timeout/failure timer
                 metrics.verifier_runtime_per_module_timeout_latency.observe(per_module_meter_verifier_timer.stop_and_discard());
                 metrics.verifier_timeout_metrics.with_label_values(&[BytecodeVerifierMetrics::MOVE_VERIFIER_TAG, BytecodeVerifierMetrics::TIMEOUT_TAG]).inc();
                 return Err(SuiError::ModuleVerificationFailure {
-                    error: "Verification timedout".to_string(),
+                    error: format!("Verification timedout: {}", e),
                 });
             };
-        } else if let Err(err) = sui_verify_module_metered(protocol_config, module, &BTreeMap::new(), meter) {
-                // Discard success timer, but record timeout/failure timer
-                metrics.verifier_runtime_per_module_timeout_latency.observe(per_module_meter_verifier_timer.stop_and_discard());
-                metrics.verifier_timeout_metrics.with_label_values(&[ BytecodeVerifierMetrics::SUI_VERIFIER_TAG, BytecodeVerifierMetrics::TIMEOUT_TAG]).inc();
-                return Err(err.into());
-        } else {
-            // Save the success timer
-            per_module_meter_verifier_timer.stop_and_record();
-            metrics.verifier_timeout_metrics.with_label_values(&[BytecodeVerifierMetrics::OVERALL_TAG, BytecodeVerifierMetrics::SUCCESS_TAG]).inc();
+        } else if let Err(err) = sui_verify_module_metered_check_timeout_only(protocol_config, module, &BTreeMap::new(), meter) {
+            // We only checked that the failure was due to timeout
+            // Discard success timer, but record timeout/failure timer
+            metrics.verifier_runtime_per_module_timeout_latency.observe(per_module_meter_verifier_timer.stop_and_discard());
+            metrics.verifier_timeout_metrics.with_label_values(&[ BytecodeVerifierMetrics::SUI_VERIFIER_TAG, BytecodeVerifierMetrics::TIMEOUT_TAG]).inc();
+            return Err(err.into());
         }
+        // Save the success timer
+        per_module_meter_verifier_timer.stop_and_record();
+        metrics.verifier_timeout_metrics.with_label_values(&[BytecodeVerifierMetrics::OVERALL_TAG, BytecodeVerifierMetrics::SUCCESS_TAG]).inc();
     }
     Ok(())
 }

sui-execution/latest/sui-verifier/src/id_leak_verifier.rs

@@ -36,7 +36,10 @@ use sui_types::{
     SUI_FRAMEWORK_ADDRESS, SUI_SYSTEM_ADDRESS,
 };
 
-use crate::{verification_failure, TEST_SCENARIO_MODULE_NAME};
+use crate::{
+    check_for_verifier_timeout, to_verification_timeout_error, verification_failure,
+    TEST_SCENARIO_MODULE_NAME,
+};
 pub(crate) const JOIN_BASE_COST: u128 = 10;
 pub(crate) const JOIN_PER_LOCAL_COST: u128 = 5;
 pub(crate) const STEP_BASE_COST: u128 = 15;
@@ -115,7 +118,10 @@ fn verify_id_leak(module: &CompiledModule, meter: &mut impl Meter) -> Result<(),
         verifier
             .analyze_function(initial_state, &func_view, meter)
             .map_err(|err| {
-                if let Some(message) = err.source().as_ref() {
+                // Handle verifificaiton timeout specially
+                if check_for_verifier_timeout(&err.major_status()) {
+                    to_verification_timeout_error(err.to_string())
+                } else if let Some(message) = err.source().as_ref() {
                     let function_name = binary_view
                         .identifier_at(binary_view.function_handle_at(func_def.function).name);
                     let module_name = module.self_id();

sui-execution/latest/sui-verifier/src/lib.rs

@@ -11,7 +11,7 @@ pub mod one_time_witness_verifier;
 pub mod private_generics;
 pub mod struct_with_key_verifier;
 
-use move_core_types::{ident_str, identifier::IdentStr};
+use move_core_types::{ident_str, identifier::IdentStr, vm_status::StatusCode};
 use sui_types::error::{ExecutionError, ExecutionErrorKind};
 
 pub const INIT_FN_NAME: &IdentStr = ident_str!("init");
@@ -20,3 +20,19 @@ pub const TEST_SCENARIO_MODULE_NAME: &str = "test_scenario";
 fn verification_failure(error: String) -> ExecutionError {
     ExecutionError::new_with_source(ExecutionErrorKind::SuiMoveVerificationError, error)
 }
+
+fn to_verification_timeout_error(error: String) -> ExecutionError {
+    ExecutionError::new_with_source(ExecutionErrorKind::SuiMoveVerificationTimedout, error)
+}
+
+/// Runs the Move verifier and checks if the error counts as a Move verifier timeout
+/// NOTE: this function only check if the verifier error is a timeout
+/// All other errors are ignored
+pub fn check_for_verifier_timeout(major_status_code: &StatusCode) -> bool {
+    [
+        StatusCode::PROGRAM_TOO_COMPLEX,
+        // Do we want to make this a substatus of `PROGRAM_TOO_COMPLEX`?
+        StatusCode::TOO_MANY_BACK_EDGES,
+    ]
+    .contains(major_status_code)
+}

sui-execution/latest/sui-verifier/src/verifier.rs

@@ -29,10 +29,42 @@ pub fn sui_verify_module_metered(
     one_time_witness_verifier::verify_module(module, fn_info_map)
 }
 
+/// Runs the Sui verifier and checks if the error counts as a Sui verifier timeout
+/// NOTE: this function only check if the verifier error is a timeout
+/// All other errors are ignored
+pub fn sui_verify_module_metered_check_timeout_only(
+    config: &ProtocolConfig,
+    module: &CompiledModule,
+    fn_info_map: &FnInfoMap,
+    meter: &mut impl Meter,
+) -> Result<(), ExecutionError> {
+    // Checks if the error counts as a Sui verifier timeout
+    if let Err(error) = sui_verify_module_metered(config, module, fn_info_map, meter) {
+        if matches!(
+            error.kind(),
+            sui_types::execution_status::ExecutionFailureStatus::SuiMoveVerificationTimedout
+        ) {
+            return Err(error);
+        }
+    }
+    // Any other scenario, including a non-timeout error counts as Ok
+    Ok(())
+}
+
 pub fn sui_verify_module_unmetered(
     config: &ProtocolConfig,
     module: &CompiledModule,
     fn_info_map: &FnInfoMap,
 ) -> Result<(), ExecutionError> {
-    sui_verify_module_metered(config, module, fn_info_map, &mut DummyMeter)
+    sui_verify_module_metered(config, module, fn_info_map, &mut DummyMeter).map_err(|err| {
+        // We must never see timeout error in execution
+        debug_assert!(
+            !matches!(
+                err.kind(),
+                sui_types::execution_status::ExecutionFailureStatus::SuiMoveVerificationTimedout
+            ),
+            "Unexpected timeout error in execution"
+        );
+        err
+    })
 }