feat: Clarity eval hook call-tracing improvements#7401
Open
cylewitruk-stacks wants to merge 9 commits into
Open
feat: Clarity eval hook call-tracing improvements#7401cylewitruk-stacks wants to merge 9 commits into
cylewitruk-stacks wants to merge 9 commits into
Conversation
…l-hook-tracing-improvements
…l-hook-tracing-improvements
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces an opt-in, higher-level Clarity VM hook API for structured call tracing (call begin/arg eval/call finish plus top-level lifecycle events), and adds a reusable CallTraceHook collector that reconstructs resolved call trees with timing and cost attribution. It also wires the new analysis-time budget parameter through block builder transaction-mining call sites to support per-tx analysis deadlines.
Changes:
- Adds
vm::hooks(newEvalHooksurface + call tracing event model) and instruments the VM evaluator to emit structured call/lifecycle notifications. - Adds
vm::hooks::trace::CallTraceHookto collect call trees with resolved args/results, inclusive/exclusive cost, and wall-clock timing. - Extends block-builder transaction mining to pass an additional per-tx
max_analysis_timebudget and updates affected call sites/tests.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| stackslib/src/net/api/tests/postblock_proposal.rs | Updates postblock proposal API tests to pass the new max_analysis_time argument when mining transactions. |
| stackslib/src/net/api/postblock_proposal.rs | Adds per-tx analysis-time budgeting during proposal validation and threads the new argument through replay/validation mining calls. |
| stackslib/src/net/api/blockreplay.rs | Updates replay mining call to include the new max_analysis_time argument. |
| stackslib/src/clarity_vm/tests/ephemeral.rs | Updates ephemeral replay test mining call to include the new max_analysis_time argument. |
| stackslib/src/clarity_vm/clarity.rs | Adjusts with_abort_callback signature/lifetimes to support hook-aware OwnedEnvironment usage. |
| stackslib/src/chainstate/tests/consensus.rs | Updates test mining calls to include the new max_analysis_time argument. |
| stackslib/src/chainstate/stacks/tests/chain_histories.rs | Updates helper mining calls to include the new max_analysis_time argument. |
| stackslib/src/chainstate/stacks/miner.rs | Extends BlockBuilder APIs to accept max_analysis_time and threads miner settings through mining selection paths. |
| stackslib/src/chainstate/nakamoto/tests/node.rs | Updates Nakamoto test mining calls to include the new max_analysis_time argument. |
| stackslib/src/chainstate/nakamoto/shadow.rs | Updates shadow builder mining calls to include the new max_analysis_time argument. |
| stackslib/src/chainstate/nakamoto/miner.rs | Removes builder-stored analysis deadline in favor of per-call max_analysis_time and forwards it through mining. |
| stackslib/src/chainstate/coordinator/tests.rs | Updates coordinator test mining calls to include the new max_analysis_time argument. |
| stacks-node/src/tests/signer/v0/tenure_extend.rs | Updates signer test mining call to include the new max_analysis_time argument. |
| stacks-node/src/tests/nakamoto_integrations.rs | Updates integration test mining call to include the new max_analysis_time argument. |
| clarity/src/vm/tests/mod.rs | Registers the new hooks test module. |
| clarity/src/vm/tests/hooks.rs | Adds extensive unit tests covering lifecycle events, call tracing, lazy special forms, fold, trait dispatch, nested contract calls, and state reads/writes. |
| clarity/src/vm/mod.rs | Moves EvalHook into vm::hooks, instruments eval/apply paths to emit call/lifecycle hook notifications, and refactors builtin dispatch representation. |
| clarity/src/vm/hooks/mod.rs | Introduces the new public hook API: execution lifecycle + call begin/arg eval/finish notifications and supporting types. |
| clarity/src/vm/hooks/internals.rs | Adds internal notifier plumbing (EvalHookNotifier) and a no-op CallTraceFrame wrapper for near-zero overhead when hooks are absent. |
| clarity/src/vm/hooks/trace.rs | Implements CallTraceHook collector to reconstruct resolved call trees with cost/timing attribution and includes unit tests for cost math helpers. |
| clarity/src/vm/functions/mod.rs | Refactors reserved function lookup to return CallableType::Builtin { clarity_name, kind } and supports call tracing metadata. |
| clarity/src/vm/contexts.rs | Adds hook registration, execution lifecycle notifications, call tracing around entry/contract-call routing, and introduces FunctionExecutionOptions. |
| clarity/src/vm/clarity.rs | Updates TransactionConnection::with_abort_callback signature to support hook-aware OwnedEnvironment<'_, 'hooks>. |
| clarity/src/vm/callables.rs | Refactors callable representation to split builtins into CallableType::Builtin + BuiltinKind, adds call trace hook construction, and exposes function name/arg accessors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…l-hook-tracing-improvements
…l-hook-tracing-improvements
Contributor
Author
|
@hugo-stacks this would be a small breaking change for Clarinet -- worth a look by you guys. Basically, the changes here provide deeper call tracing hooks which allow tx replayers (which use core "append block" flows) to instrument Clarity VM execution more accurately during block/tx replay. |
Contributor
|
Thanks for the heads up @cylewitruk-stacks I'll keep an eye on it |
Contributor
Author
Yeah, it does (with new fn's with default impls). Behavior itself shouldn't be changed for the fn's Clarinet currently uses, but the trait moved into the |
Contributor
|
should this target develop instead? |
…l-hook-tracing-improvements
Comment on lines
+48
to
+58
| /// ```text | ||
| /// ▶ "trace-test.entry" [public] (1 arg) | ||
| /// 𝑎₁ (seed) = u0 | ||
| /// ▶ "trace-test.foo" [private] (2 args) | ||
| /// ↳ eval 𝑎₁ (a) | ||
| /// ▶ "+" [builtin] (2 args) | ||
| /// ◀ "+" ⇒ u42 | ||
| /// 𝑎₁ (a) = u42 | ||
| /// ◀ "trace-test.foo" ⇒ u84 | ||
| /// ◀ "trace-test.entry" ⇒ (ok u84) | ||
| /// ``` |
Comment on lines
+60
to
+64
| /// Called when top-level execution completes. | ||
| /// | ||
| /// **Note:** This is never called by the Clarity VM itself, however `clarinet` currently | ||
| /// implements it internally as part of its REPL interpreter. | ||
| fn did_complete(&mut self, _result: Result<&mut ExecutionResult, String>) {} |
Coverage Report for CI Build 29104084057Coverage increased (+0.9%) to 86.541%Details
Uncovered Changes
Coverage Regressions10177 previously-covered lines in 130 files lost coverage.
Coverage Stats
💛 - Coveralls |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds opt-in Clarity VM hook support for structured call tracing.
Note for reviewers: most of this PR's diff is new tests.
Resolves: https://github.com/stx-labs/core-epics/issues/375
This PR extends EvalHook with:
vm::hooks::trace::CallTraceHook, a reusable collector that reconstructs a call tree with resolved args, results, inclusive/exclusive execution cost, wall time, and nested calls. While fully functional (and public), it was largely used to verify the necessary functionality of the call-trace hooks themselves and serves as a good example for anyone needing to implement their own.Why
stacks-benchand other replay tooling would benefit from understanding which logical VM calls executed during historical transactions, what values they consumed/returned, and where cost was charged. Existing expression-level eval hooks are too low-level for this and require downstream tooling to duplicate VM call-resolution logic.Notes
vm::hooks.did_completeremains unchanged; it is still an external/Clarinet-style completion hook, not emitted by the VM.stackslibblock/transaction-processing code (which is used bystacks-inspectandstacks-bench); that will be in a followup PR -- this is theclarity-side implementation.EvalHookmoved into thevm::hooksnamespace (from a directvm::EvalHookexport, which will break existing consumers -- but it's a simple module path update). I chose to do this to avoidhookstypes polluting the corevmnamespace for opt-in functionality.Testing
fold/step functions, trait-dispatched calls, nested contract calls, and state reads/writes visible through Clarity values.Example output from the debug tracer in tests/hooks.rs: eval_hook_captures_nested_contract_calls_folding_over_contract_calls
Link to test
Checklist
docs/property-testing.md)changelog.d/README.md)