Replace ExecTracker trait and add PruneTracker extension trait
#325
+357
−166
Conversation
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
The tests call `iter.len()` a lot. The default implementation of this method passes through to `iter.size_hint()` and also asserts that both the low and the high estimates are equal. So these tests are pretty thorough.
Returning a concrete type lets us get more trait methods rather than just `Iterator`. In particular we should be able to get `ExactSizeIterator` and `Fuse` which are important for efficiency. We also want to name this type elsewhere, and even the existential one was annoying to type, so we add an alias for it.
This whole type is pub(super). It should not have public methods.
There are three basic operations on the frame stack in the bit machine: creating a new write frame, popping the write stack onto the read stack, and dropping a read frame. Our code uses shorthand like "new_frame" and "drop_frame" which makes it easy to forget which stacks these operations are operating on. This makes it harder to understand the code for somebody who doesn't currently have the bit machine algorithm loaded into their head. Rename to make stuff easier to follow.
apoelstra
force-pushed
the
2025-12/exectracker
branch
from
6a8a265 to
6e34c44
Compare
Closed
uncomputable
previously approved these changes
Dec 7, 2025
|
This implementation is much better than mine! But I also see a minor issue in It would also be helpful to have a default tracker such as |
Collaborator
Author
Ah, great point. Let me what I can do to recover this. I agree it's important to at least notice if we enter a failed jet.
Yeah, good idea. |
Collaborator
Author
|
I think I will replace the output iterator with an enum #[derive(Debug, PartialEq, Eq, Clone)]
enum NodeOutput {
/// Node was not unit, iden, witness or jet, and thus has no currently-available output since its children have not yet run.
NonTerminal,
/// Node was a jet which "failed", i.e. aborted the program
JetFailed,
/// Node succeeded. This is its output frame.
Success(FrameIter),
}And I will defer the error return on This will also let me simplify all these comments which currently warn the user not to use the |
Contributor
|
I believe we can close this PR: #323 in favor of this one I like the change too, the only part that is missing IMO is a "default" (basic) implementation of the tracker that the end dev can use If we can add "default" tracker and mention it in the docs for the |
apoelstra
force-pushed
the
2025-12/exectracker
branch
from
6e34c44 to
e22f8d9
Compare
Contributor
Debug output with |
Contributor
|
Let me try a few things, so I can showcase what I would love to see |
Contributor
|
Now I understand why it looks like this... All I wanted was to see the output as SimplicityHL statements, rather than the actual Bit Machine, because now it is more like assembly tracks rather than high-level call traces Sorry for the confusion |
Contributor
What is a proper way to parse dbg statements? This is what I receive from the debug symbols Though for some reason it is not u32, but something like (bool, u32) when I am reading the Code: if let Ok(input_val) =
SimValue::from_padded_bits(&mut (input.clone()), &node.arrow().source)
{
let inner_ty: Option<ResolvedType> = match tracked_call.name() {
TrackedCallName::Debug(ty) => Some(ty.clone()),
_ => None
};
if let Some(inner_ty) = inner_ty {
let test = StructuralValue::from(input_val.clone());
dbg!(&test, &inner_ty);
let test1 = Value::reconstruct(&test, &ResolvedType::parse_from_str(&format!("(bool, {})", &inner_ty.clone())).unwrap());
dbg!(test1);
};
} |
Contributor
if let Some(tracked_call) = self.debug_symbols.get(cmr) {
let mut input_frame = input.clone();
input_frame.next();
if let Ok(input_val) =
SimValue::from_padded_bits(&mut input_frame, &node.arrow().target)
&& let Some(Either::Right(debug_value)) =
tracked_call.map_value(&StructuralValue::from(input_val))
{
dbg!(debug_value);
}
}this is what I have to do, to be able to read statements wrapped into the This thing that I cannot understand is why I need to do this |
Contributor
|
Here is a PoC implementation of tracker that I could consider a default one for the SimplicityHL: impl ExecTracker<Elements> for DefaultTracker<'_> {
fn visit_node(&mut self, node: &RedeemNode<Elements>, input: FrameIter, output: NodeOutput) {
self.inner.visit_node(node, input.clone(), output.clone());
match node.inner() {
Inner::Jet(jet) => {
if let Some(sink) = self.jet_trace_sink.as_mut()
&& let NodeOutput::Success(output_iter) = output
{
let mut output_frame = output_iter.clone();
output_frame.next();
let parsed_result = Value::reconstruct(
&StructuralValue::from(
SimValue::from_padded_bits(
&mut (output_frame.clone()),
&node.arrow().target,
)
.expect("output from bit machine will always be well-formed"),
),
&target_type(*jet)
.resolve(|_: &AliasName| -> Option<ResolvedType> { None })
.expect("target type of built-in jet always resolves"),
);
let mut input_frame = input.clone();
input_frame.next();
if let Ok(args) = parse_args_from_frame(*jet, &mut input_frame)
&& let Some(result) = parsed_result
{
sink(*jet, &args, &result);
}
}
}
// Handle debug calls (encoded as AssertL with CMR as key into debug symbols)
Inner::AssertL(_, cmr) => {
if let Some(sink) = self.debug_sink.as_mut()
&& let Some(tracked_call) = self.debug_symbols.get(cmr)
{
let mut input_frame = input.clone();
input_frame.next();
if let Ok(input_val) =
SimValue::from_padded_bits(&mut input_frame, &node.arrow().target)
&& let Some(Either::Right(debug_value)) =
tracked_call.map_value(&StructuralValue::from(input_val))
{
sink(debug_value.text(), debug_value.value());
}
}
}
_ => {}
}
}
}
fn parse_args_from_frame(jet: Elements, input_frame: &mut FrameIter) -> Result<Vec<Value>> {
let jet_source_types = source_type(jet);
if jet_source_types.is_empty() {
return Ok(vec![]);
}
let arguments_blob = SimValue::from_padded_bits(input_frame, &jet.source_ty().to_final())
.expect("input from bit machine will always be well-formed");
let mut args = Vec::with_capacity(jet_source_types.len());
collect_args(&arguments_blob.as_ref(), args.capacity(), &mut args)?;
Ok(args
.into_iter()
.zip(jet_source_types.iter())
.map(|(argument, aliased_type)| {
Value::reconstruct(&argument.into(), &resolve_type(aliased_type))
.expect("correct structure of the value of compiled program is guaranteed")
})
.collect())
}
/// Traverses a product and collects the arguments.
fn collect_args(node: &ValueRef, num_args: usize, args: &mut Vec<SimValue>) -> Result<()> {
if num_args == 0 {
return Ok(());
}
if num_args == 1 {
args.push(node.to_value());
Ok(())
} else if let Some((left, right)) = node.as_product() {
args.push(left.to_value());
collect_args(&right, num_args - 1, args)
} else {
Err(anyhow!(
"unexpected value structure while collecting arguments"
))
}
}
fn resolve_type(aliased_type: &AliasedType) -> ResolvedType {
aliased_type
.resolve(|_: &AliasName| -> Option<ResolvedType> { None })
.expect("target type of built-in jet always resolves")
} |
Contributor
|
Here is an example output: |
Collaborator
Author
|
Okay, but this repo is rust-simplicity, not SimplicityHL. |
Collaborator
Author
|
I can certainly improve the |
Contributor
Yeah, though, the thing that we are doing here is going to be used there, so this what I wanted to test
Do you have an idea why I should use |
Collaborator
Author
Okay, but there's nothing I can do in this PR to do this. I can open a draft PR to SimplicityHL (or you can, using your above code). The reason you need to advance a bit is because the |
Collaborator
There is infrastructure in SimplicityHL to reconstruct and display values based on the low-level Simplicity units, sums and pairs. Simplicity is not SimplicityHL, but I think we could group pairs of bits into bit / hex strings. We could go further and group buffer values (aka the sha2 context), too. |
KyrylR
reviewed
Dec 10, 2025
src/bit_machine/tracker.rs
Outdated
Comment on lines
3
to
8
| //! Simplicity Execution | ||
| //! | ||
| //! Implementation of the Bit Machine, without TCO, as TCO precludes some | ||
| //! frame management optimizations which can be used to great benefit. | ||
| //! | ||
Contributor
There was a problem hiding this comment.
Copy pasted from mod.rs? (it seems like it does not belong here)
src/bit_machine/tracker.rs
Outdated
|
|
||
| /// An object which can be used to introspect the execution of the Bit Machine. | ||
| /// | ||
| /// As an example of what you can do with this |
Contributor
There was a problem hiding this comment.
Seems like a cut-off message
src/bit_machine/tracker.rs
Outdated
| /// *pre* ordering. That is, for the program `comp iden unit` the nodes will be visited | ||
| /// in the order `comp`, `iden`, `unit`. | ||
| /// | ||
| /// This method be used for logging, to track left or write accesses of the children of a |
Contributor
There was a problem hiding this comment.
Suggested change
| /// This method be used for logging, to track left or write accesses of the children of a | |
| /// This method should be used for logging, to track left or write accesses of the children of a |
Maybe can instead of should? (not sure)
Also left or write sounds weird
Collaborator
Author
There was a problem hiding this comment.
Yep, thanks. Will do can (and change write to right which I'm pretty sure is what I meant).
Contributor
|
Overall I like the change, it also made a few things clearer for me, thanks |
KyrylR
reviewed
Dec 10, 2025
src/bit_machine/tracker.rs
Outdated
Comment on lines
153
to
157
| ) { | ||
| let input_val = Value::from_padded_bits(&mut input, &node.arrow().source) | ||
| .expect("input from bit machine will always be well-formed"); |
Contributor
There was a problem hiding this comment.
Suggested change
| ) { | |
| let input_val = Value::from_padded_bits(&mut input, &node.arrow().source) | |
| .expect("input from bit machine will always be well-formed"); | |
| ) { | |
| self.inner.visit_node(node, input.clone(), output.clone()); | |
| let input_val = Value::from_padded_bits(&mut input, &node.arrow().source) | |
| .expect("input from bit machine will always be well-formed"); |
Is it ok that we do not have self.inner.visit_node(node, input.clone(), output.clone()); here?
Collaborator
Author
There was a problem hiding this comment.
oo, no, it's not! Good catch.
Comment on lines
68
to
72
| /// Returns true if the left branch of the of the `Case` node with the IHR `ihr` was taken. | ||
| fn contains_left(&self, ihr: Ihr) -> bool; | ||
|
|
||
| /// Returns true if the left branch of the of the `Case` node with the IHR `ihr` was taken. | ||
| fn contains_right(&self, ihr: Ihr) -> bool; |
Contributor
There was a problem hiding this comment.
Suggested change
| /// Returns true if the left branch of the of the `Case` node with the IHR `ihr` was taken. | |
| fn contains_left(&self, ihr: Ihr) -> bool; | |
| /// Returns true if the left branch of the of the `Case` node with the IHR `ihr` was taken. | |
| fn contains_right(&self, ihr: Ihr) -> bool; | |
| /// Returns true if the left branch of the of the `Case` node with the IHR `ihr` was taken. | |
| fn contains_left(&self, ihr: Ihr) -> bool; | |
| /// Returns true if the right branch of the of the `Case` node with the IHR `ihr` was taken. | |
| fn contains_right(&self, ihr: Ihr) -> bool; |
src/bit_machine/tracker.rs
Outdated
| // decoding works. | ||
| let _input_val = Value::from_padded_bits(&mut input, &node.arrow().source); | ||
| if let NodeOutput::Success(mut output) = output { | ||
| let _output_val = Value::from_padded_bits(&mut output, &node.arrow().source); |
Contributor
There was a problem hiding this comment.
Suggested change
| let _output_val = Value::from_padded_bits(&mut output, &node.arrow().source); | |
| let _output_val = Value::from_padded_bits(&mut output, &node.arrow().target); |
Is there any reason to use .source instead of .target?
Collaborator
Author
There was a problem hiding this comment.
Nope, it should be target.
I changed the let _output_val = X construct to X.expect(). Now the tests fail with source and pass with target. Good catch!
Re-export everything so there is no API change. This just tidies up the code a little bit.
This eliminates the private `exec_prune` method which was weirdly-named and no longer makes sense now that we're using a generic tracker rather than a SetTracker.
This replaces the `ExecTracker` API with one which is able to detect every node, not just cases and jets; which is able to read the input value for every node (as a bit iterator which can be converted to a value with Value::from_padded_bits) and the output value for terminal nodes; and which can do all the existing things that the tracker can do. I suspect we want to add some examples or unit tests, in particular around "debug nodes". Fixes BlockstreamResearch#324
apoelstra
force-pushed
the
2025-12/exectracker
branch
from
e22f8d9 to
0b6b6f8
Compare
Collaborator
Author
|
Addressed @KyrylR's comments. |
apoelstra
force-pushed
the
2025-12/exectracker
branch
from
0b6b6f8 to
ed79edf
Compare
KyrylR
approved these changes
Dec 10, 2025
src/bit_machine/tracker.rs
Outdated
| NodeOutput::JetFailed => eprintln!(" JET FAILED"), | ||
| NodeOutput::Success(mut output) => { | ||
| let output_val = Value::from_padded_bits(&mut output, &node.arrow().target) | ||
| .expect("input from bit machine will always be well-formed"); |
Contributor
There was a problem hiding this comment.
Suggested change
| .expect("input from bit machine will always be well-formed"); | |
| .expect("output from bit machine will always be well-formed"); |
Except that, everything else LGTM
Outputting to stderr is not super useful but this can be used as a demo of what the tracker is able to do.
apoelstra
force-pushed
the
2025-12/exectracker
branch
from
ed79edf to
3bf7cef
Compare
Contributor
|
ACK 3bf7cef |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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 the ability for the execution tracker to track every visited node, to view the "input" (top read frame) of every node and the "output" (top write frame after execution) of every terminal node. Allows it to read this data in the form of a bit iterator, which is much easier to work with than the
&[UWORD]that the old jet interface provided. Allows tracking "debug" nodes without special-purpose methods that enable/disable costly conversions.Also adds a
PruneTrackerextension trait which can be used to introspect or manipulate the pruning process.See #323 and #324 for motivation.
Fixes #324.