Add consensus tests to RuntimeError and some unit tests to unreachable variants

Signed-off-by: Jacinta Ferrant <[email protected]>

Author: jacinta-stacks

clarity-types/src/types/mod.rs

@@ -1445,6 +1445,10 @@ impl PrincipalData {
         literal: &str,
     ) -> Result<StandardPrincipalData, VmExecutionError> {
         let (version, data) = c32::c32_address_decode(literal).map_err(|x| {
+            // This `TypeParseFailure` is unreachable in normal Clarity execution.
+            // - All principal literals are validated by the Clarity lexer *before* reaching `parse_standard_principal`.
+            // - The lexer rejects any literal containing characters outside the C32 alphabet.
+            // Therefore, only malformed input fed directly into low-level VM entry points can cause this branch to execute.
             RuntimeError::TypeParseFailure(format!("Invalid principal literal: {x}"))
         })?;
         if data.len() != 20 {

clarity/src/vm/contexts.rs

@@ -286,37 +286,52 @@ impl AssetMap {
         }
     }
 
-    // This will get the next amount for a (principal, stx) entry in the stx table.
+    /// This will get the next amount for a (principal, stx) entry in the stx table.
     fn get_next_stx_amount(
         &self,
         principal: &PrincipalData,
         amount: u128,
     ) -> Result<u128, VmExecutionError> {
+        // `ArithmeticOverflow` in this function is **unreachable** in normal Clarity execution because:
+        // - Every `stx-transfer?` or `stx-burn?` is validated against the sender’s
+        //   **unlocked balance** before being queued in `AssetMap`.
+        // - The unlocked balance is a subset of `stx-liquid-supply`.
+        // - All balance updates in Clarity use the `+` operator **before** logging to `AssetMap`.
+        // - `+` performs `checked_add` and returns `RuntimeError::ArithmeticOverflow` **first**.
         let current_amount = self.stx_map.get(principal).unwrap_or(&0);
         current_amount
             .checked_add(amount)
             .ok_or(RuntimeError::ArithmeticOverflow.into())
     }
 
-    // This will get the next amount for a (principal, stx) entry in the burn table.
+    /// This will get the next amount for a (principal, stx) entry in the burn table.
     fn get_next_stx_burn_amount(
         &self,
         principal: &PrincipalData,
         amount: u128,
     ) -> Result<u128, VmExecutionError> {
+        // `ArithmeticOverflow` in this function is **unreachable** in normal Clarity execution because:
+        // - Every `stx-burn?` is validated against the sender’s **unlocked balance** first.
+        // - Unlocked balance is a subset of `stx-liquid-supply`, which is <= `u128::MAX`.
+        // - All balance updates in Clarity use the `+` operator **before** logging to `AssetMap`.
+        // - `+` performs `checked_add` and returns `RuntimeError::ArithmeticOverflow` **first**.
         let current_amount = self.burn_map.get(principal).unwrap_or(&0);
         current_amount
             .checked_add(amount)
             .ok_or(RuntimeError::ArithmeticOverflow.into())
     }
 
-    // This will get the next amount for a (principal, asset) entry in the asset table.
+    /// This will get the next amount for a (principal, asset) entry in the asset table.
     fn get_next_amount(
         &self,
         principal: &PrincipalData,
         asset: &AssetIdentifier,
         amount: u128,
     ) -> Result<u128, VmExecutionError> {
+        // `ArithmeticOverflow` in this function is **unreachable** in normal Clarity execution because:
+        // - The inner transaction must have **partially succeeded** to log any assets.
+        // - All balance updates in Clarity use the `+` operator **before** logging to `AssetMap`.
+        // - `+` performs `checked_add` and returns `RuntimeError::ArithmeticOverflow` **first**.
         let current_amount = self
             .token_map
             .get(principal)
@@ -1011,6 +1026,13 @@ impl<'a, 'b, 'hooks> Environment<'a, 'b, 'hooks> {
         .expressions;
 
         if parsed.is_empty() {
+            // `TypeParseFailure` is **unreachable** in standard Clarity VM execution.
+            // - `eval_read_only` parses a raw program string into an AST.
+            // - Any empty or invalid program would be rejected at publish/deploy time or earlier parsing stages.
+            // - Therefore, `parsed.is_empty()` cannot occur for programs originating from a valid contract
+            // or transaction.
+            // - Only malformed input fed directly to this internal method (e.g., in unit tests or
+            // artificial VM invocations) can trigger this error.
             return Err(RuntimeError::TypeParseFailure(
                 "Expected a program of at least length 1".to_string(),
             )
@@ -1060,6 +1082,12 @@ impl<'a, 'b, 'hooks> Environment<'a, 'b, 'hooks> {
         .expressions;
 
         if parsed.is_empty() {
+            // `TypeParseFailure` is **unreachable** in standard Clarity VM execution.
+            // - `eval_raw` parses a raw program string into an AST.
+            // - All programs deployed or called via the standard VM go through static parsing and validation first.
+            // - Any empty or invalid program would be rejected at publish/deploy time or earlier parsing stages.
+            // - Therefore, `parsed.is_empty()` cannot occur for a program that originates from a valid Clarity contract or transaction.
+            // Only malformed input directly fed to this internal method (e.g., in unit tests) can trigger this error.
             return Err(RuntimeError::TypeParseFailure(
                 "Expected a program of at least length 1".to_string(),
             )
@@ -1940,6 +1968,14 @@ impl<'a> LocalContext<'a> {
 
     pub fn extend(&'a self) -> Result<LocalContext<'a>, VmExecutionError> {
         if self.depth >= MAX_CONTEXT_DEPTH {
+            // `MaxContextDepthReached` in this function is **unreachable** in normal Clarity execution because:
+            // - Every function call in Clarity increments both the call stack depth and the local context depth.
+            // - The VM enforces `MAX_CALL_STACK_DEPTH` (currently 64) **before** `MAX_CONTEXT_DEPTH` (256).
+            // - This means no contract can create more than 64 nested function calls, preventing context depth from reaching 256.
+            // - Nested expressions (`let`, `begin`, `if`, etc.) increment context depth, but the Clarity parser enforces
+            //   `ExpressionStackDepthTooDeep` long before MAX_CONTEXT_DEPTH nested contexts can be written.
+            // - As a result, `MaxContextDepthReached` can only occur in artificial Rust-level tests calling `LocalContext::extend()`,
+            //   not in deployed contract execution.
             Err(RuntimeError::MaxContextDepthReached.into())
         } else {
             Ok(LocalContext {
@@ -2292,4 +2328,108 @@ mod test {
             TypeSignature::CallableType(CallableSubtype::Trait(trait_id))
         );
     }
+
+    #[test]
+    fn asset_map_arithmetic_overflows() {
+        let a_contract_id = QualifiedContractIdentifier::local("a").unwrap();
+        let b_contract_id = QualifiedContractIdentifier::local("b").unwrap();
+        let p1 = PrincipalData::Contract(a_contract_id.clone());
+        let p2 = PrincipalData::Contract(b_contract_id.clone());
+        let t1 = AssetIdentifier {
+            contract_identifier: a_contract_id,
+            asset_name: "a".into(),
+        };
+
+        let mut am1 = AssetMap::new();
+        let mut am2 = AssetMap::new();
+
+        // Token transfer: add u128::MAX followed by 1 to overflow
+        am1.add_token_transfer(&p1, t1.clone(), u128::MAX).unwrap();
+        assert!(matches!(
+            am1.add_token_transfer(&p1, t1.clone(), 1).unwrap_err(),
+            VmExecutionError::Runtime(RuntimeError::ArithmeticOverflow, _)
+        ));
+
+        // STX burn: add u128::MAX followed by 1 to overflow
+        am1.add_stx_burn(&p1, u128::MAX).unwrap();
+        assert!(matches!(
+            am1.add_stx_burn(&p1, 1).unwrap_err(),
+            VmExecutionError::Runtime(RuntimeError::ArithmeticOverflow, _)
+        ));
+
+        // STX transfer: add u128::MAX followed by 1 to overflow
+        am1.add_stx_transfer(&p1, u128::MAX).unwrap();
+        assert!(matches!(
+            am1.add_stx_transfer(&p1, 1).unwrap_err(),
+            VmExecutionError::Runtime(RuntimeError::ArithmeticOverflow, _)
+        ));
+
+        // commit_other: merge two maps where sum exceeds u128::MAX
+        am2.add_token_transfer(&p1, t1.clone(), u128::MAX).unwrap();
+        assert!(matches!(
+            am1.commit_other(am2).unwrap_err(),
+            VmExecutionError::Runtime(RuntimeError::ArithmeticOverflow, _)
+        ));
+    }
+
+    #[test]
+    fn eval_raw_empty_program() {
+        // Setup environment
+        let mut tl_env_factory = tl_env_factory();
+        let mut env = tl_env_factory.get_env(StacksEpochId::latest());
+
+        // Call eval_read_only with an empty program
+        let program = ""; // empty program triggers parsed.is_empty()
+        let err = env.eval_raw(program).unwrap_err();
+
+        assert!(
+            matches!(
+            err,
+            VmExecutionError::Runtime(RuntimeError::TypeParseFailure(msg), _) if msg.contains("Expected a program of at least length 1")),
+            "Expected a type parse failure"
+        );
+    }
+
+    #[test]
+    fn eval_read_only_empty_program() {
+        // Setup environment
+        let mut tl_env_factory = tl_env_factory();
+        let mut env = tl_env_factory.get_env(StacksEpochId::latest());
+
+        // Construct a dummy contract context
+        let contract_id = QualifiedContractIdentifier::local("dummy-contract").unwrap();
+
+        // Call eval_read_only with an empty program
+        let program = ""; // empty program triggers parsed.is_empty()
+        let err = env.eval_read_only(&contract_id, program).unwrap_err();
+
+        assert!(
+            matches!(
+            err,
+            VmExecutionError::Runtime(RuntimeError::TypeParseFailure(msg), _) if msg.contains("Expected a program of at least length 1")),
+            "Expected a type parse failure"
+        );
+    }
+
+    #[test]
+    fn max_context_depth_exceeded() {
+        let root = LocalContext {
+            function_context: None,
+            parent: None,
+            callable_contracts: HashMap::new(),
+            variables: HashMap::new(),
+            depth: MAX_CONTEXT_DEPTH - 1,
+        };
+        // We should be able to extend once successfully.
+        let result = root.extend().unwrap();
+        // We are now at the MAX_CONTEXT_DEPTH and should fail.
+        let result_2 = result.extend();
+        assert!(matches!(
+            result_2,
+            Err(VmExecutionError::Runtime(
+                RuntimeError::MaxContextDepthReached,
+                _
+            ))
+        ));
+    }
 }

clarity/src/vm/costs/cost_functions.rs

@@ -171,6 +171,10 @@ pub fn linear(n: u64, a: u64, b: u64) -> u64 {
 }
 pub fn logn(n: u64, a: u64, b: u64) -> Result<u64, VmExecutionError> {
     if n < 1 {
+        // This branch is **unreachable** in standard Clarity execution:
+        // - `logn` is only called from tuple access operations.
+        // - Tuples must have at least one field, so `n >= 1` is always true (this is enforced via static checks).
+        // - Hitting this branch requires manual VM manipulation or internal test harnesses.
         return Err(VmExecutionError::Runtime(
             RuntimeError::Arithmetic("log2 must be passed a positive integer".to_string()),
             Some(vec![]),

clarity/src/vm/database/clarity_db.rs

@@ -944,6 +944,10 @@ impl<'a> ClarityDatabase<'a> {
 
     pub fn decrement_ustx_liquid_supply(&mut self, decr_by: u128) -> Result<(), VmExecutionError> {
         let current = self.get_total_liquid_ustx()?;
+        // This `ArithmeticUnderflow` is **unreachable** in normal Clarity execution.
+        // The sender's balance is always checked first (`amount <= sender_balance`),
+        // and `sender_balance <= current_supply` always holds.
+        // Thus, `decr_by > current_supply` cannot occur.
         let next = current.checked_sub(decr_by).ok_or_else(|| {
             error!("`stx-burn?` accepted that reduces `ustx-liquid-supply` below 0");
             RuntimeError::ArithmeticUnderflow
@@ -2091,6 +2095,10 @@ impl ClarityDatabase<'_> {
         })?;
 
         if amount > current_supply {
+            // `SupplyUnderflow` is **unreachable** in normal Clarity execution:
+            // the sender's balance is checked first (`amount <= sender_balance`),
+            // and `sender_balance <= current_supply` always holds.
+            // Thus, `amount > current_supply` cannot occur.
             return Err(RuntimeError::SupplyUnderflow(current_supply, amount).into());
         }
 
@@ -2411,3 +2419,142 @@ impl ClarityDatabase<'_> {
         Ok(epoch.epoch_id)
     }
 }
+
+#[test]
+fn increment_ustx_liquid_supply_overflow() {
+    use crate::vm::database::MemoryBackingStore;
+    use crate::vm::errors::{RuntimeError, VmExecutionError};
+
+    let mut store = MemoryBackingStore::new();
+    let mut db = store.as_clarity_db();
+
+    db.begin();
+    // Set the liquid supply to one less than the max
+    db.set_ustx_liquid_supply(u128::MAX - 1)
+        .expect("Failed to set liquid supply");
+    // Trust but verify.
+    assert_eq!(
+        db.get_total_liquid_ustx().unwrap(),
+        u128::MAX - 1,
+        "Supply should now be u128::MAX - 1"
+    );
+
+    db.increment_ustx_liquid_supply(1)
+        .expect("Increment by 1 should succeed");
+
+    // Trust but verify.
+    assert_eq!(
+        db.get_total_liquid_ustx().unwrap(),
+        u128::MAX,
+        "Supply should now be u128::MAX"
+    );
+
+    // Attempt to overflow
+    let err = db.increment_ustx_liquid_supply(1).unwrap_err();
+    assert!(matches!(
+        err,
+        VmExecutionError::Runtime(RuntimeError::ArithmeticOverflow, _)
+    ));
+
+    // Verify adding 0 doesn't overflow
+    db.increment_ustx_liquid_supply(0)
+        .expect("Increment by 0 should succeed");
+
+    assert_eq!(db.get_total_liquid_ustx().unwrap(), u128::MAX);
+
+    db.commit().unwrap();
+}
+
+#[test]
+fn checked_decrease_token_supply_underflow() {
+    use crate::vm::database::{MemoryBackingStore, StoreType};
+    use crate::vm::errors::{RuntimeError, VmExecutionError};
+
+    let mut store = MemoryBackingStore::new();
+    let mut db = store.as_clarity_db();
+    let contract_id = QualifiedContractIdentifier::transient();
+    let token_name = "token".to_string();
+
+    db.begin();
+
+    // Set initial supply to 1000
+    let key =
+        ClarityDatabase::make_key_for_trip(&contract_id, StoreType::CirculatingSupply, &token_name);
+    db.put_data(&key, &1000u128)
+        .expect("Failed to set initial token supply");
+
+    // Trust but verify.
+    let current_supply: u128 = db.get_data(&key).unwrap().unwrap();
+    assert_eq!(current_supply, 1000, "Initial supply should be 1000");
+
+    // Decrease by 500: should succeed
+    db.checked_decrease_token_supply(&contract_id, &token_name, 500)
+        .expect("Decreasing by 500 should succeed");
+
+    let new_supply: u128 = db.get_data(&key).unwrap().unwrap();
+    assert_eq!(new_supply, 500, "Supply should now be 500");
+
+    // Decrease by 0: should succeed (no change)
+    db.checked_decrease_token_supply(&contract_id, &token_name, 0)
+        .expect("Decreasing by 0 should succeed");
+    let supply_after_zero: u128 = db.get_data(&key).unwrap().unwrap();
+    assert_eq!(
+        supply_after_zero, 500,
+        "Supply should remain 500 after decreasing by 0"
+    );
+
+    // Attempt to decrease by 501; should trigger SupplyUnderflow
+    let err = db
+        .checked_decrease_token_supply(&contract_id, &token_name, 501)
+        .unwrap_err();
+
+    assert!(
+        matches!(
+            err,
+            VmExecutionError::Runtime(RuntimeError::SupplyUnderflow(500, 501), _)
+        ),
+        "Expected SupplyUnderflow(500, 501), got: {err:?}"
+    );
+
+    // Supply should remain unchanged after failed underflow
+    let final_supply: u128 = db.get_data(&key).unwrap().unwrap();
+    assert_eq!(
+        final_supply, 500,
+        "Supply should not change after underflow error"
+    );
+
+    db.commit().unwrap();
+}
+
+#[test]
+fn trigger_no_such_token_rust() {
+    use crate::vm::database::MemoryBackingStore;
+    use crate::vm::errors::{RuntimeError, VmExecutionError};
+    // Set up a memory backing store and Clarity database
+    let mut store = MemoryBackingStore::default();
+    let mut db = store.as_clarity_db();
+
+    db.begin();
+    // Define a fake contract identifier
+    let contract_id = QualifiedContractIdentifier::transient();
+
+    // Simulate querying a non-existent NFT
+    let asset_id = Value::Bool(false); // this token does not exist
+    let asset_name = "test-nft";
+
+    // Call get_nft_owner directly
+    let err = db
+        .get_nft_owner(
+            &contract_id,
+            asset_name,
+            &asset_id,
+            &TypeSignature::BoolType,
+        )
+        .unwrap_err();
+
+    // Assert that it produces NoSuchToken
+    assert!(
+        matches!(err, VmExecutionError::Runtime(RuntimeError::NoSuchToken, _)),
+        "Expected NoSuchToken. Got: {err}"
+    );
+}