Merge pull request #6717 from jacinta-stacks/chore/aac-runtime-check-error-consensus-tests

AAC Testing: Runtime `CheckErrorKind` batch number 1

Author: francesco-stacks

clarity-types/src/errors/analysis.rs

@@ -480,10 +480,6 @@ pub enum CheckErrorKind {
     /// String contains invalid UTF-8 encoding.
     InvalidUTF8Encoding,
 
-    // secp256k1 signature
-    /// Invalid secp256k1 signature provided in an expression.
-    InvalidSecp65k1Signature,
-
     /// Attempt to write to contract state in a read-only function.
     WriteAttemptedInReadOnly,
     /// `at-block` closure must be read-only but contains write operations.
@@ -814,7 +810,6 @@ impl DiagnosableError for CheckErrorKind {
             CheckErrorKind::TraitTooManyMethods(found, allowed) => format!("too many trait methods specified: found {found}, the maximum is {allowed}"),
             CheckErrorKind::InvalidCharactersDetected => "invalid characters detected".into(),
             CheckErrorKind::InvalidUTF8Encoding => "invalid UTF8 encoding".into(),
-            CheckErrorKind::InvalidSecp65k1Signature => "invalid seckp256k1 signature".into(),
             CheckErrorKind::TypeAlreadyAnnotatedFailure | CheckErrorKind::CheckerImplementationFailure => {
                 "internal error - please file an issue on https://github.com/stacks-network/stacks-blockchain".into()
             },

clarity-types/src/tests/types/mod.rs

@@ -585,3 +585,46 @@ fn test_utf8_data_len_returns_vm_internal_error() {
         err
     );
 }
+
+#[test]
+fn invalid_utf8_encoding_from_oob_unicode_escape() {
+    // This is a syntactically valid escape: \u{HEX}
+    // BUT 110000 > 10FFFF (max Unicode scalar)
+    // So oob Unicode → char::from_u32(None) → InvalidUTF8Encoding
+    let bad_utf8_literal = "\\u{110000}".to_string();
+
+    let err = Value::string_utf8_from_string_utf8_literal(bad_utf8_literal).unwrap_err();
+    assert!(matches!(
+        err,
+        VmExecutionError::Unchecked(CheckErrorKind::InvalidUTF8Encoding)
+    ));
+}
+
+#[test]
+fn invalid_string_ascii_from_bytes() {
+    // 0xFF is NOT:
+    // - ASCII alphanumeric
+    // - ASCII punctuation
+    // - ASCII whitespace
+    let bad_bytes = vec![0xFF];
+
+    let err = Value::string_ascii_from_bytes(bad_bytes).unwrap_err();
+
+    assert!(matches!(
+        err,
+        VmExecutionError::Unchecked(CheckErrorKind::InvalidCharactersDetected)
+    ));
+}
+
+#[test]
+fn invalid_utf8_string_from_bytes() {
+    // 0x80 is an invalid standalone UTF-8 continuation byte
+    let bad_bytes = vec![0x80];
+
+    let err = Value::string_utf8_from_bytes(bad_bytes).unwrap_err();
+
+    assert!(matches!(
+        err,
+        VmExecutionError::Unchecked(CheckErrorKind::InvalidCharactersDetected)
+    ));
+}

clarity-types/src/types/mod.rs

@@ -1052,6 +1052,8 @@ impl Value {
                     .ok_or_else(|| VmInternalError::Expect("Expected capture".into()))?;
                 let scalar_value = window[matched.start()..matched.end()].to_string();
                 let unicode_char = {
+                    // This first InvalidUTF8Encoding is logically unreachable: the escape regex rejects non-hex digits,
+                    // so from_str_radix only sees valid hex and never errors here.
                     let u = u32::from_str_radix(&scalar_value, 16)
                         .map_err(|_| CheckErrorKind::InvalidUTF8Encoding)?;
                     let c = char::from_u32(u).ok_or_else(|| CheckErrorKind::InvalidUTF8Encoding)?;

clarity/src/vm/contexts.rs

@@ -2087,12 +2087,16 @@ impl CallStack {
 
 #[cfg(test)]
 mod test {
+    use stacks_common::consts::CHAIN_ID_TESTNET;
     use stacks_common::types::chainstate::StacksAddress;
     use stacks_common::util::hash::Hash160;
 
     use super::*;
     use crate::vm::callables::DefineType;
-    use crate::vm::tests::{test_epochs, tl_env_factory, TopLevelMemoryEnvironmentGenerator};
+    use crate::vm::database::MemoryBackingStore;
+    use crate::vm::tests::{
+        test_clarity_versions, test_epochs, tl_env_factory, TopLevelMemoryEnvironmentGenerator,
+    };
     use crate::vm::types::signatures::CallableSubtype;
     use crate::vm::types::StandardPrincipalData;
 
@@ -2432,4 +2436,54 @@ mod test {
             ))
         ));
     }
+
+    #[apply(test_clarity_versions)]
+    fn vm_initialize_contract_already_exists(
+        #[case] version: ClarityVersion,
+        #[case] epoch: StacksEpochId,
+    ) {
+        // --- Setup VM ---
+        let mut marf = MemoryBackingStore::new();
+        let mut global_context = GlobalContext::new(
+            false,
+            CHAIN_ID_TESTNET,
+            marf.as_clarity_db(),
+            LimitedCostTracker::new_free(),
+            StacksEpochId::Epoch21, // any modern epoch
+        );
+
+        let mut call_stack = CallStack::new();
+
+        let contract_context =
+            ContractContext::new(QualifiedContractIdentifier::transient(), version);
+
+        let mut env = Environment::new(
+            &mut global_context,
+            &contract_context,
+            &mut call_stack,
+            None,
+            None,
+            None,
+        );
+
+        let contract_id = QualifiedContractIdentifier::local("dup").unwrap();
+
+        let contract_src = "(define-public (ping) (ok u1))";
+
+        let ast = ast::build_ast(&contract_id, contract_src, &mut env, version, epoch).unwrap();
+
+        // First initialization succeeds
+        env.initialize_contract_from_ast(contract_id.clone(), version, &ast, contract_src)
+            .unwrap();
+
+        // Second initialization hits ContractAlreadyExists
+        let err = env
+            .initialize_contract_from_ast(contract_id.clone(), version, &ast, contract_src)
+            .unwrap_err();
+
+        assert!(matches!(
+            err,
+            VmExecutionError::Unchecked(CheckErrorKind::ContractAlreadyExists(_))
+        ));
+    }
 }

clarity/src/vm/functions/crypto.rs

@@ -193,16 +193,14 @@ pub fn special_secp256k1_recover(
         }
     };
 
-    match secp256k1_recover(message, signature)
-        .map_err(|_| CheckErrorKind::InvalidSecp65k1Signature)
-    {
-        Ok(pubkey) => Ok(Value::okay(
-            Value::buff_from(pubkey.to_vec())
-                .map_err(|_| VmInternalError::Expect("Failed to construct buff".into()))?,
-        )
-        .map_err(|_| VmInternalError::Expect("Failed to construct ok".into()))?),
-        _ => Ok(Value::err_uint(1)),
-    }
+    let Ok(pubkey) = secp256k1_recover(message, signature) else {
+        // We do not return the runtime error. Immediately map this to an error code.
+        return Ok(Value::err_uint(1));
+    };
+    let pubkey_buff = Value::buff_from(pubkey.to_vec())
+        .map_err(|_| VmInternalError::Expect("Failed to construct buff".into()))?;
+    Ok(Value::okay(pubkey_buff)
+        .map_err(|_| VmInternalError::Expect("Failed to construct ok".into()))?)
 }
 
 pub fn special_secp256k1_verify(

clarity/src/vm/functions/define.rs

@@ -460,3 +460,129 @@ pub fn evaluate_define(
         Ok(DefineResult::NoDefine)
     }
 }
+
+#[cfg(test)]
+mod test {
+    use clarity_types::errors::CheckErrorKind;
+    use clarity_types::representations::SymbolicExpression;
+    use clarity_types::types::QualifiedContractIdentifier;
+    use clarity_types::{Value, VmExecutionError};
+    use stacks_common::consts::CHAIN_ID_TESTNET;
+    use stacks_common::types::StacksEpochId;
+
+    use crate::vm::analysis::type_checker::v2_1::MAX_FUNCTION_PARAMETERS;
+    use crate::vm::callables::DefineType;
+    use crate::vm::contexts::GlobalContext;
+    use crate::vm::costs::LimitedCostTracker;
+    use crate::vm::database::MemoryBackingStore;
+    use crate::vm::functions::define::{handle_define_function, handle_define_trait};
+    use crate::vm::tests::test_clarity_versions;
+    use crate::vm::{CallStack, ClarityVersion, ContractContext, Environment, LocalContext};
+
+    #[apply(test_clarity_versions)]
+    fn bad_syntax_binding_define_function(
+        #[case] version: ClarityVersion,
+        #[case] epoch: StacksEpochId,
+    ) {
+        // ---- BAD SIGNATURE ----
+        // Instead of ((x uint)), we pass (x)
+        let bad_signature = vec![
+            SymbolicExpression::atom("f".into()),
+            SymbolicExpression::atom("x".into()), // NOT a (name type) list
+        ];
+
+        let body = SymbolicExpression::atom_value(Value::UInt(1));
+
+        let mut marf = MemoryBackingStore::new();
+        let mut global_context = GlobalContext::new(
+            false,
+            CHAIN_ID_TESTNET,
+            marf.as_clarity_db(),
+            LimitedCostTracker::new_free(),
+            epoch,
+        );
+
+        let contract_context =
+            ContractContext::new(QualifiedContractIdentifier::transient(), version);
+
+        let context = LocalContext::new();
+        let mut call_stack = CallStack::new();
+
+        let mut env = Environment::new(
+            &mut global_context,
+            &contract_context,
+            &mut call_stack,
+            None,
+            None,
+            None,
+        );
+
+        let result = handle_define_function(&bad_signature, &body, &mut env, DefineType::Public);
+
+        assert!(matches!(
+            result,
+            Err(VmExecutionError::Unchecked(
+                CheckErrorKind::BadSyntaxBinding(_)
+            ))
+        ));
+    }
+
+    #[apply(test_clarity_versions)]
+    fn handle_define_trait_too_many_function_parameters(
+        #[case] version: ClarityVersion,
+        #[case] epoch: StacksEpochId,
+    ) {
+        if epoch < StacksEpochId::Epoch33 {
+            return;
+        }
+        // Build a trait method with MORE than MAX_FUNCTION_PARAMETERS arguments
+        // (f (uint uint uint ... ) (response uint uint))
+        let too_many_args =
+            vec![SymbolicExpression::atom("uint".into()); MAX_FUNCTION_PARAMETERS + 1];
+
+        let method = SymbolicExpression::list(vec![
+            SymbolicExpression::atom("f".into()),
+            SymbolicExpression::list(too_many_args),
+            SymbolicExpression::list(vec![
+                SymbolicExpression::atom("response".into()),
+                SymbolicExpression::atom("uint".into()),
+                SymbolicExpression::atom("uint".into()),
+            ]),
+        ]);
+
+        // This is the `( (f (...) (response ...)) )` wrapper
+        let trait_body = vec![SymbolicExpression::list(vec![method])];
+
+        let mut marf = MemoryBackingStore::new();
+        let mut global_context = GlobalContext::new(
+            false,
+            CHAIN_ID_TESTNET,
+            marf.as_clarity_db(),
+            LimitedCostTracker::new_free(),
+            epoch,
+        );
+
+        let contract_context =
+            ContractContext::new(QualifiedContractIdentifier::transient(), version);
+
+        let mut call_stack = CallStack::new();
+
+        let mut env = Environment::new(
+            &mut global_context,
+            &contract_context,
+            &mut call_stack,
+            None,
+            None,
+            None,
+        );
+
+        let result = handle_define_trait(&"bad-trait".into(), &trait_body, &mut env);
+
+        assert!(matches!(
+            result,
+            Err(VmExecutionError::Unchecked(
+                CheckErrorKind::TooManyFunctionParameters(found, max)
+            ))
+        ));
+    }
+}