Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
### Added

- In the `/v3/transaction/{txid}` RPC endpoint, added `block_height` and `is_canonical` to the response.
- Added the `events` array to readonly endpoints output.

## [3.3.0.0.2]

Expand Down
53 changes: 48 additions & 5 deletions clarity/src/vm/contexts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1124,6 +1124,16 @@
&self.global_context.epoch_id
}

pub fn execute_contract_with_events(
&mut self,
contract: &QualifiedContractIdentifier,
tx_name: &str,
args: &[SymbolicExpression],
read_only: bool,
) -> Result<(Value, Vec<StacksTransactionEvent>), VmExecutionError> {
self.inner_execute_contract(contract, tx_name, args, read_only, false)
}

pub fn execute_contract(
&mut self,
contract: &QualifiedContractIdentifier,
Expand All @@ -1132,6 +1142,7 @@
read_only: bool,
) -> Result<Value, VmExecutionError> {
self.inner_execute_contract(contract, tx_name, args, read_only, false)
.map(|(value, _)| value)
}

/// This method is exposed for callers that need to invoke a private method directly.
Expand All @@ -1145,6 +1156,7 @@
read_only: bool,
) -> Result<Value, VmExecutionError> {
self.inner_execute_contract(contract, tx_name, args, read_only, true)
.map(|(value, _)| value)
}

/// This method handles actual execution of contract-calls on a contract.
Expand All @@ -1160,7 +1172,7 @@
args: &[SymbolicExpression],
read_only: bool,
allow_private: bool,
) -> Result<Value, VmExecutionError> {
) -> Result<(Value, Vec<StacksTransactionEvent>), VmExecutionError> {
let contract_size = self
.global_context
.database
Expand Down Expand Up @@ -1208,11 +1220,11 @@
return Err(CheckErrorKind::CircularReference(vec![func_identifier.to_string()]).into())
}
self.call_stack.insert(&func_identifier, true);
let res = self.execute_function_as_transaction(&func, &args, Some(&contract.contract_context), allow_private);
let res = self.execute_function_as_transaction_and_events(&func, &args, Some(&contract.contract_context), allow_private);
self.call_stack.remove(&func_identifier, true)?;

match res {
Ok(value) => {
Ok((value, events)) => {
if let Some(handler) = self.global_context.database.get_cc_special_cases_handler() {
handler(
self.global_context,
Expand All @@ -1224,7 +1236,7 @@
&value
)?;
}
Ok(value)
Ok((value, events))
},
Err(e) => Err(e)
}
Expand All @@ -1238,6 +1250,23 @@
next_contract_context: Option<&ContractContext>,
allow_private: bool,
) -> Result<Value, VmExecutionError> {
let (value, _events) = self.execute_function_as_transaction_and_events(
function,
args,
next_contract_context,
allow_private,
)?;

Ok(value)
}

pub fn execute_function_as_transaction_and_events(
&mut self,
function: &DefinedFunction,
args: &[Value],
next_contract_context: Option<&ContractContext>,
allow_private: bool,
) -> Result<(Value, Vec<StacksTransactionEvent>), VmExecutionError> {
let make_read_only = function.is_read_only();

if make_read_only {
Expand All @@ -1261,11 +1290,25 @@
function.execute_apply(args, &mut nested_env)
};

if make_read_only {
// retrieve all the events
let mut events = vec![];
self.global_context
.event_batches
.iter()
.for_each(|event_batch| {
events.extend(event_batch.events.clone());

Check failure on line 1299 in clarity/src/vm/contexts.rs

View workflow job for this annotation

GitHub Actions / Clippy Check

no field `events` on type `&(vm::contexts::EventBatch, u64)`

Check failure on line 1299 in clarity/src/vm/contexts.rs

View workflow job for this annotation

GitHub Actions / Constants Check / Check the constants from stacks-inspect

no field `events` on type `&(EventBatch, u64)`

Check failure on line 1299 in clarity/src/vm/contexts.rs

View workflow job for this annotation

GitHub Actions / Cargo Hack Check / Fuzz targets (nightly)

no field `events` on type `&(EventBatch, u64)`

Check failure on line 1299 in clarity/src/vm/contexts.rs

View workflow job for this annotation

GitHub Actions / Cargo Hack Check / WASM targets

no field `events` on type `&(EventBatch, u64)`

Check failure on line 1299 in clarity/src/vm/contexts.rs

View workflow job for this annotation

GitHub Actions / Cargo Hack Check / All Crates (Windows/Linux)

no field `events` on type `&(EventBatch, u64)`

Check failure on line 1299 in clarity/src/vm/contexts.rs

View workflow job for this annotation

GitHub Actions / Create Test Cache / Test Archive

no field `events` on type `&(EventBatch, u64)`
});
Comment on lines +1294 to +1300
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having some difficulty reasoning about what this returns. This makes it so that the execute_function_as_transaction_and_events returns the events at the time of the outermost call -- ignoring the result of the transaction or the read-only-ness but only for the outermost call.

Imagine like the following:

(define-read-only (foo)
  (begin (print "foo") (err u1)))
(define-public (bar)
  (begin (print "bar") (foo)))
(define-public (baz)
  (begin (print "baz") (bar)))

Then I think what we'd see is:

execute_function_as_transaction_and_events(foo) -> events: [ "foo" ]
execute_function_as_transaction_and_events(bar) -> events: [ "bar" ]
execute_function_as_transaction_and_events(baz) -> events: [ "baz" ]

Which would be weird, right?

In normal clarity execution, read-only calls won't emit events: they get rolled back. So block events won't include them. With this change, the RPC endpoitn would return block events for those read-only calls, but only the outermost calls. The same logic applies to failing txs (seems bad, even though I'm not sure that failed txs are frequently invoked via the read-only endpoint.).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went ahead and tested this on @rob-stacks's branch:

diff --git a/clarity/src/vm/contexts.rs b/clarity/src/vm/contexts.rs
index 64dab3e1c2..73336a94cc 100644
--- a/clarity/src/vm/contexts.rs
+++ b/clarity/src/vm/contexts.rs
@@ -1009,11 +1009,11 @@ impl<'a, 'b, 'hooks> Environment<'a, 'b, 'hooks> {
         )
     }
 
-    pub fn eval_read_only(
+    fn inner_eval_read_only(
         &mut self,
         contract_identifier: &QualifiedContractIdentifier,
         program: &str,
-    ) -> Result<Value, VmExecutionError> {
+    ) -> Result<(Value, Vec<StacksTransactionEvent>), VmExecutionError> {
         let clarity_version = self.contract_context.clarity_version;
 
         let parsed = ast::build_ast(
@@ -1062,10 +1062,34 @@ impl<'a, 'b, 'hooks> Environment<'a, 'b, 'hooks> {
             let local_context = LocalContext::new();
             eval(&parsed[0], &mut nested_env, &local_context)
         };
+        
+        let mut events = vec![];
+        self.global_context
+            .event_batches
+            .iter()
+            .for_each(|event_batch| {
+                events.extend(event_batch.events.clone());
+            });
 
         self.global_context.roll_back()?;
 
-        result
+        result.map(|value| (value, events))
+    }
+
+    pub fn eval_read_only(
+        &mut self,
+        contract_identifier: &QualifiedContractIdentifier,
+        program: &str,
+    ) -> Result<Value, VmExecutionError> {
+        self.inner_eval_read_only(contract_identifier, program).map(|(value, _)| value)
+    }
+
+    pub fn eval_read_only_with_events(
+        &mut self,
+        contract_identifier: &QualifiedContractIdentifier,
+        program: &str,
+    ) -> Result<(Value, Vec<StacksTransactionEvent>), VmExecutionError> {
+        self.inner_eval_read_only(contract_identifier, program)
     }
 
     pub fn eval_raw(&mut self, program: &str) -> Result<Value, VmExecutionError> {
diff --git a/contrib/clarity-cli/src/lib.rs b/contrib/clarity-cli/src/lib.rs
index b029bb03bd..d884674fb9 100644
--- a/contrib/clarity-cli/src/lib.rs
+++ b/contrib/clarity-cli/src/lib.rs
@@ -1486,20 +1486,23 @@ pub fn invoke_command(invoked_by: &str, args: &[String]) -> (i32, Option<serde_j
             let placeholder_context =
                 ContractContext::new(QualifiedContractIdentifier::transient(), clarity_version);
 
-            let (_, _, result_and_cost) = in_block(header_db, marf_kv, |header_db, mut marf| {
-                let result_and_cost =
+            let (_, _, result_and_events_and_cost) = in_block(header_db, marf_kv, |header_db, mut marf| {
+                let result_and_events_and_cost =
                     with_env_costs(mainnet, epoch, &header_db, &mut marf, None, |vm_env| {
                         vm_env
                             .get_exec_environment(None, None, &placeholder_context)
-                            .eval_read_only(&eval_input.contract_identifier, &eval_input.content)
+                            .eval_read_only_with_events(&eval_input.contract_identifier, &eval_input.content)
                     });
-                (header_db, marf, result_and_cost)
+                (header_db, marf, result_and_events_and_cost)
             });
 
-            match result_and_cost {
-                (Ok(result), cost) => {
+            match result_and_events_and_cost {
+                (Ok((result, events)), cost) => {
+                    let events_json : Vec<_> = events.into_iter().enumerate().map(|(i, ev)| ev.json_serialize(i, &"", true).unwrap()).collect();
+
                     let mut result_json = json!({
                         "output": serde_json::to_value(&result).unwrap(),
+                        "events": events_json,
                         "success": true,
                     });
 

I also expanded your test case:

$ cat /tmp/clarity-events-test.clar 
(define-read-only (foo)
  (begin (print "foo") (ok u1)))
(define-public (bar)
  (begin (print "bar") (foo)))
(define-public (baz)
  (begin (print "baz") (bar)))
(define-public (quux)
  (begin (print "quux") (baz)))
(define-public (xyzzy)
  (begin (print "xyzzy") (quux)))

$ clarity-cli initialize /tmp/clarity-events-test.db
INFO [1765572084.549427] [stackslib/src/chainstate/stacks/index/file.rs:242] [main] Migrate 0 blocks to external blob storage at /tmp/clarity-events-test.db/marf.sqlite.blobs
INFO [1765572084.549446] [stackslib/src/chainstate/stacks/index/file.rs:174] [main] Preemptively vacuuming the database file to free up space after copying trie blobs to a separate file
{"message":"Database created.","network":"mainnet"}
$ clarity-cli launch SP2WS2GQHMARP71QE4T8SZ37BAAGKXV4FBX5ANB8.test /tmp/clarity-events-test.clar /tmp/clarity-events-test.db
$ clarity-cli launch SP2WS2GQHMARP71QE4T8SZ37BAAGKXV4FBX5ANB8.test /tmp/clarity-events-test.clar /tmp/clarity-events-test.db 
{"events":[],"message":"Contract initialized!"}
$ cat /tmp/call.clar 
(xyzzy)
$ clarity-cli eval SP2WS2GQHMARP71QE4T8SZ37BAAGKXV4FBX5ANB8.test /tmp/call.clar /tmp/clarity-events-test.db/ | jq
{
  "events": [
    {
      "committed": true,
      "contract_event": {
        "contract_identifier": "SP2WS2GQHMARP71QE4T8SZ37BAAGKXV4FBX5ANB8.test",
        "raw_value": "0x0d0000000578797a7a79",
        "topic": "print",
        "value": {
          "Sequence": {
            "String": {
              "ASCII": {
                "data": [
                  120,
                  121,
                  122,
                  122,
                  121
                ]
              }
            }
          }
        }
      },
      "event_index": 0,
      "txid": "0x\"\"",
      "type": "contract_event"
    },
    {
      "committed": true,
      "contract_event": {
        "contract_identifier": "SP2WS2GQHMARP71QE4T8SZ37BAAGKXV4FBX5ANB8.test",
        "raw_value": "0x0d0000000471757578",
        "topic": "print",
        "value": {
          "Sequence": {
            "String": {
              "ASCII": {
                "data": [
                  113,
                  117,
                  117,
                  120
                ]
              }
            }
          }
        }
      },
      "event_index": 1,
      "txid": "0x\"\"",
      "type": "contract_event"
    },
    {
      "committed": true,
      "contract_event": {
        "contract_identifier": "SP2WS2GQHMARP71QE4T8SZ37BAAGKXV4FBX5ANB8.test",
        "raw_value": "0x0d0000000362617a",
        "topic": "print",
        "value": {
          "Sequence": {
            "String": {
              "ASCII": {
                "data": [
                  98,
                  97,
                  122
                ]
              }
            }
          }
        }
      },
      "event_index": 2,
      "txid": "0x\"\"",
      "type": "contract_event"
    },
    {
      "committed": true,
      "contract_event": {
        "contract_identifier": "SP2WS2GQHMARP71QE4T8SZ37BAAGKXV4FBX5ANB8.test",
        "raw_value": "0x0d00000003626172",
        "topic": "print",
        "value": {
          "Sequence": {
            "String": {
              "ASCII": {
                "data": [
                  98,
                  97,
                  114
                ]
              }
            }
          }
        }
      },
      "event_index": 3,
      "txid": "0x\"\"",
      "type": "contract_event"
    }
  ],
  "output": {
    "Response": {
      "committed": true,
      "data": {
        "UInt": 1
      }
    }
  },
  "output_serialized": "070100000000000000000000000000000001",
  "success": true
}

Looking at these events, the only event which does not get captured is the innermost event generated by (print "foo"). Meaning:

  • This is a bug -- why is the innermost event dropped?
  • A read-only evaluation from an Environment<_', _'> will result in the GlobalContext capturing all events, except for the inner-most one it seems.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have extended the test contract for having nested calls (and related events generation). As reported by @jcnelson all of the events are reported (in the order they are generated). I was not able to reproduce the issue with the innermost (but probably it was caused by some earlier commit that i reverted in the process)

Copy link
Contributor

@aaronb-stacks aaronb-stacks Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe that this is correct. If you can't replicate this in testing, I think that's because testing is insufficient. The code definitely drops events for innermost read-only calls and error results for transactions (but does not do either of those things for outermost read-only calls and error results.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In more detail, the returned events Vec is not used anywhere but in the API invocations. This means that when transactions are nested at all, the returned events Vec won't be added before the events get rolled back.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at these events, the only event which does not get captured is the innermost event generated by (print "foo"). Meaning:

This is a bug -- why is the innermost event dropped?
A read-only evaluation from an Environment<_', _'> will result in the GlobalContext capturing all events, except for the inner-most one it seems.

Your example is actually different from the one I posted:

(define-read-only (foo) (begin (print "foo") (err u1)))
(define-public (bar) (begin (print "bar") (foo)))
(define-public (baz) (begin (print "baz") (bar)))

Without this PR, I don't think any of these produce events at all -- the read-only function's events are rolled-back when the VM rolls back the read-only execution. bar's invocation is rolled-back because it returns err u1, so the bar event is not emitted. The same is true for baz.

With the changes in this PR, the gathering of events before the rollback for API invocations means that the outermost invocations save those events before they get rolled back. In the case of err results, that seems bad. In the case of read-only results, that seems good because its the point of this PR, but the fact that its only working on the outermost invocations comes down to the way that this PR is gathering events (by pulling them out of the batch a the outermost call, after rollbacks have been applied in internal calls).

Your patch in this thread I think confuses matters further -- none of the examples above produce any events in the CLI with your patch. That's because all of them are rolled back before your patch does any event collection.

The higher-level point here is that the current event system specifically goes through the effort of dropping events whenever mutations are being rolled back (relevant times when this happens: read-only calls, transaction failures, at-block invocations). The changes in this PR (and your patch) only collect events from an outermost call, meaning that any such dropped events won't be picked up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The higher-level point here is that the current event system specifically goes through the effort of dropping events whenever mutations are being rolled back (relevant times when this happens: read-only calls, transaction failures, at-block invocations).

This is actually news to me. Is this API contract spelled out anywhere? Is it covered by unit tests?

The changes in this PR (and your patch) only collect events from an outermost call, meaning that any such dropped events won't be picked up.

Okay, yes, I can see that now, given where all the calls to global_context.roll_back() happen.


Okay, so a new strategy is needed here. @aaronb-stacks would it be okay to add a way to buffer up event batches in GlobalContext that would not be rolled back via roll_back()? This would only be consumed by the RPC endpoint, and these events would not be gathered by default. The caller would need to request it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think my preference here would be add some kind of callback fn/hook to the event batch struct. That way, the RPC endpoint can specify exactly how it wants to deal with events and it won't impact any other callers.


let result = if make_read_only {
self.global_context.roll_back()?;
result
} else {
self.global_context.handle_tx_result(result, allow_private)
};

match result {
Ok(result) => Ok((result, events)),
Err(e) => Err(e),
}
}

Expand Down
15 changes: 15 additions & 0 deletions docs/rpc/components/schemas/read-only-function-result.schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,18 @@ oneOf:
cause:
type: string
description: A string representing the cause of the error.
events:
type: array
description: Contract events generated by the call
items:
type: object
properties:
sender:
type: string
description: The contract address generating the event.
key:
type: string
description: Who generated the event (generally "print").
value:
type: string
description: Hex-encoded Clarity value of the event.
Loading
Loading