Merge branch 'aac-client-breaking' into chore/rename-interpreter-error-to-vm-internal-error

Author: Jiloc

.github/workflows/clippy.yml

@@ -13,6 +13,7 @@ on:
   pull_request:
     branches:
       - develop
+      - aac-client-breaking # temp branch for aac work
     types:
       - opened
       - reopened

clarity-types/src/errors/mod.rs

@@ -59,7 +59,7 @@ pub enum Error {
     /// indicating a mismatch in function entry/exit logic.
     Internal(VmInternalError),
     Runtime(RuntimeErrorType, Option<StackTrace>),
-    ShortReturn(ShortReturnType),
+    EarlyReturn(EarlyReturnError),
 }
 
 /// Represents an internal, unrecoverable error within the Clarity VM.
@@ -148,8 +148,15 @@ pub enum RuntimeErrorType {
 }
 
 #[derive(Debug, PartialEq)]
-pub enum ShortReturnType {
-    ExpectedValue(Box<Value>),
+/// Errors triggered during Clarity contract evaluation that cause early termination.
+/// These errors halt evaluation and fail the transaction.
+pub enum EarlyReturnError {
+    /// Failed to unwrap an `Optional` (`none`) or `Response` (`err` or `ok`) Clarity value.
+    /// The `Box<Value>` holds the original or thrown value. Triggered by `try!`, `unwrap-or`, or
+    /// `unwrap-err-or`.
+    UnwrapFailed(Box<Value>),
+    /// An 'asserts!' expression evaluated to false.
+    /// The `Box<Value>` holds the value provided as the second argument to `asserts!`.
     AssertionFailed(Box<Value>),
 }
 
@@ -166,7 +173,7 @@ impl PartialEq<Error> for Error {
         match (self, other) {
             (Error::Runtime(x, _), Error::Runtime(y, _)) => x == y,
             (Error::Unchecked(x), Error::Unchecked(y)) => x == y,
-            (Error::ShortReturn(x), Error::ShortReturn(y)) => x == y,
+            (Error::EarlyReturn(x), Error::EarlyReturn(y)) => x == y,
             (Error::Internal(x), Error::Internal(y)) => x == y,
             _ => false,
         }
@@ -252,9 +259,9 @@ impl From<(CheckErrors, &SymbolicExpression)> for Error {
     }
 }
 
-impl From<ShortReturnType> for Error {
-    fn from(err: ShortReturnType) -> Self {
-        Error::ShortReturn(err)
+impl From<EarlyReturnError> for Error {
+    fn from(err: EarlyReturnError) -> Self {
+        Error::EarlyReturn(err)
     }
 }
 
@@ -269,11 +276,11 @@ impl From<Error> for () {
     fn from(_err: Error) -> Self {}
 }
 
-impl From<ShortReturnType> for Value {
-    fn from(val: ShortReturnType) -> Self {
+impl From<EarlyReturnError> for Value {
+    fn from(val: EarlyReturnError) -> Self {
         match val {
-            ShortReturnType::ExpectedValue(v) => *v,
-            ShortReturnType::AssertionFailed(v) => *v,
+            EarlyReturnError::UnwrapFailed(v) => *v,
+            EarlyReturnError::AssertionFailed(v) => *v,
         }
     }
 }
@@ -285,15 +292,15 @@ mod test {
     #[test]
     fn equality() {
         assert_eq!(
-            Error::ShortReturn(ShortReturnType::ExpectedValue(Box::new(Value::Bool(true)))),
-            Error::ShortReturn(ShortReturnType::ExpectedValue(Box::new(Value::Bool(true))))
+            Error::EarlyReturn(EarlyReturnError::UnwrapFailed(Box::new(Value::Bool(true)))),
+            Error::EarlyReturn(EarlyReturnError::UnwrapFailed(Box::new(Value::Bool(true))))
         );
         assert_eq!(
             Error::Internal(VmInternalError::InvariantViolation("".to_string())),
             Error::Internal(VmInternalError::InvariantViolation("".to_string()))
         );
         assert!(
-            Error::ShortReturn(ShortReturnType::ExpectedValue(Box::new(Value::Bool(true))))
+            Error::EarlyReturn(EarlyReturnError::UnwrapFailed(Box::new(Value::Bool(true))))
                 != Error::Internal(VmInternalError::InvariantViolation("".to_string()))
         );
     }

clarity-types/src/types/mod.rs

@@ -560,10 +560,10 @@ impl SequenceData {
     pub fn concat(&mut self, epoch: &StacksEpochId, other_seq: SequenceData) -> Result<()> {
         match (self, other_seq) {
             (SequenceData::List(inner_data), SequenceData::List(other_inner_data)) => {
-                inner_data.append(epoch, other_inner_data)
+                inner_data.append(epoch, other_inner_data)?;
             }
             (SequenceData::Buffer(inner_data), SequenceData::Buffer(ref mut other_inner_data)) => {
-                inner_data.append(other_inner_data)
+                inner_data.append(other_inner_data);
             }
             (
                 SequenceData::String(CharType::ASCII(inner_data)),
@@ -573,8 +573,8 @@ impl SequenceData {
                 SequenceData::String(CharType::UTF8(inner_data)),
                 SequenceData::String(CharType::UTF8(ref mut other_inner_data)),
             ) => inner_data.append(other_inner_data),
-            _ => Err(RuntimeErrorType::BadTypeConstruction.into()),
-        }?;
+            _ => return Err(RuntimeErrorType::BadTypeConstruction.into()),
+        };
         Ok(())
     }
 
@@ -1240,9 +1240,8 @@ impl BuffData {
         self.data.as_slice()
     }
 
-    fn append(&mut self, other_seq: &mut BuffData) -> Result<()> {
+    fn append(&mut self, other_seq: &mut BuffData) {
         self.data.append(&mut other_seq.data);
-        Ok(())
     }
 
     pub fn empty() -> Self {
@@ -1279,9 +1278,8 @@ impl ListData {
 }
 
 impl ASCIIData {
-    fn append(&mut self, other_seq: &mut ASCIIData) -> Result<()> {
+    fn append(&mut self, other_seq: &mut ASCIIData) {
         self.data.append(&mut other_seq.data);
-        Ok(())
     }
 
     pub fn len(&self) -> Result<BufferLength> {
@@ -1293,9 +1291,8 @@ impl ASCIIData {
 }
 
 impl UTF8Data {
-    fn append(&mut self, other_seq: &mut UTF8Data) -> Result<()> {
+    fn append(&mut self, other_seq: &mut UTF8Data) {
         self.data.append(&mut other_seq.data);
-        Ok(())
     }
 
     pub fn len(&self) -> Result<BufferLength> {
@@ -1528,12 +1525,11 @@ impl TupleData {
     fn new(
         type_signature: TupleTypeSignature,
         data_map: BTreeMap<ClarityName, Value>,
-    ) -> Result<TupleData> {
-        let t = TupleData {
+    ) -> TupleData {
+        TupleData {
             type_signature,
             data_map,
-        };
-        Ok(t)
+        }
     }
 
     /// Return the number of fields in this tuple value
@@ -1561,7 +1557,7 @@ impl TupleData {
             data_map.insert(name, value);
         }
 
-        Self::new(TupleTypeSignature::try_from(type_map)?, data_map)
+        Ok(Self::new(TupleTypeSignature::try_from(type_map)?, data_map))
     }
 
     // TODO: add tests from mutation testing results #4834
@@ -1581,7 +1577,7 @@ impl TupleData {
             }
             data_map.insert(name, value);
         }
-        Self::new(expected.clone(), data_map)
+        Ok(Self::new(expected.clone(), data_map))
     }
 
     pub fn get(&self, name: &str) -> Result<&Value> {

clarity/src/vm/analysis/type_checker/v2_05/contexts.rs

@@ -173,12 +173,8 @@ impl ContractContext {
         Ok(())
     }
 
-    pub fn add_implemented_trait(
-        &mut self,
-        trait_identifier: TraitIdentifier,
-    ) -> Result<(), CheckError> {
+    pub fn add_implemented_trait(&mut self, trait_identifier: TraitIdentifier) {
         self.implemented_traits.insert(trait_identifier);
-        Ok(())
     }
 
     pub fn get_trait(&self, trait_name: &str) -> Option<&BTreeMap<ClarityName, FunctionSignature>> {

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

@@ -1008,7 +1008,7 @@ impl<'a, 'b> TypeChecker<'a, 'b> {
                 }
                 DefineFunctionsParsed::ImplTrait { trait_identifier } => {
                     self.contract_context
-                        .add_implemented_trait(trait_identifier.clone())?;
+                        .add_implemented_trait(trait_identifier.clone());
                 }
             };
             Ok(Some(()))

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

@@ -61,7 +61,7 @@ impl TraitContext {
         contract_identifier: QualifiedContractIdentifier,
         trait_name: ClarityName,
         trait_signature: BTreeMap<ClarityName, FunctionSignature>,
-    ) -> Result<(), CheckError> {
+    ) {
         match self {
             Self::Clarity1(map) => {
                 map.insert(trait_name, trait_signature);
@@ -77,15 +77,14 @@ impl TraitContext {
                 );
             }
         }
-        Ok(())
     }
 
     pub fn add_used_trait(
         &mut self,
         alias: ClarityName,
         trait_id: TraitIdentifier,
         trait_signature: BTreeMap<ClarityName, FunctionSignature>,
-    ) -> Result<(), CheckError> {
+    ) {
         match self {
             Self::Clarity1(map) => {
                 map.insert(trait_id.name, trait_signature);
@@ -95,7 +94,6 @@ impl TraitContext {
                 all.insert(trait_id, trait_signature);
             }
         }
-        Ok(())
     }
 
     pub fn get_trait(
@@ -293,7 +291,8 @@ impl ContractContext {
             self.contract_identifier.clone(),
             trait_name,
             trait_signature,
-        )
+        );
+        Ok(())
     }
 
     pub fn add_used_trait(
@@ -306,15 +305,12 @@ impl ContractContext {
             self.check_name_used(&alias)?;
         }
 
-        self.traits.add_used_trait(alias, trait_id, trait_signature)
+        self.traits.add_used_trait(alias, trait_id, trait_signature);
+        Ok(())
     }
 
-    pub fn add_implemented_trait(
-        &mut self,
-        trait_identifier: TraitIdentifier,
-    ) -> Result<(), CheckError> {
+    pub fn add_implemented_trait(&mut self, trait_identifier: TraitIdentifier) {
         self.implemented_traits.insert(trait_identifier);
-        Ok(())
     }
 
     pub fn get_trait(

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

@@ -1888,7 +1888,7 @@ impl<'a, 'b> TypeChecker<'a, 'b> {
                         self.add_memory(trait_identifier.name.len().into())?;
                     }
                     self.contract_context
-                        .add_implemented_trait(trait_identifier.clone())?;
+                        .add_implemented_trait(trait_identifier.clone());
                 }
             };
             Ok(Some(()))

clarity/src/vm/ast/definition_sorter/mod.rs

@@ -87,7 +87,7 @@ impl DefinitionSorter {
         )?;
 
         let mut walker = GraphWalker::new();
-        let sorted_indexes = walker.get_sorted_dependencies(&self.graph)?;
+        let sorted_indexes = walker.get_sorted_dependencies(&self.graph);
 
         if let Some(deps) = walker.get_cycling_dependencies(&self.graph, &sorted_indexes) {
             let functions_names = deps
@@ -461,13 +461,12 @@ impl GraphWalker {
     }
 
     /// Depth-first search producing a post-order sort
-    fn get_sorted_dependencies(&mut self, graph: &Graph) -> ParseResult<Vec<usize>> {
+    fn get_sorted_dependencies(&mut self, graph: &Graph) -> Vec<usize> {
         let mut sorted_indexes = Vec::<usize>::new();
         for expr_index in 0..graph.nodes_count() {
             self.sort_dependencies_recursion(expr_index, graph, &mut sorted_indexes);
         }
-
-        Ok(sorted_indexes)
+        sorted_indexes
     }
 
     fn sort_dependencies_recursion(

clarity/src/vm/callables.rs

@@ -294,7 +294,7 @@ impl DefinedFunction {
         match result {
             Ok(r) => Ok(r),
             Err(e) => match e {
-                Error::ShortReturn(v) => Ok(v.into()),
+                Error::EarlyReturn(v) => Ok(v.into()),
                 _ => Err(e),
             },
         }

clarity/src/vm/errors.rs

@@ -15,7 +15,8 @@
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 pub use clarity_types::errors::{
-    Error, IncomparableError, InterpreterResult, RuntimeErrorType, ShortReturnType, VmInternalError,
+    EarlyReturnError, Error, IncomparableError, VmInternalError, InterpreterResult,
+    RuntimeErrorType,
 };
 
 pub use crate::vm::analysis::errors::{