refactor: Remove WitnessNode

The difference between ConstructNode and WitnessNode was always
confusing.

ConstructNode came earlier and was used mostly for decoding programs.
Its witness nodes were always empty because there are no witness
values during decoding.

WitnessNode was added later for creating satisfied Simplicity policies.
Its witness nodes were potentially filled (Option<Value>) because a
witness value might exist (satisfied fragment) or not exist (unsatisfied
fragment).

The code base for ConstructNode and for WitnessNode is almost identical.
Small changes make both identical and allow us to remove one of them.

In this commit, we set <Construct as Marker>::Witness to Option<Value>
and by copy over the finalization code from WitnessNode. We replace all
code occurrences of `Witness*` with `Construct*`. Finally, we delete
src/node/witness.rs entirely.

Author: uncomputable

src/bit_encoding/decode.rs

@@ -9,7 +9,7 @@ use crate::dag::{Dag, DagLike, InternalSharing};
 use crate::jet::Jet;
 use crate::merkle::cmr::Cmr;
 use crate::node::{
-    ConstructNode, CoreConstructible, DisconnectConstructible, JetConstructible, NoWitness,
+    ConstructNode, CoreConstructible, DisconnectConstructible, JetConstructible,
     WitnessConstructible,
 };
 use crate::types;
@@ -235,7 +235,7 @@ pub fn decode_expression<I: Iterator<Item = u8>, J: Jet>(
                 converted[i].get()?,
                 &Some(Arc::clone(converted[j].get()?)),
             )?),
-            DecodeNode::Witness => Node(ArcNode::witness(&inference_context, NoWitness)),
+            DecodeNode::Witness => Node(ArcNode::witness(&inference_context, None)),
             DecodeNode::Fail(entropy) => Node(ArcNode::fail(&inference_context, entropy)),
             DecodeNode::Hidden(cmr) => {
                 if !hidden_set.insert(cmr) {

src/human_encoding/mod.rs

@@ -13,7 +13,7 @@ mod parse;
 use crate::dag::{DagLike, MaxSharing};
 use crate::jet::Jet;
 use crate::node::{self, CommitNode, NoWitness};
-use crate::{Cmr, Imr, Value, WitnessNode};
+use crate::{Cmr, ConstructNode, Imr, Value};
 
 use std::collections::HashMap;
 use std::str;
@@ -213,9 +213,9 @@ impl<J: Jet> Forest<J> {
     pub fn to_witness_node(
         &self,
         witness: &HashMap<Arc<str>, Value>,
-    ) -> Option<Arc<WitnessNode<J>>> {
+    ) -> Option<Arc<ConstructNode<J>>> {
         let main = self.roots.get("main")?;
-        Some(main.to_witness_node(witness, self.roots()))
+        Some(main.to_construct_node(witness, self.roots()))
     }
 }
 

src/human_encoding/named_node.rs

@@ -6,13 +6,12 @@ use crate::dag::{InternalSharing, MaxSharing, PostOrderIterItem};
 use crate::human_encoding::{Error, ErrorSet, Position, WitnessOrHole};
 use crate::jet::Jet;
 use crate::node::{
-    self, Commit, CommitData, CommitNode, Converter, Inner, NoDisconnect, NoWitness, Node, Witness,
-    WitnessData,
+    self, Commit, CommitData, CommitNode, Construct, ConstructData, Constructible as _, Converter,
+    CoreConstructible as _, Inner, NoDisconnect, NoWitness, Node,
 };
-use crate::node::{Construct, ConstructData, Constructible as _, CoreConstructible as _};
 use crate::types;
 use crate::types::arrow::{Arrow, FinalArrow};
-use crate::{encode, Value, WitnessNode};
+use crate::{encode, ConstructNode, Value};
 use crate::{BitWriter, Cmr, Imr};
 
 use std::collections::HashMap;
@@ -108,19 +107,19 @@ impl<J: Jet> NamedCommitNode<J> {
             .unwrap()
     }
 
-    pub fn to_witness_node(
+    pub fn to_construct_node(
         &self,
         witness: &HashMap<Arc<str>, Value>,
         disconnect: &HashMap<Arc<str>, Arc<NamedCommitNode<J>>>,
-    ) -> Arc<WitnessNode<J>> {
+    ) -> Arc<ConstructNode<J>> {
         struct Populator<'a, J: Jet> {
             witness_map: &'a HashMap<Arc<str>, Value>,
             disconnect_map: &'a HashMap<Arc<str>, Arc<NamedCommitNode<J>>>,
             inference_context: types::Context,
             phantom: PhantomData<J>,
         }
 
-        impl<J: Jet> Converter<Named<Commit<J>>, Witness<J>> for Populator<'_, J> {
+        impl<J: Jet> Converter<Named<Commit<J>>, Construct<J>> for Populator<'_, J> {
             type Error = ();
 
             fn convert_witness(
@@ -140,9 +139,9 @@ impl<J: Jet> NamedCommitNode<J> {
             fn convert_disconnect(
                 &mut self,
                 data: &PostOrderIterItem<&Node<Named<Commit<J>>>>,
-                maybe_converted: Option<&Arc<Node<Witness<J>>>>,
+                maybe_converted: Option<&Arc<Node<Construct<J>>>>,
                 _: &Arc<str>,
-            ) -> Result<Option<Arc<Node<Witness<J>>>>, Self::Error> {
+            ) -> Result<Option<Arc<Node<Construct<J>>>>, Self::Error> {
                 if let Some(converted) = maybe_converted {
                     Ok(Some(converted.clone()))
                 } else {
@@ -172,16 +171,16 @@ impl<J: Jet> NamedCommitNode<J> {
                 &mut self,
                 _: &PostOrderIterItem<&Node<Named<Commit<J>>>>,
                 inner: Inner<
-                    &Arc<Node<Witness<J>>>,
+                    &Arc<Node<Construct<J>>>,
                     J,
-                    &Option<Arc<WitnessNode<J>>>,
+                    &Option<Arc<ConstructNode<J>>>,
                     &Option<Value>,
                 >,
-            ) -> Result<WitnessData<J>, Self::Error> {
+            ) -> Result<ConstructData<J>, Self::Error> {
                 let inner = inner
                     .map(|node| node.cached_data())
                     .map_witness(|maybe_value| maybe_value.clone());
-                Ok(WitnessData::from_inner(&self.inference_context, inner)
+                Ok(ConstructData::from_inner(&self.inference_context, inner)
                     .expect("types are already finalized"))
             }
         }
@@ -260,7 +259,7 @@ impl<J: Jet> NamedConstructNode<J> {
                 .as_ref()
                 .map(|data| &data.cached_data().internal)
                 .map_disconnect(|_| &None)
-                .map_witness(|_| NoWitness),
+                .map_witness(|_| None),
         )?;
         let named_data = NamedConstructData {
             internal: construct_data,

src/human_encoding/parse/mod.rs

@@ -591,7 +591,7 @@ mod tests {
                 assert_eq!(main.cmr().to_string(), cmr);
 
                 let program = main
-                    .to_witness_node(witness, &forest)
+                    .to_construct_node(witness, &forest)
                     .finalize_unpruned()
                     .expect("finalize");
 

src/lib.rs

@@ -65,7 +65,7 @@ pub use crate::merkle::{
     tmr::Tmr,
     FailEntropy, HasCmr,
 };
-pub use crate::node::{CommitNode, ConstructNode, Hiding, RedeemNode, WitnessNode};
+pub use crate::node::{CommitNode, ConstructNode, Hiding, RedeemNode};
 pub use crate::value::{Value, Word};
 pub use simplicity_sys as ffi;
 use std::fmt;

src/node/commit.rs

@@ -3,7 +3,7 @@
 use crate::dag::{DagLike, MaxSharing, NoSharing, PostOrderIterItem};
 use crate::jet::Jet;
 use crate::types::arrow::{Arrow, FinalArrow};
-use crate::{encode, types};
+use crate::{encode, types, Value};
 use crate::{Amr, BitIter, BitWriter, Cmr, Error, FirstPassImr, Imr};
 
 use super::{
@@ -213,8 +213,8 @@ impl<J: Jet> CommitNode<J> {
                 &mut self,
                 _: &PostOrderIterItem<&CommitNode<J>>,
                 _: &NoWitness,
-            ) -> Result<NoWitness, Self::Error> {
-                Ok(NoWitness)
+            ) -> Result<Option<Value>, Self::Error> {
+                Ok(None)
             }
 
             fn convert_disconnect(
@@ -229,7 +229,12 @@ impl<J: Jet> CommitNode<J> {
             fn convert_data(
                 &mut self,
                 _: &PostOrderIterItem<&CommitNode<J>>,
-                inner: Inner<&Arc<ConstructNode<J>>, J, &Option<Arc<ConstructNode<J>>>, &NoWitness>,
+                inner: Inner<
+                    &Arc<ConstructNode<J>>,
+                    J,
+                    &Option<Arc<ConstructNode<J>>>,
+                    &Option<Value>,
+                >,
             ) -> Result<ConstructData<J>, Self::Error> {
                 let inner = inner
                     .map(|node| node.arrow())

src/node/construct.rs

@@ -1,18 +1,17 @@
 // SPDX-License-Identifier: CC0-1.0
 
 use crate::dag::{InternalSharing, PostOrderIterItem};
-use crate::encode;
 use crate::jet::Jet;
 use crate::types::{self, arrow::Arrow};
-use crate::{BitIter, BitWriter, Cmr, FailEntropy};
+use crate::{encode, BitIter, BitWriter, Cmr, FailEntropy, RedeemNode, Value, Word};
 
-use crate::value::Word;
 use std::io;
 use std::marker::PhantomData;
 use std::sync::Arc;
 
 use super::{
     Commit, CommitData, CommitNode, Converter, Inner, Marker, NoDisconnect, NoWitness, Node,
+    Redeem, RedeemData,
 };
 use super::{CoreConstructible, DisconnectConstructible, JetConstructible, WitnessConstructible};
 
@@ -33,7 +32,7 @@ pub struct Construct<J> {
 
 impl<J: Jet> Marker for Construct<J> {
     type CachedData = ConstructData<J>;
-    type Witness = NoWitness;
+    type Witness = Option<Value>;
     type Disconnect = Option<Arc<ConstructNode<J>>>;
     type SharingId = ConstructId;
     type Jet = J;
@@ -90,7 +89,7 @@ impl<J: Jet> ConstructNode<J> {
             fn convert_witness(
                 &mut self,
                 _: &PostOrderIterItem<&ConstructNode<J>>,
-                _: &NoWitness,
+                _: &Option<Value>,
             ) -> Result<NoWitness, Self::Error> {
                 Ok(NoWitness)
             }
@@ -119,6 +118,104 @@ impl<J: Jet> ConstructNode<J> {
         self.convert::<InternalSharing, _, _>(&mut FinalizeTypes(PhantomData))
     }
 
+    /// Finalize the witness program as an unpruned redeem program.
+    ///
+    /// Witness nodes must be populated with sufficient data,
+    /// to ensure that the resulting redeem program successfully runs on the Bit Machine.
+    /// Furthermore, **all** disconnected branches must be populated,
+    /// even those that are not executed.
+    ///
+    /// The resulting redeem program is **not pruned**.
+    ///
+    /// ## See
+    ///
+    /// [`RedeemNode::prune`]
+    pub fn finalize_unpruned(&self) -> Result<Arc<RedeemNode<J>>, crate::Error> {
+        struct Finalizer<J>(PhantomData<J>);
+
+        impl<J: Jet> Converter<Construct<J>, Redeem<J>> for Finalizer<J> {
+            type Error = crate::Error;
+
+            fn convert_witness(
+                &mut self,
+                data: &PostOrderIterItem<&ConstructNode<J>>,
+                wit: &Option<Value>,
+            ) -> Result<Value, Self::Error> {
+                if let Some(ref wit) = wit {
+                    Ok(wit.shallow_clone())
+                } else {
+                    // We insert a zero value into unpopulated witness nodes,
+                    // assuming that this node will later be pruned out of the program.
+                    //
+                    // Pruning requires running a program on the Bit Machine,
+                    // which in turn requires a program with fully populated witness nodes.
+                    // It would be horrible UX to force the caller to provide witness data
+                    // even for unexecuted branches, so we insert zero values here.
+                    //
+                    // If this node is executed after all, then the caller can fix the witness
+                    // data based on the returned execution error.
+                    //
+                    // Zero values may "accidentally" satisfy a program even if the caller
+                    // didn't provide any witness data. However, this is only the case for the
+                    // most trivial programs. The only place where we must be careful is our
+                    // unit tests, which tend to include these kinds of trivial programs.
+                    let ty = data.node.arrow().target.finalize()?;
+                    Ok(Value::zero(&ty))
+                }
+            }
+
+            fn convert_disconnect(
+                &mut self,
+                _: &PostOrderIterItem<&ConstructNode<J>>,
+                maybe_converted: Option<&Arc<RedeemNode<J>>>,
+                _: &Option<Arc<ConstructNode<J>>>,
+            ) -> Result<Arc<RedeemNode<J>>, Self::Error> {
+                if let Some(child) = maybe_converted {
+                    Ok(Arc::clone(child))
+                } else {
+                    Err(crate::Error::DisconnectRedeemTime)
+                }
+            }
+
+            fn convert_data(
+                &mut self,
+                data: &PostOrderIterItem<&ConstructNode<J>>,
+                inner: Inner<&Arc<RedeemNode<J>>, J, &Arc<RedeemNode<J>>, &Value>,
+            ) -> Result<Arc<RedeemData<J>>, Self::Error> {
+                let converted_data = inner
+                    .map(|node| node.cached_data())
+                    .map_disconnect(|node| node.cached_data())
+                    .map_witness(Value::shallow_clone);
+                Ok(Arc::new(RedeemData::new(
+                    data.node.arrow().finalize()?,
+                    converted_data,
+                )))
+            }
+        }
+
+        self.convert::<InternalSharing, _, _>(&mut Finalizer(PhantomData))
+    }
+
+    /// Finalize the witness program as a pruned redeem program.
+    ///
+    /// Witness nodes must be populated with sufficient data,
+    /// to ensure that the resulting redeem program successfully runs on the Bit Machine.
+    /// Furthermore, **all** disconnected branches must be populated,
+    /// even those that are not executed.
+    ///
+    /// The resulting redeem program is **pruned** based on the given transaction environment.
+    ///
+    /// ## See
+    ///
+    /// [`RedeemNode::prune`]
+    pub fn finalize_pruned(
+        &self,
+        env: &J::Environment,
+    ) -> Result<Arc<RedeemNode<J>>, crate::Error> {
+        let unpruned = self.finalize_unpruned()?;
+        unpruned.prune(env).map_err(crate::Error::Execution)
+    }
+
     /// Decode a Simplicity expression from bits, without witness data.
     ///
     /// # Usage
@@ -278,10 +375,10 @@ impl<J: Jet> DisconnectConstructible<Option<Arc<ConstructNode<J>>>> for Construc
     }
 }
 
-impl<J> WitnessConstructible<NoWitness> for ConstructData<J> {
-    fn witness(inference_context: &types::Context, witness: NoWitness) -> Self {
+impl<J> WitnessConstructible<Option<Value>> for ConstructData<J> {
+    fn witness(inference_context: &types::Context, _witness: Option<Value>) -> Self {
         ConstructData {
-            arrow: Arrow::witness(inference_context, witness),
+            arrow: Arrow::witness(inference_context, NoWitness),
             phantom: PhantomData,
         }
     }
@@ -340,7 +437,7 @@ mod tests {
     fn occurs_check_3() {
         let ctx = types::Context::new();
         // A similar example that caused a slightly different deadlock in the past.
-        let wit = Arc::<ConstructNode<Core>>::witness(&ctx, NoWitness);
+        let wit = Arc::<ConstructNode<Core>>::witness(&ctx, None);
         let drop = Arc::<ConstructNode<Core>>::drop_(&wit);
 
         let comp1 = Arc::<ConstructNode<Core>>::comp(&drop, &drop).unwrap();

src/node/mod.rs

@@ -78,7 +78,6 @@ mod display;
 mod hiding;
 mod inner;
 mod redeem;
-mod witness;
 
 use crate::value::Word;
 pub use commit::{Commit, CommitData, CommitNode};
@@ -89,7 +88,6 @@ use display::DisplayExpr;
 pub use hiding::Hiding;
 pub use inner::Inner;
 pub use redeem::{Redeem, RedeemData, RedeemNode};
-pub use witness::{Witness, WitnessData, WitnessNode};
 
 // This trait should only be implemented on empty types, so we can demand
 // every trait bound under the sun. Doing so will make #[derive]s easier