diff --git a/changelog.d/underscore-identifiers.added b/changelog.d/underscore-identifiers.added new file mode 100644 index 00000000000..9fbb89d3572 --- /dev/null +++ b/changelog.d/underscore-identifiers.added @@ -0,0 +1 @@ +Clarity 6: Allow identifiers to begin with `_`, and allow bare `_` identifier in `let` and `match` bindings to discard value diff --git a/clarity-types/src/representations.rs b/clarity-types/src/representations.rs index 5dbb5a4b867..49b089889c8 100644 --- a/clarity-types/src/representations.rs +++ b/clarity-types/src/representations.rs @@ -30,6 +30,10 @@ pub const CONTRACT_MIN_NAME_LENGTH: usize = 1; pub const CONTRACT_MAX_NAME_LENGTH: usize = 40; pub const MAX_STRING_LEN: u8 = 128; +/// The bare `_` identifier — a discard pattern in `let`/`match` bindings +/// from Clarity 6 onwards, rejected as a name in every other position. +pub const DISCARD_IDENTIFIER: &str = "_"; + lazy_static! { pub static ref STANDARD_PRINCIPAL_REGEX_STRING: String = "[0123456789ABCDEFGHJKMNPQRSTVWXYZ]{28,41}".into(); @@ -48,8 +52,13 @@ lazy_static! { "({})|({})", *STANDARD_PRINCIPAL_REGEX_STRING, *CONTRACT_PRINCIPAL_REGEX_STRING ); + // `ClarityName` permits identifiers to begin with `_`, including the + // bare `_` discard binding (Clarity 6). Wire-level rejection of + // `_`-prefixed names for pre-Clarity-6 transactions happens at + // `StacksBlock::validate_transaction_static_epoch`, not at this codec + // layer — see that function for the consensus reasoning. pub static ref CLARITY_NAME_REGEX_STRING: String = - "^[a-zA-Z]([a-zA-Z0-9]|[-_!?+<>=/*])*$|^[-+=/*]$|^[<>]=?$".into(); + "^[a-zA-Z_]([a-zA-Z0-9]|[-_!?+<>=/*])*$|^[-+=/*]$|^[<>]=?$".into(); pub static ref CLARITY_NAME_REGEX: Regex = { Regex::new(CLARITY_NAME_REGEX_STRING.as_str()).unwrap() diff --git a/clarity-types/src/tests/representations.rs b/clarity-types/src/tests/representations.rs index 4494034c8a7..f589093f277 100644 --- a/clarity-types/src/tests/representations.rs +++ b/clarity-types/src/tests/representations.rs @@ -38,6 +38,12 @@ use crate::stacks_common::codec::StacksMessageCodec; #[case::slash("/")] #[case::dash_only("-")] #[case::equals("=")] +#[case::leading_underscore("_admin")] +#[case::leading_underscore_with_operators("_check!?")] +#[case::leading_underscore_with_digits("_var123")] +#[case::bare_underscore("_")] +#[case::double_underscore("__")] +#[case::underscore_then_dash("_-")] fn test_clarity_name_valid(#[case] name: &str) { let clarity_name = ClarityName::try_from(name.to_string()) .unwrap_or_else(|_| panic!("Should parse valid clarity name: {name}")); diff --git a/clarity-types/src/tests/types/mod.rs b/clarity-types/src/tests/types/mod.rs index 4cf492aba7f..8f4fe71f47c 100644 --- a/clarity-types/src/tests/types/mod.rs +++ b/clarity-types/src/tests/types/mod.rs @@ -902,3 +902,136 @@ fn test_sequence_try_retain_internal_error() { let err = seq.try_retain::<(), _>(utils::keep_all).unwrap_err(); assert!(matches!(err, RetainValuesError::Internal(_))); } + +// `Value::find_invalid_tuple_key` — the recursive walker that backs the +// transaction-admission epoch gate for `_`-prefixed tuple keys. + +/// Build a one-key tuple Value with the given key name. +fn tuple_with_key(key: &str) -> Value { + Value::Tuple( + TupleData::from_data(vec![( + ClarityName::try_from(key.to_string()).unwrap(), + Value::Int(0), + )]) + .unwrap(), + ) +} + +/// The bare `_` tuple key is invalid at *every* epoch — it's reserved +/// as the `let` / `match` discard marker and cannot be referenced. +#[rstest] +#[case::pre_clarity6(StacksEpochId::Epoch34)] +#[case::clarity6(StacksEpochId::Epoch40)] +fn test_find_invalid_tuple_key_rejects_bare_underscore(#[case] epoch: StacksEpochId) { + let value = tuple_with_key("_"); + assert_eq!(value.find_invalid_tuple_key(epoch), Some("_")); +} + +/// Pre-Clarity-6 epochs reject *every* leading-`_` tuple key — matches +/// the narrow wire codec on un-upgraded nodes so consensus is preserved +/// during the upgrade window. +#[rstest] +#[case::leading_underscore("_admin")] +#[case::underscore_with_operators("_check!?")] +#[case::underscore_with_digits("_var123")] +fn test_find_invalid_tuple_key_rejects_leading_underscore_pre_clarity6(#[case] key: &str) { + let value = tuple_with_key(key); + assert_eq!( + value.find_invalid_tuple_key(StacksEpochId::Epoch34), + Some(key), + ); +} + +/// Post-activation, `_foo`-shaped tuple keys are permitted. +#[rstest] +#[case::leading_underscore("_admin")] +#[case::underscore_with_operators("_check!?")] +#[case::underscore_with_digits("_var123")] +fn test_find_invalid_tuple_key_admits_leading_underscore_in_clarity6(#[case] key: &str) { + let value = tuple_with_key(key); + assert_eq!(value.find_invalid_tuple_key(StacksEpochId::Epoch40), None); +} + +/// Plain identifiers are admissible at every epoch. +#[rstest] +#[case::plain("foo")] +#[case::with_dash("foo-bar")] +#[case::with_interior_underscore("foo_bar")] +fn test_find_invalid_tuple_key_admits_plain_keys(#[case] key: &str) { + let value = tuple_with_key(key); + assert!( + value + .find_invalid_tuple_key(StacksEpochId::Epoch34) + .is_none() + && value + .find_invalid_tuple_key(StacksEpochId::Epoch40) + .is_none(), + "key {key:?} should be admissible at every epoch", + ); +} + +/// Leaf and tuple-free variants short-circuit to `None`. +#[test] +fn test_find_invalid_tuple_key_leaf_values() { + let epoch = StacksEpochId::Epoch34; + assert!(Value::Int(1).find_invalid_tuple_key(epoch).is_none()); + assert!(Value::UInt(1).find_invalid_tuple_key(epoch).is_none()); + assert!(Value::Bool(true).find_invalid_tuple_key(epoch).is_none()); + assert!(Value::none().find_invalid_tuple_key(epoch).is_none()); + assert!( + Value::buff_from(vec![1, 2, 3]) + .unwrap() + .find_invalid_tuple_key(epoch) + .is_none() + ); +} + +/// The walker descends into `Optional::Some`, `Response`, `Sequence(List)`, +/// and nested `Tuple` so an invalid key buried inside is still surfaced. +#[test] +fn test_find_invalid_tuple_key_descends_into_compound_values() { + let epoch = StacksEpochId::Epoch34; + + // tuple inside `(some ...)` + let some_value = Value::some(tuple_with_key("_buried")).unwrap(); + assert_eq!(some_value.find_invalid_tuple_key(epoch), Some("_buried")); + + // tuple inside `(ok ...)` + let ok_value = Value::okay(tuple_with_key("_buried")).unwrap(); + assert_eq!(ok_value.find_invalid_tuple_key(epoch), Some("_buried")); + + // tuple inside `(err ...)` + let err_value = Value::error(tuple_with_key("_buried")).unwrap(); + assert_eq!(err_value.find_invalid_tuple_key(epoch), Some("_buried")); + + // Tuple inside a list. Clarity lists are homogeneous, so the + // single-element form is sufficient: the walker's for-loop over + // `Sequence(List)` makes the second-element behavior identical to + // the first. + let list_value = Value::list_from(vec![tuple_with_key("_buried")]).unwrap(); + assert_eq!(list_value.find_invalid_tuple_key(epoch), Some("_buried")); + + // tuple nested inside a tuple — `_buried` is the inner key, but the + // walker reports the *first* offender it encounters and either inner + // or outer is fine; here only the inner has a `_` prefix. + let nested = Value::Tuple( + TupleData::from_data(vec![( + ClarityName::from_literal("outer"), + tuple_with_key("_buried"), + )]) + .unwrap(), + ); + assert_eq!(nested.find_invalid_tuple_key(epoch), Some("_buried")); +} + +/// When the same value sits behind `(none)` (no payload), nothing is +/// reported — the walker doesn't peek inside `Optional::None`. +#[test] +fn test_find_invalid_tuple_key_optional_none_is_inert() { + let none_value = Value::none(); + assert!( + none_value + .find_invalid_tuple_key(StacksEpochId::Epoch34) + .is_none() + ); +} diff --git a/clarity-types/src/types/mod.rs b/clarity-types/src/types/mod.rs index d541c78a3e7..1ba72e62061 100644 --- a/clarity-types/src/types/mod.rs +++ b/clarity-types/src/types/mod.rs @@ -37,7 +37,7 @@ pub use self::signatures::{ TupleTypeSignature, TypeSignature, }; use crate::errors::ClarityTypeError; -use crate::representations::{ClarityName, ContractName, SymbolicExpression}; +use crate::representations::{ClarityName, ContractName, DISCARD_IDENTIFIER, SymbolicExpression}; /// Maximum size in bytes allowed for types. pub const MAX_VALUE_SIZE: u32 = 1024 * 1024; // 1MB @@ -970,6 +970,63 @@ impl PartialEq for TupleData { pub const NONE: Value = Value::Optional(OptionalData { data: None }); impl Value { + /// Walk this value recursively and return the first tuple key that + /// would not be accepted at `epoch`. Bare `_` is invalid at every + /// epoch (reserved as the discard pattern); other leading-`_` keys + /// are invalid pre-Clarity-6. + /// + /// NOTE: this is the consensus-critical companion to the value- + /// sanitization routine above the `Value` enum. Any new compound + /// `Value` variant — one that can carry other values — must be + /// handled here too, or `_`-prefixed keys could escape into a + /// transaction's wire payload. + pub fn find_invalid_tuple_key(&self, epoch: StacksEpochId) -> Option<&str> { + match self { + Value::Tuple(data) => { + for (key, value) in data.data_map.iter() { + if Self::tuple_key_invalid_for_epoch(key.as_str(), epoch) { + return Some(key.as_str()); + } + if let Some(found) = value.find_invalid_tuple_key(epoch) { + return Some(found); + } + } + None + } + Value::Sequence(SequenceData::List(list)) => { + for item in &list.data { + if let Some(found) = item.find_invalid_tuple_key(epoch) { + return Some(found); + } + } + None + } + Value::Optional(OptionalData { data: Some(inner) }) => { + inner.find_invalid_tuple_key(epoch) + } + Value::Response(ResponseData { data, .. }) => data.find_invalid_tuple_key(epoch), + // Leaf / no-tuple variants. + Value::Int(_) + | Value::UInt(_) + | Value::Bool(_) + | Value::Principal(_) + | Value::CallableContract(_) + | Value::Sequence(SequenceData::Buffer(_)) + | Value::Sequence(SequenceData::String(_)) + | Value::Optional(OptionalData { data: None }) => None, + } + } + + fn tuple_key_invalid_for_epoch(key: &str, epoch: StacksEpochId) -> bool { + if key == DISCARD_IDENTIFIER { + return true; + } + if epoch < StacksEpochId::Epoch40 && key.starts_with('_') { + return true; + } + false + } + pub fn some(data: Value) -> Result { if data.size()? + WRAPPER_VALUE_SIZE > MAX_VALUE_SIZE { Err(ClarityTypeError::ValueTooLarge) diff --git a/clarity-types/src/types/serialization.rs b/clarity-types/src/types/serialization.rs index 2aa1cd26268..91e9a9f7895 100644 --- a/clarity-types/src/types/serialization.rs +++ b/clarity-types/src/types/serialization.rs @@ -539,8 +539,14 @@ impl Value { } else { BOUND_VALUE_SERIALIZATION_BYTES as u64 }; + let max_depth = if sanitize { + MAX_TYPE_DEPTH as usize + } else { + UNSANITIZED_DEPTH_CHECK + }; let mut bound_reader = BoundReader::from_reader(r, bound_value_serialization_bytes); - let value = Value::inner_deserialize_read(&mut bound_reader, expected_type, sanitize)?; + let value = + Value::inner_deserialize_read(&mut bound_reader, expected_type, sanitize, max_depth)?; let bytes_read = bound_reader.num_read(); if let Some(expected_type) = expected_type { let expect_size = match expected_type.max_serialized_size() { @@ -569,6 +575,7 @@ impl Value { r: &mut R, top_expected_type: Option<&TypeSignature>, sanitize: bool, + max_depth: usize, ) -> Result { use super::Value::*; @@ -577,12 +584,7 @@ impl Value { }]; while !stack.is_empty() { - let depth_check = if sanitize { - MAX_TYPE_DEPTH as usize - } else { - UNSANITIZED_DEPTH_CHECK - }; - if stack.len() > depth_check { + if stack.len() > max_depth { return Err(ClarityTypeError::TypeSignatureTooDeep.into()); } @@ -1164,13 +1166,23 @@ impl Value { /// Try to deserialize a value without type information. This *does not* perform sanitization /// so it should not be used when decoding clarity database values. - /// Public for testing purposes only. - pub(crate) fn try_deserialize_bytes_untyped( - bytes: &Vec, - ) -> Result { + pub fn try_deserialize_bytes_untyped(bytes: &Vec) -> Result { Value::deserialize_read(&mut bytes.as_slice(), None, false) } + /// Untyped deserialize using the full `MAX_TYPE_DEPTH` cap rather than the + /// legacy pre-2.4 depth. Pair with a typed sanitizing pass to inspect keys + /// the typed pass would otherwise elide — the depths must match, or values + /// nested beyond the legacy cap escape pre-checks. + pub fn try_deserialize_bytes_untyped_full_depth( + bytes: &[u8], + ) -> Result { + let mut reader = bytes; + let mut bound_reader = + BoundReader::from_reader(&mut reader, BOUND_VALUE_SERIALIZATION_BYTES as u64); + Value::inner_deserialize_read(&mut bound_reader, None, false, MAX_TYPE_DEPTH as usize) + } + /// Try to deserialize a value from a hex string without type information. This *does not* /// perform sanitization. pub fn try_deserialize_hex_untyped(hex: &str) -> Result { diff --git a/clarity-types/src/version.rs b/clarity-types/src/version.rs index 8273b9eb372..024a1a148c6 100644 --- a/clarity-types/src/version.rs +++ b/clarity-types/src/version.rs @@ -135,6 +135,15 @@ impl ClarityVersion { pub fn supports_variadic_concat(&self) -> bool { self >= &ClarityVersion::Clarity6 } + + /// Beginning in Clarity 6, identifiers may begin with `_`: + /// 1. Any identifier can start with `_` (e.g. `_foo`, `_admin`). + /// 2. A bare `_` can be used in `let` / `match` expressions to + /// discard the result of an expression. The expression is + /// evaluated, but the result cannot be referenced. + pub fn allows_underscore_prefix(&self) -> bool { + self >= &ClarityVersion::Clarity6 + } } impl FromStr for ClarityVersion { diff --git a/clarity/src/vm/analysis/errors.rs b/clarity/src/vm/analysis/errors.rs index 6f2cf3a79e0..5ad09d4e7e9 100644 --- a/clarity/src/vm/analysis/errors.rs +++ b/clarity/src/vm/analysis/errors.rs @@ -17,7 +17,7 @@ use std::{error, fmt}; use clarity_types::errors::ClarityTypeError; -use clarity_types::representations::SymbolicExpression; +use clarity_types::representations::{DISCARD_IDENTIFIER, SymbolicExpression}; use clarity_types::types::{TraitIdentifier, TupleTypeSignature, TypeSignature}; use stacks_common::types::StacksEpochId; @@ -215,6 +215,11 @@ pub enum CommonCheckErrorKind { /// Too many trait methods specified. /// The first `usize` represents the number of methods found, the second the maximum allowed. TraitTooManyMethods(usize, usize), + /// Clarity 6: bare `_` is reserved as the discard pattern in `let`/`match` + /// bindings and cannot be used to name a top-level definition, function + /// argument, trait method, tuple key, tuple-type field, or `use-trait` + /// alias. + BareUnderscoreReserved, } /// An error detected during the static analysis of a smart contract at deployment time. @@ -405,6 +410,11 @@ pub enum StaticCheckErrorKind { /// Name (e.g., variable, function) is already in use within the same scope. /// The `String` wraps the conflicting name. NameAlreadyUsed(String), + /// Clarity 6: bare `_` is reserved as the discard pattern in `let`/`match` + /// bindings and cannot be used to name a top-level definition, function + /// argument, trait method, tuple key, tuple-type field, or `use-trait` + /// alias. + BareUnderscoreReserved, /// Name is a reserved word in Clarity and cannot be used. /// The `String` wraps the reserved name. ReservedWord(String), @@ -619,6 +629,11 @@ pub enum RuntimeCheckErrorKind { /// Name (e.g., variable, function) is already in use within the same scope. /// The `String` wraps the conflicting name. NameAlreadyUsed(String), + /// Clarity 6: bare `_` is reserved as the discard pattern in `let`/`match` + /// bindings and cannot be used to name a top-level definition, function + /// argument, trait method, tuple key, tuple-type field, or `use-trait` + /// alias. + BareUnderscoreReserved, /// Referenced function is not defined in the current scope. /// The `String` wraps the non-existent function name. @@ -1030,6 +1045,9 @@ impl From for RuntimeCheckErrorKind { CommonCheckErrorKind::UnknownTypeName(name) => { RuntimeCheckErrorKind::Unreachable(format!("Unknown type name: {name}")) } + CommonCheckErrorKind::BareUnderscoreReserved => { + RuntimeCheckErrorKind::BareUnderscoreReserved + } } } } @@ -1071,6 +1089,9 @@ impl From for StaticCheckErrorKind { CommonCheckErrorKind::UnknownTypeName(name) => { StaticCheckErrorKind::UnknownTypeName(name) } + CommonCheckErrorKind::BareUnderscoreReserved => { + StaticCheckErrorKind::BareUnderscoreReserved + } } } } @@ -1213,6 +1234,9 @@ impl DiagnosableError for StaticCheckErrorKind { StaticCheckErrorKind::GetStacksBlockInfoExpectPropertyName => "missing property name for stacks block info introspection".into(), StaticCheckErrorKind::GetTenureInfoExpectPropertyName => "missing property name for tenure info introspection".into(), StaticCheckErrorKind::NameAlreadyUsed(name) => format!("defining '{name}' conflicts with previous value"), + StaticCheckErrorKind::BareUnderscoreReserved => { + "'_' is reserved as a discard pattern and cannot be used as a name".into() + } StaticCheckErrorKind::ReservedWord(name) => format!("{name} is a reserved word"), StaticCheckErrorKind::NonFunctionApplication => "expecting expression of type function".into(), StaticCheckErrorKind::ExpectedListApplication => "expecting expression of type list".into(), @@ -1221,6 +1245,9 @@ impl DiagnosableError for StaticCheckErrorKind { StaticCheckErrorKind::BadLetSyntax => "invalid syntax of 'let'".into(), StaticCheckErrorKind::BadSyntaxBinding(binding_error) => format!("invalid syntax binding: {}", &binding_error.message()), StaticCheckErrorKind::MaxContextDepthReached => "reached depth limit".into(), + StaticCheckErrorKind::UndefinedVariable(var_name) if var_name == DISCARD_IDENTIFIER => { + format!("'{DISCARD_IDENTIFIER}' is reserved as a discard pattern; it cannot be referenced as a variable") + } StaticCheckErrorKind::UndefinedVariable(var_name) => format!("use of unresolved variable '{var_name}'"), StaticCheckErrorKind::RequiresAtLeastArguments(expected, found) => format!("expecting >= {expected} arguments, got {found}"), StaticCheckErrorKind::RequiresAtMostArguments(expected, found) => format!("expecting < {expected} arguments, got {found}"), diff --git a/clarity/src/vm/analysis/tests/mod.rs b/clarity/src/vm/analysis/tests/mod.rs index 2597711e00a..a47bc98d244 100644 --- a/clarity/src/vm/analysis/tests/mod.rs +++ b/clarity/src/vm/analysis/tests/mod.rs @@ -362,6 +362,21 @@ fn test_unbound_variable() { assert!(format!("{}", err.diagnostic).contains("use of unresolved variable 'unicorn'")); } +/// Clarity 6: referencing `_` (the discard pattern) gets a specific +/// diagnostic instead of the generic "unresolved variable" message, +/// because the user's mistake is conceptually different — they tried to +/// read back a value that the language has explicitly discarded. +#[test] +fn test_unbound_variable_discard_pattern_has_specific_message() { + let snippet = "(let ((_ 1)) _)"; + let err = mem_type_check(snippet).unwrap_err(); + let formatted = format!("{}", err.diagnostic); + assert!( + formatted.contains("'_' is reserved as a discard pattern"), + "expected discard-specific diagnostic, got: {formatted}" + ); +} + #[test] fn test_variadic_needs_one_argument() { let snippet = "(begin)"; diff --git a/clarity/src/vm/analysis/type_checker/v2_1/contexts.rs b/clarity/src/vm/analysis/type_checker/v2_1/contexts.rs index 2b1e152cb6a..4f46b6ca619 100644 --- a/clarity/src/vm/analysis/type_checker/v2_1/contexts.rs +++ b/clarity/src/vm/analysis/type_checker/v2_1/contexts.rs @@ -20,7 +20,7 @@ use crate::vm::ClarityVersion; use crate::vm::analysis::errors::{StaticCheckError, StaticCheckErrorKind}; use crate::vm::analysis::type_checker::is_reserved_word; use crate::vm::analysis::types::ContractAnalysis; -use crate::vm::representations::ClarityName; +use crate::vm::representations::{ClarityName, DISCARD_IDENTIFIER}; use crate::vm::types::signatures::FunctionSignature; use crate::vm::types::{FunctionType, QualifiedContractIdentifier, TraitIdentifier, TypeSignature}; @@ -167,6 +167,11 @@ impl ContractContext { } pub fn check_name_used(&self, name: &str) -> Result<(), StaticCheckError> { + if name == DISCARD_IDENTIFIER { + return Err(StaticCheckError::new( + StaticCheckErrorKind::BareUnderscoreReserved, + )); + } if is_reserved_word(name, self.clarity_version) { return Err(StaticCheckError::new(StaticCheckErrorKind::ReservedWord( name.to_string(), diff --git a/clarity/src/vm/analysis/type_checker/v2_1/natives/mod.rs b/clarity/src/vm/analysis/type_checker/v2_1/natives/mod.rs index 2d4a1fcb853..bbf9ecf08c0 100644 --- a/clarity/src/vm/analysis/type_checker/v2_1/natives/mod.rs +++ b/clarity/src/vm/analysis/type_checker/v2_1/natives/mod.rs @@ -28,6 +28,7 @@ use crate::vm::costs::{CostErrors, CostTracker, analysis_typecheck_cost, runtime use crate::vm::diagnostic::DiagnosableError; use crate::vm::functions::bitcoin::VERIFY_MERKLE_PROOF_MAX_DEPTH; use crate::vm::functions::{NativeFunctions, handle_binding_list}; +use crate::vm::representations::DISCARD_IDENTIFIER; use crate::vm::types::signatures::{ CallableSubtype, FunctionArgSignature, FunctionReturnsSignature, SequenceSubtype, }; @@ -253,6 +254,11 @@ pub fn check_special_tuple_cons( args, SyntaxBindingErrorType::TupleCons, |var_name, var_sexp| { + // Clarity 6: bare `_` cannot name a tuple key — it would be + // referenceable via `get`, contradicting the discard semantics. + if var_name.as_str() == DISCARD_IDENTIFIER { + return Err(StaticCheckErrorKind::BareUnderscoreReserved.into()); + } checker.type_check(var_sexp, context).and_then(|var_type| { runtime_cost( ClarityCostFunction::AnalysisTupleItemsCheck, @@ -305,11 +311,17 @@ fn check_special_let( binding_list, SyntaxBindingErrorType::Let, |var_name, var_sexp| { - checker.contract_context.check_name_used(var_name)?; - if out_context.lookup_variable_type(var_name).is_some() { - return Err(StaticCheckError::new( - StaticCheckErrorKind::NameAlreadyUsed(var_name.to_string()), - )); + // Clarity 6: bare `_` is a discard binding — still type-check + // the value (so type errors surface) but don't add to scope. + let is_discard = var_name.as_str() == DISCARD_IDENTIFIER + && checker.clarity_version.allows_underscore_prefix(); + if !is_discard { + checker.contract_context.check_name_used(var_name)?; + if out_context.lookup_variable_type(var_name).is_some() { + return Err(StaticCheckError::new( + StaticCheckErrorKind::NameAlreadyUsed(var_name.to_string()), + )); + } } let typed_result = checker.type_check(var_sexp, &out_context)?; @@ -328,7 +340,13 @@ fn check_special_let( .ok_or_else(|| CostErrors::CostOverflow)?; checker.add_memory(memory_use)?; } - out_context.add_variable_type(var_name.clone(), typed_result, checker.clarity_version); + if !is_discard { + out_context.add_variable_type( + var_name.clone(), + typed_result, + checker.clarity_version, + ); + } Ok(()) }, )?; diff --git a/clarity/src/vm/analysis/type_checker/v2_1/natives/options.rs b/clarity/src/vm/analysis/type_checker/v2_1/natives/options.rs index f52a1330116..8f8a1afe637 100644 --- a/clarity/src/vm/analysis/type_checker/v2_1/natives/options.rs +++ b/clarity/src/vm/analysis/type_checker/v2_1/natives/options.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use clarity_types::representations::ClarityName; +use clarity_types::representations::{ClarityName, DISCARD_IDENTIFIER}; use clarity_types::types::TypeSignature; use stacks_common::types::StacksEpochId; @@ -306,14 +306,20 @@ fn eval_with_new_binding( .ok_or_else(|| CostErrors::CostOverflow)?; checker.add_memory(memory_use)?; } - checker.contract_context.check_name_used(&bind_name)?; + // Clarity 6: bare `_` discards the matched value — don't place the + // name in the typing context for the branch body. + let is_discard = bind_name.as_str() == DISCARD_IDENTIFIER + && checker.clarity_version.allows_underscore_prefix(); + if !is_discard { + checker.contract_context.check_name_used(&bind_name)?; + + if inner_context.lookup_variable_type(&bind_name).is_some() { + return Err(StaticCheckErrorKind::NameAlreadyUsed(bind_name.into()).into()); + } - if inner_context.lookup_variable_type(&bind_name).is_some() { - return Err(StaticCheckErrorKind::NameAlreadyUsed(bind_name.into()).into()); + inner_context.add_variable_type(bind_name, bind_type, checker.clarity_version); } - inner_context.add_variable_type(bind_name, bind_type, checker.clarity_version); - let result = checker.type_check(body, &inner_context); if checker.epoch.analysis_memory() { checker.drop_memory(memory_use)?; diff --git a/clarity/src/vm/ast/errors.rs b/clarity/src/vm/ast/errors.rs index ab93d0f6ddf..4a478135a54 100644 --- a/clarity/src/vm/ast/errors.rs +++ b/clarity/src/vm/ast/errors.rs @@ -176,6 +176,10 @@ pub enum ParseErrorKind { /// Contract name contains invalid characters or violates naming rules. /// The `String` represents the invalid contract name. IllegalContractName(String), + /// Identifier starts with an underscore in a Clarity version that predates + /// `ClarityVersion::Clarity6`, where leading-`_` names are not yet + /// permitted. The `String` is the offending name. + UnderscoreIdentifierNotAllowed(String), // Notes /// Indicates a token mismatch for internal parser diagnostics. @@ -403,6 +407,9 @@ impl DiagnosableError for ParseErrorKind { } ParseErrorKind::TupleValueExpected => "expected value expression for tuple".into(), ParseErrorKind::IllegalClarityName(name) => format!("illegal clarity name, '{name}'"), + ParseErrorKind::UnderscoreIdentifierNotAllowed(name) => { + format!("identifier '{name}' starts with '_', which requires Clarity 6 or later") + } ParseErrorKind::IllegalASCIIString(s) => format!("illegal ascii string \"{s}\""), ParseErrorKind::ExpectedWhitespace => "expected whitespace before expression".into(), ParseErrorKind::NoteToMatchThis(token) => format!("to match this '{token}'"), diff --git a/clarity/src/vm/ast/mod.rs b/clarity/src/vm/ast/mod.rs index 561df22e8ad..dba426f0cbf 100644 --- a/clarity/src/vm/ast/mod.rs +++ b/clarity/src/vm/ast/mod.rs @@ -23,6 +23,7 @@ pub mod errors; pub mod stack_depth_checker; pub mod sugar_expander; pub mod types; +pub mod underscore_checker; use stacks_common::types::StacksEpochId; use self::definition_sorter::DefinitionSorter; @@ -36,6 +37,7 @@ use self::sugar_expander::SugarExpander; use self::traits_resolver::TraitsResolver; use self::types::BuildASTPass; pub use self::types::ContractAST; +use self::underscore_checker::UnderscoreIdentifierChecker; use crate::vm::ClarityVersion; use crate::vm::costs::cost_functions::ClarityCostFunction; use crate::vm::costs::{CostTracker, runtime_cost}; @@ -194,6 +196,18 @@ fn inner_build_ast( _ => (), } + // Clarity 6: reject identifiers beginning with `_` for `ClarityVersion < + // Clarity6`. The wire-level regex and the v2 lexer both accept them so + // that the parser can produce a precise diagnostic here. + match UnderscoreIdentifierChecker::run_pass(&mut contract_ast, clarity_version, epoch) { + Err(e) if error_early => return Err(e), + Err(e) => { + diagnostics.push(e.diagnostic); + success = false; + } + _ => (), + } + match ExpressionIdentifier::run_pre_expression_pass(&mut contract_ast, clarity_version) { Err(e) if error_early => return Err(e), Err(e) => { diff --git a/clarity/src/vm/ast/parser/v2/lexer/mod.rs b/clarity/src/vm/ast/parser/v2/lexer/mod.rs index fd2e232c8b0..586d79a009b 100644 --- a/clarity/src/vm/ast/parser/v2/lexer/mod.rs +++ b/clarity/src/vm/ast/parser/v2/lexer/mod.rs @@ -733,7 +733,10 @@ impl<'a> Lexer<'a> { self.read_char()?; if self.next == '=' { Token::LessEqual - } else if self.next.is_ascii_alphabetic() { + } else if self.next.is_ascii_alphabetic() || self.next == '_' { + // `_` may lead a trait identifier in Clarity 6 onwards; + // accept unconditionally here and let the version-gated + // AST pass reject in older versions. self.read_trait_identifier()? } else { advance = false; @@ -811,7 +814,10 @@ impl<'a> Lexer<'a> { } } _ => { - if self.next.is_ascii_alphabetic() { + // `_` may lead an identifier from Clarity 6 onwards. The lexer + // accepts it unconditionally; an AST pass rejects underscore + // identifiers for `ClarityVersion < Clarity6`. + if self.next.is_ascii_alphabetic() || self.next == '_' { advance = false; self.read_identifier(None)? } else if self.next.is_ascii_digit() { diff --git a/clarity/src/vm/ast/underscore_checker.rs b/clarity/src/vm/ast/underscore_checker.rs new file mode 100644 index 00000000000..de17367826f --- /dev/null +++ b/clarity/src/vm/ast/underscore_checker.rs @@ -0,0 +1,343 @@ +// Copyright (C) 2025-2026 Stacks Open Internet Foundation +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +//! AST pass that rejects identifiers beginning with `_` for pre-Clarity-6 +//! contracts. +//! +//! The wide `ClarityName` regex and the v2 lexer accept underscore-led +//! names unconditionally so the parser can produce a well-formed AST and +//! emit a precise version-aware diagnostic here. The admission walker in +//! `StacksBlock::validate_transaction_static_epoch` enforces the same +//! rule at the wire layer (rejecting leading-`_` `function_name`, +//! `asset_name`, and tuple keys in transactions whose active epoch is +//! pre-Clarity-6) so updated and un-updated nodes agree during the +//! upgrade window. + +use clarity_types::representations::ClarityName; +use stacks_common::types::StacksEpochId; + +use crate::vm::ClarityVersion; +use crate::vm::ast::errors::{ParseError, ParseErrorKind, ParseResult}; +use crate::vm::ast::types::{BuildASTPass, ContractAST}; +use crate::vm::representations::{PreSymbolicExpression, PreSymbolicExpressionType}; + +pub struct UnderscoreIdentifierChecker; + +impl BuildASTPass for UnderscoreIdentifierChecker { + fn run_pass( + contract_ast: &mut ContractAST, + version: ClarityVersion, + _epoch: StacksEpochId, + ) -> ParseResult<()> { + if version.allows_underscore_prefix() { + return Ok(()); + } + check(&contract_ast.pre_expressions) + } +} + +fn check(exprs: &[PreSymbolicExpression]) -> ParseResult<()> { + for expr in exprs { + check_one(expr)?; + } + Ok(()) +} + +fn check_one(expr: &PreSymbolicExpression) -> ParseResult<()> { + match &expr.pre_expr { + PreSymbolicExpressionType::Atom(name) => reject_if_underscore(name, expr), + PreSymbolicExpressionType::TraitReference(name) => reject_if_underscore(name, expr), + PreSymbolicExpressionType::SugaredFieldIdentifier(_, trait_name) => { + reject_if_underscore(trait_name, expr) + } + PreSymbolicExpressionType::FieldIdentifier(trait_id) => { + reject_if_underscore(&trait_id.name, expr) + } + PreSymbolicExpressionType::List(inner) | PreSymbolicExpressionType::Tuple(inner) => { + check(inner) + } + PreSymbolicExpressionType::AtomValue(_) + | PreSymbolicExpressionType::SugaredContractIdentifier(_) + | PreSymbolicExpressionType::Comment(_) + | PreSymbolicExpressionType::Placeholder(_) => Ok(()), + } +} + +fn reject_if_underscore(name: &ClarityName, expr: &PreSymbolicExpression) -> ParseResult<()> { + if !name.starts_with('_') { + return Ok(()); + } + let mut err = ParseError::new(ParseErrorKind::UnderscoreIdentifierNotAllowed( + name.to_string(), + )); + err.diagnostic.spans = vec![expr.span().clone()]; + Err(err) +} + +#[cfg(test)] +mod tests { + use clarity_types::representations::MAX_STRING_LEN; + use clarity_types::types::QualifiedContractIdentifier; + use pinny::tag; + use proptest::prelude::*; + use proptest::string::string_regex; + + use super::*; + use crate::vm::ast::{build_ast, build_ast_with_diagnostics}; + use crate::vm::costs::LimitedCostTracker; + + fn parses(source: &str, version: ClarityVersion) -> bool { + let contract_id = QualifiedContractIdentifier::transient(); + let (_ast, _diag, success) = build_ast_with_diagnostics( + &contract_id, + source, + &mut LimitedCostTracker::new_free(), + version, + StacksEpochId::latest(), + ); + success + } + + /// Like `parses(...)` but with error-early enabled, so callers can match + /// on the specific `ParseErrorKind` returned for a pre-Clarity-6 contract. + fn parse_err(source: &str, version: ClarityVersion) -> ParseErrorKind { + let contract_id = QualifiedContractIdentifier::transient(); + let err = build_ast( + &contract_id, + source, + &mut LimitedCostTracker::new_free(), + version, + StacksEpochId::latest(), + ) + .expect_err("expected parse error"); + *err.err + } + + #[test] + fn underscore_prefix_rejected_pre_clarity6() { + assert!(!parses( + "(define-constant _admin 'SP2J6ZY48GV1EZ5V2V5RB9MP66SW86PYKKNRV9EJ7)", + ClarityVersion::Clarity5, + )); + } + + #[test] + fn underscore_prefix_accepted_in_clarity6() { + assert!(parses( + "(define-constant _admin 'SP2J6ZY48GV1EZ5V2V5RB9MP66SW86PYKKNRV9EJ7)", + ClarityVersion::Clarity6, + )); + } + + #[test] + fn underscore_prefix_in_let_binding_pre_clarity6_rejected() { + assert!(!parses("(let ((_x 1)) (+ _x 1))", ClarityVersion::Clarity5,)); + } + + #[test] + fn bare_underscore_rejected_pre_clarity6() { + assert!(!parses("(let ((_ 1)) 0)", ClarityVersion::Clarity5)); + } + + #[test] + fn bare_underscore_accepted_in_clarity6() { + assert!(parses("(let ((_ 1)) 0)", ClarityVersion::Clarity6)); + } + + #[test] + fn underscore_in_function_arg_rejected_pre_clarity6() { + assert!(!parses( + "(define-public (foo (_addr principal)) (ok _addr))", + ClarityVersion::Clarity5, + )); + } + + #[test] + fn nested_underscore_atom_rejected_pre_clarity6() { + // The check must descend into nested lists. + assert!(!parses( + "(define-public (foo) (ok (let ((y 1)) _bad)))", + ClarityVersion::Clarity5, + )); + } + + #[test] + fn non_underscore_names_still_accepted_pre_clarity6() { + // Regression guard: legacy identifiers must still parse. + assert!(parses( + "(define-constant admin 'SP2J6ZY48GV1EZ5V2V5RB9MP66SW86PYKKNRV9EJ7) + (define-public (foo (addr principal)) (ok addr))", + ClarityVersion::Clarity5, + )); + } + + #[test] + fn interior_underscore_still_accepted_pre_clarity6() { + // Underscores inside an identifier remain legal pre-Clarity-6; + // only the leading position is gated. + assert!(parses( + "(define-constant my_admin 'SP2J6ZY48GV1EZ5V2V5RB9MP66SW86PYKKNRV9EJ7)", + ClarityVersion::Clarity5, + )); + } + + #[test] + fn underscore_trait_reference_rejected_pre_clarity6() { + assert!(!parses( + "(define-trait t ((bar (<_foo>) (response uint uint))))", + ClarityVersion::Clarity5, + )); + } + + #[test] + fn underscore_trait_reference_accepted_in_clarity6() { + // Defines `_foo` so the later `TraitsResolver` pass doesn't fail with + // `TraitReferenceUnknown` — pre-Clarity-6 short-circuits before that. + assert!(parses( + "(define-trait _foo ((m (uint) (response uint uint)))) + (define-trait t ((bar (<_foo>) (response uint uint))))", + ClarityVersion::Clarity6, + )); + } + + #[test] + fn underscore_in_let_binding_accepted_in_clarity6() { + assert!(parses("(let ((_x 1)) (+ _x 1))", ClarityVersion::Clarity6)); + } + + #[test] + fn underscore_in_function_arg_accepted_in_clarity6() { + assert!(parses( + "(define-public (foo (_addr principal)) (ok _addr))", + ClarityVersion::Clarity6, + )); + } + + #[test] + fn underscore_in_tuple_key_rejected_pre_clarity6() { + // The pass descends into `Tuple` nodes — exercise that match arm via a + // tuple-literal whose key is underscore-prefixed. + assert!(!parses( + "(define-constant x { _k: 1 })", + ClarityVersion::Clarity5, + )); + } + + #[test] + fn underscore_in_tuple_key_accepted_in_clarity6() { + assert!(parses( + "(define-constant x { _k: 1 })", + ClarityVersion::Clarity6, + )); + } + + #[test] + fn underscore_in_sugared_field_identifier_rejected_pre_clarity6() { + // `.contract.trait` desugars to `SugaredFieldIdentifier`. The trait + // name `_t` should trigger the leading-`_` check via that arm. + assert!(!parses( + "(use-trait t .my-contract._t)", + ClarityVersion::Clarity5, + )); + } + + #[test] + fn underscore_in_fully_qualified_field_identifier_rejected_pre_clarity6() { + // The fully-qualified `'..` form yields a + // `FieldIdentifier(TraitIdentifier { name, ... })`; exercise that + // distinct match arm. + assert!(!parses( + "(use-trait t 'SP2J6ZY48GV1EZ5V2V5RB9MP66SW86PYKKNRV9EJ7.my-contract._t)", + ClarityVersion::Clarity5, + )); + } + + /// Regression guard: the gate must emit `UnderscoreIdentifierNotAllowed`, + /// not (say) a generic `IllegalClarityName`. The boolean-only tests + /// above would not catch such a drift. + #[test] + fn rejection_emits_underscore_identifier_not_allowed_kind() { + let err = parse_err( + "(define-constant _admin 'SP2J6ZY48GV1EZ5V2V5RB9MP66SW86PYKKNRV9EJ7)", + ClarityVersion::Clarity5, + ); + let ParseErrorKind::UnderscoreIdentifierNotAllowed(name) = err else { + panic!("expected UnderscoreIdentifierNotAllowed, got {err:?}"); + }; + assert_eq!(name, "_admin"); + } + + /// Regression guard: operator names like `+`, `<=`, `*` never start with + /// `_` and must pass the pass cleanly under any Clarity version. + #[test] + fn leading_operator_names_unaffected_pre_clarity6() { + assert!(parses( + "(define-private (foo) (+ 1 2)) (define-private (bar) (<= 1 2))", + ClarityVersion::Clarity5, + )); + } + + /// Bare `_` is reserved as a discard pattern; outside `let`/`match` + /// bindings it is rejected at the analyzer/runtime layer (see + /// `check_name_used` / `check_legal_define`). The AST pass itself + /// happily lets `_` through in Clarity 6+ — the rejection lives one + /// layer up so let/match discards can use the same character. + #[test] + fn bare_underscore_passes_ast_pass_in_clarity6() { + // AST pass alone accepts bare `_` as a define name; the analyzer + // will reject it. The full-pipeline rejection is covered by + // `test_bare_underscore_as_define_name_rejected_in_clarity6` in + // `vm::tests::simple_apply_eval`. + assert!(parses("(define-constant _ 1)", ClarityVersion::Clarity6)); + } + + #[test] + fn bare_underscore_as_define_name_rejected_pre_clarity6() { + assert!(!parses("(define-constant _ 1)", ClarityVersion::Clarity5)); + } + + /// Generates valid `_`-led `ClarityName` strings (including bare `_`), + /// bounded by `MAX_STRING_LEN`. + fn any_underscore_led_clarity_name() -> impl Strategy { + string_regex(&format!( + "_[a-zA-Z0-9_!?+<>=/*-]{{0,{}}}", + (MAX_STRING_LEN as usize).saturating_sub(1) + )) + .unwrap() + } + + /// For every valid `_`-led `ClarityName`, pre-Clarity-6 must reject with + /// `UnderscoreIdentifierNotAllowed(name)` and Clarity 6 must accept. + /// Covers bare `_`, `_>=` / `_+!` shapes, and names at the + /// `MAX_STRING_LEN` boundary. + #[tag(t_prop)] + #[test] + fn prop_underscore_led_names_gated_by_clarity_version() { + proptest!(|(name in any_underscore_led_clarity_name())| { + let src = format!("(define-constant {name} 1)"); + + let err = parse_err(&src, ClarityVersion::Clarity5); + prop_assert!( + matches!(&err, ParseErrorKind::UnderscoreIdentifierNotAllowed(n) if n == &name), + "expected UnderscoreIdentifierNotAllowed({name:?}), got {err:?}" + ); + + prop_assert!( + parses(&src, ClarityVersion::Clarity6), + "expected `_`-led name {name:?} to parse in Clarity 6" + ); + }); + } +} diff --git a/clarity/src/vm/functions/conversions.rs b/clarity/src/vm/functions/conversions.rs index 8212405d349..da681b38522 100644 --- a/clarity/src/vm/functions/conversions.rs +++ b/clarity/src/vm/functions/conversions.rs @@ -349,6 +349,20 @@ pub fn from_consensus_buff( }; runtime_cost(ClarityCostFunction::FromConsensusBuff, exec_state, input)?; + // Reject epoch-invalid tuple keys before the typed pass: typed + // deserialization sanitizes (elides) keys not in the expected type, + // which would strip the evidence before any post-deserialize walker + // could see it. Must use the full-depth untyped path: the typed pass + // reaches `MAX_TYPE_DEPTH`, so a shallower pre-scan would let bad keys + // nested past the legacy cap slip through. + if let Ok(unsanitized) = Value::try_deserialize_bytes_untyped_full_depth(input_bytes) + && unsanitized + .find_invalid_tuple_key(*exec_state.epoch()) + .is_some() + { + return Ok(Value::none()); + } + // Perform the deserialization and check that it deserialized to the expected // type. A type mismatch at this point is an error that should be surfaced in // Clarity (as a none return). diff --git a/clarity/src/vm/functions/define.rs b/clarity/src/vm/functions/define.rs index bceab2fbecb..672e133c3ce 100644 --- a/clarity/src/vm/functions/define.rs +++ b/clarity/src/vm/functions/define.rs @@ -24,7 +24,7 @@ use crate::vm::errors::{ }; use crate::vm::eval; use crate::vm::representations::SymbolicExpressionType::Field; -use crate::vm::representations::{ClarityName, SymbolicExpression}; +use crate::vm::representations::{ClarityName, DISCARD_IDENTIFIER, SymbolicExpression}; use crate::vm::types::signatures::FunctionSignature; use crate::vm::types::{ TraitIdentifier, TypeSignature, TypeSignatureExt as _, Value, parse_name_type_pairs, @@ -123,6 +123,9 @@ fn check_legal_define( name: &str, contract_context: &ContractContext, ) -> Result<(), RuntimeCheckErrorKind> { + if name == DISCARD_IDENTIFIER { + return Err(RuntimeCheckErrorKind::BareUnderscoreReserved); + } if contract_context.is_name_used(name) { Err(RuntimeCheckErrorKind::NameAlreadyUsed(name.to_string())) } else { @@ -294,8 +297,17 @@ fn handle_define_trait( Ok(DefineResult::Trait(name.clone(), trait_signature)) } -fn handle_use_trait(name: &ClarityName, trait_identifier: &TraitIdentifier) -> DefineResult { - DefineResult::UseTrait(name.clone(), trait_identifier.clone()) +fn handle_use_trait( + name: &ClarityName, + trait_identifier: &TraitIdentifier, +) -> Result { + if name.as_str() == DISCARD_IDENTIFIER { + return Err(RuntimeCheckErrorKind::BareUnderscoreReserved.into()); + } + Ok(DefineResult::UseTrait( + name.clone(), + trait_identifier.clone(), + )) } fn handle_impl_trait(trait_identifier: &TraitIdentifier) -> DefineResult { @@ -498,7 +510,7 @@ pub fn evaluate_define( DefineFunctionsParsed::UseTrait { name, trait_identifier, - } => Ok(handle_use_trait(name, trait_identifier)), + } => handle_use_trait(name, trait_identifier), DefineFunctionsParsed::ImplTrait { trait_identifier } => { Ok(handle_impl_trait(trait_identifier)) } diff --git a/clarity/src/vm/functions/mod.rs b/clarity/src/vm/functions/mod.rs index edd91ba4258..69f193a1de9 100644 --- a/clarity/src/vm/functions/mod.rs +++ b/clarity/src/vm/functions/mod.rs @@ -27,7 +27,9 @@ use crate::vm::errors::{ VmExecutionError, check_argument_count, check_arguments_at_least, }; pub use crate::vm::functions::assets::stx_transfer_consolidated; -use crate::vm::representations::{ClarityName, SymbolicExpression, SymbolicExpressionType}; +use crate::vm::representations::{ + ClarityName, DISCARD_IDENTIFIER, SymbolicExpression, SymbolicExpressionType, +}; use crate::vm::types::{BuffData, PrincipalData, SequenceData, TypeSignature, Value}; use crate::vm::{LocalContext, eval, is_reserved}; @@ -819,9 +821,15 @@ fn special_let( finally_drop_memory!( exec_state, memory_use; { handle_binding_list::<_, VmExecutionError>(bindings, SyntaxBindingErrorType::Let, |binding_name, var_sexp| { - if is_reserved(binding_name, invoke_ctx.contract_context.get_clarity_version()) || - invoke_ctx.contract_context.lookup_function(binding_name).is_some() || - inner_context.lookup_variable(binding_name).is_some() { + // Clarity 6: bare `_` is a discard binding — evaluate the value + // (preserving `try!`/`unwrap!` short-circuit) but don't bind it. + let is_discard = binding_name.as_str() == DISCARD_IDENTIFIER + && invoke_ctx.contract_context.get_clarity_version().allows_underscore_prefix(); + + if !is_discard + && (is_reserved(binding_name, invoke_ctx.contract_context.get_clarity_version()) || + invoke_ctx.contract_context.lookup_function(binding_name).is_some() || + inner_context.lookup_variable(binding_name).is_some()) { return Err(RuntimeCheckErrorKind::NameAlreadyUsed(binding_name.clone().into()).into()) } @@ -831,6 +839,9 @@ fn special_let( exec_state.add_memory(bind_mem_use)?; memory_use += bind_mem_use; // no check needed, b/c it's done in add_memory. let binding_value = binding_value.clone_with_cost(exec_state)?; + if is_discard { + return Ok(()); + } if *invoke_ctx.contract_context.get_clarity_version() >= ClarityVersion::Clarity2 && let CallableContract(trait_data) = &binding_value { inner_context.callable_contracts.insert(binding_name.clone(), trait_data.clone()); } diff --git a/clarity/src/vm/functions/options.rs b/clarity/src/vm/functions/options.rs index c338fd2efe4..df0dcff10e0 100644 --- a/clarity/src/vm/functions/options.rs +++ b/clarity/src/vm/functions/options.rs @@ -22,6 +22,7 @@ use crate::vm::errors::{ EarlyReturnError, RuntimeCheckErrorKind, RuntimeError, VmExecutionError, VmInternalError, check_arguments_at_least, }; +use crate::vm::representations::DISCARD_IDENTIFIER; use crate::vm::types::{CallableData, OptionalData, ResponseData, TypeSignature, Value}; use crate::vm::{self, ClarityName, ClarityVersion, SymbolicExpression}; @@ -128,14 +129,22 @@ fn eval_with_new_binding( context: &LocalContext, ) -> Result { let mut inner_context = context.extend()?; - if vm::is_reserved( - &bind_name, - invoke_ctx.contract_context.get_clarity_version(), - ) || invoke_ctx - .contract_context - .lookup_function(&bind_name) - .is_some() - || inner_context.lookup_variable(&bind_name).is_some() + // Clarity 6: bare `_` discards the matched value — execute the branch + // without binding the name. + let is_discard = bind_name.as_str() == DISCARD_IDENTIFIER + && invoke_ctx + .contract_context + .get_clarity_version() + .allows_underscore_prefix(); + if !is_discard + && (vm::is_reserved( + &bind_name, + invoke_ctx.contract_context.get_clarity_version(), + ) || invoke_ctx + .contract_context + .lookup_function(&bind_name) + .is_some() + || inner_context.lookup_variable(&bind_name).is_some()) { return Err(RuntimeCheckErrorKind::NameAlreadyUsed(bind_name.into()).into()); } @@ -143,18 +152,20 @@ fn eval_with_new_binding( let memory_use = bind_value.get_memory_use()?; exec_state.add_memory(memory_use)?; - if *invoke_ctx.contract_context.get_clarity_version() >= ClarityVersion::Clarity2 - && let CallableContract(trait_data) = &bind_value - { - inner_context.callable_contracts.insert( - bind_name.clone(), - CallableData { - contract_identifier: trait_data.contract_identifier.clone(), - trait_identifier: trait_data.trait_identifier.clone(), - }, - ); + if !is_discard { + if *invoke_ctx.contract_context.get_clarity_version() >= ClarityVersion::Clarity2 + && let CallableContract(trait_data) = &bind_value + { + inner_context.callable_contracts.insert( + bind_name.clone(), + CallableData { + contract_identifier: trait_data.contract_identifier.clone(), + trait_identifier: trait_data.trait_identifier.clone(), + }, + ); + } + inner_context.variables.insert(bind_name, bind_value); } - inner_context.variables.insert(bind_name, bind_value); let result = vm::eval(body, exec_state, invoke_ctx, &inner_context) .and_then(|v| v.clone_with_cost(exec_state)); diff --git a/clarity/src/vm/functions/tuples.rs b/clarity/src/vm/functions/tuples.rs index d3f062db07e..16fb988e868 100644 --- a/clarity/src/vm/functions/tuples.rs +++ b/clarity/src/vm/functions/tuples.rs @@ -20,7 +20,7 @@ use crate::vm::errors::{ RuntimeCheckErrorKind, SyntaxBindingErrorType, VmExecutionError, VmInternalError, check_argument_count, check_arguments_at_least, }; -use crate::vm::representations::SymbolicExpression; +use crate::vm::representations::{DISCARD_IDENTIFIER, SymbolicExpression}; use crate::vm::types::{TupleData, TypeSignature, Value}; use crate::vm::{LocalContext, eval}; @@ -36,6 +36,18 @@ pub fn tuple_cons( check_arguments_at_least(1, args)?; + // Clarity 6: reject bare `_` keys before evaluating any values — matches + // the analyzer's ordering and avoids paying for evaluations that will be + // discarded. A `_` key would create a referenceable binding via `get`. + for arg in args { + if let Some(pair) = arg.match_list() + && let Some(name) = pair.first().and_then(|e| e.match_atom()) + && name.as_str() == DISCARD_IDENTIFIER + { + return Err(RuntimeCheckErrorKind::BareUnderscoreReserved.into()); + } + } + let bindings = parse_eval_bindings( args, SyntaxBindingErrorType::TupleCons, diff --git a/clarity/src/vm/representations.rs b/clarity/src/vm/representations.rs index 23db9e05905..7f2c6370ed0 100644 --- a/clarity/src/vm/representations.rs +++ b/clarity/src/vm/representations.rs @@ -17,7 +17,7 @@ pub use clarity_types::representations::{ CLARITY_NAME_REGEX, CLARITY_NAME_REGEX_STRING, CONTRACT_MAX_NAME_LENGTH, CONTRACT_MIN_NAME_LENGTH, CONTRACT_NAME_REGEX, CONTRACT_NAME_REGEX_STRING, - CONTRACT_PRINCIPAL_REGEX_STRING, ClarityName, ContractName, MAX_STRING_LEN, + CONTRACT_PRINCIPAL_REGEX_STRING, ClarityName, ContractName, DISCARD_IDENTIFIER, MAX_STRING_LEN, PRINCIPAL_DATA_REGEX_STRING, PreSymbolicExpression, PreSymbolicExpressionType, STANDARD_PRINCIPAL_REGEX_STRING, Span, SymbolicExpression, SymbolicExpressionCommon, SymbolicExpressionType, TraitDefinition, depth_traverse, diff --git a/clarity/src/vm/tests/conversions.rs b/clarity/src/vm/tests/conversions.rs index 7dddd5bdfe5..79cccb7df4f 100644 --- a/clarity/src/vm/tests/conversions.rs +++ b/clarity/src/vm/tests/conversions.rs @@ -30,9 +30,10 @@ use crate::vm::tests::test_clarity_versions; use crate::vm::types::SequenceSubtype::BufferType; use crate::vm::types::TypeSignature::SequenceType; use crate::vm::types::{ - ASCIIData, BuffData, BufferLength, CharType, SequenceData, TypeSignature, UTF8Data, Value, + ASCIIData, BuffData, BufferLength, CharType, SequenceData, TupleData, TypeSignature, UTF8Data, + Value, }; -use crate::vm::{ClarityVersion, execute_v2, execute_with_parameters}; +use crate::vm::{ClarityName, ClarityVersion, execute_v2, execute_with_parameters}; #[test] fn test_simple_buff_to_int_le() { @@ -625,6 +626,137 @@ fn test_from_consensus_buff_unexpected_serialization_epoch_gate( ); } +/// A `from-consensus-buff?` buffer encoding a tuple with a leading-`_` key +/// must deserialize to `none` pre-Clarity-6 and to the typed-sanitized tuple +/// from Clarity 6 onward. +#[apply(test_clarity_versions)] +fn test_from_consensus_buff_rejects_underscore_tuple_key_pre_clarity6( + version: ClarityVersion, + epoch: StacksEpochId, +) { + // `from-consensus-buff?` is only available in Clarity 2+. + if version < ClarityVersion::Clarity2 { + return; + } + + let tuple_with_underscore = Value::Tuple( + TupleData::from_data(vec![ + (ClarityName::from_literal("a"), Value::Int(456)), + (ClarityName::from_literal("_x"), Value::Int(123)), + ]) + .expect("construction of `_x` tuple should succeed with the relaxed regex"), + ); + let hex = tuple_with_underscore + .serialize_to_hex() + .expect("serialize tuple to hex"); + + let program = format!("(from-consensus-buff? {{a: int}} 0x{hex})"); + let result = execute_with_parameters(&program, version, epoch, false) + .expect("from-consensus-buff? must not crash") + .expect("from-consensus-buff? must produce a value"); + + if epoch < StacksEpochId::Epoch40 { + assert_eq!( + result, + Value::none(), + "pre-Clarity-6 epochs must reject `_`-prefixed tuple keys at runtime to match unpatched-node behavior" + ); + } else { + let expected = Value::some(Value::Tuple( + TupleData::from_data(vec![(ClarityName::from_literal("a"), Value::Int(456))]).unwrap(), + )) + .unwrap(); + assert_eq!( + result, expected, + "Clarity 6+ should accept the buffer and return the typed-sanitized tuple" + ); + } +} + +/// A `from-consensus-buff?` buffer encoding a tuple keyed by bare `_` must +/// always deserialize to `none`; bare `_` is an invalid tuple key at every +/// epoch (rejected by the codec pre-Clarity-6, reserved as the discard +/// pattern from Clarity 6 onward). +#[apply(test_clarity_versions)] +fn test_from_consensus_buff_rejects_bare_underscore_tuple_key( + version: ClarityVersion, + epoch: StacksEpochId, +) { + if version < ClarityVersion::Clarity2 { + return; + } + + let tuple_with_bare_underscore = Value::Tuple( + TupleData::from_data(vec![(ClarityName::from_literal("_"), Value::Int(7))]) + .expect("construction of bare-`_` tuple should succeed with the relaxed regex"), + ); + let hex = tuple_with_bare_underscore + .serialize_to_hex() + .expect("serialize tuple to hex"); + + let program = format!("(from-consensus-buff? {{a: int}} 0x{hex})"); + let result = execute_with_parameters(&program, version, epoch, false) + .expect("from-consensus-buff? must not crash") + .expect("from-consensus-buff? must produce a value"); + + assert_eq!( + result, + Value::none(), + "bare `_` is reserved as the discard pattern at every epoch and must never appear as a runtime tuple key" + ); +} + +/// A `from-consensus-buff?` buffer whose offending `_`-prefixed tuple key sits +/// deeper than the legacy untyped depth cap must still deserialize to `none` +/// pre-Clarity-6; the typed pass reaches `MAX_TYPE_DEPTH`, so the pre-check +/// must reach it too or the bad key escapes detection. +#[apply(test_clarity_versions)] +fn test_from_consensus_buff_deeply_nested_underscore_tuple_key_pre_clarity6( + version: ClarityVersion, + epoch: StacksEpochId, +) { + // `from-consensus-buff?` is only available in Clarity 2+; the divergence is + // confined to the pre-Clarity-6 (pre-Epoch40) upgrade window. + if version < ClarityVersion::Clarity2 || epoch >= StacksEpochId::Epoch40 { + return; + } + + // Bury the offending tuple under enough `optional` layers that the + // leading-`_` key sits deeper than the untyped pre-check's depth-16 cap + // but within the typed pass's depth-32 cap. + const DEPTH: usize = 20; + + let mut value = Value::Tuple( + TupleData::from_data(vec![ + (ClarityName::from_literal("a"), Value::Int(456)), + (ClarityName::from_literal("_x"), Value::Int(123)), + ]) + .expect("construction of `_x` tuple should succeed with the relaxed regex"), + ); + for _ in 0..DEPTH { + value = Value::some(value).expect("nest value in optional"); + } + let hex = value.serialize_to_hex().expect("serialize value to hex"); + + let type_repr = format!( + "{}{{a: int}}{}", + "(optional ".repeat(DEPTH), + ")".repeat(DEPTH) + ); + let program = format!("(from-consensus-buff? {type_repr} 0x{hex})"); + let result = execute_with_parameters(&program, version, epoch, false) + .expect("from-consensus-buff? must not crash") + .expect("from-consensus-buff? must produce a value"); + + assert_eq!( + result, + Value::none(), + "pre-Clarity-6 epochs must reject a `_`-prefixed tuple key at ANY nesting \ + depth to match unpatched-node behavior, but a key below depth 16 escaped \ + the pre-check (got {result:?})" + ); +} + fn evaluate_to_ascii(snippet: &str) -> Value { execute_versioned(snippet, ClarityVersion::latest()) .unwrap_or_else(|e| panic!("Execution failed for snippet `{snippet}`: {e:?}")) diff --git a/clarity/src/vm/tests/representations.rs b/clarity/src/vm/tests/representations.rs index 53e6059af2c..43218a034be 100644 --- a/clarity/src/vm/tests/representations.rs +++ b/clarity/src/vm/tests/representations.rs @@ -34,18 +34,22 @@ fn assert_regex_unchanged(actual: &str, expected: &str) { /// This function creates a branched strategy based on the `CLARITY_NAME_REGEX_STRING` pattern. /// /// The strategy covers three categories of valid names: -/// - Letter-based names starting with a letter followed by alphanumeric or symbol characters +/// - Identifier names starting with a letter or `_` (Clarity 6 added +/// the `_` leading position, including the bare `_` discard name) followed +/// by zero or more alphanumeric or symbol characters /// - Single arithmetic operators (`-`, `+`, `=`, `/`, `*`) /// - Comparison operators (`<`, `>`, `<=`, `>=`) fn any_valid_clarity_name() -> impl Strategy { // Ensure the regex branches match the actual validator. assert_regex_unchanged( CLARITY_NAME_REGEX_STRING.as_str(), - "^[a-zA-Z]([a-zA-Z0-9]|[-_!?+<>=/*])*$|^[-+=/*]$|^[<>]=?$", + "^[a-zA-Z_]([a-zA-Z0-9]|[-_!?+<>=/*])*$|^[-+=/*]$|^[<>]=?$", ); - let letter_names = string_regex(&format!( - "[a-zA-Z][a-zA-Z0-9_!?+<>=/*-]{{0,{}}}", + // Identifier names: letter-or-underscore start (the `_` case includes the + // bare `_` because the body length floor is 0). + let identifier_names = string_regex(&format!( + "[a-zA-Z_][a-zA-Z0-9_!?+<>=/*-]{{0,{}}}", (MAX_STRING_LEN as usize).saturating_sub(1) )) .unwrap(); @@ -65,7 +69,7 @@ fn any_valid_clarity_name() -> impl Strategy { Just(">=".to_string()), ]; - prop_oneof![letter_names, single_ops, comparison_ops] + prop_oneof![identifier_names, single_ops, comparison_ops] } #[tag(t_prop)] @@ -87,20 +91,20 @@ fn prop_clarity_name_valid_patterns() { /// by `ClarityName::try_from()` validation by systematically violating each valid branch. /// /// The strategy generates names that violate the three valid branches: -/// - Branch 1 violations: Invalid starting characters or invalid characters in letter-based names +/// - Branch 1 violations: Invalid starting characters or invalid characters in identifier names /// - Branch 2 violations: Multi-character strings starting with single operators /// - Branch 3 violations: Invalid extensions to comparison operators /// - General violations: Empty strings and length violations /// /// Valid branches being violated: -/// 1. `^[a-zA-Z]([a-zA-Z0-9]|[-_!?+<>=/*])*$` - Letter-based names -/// 2. `^[-+=/*]$` - Single arithmetic operators -/// 3. `^[<>]=?$` - Comparison operators +/// 1. `^[a-zA-Z_]([a-zA-Z0-9]|[-_!?+<>=/*])*$` - Identifier names (letter- or `_`-led) +/// 2. `^[-+=/*]$` - Single arithmetic operators +/// 3. `^[<>]=?$` - Comparison operators fn any_invalid_clarity_name() -> impl Strategy { // Ensure the regex branches match the actual validator. assert_regex_unchanged( CLARITY_NAME_REGEX_STRING.as_str(), - "^[a-zA-Z]([a-zA-Z0-9]|[-_!?+<>=/*])*$|^[-+=/*]$|^[<>]=?$", + "^[a-zA-Z_]([a-zA-Z0-9]|[-_!?+<>=/*])*$|^[-+=/*]$|^[<>]=?$", ); let empty_string = Just("".to_string()); diff --git a/clarity/src/vm/tests/simple_apply_eval.rs b/clarity/src/vm/tests/simple_apply_eval.rs index 9ce94dd2d76..dcaa4bb3967 100644 --- a/clarity/src/vm/tests/simple_apply_eval.rs +++ b/clarity/src/vm/tests/simple_apply_eval.rs @@ -64,6 +64,350 @@ fn test_doubly_defined_persisted_vars() { } } +/// Clarity 6: bare `_` is a discard binding in `let`. The value is evaluated +/// (so early-exit forms like `unwrap-panic` still fire) but the name is not +/// added to scope, and multiple `_` bindings do not conflict. +#[test] +fn test_let_discard_bare_underscore() { + let program = "(let ((_ 1) (_ 2) (x 3)) (+ x 4))"; + let result = execute_with_parameters( + program, + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap() + .unwrap(); + assert_eq!(result, Value::Int(7)); +} + +/// Clarity 6: a bare `_` binding in `let` does not place `_` in scope. The body +/// referring to `_` should fail with an unbound-variable error rather than +/// returning the discarded value. +#[test] +fn test_let_discard_underscore_not_referenceable() { + let program = "(let ((_ 7)) _)"; + let err = execute_with_parameters( + program, + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap_err(); + let msg = format!("{err:?}"); + assert!( + msg.contains("Undefined variable: _"), + "expected an unbound-variable error for `_`, got: {msg}" + ); +} + +/// Clarity 6: a bare-`_` `let` binding must short-circuit on `try!` just +/// like a regular binding would — the SIP's worked example uses this. +#[test] +fn test_let_discard_with_try_short_circuits() { + let program = "(define-public (foo) + (let ((_ (try! (err u7)))) + (ok u0))) + (foo)"; + let result = execute_with_parameters( + program, + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap() + .unwrap(); + // `try!` should propagate `(err u7)` out of `foo` instead of returning `(ok u0)`. + let msg = format!("{result:?}"); + assert!( + msg.contains("Response(ResponseData") && msg.contains("UInt(7)"), + "expected `(err u7)` propagated by `try!`, got: {msg}" + ); +} + +/// Nested `let`s where both outer and inner bind bare `_`. Each scope's +/// discard is independent — neither should leak the other's `_` nor raise +/// `NameAlreadyUsed` on the inner binding. +#[test] +fn test_nested_let_with_bare_underscore_discards() { + let program = "(let ((_ 1) (x 10)) + (let ((_ 2) (y 20)) + (+ x y)))"; + let result = execute_with_parameters( + program, + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap() + .unwrap(); + assert_eq!(result, Value::Int(30)); +} + +/// Symmetry with `test_let_underscore_prefix_is_regular_binding`: +/// underscore-prefixed match-arm names are regular bindings (not discards) +/// and can be referenced in the branch body. +#[test] +fn test_match_underscore_prefix_is_regular_binding() { + let program = "(match (some 42) _val _val 0)"; + let result = execute_with_parameters( + program, + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap() + .unwrap(); + assert_eq!(result, Value::Int(42)); +} + +/// Clarity 6: underscore-prefixed names (e.g. `_admin`) are *regular* bindings +/// — the leading `_` is just a convention. They can be read back. +#[test] +fn test_let_underscore_prefix_is_regular_binding() { + let program = "(let ((_admin 42)) _admin)"; + let result = execute_with_parameters( + program, + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap() + .unwrap(); + assert_eq!(result, Value::Int(42)); +} + +/// Clarity 6: bare `_` in `match` (optional form) discards the matched value +/// without binding the name. +#[test] +fn test_match_opt_discard_bare_underscore() { + let program = "(match (some 5) _ 1 0)"; + let result = execute_with_parameters( + program, + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap() + .unwrap(); + assert_eq!(result, Value::Int(1)); +} + +/// Clarity 6: bare `_` in `match` (response form) discards on both arms. +#[test] +fn test_match_resp_discard_bare_underscore() { + let program_ok = "(match (ok 7) _ 1 _ 2)"; + let result = execute_with_parameters( + program_ok, + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap() + .unwrap(); + assert_eq!(result, Value::Int(1)); + + // Force the err-arm with a `response int int` whose err side is taken. + let program_err = "(match (err 7) _ 1 _ 2)"; + let result = execute_with_parameters( + program_err, + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap() + .unwrap(); + assert_eq!(result, Value::Int(2)); +} + +/// A bare-`_` `let` binding must still evaluate its bound expression. +/// A guaranteed runtime fault in the expression proves evaluation +/// happened: were it skipped, the let would return `0` instead. +#[test] +fn test_let_discard_still_evaluates_value() { + let program = "(let ((_ (/ 1 0))) 0)"; + let err = execute_with_parameters( + program, + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap_err(); + let msg = format!("{err:?}"); + assert!( + msg.contains("DivisionByZero"), + "expected a division-by-zero error, got: {msg}" + ); +} + +/// Some-arm taken with a bare-`_` bind: discards `99`, returns `11`. +#[test] +fn test_match_opt_none_arm_with_discard_some() { + let program = "(match (some 99) _ 11 22)"; + let result = execute_with_parameters( + program, + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap() + .unwrap(); + assert_eq!(result, Value::Int(11)); +} + +/// Rejected by the analyzer's `check_name_used`. +#[test] +fn test_bare_underscore_as_define_name_rejected_in_clarity6() { + let program = "(define-constant _ 1)"; + let err = execute_with_parameters( + program, + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap_err(); + let msg = format!("{err:?}"); + assert!( + msg.contains("BareUnderscoreReserved"), + "expected BareUnderscoreReserved error, got: {msg}" + ); +} + +/// Clarity 6: bare `_` cannot name a `use-trait` alias — would create a +/// referenceable `<_>` trait alias otherwise. +#[test] +fn test_bare_underscore_as_use_trait_alias_rejected_in_clarity6() { + let err = execute_with_parameters( + "(use-trait _ 'SP2J6ZY48GV1EZ5V2V5RB9MP66SW86PYKKNRV9EJ7.foo.bar)", + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap_err(); + let msg = format!("{err:?}"); + assert!( + msg.contains("BareUnderscoreReserved"), + "expected BareUnderscoreReserved error, got: {msg}" + ); +} + +/// Clarity 6: bare `_` cannot name a `define-trait` method — implementing +/// contracts would have a referenceable `_` function. +#[test] +fn test_bare_underscore_as_trait_method_rejected_in_clarity6() { + let err = execute_with_parameters( + "(define-trait t ((_ (uint) (response uint uint))))", + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap_err(); + let msg = format!("{err:?}"); + assert!( + msg.contains("BareUnderscoreReserved"), + "expected BareUnderscoreReserved error, got: {msg}" + ); +} + +/// Clarity 6: bare `_` cannot be a tuple key — `(get _ tup)` would resolve +/// the value, making `_` referenceable. +#[test] +fn test_bare_underscore_as_tuple_key_rejected_in_clarity6() { + let err = execute_with_parameters( + "(define-constant x { _: u1 })", + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap_err(); + let msg = format!("{err:?}"); + assert!( + msg.contains("BareUnderscoreReserved"), + "expected BareUnderscoreReserved error, got: {msg}" + ); +} + +/// Clarity 6: bare `_` is rejected in tuple TYPE positions too, not just +/// tuple-literal values. `(define-map foo { _: uint } …)` would otherwise +/// register a referenceable `_` field. +#[test] +fn test_bare_underscore_as_tuple_type_key_rejected_in_clarity6() { + let err = execute_with_parameters( + "(define-map foo { _: uint } { val: uint })", + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap_err(); + let msg = format!("{err:?}"); + assert!( + msg.contains("BareUnderscoreReserved"), + "expected BareUnderscoreReserved error, got: {msg}" + ); +} + +#[test] +fn test_bare_underscore_as_function_arg_rejected_in_clarity6() { + let program = "(define-public (foo (_ uint)) (ok true)) (foo u1)"; + let err = execute_with_parameters( + program, + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap_err(); + let msg = format!("{err:?}"); + assert!( + msg.contains("BareUnderscoreReserved"), + "expected BareUnderscoreReserved error, got: {msg}" + ); +} + +/// The SIP requires that referencing `_` after a discard is an +/// *analysis* error (not just a runtime failure). The analyzer's +/// type-check sees `_` as unbound and emits `UndefinedVariable("_")`. +#[test] +fn test_let_discard_underscore_reference_is_analysis_error() { + let err = crate::vm::analysis::type_checker::v2_1::tests::mem_type_check("(let ((_ 7)) _)") + .expect_err("expected analysis error"); + let msg = format!("{err:?}"); + assert!( + msg.contains("UndefinedVariable") && msg.contains("\"_\""), + "expected `UndefinedVariable(\"_\")` analysis error, got: {msg}" + ); +} + +/// Same SIP requirement for `match` arms. +#[test] +fn test_match_discard_underscore_reference_is_analysis_error() { + let err = + crate::vm::analysis::type_checker::v2_1::tests::mem_type_check("(match (some 5) _ _ 0)") + .expect_err("expected analysis error"); + let msg = format!("{err:?}"); + assert!( + msg.contains("UndefinedVariable") && msg.contains("\"_\""), + "expected `UndefinedVariable(\"_\")` analysis error, got: {msg}" + ); +} + +/// Clarity 6: a bare-`_` `match` branch must not be referenceable in its body. +#[test] +fn test_match_opt_discard_underscore_not_referenceable() { + let program = "(match (some 5) _ _ 0)"; + let err = execute_with_parameters( + program, + ClarityVersion::Clarity6, + StacksEpochId::Epoch40, + false, + ) + .unwrap_err(); + let msg = format!("{err:?}"); + assert!( + msg.contains("Undefined variable: _"), + "expected an unbound-variable error for `_`, got: {msg}" + ); +} + #[apply(test_clarity_versions)] fn test_simple_let(#[case] version: ClarityVersion, #[case] epoch: StacksEpochId) { /* diff --git a/clarity/src/vm/types/signatures.rs b/clarity/src/vm/types/signatures.rs index 62659be5daa..9459cdd1547 100644 --- a/clarity/src/vm/types/signatures.rs +++ b/clarity/src/vm/types/signatures.rs @@ -31,7 +31,7 @@ use crate::vm::analysis::type_checker::v2_1::{MAX_FUNCTION_PARAMETERS, MAX_TRAIT use crate::vm::costs::{CostOverflowingMath, runtime_cost}; use crate::vm::errors::{SyntaxBindingError, SyntaxBindingErrorType}; use crate::vm::representations::{ - ClarityName, SymbolicExpression, SymbolicExpressionType, TraitDefinition, + ClarityName, DISCARD_IDENTIFIER, SymbolicExpression, SymbolicExpressionType, TraitDefinition, }; #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -255,6 +255,15 @@ impl TypeSignatureExt for TypeSignature { SyntaxBindingErrorType::TupleCons, accounting, )?; + // Clarity 6: bare `_` cannot name a tuple-type key. Without this, + // `(define-map foo { _: uint } …)` would register a referenceable + // `_` field accessible via `(get _ …)`. + if mapped_key_types + .iter() + .any(|(name, _)| name.as_str() == DISCARD_IDENTIFIER) + { + return Err(CommonCheckErrorKind::BareUnderscoreReserved); + } let tuple_type_signature = TupleTypeSignature::try_from(mapped_key_types)?; Ok(TypeSignature::from(tuple_type_signature)) } @@ -439,6 +448,11 @@ impl TypeSignatureExt for TypeSignature { let fn_name = args[0] .match_atom() .ok_or(CommonCheckErrorKind::DefineTraitBadSignature)?; + // Clarity 6: bare `_` is reserved as a discard pattern and cannot + // name a trait method. + if fn_name.as_str() == DISCARD_IDENTIFIER { + return Err(CommonCheckErrorKind::BareUnderscoreReserved); + } // Extract function's arguments let fn_args_exprs = args[1] diff --git a/contrib/stacks-cli/src/main.rs b/contrib/stacks-cli/src/main.rs index c2cbdbe047e..a26954a439d 100644 --- a/contrib/stacks-cli/src/main.rs +++ b/contrib/stacks-cli/src/main.rs @@ -24,8 +24,9 @@ use std::{env, fs, io}; use clarity::vm::ast::errors::ParseError; use clarity::vm::errors::{ClarityEvalError, ClarityTypeError, VmExecutionError}; +use clarity::vm::representations::ClarityName; use clarity::vm::types::PrincipalData; -use clarity::vm::{ClarityName, ClarityVersion, ContractName, Value}; +use clarity::vm::{ClarityVersion, ContractName, Value}; use clarity_cli::vm_execute; use stacks_common::address::{AddressHashMode, b58}; use stacks_common::codec::{Error as CodecError, StacksMessageCodec}; diff --git a/stacks-codec/src/transaction.rs b/stacks-codec/src/transaction.rs index bfffb73f039..e63f8cee4a1 100644 --- a/stacks-codec/src/transaction.rs +++ b/stacks-codec/src/transaction.rs @@ -2038,7 +2038,7 @@ impl TransactionAuth { } } -/// A transaction that calls into a smart contract +/// A transaction that calls into a smart contract. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct TransactionContractCall { pub address: StacksAddress, @@ -2133,7 +2133,7 @@ impl StacksMessageCodec for TransactionSmartContract { } } -/// Encoding of an asset type identifier +/// Encoding of an asset type identifier. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct AssetInfo { pub contract_address: StacksAddress, @@ -3693,6 +3693,69 @@ mod tests { ); } + /// The wide `ClarityName` codec admits leading-`_` bytes — Clarity 6's + /// relaxation lives at the codec layer. Consensus during the upgrade + /// window is preserved by `StacksBlock::validate_transaction_static_epoch`, + /// which rejects the resulting `TransactionContractCall` at admission + /// when the active epoch is pre-Clarity-6; see the admission tests in + /// `chainstate/stacks/block.rs`. + #[test] + fn tx_contract_call_function_name_with_leading_underscore_round_trips() { + let address = StacksAddress::new(1, Hash160([0xff; 20])).unwrap(); + let contract_name = ContractName::try_from("hello-contract-name").unwrap(); + let function_name_str = "_admin"; + let mut function_name_bytes = vec![function_name_str.len() as u8]; + function_name_bytes.extend_from_slice(function_name_str.as_bytes()); + + let function_args: Vec = vec![]; + + let mut contract_call_bytes = vec![]; + address + .consensus_serialize(&mut contract_call_bytes) + .unwrap(); + contract_name + .consensus_serialize(&mut contract_call_bytes) + .unwrap(); + contract_call_bytes.extend_from_slice(&function_name_bytes); + function_args + .consensus_serialize(&mut contract_call_bytes) + .unwrap(); + + let mut tx_bytes = vec![TransactionPayloadID::ContractCall as u8]; + tx_bytes.append(&mut contract_call_bytes); + + let payload = TransactionPayload::consensus_deserialize(&mut &tx_bytes[..]) + .expect("leading-`_` function_name should round-trip at the codec layer"); + let TransactionPayload::ContractCall(cc) = payload else { + panic!("expected ContractCall payload"); + }; + assert_eq!(cc.function_name.as_str(), "_admin"); + } + + /// Analogous to the function_name test: leading-`_` `asset_name` + /// round-trips at the `AssetInfo` codec layer; admission gates it. + #[test] + fn tx_asset_info_asset_name_with_leading_underscore_round_trips() { + let contract_address = StacksAddress::new(1, Hash160([0xff; 20])).unwrap(); + let contract_name = ContractName::try_from("hello-contract-name").unwrap(); + let asset_name_str = "_admin"; + let mut asset_name_bytes = vec![asset_name_str.len() as u8]; + asset_name_bytes.extend_from_slice(asset_name_str.as_bytes()); + + let mut asset_info_bytes = vec![]; + contract_address + .consensus_serialize(&mut asset_info_bytes) + .unwrap(); + contract_name + .consensus_serialize(&mut asset_info_bytes) + .unwrap(); + asset_info_bytes.extend_from_slice(&asset_name_bytes); + + let info = AssetInfo::consensus_deserialize(&mut &asset_info_bytes[..]) + .expect("leading-`_` asset_name should round-trip at the codec layer"); + assert_eq!(info.asset_name.as_str(), "_admin"); + } + #[test] fn tx_stacks_asset() { let addr = StacksAddress::new(1, Hash160([0xff; 20])).unwrap(); diff --git a/stacks-node/src/event_dispatcher/tests.rs b/stacks-node/src/event_dispatcher/tests.rs index 5f4c2936e86..ce0bd8ccb0b 100644 --- a/stacks-node/src/event_dispatcher/tests.rs +++ b/stacks-node/src/event_dispatcher/tests.rs @@ -22,8 +22,9 @@ use std::time::{Instant, SystemTime}; use clarity::boot_util::boot_code_id; use clarity::vm::costs::ExecutionCost; use clarity::vm::events::SmartContractEventData; +use clarity::vm::representations::ClarityName; use clarity::vm::types::StacksAddressExtensions; -use clarity::vm::{ClarityName, ContractName, Value}; +use clarity::vm::{ContractName, Value}; use rusqlite::Connection; use serial_test::serial; use stacks::address::{AddressHashMode, C32_ADDRESS_VERSION_TESTNET_SINGLESIG}; diff --git a/stacks-node/src/tests/neon_integrations.rs b/stacks-node/src/tests/neon_integrations.rs index 916e3a5b297..db2e5d635c4 100644 --- a/stacks-node/src/tests/neon_integrations.rs +++ b/stacks-node/src/tests/neon_integrations.rs @@ -23,11 +23,10 @@ use std::{cmp, env, fs, io, thread}; use clarity::vm::ast::stack_depth_checker::StackDepthLimits; use clarity::vm::costs::ExecutionCost; +use clarity::vm::representations::ClarityName; use clarity::vm::types::serialization::SerializationError; use clarity::vm::types::PrincipalData; -use clarity::vm::{ - execute_with_parameters as execute, ClarityName, ClarityVersion, ContractName, Value, -}; +use clarity::vm::{execute_with_parameters as execute, ClarityVersion, ContractName, Value}; use rusqlite::params; use serde::Deserialize; use serde_json::json; diff --git a/stackslib/src/chainstate/coordinator/tests.rs b/stackslib/src/chainstate/coordinator/tests.rs index f27d294620a..a8e823d0e3f 100644 --- a/stackslib/src/chainstate/coordinator/tests.rs +++ b/stackslib/src/chainstate/coordinator/tests.rs @@ -24,8 +24,9 @@ use clarity::vm::clarity::TransactionConnection; use clarity::vm::costs::{ExecutionCost, LimitedCostTracker}; use clarity::vm::database::BurnStateDB; use clarity::vm::errors::ClarityEvalError; +use clarity::vm::representations::ClarityName; use clarity::vm::types::{PrincipalData, QualifiedContractIdentifier}; -use clarity::vm::{ClarityName, ContractName, Value}; +use clarity::vm::{ContractName, Value}; use lazy_static::lazy_static; use rusqlite::Connection; use stacks_common::address; diff --git a/stackslib/src/chainstate/nakamoto/tests/mod.rs b/stackslib/src/chainstate/nakamoto/tests/mod.rs index f7bbc26e4c2..b8677987e75 100644 --- a/stackslib/src/chainstate/nakamoto/tests/mod.rs +++ b/stackslib/src/chainstate/nakamoto/tests/mod.rs @@ -19,8 +19,9 @@ use std::collections::HashMap; use clarity::types::chainstate::{SortitionId, StacksBlockId}; use clarity::util::secp256k1::Secp256k1PrivateKey; use clarity::vm::costs::ExecutionCost; +use clarity::vm::representations::ClarityName; use clarity::vm::types::StacksAddressExtensions; -use clarity::vm::{ClarityName, ContractName, Value}; +use clarity::vm::{ContractName, Value}; use libstackerdb::StackerDBChunkData; use rand::distributions::Standard; use rand::{thread_rng, Rng, RngCore}; diff --git a/stackslib/src/chainstate/stacks/block.rs b/stackslib/src/chainstate/stacks/block.rs index ee1ac7a0483..2eb0262234c 100644 --- a/stackslib/src/chainstate/stacks/block.rs +++ b/stackslib/src/chainstate/stacks/block.rs @@ -631,6 +631,68 @@ impl StacksBlock { error!("Authentication mode not supported in Epoch {epoch_id}"); return false; } + // Reject leading-`_` names at every wire position when + // `epoch_id < Epoch40` (i.e., before Clarity 6 activates). + // This mirrors un-upgraded nodes' narrow wire codec and so + // preserves consensus during the upgrade window. Bare `_` is + // *also* rejected for tuple keys at every epoch (it would + // otherwise be referenceable via `(get _ …)`, contradicting + // its discard semantic). `SmartContract.code_body` is + // intentionally skipped: its bytes are version-blind + // `StacksString` (no `ClarityName` codec step), and + // source-level rejection happens in `UnderscoreIdentifierChecker`. + if let TransactionPayload::ContractCall(ref cc) = &tx.payload { + if epoch_id < StacksEpochId::Epoch40 && cc.function_name.starts_with('_') { + error!( + "Disallowed leading-`_` function_name pre-Clarity-6"; + "txid" => %tx.txid(), + "epoch" => %epoch_id, + "function_name" => %cc.function_name, + ); + return false; + } + for arg in &cc.function_args { + if let Some(bad_key) = arg.find_invalid_tuple_key(epoch_id) { + error!( + "Disallowed tuple key in function argument"; + "txid" => %tx.txid(), + "epoch" => %epoch_id, + "key" => %bad_key, + ); + return false; + } + } + } + for post_condition in tx.post_conditions.iter() { + let asset_info_opt = match post_condition { + TransactionPostCondition::Fungible(_, ref ai, _, _) + | TransactionPostCondition::Nonfungible(_, ref ai, _, _) => Some(ai), + TransactionPostCondition::STX(..) => None, + }; + if let Some(ai) = asset_info_opt { + if epoch_id < StacksEpochId::Epoch40 && ai.asset_name.starts_with('_') { + error!( + "Disallowed leading-`_` asset_name pre-Clarity-6"; + "txid" => %tx.txid(), + "epoch" => %epoch_id, + "asset_name" => %ai.asset_name, + ); + return false; + } + } + if let TransactionPostCondition::Nonfungible(_, _, ref asset_value, _) = post_condition + { + if let Some(bad_key) = asset_value.find_invalid_tuple_key(epoch_id) { + error!( + "Disallowed tuple key in NFT post-condition asset value"; + "txid" => %tx.txid(), + "epoch" => %epoch_id, + "key" => %bad_key, + ); + return false; + } + } + } return true; } @@ -801,6 +863,8 @@ impl StacksMicroblock { #[cfg(test)] mod test { use clarity::types::PublicKey; + use clarity::vm::representations::ClarityName; + use clarity::vm::types::TupleData; use rstest::rstest; use stacks_common::address::*; use stacks_common::types::chainstate::StacksAddress; @@ -2061,6 +2125,277 @@ mod test { ); } + /// Build a single-key tuple `Value` for use as a function argument + /// or post-condition asset value in the tests below. + fn single_key_tuple(key: &str) -> Value { + Value::Tuple( + TupleData::from_data(vec![( + ClarityName::try_from(key.to_string()).unwrap(), + Value::Int(0), + )]) + .unwrap(), + ) + } + + fn admission_test_auth(privk: &StacksPrivateKey) -> TransactionAuth { + TransactionAuth::Standard( + TransactionSpendingCondition::new_singlesig_p2pkh(StacksPublicKey::from_private(privk)) + .unwrap(), + ) + } + + /// Contract-call transaction whose `function_args` contain a one-key + /// tuple with the supplied key. + fn admission_test_contract_call_with_tuple_arg(key: &str) -> StacksTransaction { + let privk = StacksPrivateKey::random(); + StacksTransaction::new( + TransactionVersion::Testnet, + admission_test_auth(&privk), + TransactionPayload::ContractCall(TransactionContractCall { + address: StacksAddress::new(1, Hash160([0x11; 20])).unwrap(), + contract_name: ContractName::try_from("hello-world").unwrap(), + function_name: ClarityName::try_from("do-thing").unwrap(), + function_args: vec![single_key_tuple(key)], + }), + ) + } + + /// Token-transfer transaction carrying a single Nonfungible + /// post-condition whose `asset_value` is a one-key tuple with the + /// supplied key. + fn admission_test_nft_post_condition_with_tuple(key: &str) -> StacksTransaction { + let privk = StacksPrivateKey::random(); + let mut tx = StacksTransaction::new( + TransactionVersion::Testnet, + admission_test_auth(&privk), + TransactionPayload::TokenTransfer( + PrincipalData::from(StacksAddress::new(1, Hash160([0x11; 20])).unwrap()), + 1, + TokenTransferMemo([0u8; 34]), + ), + ); + tx.post_conditions + .push(TransactionPostCondition::Nonfungible( + PostConditionPrincipal::Origin, + AssetInfo { + contract_address: StacksAddress::new(1, Hash160([0x22; 20])).unwrap(), + contract_name: ContractName::try_from("hello-world").unwrap(), + asset_name: ClarityName::try_from("asset").unwrap(), + }, + single_key_tuple(key), + NonfungibleConditionCode::Sent, + )); + tx + } + + /// Build a contract-call transaction whose `function_name` is the + /// supplied string. Bypasses `ClarityName`-style construction + /// so we can exercise admission rejection independently of any + /// type-system constraints on field construction. + fn admission_test_contract_call_with_function_name(name: &str) -> StacksTransaction { + let privk = StacksPrivateKey::random(); + StacksTransaction::new( + TransactionVersion::Testnet, + admission_test_auth(&privk), + TransactionPayload::ContractCall(TransactionContractCall { + address: StacksAddress::new(1, Hash160([0x11; 20])).unwrap(), + contract_name: ContractName::try_from("hello-world").unwrap(), + function_name: ClarityName::try_from(name).unwrap(), + function_args: vec![], + }), + ) + } + + /// Token-transfer transaction carrying one NFT post-condition whose + /// `asset_name` is the supplied string. + fn admission_test_nft_post_condition_with_asset_name(name: &str) -> StacksTransaction { + let privk = StacksPrivateKey::random(); + let mut tx = StacksTransaction::new( + TransactionVersion::Testnet, + admission_test_auth(&privk), + TransactionPayload::TokenTransfer( + PrincipalData::from(StacksAddress::new(1, Hash160([0x11; 20])).unwrap()), + 1, + TokenTransferMemo([0u8; 34]), + ), + ); + tx.post_conditions + .push(TransactionPostCondition::Nonfungible( + PostConditionPrincipal::Origin, + AssetInfo { + contract_address: StacksAddress::new(1, Hash160([0x22; 20])).unwrap(), + contract_name: ContractName::try_from("hello-world").unwrap(), + asset_name: ClarityName::try_from(name).unwrap(), + }, + Value::Int(0), + NonfungibleConditionCode::Sent, + )); + tx + } + + /// Build a contract-call transaction whose single `function_args` + /// element is the supplied value. Used to exercise the admission + /// walker's descent through compound containers. + fn admission_test_contract_call_with_arg(arg: Value) -> StacksTransaction { + let privk = StacksPrivateKey::random(); + StacksTransaction::new( + TransactionVersion::Testnet, + admission_test_auth(&privk), + TransactionPayload::ContractCall(TransactionContractCall { + address: StacksAddress::new(1, Hash160([0x11; 20])).unwrap(), + contract_name: ContractName::try_from("hello-world").unwrap(), + function_name: ClarityName::try_from("do-thing").unwrap(), + function_args: vec![arg], + }), + ) + } + + /// Wrap a `_buried`-keyed tuple inside the supplied compound shape so + /// `find_invalid_tuple_key`'s recursion paths can be exercised + /// end-to-end through the admission walker. + fn buried_underscore_value(shape: &str) -> Value { + let bad_tuple = single_key_tuple("_buried"); + match shape { + "some" => Value::some(bad_tuple).unwrap(), + "ok" => Value::okay(bad_tuple).unwrap(), + "err" => Value::error(bad_tuple).unwrap(), + "list" => Value::list_from(vec![bad_tuple]).unwrap(), + "nested_tuple" => Value::Tuple( + TupleData::from_data(vec![(ClarityName::from_literal("outer"), bad_tuple)]) + .unwrap(), + ), + other => panic!("unknown shape: {other}"), + } + } + + /// Admission walker: tuple keys in `function_args` and NFT + /// `asset_value`. Plain keys are admissible at every epoch; bare + /// `_` is rejected at every epoch (reserved as the discard marker); + /// leading-`_` is rejected pre-Clarity-6 (to match un-upgraded + /// nodes' narrow wire codec) and admitted from Clarity-6 onward. + #[rstest] + #[case::plain_epoch33(StacksEpochId::Epoch33, "foo", true)] + #[case::plain_epoch34(StacksEpochId::Epoch34, "foo", true)] + #[case::plain_epoch40(StacksEpochId::Epoch40, "foo", true)] + #[case::bare_underscore_epoch33(StacksEpochId::Epoch33, "_", false)] + #[case::bare_underscore_epoch34(StacksEpochId::Epoch34, "_", false)] + #[case::bare_underscore_epoch40(StacksEpochId::Epoch40, "_", false)] + #[case::leading_underscore_epoch33(StacksEpochId::Epoch33, "_admin", false)] + #[case::leading_underscore_epoch34(StacksEpochId::Epoch34, "_admin", false)] + #[case::leading_underscore_epoch40(StacksEpochId::Epoch40, "_admin", true)] + fn test_validate_transaction_static_epoch_tuple_keys( + #[case] epoch_id: StacksEpochId, + #[case] key: &str, + #[case] expected_admitted: bool, + ) { + let cc = admission_test_contract_call_with_tuple_arg(key); + assert_eq!( + StacksBlock::validate_transaction_static_epoch(&cc, epoch_id), + expected_admitted, + "function_args tuple key {key:?} at {epoch_id}", + ); + + let pc = admission_test_nft_post_condition_with_tuple(key); + assert_eq!( + StacksBlock::validate_transaction_static_epoch(&pc, epoch_id), + expected_admitted, + "NFT post-condition tuple key {key:?} at {epoch_id}", + ); + } + + /// Admission walker: scalar names — `TransactionContractCall.function_name` + /// and `AssetInfo.asset_name`. Plain names admissible everywhere; + /// leading-`_` rejected pre-Clarity-6, admitted from Clarity-6. + /// + /// Bare `_` is unreachable on this path: `ClarityName::try_from` + /// accepts it but the analyzer's `BareUnderscoreReserved` blocks + /// any contract that would declare a function or asset called `_`, + /// so no such transaction can refer to a real deployable target. + #[rstest] + #[case::plain_epoch33(StacksEpochId::Epoch33, "do-thing", "asset", true)] + #[case::plain_epoch34(StacksEpochId::Epoch34, "do-thing", "asset", true)] + #[case::plain_epoch40(StacksEpochId::Epoch40, "do-thing", "asset", true)] + #[case::leading_underscore_epoch33(StacksEpochId::Epoch33, "_admin", "_admin", false)] + #[case::leading_underscore_epoch34(StacksEpochId::Epoch34, "_admin", "_admin", false)] + #[case::leading_underscore_epoch40(StacksEpochId::Epoch40, "_admin", "_admin", true)] + fn test_validate_transaction_static_epoch_scalar_names( + #[case] epoch_id: StacksEpochId, + #[case] function_name: &str, + #[case] asset_name: &str, + #[case] expected_admitted: bool, + ) { + let cc = admission_test_contract_call_with_function_name(function_name); + assert_eq!( + StacksBlock::validate_transaction_static_epoch(&cc, epoch_id), + expected_admitted, + "function_name {function_name:?} at {epoch_id}", + ); + + let pc = admission_test_nft_post_condition_with_asset_name(asset_name); + assert_eq!( + StacksBlock::validate_transaction_static_epoch(&pc, epoch_id), + expected_admitted, + "asset_name {asset_name:?} at {epoch_id}", + ); + } + + /// Admission must descend into compound containers in `function_args` + /// to find buried `_`-prefixed tuple keys: rejected pre-Clarity-6, + /// admitted from Clarity-6 onward. Covers `Optional::Some`, + /// `Response` (ok / err), `Sequence(List)`, and nested `Tuple`. + #[rstest] + #[case::some_pre_clarity6("some", StacksEpochId::Epoch34, false)] + #[case::some_clarity6("some", StacksEpochId::Epoch40, true)] + #[case::ok_pre_clarity6("ok", StacksEpochId::Epoch34, false)] + #[case::ok_clarity6("ok", StacksEpochId::Epoch40, true)] + #[case::err_pre_clarity6("err", StacksEpochId::Epoch34, false)] + #[case::err_clarity6("err", StacksEpochId::Epoch40, true)] + #[case::list_pre_clarity6("list", StacksEpochId::Epoch34, false)] + #[case::list_clarity6("list", StacksEpochId::Epoch40, true)] + #[case::nested_tuple_pre_clarity6("nested_tuple", StacksEpochId::Epoch34, false)] + #[case::nested_tuple_clarity6("nested_tuple", StacksEpochId::Epoch40, true)] + fn test_validate_transaction_static_epoch_descends_into_compound_function_args( + #[case] shape: &str, + #[case] epoch_id: StacksEpochId, + #[case] expected_admitted: bool, + ) { + let arg = buried_underscore_value(shape); + let cc = admission_test_contract_call_with_arg(arg); + assert_eq!( + StacksBlock::validate_transaction_static_epoch(&cc, epoch_id), + expected_admitted, + "`_buried` inside {shape} at {epoch_id}", + ); + } + + /// Bare `_` tuple keys are rejected at every epoch — even when + /// buried inside compound containers — because the walker's + /// every-epoch rule short-circuits before the leading-`_` check. + #[rstest] + #[case::some_epoch34("some", StacksEpochId::Epoch34)] + #[case::some_epoch40("some", StacksEpochId::Epoch40)] + #[case::ok_epoch34("ok", StacksEpochId::Epoch34)] + #[case::ok_epoch40("ok", StacksEpochId::Epoch40)] + #[case::list_epoch34("list", StacksEpochId::Epoch34)] + #[case::list_epoch40("list", StacksEpochId::Epoch40)] + fn test_validate_transaction_static_epoch_rejects_bare_underscore_inside_compound( + #[case] shape: &str, + #[case] epoch_id: StacksEpochId, + ) { + let bare = single_key_tuple("_"); + let arg = match shape { + "some" => Value::some(bare).unwrap(), + "ok" => Value::okay(bare).unwrap(), + "list" => Value::list_from(vec![bare]).unwrap(), + other => panic!("unknown shape: {other}"), + }; + let cc = admission_test_contract_call_with_arg(arg); + assert!( + !StacksBlock::validate_transaction_static_epoch(&cc, epoch_id), + "bare `_` inside {shape} at {epoch_id} should be rejected", + ); + } + // TODO: // * size limits } diff --git a/stackslib/src/chainstate/stacks/db/transactions.rs b/stackslib/src/chainstate/stacks/db/transactions.rs index 01fc8fc3772..80d4b72cd09 100644 --- a/stackslib/src/chainstate/stacks/db/transactions.rs +++ b/stackslib/src/chainstate/stacks/db/transactions.rs @@ -1790,7 +1790,7 @@ impl StacksChainState { #[cfg(test)] pub mod test { use clarity::util::secp256k1::Secp256k1PrivateKey; - use clarity::vm::representations::{ClarityName, ContractName}; + use clarity::vm::representations::ContractName; use clarity::vm::test_util::{UnitTestBurnStateDB, TEST_BURN_STATE_DB}; use clarity::vm::tests::TEST_HEADER_DB; use clarity::vm::types::ResponseData; diff --git a/stackslib/src/chainstate/stacks/mod.rs b/stackslib/src/chainstate/stacks/mod.rs index ac3d1437d56..921122f3c22 100644 --- a/stackslib/src/chainstate/stacks/mod.rs +++ b/stackslib/src/chainstate/stacks/mod.rs @@ -489,7 +489,7 @@ pub const MAX_MICROBLOCK_SIZE: u32 = 65536; #[cfg(test)] pub mod test { - use clarity::vm::representations::{ClarityName, ContractName}; + use clarity::vm::representations::ContractName; use clarity::vm::ClarityVersion; use stacks_common::bitvec::BitVec; use stacks_common::util::get_epoch_time_secs; diff --git a/stackslib/src/chainstate/tests/consensus.rs b/stackslib/src/chainstate/tests/consensus.rs index 582171d1aa0..037cada1635 100644 --- a/stackslib/src/chainstate/tests/consensus.rs +++ b/stackslib/src/chainstate/tests/consensus.rs @@ -23,8 +23,9 @@ use clarity::types::{EpochList, StacksEpoch, StacksEpochId}; use clarity::util::hash::{Hash160, MerkleTree, Sha512Trunc256Sum}; use clarity::util::secp256k1::MessageSignature; use clarity::vm::costs::ExecutionCost; +use clarity::vm::representations::ClarityName; use clarity::vm::types::{PrincipalData, ResponseData}; -use clarity::vm::{ClarityName, ClarityVersion, ContractName, Value as ClarityValue}; +use clarity::vm::{ClarityVersion, ContractName, Value as ClarityValue}; use serde::{Deserialize, Serialize, Serializer}; use stacks_common::bitvec::BitVec; diff --git a/stackslib/src/chainstate/tests/madhouse/commands/sip040.rs b/stackslib/src/chainstate/tests/madhouse/commands/sip040.rs index 75bb4076988..543096f9ba6 100644 --- a/stackslib/src/chainstate/tests/madhouse/commands/sip040.rs +++ b/stackslib/src/chainstate/tests/madhouse/commands/sip040.rs @@ -15,8 +15,9 @@ use std::sync::Arc; +use clarity::vm::representations::ClarityName; use clarity::vm::types::PrincipalData; -use clarity::vm::{ClarityName, ClarityVersion, ContractName, Value}; +use clarity::vm::{ClarityVersion, ContractName, Value}; use madhouse::{Command, CommandWrapper}; use proptest::array::uniform4; use proptest::prelude::{any, Just, Strategy}; diff --git a/stackslib/src/chainstate/tests/parse_tests.rs b/stackslib/src/chainstate/tests/parse_tests.rs index 6d4317dafb4..c9a8387a370 100644 --- a/stackslib/src/chainstate/tests/parse_tests.rs +++ b/stackslib/src/chainstate/tests/parse_tests.rs @@ -102,6 +102,7 @@ fn variant_coverage_report(variant: ParseErrorKind) { IllegalClarityName(_) => Unreachable_Functionally("prevented by Lexer checks returning `Lexer` variant"), IllegalASCIIString(_) => Tested(vec![test_illegal_ascii_string]), IllegalContractName(_) => Unreachable_Functionally("prevented by Lexer checks returning `Lexer` variant or Parser by MAX_CONTRACT_NAME_LEN returning `ContractNameTooLong` variant"), + UnderscoreIdentifierNotAllowed(_) => Ignored("Reachable via deploys of pre-Clarity-6 contracts that contain `_`-prefixed identifiers. Covered by `clarity::vm::ast::underscore_checker::tests` rather than consensus-snapshot tests."), NoteToMatchThis(_) => Unreachable_Functionally("It is reachable, but only visible in diagnostic mode as it comes as a later diagnostic error"), UnexpectedParserFailure => Unreachable_ExpectLike, InterpreterFailure => Unreachable_ExpectLike, // currently cause block rejection diff --git a/stackslib/src/chainstate/tests/runtime_analysis_tests.rs b/stackslib/src/chainstate/tests/runtime_analysis_tests.rs index d6e66595a10..37593945106 100644 --- a/stackslib/src/chainstate/tests/runtime_analysis_tests.rs +++ b/stackslib/src/chainstate/tests/runtime_analysis_tests.rs @@ -144,6 +144,7 @@ fn variant_coverage_report(variant: RuntimeCheckErrorKind) { InvalidUTF8Encoding => { Ignored("Only reachable via legacy v1 parsing paths") }, + BareUnderscoreReserved => Ignored("Reachable only in Clarity 6+ when `_` appears as a name in a non-discard position. Covered by `clarity::vm::tests::simple_apply_eval` rather than consensus-snapshot tests."), }; } diff --git a/stackslib/src/chainstate/tests/snapshots/blockstack_lib__chainstate__tests__parse_tests__lexer_unknown_symbol.snap b/stackslib/src/chainstate/tests/snapshots/blockstack_lib__chainstate__tests__parse_tests__lexer_unknown_symbol.snap index e71f6cecf23..2326845590f 100644 --- a/stackslib/src/chainstate/tests/snapshots/blockstack_lib__chainstate__tests__parse_tests__lexer_unknown_symbol.snap +++ b/stackslib/src/chainstate/tests/snapshots/blockstack_lib__chainstate__tests__parse_tests__lexer_unknown_symbol.snap @@ -1,6 +1,8 @@ --- source: stackslib/src/chainstate/tests/parse_tests.rs -expression: result +assertion_line: 477 +expression: result.get_expected_results() +snapshot_kind: text --- [ Success(ExpectedBlockOutput( @@ -9,7 +11,7 @@ expression: result transactions: [ ExpectedTransactionOutput( tx: "SmartContract(name: my-contract-Epoch3_4-Clarity1, code_body: [..], clarity_version: Some(Clarity1))", - vm_error: "Some(unknown symbol, \'_\') [NON-CONSENSUS BREAKING]", + vm_error: "Some(:0:0: identifier \'_\' starts with \'_\', which requires Clarity 6 or later) [NON-CONSENSUS BREAKING]", return_type: Response(ResponseData( committed: false, data: Optional(OptionalData( @@ -39,7 +41,7 @@ expression: result transactions: [ ExpectedTransactionOutput( tx: "SmartContract(name: my-contract-Epoch3_4-Clarity2, code_body: [..], clarity_version: Some(Clarity2))", - vm_error: "Some(unknown symbol, \'_\') [NON-CONSENSUS BREAKING]", + vm_error: "Some(:0:0: identifier \'_\' starts with \'_\', which requires Clarity 6 or later) [NON-CONSENSUS BREAKING]", return_type: Response(ResponseData( committed: false, data: Optional(OptionalData( @@ -69,7 +71,7 @@ expression: result transactions: [ ExpectedTransactionOutput( tx: "SmartContract(name: my-contract-Epoch3_4-Clarity3, code_body: [..], clarity_version: Some(Clarity3))", - vm_error: "Some(unknown symbol, \'_\') [NON-CONSENSUS BREAKING]", + vm_error: "Some(:0:0: identifier \'_\' starts with \'_\', which requires Clarity 6 or later) [NON-CONSENSUS BREAKING]", return_type: Response(ResponseData( committed: false, data: Optional(OptionalData( @@ -99,7 +101,7 @@ expression: result transactions: [ ExpectedTransactionOutput( tx: "SmartContract(name: my-contract-Epoch3_4-Clarity4, code_body: [..], clarity_version: Some(Clarity4))", - vm_error: "Some(unknown symbol, \'_\') [NON-CONSENSUS BREAKING]", + vm_error: "Some(:0:0: identifier \'_\' starts with \'_\', which requires Clarity 6 or later) [NON-CONSENSUS BREAKING]", return_type: Response(ResponseData( committed: false, data: Optional(OptionalData( @@ -129,7 +131,7 @@ expression: result transactions: [ ExpectedTransactionOutput( tx: "SmartContract(name: my-contract-Epoch3_4-Clarity5, code_body: [..], clarity_version: Some(Clarity5))", - vm_error: "Some(unknown symbol, \'_\') [NON-CONSENSUS BREAKING]", + vm_error: "Some(:0:0: identifier \'_\' starts with \'_\', which requires Clarity 6 or later) [NON-CONSENSUS BREAKING]", return_type: Response(ResponseData( committed: false, data: Optional(OptionalData( diff --git a/stackslib/src/chainstate/tests/static_analysis_tests.rs b/stackslib/src/chainstate/tests/static_analysis_tests.rs index b5717496087..6b4a7060761 100644 --- a/stackslib/src/chainstate/tests/static_analysis_tests.rs +++ b/stackslib/src/chainstate/tests/static_analysis_tests.rs @@ -177,6 +177,7 @@ fn variant_coverage_report(variant: StaticCheckErrorKind) { WithNftExpectedListOfIdentifiers => Tested(vec![static_check_error_with_nft_expected_list_of_identifiers]), MaxIdentifierLengthExceeded(_, _) => Tested(vec![static_check_error_max_identifier_length_exceeded]), TooManyAllowances(_, _) => Tested(vec![static_check_error_too_many_allowances]), + BareUnderscoreReserved => Ignored("Reachable only in Clarity 6+ when `_` appears as a name in a non-discard position. Covered by `clarity::vm::tests::simple_apply_eval` rather than consensus-snapshot tests."), } } diff --git a/stackslib/src/clarity_vm/clarity.rs b/stackslib/src/clarity_vm/clarity.rs index 5d3e44961b2..3733315a1c6 100644 --- a/stackslib/src/clarity_vm/clarity.rs +++ b/stackslib/src/clarity_vm/clarity.rs @@ -2523,9 +2523,9 @@ mod tests { use clarity::types::chainstate::{BurnchainHeaderHash, SortitionId, StacksAddress}; use clarity::vm::analysis::errors::RuntimeCheckErrorKind; use clarity::vm::database::{ClarityBackingStore, STXBalance, SqliteConnection}; + use clarity::vm::representations::ClarityName; use clarity::vm::test_util::{TEST_BURN_STATE_DB, TEST_HEADER_DB}; use clarity::vm::types::{StandardPrincipalData, TupleData, Value}; - use clarity::vm::ClarityName; use stacks_common::consts::CHAIN_ID_TESTNET; use stacks_common::types::chainstate::ConsensusHash; use stacks_common::types::sqlite::NO_PARAMS; diff --git a/stackslib/src/clarity_vm/tests/ephemeral.rs b/stackslib/src/clarity_vm/tests/ephemeral.rs index ef9410c62e3..733d0a6038b 100644 --- a/stackslib/src/clarity_vm/tests/ephemeral.rs +++ b/stackslib/src/clarity_vm/tests/ephemeral.rs @@ -15,8 +15,9 @@ use std::fs; +use clarity::vm::representations::ClarityName; use clarity::vm::types::StacksAddressExtensions; -use clarity::vm::{ClarityName, ContractName}; +use clarity::vm::ContractName; use pinny::tag; use proptest::prelude::*; use rand::seq::SliceRandom; diff --git a/stackslib/src/core/test_util.rs b/stackslib/src/core/test_util.rs index 9d17fdfd998..51cec1962eb 100644 --- a/stackslib/src/core/test_util.rs +++ b/stackslib/src/core/test_util.rs @@ -21,9 +21,10 @@ use clarity::types::chainstate::{ BlockHeaderHash, ConsensusHash, StacksAddress, StacksPrivateKey, StacksPublicKey, }; use clarity::vm::costs::ExecutionCost; +use clarity::vm::representations::ClarityName; use clarity::vm::tests::BurnStateDB; use clarity::vm::types::PrincipalData; -use clarity::vm::{ClarityName, ClarityVersion, ContractName, Value}; +use clarity::vm::{ClarityVersion, ContractName, Value}; use crate::chainstate::stacks::db::StacksChainState; use crate::chainstate::stacks::miner::{BlockBuilderSettings, StacksMicroblockBuilder}; diff --git a/stackslib/src/cost_estimates/tests/cost_estimators.rs b/stackslib/src/cost_estimates/tests/cost_estimators.rs index 0c94e2713d8..47fd848a076 100644 --- a/stackslib/src/cost_estimates/tests/cost_estimators.rs +++ b/stackslib/src/cost_estimates/tests/cost_estimators.rs @@ -16,8 +16,9 @@ use std::env; use clarity::vm::costs::ExecutionCost; +use clarity::vm::representations::ClarityName; use clarity::vm::types::{PrincipalData, StandardPrincipalData}; -use clarity::vm::{ClarityName, ContractName, Value}; +use clarity::vm::{ContractName, Value}; use rand::Rng; use stacks_common::types::chainstate::StacksAddress; use stacks_common::util::hash::{to_hex, Hash160}; diff --git a/stackslib/src/cost_estimates/tests/fee_medians.rs b/stackslib/src/cost_estimates/tests/fee_medians.rs index a12626ebad8..81c6e22dc52 100644 --- a/stackslib/src/cost_estimates/tests/fee_medians.rs +++ b/stackslib/src/cost_estimates/tests/fee_medians.rs @@ -16,7 +16,8 @@ use std::env; use clarity::vm::costs::ExecutionCost; -use clarity::vm::{ClarityName, ContractName, Value}; +use clarity::vm::representations::ClarityName; +use clarity::vm::{ContractName, Value}; use rand::Rng; use stacks_common::types::chainstate::StacksAddress; use stacks_common::util::hash::{to_hex, Hash160}; diff --git a/stackslib/src/cost_estimates/tests/fee_scalar.rs b/stackslib/src/cost_estimates/tests/fee_scalar.rs index b95decc8210..de686d00f29 100644 --- a/stackslib/src/cost_estimates/tests/fee_scalar.rs +++ b/stackslib/src/cost_estimates/tests/fee_scalar.rs @@ -16,8 +16,9 @@ use std::env; use clarity::vm::costs::ExecutionCost; +use clarity::vm::representations::ClarityName; use clarity::vm::types::{PrincipalData, StandardPrincipalData}; -use clarity::vm::{ClarityName, ContractName, Value}; +use clarity::vm::{ContractName, Value}; use rand::seq::SliceRandom; use rand::Rng; use stacks_common::types::chainstate::StacksAddress; diff --git a/stackslib/src/net/api/tests/blockreplay.rs b/stackslib/src/net/api/tests/blockreplay.rs index 751dae89898..e11fbde2eba 100644 --- a/stackslib/src/net/api/tests/blockreplay.rs +++ b/stackslib/src/net/api/tests/blockreplay.rs @@ -17,7 +17,8 @@ use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use clarity::types::chainstate::StacksPrivateKey; -use clarity::vm::{ClarityName, ContractName}; +use clarity::vm::representations::ClarityName; +use clarity::vm::ContractName; use stacks_common::consts::CHAIN_ID_TESTNET; use stacks_common::types::chainstate::StacksBlockId; diff --git a/stackslib/src/net/api/tests/blocksimulate.rs b/stackslib/src/net/api/tests/blocksimulate.rs index 3d513fba225..92f25d33c1b 100644 --- a/stackslib/src/net/api/tests/blocksimulate.rs +++ b/stackslib/src/net/api/tests/blocksimulate.rs @@ -17,8 +17,9 @@ use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use clarity::types::chainstate::StacksPrivateKey; +use clarity::vm::representations::ClarityName; use clarity::vm::types::PrincipalData; -use clarity::vm::{ClarityName, ContractName}; +use clarity::vm::ContractName; use stacks_common::consts::CHAIN_ID_TESTNET; use stacks_common::types::chainstate::StacksBlockId;