Merge BlockstreamResearch#209: Better human

05c81ba Test: Witness nodes have exactly one name (Christian Lewe)
61c5227 Test: Human encoding: Refactor parsing (Christian Lewe)
b0c0ef7 Test: Human encoding: Filled holes (Christian Lewe)
e8e82ac Test: Human encoding: Filled witness (Christian Lewe)
b8ccec8 Test: Human encoding: Refactor finalization (Christian Lewe)
9c45abf Test: Human encoding: Move parsing test (Christian Lewe)
8a5fbbc Doc: Forest::to_witness_node (Christian Lewe)
37d728c Human encoding: Store hole name in named disc nodes (Christian Lewe)
f207d6e Derive traits (Christian Lewe)

Pull request description:

  Fixes BlockstreamResearch#181

ACKs for top commit:
  apoelstra:
    ACK 05c81ba

Tree-SHA512: c73d49cf16985d99c2b1a6086be8398a036d1367eaf7e9bd00308c58f1887e54c8bb288b0188470908d443804b3860bd271deb7373daeb29ac2fe82616a906db

Author: apoelstra

src/dag.rs

@@ -14,6 +14,7 @@ use crate::node::{self, Disconnectable, Node};
 /// as whether they represent `witness` or `disconnect` combinators, which
 /// are treated specially when working with the graph structure of
 /// Simplicty programs.
+#[derive(Clone, Debug)]
 pub enum Dag<T> {
     /// Combinator with no children
     Nullary,
@@ -347,6 +348,7 @@ pub trait DagLike: Sized {
 /// To avoid confusion, this structure cannot be directly costructed.
 /// Instead it is implicit in the [`DagLike::rtl_post_order_iter`]
 /// method.
+#[derive(Clone, Debug)]
 pub struct SwapChildren<D>(D);
 
 impl<D: DagLike> DagLike for SwapChildren<D> {
@@ -510,6 +512,7 @@ pub struct PostOrderIter<D, S> {
 }
 
 /// A set of data yielded by a `PostOrderIter`
+#[derive(Clone, Debug)]
 pub struct PostOrderIterItem<D> {
     /// The actual node data
     pub node: D,

src/human_encoding/mod.rs

@@ -13,7 +13,7 @@ mod serialize;
 
 use crate::dag::{DagLike, MaxSharing};
 use crate::jet::Jet;
-use crate::node::{self, CommitNode};
+use crate::node::{self, CommitNode, NoWitness};
 use crate::{Cmr, Imr, Value, WitnessNode};
 
 use std::collections::HashMap;
@@ -22,7 +22,6 @@ use std::sync::Arc;
 
 pub use self::error::{Error, ErrorSet};
 pub use self::named_node::NamedCommitNode;
-pub use self::parse::parse;
 
 /// Line/column pair
 ///
@@ -77,6 +76,12 @@ impl WitnessOrHole {
     }
 }
 
+impl<'a> From<&'a NoWitness> for WitnessOrHole {
+    fn from(_: &NoWitness) -> Self {
+        WitnessOrHole::Witness
+    }
+}
+
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Forest<J: Jet> {
     roots: HashMap<Arc<str>, Arc<NamedCommitNode<J>>>,
@@ -136,7 +141,6 @@ impl<J: Jet> Forest<J> {
         let mut witness_lines = vec![];
         let mut const_lines = vec![];
         let mut program_lines = vec![];
-        let mut disc_idx = 0;
         // Pass 1: compute string data for every node
         for root in self.roots.values() {
             for data in root.as_ref().post_order_iter::<MaxSharing<_>>() {
@@ -161,9 +165,8 @@ impl<J: Jet> Forest<J> {
                 } else if let node::Inner::AssertL(_, cmr) = node.inner() {
                     expr_str.push_str(" #");
                     expr_str.push_str(&cmr.to_string());
-                } else if let node::Inner::Disconnect(_, node::NoDisconnect) = node.inner() {
-                    expr_str.push_str(&format!(" ?disc_expr_{}", disc_idx));
-                    disc_idx += 1;
+                } else if let node::Inner::Disconnect(_, hole_name) = node.inner() {
+                    expr_str.push_str(&format!(" ?{}", hole_name));
                 }
 
                 let arrow = node.arrow();
@@ -205,6 +208,9 @@ impl<J: Jet> Forest<J> {
         ret
     }
 
+    /// Convert the forest into a witness node.
+    ///
+    /// Succeeds if the forest contains a "main" root and returns `None` otherwise.
     pub fn to_witness_node(
         &self,
         witness: &HashMap<Arc<str>, Arc<Value>>,
@@ -217,99 +223,160 @@ impl<J: Jet> Forest<J> {
 #[cfg(test)]
 mod tests {
     use crate::human_encoding::Forest;
-    use crate::jet::Core;
-    use crate::{Error, Value};
+    use crate::jet::{Core, Jet};
+    use crate::{BitMachine, Value};
     use std::collections::HashMap;
     use std::sync::Arc;
 
-    #[test]
-    fn to_witness_node_missing_witness() {
-        let s = "
-            wit1 := witness : 1 -> 2^32
-            wit2 := witness : 1 -> 2^32
+    fn assert_finalize_ok<J: Jet>(
+        s: &str,
+        witness: &HashMap<Arc<str>, Arc<Value>>,
+        env: &J::Environment,
+    ) {
+        let program = Forest::<J>::parse(s)
+            .expect("Failed to parse human encoding")
+            .to_witness_node(witness)
+            .expect("Forest is missing expected root")
+            .finalize()
+            .expect("Failed to finalize");
+        let mut mac = BitMachine::for_program(&program);
+        mac.exec(&program, env).expect("Failed to run program");
+    }
 
-            wits_are_equal := comp (pair wit1 wit2) jet_eq_32 : 1 -> 2
-            main := comp wits_are_equal jet_verify            : 1 -> 1
-        ";
-        let forest = Forest::<Core>::parse(s).unwrap();
-        let mut witness = HashMap::new();
-        witness.insert(Arc::from("wit1"), Value::u32(1337));
-
-        match forest.to_witness_node(&witness).unwrap().finalize() {
-            Ok(_) => panic!("Insufficient witness map should fail"),
-            Err(Error::IncompleteFinalization) => {}
-            Err(error) => panic!("Unexpected error {}", error),
+    fn assert_finalize_err<J: Jet>(
+        s: &str,
+        witness: &HashMap<Arc<str>, Arc<Value>>,
+        env: &J::Environment,
+        err_msg: &'static str,
+    ) {
+        let program = match Forest::<J>::parse(s)
+            .expect("Failed to parse human encoding")
+            .to_witness_node(witness)
+            .expect("Forest is missing expected root")
+            .finalize()
+        {
+            Ok(program) => program,
+            Err(error) => {
+                assert_eq!(&error.to_string(), err_msg);
+                return;
+            }
+        };
+        let mut mac = BitMachine::for_program(&program);
+        match mac.exec(&program, env) {
+            Ok(_) => panic!("Execution is expected to fail"),
+            Err(error) => assert_eq!(&error.to_string(), err_msg),
         }
     }
 
     #[test]
-    fn parse_duplicate_witness_in_disconnected_branch() {
+    fn filled_witness() {
         let s = "
-            wit1 := witness
-            main := comp wit1 comp disconnect iden ?dis2 unit
-
-            wit1 := witness
-            dis2 := wit1
+            a := witness
+            b := witness
+            main := comp
+                comp
+                    pair a b
+                    jet_lt_8
+                jet_verify
         ";
 
-        match Forest::<Core>::parse(s) {
-            Ok(_) => panic!("Duplicate witness names should fail"),
-            Err(set) => {
-                let errors: Vec<_> = set.iter().collect();
-                assert_eq!(1, errors.len());
-                match errors[0] {
-                    super::error::Error::NameRepeated { .. } => {}
-                    error => panic!("Unexpected error {}", error),
-                }
-            }
-        }
+        let a_less_than_b = HashMap::from([
+            (Arc::from("a"), Value::u8(0x00)),
+            (Arc::from("b"), Value::u8(0x01)),
+        ]);
+        assert_finalize_ok::<Core>(s, &a_less_than_b, &());
+
+        let b_greater_equal_a = HashMap::from([
+            (Arc::from("a"), Value::u8(0x01)),
+            (Arc::from("b"), Value::u8(0x01)),
+        ]);
+        assert_finalize_err::<Core>(s, &b_greater_equal_a, &(), "Jet failed during execution");
     }
 
     #[test]
-    fn to_witness_node_unfilled_hole() {
-        let s = "
-            wit1 := witness
-            main := comp wit1 comp disconnect iden ?dis2 unit
-        ";
-        let forest = Forest::<Core>::parse(s).unwrap();
-        let witness = HashMap::new();
-
-        match forest.to_witness_node(&witness).unwrap().finalize() {
-            Ok(_) => panic!("Duplicate witness names should fail"),
-            Err(Error::IncompleteFinalization) => {}
-            Err(error) => panic!("Unexpected error {}", error),
-        }
+    fn unfilled_witness() {
+        let witness = HashMap::from([(Arc::from("wit1"), Value::u32(1337))]);
+        assert_finalize_err::<Core>(
+            "
+                wit1 := witness : 1 -> 2^32
+                wit2 := witness : 1 -> 2^32
+
+                wits_are_equal := comp (pair wit1 wit2) jet_eq_32 : 1 -> 2
+                main := comp wits_are_equal jet_verify            : 1 -> 1
+            ",
+            &witness,
+            &(),
+            "unable to satisfy program",
+        );
     }
 
     #[test]
-    fn to_witness_node_pruned_witness() {
+    fn unfilled_witness_pruned() {
         let s = "
             wit1 := witness
             wit2 := witness
             main := comp (pair wit1 unit) case unit wit2
         ";
-        let forest = Forest::<Core>::parse(s).unwrap();
-        let mut witness = HashMap::new();
-        witness.insert(Arc::from("wit1"), Value::u1(0));
-
-        match forest.to_witness_node(&witness).unwrap().finalize() {
-            Ok(_) => {}
-            Err(e) => panic!("Unexpected error {}", e),
-        }
+        let wit2_is_pruned = HashMap::from([(Arc::from("wit1"), Value::u1(0))]);
+        assert_finalize_ok::<Core>(s, &wit2_is_pruned, &());
+
+        let wit2_is_missing = HashMap::from([(Arc::from("wit1"), Value::u1(1))]);
+        // FIXME The finalization should fail
+        // This doesn't happen because we don't run the program,
+        // so we cannot always determine which nodes must be pruned
+        assert_finalize_err::<Core>(
+            s,
+            &wit2_is_missing,
+            &(),
+            "Execution reached a pruned branch: bf12681a76fc7c00c63e583c25cc97237337d6aca30d3f4a664075445385c648"
+        );
+
+        let wit2_is_present = HashMap::from([
+            (Arc::from("wit1"), Value::u1(1)),
+            (Arc::from("wit2"), Value::unit()),
+        ]);
+        assert_finalize_ok::<Core>(s, &wit2_is_present, &());
+    }
 
-        witness.insert(Arc::from("wit1"), Value::u1(1));
+    #[test]
+    fn filled_hole() {
+        let empty = HashMap::new();
+        assert_finalize_ok::<Core>(
+            "
+                id1 := iden : 2^256 * 1 -> 2^256 * 1
+                main := comp (disconnect id1 ?hole) unit
+                hole := unit
+            ",
+            &empty,
+            &(),
+        );
+    }
 
-        match forest.to_witness_node(&witness).unwrap().finalize() {
-            Ok(_) => {}
-            Err(Error::IncompleteFinalization) => {}
-            Err(e) => panic!("Unexpected error {}", e),
-        }
+    #[test]
+    fn unfilled_hole() {
+        let empty = HashMap::new();
+        assert_finalize_err::<Core>(
+            "
+                wit1 := witness
+                main := comp wit1 comp disconnect iden ?dis2 unit
+            ",
+            &empty,
+            &(),
+            "unable to satisfy program",
+        );
+    }
 
-        witness.insert(Arc::from("wit2"), Value::unit());
+    #[test]
+    fn witness_name_override() {
+        let s = "
+            wit1 := witness
+            wit2 := wit1
+            main := comp wit2 iden
+        ";
+        let wit1_populated = HashMap::from([(Arc::from("wit1"), Value::unit())]);
+        assert_finalize_err::<Core>(s, &wit1_populated, &(), "unable to satisfy program");
 
-        match forest.to_witness_node(&witness).unwrap().finalize() {
-            Ok(_) => {}
-            Err(e) => panic!("Unexpected error {}", e),
-        }
+        let wit2_populated = HashMap::from([(Arc::from("wit2"), Value::unit())]);
+        assert_finalize_ok::<Core>(s, &wit2_populated, &());
     }
 }