Merge branch 'develop' into feat/epoch-3.3-tasks

Author: obycode

CHANGELOG.md

@@ -20,6 +20,7 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 - Renamed Clarity 4's new `block-time` to `stacks-block-time`
 - Improve cost-tracking for type-checking function arguments in epoch 3.3 (see [#6425](https://github.com/stacks-network/stacks-core/issues/6425))
 - Replaced `libsecp256k1` with `k256` and `p256` from RustCrypto and removed separate Wasm implementations.
+- Added limits in the type-checker for the number of parameters in functions (maximum 256), and the number of methods in traits (maximum 256). These limits are enforced starting in Epoch 3.3.
 
 ## [3.2.0.0.2]
 

clarity-types/src/errors/analysis.rs

@@ -280,6 +280,7 @@ pub enum CheckErrors {
     DefaultTypesMustMatch(Box<TypeSignature>, Box<TypeSignature>),
     IllegalOrUnknownFunctionApplication(String),
     UnknownFunction(String),
+    TooManyFunctionParameters(usize, usize),
 
     // traits
     NoSuchTrait(String, String),
@@ -294,6 +295,7 @@ pub enum CheckErrors {
     TraitBasedContractCallInReadOnly,
     ContractOfExpectsTrait,
     IncompatibleTrait(Box<TraitIdentifier>, Box<TraitIdentifier>),
+    TraitTooManyMethods(usize, usize),
 
     // strings
     InvalidCharactersDetected,
@@ -587,6 +589,7 @@ impl DiagnosableError for CheckErrors {
             CheckErrors::DefaultTypesMustMatch(type_1, type_2) => format!("expression types passed in 'default-to' must match (got '{type_1}' and '{type_2}')"),
             CheckErrors::IllegalOrUnknownFunctionApplication(function_name) => format!("use of illegal / unresolved function '{function_name}"),
             CheckErrors::UnknownFunction(function_name) => format!("use of unresolved function '{function_name}'"),
+            CheckErrors::TooManyFunctionParameters(found, allowed) => format!("too many function parameters specified: found {found}, the maximum is {allowed}"),
             CheckErrors::TraitBasedContractCallInReadOnly => "use of trait based contract calls are not allowed in read-only context".into(),
             CheckErrors::WriteAttemptedInReadOnly => "expecting read-only statements, detected a writing operation".into(),
             CheckErrors::AtBlockClosureMustBeReadOnly => "(at-block ...) closures expect read-only statements, but detected a writing operation".into(),
@@ -605,6 +608,7 @@ impl DiagnosableError for CheckErrors {
             CheckErrors::TraitReferenceNotAllowed => "trait references can not be stored".into(),
             CheckErrors::ContractOfExpectsTrait => "trait reference expected".into(),
             CheckErrors::IncompatibleTrait(expected_trait, actual_trait) => format!("trait '{actual_trait}' is not a compatible with expected trait, '{expected_trait}'"),
+            CheckErrors::TraitTooManyMethods(found, allowed) => format!("too many trait methods specified: found {found}, the maximum is {allowed}"),
             CheckErrors::InvalidCharactersDetected => "invalid characters detected".into(),
             CheckErrors::InvalidUTF8Encoding => "invalid UTF8 encoding".into(),
             CheckErrors::InvalidSecp65k1Signature => "invalid seckp256k1 signature".into(),

clarity/src/vm/analysis/mod.rs

@@ -55,7 +55,7 @@ pub fn mem_type_check(
 ) -> Result<(Option<TypeSignature>, ContractAnalysis), CheckError> {
     let contract_identifier = QualifiedContractIdentifier::transient();
     let contract = build_ast(&contract_identifier, snippet, &mut (), version, epoch)
-        .map_err(|_| CheckErrors::Expects("Failed to build AST".into()))?
+        .map_err(|e| CheckErrors::Expects(format!("Failed to build AST: {e}")))?
         .expressions;
 
     let mut marf = MemoryBackingStore::new();

clarity/src/vm/analysis/type_checker/v2_1/mod.rs

@@ -57,6 +57,13 @@ use crate::vm::ClarityVersion;
 #[cfg(test)]
 pub mod tests;
 
+/// The maximum number of parameters a function definition can have.
+/// This limit is enforced starting in Epoch 3.3.
+pub const MAX_FUNCTION_PARAMETERS: usize = 256;
+/// The maximum number of methods a trait definition can have.
+/// This limit is enforced starting in Epoch 3.3.
+pub const MAX_TRAIT_METHODS: usize = 256;
+
 /*
 
 Type-checking in our language is achieved through a single-direction inference.
@@ -1293,6 +1300,15 @@ impl<'a, 'b> TypeChecker<'a, 'b> {
         let (function_name, args) = signature
             .split_first()
             .ok_or(CheckErrors::RequiresAtLeastArguments(1, 0))?;
+
+        if self.epoch.limits_parameter_and_method_count() && args.len() > MAX_FUNCTION_PARAMETERS {
+            return Err(CheckErrors::TooManyFunctionParameters(
+                args.len(),
+                MAX_FUNCTION_PARAMETERS,
+            )
+            .into());
+        }
+
         let function_name = function_name
             .match_atom()
             .ok_or(CheckErrors::BadFunctionName)?;
@@ -1677,7 +1693,7 @@ impl<'a, 'b> TypeChecker<'a, 'b> {
         let trait_signature = TypeSignature::parse_trait_type_repr(
             function_types,
             &mut (),
-            StacksEpochId::Epoch21,
+            self.epoch,
             self.clarity_version,
         )?;
 

clarity/src/vm/analysis/type_checker/v2_1/tests/mod.rs

@@ -14,6 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+use clarity_types::token::Token;
 use clarity_types::types::SequenceSubtype;
 #[cfg(test)]
 use rstest::rstest;
@@ -23,6 +24,7 @@ use stacks_common::types::StacksEpochId;
 
 use crate::vm::analysis::errors::{CheckError, CheckErrors, SyntaxBindingError};
 use crate::vm::analysis::mem_type_check as mem_run_analysis;
+use crate::vm::analysis::type_checker::v2_1::{MAX_FUNCTION_PARAMETERS, MAX_TRAIT_METHODS};
 use crate::vm::analysis::types::ContractAnalysis;
 use crate::vm::ast::build_ast;
 use crate::vm::ast::errors::ParseErrors;
@@ -71,13 +73,13 @@ fn type_check_helper(exp: &str) -> Result<TypeSignature, CheckError> {
 fn type_check_helper_version(
     exp: &str,
     version: ClarityVersion,
+    epoch: StacksEpochId,
 ) -> Result<TypeSignature, CheckError> {
-    mem_run_analysis(exp, version, StacksEpochId::latest())
-        .map(|(type_sig_opt, _)| type_sig_opt.unwrap())
+    mem_run_analysis(exp, version, epoch).map(|(type_sig_opt, _)| type_sig_opt.unwrap())
 }
 
 fn type_check_helper_v1(exp: &str) -> Result<TypeSignature, CheckError> {
-    type_check_helper_version(exp, ClarityVersion::Clarity1)
+    type_check_helper_version(exp, ClarityVersion::Clarity1, StacksEpochId::latest())
 }
 
 fn buff_type(size: u32) -> TypeSignature {
@@ -286,7 +288,12 @@ fn test_get_block_info() {
             expected,
             &format!(
                 "{}",
-                type_check_helper_version(good_test, ClarityVersion::Clarity2).unwrap()
+                type_check_helper_version(
+                    good_test,
+                    ClarityVersion::Clarity2,
+                    StacksEpochId::latest()
+                )
+                .unwrap()
             )
         );
     }
@@ -295,15 +302,20 @@ fn test_get_block_info() {
             expected_v210,
             &format!(
                 "{}",
-                type_check_helper_version(good_test_v210, ClarityVersion::Clarity2).unwrap()
+                type_check_helper_version(
+                    good_test_v210,
+                    ClarityVersion::Clarity2,
+                    StacksEpochId::latest()
+                )
+                .unwrap()
             )
         );
     }
 
     for (bad_test, expected) in bad.iter().zip(bad_expected.iter()) {
         assert_eq!(
             *expected,
-            *type_check_helper_version(bad_test, ClarityVersion::Clarity2)
+            *type_check_helper_version(bad_test, ClarityVersion::Clarity2, StacksEpochId::latest())
                 .unwrap_err()
                 .err
         );
@@ -354,6 +366,71 @@ fn test_get_burn_block_info() {
     }
 }
 
+#[apply(test_clarity_versions)]
+fn test_define_functions(#[case] version: ClarityVersion, #[case] epoch: StacksEpochId) {
+    let good = [
+        "(define-private (foo (a uint) (b int)) true)",
+        "(define-public (bar (x (buff 32))) (ok x))",
+        "(define-read-only (baz (p principal)) p)",
+    ];
+
+    for good_test in good.iter() {
+        mem_type_check(good_test).unwrap();
+    }
+
+    // Tests that fail only after epoch 3.3
+    let bad = [
+        format!(
+            "(define-private (foo {}) true)",
+            (0..(MAX_FUNCTION_PARAMETERS + 1))
+                .map(|i| format!("(param-{} uint)", i))
+                .collect::<Vec<String>>()
+                .join(" ")
+        ),
+        format!(
+            "(define-public (foo {}) (ok true))",
+            (0..(MAX_FUNCTION_PARAMETERS + 1))
+                .map(|i| format!("(param-{} uint)", i))
+                .collect::<Vec<String>>()
+                .join(" ")
+        ),
+        format!(
+            "(define-read-only (foo {}) true)",
+            (0..(MAX_FUNCTION_PARAMETERS + 1))
+                .map(|i| format!("(param-{} uint)", i))
+                .collect::<Vec<String>>()
+                .join(" ")
+        ),
+    ];
+    let bad_expected = [
+        CheckErrors::TooManyFunctionParameters(
+            MAX_FUNCTION_PARAMETERS + 1,
+            MAX_FUNCTION_PARAMETERS,
+        ),
+        CheckErrors::TooManyFunctionParameters(
+            MAX_FUNCTION_PARAMETERS + 1,
+            MAX_FUNCTION_PARAMETERS,
+        ),
+        CheckErrors::TooManyFunctionParameters(
+            MAX_FUNCTION_PARAMETERS + 1,
+            MAX_FUNCTION_PARAMETERS,
+        ),
+    ];
+
+    for (bad_test, expected) in bad.iter().zip(bad_expected.iter()) {
+        if epoch.limits_parameter_and_method_count() {
+            assert_eq!(
+                *expected,
+                *type_check_helper_version(bad_test, version, epoch)
+                    .unwrap_err()
+                    .err
+            );
+        } else {
+            mem_run_analysis(bad_test, version, epoch).unwrap();
+        }
+    }
+}
+
 #[apply(test_clarity_versions)]
 fn test_define_trait(#[case] version: ClarityVersion, #[case] epoch: StacksEpochId) {
     let good = [
@@ -371,31 +448,80 @@ fn test_define_trait(#[case] version: ClarityVersion, #[case] epoch: StacksEpoch
         "(define-trait trait-1 ((get-1 uint uint)))",
         "(define-trait trait-1 ((get-1 (uint) (uint))))",
         "(define-trait trait-1 ((get-1 (response uint uint))))",
-        "(define-trait trait-1)",
-        "(define-trait)",
+        "(define-trait trait-1 ((get-1 (uint) (response uint uint)) u1))",
     ];
     let bad_expected = [
         CheckErrors::InvalidTypeDescription,
         CheckErrors::DefineTraitBadSignature,
         CheckErrors::DefineTraitBadSignature,
         CheckErrors::InvalidTypeDescription,
+        CheckErrors::DefineTraitBadSignature,
     ];
 
     for (bad_test, expected) in bad.iter().zip(bad_expected.iter()) {
         assert_eq!(*expected, *type_check_helper(bad_test).unwrap_err().err);
     }
 
-    let bad = ["(define-trait trait-1)", "(define-trait)"];
+    // Tests that fail before type-checker
+    let bad = [
+        "(define-trait trait-1)",
+        "(define-trait)",
+        "(define-trait trait-1 ((get-1 (uint) (response uint uint)))) u1)",
+    ];
     let bad_expected = [
         ParseErrors::DefineTraitBadSignature,
         ParseErrors::DefineTraitBadSignature,
+        if epoch == StacksEpochId::Epoch20 || epoch == StacksEpochId::Epoch2_05 {
+            // the pre-2.1 parser returns less instructive errors
+            ParseErrors::ClosingParenthesisUnexpected
+        } else {
+            ParseErrors::UnexpectedToken(Token::Rparen)
+        },
     ];
 
     let contract_identifier = QualifiedContractIdentifier::transient();
     for (bad_test, expected) in bad.iter().zip(bad_expected.iter()) {
         let res = build_ast(&contract_identifier, bad_test, &mut (), version, epoch).unwrap_err();
         assert_eq!(*expected, *res.err);
     }
+
+    // Tests that fail only after epoch 3.3
+    let bad = [
+        format!(
+            "(define-trait trait-1 ({}))",
+            (0..(MAX_TRAIT_METHODS + 1))
+                .map(|i| format!("(method-{} (uint) (response uint uint))", i))
+                .collect::<Vec<String>>()
+                .join(" ")
+        ),
+        format!(
+            "(define-trait trait-1 ((method ({}) (response uint uint))))",
+            (0..(MAX_FUNCTION_PARAMETERS + 1))
+                .map(|i| "uint".to_string())
+                .collect::<Vec<String>>()
+                .join(" ")
+        ),
+    ];
+    let bad_expected = [
+        CheckErrors::TraitTooManyMethods(MAX_TRAIT_METHODS + 1, MAX_TRAIT_METHODS),
+        CheckErrors::TooManyFunctionParameters(
+            MAX_FUNCTION_PARAMETERS + 1,
+            MAX_FUNCTION_PARAMETERS,
+        ),
+    ];
+
+    for (bad_test, expected) in bad.iter().zip(bad_expected.iter()) {
+        if epoch.limits_parameter_and_method_count() {
+            assert_eq!(
+                *expected,
+                *type_check_helper_version(bad_test, version, epoch)
+                    .unwrap_err()
+                    .err
+            );
+        } else {
+            mem_run_analysis(bad_test, version, epoch).unwrap();
+        }
+    }
 }
 
 #[apply(test_clarity_versions)]