Skip to content

fix: fix list_fold pruning too soon#292

Draft
stringhandler wants to merge 1 commit intoBlockstreamResearch:masterfrom
stringhandler:fix/list
Draft

fix: fix list_fold pruning too soon#292
stringhandler wants to merge 1 commit intoBlockstreamResearch:masterfrom
stringhandler:fix/list

Conversation

@stringhandler
Copy link
Copy Markdown
Contributor

NOTE: This code was generated by Claude.

I asked Claude to add some covering tests for list_fold using different lengths. It helpfully identified a bug and fixed it. To be honest, I do not understand this portion of the code well enough to review it properly, but I'm posting it here as a draft PR in case we can review it and understand the problem.

The tests it generated certainly fail with the old code, as shown in #291

Related to #288

@apoelstra
Copy link
Copy Markdown
Contributor

apoelstra commented Apr 22, 2026

I'm extremely skeptical of this fix. If you remove debug symbols and pruning in the test code, you get errors like

thread 'compile::tests::list_fold_one_element_bound4' (1233321) panicked at /home/apoelstra/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/simplicity-lang-0.7.0/src/bit_machine/mod.rs:80:9:

which seems just crazy. I agree with your intuition that we're somehow producing a corrupted program tree, maybe by cloning things we shouldn't. This PR seems to change the fold logic, which is unlikely to improve anything except by accident.

Never mind, I messed up my bit-machine construction. I am still skeptical of this fix.

@apoelstra
Copy link
Copy Markdown
Contributor

You can disable pruning in the test code with

diff --git a/src/lib.rs b/src/lib.rs
index 6e9bca7..832e1f6 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -451,9 +451,12 @@ pub(crate) mod tests {
         fn run(self) -> Result<(), simplicity::bit_machine::ExecutionError> {
             let env = dummy_env::dummy_with(self.lock_time, self.sequence, self.include_fee_output);
             let pruned = self.program.redeem().prune(&env)?;
-            let mut mac = BitMachine::for_program(&pruned)
+            println!("redeem  {:?}", self.program.redeem().display_expr());
+            println!("pruned  {:?}", pruned.display_expr());
+
+            let mut mac = BitMachine::for_program(&self.program.redeem())
                 .expect("program should be within reasonable bounds");
-            mac.exec(&pruned, &env).map(|_| ())
+            mac.exec(&self.program.redeem(), &env).map(|_| ())
         }

         pub fn assert_run_success(self) {

Then the bound4 test passes. So I suspect this is actually a bug in rust-simplicity's pruning logic. I think it should be true that for any Redeem program you can find:

  1. If BitMachine::exec succeeds on that program...
  2. then Redeem::prune should succeed on that program...
  3. and BitMachine::exec should succeed on the output of pruning

assuming the tx environment is the same in all cases (which it is here, though this doesn't even matter since we're not using any tx primitives).

I would suggest adding a fuzztest to rust-simplicity trying to find simple cases where the above fails.

@stringhandler
Copy link
Copy Markdown
Contributor Author

I agree. There may be a bug here, but more importantly, the pruning should fail first.

I'll add the fuzz test

@stringhandler
Copy link
Copy Markdown
Contributor Author

Just an update here. I did look into creating the fuzzing test today. The main difficulty is that very few programs generated in the fuzzers are valid enough to even pass the BitMachine a first time (around 5-6%). I switched strategies to try generate better programs in SimplicityHL, but I saw that it also doesn't generate that many valid programs. I created pr #302 for now but it doesn't often generate programs that are interesting enough to even be pruned.

I plan to continue this next week though and see if I can improve the programs generated in fuzzing to the point that this test is getting actual hits.

@apoelstra
Copy link
Copy Markdown
Contributor

@stringhandler I think you should be able to create the fuzztest in rust-simplicity directly using Simplicity programs, and bypass SimplicityHL entirely. My feeling is that the SimplicityHL fold stuff is a red herring; I think we have a general pruning bug and it just happens that fold is where the HL compiler hits it.

Though your work in #302 is independently valuable.

@apoelstra
Copy link
Copy Markdown
Contributor

I am traveling for the next week but will be back at work after that and can probably help out.

@stringhandler
Copy link
Copy Markdown
Contributor Author

Yeah agree with it being a pruning bug. I'll continue to try get more realistic programs in rust-simplicity and SimplicityHL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants