fix: fix list_fold pruning too soon#292
fix: fix list_fold pruning too soon#292stringhandler wants to merge 1 commit intoBlockstreamResearch:masterfrom
Conversation
|
I'm extremely skeptical of this fix.
Never mind, I messed up my bit-machine construction. I am still skeptical of this fix. |
|
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
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. |
|
I agree. There may be a bug here, but more importantly, the pruning should fail first. I'll add the fuzz test |
|
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. |
|
@stringhandler I think you should be able to create the fuzztest in Though your work in #302 is independently valuable. |
|
I am traveling for the next week but will be back at work after that and can probably help out. |
|
Yeah agree with it being a pruning bug. I'll continue to try get more realistic programs in rust-simplicity and SimplicityHL |
NOTE: This code was generated by Claude.
I asked Claude to add some covering tests for
list_foldusing 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