Merge #159: Fix array_fold "powers of 2" bug and turn on serde by default

f64ba1f array_fold: a few minor cleanups (Andrew Poelstra)
75b93b0 array_fold: add regression test for power-of-2 size tree array folding (Andrew Poelstra)
21f6ff8 array_fold: handle powers of two correctly (Andrew Poelstra)
be22f90 turn on 'serde' feature by default (Andrew Poelstra)
17e4937 simc: add explicit error when trying to use a .wit file without serde (Andrew Poelstra)

Pull request description:

  The serde thing is unrelated but it was super simple so I threw it in here. Can split it out if someone complains.
  
  The main part of this PR is the 4-line patch
  
  ```diff
  --- a/src/compile/builtins.rs
  +++ b/src/compile/builtins.rs
  @@ -21,6 +21,6 @@
           f_powers_of_two: &Vec<ProgNode>,
       ) -> Result<ProgNode, simplicity::types::Error> {
  -        if n == 1 {
  -            return Ok(f_powers_of_two[0].clone());
  +        if n.is_power_of_two() {
  +            return Ok(f_powers_of_two[n.ilog2() as usize].clone());
           }
           // For n > 1 the next largest power is always >= 0
  ```
  
  Essentially, in `array_fold` we start by computing the folds for a bunch of power-of-2 array sizes, then use these as building blocks to do non-powers-of-2 in a sort of unbalanced tree algorithm. However, if our array size was exactly a power of 2 (or, I suspect, a multiple of any power of 2 except 1..), we would attempt to fold a power-of-2 tree with a 0-sized tree, which led to an assertion failure. In these cases we shouldn't be folding at all; we should be looking at our precomputed fold.
  
  I put this simple patch in one commit, and the following commit refactors the logic a bit to make this clearer.
  
  Fixes #144 
  Fixes #153


ACKs for top commit:
  canndrew:
    ACK f64ba1f


Tree-SHA512: 6f37e26d2b3be2631d19d3d93b6e4912260669c4185ed40b17081e9a1d576564cca48397f21ffa76faa77cd40d8c4cad65a5af285e46322c8c3a47d3825db9a7

Author: apoelstra

Cargo.toml

@@ -18,6 +18,7 @@ name = "simc"
 path = "src/main.rs"
 
 [features]
+default = [ "serde" ]
 serde = ["dep:serde", "dep:serde_json"]
 
 [dependencies]

examples/array_fold_2n.simf

@@ -0,0 +1,12 @@
+// From https://github.com/BlockstreamResearch/SimplicityHL/issues/153
+
+fn sum(elt: u32, acc: u32) -> u32 {
+    let (_, acc): (bool, u32) = jet::add_32(elt, acc);
+    acc
+}
+
+fn main() {
+    let arr: [u32; 8] = [1, 1, 1, 1, 1, 1, 1, 1];
+    let sum: u32 = array_fold::<sum, 8>(arr, 0);
+    assert!(jet::eq_32(sum, 8));
+}

src/compile/builtins.rs

@@ -1,4 +1,5 @@
 use std::num::NonZeroUsize;
+use std::sync::Arc;
 
 use simplicity::node::CoreConstructible;
 
@@ -18,16 +19,20 @@ pub fn array_fold(size: NonZeroUsize, f: &ProgNode) -> Result<ProgNode, simplici
     /// Recursively fold the array using the precomputed folding functions.
     fn tree_fold(
         n: usize,
-        f_powers_of_two: &Vec<ProgNode>,
+        f_powers_of_two: &[ProgNode],
     ) -> Result<ProgNode, simplicity::types::Error> {
-        if n == 1 {
-            return Ok(f_powers_of_two[0].clone());
-        }
-        // For n > 1 the next largest power is always >= 0
+        // Array is a left-balanced (right-associative) binary tree.
         let max_pow2 = n.ilog2() as usize;
+        debug_assert!(max_pow2 < f_powers_of_two.len());
+        let f_right = &f_powers_of_two[max_pow2];
+
+        // If the tree is balanced, return precomputed solution.
         let size_right = 1 << max_pow2;
-        // Array is a left-balanced (right-associative) binary tree.
-        let f_right = f_powers_of_two.get(max_pow2).expect("max_pow2 OOB");
+        if n == size_right {
+            return Ok(Arc::clone(f_right));
+        }
+        debug_assert!(size_right < n);
+
         let f_left = tree_fold(n - size_right, f_powers_of_two)?;
         f_array_fold(&f_left, f_right)
     }
@@ -51,7 +56,7 @@ pub fn array_fold(size: NonZeroUsize, f: &ProgNode) -> Result<ProgNode, simplici
 
     // Precompute the folding functions for arrays of size 2^i where i < n.
     let n = size.get();
-    let mut f_powers_of_two: Vec<ProgNode> = Vec::with_capacity(n.ilog2() as usize);
+    let mut f_powers_of_two: Vec<ProgNode> = Vec::with_capacity(1 + n.ilog2() as usize);
 
     // An array of size 1 is just the element itself, so f_array_fold_1 is the same as the folding function.
     let mut f_prev = f.clone();
@@ -60,7 +65,7 @@ pub fn array_fold(size: NonZeroUsize, f: &ProgNode) -> Result<ProgNode, simplici
     let mut i = 1;
     while i < n {
         f_prev = f_array_fold(&f_prev, &f_prev)?;
-        f_powers_of_two.push(f_prev.clone());
+        f_powers_of_two.push(Arc::clone(&f_prev));
         i *= 2;
     }
 

src/lib.rs

@@ -436,6 +436,13 @@ pub(crate) mod tests {
             .assert_run_success();
     }
 
+    #[test]
+    fn regression_153() {
+        TestCase::program_file("./examples/array_fold_2n.simf")
+            .with_witness_values(WitnessValues::default())
+            .assert_run_success();
+    }
+
     #[test]
     #[cfg(feature = "serde")]
     fn sighash_non_interactive_fee_bump() {

src/main.rs

@@ -33,7 +33,6 @@ fn run() -> Result<(), String> {
             )
     };
 
-    #[cfg(feature = "serde")]
     let command = {
         command.arg(
             Arg::new("wit_file")
@@ -75,7 +74,15 @@ fn run() -> Result<(), String> {
             .transpose()?
     };
     #[cfg(not(feature = "serde"))]
-    let witness_opt: Option<simplicityhl::WitnessValues> = None;
+    let witness_opt: Option<simplicityhl::WitnessValues> = {
+        if matches.contains_id("wit_file") {
+            return Err(
+                "Program was compiled without the 'serde' feature and cannot process .wit files."
+                    .into(),
+            );
+        }
+        None
+    };
 
     if let Some(witness) = witness_opt {
         let satisfied = compiled.satisfy(witness)?;